[PATCH 08/11] Add code to write rom of the OSTC3

[Anton Lundin]

On 17 December, 2014 - Jef Driesen wrote:

> On 16-12-14 20:06, Anton Lundin wrote:
> >On 16 December, 2014 - Jef Driesen wrote:
> >
> >>On 2014-11-21 21:28, Anton Lundin wrote:
> >>>+static dc_status_t
> >>>+hw_ostc3_device_write_block (dc_device_t *abstract, unsigned int
> >>>addr, unsigned char block[], unsigned int block_size)
> >>>+{
> >>>+	dc_status_t rc = DC_STATUS_SUCCESS;
> >>>+	hw_ostc3_device_t *device = (hw_ostc3_device_t *) abstract;
> >>>+	dc_context_t *context = (abstract ? abstract->context : NULL);
> >>>+#if 0
> >>>+	// FIXME: This variant doens't work
> >>>+	// Things doesn't get written to memory. Timing issues?
> >>>+	// Version below works, serial_sleep (2) is probably the thing.
> >>>+	unsigned char buffer[4 + block_size];
> >>>+	buffer[0] = WRITE_BLOCK;
> >>>+	uint24_be_array(addr, buffer + 1);
> >>>+	memcpy(buffer + 4, block, block_size);
> >>>+
> >>>+	rc = hw_ostc3_transfer(device, NULL, WRITE_BLOCK, buffer,
> >>>sizeof(buffer), NULL, 0);
> >>>+	serial_sleep (device->port, 1100);
> >>>+	return rc;
> >>>+#else
> >...
> >>
> >>With the exception of those sleep calls, that's identical to the
> >>hw_ostc3_transfer() function. Notice how your code is missing the read of
> >>the echo. I'm pretty sure that got lost due to the flushing. So this
> >>probably works correctly only by accident.
> >>
> >>The reason why hw_ostc3_transfer doesn't work for you, might be because you
> >>include the WRITE_BLOCK byte in the input buffer. That causes it to send
> >>twice. The ostc probably doesn't like that. Can you have a look at this? I
> >>have the feeling that we can use hw_ostc3_transfer here after all. Worst
> >>case we may need to add those sleep calls there, wrapped in a "if (cmd ==
> >>WRITE_BLOCK)".
> >>
> >>>+	n = serial_read (device->port, &response, sizeof (response));
> >>>+	if (n != sizeof (response) || response != S_READY)
> >>>+		return EXITCODE (n);
> >>
> >>You should split those checks in two parts. Right now you'll return
> >>DC_STATUS_TIMEOUT when response!=S_READY, and that's misleading. If we can
> >>re-use hw_ostc3_transfer, then this is no longer an issue.
> >>
> >
> >After i started to rewrite the code for like the .. 7th time i spotted
> >the bug in the hw_ostc3_transfer-based variant, and indeed it was the
> >extra WRITE_BLOCK i never spotted.
> >
> >Now the hw_ostc3_device_write_block-helper looks like this:
> >
> >static dc_status_t
> >hw_ostc3_device_write_block (hw_ostc3_device_t *device, unsigned int addr, unsigned char block[], unsigned int block_size)
> >{
> >	unsigned char buffer[3 + block_size];
> >	array_uint24_be_set(buffer, addr);
> >	memcpy(buffer + 3, block, block_size);
> >
> >	return hw_ostc3_transfer(device, NULL, WRITE_BLOCK, buffer, sizeof(buffer), NULL, 0);
> >}
> >
> >
> >Just like hw_ostc3_device_read_block, and works just fine.
> 
> Excellent news! Just one comment. Use a fixed size array here. Variable
> Length Arrays (VLA) are not supported in C++, so this breaks msvc
> compatibility :-(
> 

I constrained to only being capable of writing SZ_FIRMWARE_BLOCK sized
blocks.

> There are also a few other minor issues for msvc. See the attached patch.
> 

Fixed.

//Anton

-- 
Anton Lundin	+46702-161604