On Fri, May 08, 2026 at 04:42:45PM +0200, Gregor Herburger wrote:
> Raspberry Pis have OTP registers which can be accessed through the
> videocore firmware. Add a nvmem driver to support these OTP registers.
>
> Signed-off-by: Gregor Herburger <[email protected]>
> ---
> drivers/nvmem/Kconfig | 10 +++
> drivers/nvmem/Makefile | 1 +
> drivers/nvmem/raspberrypi-otp.c | 130
> +++++++++++++++++++++++++++++
> include/soc/bcm2835/raspberrypi-firmware.h | 9 ++
> 4 files changed, 150 insertions(+)
>
> diff --git a/drivers/nvmem/Kconfig b/drivers/nvmem/Kconfig
> index 74ddbd0f79b0..aac31f43385e 100644
> --- a/drivers/nvmem/Kconfig
> +++ b/drivers/nvmem/Kconfig
> @@ -483,4 +483,14 @@ config NVMEM_QORIQ_EFUSE
> This driver can also be built as a module. If so, the module
> will be called nvmem_qoriq_efuse.
>
> +config NVMEM_RASPBERRYPI_OTP
> + tristate "Raspberry Pi OTP support"
> + depends on RASPBERRYPI_FIRMWARE || (COMPILE_TEST &&
> !RASPBERRYPI_FIRMWARE)
The '&& !RASPBERRYPI_FIRMWARE' clause looks weird, is it really necessary?
> + help
> + This driver provides access to the Raspberry Pi OTP memory via the
> + nvmem subsystem. The driver supports the customer OTP as well as the
> + device specific private key OTP (BCM2712 only).
> +
> + This driver can also be built as a module. If so, the module
> + will be called raspberrypi-otp.
While we are here: The empty line here is now missing.
> endif
> diff --git a/drivers/nvmem/Makefile b/drivers/nvmem/Makefile
> index 7252b8ec88d4..8ca2095e068f 100644
> --- a/drivers/nvmem/Makefile
> +++ b/drivers/nvmem/Makefile
> @@ -95,3 +95,4 @@ obj-$(CONFIG_NVMEM_ZYNQMP) += nvmem_zynqmp_nvmem.o
> nvmem_zynqmp_nvmem-y := zynqmp_nvmem.o
> obj-$(CONFIG_NVMEM_QORIQ_EFUSE) += nvmem-qoriq-efuse.o
> nvmem-qoriq-efuse-y := qoriq-efuse.o
> +obj-$(CONFIG_NVMEM_RASPBERRYPI_OTP) += raspberrypi-otp.o
> diff --git a/drivers/nvmem/raspberrypi-otp.c b/drivers/nvmem/raspberrypi-otp.c
> new file mode 100644
> index 000000000000..393640cb9e32
> --- /dev/null
> +++ b/drivers/nvmem/raspberrypi-otp.c
> @@ -0,0 +1,130 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +
> +#include <linux/overflow.h>
This looks unused.
Instead maybe add linux/types.h, linux/align.h
> +#include <linux/module.h>
> +#include <linux/nvmem-provider.h>
> +#include <linux/platform_device.h>
> +#include <soc/bcm2835/raspberrypi-firmware.h>
> +
> +struct rpi_otp_priv {
> + struct rpi_firmware *fw;
> + struct device *dev;
This looks unused.
> + u32 read_tag;
> + u32 write_tag;
> +};
> +
> +struct rpi_otp_header {
> + __le32 start;
> + __le32 count;
> + __le32 data[] __counted_by_le(count);
> +};
> +
> +static int rpi_otp_read(void *context, unsigned int offset, void *buf,
> size_t bytes)
> +{
> + struct rpi_otp_priv *priv = context;
> + struct rpi_otp_header *fwbuf;
> + u32 count;
> + int ret;
> +
> + if (!IS_ALIGNED(offset, 4) || !IS_ALIGNED(bytes, 4))
> + return -EINVAL;
Isn't this already enforced by the nvmem core?
(...)
> +static int rpi_otp_probe(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct nvmem_device *nvmem;
> + struct rpi_otp_priv *priv;
> + const struct rpi_otp_driver_data *data;
> + struct nvmem_config config = {
> + .read_only = false,
> + .word_size = 4,
> + .stride = 4,
> + .reg_read = rpi_otp_read,
> + .reg_write = rpi_otp_write,
> + .id = NVMEM_DEVID_NONE,
> + };
> +
> + priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> + if (!priv)
> + return -ENOMEM;
> +
> + data = dev_get_platdata(dev);
> + if (!data)
> + return -ENODEV;
I would do this before the devm_kzalloc().
> +
> + priv->fw = dev_get_drvdata(dev->parent);
> + priv->dev = dev;
> + priv->read_tag = data->read_tag;
> + priv->write_tag = data->write_tag;
> + config.dev = dev;
> + config.priv = priv;
> + config.name = data->name;
> + config.size = data->size;
> +
> + nvmem = devm_nvmem_register(dev, &config);
> + if (IS_ERR(nvmem))
> + return dev_err_probe(dev, PTR_ERR(nvmem), "error registering
> nvmem config\n");
> +
> + return 0;
> +}
> +
> +static struct platform_driver raspberry_otp_driver = {
> + .probe = rpi_otp_probe,
> + .driver = {
> + .name = "raspberrypi-otp",
> + },
> +};
> +module_platform_driver(raspberry_otp_driver);
> +
> +MODULE_AUTHOR("Gregor Herburger <[email protected]>");
> +MODULE_DESCRIPTION("Raspberry Pi OTP driver");
> +MODULE_LICENSE("GPL");
> +MODULE_ALIAS("platform:raspberrypi-otp");
Instead of the manual module alias here and the implicit matching of the
platform driver this should use an explicit matching table:
static const struct platform_device_id foo_id[] = {
{ "raspberrypi-otp" },
{}
};
MODULE_DEVICE_TABLE(platform, foo_id);
static struct platform_driver raspberry_otp_driver = {
...
.id_table = foo_id,
};
module_platform_driver(raspberry_otp_driver);
(...)
