-
Notifications
You must be signed in to change notification settings - Fork 8k
Add NVMEM support #96379
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 NVMEM support #96379
Conversation
3950927
to
4536d4e
Compare
@pdgendt this is a nice addition. Can it be extended to other backends (e.g. fuses, bbram) ? IMHO it should be straightforward to add a settings backend for this (e.g. store/retrieve items under "nvmem/xxx"). |
That's the idea, in
Sure, maybe. Outside of the scope of this PR though. |
91b7c21
to
1ae57e1
Compare
Removed the draft state to get some early feedback before adding more documentation bits. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please review linux binding https://www.kernel.org/doc/Documentation/devicetree/bindings/nvmem/ , note also the layouts folder
aeee230
to
c1a5bc6
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like it! Simple, yet extensible and a good level of abstraction. A few comments below:
cell_nodelabel: cell@START_OFFSET { | ||
reg = <0xSTART_OFFSET 0xSIZE>; | ||
#nvmem-cell-cells = <0>; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hello Pieter, what are you planning to do with #nvmem-cell-cells? Isn't it enough to work with reg and #address-cells + #size-cells. It's not documented for now so might as well remove it if not necessary?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is required to work with phandle-array
binding types (see the docs). I didn't want to introduce special treatment in some of the python code for this.
I don't have plans for this, but it can be used for future types of NVMEM providers. I can't give an example from the top of my head though.
* #size-cells = <1>; | ||
* mac_address: mac_address@fa { | ||
* reg = <0xfa 0x06>; | ||
* #nvmem-cell-cells = <0>; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If not necessary (previous comment), can remove occurences in this file.
|
||
int nvmem_cell_read(const struct nvmem_cell *cell, void *buf, off_t off, size_t len) | ||
{ | ||
if (off < 0 || cell->size < off + len) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is it necessary to check if cell
isn't NULL? ditto in other exposed APIs if so.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IMO it's up to the caller to check for non-NULL cells, same for buf
, the contract of the API to the pass a pointer to a struct nvmem_cell
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've updated the PR with asserts for non-NULL cell pointers.
* | ||
* @kconfig_dep{CONFIG_NVMEM} | ||
* | ||
* @return 0 on success, negative errno code on failure. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
some of these use @return
some use @retval
, I vaugley remember that @retval
needs multiple lines one per value, unsure on @return
or if one should be used over the other
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Had one (wrong) use of @retval
, updated, thanks!
Adds property nvmem-cells for referencing a node that stores some configuration information. A typical use case is the reading of MAC address from an EEPROM device. Signed-off-by: Andriy Gelman <[email protected]> Signed-off-by: Pieter De Gendt <[email protected]>
Add a devicetree binding to represent a fixed layout in an NVMEM provider. Signed-off-by: Pieter De Gendt <[email protected]>
This commit adds the NVMEM subsystem with a basic implementation for use with EEPROM devices. Signed-off-by: Pieter De Gendt <[email protected]>
Add basic testing to validate the API of the NVMEM subsystem. Add testing with an EEPROM backend. Signed-off-by: Pieter De Gendt <[email protected]>
Add test cases tot the devicetree API testsuite for NVMEM macros. Signed-off-by: Pieter De Gendt <[email protected]>
Introduce a page for the Non-Volatile Memory subsystem with a reference to the doxygen API. Signed-off-by: Pieter De Gendt <[email protected]>
Add an entry to the API and options section for the Non-Volatile Memory subsystem. Signed-off-by: Pieter De Gendt <[email protected]>
Create a section for the newly maintained NVMEM area. Signed-off-by: Pieter De Gendt <[email protected]>
Can this be used with SoC Flash, external NOR Flash? |
|
Current implementation in this PR only supports EEPROMs as NVMEM providers, but this can very easily be extended in the future. |
I guess it can, especially for read-only cells. The tricky bit would probably be writing, as it could require additional erase logic. |
nice stuff, i have seen this on arch wg agenda last week, are there some minutes somewhere with the outcome? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Binding looks like a good start, no need to boil the ocean and support everything out of the box IMO.
Not sure if notes were taken, I presented the idea and what it is trying to solve (the MAC address logic in network drivers for me), feedback was very positive overall, as far as I can remember. |
Architecture WG meeting, 2025-09-30:
|
My apologies, just realised I never got around to cleanup my notes from the meeting and actually post them. |
Introduce the NVMEM subsystem. Inspired from linux this subsystem allows to read and write data to Non-Volatile memory, for example to store a MAC address or SoC/Device specific data.
It comes with support for EEPROM devices. The device API backend is determined at runtime using
DEVICE_API_IS
.See #96598 for an example use case.
Derived from #74835