[PATCH v3 08/13] misc: Add support for nvmem cells
Simon Glass
sjg at chromium.org
Mon Apr 25 07:48:00 CEST 2022
Hi Sean,
On Mon, 18 Apr 2022 at 13:37, Sean Anderson <sean.anderson at seco.com> wrote:
>
> This adds support for "nvmem cells" as seen in Linux. The nvmem device
> class in Linux is used for various assorted ROMs and EEPROMs. In this
> sense, it is similar to UCLASS_MISC, but also includes
> UCLASS_I2C_EEPROM, UCLASS_RTC, and UCLASS_MTD. New drivers corresponding
> to a Linux-style nvmem device should be implemented as one of the
> previously-mentioned uclasses. The nvmem API acts as a compatibility
> layer to adapt the (slightly different) APIs of these uclasses. It also
> handles the lookup of nvmem cells.
>
> While nvmem devices can be accessed directly, they are most often used
> by reading/writing contiguous values called "cells". Cells typically
> hold information like calibration, versions, or configuration (such as
> mac addresses).
>
> nvmem devices can specify "cells" in their device tree:
>
> qfprom: eeprom at 700000 {
> #address-cells = <1>;
> #size-cells = <1>;
> reg = <0x00700000 0x100000>;
>
> /* ... */
>
> tsens_calibration: calib at 404 {
> reg = <0x404 0x10>;
> };
> };
>
> which can then be referenced like:
>
> tsens {
> /* ... */
> nvmem-cells = <&tsens_calibration>;
> nvmem-cell-names = "calibration";
> };
>
> The tsens driver could then read the calibration value like:
>
> struct nvmem_cell cal_cell;
> u8 cal[16];
> nvmem_cell_get_by_name(dev, "calibration", &cal_cell);
> nvmem_cell_read(&cal_cell, cal, sizeof(cal));
>
> Because nvmem devices are not all of the same uclass, supported uclasses
> must register a nvmem_interface struct. This allows CONFIG_NVMEM to be
> enabled without depending on specific uclasses. At the moment,
> nvmem_interface is very bare-bones, and assumes that no initialization
> is necessary. However, this could be amended in the future.
>
> Although I2C_EEPROM and MISC are quite similar (and could likely be
> unified), they present different read/write function signatures. To
> abstract over this, NVMEM uses the same read/write signature as Linux.
> In particular, short read/writes are not allowed, which is allowed by
> MISC.
>
> The functionality implemented by nvmem cells is very similar to that
> provided by i2c_eeprom_partition. "fixed-partition"s for eeproms does
> not seem to have made its way into Linux or into any device tree other
> than sandbox. It is possible that with the introduction of this API it
> would be possible to remove it.
I still think this would be better as a separate uclass, with child
devices created at bind time in each of the respective uclasses, like
mmc_bind() does. Then you will see the nvmem devices in the DM tree.
Wouldn't we want to add a command to access the nvmem devices? This
patch feels like a shortcut to me and I'm not sure of the benefit of
that shortcut.
>
> Signed-off-by: Sean Anderson <sean.anderson at seco.com>
> ---
>
> (no changes since v2)
>
> Changes in v2:
> - Call the appropriate API functions directly from
> nvmem_cell_(read|write). This means we can drop the nvmem_interface
> machinery.
>
> MAINTAINERS | 7 ++
> doc/api/index.rst | 1 +
> doc/api/nvmem.rst | 7 ++
> drivers/misc/Kconfig | 16 +++++
> drivers/misc/Makefile | 1 +
> drivers/misc/nvmem.c | 142 +++++++++++++++++++++++++++++++++++++++
> include/nvmem.h | 151 ++++++++++++++++++++++++++++++++++++++++++
> 7 files changed, 325 insertions(+)
> create mode 100644 doc/api/nvmem.rst
> create mode 100644 drivers/misc/nvmem.c
> create mode 100644 include/nvmem.h
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 34446127d4..5175607de2 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -1064,6 +1064,13 @@ F: cmd/nvme.c
> F: include/nvme.h
> F: doc/develop/driver-model/nvme.rst
>
> +NVMEM
> +M: Sean Anderson <seanga2 at gmail.com>
> +S: Maintained
> +F: doc/api/nvmem.rst
> +F: drivers/misc/nvmem.c
> +F: include/nvmem.h
> +
> NXP C45 TJA11XX PHY DRIVER
> M: Radu Pirea <radu-nicolae.pirea at oss.nxp.com>
> S: Maintained
> diff --git a/doc/api/index.rst b/doc/api/index.rst
> index 72fea981b7..a9338cfef9 100644
> --- a/doc/api/index.rst
> +++ b/doc/api/index.rst
> @@ -14,6 +14,7 @@ U-Boot API documentation
> linker_lists
> lmb
> logging
> + nvmem
> pinctrl
> rng
> sandbox
> diff --git a/doc/api/nvmem.rst b/doc/api/nvmem.rst
> new file mode 100644
> index 0000000000..15c9b5b839
> --- /dev/null
> +++ b/doc/api/nvmem.rst
> @@ -0,0 +1,7 @@
> +.. SPDX-License-Identifier: GPL-2.0+
> +
> +NVMEM API
> +=========
> +
> +.. kernel-doc:: include/nvmem.h
> + :internal:
> diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
> index 10fd601278..218dc18a1d 100644
> --- a/drivers/misc/Kconfig
> +++ b/drivers/misc/Kconfig
> @@ -31,6 +31,22 @@ config TPL_MISC
> set of generic read, write and ioctl methods may be used to
> access the device.
>
> +config NVMEM
> + bool "NVMEM support"
> + help
> + This adds support for a common interface to different types of
> + non-volatile memory. Consumers can use nvmem-cells properties to look
> + up hardware configuration data such as MAC addresses and calibration
> + settings.
> +
> +config SPL_NVMEM
> + bool "NVMEM support in SPL"
> + help
> + This adds support for a common interface to different types of
> + non-volatile memory. Consumers can use nvmem-cells properties to look
> + up hardware configuration data such as MAC addresses and calibration
> + settings.
> +
> config ALTERA_SYSID
> bool "Altera Sysid support"
> depends on MISC
> diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
> index 6150d01e88..03fad7a23f 100644
> --- a/drivers/misc/Makefile
> +++ b/drivers/misc/Makefile
> @@ -4,6 +4,7 @@
> # Wolfgang Denk, DENX Software Engineering, wd at denx.de.
>
> obj-$(CONFIG_MISC) += misc-uclass.o
> +obj-$(CONFIG_$(SPL_TPL_)NVMEM) += nvmem.o
>
> obj-$(CONFIG_$(SPL_TPL_)CROS_EC) += cros_ec.o
> obj-$(CONFIG_$(SPL_TPL_)CROS_EC_SANDBOX) += cros_ec_sandbox.o
> diff --git a/drivers/misc/nvmem.c b/drivers/misc/nvmem.c
> new file mode 100644
> index 0000000000..fd80a72394
> --- /dev/null
> +++ b/drivers/misc/nvmem.c
> @@ -0,0 +1,142 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright (C) 2022 Sean Anderson <sean.anderson at seco.com>
> + */
> +
> +#include <common.h>
> +#include <i2c_eeprom.h>
> +#include <linker_lists.h>
> +#include <misc.h>
> +#include <nvmem.h>
> +#include <rtc.h>
> +#include <dm/device_compat.h>
> +#include <dm/ofnode.h>
> +#include <dm/read.h>
> +#include <dm/uclass.h>
> +
> +int nvmem_cell_read(struct nvmem_cell *cell, void *buf, size_t size)
> +{
> + dev_dbg(cell->nvmem, "%s: off=%u size=%zu\n", __func__, cell->offset, size);
> + if (size != cell->size)
> + return -EINVAL;
> +
> + switch (cell->nvmem->driver->id) {
> + case UCLASS_I2C_EEPROM:
> + return i2c_eeprom_read(cell->nvmem, cell->offset, buf, size);
> + case UCLASS_MISC: {
> + int ret = misc_read(cell->nvmem, cell->offset, buf, size);
> +
> + if (ret < 0)
> + return ret;
> + if (ret != size)
> + return -EIO;
> + return 0;
> + }
> + case UCLASS_RTC:
> + return dm_rtc_read(cell->nvmem, cell->offset, buf, size);
> + default:
> + return -ENOSYS;
> + }
> +}
> +
> +int nvmem_cell_write(struct nvmem_cell *cell, const void *buf, size_t size)
> +{
> + dev_dbg(cell->nvmem, "%s: off=%u size=%zu\n", __func__, cell->offset, size);
> + if (size != cell->size)
> + return -EINVAL;
> +
> + switch (cell->nvmem->driver->id) {
> + case UCLASS_I2C_EEPROM:
> + return i2c_eeprom_write(cell->nvmem, cell->offset, buf, size);
> + case UCLASS_MISC: {
> + int ret = misc_write(cell->nvmem, cell->offset, buf, size);
> +
> + if (ret < 0)
> + return ret;
> + if (ret != size)
> + return -EIO;
> + return 0;
> + }
> + case UCLASS_RTC:
> + return dm_rtc_write(cell->nvmem, cell->offset, buf, size);
> + default:
> + return -ENOSYS;
> + }
> +}
> +
> +/**
> + * nvmem_get_device() - Get an nvmem device for a cell
> + * @node: ofnode of the nvmem device
> + * @cell: Cell to look up
> + *
> + * Try to find a nvmem-compatible device by going through the nvmem interfaces.
> + *
> + * Return:
> + * * 0 on success
> + * * -ENODEV if we didn't find anything
> + * * A negative error if there was a problem looking up the device
> + */
> +static int nvmem_get_device(ofnode node, struct nvmem_cell *cell)
> +{
> + int i, ret;
> + enum uclass_id ids[] = {
> + UCLASS_I2C_EEPROM,
> + UCLASS_MISC,
> + UCLASS_RTC,
> + };
> +
> + for (i = 0; i < ARRAY_SIZE(ids); i++) {
> + ret = uclass_get_device_by_ofnode(ids[i], node, &cell->nvmem);
> + if (!ret)
> + return 0;
> + if (ret != -ENODEV)
> + return ret;
> + }
> +
> + return -ENODEV;
> +}
> +
> +int nvmem_cell_get_by_index(struct udevice *dev, int index,
> + struct nvmem_cell *cell)
> +{
> + fdt_addr_t offset;
> + fdt_size_t size = FDT_SIZE_T_NONE;
> + int ret;
> + struct ofnode_phandle_args args;
> +
> + dev_dbg(dev, "%s: index=%d\n", __func__, index);
> +
> + ret = dev_read_phandle_with_args(dev, "nvmem-cells", NULL, 0, index,
> + &args);
> + if (ret)
> + return ret;
> +
> + ret = nvmem_get_device(ofnode_get_parent(args.node), cell);
> + if (ret)
> + return ret;
> +
> + offset = ofnode_get_addr_size_index_notrans(args.node, 0, &size);
> + if (offset == FDT_ADDR_T_NONE || size == FDT_SIZE_T_NONE) {
> + dev_dbg(cell->nvmem, "missing address or size for %s\n",
> + ofnode_get_name(args.node));
> + return -EINVAL;
> + }
> +
> + cell->offset = offset;
> + cell->size = size;
> + return 0;
> +}
> +
> +int nvmem_cell_get_by_name(struct udevice *dev, const char *name,
> + struct nvmem_cell *cell)
> +{
> + int index;
> +
> + dev_dbg(dev, "%s, name=%s\n", __func__, name);
> +
> + index = dev_read_stringlist_search(dev, "nvmem-cell-names", name);
> + if (index < 0)
> + return index;
> +
> + return nvmem_cell_get_by_index(dev, index, cell);
> +}
> diff --git a/include/nvmem.h b/include/nvmem.h
> new file mode 100644
> index 0000000000..2751713a68
> --- /dev/null
> +++ b/include/nvmem.h
> @@ -0,0 +1,151 @@
> +/* SPDX-License-Identifier: GPL-2.0+ */
> +/*
> + * Copyright (c) 2022 Sean Anderson <sean.anderson at seco.com>
> + */
> +
> +#ifndef NVMEM_H
> +#define NVMEM_H
> +
> +/**
> + * typedef nvmem_reg_read_t - Read a register from an nvmem device
> + *
> + * @dev: The device to read from
> + * @offset: The offset of the register from the beginning of @dev
> + * @buf: The buffer to read into
> + * @size: The size of @buf, in bytes
> + *
> + * Return:
> + * * 0 on success
> + * * A negative error on failure
> + */
> +typedef int (*nvmem_reg_read_t)(struct udevice *dev, unsigned int offset,
> + void *buf, size_t size);
> +
> +/**
> + * typedef nvmem_reg_write_t - Write a register to an nvmem device
> + * @dev: The device to write
> + * @offset: The offset of the register from the beginning of @dev
> + * @buf: The buffer to write
> + * @size: The size of @buf, in bytes
> + *
> + * Return:
> + * * 0 on success
> + * * -ENOSYS if the device is read-only
> + * * A negative error on other failures
> + */
> +typedef int (*nvmem_reg_write_t)(struct udevice *dev, unsigned int offset,
> + const void *buf, size_t size);
> +
> +/**
> + * struct nvmem_cell - One datum within non-volatile memory
> + * @nvmem: The backing storage device
> + * @offset: The offset of the cell from the start of @nvmem
> + * @size: The size of the cell, in bytes
> + */
> +struct nvmem_cell {
> + struct udevice *nvmem;
> + unsigned int offset;
> + size_t size;
> +};
> +
> +struct udevice;
nit: can you put this at the top of the file, after any #define stuff
but before declarations?
> +
> +#if CONFIG_IS_ENABLED(NVMEM)
> +
> +/**
> + * nvmem_cell_read() - Read the value of an nvmem cell
> + * @cell: The nvmem cell to read
> + * @buf: The buffer to read into
> + * @size: The size of @buf
> + *
> + * Return:
> + * * 0 on success
> + * * -EINVAL if @buf is not the same size as @cell.
> + * * -ENOSYS if CONFIG_NVMEM is disabled
> + * * A negative error if there was a problem reading the underlying storage
> + */
> +int nvmem_cell_read(struct nvmem_cell *cell, void *buf, size_t size);
> +
> +/**
> + * nvmem_cell_write() - Write a value to an nvmem cell
> + * @cell: The nvmem cell to write
> + * @buf: The buffer to write from
> + * @size: The size of @buf
> + *
> + * Return:
> + * * 0 on success
> + * * -EINVAL if @buf is not the same size as @cell
> + * * -ENOSYS if @cell is read-only, or if CONFIG_NVMEM is disabled
> + * * A negative error if there was a problem writing the underlying storage
> + */
> +int nvmem_cell_write(struct nvmem_cell *cell, const void *buf, size_t size);
> +
> +/**
> + * nvmem_cell_get_by_index() - Get an nvmem cell from a given device and index
> + * @dev: The device that uses the nvmem cell
> + * @index: The index of the cell in nvmem-cells
> + * @cell: The cell to initialize
> + *
> + * Look up the nvmem cell referenced by the phandle at @index in nvmem-cells in
> + * @dev.
> + *
> + * Return:
> + * * 0 on success
> + * * -EINVAL if the regs property is missing, empty, or undersized
> + * * -ENODEV if the nvmem device is missing or unimplemented
> + * * -ENOSYS if CONFIG_NVMEM is disabled
> + * * A negative error if there was a problem reading nvmem-cells or getting the
> + * device
> + */
> +int nvmem_cell_get_by_index(struct udevice *dev, int index,
> + struct nvmem_cell *cell);
> +
> +/**
> + * nvmem_cell_get_by_name() - Get an nvmem cell from a given device and name
> + * @dev: The device that uses the nvmem cell
> + * @name: The name of the nvmem cell
> + * @cell: The cell to initialize
> + *
> + * Look up the nvmem cell referenced by @name in the nvmem-cell-names property
> + * of @dev.
> + *
> + * Return:
> + * * 0 on success
> + * * -EINVAL if the regs property is missing, empty, or undersized
> + * * -ENODEV if the nvmem device is missing or unimplemented
> + * * -ENODATA if @name is not in nvmem-cell-names
> + * * -ENOSYS if CONFIG_NVMEM is disabled
> + * * A negative error if there was a problem reading nvmem-cell-names,
> + * nvmem-cells, or getting the device
> + */
> +int nvmem_cell_get_by_name(struct udevice *dev, const char *name,
> + struct nvmem_cell *cell);
> +
> +#else /* CONFIG_NVMEM */
> +
> +static inline int nvmem_cell_read(struct nvmem_cell *cell, void *buf, int size)
> +{
> + return -ENOSYS;
> +}
> +
> +static inline int nvmem_cell_write(struct nvmem_cell *cell, const void *buf,
> + int size)
> +{
> + return -ENOSYS;
> +}
> +
> +static inline int nvmem_cell_get_by_index(struct udevice *dev, int index,
> + struct nvmem_cell *cell)
> +{
> + return -ENOSYS;
> +}
> +
> +static inline int nvmem_cell_get_by_name(struct udevice *dev, const char *name,
> + struct nvmem_cell *cell)
> +{
> + return -ENOSYS;
> +}
> +
> +#endif /* CONFIG_NVMEM */
> +
> +#endif /* NVMEM_H */
> --
> 2.35.1.1320.gc452695387.dirty
>
Regards,
Simon
More information about the U-Boot
mailing list