Hi,
> Von: Andrew Lunn [mailto:[EMAIL PROTECTED] > > +cdl_package CYGPKG_FS_FIS { > + display "FIS update and filesystem" > + include_dir cyg/fs > + > + requires CYGPKG_ISOINFRA > + requires CYGINT_ISO_ERRNO > + requires CYGINT_ISO_ERRNO_CODES > + requires CYGPKG_LIBC_STRING > + > + compile -library=libextras.a fisupdate.c > > Why should this go into libextras.a? It is not a device driver. We > want the linker to discard this code if its not used. Ok. What's actually the exact difference between libextras.a and the normal lib ? I just copied this from somewhere. > + cdl_option CYGDBG_FS_FIS_DEBUG_OUTPUT { > + display "Enable debug output" > + default_value 1 > + } > > This kind of suggests the code still needs debugging. I would > prefer to see the default as 0. Ok. > +// how many entries can the FIS contain at most ? > initialized in fis_init() > +int fis_max_entries=-1; > > This should be a static variable to avoid pollution. Ok. > +/* If an image with the given name exists its index is returned and > + entry is filled appropriatly. Using a NULL-pointer for entry is also > + ok, then only the index will be returned. If no such image > exists, it returns -1.*/ > +int fis_find_entry(const char* name, struct fis_table_entry* entry) > +{ > > Should this be static? It looks like it is just a helper > function for fis_get_entry(). It is used also in the filesystem implementation, that's why I removed the static. > + // write them for now into the flash > + > CYGACC_CALL_IF_FLASH_FIS_OP2(CYGNUM_CALL_IF_FLASH_FIS_START_UP > DATE, 0, NULL); > + // and make the new written directory now valid, because > once the erasing has started the data is corrupt > + > CYGACC_CALL_IF_FLASH_FIS_OP2(CYGNUM_CALL_IF_FLASH_FIS_FINISH_U > PDATE, 0, NULL); > + > + // erase it > + cyg_interrupt_disable(); > + flash_unlock((void *)oldEntry.flash_base, oldEntry.size, > (void **)&err_addr); > > flash_unlock is not guaranteed to exist. You need to use > CYGHWR_IO_FLASH_BLOCK_LOCKING Ok. > Also, you don't seem to be consistent. fis_program_data() does not > unlock/lock. The unlock/lock calls are around the call to fis_program_data(). > I would like to see all these global scope functions have cyg_ prefix > otherwise they cause name space pollution. So cyg_fis_get_entry() etc. ? Bye Alex
