Skip to content
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.

Sign up for GitHub

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

Jump to bottom

spiffs_page_delete relies on 0 -> 1 transitions being ignored #172

Closed
rojer opened this issue Sep 22, 2017 · 9 comments · Fixed by #173
Closed

spiffs_page_delete relies on 0 -> 1 transitions being ignored #172

rojer opened this issue Sep 22, 2017 · 9 comments · Fixed by #173

Comments

@rojer
Copy link
Contributor

rojer commented Sep 22, 2017

I am working with a flash controller that does not like it when application tried to set bits from 0 to 1.
naturally, it's not possible, and most controllers just ignore it, but not this one - it raises an error for such writes, and I found that SPIFFS is a bit sloppy in this regard.

It seems that spiffs_page_delete tries to set the flags blindly, assuming that 0 -> 1 transitions will be ignored. Well, that is not always the case and SPIFFS should not assume this behavior of the underlying controller.
@rojer rojer mentioned this issue Sep 22, 2017
Merged
@rojer
Copy link
Contributor Author

rojer commented Sep 22, 2017

#173 is my quick whack at it. i'm not at all sure this is all that needs to be done, but tests are now passing without the "flags exemption".

cesantabot pushed a commit to cesanta/mongoose-os that referenced this issue Sep 25, 2017
@cesantabot
CC3220: Use built-in flash for SPIFFS
7ef8bd1 
We can drop the SLFS container contraption now! It doesn't work properly on CC3220 anyway: it is no longer possible to read an SLFS file while it's open for writing, it needs to be re-opened read-only, which means even more container switches and even more terrible write performance. Good riddance.

Instead, we put SPIFFS at the end of internal flash. The TRM speaks about flash bing organized in two banks of 512K, so i think this limits code size to 512K (or we'll have to put flash programming routines in RAM and play other tricks to avoid trying to execute code from the bank that is being programmed). But we're nowhere near 512K anyway (~200K currently), so it's grand.

Since it is not possible to program internal flash directly when flashing, we instead put a container on SLFS and transfer it to internal flash on first boot.

TI's flash controller is a bit anal-retentive about writes and raises an error if you try to flip bits from 0 to 1 instead of just ignoring it like everybody else in the world.
Turns out, SPIFFS is playing fast and loose with this in some places, which has been reported upstream: pellepl/spiffs#172
This PR integrates pellepl/spiffs#173 which is my humble attempt at fixing it, and seems to work ok.

PUBLISHED_FROM=65ffe0169b75a5c5685f6a336eb6fd875f181195
cesantabot pushed a commit to mongoose-os-libs/wifi that referenced this issue Sep 25, 2017
@cesantabot
CC3220: Use built-in flash for SPIFFS
8a4a759 
We can drop the SLFS container contraption now! It doesn't work properly on CC3220 anyway: it is no longer possible to read an SLFS file while it's open for writing, it needs to be re-opened read-only, which means even more container switches and even more terrible write performance. Good riddance.

Instead, we put SPIFFS at the end of internal flash. The TRM speaks about flash bing organized in two banks of 512K, so i think this limits code size to 512K (or we'll have to put flash programming routines in RAM and play other tricks to avoid trying to execute code from the bank that is being programmed). But we're nowhere near 512K anyway (~200K currently), so it's grand.

Since it is not possible to program internal flash directly when flashing, we instead put a container on SLFS and transfer it to internal flash on first boot.

TI's flash controller is a bit anal-retentive about writes and raises an error if you try to flip bits from 0 to 1 instead of just ignoring it like everybody else in the world.
Turns out, SPIFFS is playing fast and loose with this in some places, which has been reported upstream: pellepl/spiffs#172
This PR integrates pellepl/spiffs#173 which is my humble attempt at fixing it, and seems to work ok.

PUBLISHED_FROM=65ffe0169b75a5c5685f6a336eb6fd875f181195
@pellepl
Copy link
Owner

pellepl commented Oct 7, 2017

Sorry for belated reply.
Transitions 0->1 being ignored is one of the main presumptions when writing spiffs. What flash controller is it? I have never stumbled upon one not allowing this.

I am deliberately sloppy here as, in my view, this is an optimization.
Looking at your fix I am actually a bit surprised, I think you covered it all. I thought there would be more cases.
What I would like to have is a config define around the readout as it will affect performance somewhat. At least to my experience, 99% of all flashes do not have a problem with ignoring 0->1.

@rojer
Copy link
Contributor Author

rojer commented Oct 7, 2017 •
edited
Loading

controller is the one built into TI CC3220, but i think it's generic to TI (Tiva chips have the same).
it's described in chapter 21 of the TRM.
in particular, bit 10 of the FCRIS register is:

INVDRIS -- Invalid Data Raw Interrupt Status
...
1h = An interrupt is pending because a bit that was previously
programmed as a 0 is now being requested to be programmed as a
1.

and indeed controller refuses the entire write operation if any of the data loaded into the data registers tries to flip bits from 0 to 1.

yes, it saves a read. makes sense to make it a config option, sure.

@rojer
Copy link
Contributor Author

rojer commented Oct 7, 2017

what would you call the option? i'll amend #173.

@pellepl
Copy link
Owner

pellepl commented Oct 7, 2017 via email

@pellepl
Copy link
Owner

pellepl commented Oct 7, 2017

Off topic, but the CC32 flash, that is the internal flash rom you're referring to? Not anything external to the uC itself?

@rojer
Copy link
Contributor Author

rojer commented Oct 7, 2017 •
edited
Loading

SPIFFS_CONFIG_NO_BLIND_WRITES ? with the default of 0, meaning blind writes are allowed and 0 -> 1 transitions are assumed to be ignored by the controller.

yes, i'm using part of internal flash for the filesystem.

@pellepl
Copy link
Owner

pellepl commented Oct 7, 2017 via email

@rojer
Copy link
Contributor Author

rojer commented Oct 8, 2017

@pellepl ok, please take a look at #173

@rojer rojer mentioned this issue Jan 9, 2018
Open
@devyte devyte mentioned this issue Jan 22, 2018
Closed
@drzony drzony mentioned this issue Aug 11, 2020
Merged
@drzony drzony mentioned this issue Oct 8, 2020
Open
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Be careful resetting flag bits
2 participants
@rojer @pellepl