SPIFFS filesystem is not using SPIFFS_USE_MAGIC

[hreintke]

@anakod : @raburton : @alonewolfx2 :
I noticed that in the spiffs config the the parameter SPIFFS_USE_MAGIC is not set.
That results in a Spiffs filesystem in which when mounting there a absolutely no check on validity of the "disk" -> It is only discovered when later using filesystem.

Now, with the rBoot system in place there is also the possibility to "load" filesystems.
Because of the magic setting there is no possibility to verify whether an uploaded FS is correct.
I am planning that verification in the rBoot Generic implementation I am currently working on.

I did some investigation and it looks like it doesn't take too much effort to include the magic setting into Sming. The spiffy included in Sming is already taking the spiffs config used Sming config parameters and will create correct filesystems when setting magic. Needs only a small update to verify internal mount result.

It will be a "backward breaking update" when implementing this as filesystems created without "magic" are not compatible with filesystems using "magic"

• What was the reason for not using SPIFFS_USE_MAGIC ?
• Do you see any issues arising when implementing "magic"

[hreintke]

Checking history.
Probably the SPIFFS_USE_MAGIC option was included in Sming when Richard upgraded to SPIFFS version 0.3.3

[raburton]

Yes, it's a new thing. I don't know how useful it is (I didn't investigate), but if you think it's a good thing then we can easily enable it. The nice thing about having spiffs in Sming is that we can make changes like this (that would be backwards incompatible) and all news spiffs created with a new version of Sming will automatically be correct for roms created with the same new version of Sming.

[hreintke]

When using SPIFFS_USE_MAGIC the spiffs_mount returns OK or ERROR when mounting.
Without option spiffs_mount is setting internal variables and always returns OK.
Almost no additional overhead, no additional memory usage.
In my opinion, this would be a way to

• Prevent issues which now only shows when using spiffs system
• Upgrading to later spiffs version which might have SPI layout differences will be noticed
• When upgrading Sming needs to change spiffs parameters

For now I see the following updates to Sming needed :

• set option in SPIFSS
• update coding around spiff_mount
• Use spiffs_format(...) instead of "just erasing" -> localized in sming, availlable in spiffs
• spiffy update to create "Magic included" spiffs file

Will continue working on this change

BTW : I found the 0x4020000 needed for mounting
#define INTERNAL_FLASH_START_ADDRESS 0x40200000
I will investigate usage

[raburton]

BTW : I found the 0x4020000 needed for mounting
#define INTERNAL_FLASH_START_ADDRESS 0x40200000
I will investigate usage

This is an oddity, probably based around ported code. We don't use memory mapped flash so the flash read routines just subtract this value to get to the real flash address. Seems silly, wants fixing really.

[hreintke]

Yes, the flashmem.h file says :

// Based on NodeMCU platform_flash
// https://github.com/nodemcu/nodemcu-firmware

This implements an access to flash which hides the physical characteristics of flash.
using the blocksizes of flash and reading/writing additional blocks if over block boundary.
In this way the flash is just a continuous area of storage in which you can read/write data.
It also has the function to find the first free block after application (_flash_code_end in link file).
As far I can see now, that is only used for finding the startaddress of spiffs area when not "mount_manual" and not used in combination with INTERNAL_FLASH_START_ADDRESS.

We should reconsider the usage/access to flash in combination with the rboot options.

Question :
In flashmem all spi read/write are precede by a register setting : f.e.

  WRITE_PERI_REG(0x60000914, 0x73);
  r = spi_flash_write(toaddr, apbuf?(uint32 *)apbuf:(uint32 *)from, size);

Any idea why this done/needed ?

[raburton]

Google suggests it has something to do with the watchdog. Probably feeding it, becasue nodemcu used to do this (probably where this got copied from) but now calls a proper api: https://github.com/nodemcu/nodemcu-firmware/pull/703/files.

[hreintke]

@raburton :
I now understand how the
#define INTERNAL_FLASH_START_ADDRESS 0x40200000
is used within sming.
It is used to calculate the first free available block after the program on flash -> starting block of the spiffs filesystem.

Strange in the implementation is that it not used when actually calculating this but "changes every read/write". It is easy to revert -> Then we can remove the adding of "0x420..." when doing a spiffs_mount_manual.

Because we do need the value I was wondering whether it really need to be hardcoded or can be calculated -> When LD files changes, automatic using the right value.
In the LD file there are the following lines :

     .irom0.text : ALIGN(4)
  {
    _irom0_text_start = ABSOLUTE(.);
....
....
    _irom0_text_end = ABSOLUTE(.);
    _flash_code_end = ABSOLUTE(.);
  } >irom0_0_seg :irom0_0_phdr

The _flash_code_end, together with the INTERNAL_FLASH_START_ADDRESS
is now used for calculating.
If I read the _irom0_text_start I get the value 0x40209000 the flash start address + the 0x9000 offset from the esptool -> close but not the right figure. And when I "just subtract the 0x9000" I have another hardcoded value in sming.
Do you know whether there is a way to find the flash_start_address ?

Other question.
I now have two updates which I can make to flashmem & spiffs (revert internal_flash_address usage and use os spiffs magic).
What do you think is best. Create two separate PR's : first spiffs, then flashmem or combining the two so we can test together.

[raburton]

I don't see a problem with the hard coded flash start address, it is a fixed value specific to esp.

I'd do the flashmem changes as a separate PR to the spiffs/y feature as they are unrelated changes. Are you making the flash read/write functions work with real flash address then, instead of user having to add 0x40200000, then the function subtracting it again? That would be much nicer, and something I intended to look into a while back but never got chance.

[hreintke]

Ok, I'll keep the hardcoded start address in.

Yes, flash read/write will use real addresses then. Much clearer when using.
When implemented also applications using spiffs_mount_manual need to update to real address.
Or we implement something like If (reqAdddress > start_flash) realAddress = reqAddress - start_flash;
but I prefer to get that clean too.

[raburton]

Yes, backwards compatibility would be nice, and easy to do, but not essential if we document in the release notes. Although as we learned when we stopped shipping a pre-compiled libsming.a nobody bothers to read the release notes.

[raburton]

I've made the required changes to spiffy for you, it now uses SPIFFS_format to create the empty file, which means that when you enable SPIFFS_USE_MAGIC spiffy will be doing the right thing. #439

[hreintke]

Thanks, works correctly with my work in progress on Magic

Status is that the "basic functionality" works but need (some) additional effort to get format also correctly implemented. Currently that also uses the "sming format" which now should go to SPIFFS_format.

I noticed that you also created a spiff_format_manual. Will probably create only spiffs_format() which then handles both "non-manual/automatic" and "manual" filesystems.

[hreintke]

Checking flash_mem_read/write.
Both "just return the value of the inputted "size parameter" without checking results of spi_flashread/write. -> no error processing possible

Because of that f.e. the api_spiffs_write always returns OK.

static s32_t api_spiffs_write(u32_t addr, u32_t size, u8_t *src)
{
  //debugf("api_spiffs_write");
  flashmem_write(src, addr, size);
  return SPIFFS_OK;
}

Will not include this in the update for start_flash_update but I think needs rework

[raburton]

I noticed that you also created a spiff_format_manual. Will probably create only spiffs_format() which then handles both "non-manual/automatic" and "manual" filesystems.

How will you know where to format?

[hreintke]

You only can format after first having called SPIFFS_mount .
From there the fs is available (includes config = location) which is used for formatting.

[raburton]

You can't call SPIFFS_format without first calling SPIFFS_mount, but you
can call currently call spiffs_format or spiffs_format_manual without
calling spiffs_mount first. It isn't necessarily a problem to add this
restriction, but it is a change to the current behaviour.

[hreintke]

Will be solved in Sming_RTOS by using new MAGIC_LENGTH from Spiffs.
Will not be backported to Sming NONOS.