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.

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

Fix erase sector failure on S25FL127S #101

Merged
merged 4 commits into from
Nov 20, 2017

Conversation

hanyazou
Copy link
Contributor

I found that all erase command except chip erase failed on my MKR1000 and Cypress S25FL127S (128Mbits). TestFlash.ino did not show any errors but flash.eraseSector(), eraseBlock32K() and eraseBlock64K() did not take effect.

I used my logic analyzer and noticed that the library send out some garbage on SPI bus after erase command and 3 address bytes. I added missing break statements in SPIFlash::_beginSPI() because that lacks some essential break statement in switch/case. (commit 1: 1de2c10)

Then I found that the library did not wait until the busy bit in SR1 was cleared. Timeout argument of _notBusy() must be in micro-second. 500L means 500 micro seconds which is too short to wait for completion of erase command. I've add " * 1000" to the arguments. (commit 2: 9633c2f)

And I modified TestFlash.ino to check error of erase commands execution. (commit 3: a179306)

I've tested these changes on my MKR1000 and S25FL127S and they looks working correctly. (S25FL127S does not have Erase Block 32 command however.)

@Marzogh Marzogh changed the base branch from master to development November 19, 2017 05:05
@Marzogh
Copy link
Owner

Marzogh commented Nov 19, 2017

Thanks for catching and fixing that. 😄 I'll just double check that it has't broken anything else and will roll it into a bug fix release within the next fortnight.

@Marzogh Marzogh added this to the v3.0.1 milestone Nov 19, 2017
@Marzogh Marzogh added the bugfix Fixes an existing bug in the current version of the code label Nov 19, 2017
@Marzogh Marzogh self-assigned this Nov 19, 2017
Copy link
Owner

@Marzogh Marzogh left a comment

Choose a reason for hiding this comment

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

Gah! I have no idea how this even happened! Thank you for catching the missing break;

Copy link
Owner

@Marzogh Marzogh left a comment

Choose a reason for hiding this comment

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

Thanks! Your changes make it easier to work with. I'll sort out the background code to throw an UNSUPPORTEDFUNC error when eraseBlock32 is called with the S25FL127S

@Marzogh Marzogh merged commit 405430f into Marzogh:development Nov 20, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugfix Fixes an existing bug in the current version of the code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants