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

drivers: Add NVRAM API #2353

Merged
merged 4 commits into from
May 4, 2015
Merged

drivers: Add NVRAM API #2353

merged 4 commits into from
May 4, 2015

Conversation

jnohlgard
Copy link
Member

This API is designed around non-volatile memories which do not need blockwise erase, such as ferro-electric RAM (FRAM) or magneto-resistive RAM (MRAM).

Flash devices do need blockwise erase and are not supported by this API.

Also included is a generic implementation of the NVRAM API for SPI connected FRAM devices from Cypress Semiconductor. This SPI driver may also work for MRAM from Everspin Technologies (according to the datasheet), however it has not been verified.

@jnohlgard jnohlgard added Area: drivers Area: Device drivers Type: new feature The issue requests / The PR implemements a new feature for RIOT State: WIP State: The PR is still work-in-progress and its code is not in its final presentable form yet labels Jan 26, 2015
@jnohlgard jnohlgard added CI: needs squashing Commits in this PR need to be squashed; If set, CI systems will mark this PR as unmergable and removed State: WIP State: The PR is still work-in-progress and its code is not in its final presentable form yet labels Jan 26, 2015
@jnohlgard
Copy link
Member Author

Added a test application and have successfully tested on actual hardware.

@jnohlgard jnohlgard added the State: WIP State: The PR is still work-in-progress and its code is not in its final presentable form yet label Jan 27, 2015
@jnohlgard
Copy link
Member Author

Needs a clean up of nvram.h. I'll post back when it's done.

@jnohlgard jnohlgard removed the State: WIP State: The PR is still work-in-progress and its code is not in its final presentable form yet label Jan 27, 2015
@jnohlgard
Copy link
Member Author

Updated the PR with cleaned up headers. Made all implementation specific functions static when possible, user interface is supposed to happen through nvram_t device descriptor, as in the test case.

@jnohlgard jnohlgard removed the CI: needs squashing Commits in this PR need to be squashed; If set, CI systems will mark this PR as unmergable label Jan 27, 2015
@jnohlgard
Copy link
Member Author

Squashed and rebased after some more testing on real hardware. Ready for review.

@jnohlgard jnohlgard added the Discussion: RFC The issue/PR is used as a discussion starting point about the item of the issue/PR label Jan 29, 2015
jnohlgard pushed a commit to jnohlgard/RIOT that referenced this pull request Feb 5, 2015
mulle: Disable NVRAM API until PR RIOT-OS#2353 is merged
27c1207
jnohlgard pushed a commit to jnohlgard/RIOT that referenced this pull request Feb 7, 2015
mulle: Disable NVRAM API until PR RIOT-OS#2353 is merged
28ab0a8
@jnohlgard jnohlgard assigned OlegHahm and unassigned haukepetersen Feb 9, 2015
jnohlgard pushed a commit to jnohlgard/RIOT that referenced this pull request Feb 9, 2015
mulle: Disable NVRAM API until PR RIOT-OS#2353 is merged
588f2de
@thomaseichinger
Copy link
Member

@OlegHahm ping

@jnohlgard
Copy link
Member Author

I unassigned @haukepetersen since he was on a trip and seem preoccupied with the network stack refactoring.

I assigned @OlegHahm based on his request in #1222 (comment)

jnohlgard pushed a commit to jnohlgard/RIOT that referenced this pull request Feb 17, 2015
mulle: Disable NVRAM API until PR RIOT-OS#2353 is merged
1a1b0d8
@OlegHahm
Copy link
Member

Thanks @gebart for re-assigning. I have nothing to complain about this PR, but I'm not an expert on this low-level stuff. The interface looks reasonable to me. @thomaseichinger, @PeterKietzmann, care to review?

@PeterKietzmann
Copy link
Member

I'll have a look at it but I assume I don't have something to test

@jnohlgard
Copy link
Member Author

@PeterKietzmann The hardware device SPI protocol is very limited, so there is not very much to test there, it is not even possible to read the device size or any kind of identification. The biggest part of reviewing this PR is the introduction of a new API for NVRAM devices which can handle random byte writes without sector erase.

