Skip to content

Add support for SPI1 device #182

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 3 commits into from
May 16, 2016
Merged

Add support for SPI1 device #182

merged 3 commits into from
May 16, 2016

Conversation

eriknyquist
Copy link
Contributor

Add 'SPI1' instance to current SPI library, for accessing the onboard SPI flash. Additionally, tidy up the SPI library code (there is one commit for each. The only functional changes are in the first commit "Add support for SPI1 device to SPIClass").

SPI1 can now be used (SPI1.begin()) to access
the SPI device connected to the Arduino 101's
on-board SPI flash device
Be consistent about commenting style, and
stay below 80 chars where appropriate.
@bigdinotech
Copy link
Contributor

@calvinatintel looks good. +1

@eriknyquist
Copy link
Contributor Author

@calvinatintel @SidLeung ready for review

@SidLeung
Copy link
Contributor

@eriknyquist @bigdinotech Looks good. May I suggest to use SPI_FOR_FLASH (or something better) in place of SPI1? I bet someone is going to complain about the flipping of the indexing...

@eriknyquist
Copy link
Contributor Author

@SidLeung, you are right someone will surely complain. The flipped indexing makes things easier to follow if you are also looking at EAS / schematic. However if you are not, then it does not. I'm happy either way, let me know what you think.

@eriknyquist
Copy link
Contributor Author

@SidLeung I think I misunderstood, do you mean change the instance name? so SPI1.begin() becomes SPI_FOR_FLASH.begin() (or whatever)? I think it's good to stick with the way other modules do it, i.e. Serial and Serial1

@intel-larry
Copy link

intel-larry commented May 13, 2016

For expediency – maybe get it working then sort out the names?

@SidLeung
Copy link
Contributor

@eriknyquist @intel-larry @calvinatintel Agree. Let's get it working first. Stick with the naming, put a comment that indicating which SPI is for Flash, etc.

@eriknyquist
Copy link
Contributor Author

Sounds good.

@calvinatintel
Copy link
Contributor

@eriknyquist Is this ready to be merged?

@eriknyquist
Copy link
Contributor Author

@calvinatintel I'm testing this change today against some SPI flash libraries, let's wait until Monday just in case I find an issue.

If a clock value greater than the maximum supported
is passed (i.e. > 16MHz), the resulting value written
to the BAUDR register will be 0 which will disable
the serial output clock. Check for this and ensure
the value calculated for BAUDR register is never < 2.
@eriknyquist
Copy link
Contributor Author

@SidLeung @bigdinotech @calvinatintel, fixed a bug in the SPISettings class which was preventing the Serial Flash library working out of the box. 3rd commit ready for code review, after which the whole PR can be merged.

@eriknyquist eriknyquist added this to the Bellatrix milestone May 16, 2016
@bigdinotech
Copy link
Contributor

@calvinatintel looks good. Please merge

@calvinatintel calvinatintel merged commit 8dea7b4 into arduino:master May 16, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants