OSTC3 firmware upgrades

[Anton Lundin]

On 15 November, 2014 - Jef Driesen wrote:

> On 14-11-14 18:01, Anton Lundin wrote:
> >This is is a RFC series of patches to add support for firmware upgrades
> >on the OSTC3. The code have bin tested both against a "pcb" by HW and
> >against my OSTC3 so it works and does the right things. JeanDo's
> >ostc-companion[1] have bin the reference implementation I've looked at
> >when I wrote this code.
> >
> >There are still a couple of pain-points that I would like some feedback
> >on:
> 
> I still need to have a closer look at all the implementation details. But
> let's already start with the higher level questions.
> 
> >How to start the device in service mode:
> >To get access to the necessary commands for service mode, we can't INIT
> >the device with INIT(0xBB), we need to INIT it with a special service
> >mode sequence(0xAA, 0xAB, 0xCD, 0xEF). Now the INIT is sent in
> >hw_ostc3_device_open, so how do you suggest that we refactor the code to
> >allow another sequence after open?
> >
> >The current quite crude hack is to just EXIT(0xFF) the device and
> >re-init it after a 6s sleep. This won't work against the OSTC Sport.
> 
> I'm not really sure what would be the best solution here, so I'm just
> providing some ideas here:
> 
> One solution would be to remove the INIT from open, and maintain the current
> state (none, download or service) in the device handle. Initially the state
> would be "none". Then, in each of the public functions, we check the state
> and if it's "none", we automatically enter download or service mode. If
> we're in the wrong state, we fail an error.
> 
> Another alternative is to have a second variant of the open function that
> takes a parameter to indicate which mode you want. The tricky part here, is
> that the high-level dc_device_open() function won't know about this
> function, so it will always call the download variant. Thus to flash the
> firmware, an application would be forced to use the new variant. (Note that
> after v0.5 I intend to remove the vendor_product_device_open() functions
> from the public api and require applications to use the generic
> dc_device_open() function instead. But the service mode variant can stay as
> a vendor specific function, just like the hw_ostc3_device_fwupdate function
> itself.)
> 
> Yet another option is to remove the dc_device_t parameter from the
> hw_ostc3_device_fwupdate function, and let it open the device itself. Then
> we call the service mode variant of the open function internally.
> 
> On the other hand, I noticed some comment that the key is different for the
> ostc sport. So that means we somehow have to detect the right model. This is
> done with the version command, but I'm not sure if you can use this command
> in service mode. So we may need to identify the exact model in download mode
> first and then switch to service mode anyway. You say this won't work for
> the sport, so what's the problem there? Is that maybe because it exits
> bluetooth mode, and thus the serial port disappears? (I haven't used a sport
> myself, so I simply don't know.)
> 
> Right now, I have a preference towards the second option, with a separate
> open function for the service mode. It looks the least complex to me. But
> I'm not ruling out other options yet.
> 

I was more looking at how the api for hw_ostc_device_fwupdate looks and
just created a equivalent api.

For me it really doesn't matter, but it makes sense that the both
hw_ostc_device_fwupdate and hw_ostc3_device_fwupdate works the same way.

> >The timing between command and data in write_block:
> >The current hw_ostc3_device_write_block is a bit of copy-paste from
> >hw_ostc3_transfer, to be able to send the data buffer to the device,
> >without a additional copy.
> 
> I wouldn't worry about an extra data copy. That's inexpensive compared to
> the actual I/O. But based on the comments, I guess the real problem is
> something else? Could it be that it seems there is no echo for the
> WRITE_BLOCK command?
> 

The echo comes after the data block. I've bin playing with sending it as
one write too, but for some reason the device doesn't like that and
programs nothing.

I think the magic is the serial_sleep (device->port, 2) between the
WRITE_BLOCK + address and the data.

Maybe its not that bad, its only one function that has some special
properties that can't use hw_ostc3_transfer.

> >Library triggered closed state:
> >The device reboots when it gets the FLASH_FIRM-command so how can we
> >"not fail" when the application then tries to talk to the device
> >afterwards, or even call hw_ostc3_device_close?
> 
> I assume that the problem is that after the FLASH_FIRM command, we don't
> need to send the EXIT command in hw_ostc3_device_close? Or did you mean
> something else?
> 

Exactly that. This is just the cousin to the open/service-mode problem.

> >There are also a couple of other FIXME's in the code, which I plan on
> >cleaning up, but I thought that I should send out a first RFC patch
> >series to get some additional feedback going.
> >
> >
> >Any other ideas about how the code looks?
> 
> I noticed a large number of sleep and flush calls. Are those really
> necessary? I have the feeling some of them were just copy and pasted.
> 

Not really copy-pasted, but re-implemented the same ones as where in
ostc-companion. I've removed most of them by now, and the ones left
there kinda needs to be there.


//Anton


-- 
Anton Lundin	+46702-161604