AW: AW: contributing a failsafe update meachanism for FIS from within ecos applications

["Neundorf, Alexander" <Alexander dot Neundorf at jenoptik dot com>]

> Von: Andrew Lunn [mailto:andrew@lunn.ch]
> 
> On Thu, Oct 28, 2004 at 01:32:46PM +0200, Neundorf, Alexander wrote:
> > For this I'd like to add a function which simply returns a pointer
> > to the current valid fis table. Then this can be iterated (and
> > modified) in "userspace".
> 
> I don't like that. You are spreading around the knowledge of what the
> FIS image looks like, how you operate on it etc. Thats bad engineering
> practice. There currently exists an abstraction on top of this which
> does most of what you need. Just extend the abstraction a little.

You mean hal_if.c flash_fis_op() ?
Instead of returning struct fis_image_desc it returns the members of a fis_image_desc one by one. So the app still has to know which members a fis_image_desc has.
How about a VV which returns the number of fis_image_desc's in the table and a VV something like this:

int fill_image_desc(int index, struct fis_image_desc* img)
{
   /* fill img from the contents of the fis table, can be memcpy(),
   could also be a member-by-member translation */
   memcpy(img, fis_table[index], sizeof(fis_image_desc));
   return 0;
}

This way one would not have to call 8 functions to collect all members for one entry. And there would be the possibility to insert a "translation" in get_image_desc(), so that the struct returned there and the layout on the flash don't have to be identical.
But since currently fis_image_desc is the used layout, it would IMO make sense to use this struct also for this purpose. Otherwise a new struct with more or less the same members has to be defined.

Some similar function for changing the entries and the two functions for starting and finishing the update. Still the contents of the entries given to the modification functions (flash_base etc.) would have to be filled by the application, preferably with manually defined fixed addresses. This is one of the main points, we (the user/developer) have the chance to define exactly at which address on the flash the image will be placed, we can rely on this, there is no magic happening in between which collects garbage or searches free chunks or whatever. It is simple and easy to understand what's going on. That's essential for our purpose.
 
> > I didn't plan to open files for writing. They can only be opened for
> > reading, and additionally there is a set of functions (eraseImage(),
> > createImage()) which can modify the contents of the fis. If any file
> > is opened when one of these functions is called, it returns with an
> > error. IMO the fisfs shouldn't try to act like a real RW file
> > system, for this we have jffs2, but like an RO file system,
> > accompanied by some utility functions to update the contents of this
> > RO filesystem.
> 
> I guess your createImage() call accepts the complete image. Many
> systems don't have enought RAM to be able to hold the complete image
> when doing an upgrade, but they do have enought RAM to hold one flash
> block. What they want to do is download the new image from a server a
> packet at a time and pass it to the filesystem to write to the
> flash. The filesystem caches the writes until it has a complete flash
> block and then writes it to flash.
> 
> How do you handle this case with your createImage() call?  

Currently not at all and for our solution we definitely won't put any filesystem cache/logic  in between (I already mentioned earlier ;-)
Anyway, if the createImage() function doesn't come from redboot (as you suggest if I understand correctly), there's nothing which hinders implementing writing block by block. 
 
> My solution seems much more natural if you are using the filesystem
> metaphore.
> 
> > Well, actually mount() is the point where the interesting things
> > happen, i.e. deciding which fis table to use I'd say.
> 
> Why. Redboot already knows that. Its just another example of you

Well, from the application POV the mount() call decides about the valid fis table. For the application it doesn't matter whether this is done in application code, via VV in redboot by examing the flash blocks again or via a VV by redboot just returning what redboot already knows. I'm also just not yet so deep into VV's to know everything what can be done there.

So how about this:
erase:
app to redboot: I want to erase foo
redboot: erases foo from the fis table and marks it as in progress
app: erases foo contents from the flash
app to redboot: I'm done with it
redboot: mark it valid

create:
app to redboot: I want to create "foo" at this and that address and that's the crc (all parameters preferably transferred in one function call, be it a fis_image_desc or something else)
redboot: creates foo in the fis table and marks it as in progress
app: writes foo contents on the flash, all at once or block by block
app to redboot: I'm done with it
redboot: mark it valid

create with backup:
app to redboot: I want to create "foo" at this and that address and that's the crc, and please rename existing foo to foo~
redboot: creates foo in the fis table, renames previous foo to foo~ and marks the table as in progress
app: writes new foo contents on the flash
app to redboot: I'm done with it
redboot: mark it valid

This last one renaming step is the key for the firmware update, it enables safe updating without having to modify the flash config boot script. If the process succeeds, redboot will start the new image the next time, if it fails, it will still start the old image again since still the old table will be valid.

Bye
Alex