Skip to content

inconsistent block size for spiffs in board.txt. and ld files #5412

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 15 commits into from
Dec 4, 2018

Conversation

apicquot
Copy link
Contributor

@apicquot apicquot commented Dec 2, 2018

boardtxt.py is not consistent for generating spiffs block size in boards.txt and ld files:

it takes the flash size for boards.txt but the spiffs size for the lf files: there is currently a problem for boards with flash size >= 1M and spiffs size < 1M

impacted boards 2M (SPIFFS 128K), 2M (SPIFFS 256K) 2M (SPIFFS 512K)

Suggested change is to make block size dependable on spiffs size for board.txt:
if spiffs size < 1M block size=4096 else block size=8192

… be consistent with ld files

spiffs < 1M has a 4096 block size and 8192 for all the other ones

only impacted boards 2M (128K), 2M (256K) 2M (512K)
else:
max_upload_size = 1024 * 1024 - reserved
spiffs_start = (flashsize_kb - spiffs_kb) * 1024

if spiffs_kb < 1024:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be < or <=?

@apicquot
Copy link
Contributor Author

apicquot commented Dec 2, 2018

it should be < 1024: for example 2M (SPIFFS 1M) should sill have a block size of 8192.

@devyte
Copy link
Collaborator

devyte commented Dec 2, 2018

@apicquot you mentioned a problem, but you didn't specify what. What did you encounter?

@apicquot
Copy link
Contributor Author

apicquot commented Dec 2, 2018

Currently spiffs does not work for boards 2M (SPIFFS 128K), 2M (SPIFFS 256K) and 2M (SPIFFS 512K):
the ld files contain a blockssize of 4096 and board.txt a blocksize 8192. The SPIFFS system can not be found or SPIFFS.info(fs_info) and all other SPIFFS functions fail

@devyte
Copy link
Collaborator

devyte commented Dec 2, 2018

@apicquot what about creating a SPIFFS image from the IDE for flashing? what decision is made for the block size there?

@esp8266 esp8266 deleted a comment from apicquot Dec 3, 2018
@apicquot
Copy link
Contributor Author

apicquot commented Dec 3, 2018

@devyte the block size when flashing the spiffs from the ide (parameters passed to mkspiffs) is coming from board.txt. That is why it has to be the same value than in the ld files

@d-a-v
Copy link
Collaborator

d-a-v commented Dec 3, 2018

While understandable, this is to me dangerous as-is, because users already using existing from some time 2M512 will have their spiffs reformated on boot after this patch.

You are basically stating that spiffs block size should be 4096 when spiffs size < 1MB.
Why not choosing if spiffs_kb < 512: instead of < 1024 ?

We can see below that it is 4096/0x1000 when size is < 512 and 0x2000 otherwise.
I reckon your patch is very useful and needed to make spiffs-blocksize calculated from a real understandable rule, but 1024 scares me.

