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

Add support for Microchip SST26VF016B & SST26VF032B and make fastread working #173

Closed
wants to merge 3 commits into from

Conversation

LoicGRENON
Copy link

@LoicGRENON LoicGRENON commented Apr 21, 2019

Any pull request raised here MUST be submitted according to here MUST be submitted according to this template or it will be flagged with 'Not enough information'. No action will be taken till all the prerequisite information (according to the right template) is provided. If no information is provided for over a month after the 'Not enough information' label is applied, the issue will be closed.

Pull request details

  • Please check if the PR fulfills these requirements
  • The commit message/s explain/s all the changes clearly
  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)
  • What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)
  • Bug fix
  • Added feature
  • Documentation update
  • Other - Please explain here: new chip support (I added it on March but I saw you added support for these chips in the meantime)
  • What is the current behavior? (You can also link to an open issue here)
    Fastread mode do not work (only tested with SST26VF016B & SST26VF032B chips)

  • What is the new behavior? (if this is a feature change)
    Make fastread mode works (only tested with SST26VF016B & SST26VF032B chips)

  • Does this PR introduce a breaking change? (What changes might users need to make in their application due to this PR?)
    Nothing

  • Other information:
    As I don't have any other chip, especially Winbond ones. Can you confirm that fastread mode is not actually working on your side ?
    If it working with other chip than Microchip ones, I probably need to do some verifications to identify it.


DO NOT DELETE OR EDIT anything below this

Note 1: Make sure to add all the information needed to understand the bug so that someone can help. If any essential information is missing we'll add the 'Needs more information' label and close the issue until there is enough information.

Note 2: For support questions (for example, tutorials on how to use the library), please use the Arduino Forums. This repository's issues are reserved for feature requests and bug reports.


GitHub issue state GitHub issue title GitHub issue author GitHub issue label GitHub issue comments GitHub issue age GitHub issue last updateGitHub pull request check contexts

@Marzogh Marzogh added the enhancement An enhancement to the library's function is proposed label Apr 24, 2019
@Marzogh Marzogh changed the base branch from master to development April 24, 2019 06:06
@Marzogh Marzogh self-assigned this Apr 24, 2019
@Marzogh
Copy link
Owner

Marzogh commented May 5, 2019

Hi Loic,

Thanks for this - I'll just test this over the next day or so and pull it into the development branch. W.R.T Fast Read, the Winbond chips call for 8 dummy clocks after the 24-bit address is clocked in (Re: Section 8.2.9 in the datasheet here). I don't know if the SST25 and SST26 series need that - I'll check that out as part of the testing 🙂

@Marzogh Marzogh added this to the v3.4.0 milestone May 5, 2019
@Marzogh
Copy link
Owner

Marzogh commented Jun 2, 2019

Hi mate, after checking the library code, it turns out I'd made a mistake when I wrote the _beginSPI() function that broke FastRead. The bug has now been fixed (as of commit 7e0825a) and confirmed to work on both Winbond and SST26 series chips.

Also, I had already included SST26 series support in the development branch before your pull request came through, so, I won't need to merge it here after all. However, thank you for taking the time to catch and fix the bug and add support for new chips! 🙂

@Marzogh Marzogh closed this Jun 2, 2019
@LoicGRENON
Copy link
Author

Hello,

The same fix as of commit 7e0825a is needed for SPIFram on SPIFramIO.cpp at line 164 : The dummy byte needed for fast read operation should be sent after the two-bytes address.
However, it seems to not be sufficient to make fast read to work with a FRAM chip...

@LoicGRENON
Copy link
Author

LoicGRENON commented Mar 22, 2020

Well, in fact the SPIFram::_read() template defined in SPIFram.h need to also be modified like SPIFlash::_read defined in SPIFlash.h

Need to change _nextByte(WRITE, FASTREAD); and _nextByte(WRITE, READDATA); to _beginSPI(FASTREAD); and _beginSPI(READDATA); at lines 339 and 342.
Then remove _transferAddress(); at line 344.

I made a new pull request that solve this new issue : #203

LoicGRENON added a commit to LoicGRENON/SPIMemory that referenced this pull request Mar 22, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement An enhancement to the library's function is proposed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants