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]
...
> > And that's why I changed from 
> > unsigned short valid_flag;
> > to 
> > unsigned char valid_flag1;
> > unsigned char valid_flag2;
> 
> The pointer to the structure is still unaligned which for me makes it
> potentially dangerous when access members of the structure.

You mean the structure i.e. the buffer in which the fis table is read itself ?
Yes, this is indeed right.
 
> > Maybe something with a union would make it more explicit ?
> > 
> > union
> > {
> >    struct fis_image_desc img;
> >    struct fis_valid_info valid;
> > } give_me_a_name;
> 
> I was thinking along the same lines, but slightly differently...
> 
> struct fis_image_desc {
>     union {
>         name[16];
>         struct fis_valid_info valid;
>     } u;    
>     CYG_ADDRESS   flash_base;    // Address within FLASH of image
>     CYG_ADDRESS   mem_base;      // Address in memory where 
> it executes
>     unsigned long size;          // Length of image
>     CYG_ADDRESS   entry_point;   // Execution entry point
>     unsigned long data_length;   // Length of actual data
>     unsigned char 
> _pad[CYGNUM_REDBOOT_FIS_DIRECTORY_ENTRY_SIZE-FIS_IMAGE_DESC_SI
> ZE_UNPADDED];
>     unsigned long desc_cksum;    // Checksum over image descriptor
>     unsigned long file_cksum;    // Checksum over image data
> };
> 
> The problem with this is that its much more invasive. You have to
> change all current references of name to u.name.

Would this be a problem ? I'd be willing to implement this change.
Ok, so what's your final word ?
I'll send a patch with this tonight/tomorrow.

> > > > This doesn't only apply to the redboot code, but even 
> more to the
> > > > fisfs implementation, there this has to be done more often.
> > > 
> > > I took a quick look at your fisfs code. We need to talk 
> about that...
> > 
> > Yes, sure :-)
> 
> Im note sure your general approach is right. I would put all the logic
> for updating FIS into fis_update_directory() 

fis_update_directory() is not enough. There have to be two two functions, so that I can mark the beginning of the manipulation of the data on the flash and after this is done the end.
If I am able to do this, it is possible to check (e.g. during mount()) whether there has been an update process interrupted. 

> and extend the current
> virtual vector support to allow more access to the FIS information.
> Putting code into a seperate package which manipulates the FIS
> directory is probably not a good idea in terms of maintainability. If
> all the code is in redboot it will be much less of a problem.

This is right. OTOH redboot doesn't really need to be able to perform the safe updates. If somebody accesses redboot directly (i.e. a developer), he should know what he's doing.
So all the shared code currently consists of fis.h (which is identical) and the function for finding the valid table. 
How about simply sharing the source files ?
How does the virtual vector stuff work ?

Bye
Alex