@OlegHahm
Copy link
Member

Please see http://lists.riot-os.org/pipermail/devel/2015-February/001987.html

jnohlgard pushed a commit to jnohlgard/RIOT that referenced this pull request Feb 20, 2015
mulle: Disable NVRAM API until PR RIOT-OS#2353 is merged
5b9058a
@OlegHahm
Copy link
Member

Apart from me no one seems to object the function pointers.

jnohlgard pushed a commit to jnohlgard/RIOT that referenced this pull request Mar 18, 2015
mulle: Disable NVRAM API until PR RIOT-OS#2353 is merged
a1f4329
@PeterKietzmann
Copy link
Member

No one agains this API? That is good :-) . Let's wait for a final result in discussion #2528 and then go go for it soon.

jnohlgard pushed a commit to jnohlgard/RIOT that referenced this pull request Mar 20, 2015
mulle: Disable nvram-spi until RIOT-OS#2353 is merged
c32dd5d
jnohlgard pushed a commit to jnohlgard/RIOT that referenced this pull request Mar 20, 2015
mulle: Disable NVRAM API until PR RIOT-OS#2353 is merged
70458a6
jnohlgard pushed a commit to jnohlgard/RIOT that referenced this pull request Mar 21, 2015
mulle: Disable NVRAM API until PR RIOT-OS#2353 is merged
9490b19
@OlegHahm OlegHahm force-pushed the master branch 3 times, most recently from 9f184dd to 45554bf Compare March 31, 2015 13:01
@jnohlgard jnohlgard added the CI: needs squashing Commits in this PR need to be squashed; If set, CI systems will mark this PR as unmergable label Apr 28, 2015
@jnohlgard
Copy link
Member Author

I would like to merge this. My conclusion of the discussion here and in #2528 is that nobody is strongly opposed to this interface, and that the bus initialization can be left as is in the example.

@PeterKietzmann @haukepetersen @OlegHahm any ACKs/NACKs?

Btw, rebased.

@PeterKietzmann
Copy link
Member

@gebart ok! Had a look at it again and still looks reasonable to me, even if I was not able to test. ACK. Please squash.

@OlegHahm OlegHahm modified the milestone: Release 2015.06 Apr 29, 2015
@PeterKietzmann
Copy link
Member

Travis complains about whitespace errors. The rest seems to be fine

@jnohlgard
Copy link
Member Author

Corrected whitespace error. OK to squash?
edit: nevermind, I read that you already asked me to squash.

@jnohlgard jnohlgard removed Community: Hack'n'ACK candidate This PR is a candidate for review and discussion during one of RIOT's monthly Hack'n'ACK parties Discussion: RFC The issue/PR is used as a discussion starting point about the item of the issue/PR labels May 4, 2015
@jnohlgard jnohlgard removed the CI: needs squashing Commits in this PR need to be squashed; If set, CI systems will mark this PR as unmergable label May 4, 2015
@jnohlgard
Copy link
Member Author

@PeterKietzmann Is the commit list OK now or should I further squash everything into a single commit?

@PeterKietzmann
Copy link
Member

@gebart no. The commit-messages seem reasonable to me. ACK and go

PeterKietzmann added a commit that referenced this pull request May 4, 2015
@PeterKietzmann
Merge pull request #2353 from gebart/pr/nvram-api
74e076d 
drivers: Add NVRAM API
@PeterKietzmann PeterKietzmann merged commit 74e076d into RIOT-OS:master May 4, 2015
@PeterKietzmann
Copy link
Member

Jippie

@jnohlgard jnohlgard deleted the pr/nvram-api branch May 10, 2015 08:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@PeterKietzmann PeterKietzmann

Labels
Area: drivers Area: Device drivers Type: new feature The issue requests / The PR implemements a new feature for RIOT
Projects
None yet
Milestone
Release 2015.09
Development

Successfully merging this pull request may close these issues.
5 participants
@jnohlgard @thomaseichinger @OlegHahm @PeterKietzmann @haukepetersen