tools/sdk/ld$ grep SPIFFS_block *.ld
eagle.flash.16m14m.ld:PROVIDE ( _SPIFFS_block = 0x2000 );
eagle.flash.16m15m.ld:PROVIDE ( _SPIFFS_block = 0x2000 );
eagle.flash.1m.ld:PROVIDE ( _SPIFFS_block = 0x0 );
eagle.flash.1m128.ld:PROVIDE ( _SPIFFS_block = 0x1000 );
eagle.flash.1m144.ld:PROVIDE ( _SPIFFS_block = 0x1000 );
eagle.flash.1m160.ld:PROVIDE ( _SPIFFS_block = 0x1000 );
eagle.flash.1m192.ld:PROVIDE ( _SPIFFS_block = 0x1000 );
eagle.flash.1m256.ld:PROVIDE ( _SPIFFS_block = 0x1000 );
eagle.flash.1m512.ld:PROVIDE ( _SPIFFS_block = 0x2000 );
eagle.flash.1m64.ld:PROVIDE ( _SPIFFS_block = 0x1000 );
eagle.flash.2m.ld:PROVIDE ( _SPIFFS_block = 0x0 );
eagle.flash.2m128.ld:PROVIDE ( _SPIFFS_block = 0x1000 );
eagle.flash.2m1m.ld:PROVIDE ( _SPIFFS_block = 0x2000 );
eagle.flash.2m256.ld:PROVIDE ( _SPIFFS_block = 0x1000 );
eagle.flash.2m512.ld:PROVIDE ( _SPIFFS_block = 0x2000 );
eagle.flash.4m.ld:PROVIDE ( _SPIFFS_block = 0x0 );
eagle.flash.4m1m.ld:PROVIDE ( _SPIFFS_block = 0x2000 );
eagle.flash.4m2m.ld:PROVIDE ( _SPIFFS_block = 0x2000 );
eagle.flash.4m3m.ld:PROVIDE ( _SPIFFS_block = 0x2000 );
eagle.flash.512k.ld:PROVIDE ( _SPIFFS_block = 0x0 );
eagle.flash.512k128.ld:PROVIDE ( _SPIFFS_block = 0x1000 );
eagle.flash.512k32.ld:PROVIDE ( _SPIFFS_block = 0x1000 );
eagle.flash.512k64.ld:PROVIDE ( _SPIFFS_block = 0x1000 );
eagle.flash.8m6m.ld:PROVIDE ( _SPIFFS_block = 0x2000 );
eagle.flash.8m7m.ld:PROVIDE ( _SPIFFS_block = 0x2000 );

@apicquot
Copy link
Contributor Author

apicquot commented Dec 3, 2018

good point @d-a-v

changing to if spiffs_kb < 512 is a good idea

Copy link
Collaborator

@devyte devyte left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Per @d-a-v 's comments about < 512KB.

@apicquot
Copy link
Contributor Author

apicquot commented Dec 4, 2018

indent is changed in package/package_esp8266com_index.template.json
board.txt.py likely to have been modified in master and file not generated ?

@earlephilhower
Copy link
Collaborator

#5429 will take care of the indent. platform....8266.json is being edited using string replacement not a JSON parser presently so there will be odd indent differences.

The SPIFFS blocksize change hasn't been tested by me, can't comment on it.

Copy link
Collaborator

@earlephilhower earlephilhower left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm OK w/a merge either w/the package/JSON or w/o it (spacing fix coming already).

@earlephilhower earlephilhower added this to the 2.5.0 milestone Dec 4, 2018
Copy link
Collaborator

@d-a-v d-a-v left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for adding this logic

@d-a-v d-a-v merged commit ee3b374 into esp8266:master Dec 4, 2018
@TD-er
Copy link
Contributor

TD-er commented Dec 5, 2018

While understandable, this is to me dangerous as-is, because users already using existing from some time 2M512 will have their spiffs reformated on boot after this patch.

You are basically stating that spiffs block size should be 4096 when spiffs size < 1MB.
Why not choosing if spiffs_kb < 512: instead of < 1024 ?

We can see below that it is 4096/0x1000 when size is < 512 and 0x2000 otherwise.
I reckon your patch is very useful and needed to make spiffs-blocksize calculated from a real understandable rule, but 1024 scares me.

The tests I did with the 2M/512k layout simply did not work.
So I think it is very unlikely there are working setups running this layout with working SPIFFS.

Also a practical question about the block size.
Does any file take at least 1 block? (like the name suggests)
If so, then I would even opt for smaller blocks if possible.
On 128k SPIFFS there is only room for max. 32 (very small) files (more likely less due to filesystem overhead) with minimum filesize of 4k.

@apicquot
Copy link
Contributor Author

apicquot commented Dec 5, 2018

2M 512K works great for me and so do the 2 new board configs 2M 128K and 2M 256K. I use the IDE for launching mkspiffs and setting up properly the parameters. As of the maximum number of files, my understanding is that they can more files than number of blocks, the minimum file size is the page size. I have 47 small files that works fine with the 128K so going below 4096 block size won't do any good. Hope it helps

@devyte
Copy link
Collaborator

devyte commented Dec 6, 2018

@apicquot thanks for the info!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants