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

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

> Von: Slawek [mailto:sgp@telsatgp.com.pl]
> 
> Hello!
> In message to "Andrew Lunn" <andrew@lunn.ch> sent Wed, 20 Oct 
> 2004 18:11:18
> +0200 you wrote:
> 
> NA> #define EFIS_VALID       (0xa5a5)
> NA> #define EFIS_IN_PROGRESS (0xfdfd)
> NA> #define EFIS_EMPTY       (0xffff)
> 
> Some additional ideas from somebody who watches the conversation:
> 
> 1) Why do we need EFIS_IN_PROGRESS? Isn't EFIS_EMPTY enough? 
> Both can't be used to load the application anyway.

Hmmm, ok. With IN_PROGRESS differing from 0xffff it is possible to decide whether the writing process has already started. Ok, this doesn't help that much...
 
> 2) ".FisValid" suggest this is valid FIS entry while it 
> doesn't need to be.
> Why don't use separate name for valid and for invalid (empty 
> or in progress)
> fis tables? This could also save additional space used to mark
> "valid/invalid" as this could be decided be the name.

At the moment ".FisValid" is read from the fis table it is considered to be valid.
This can't be used to separate valid and invalid since the change from ".Invalid" to ".Valid" would need an extra erase operation in between, which would kill the purpose of this whole thing.

Andrew: in your patch you removed the magic_name field from the valid_info struct. While this works, it requires quite a lot more typing:

struct fis_image_desc* img0=(struct fis_image_desc*)buf0;
struct fis_image_desc* img1=(struct fis_image_desc*)buf1;
struct fis_valid_info* fvi0
     =(struct fis_valid_info*)img0->name+sizeof(CYG_REDBOOT_VALID_MAGIC);
struct fis_valid_info* fvi1
     =(struct fis_valid_info*)img1->name+sizeof(CYG_REDBOOT_VALID_MAGIC);

instead of:

struct fis_valid_info* fvi0 =(struct fis_valid_info*)buf0;
struct fis_valid_info* fvi1 =(struct fis_valid_info*)buf1;

which IMHO makes it much easier to mix things up and introduce bugs, while it doesn't help against the padding/alignment problem (AFAIK).
This doesn't only apply to the redboot code, but even more to the fisfs implementation, there this has to be done more often.

So I'd like to modify this structure like this:

#define CYG_REDBOOT_VALID_MAGIC        ".FisValid"
#define CYG_REDBOOT_VALID_MAGIC_LENGTH 10

#define CYG_REDBOOT_RFIS_VALID       0xa5
//maybe get rid of one of both:
#define CYG_REDBOOT_RFIS_IN_PROGRESS 0xfd
#define CYG_REDBOOT_RFIS_EMPTY       0xff

struct fis_valid_info
{
   char magic_name[CYG_REDBOOT_VALID_MAGIC_LENGTH];
   unsigned char valid_flag1;
   unsigned char valid_flag2
   unsigned long version_count:
};

Still the size can be checked with
CYG_ASSERT(sizeof(struct fis_valid_info) <=sizeof(struct fis_image_desc), "size mismatch");

What do you think ?

Bye
Alex