-
Notifications
You must be signed in to change notification settings - Fork 8.2k
drivers: fuse: add fuse driver subsystem with STM32 Ethernet MAC adress loading #98149
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
base: main
Are you sure you want to change the base?
Conversation
WIP Signed-off-by: Pieter De Gendt <[email protected]>
Given an ethernet MAC configuration struct, load a MAC address into a provided array. Signed-off-by: Pieter De Gendt <[email protected]>
Turn ethernet controller devicetree nodes into NVMEM consumers. Allow passing mac-address cells. Signed-off-by: Pieter De Gendt <[email protected]>
Rework the Atmel SAM GMAC driver to read a MAC address from the provided NVMEM cell instead of using I2C commands directly. Signed-off-by: Pieter De Gendt <[email protected]>
Introduce a new fuse subsystem to be able to interact with One Time Programmable memory(OTP/fuses). For now, add basic read()/program() APIs. Add the fuse_stm32_bsec driver as first user to be able to access OTPs on some STM32 platforms. File drivers/fuse.h is inspired by drivers/eeprom.h as the basic features are similar. Signed-off-by: Gatien Chevallier <[email protected]>
Fuse drivers are the interface to One Time Programmable memory, which is NVMEM. Add the interface with the fuse API to the NVMEM one. Signed-off-by: Gatien Chevallier <[email protected]>
This new API handles if the MAC address should be fetched from NVMEM, is static or be randomly generated. Use it so that the driver can fetch the MAC address from the OTP fuses, when possible. Signed-off-by: Gatien Chevallier <[email protected]>
The STM32N6 has a BSEC peripheral for the OTP management, add it to the SoC device tree file. Moreover, there are 4 OTP words dedicated for Ethernet MAC addresses, describe them in the NVMEM layout. Signed-off-by: Gatien Chevallier <[email protected]>
Use the "mac_address0" NVMEM cell to be able to read the ethernet MAC address from the OTP fuses. Signed-off-by: Gatien Chevallier <[email protected]>
|
Hello @GseoC, and thank you very much for your first pull request to the Zephyr project! |
|
|
I have added the Architecture review label to ensure the proposed fuse subsystem gets discussed there before introducing it. |
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.
Thanks for opening this PR! This is a great starting point to discuss in the Arch meeting group, I think.
I went with a different approach to use the syscon API in #98140, either works for me, but we'll need to come to resolution on that first.
| typedef int (*fuse_api_read)(const struct device *dev, off_t offset, void *data, size_t len); | ||
|
|
||
| /** | ||
| * @brief Callback API upon writing to the fuse. | ||
| * See @a fuse_program() for argument description | ||
| */ | ||
| typedef int (*fuse_api_program)(const struct device *dev, off_t offset, const void *data, | ||
| size_t 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.
these are basically identical to the eeprom api. I don't think we need a new api for fuses, if just the name is different
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.
agree with you, see discussion in this PR #98140.
What we need is NVMEM device driver model, NVMEM device driver model includes FUSE and EEPROM.
I have a reference implementations main...nxp-upstream:zephyr:add-nvmem-device-driver-model, which can read the correction data in OTP through NVMEM in the sensor driver
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.
As mentionned, the API is quite basic for now, but there are mechanism specific to fuses that can and should be implemented such as shadowing, locking, etc...
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'm also up for an NVMEM device driver model but then we need to implement a provider registering mechanism.
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.
The driver can be the provider already? Thanks @ZhaoxiangJin for your proposal too, let's bring it together in the Arch meeting.
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.
The main thing with fuses, otp or lockable security registers, ist that the way to to program them varies greatly. combining all the ways into a api will get difficult. Especially if we want a similar behavior for different devices, when using the api. Most abstraction would be to only have a write, which has to do everything. Also means no difference to the eeprom api. or expose lots of functionality, but then it is not really portable and the use of it is still peripheral specific. Do we even should provide a common api for writing fuses, might providing only the read part be enough?
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 think it's important to differentiate from EEPROM, this has not the same constraints and consequences, even if the api could be similar.
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.
there are plenty of API in the world with the same function signatures, does that mean they all do the same things? no, of course not. so this block is wrong.
mathieuchopstm
left a comment
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.
Ignoring the architectural concern:
| select FUSE | ||
| select NVMEM |
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.
Maybe if SOC_SERIES_N6X?
| if (len == 0U) { | ||
| LOG_ERR("Read OTP with a len of 0"); | ||
| return -EPERM; | ||
| } |
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 should probably be brushed off with a return 0; instead?
| return -EINVAL; | ||
| } | ||
|
|
||
| handle.Instance = BSEC; |
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.
Should come from DTS (even if there will presumably never be two BSEC in a single product...)
| if (IS_ENABLED(CONFIG_NVMEM_FUSE) && DEVICE_API_IS(fuse, cell->dev)) { | ||
| return fuse_program(cell->dev, cell->offset + off, buf, 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.
Why don't we have a simple "nvmem" driver class, that acts as a nvmem provider, and wrap there, e.g. EEPROM, and use the dt_spec pattern? Looking at the nvmem subsys, it feels really weird and not with the patterns I'd expect in Zephyr.
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.
For reference, I'm interested in getting this done as I also have some WIP for SF32LB platform (rough draft here where I tried to sketch something following some Linux ideas: https://github.com/teslabs/zephyr/commits/sf32lb-hwinfo/)
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.
@pdgendt and @ZhaoxiangJin were debating about this on another PR. I think the question to be answered first is what is the problem space we are trying to solve and what requirements are there for a solution to those problems. Because otherwise I don't know how to compare the two ideas.
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.
Right, so there are different approaches currently, similar from @ZhaoxiangJin here main...nxp-upstream:zephyr:add-nvmem-device-driver-model
We can have a discussion during the feature freeze, I'll make an issue to gather some of the information.
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 will also organize my proposal and provide solutions to the issue raised by Fin(maass-hamburg) and Erwan(erwango)
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 opened #98184 that gathers some of the feedback in the different PRs/branches.



Hello,
This P-R has dependencies on #96598 (includes it).
The STM32N6 boards embeds the BSEC peripheral that offers an interface to read/program One Time Programmable (OTP) fuses. One of the possible use-case for using OTP is fusing Ethernet MAC addresses at manufacturing level.
This way, every product embeds its own MAC address.
Add a fuse subsystem destined to welcome all fuse handling drivers.
Simply define read()/program() APIs as a starting point.
Then, plug the fuse API to the NVMEM subsystem so that the STM32 Ethernet driver can get its MAC address through it.
This use-case will be extended to STM32MPU boards afterwards.