This adds a test to make sure that all the ethernet interfaces have their addresses read properly. At the moment everything is read from the environment, but the next few commits will add additional sources.

Signed-off-by: Sean Anderson sean.anderson@seco.com Reviewed-by: Simon Glass sjg@chromium.org ---

(no changes since v3)

Changes in v3: - Move patch adding test earlier in the series - Add test for eth8 as well

test/dm/eth.c | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+)

diff --git a/test/dm/eth.c b/test/dm/eth.c index e4ee695610..5437f9ea4a 100644 --- a/test/dm/eth.c +++ b/test/dm/eth.c @@ -147,6 +147,35 @@ static int dm_test_eth_act(struct unit_test_state *uts) } DM_TEST(dm_test_eth_act, UT_TESTF_SCAN_FDT);

+/* Ensure that all addresses are loaded properly */ +static int dm_test_ethaddr(struct unit_test_state *uts) +{ + static const char *const addr[] = { + "02:00:11:22:33:44", + "02:00:11:22:33:48", /* dsa slave */ + "02:00:11:22:33:45", + "02:00:11:22:33:48", /* dsa master */ + "02:00:11:22:33:46", + "02:00:11:22:33:47", + "02:00:11:22:33:48", /* dsa slave */ + "02:00:11:22:33:49", + }; + int i; + + for (i = 0; i < ARRAY_SIZE(addr); i++) { + char addrname[10]; + + if (i) + snprintf(addrname, sizeof(addrname), "eth%daddr", i + 1); + else + strcpy(addrname, "ethaddr"); + ut_asserteq_str(addr[i], env_get(addrname)); + } + + return 0; +} +DM_TEST(dm_test_ethaddr, UT_TESTF_SCAN_FDT); + /* The asserts include a return on fail; cleanup in the caller */ static int _dm_test_eth_rotate1(struct unit_test_state *uts) {

DSA interfaces use the same mac address for each interface, unless instructed otherwise. Just set eth4addr and let eth2addr and eth7addr be set automatically.

Signed-off-by: Sean Anderson sean.anderson@seco.com Reviewed-by: Simon Glass sjg@chromium.org ---

(no changes since v1)

board/sandbox/sandbox.env | 1 - 1 file changed, 1 deletion(-)

diff --git a/board/sandbox/sandbox.env b/board/sandbox/sandbox.env index 6dedc755fa..88ed7a9606 100644 --- a/board/sandbox/sandbox.env +++ b/board/sandbox/sandbox.env @@ -6,7 +6,6 @@ stdout=serial,vidconsole stderr=serial,vidconsole

ethaddr=02:00:11:22:33:44 -eth2addr=02:00:11:22:33:48 eth3addr=02:00:11:22:33:45 eth4addr=02:00:11:22:33:48 eth5addr=02:00:11:22:33:46

If the DSA master fails to probe for whatever reason, then DSA devices will continue on as if nothing is wrong. This can cause incorrect behavior. In particular, on sandbox, dsa_sandbox_probe attempts to access the master's private data. This is only safe to do if the master has been probed first. Fix this by probing the master after we look it up, and bailing out if we get an error.

Fixes: fc054d563b ("net: Introduce DSA class for Ethernet switches") Signed-off-by: Sean Anderson sean.anderson@seco.com ---

Changes in v4: - New

net/dsa-uclass.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/net/dsa-uclass.c b/net/dsa-uclass.c index 9ff55a02fb..3bf4351c84 100644 --- a/net/dsa-uclass.c +++ b/net/dsa-uclass.c @@ -477,8 +477,10 @@ static int dsa_pre_probe(struct udevice *dev) return -ENODEV; }

- uclass_find_device_by_ofnode(UCLASS_ETH, pdata->master_node, - &priv->master_dev); + err = uclass_get_device_by_ofnode(UCLASS_ETH, pdata->master_node, + &priv->master_dev); + if (err) + return err;

/* Simulate a probing event for the CPU port */ if (ops->port_probe) {

i2c_eeprom_ops->write uses a const buf, so use one for the wrapper function as well.

Signed-off-by: Sean Anderson sean.anderson@seco.com Reviewed-by: Simon Glass sjg@chromium.org ---

(no changes since v1)

drivers/misc/i2c_eeprom.c | 3 ++- include/i2c_eeprom.h | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/misc/i2c_eeprom.c b/drivers/misc/i2c_eeprom.c index 89a450d0f8..4302e180ac 100644 --- a/drivers/misc/i2c_eeprom.c +++ b/drivers/misc/i2c_eeprom.c @@ -33,7 +33,8 @@ int i2c_eeprom_read(struct udevice *dev, int offset, uint8_t *buf, int size) return ops->read(dev, offset, buf, size); }

-int i2c_eeprom_write(struct udevice *dev, int offset, uint8_t *buf, int size) +int i2c_eeprom_write(struct udevice *dev, int offset, const uint8_t *buf, + int size) { const struct i2c_eeprom_ops *ops = device_get_ops(dev);

diff --git a/include/i2c_eeprom.h b/include/i2c_eeprom.h index 3ad565684f..90fdb25232 100644 --- a/include/i2c_eeprom.h +++ b/include/i2c_eeprom.h @@ -42,7 +42,8 @@ int i2c_eeprom_read(struct udevice *dev, int offset, uint8_t *buf, int size); * * Return: 0 on success, -ve on failure */ -int i2c_eeprom_write(struct udevice *dev, int offset, uint8_t *buf, int size); +int i2c_eeprom_write(struct udevice *dev, int offset, const uint8_t *buf, + int size);

/* * i2c_eeprom_size() - get size of I2C EEPROM chip

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@700000 { #address-cells = <1>; #size-cells = <1>; reg = <0x00700000 0x100000>;

/* ... */

tsens_calibration: calib@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.

Signed-off-by: Sean Anderson sean.anderson@seco.com ---

Changes in v4: - Fix failing to find nvmem devices if some supported uclasses were not enabled. - Remove typedefs from documentation - Document some of the design background, and why a new uclass was not created.

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 | 10 +++ drivers/misc/Kconfig | 16 +++++ drivers/misc/Makefile | 1 + drivers/misc/nvmem.c | 142 ++++++++++++++++++++++++++++++++++++++++++ include/nvmem.h | 134 +++++++++++++++++++++++++++++++++++++++ 7 files changed, 311 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 56be0bfad0..01f4581d6a 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -1080,6 +1080,13 @@ F: cmd/nvme.c F: include/nvme.h F: doc/develop/driver-model/nvme.rst

+NVMEM +M: Sean Anderson seanga2@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@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..d923784652 --- /dev/null +++ b/doc/api/nvmem.rst @@ -0,0 +1,10 @@ +.. SPDX-License-Identifier: GPL-2.0+ + +NVMEM API +========= + +.. kernel-doc:: include/nvmem.h + :doc: Design + +.. kernel-doc:: include/nvmem.h + :internal: diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig index dab7a99c68..eaf7bb63ba 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@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..5a2bd1f9f7 --- /dev/null +++ b/drivers/misc/nvmem.c @@ -0,0 +1,142 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * Copyright (C) 2022 Sean Anderson sean.anderson@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 && ret != -EPFNOSUPPORT) + 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..822e698bdd --- /dev/null +++ b/include/nvmem.h @@ -0,0 +1,134 @@ +/* SPDX-License-Identifier: GPL-2.0+ */ +/* + * Copyright (c) 2022 Sean Anderson sean.anderson@seco.com + */ + +#ifndef NVMEM_H +#define NVMEM_H + +/** + * DOC: Design + * + * The NVMEM subsystem is a "meta-uclass" in that it abstracts over several + * different uclasses all with read/write APIs. One approach to implementing + * this could be to add a new sub-device for each nvmem-style device of + * UCLASS_NVMEM. This subsystem has taken the approach of using the existing + * access methods (i2c_eeprom_write, misc_write, etc.) directly. This has the + * advantage of not requiring an extra device/driver, saving on binary size and + * runtime memory usage. On the other hand, it is not idiomatic. Similar + * efforts should generally use a new uclass. + */ + +/** + * 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; + +#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 */

This adds support for reading mac addresses from the "mac-address" nvmem cell. If there is no (local-)mac-address property, then we will try reading from an nvmem cell.

For some existing examples of this property, refer to imx8mn.dtsi and imx8mp.dtsi. Unfortunately, fuse drivers have not yet been converted to DM.

Signed-off-by: Sean Anderson sean.anderson@seco.com Reviewed-by: Simon Glass sjg@chromium.org ---

(no changes since v1)

net/eth-uclass.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/net/eth-uclass.c b/net/eth-uclass.c index bcefc54ded..0f6b45b002 100644 --- a/net/eth-uclass.c +++ b/net/eth-uclass.c @@ -14,6 +14,7 @@ #include <env.h> #include <log.h> #include <net.h> +#include <nvmem.h> #include <asm/global_data.h> #include <dm/device-internal.h> #include <dm/uclass-internal.h> @@ -507,17 +508,21 @@ static bool eth_dev_get_mac_address(struct udevice *dev, u8 mac[ARP_HLEN]) { #if CONFIG_IS_ENABLED(OF_CONTROL) const uint8_t *p; + struct nvmem_cell mac_cell;

p = dev_read_u8_array_ptr(dev, "mac-address", ARP_HLEN); if (!p) p = dev_read_u8_array_ptr(dev, "local-mac-address", ARP_HLEN);

- if (!p) + if (p) { + memcpy(mac, p, ARP_HLEN); + return true; + } + + if (nvmem_cell_get_by_name(dev, "mac-address", &mac_cell)) return false;

- memcpy(mac, p, ARP_HLEN); - - return true; + return !nvmem_cell_read(&mac_cell, mac, ARP_HLEN); #else return false; #endif

This uses the nvmem API to load a mac address from an RTC.

Signed-off-by: Sean Anderson sean.anderson@seco.com Reviewed-by: Simon Glass sjg@chromium.org ---

(no changes since v1)

arch/sandbox/dts/test.dts | 9 ++++++++- drivers/rtc/i2c_rtc_emul.c | 10 ++++++++++ 2 files changed, 18 insertions(+), 1 deletion(-)

diff --git a/arch/sandbox/dts/test.dts b/arch/sandbox/dts/test.dts index 9aed071aa0..3e0b454efb 100644 --- a/arch/sandbox/dts/test.dts +++ b/arch/sandbox/dts/test.dts @@ -558,7 +558,8 @@ dsa_eth0: dsa-test-eth { compatible = "sandbox,eth"; reg = <0x10006000 0x1000>; - mac-address = [ 02 00 11 22 33 48 ]; + nvmem-cells = <&eth4_addr>; + nvmem-cell-names = "mac-address"; };

dsa-test { @@ -722,9 +723,15 @@ };

rtc_0: rtc@43 { + #address-cells = <1>; + #size-cells = <1>; reg = <0x43>; compatible = "sandbox-rtc"; sandbox,emul = <&emul0>; + + eth4_addr: mac-address@40 { + reg = <0x40 6>; + }; };

rtc_1: rtc@61 { diff --git a/drivers/rtc/i2c_rtc_emul.c b/drivers/rtc/i2c_rtc_emul.c index ba418c25da..c307d6036d 100644 --- a/drivers/rtc/i2c_rtc_emul.c +++ b/drivers/rtc/i2c_rtc_emul.c @@ -203,6 +203,15 @@ static int sandbox_i2c_rtc_bind(struct udevice *dev) return 0; }

+static int sandbox_i2c_rtc_probe(struct udevice *dev) +{ + const u8 mac[] = { 0x02, 0x00, 0x11, 0x22, 0x33, 0x48 }; + struct sandbox_i2c_rtc_plat_data *plat = dev_get_plat(dev); + + memcpy(&plat->reg[0x40], mac, sizeof(mac)); + return 0; +} + static const struct udevice_id sandbox_i2c_rtc_ids[] = { { .compatible = "sandbox,i2c-rtc-emul" }, { } @@ -213,6 +222,7 @@ U_BOOT_DRIVER(sandbox_i2c_rtc_emul) = { .id = UCLASS_I2C_EMUL, .of_match = sandbox_i2c_rtc_ids, .bind = sandbox_i2c_rtc_bind, + .probe = sandbox_i2c_rtc_probe, .priv_auto = sizeof(struct sandbox_i2c_rtc), .plat_auto = sizeof(struct sandbox_i2c_rtc_plat_data), .ops = &sandbox_i2c_rtc_emul_ops,

Instead of calling uclass methods directly, instead create some nvmem devices solely for the purpose of holding nvmem ops. This is primarily to illustrate the size difference between these approaches.

This patch should not be applied.

Signed-off-by: Sean Anderson sean.anderson@seco.com Patch-prefix: RFC ---

Changes in v4: - New

doc/api/nvmem.rst | 3 -- drivers/misc/i2c_eeprom.c | 37 ++++++++++++++ drivers/misc/misc-uclass.c | 58 +++++++++++++++++++-- drivers/misc/nvmem.c | 100 ++++++++++++------------------------- drivers/rtc/rtc-uclass.c | 46 +++++++++++++++-- include/dm/uclass-id.h | 1 + include/nvmem.h | 79 +++++++++++++++++++++++------ 7 files changed, 233 insertions(+), 91 deletions(-)

diff --git a/doc/api/nvmem.rst b/doc/api/nvmem.rst index d923784652..15c9b5b839 100644 --- a/doc/api/nvmem.rst +++ b/doc/api/nvmem.rst @@ -3,8 +3,5 @@ NVMEM API =========

-.. kernel-doc:: include/nvmem.h - :doc: Design - .. kernel-doc:: include/nvmem.h :internal: diff --git a/drivers/misc/i2c_eeprom.c b/drivers/misc/i2c_eeprom.c index 4302e180ac..3f6c7ebf4a 100644 --- a/drivers/misc/i2c_eeprom.c +++ b/drivers/misc/i2c_eeprom.c @@ -14,6 +14,7 @@ #include <dm/device-internal.h> #include <i2c.h> #include <i2c_eeprom.h> +#include <nvmem.h>

struct i2c_eeprom_drv_data { u32 size; /* size in bytes */ @@ -374,7 +375,43 @@ U_BOOT_DRIVER(i2c_eeprom_partition) = { .ops = &i2c_eeprom_partition_ops, };

+static int i2c_eeprom_nvmem_read(struct udevice *dev, unsigned int offset, + void *buf, size_t size) +{ + return i2c_eeprom_read(dev_get_parent(dev), offset, buf, size); +} + +static int i2c_eeprom_nvmem_write(struct udevice *dev, unsigned int offset, + const void *buf, size_t size) +{ + return i2c_eeprom_write(dev_get_parent(dev), offset, buf, size); +} + +static struct __maybe_unused nvmem_ops i2c_eeprom_nvmem_ops = { + .read = i2c_eeprom_nvmem_read, + .write = i2c_eeprom_nvmem_write, +}; + +#if CONFIG_IS_ENABLED(NVMEM) +U_BOOT_DRIVER(i2c_eeprom_nvmem) = { + .name = "i2c_eeprom_nvmem", + .id = UCLASS_NVMEM, + .ops = &i2c_eeprom_nvmem_ops, + .flags = DM_FLAG_NAME_ALLOCED, +}; +#endif + +#if CONFIG_IS_ENABLED(NVMEM) +static int i2c_eeprom_post_bind(struct udevice *dev) +{ + return nvmem_register(dev, DM_DRIVER_GET(misc_nvmem)); +} +#endif + UCLASS_DRIVER(i2c_eeprom) = { .id = UCLASS_I2C_EEPROM, .name = "i2c_eeprom", +#if CONFIG_IS_ENABLED(NVMEM) + .post_bind = i2c_eeprom_post_bind, +#endif }; diff --git a/drivers/misc/misc-uclass.c b/drivers/misc/misc-uclass.c index cfe9d562fa..e9b58d3abe 100644 --- a/drivers/misc/misc-uclass.c +++ b/drivers/misc/misc-uclass.c @@ -9,6 +9,7 @@ #include <dm.h> #include <errno.h> #include <misc.h> +#include <nvmem.h>

/* * Implement a miscellaneous uclass for those do not fit other more @@ -67,10 +68,61 @@ int misc_set_enabled(struct udevice *dev, bool val) return ops->set_enabled(dev, val); }

+static int misc_nvmem_read(struct udevice *dev, unsigned int offset, void *buf, + size_t size) +{ + int ret = misc_read(dev_get_parent(dev), offset, buf, size); + + if (ret < 0) + return ret; + if (ret != size) + return -EIO; + return 0; +} + +static int misc_nvmem_write(struct udevice *dev, unsigned int offset, + const void *buf, size_t size) +{ + int ret = misc_write(dev_get_parent(dev), offset, buf, size); + + if (ret < 0) + return ret; + if (ret != size) + return -EIO; + return 0; +} + +static struct __maybe_unused nvmem_ops misc_nvmem_ops = { + .read = misc_nvmem_read, + .write = misc_nvmem_write, +}; + +#if CONFIG_IS_ENABLED(NVMEM) +U_BOOT_DRIVER(misc_nvmem) = { + .name = "misc_nvmem", + .id = UCLASS_NVMEM, + .ops = &misc_nvmem_ops, + .flags = DM_FLAG_NAME_ALLOCED, +}; +#endif + +static int misc_post_bind(struct udevice *dev) +{ +#if CONFIG_IS_ENABLED(OF_REAL) + int ret = dm_scan_fdt_dev(dev); + + if (ret) + return ret; +#endif +#if CONFIG_IS_ENABLED(NVMEM) + return nvmem_register(dev, DM_DRIVER_GET(misc_nvmem)); +#else + return 0; +#endif +} + UCLASS_DRIVER(misc) = { .id = UCLASS_MISC, .name = "misc", -#if CONFIG_IS_ENABLED(OF_REAL) - .post_bind = dm_scan_fdt_dev, -#endif + .post_bind = misc_post_bind, }; diff --git a/drivers/misc/nvmem.c b/drivers/misc/nvmem.c index 5a2bd1f9f7..afd8c7b5ab 100644 --- a/drivers/misc/nvmem.c +++ b/drivers/misc/nvmem.c @@ -10,90 +10,48 @@ #include <nvmem.h> #include <rtc.h> #include <dm/device_compat.h> +#include <dm/device-internal.h> #include <dm/ofnode.h> #include <dm/read.h> #include <dm/uclass.h>

+int nvmem_register(struct udevice *dev, const struct driver* drv) +{ + char *name; + int name_size; + static const char fmt[] = "%s.nvmem"; + + assert(drv->flags & DM_FLAG_NAME_ALLOCED); + + name_size = snprintf(NULL, 0, fmt, dev->name) + 1; + name = malloc(name_size); + if (!name) + return -ENOMEM; + snprintf(name, name_size, fmt, dev->name); + + return device_bind(dev, drv, name, NULL, dev_ofnode(dev), NULL); +} + int nvmem_cell_read(struct nvmem_cell *cell, void *buf, size_t size) -{ +{ + const struct nvmem_ops *ops = dev_get_driver_ops(cell->nvmem); + 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; - } + return ops->read(cell->nvmem, cell->offset, buf, size); }

int nvmem_cell_write(struct nvmem_cell *cell, const void *buf, size_t size) { + const struct nvmem_ops *ops = dev_get_driver_ops(cell->nvmem); + 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 && ret != -EPFNOSUPPORT) - return ret; - } - - return -ENODEV; + return ops->write(cell->nvmem, cell->offset, buf, size); }

int nvmem_cell_get_by_index(struct udevice *dev, int index, @@ -111,7 +69,8 @@ int nvmem_cell_get_by_index(struct udevice *dev, int index, if (ret) return ret;

- ret = nvmem_get_device(ofnode_get_parent(args.node), cell); + ret = uclass_get_device_by_ofnode(UCLASS_NVMEM, ofnode_get_parent(args.node), + &cell->nvmem); if (ret) return ret;

@@ -140,3 +99,8 @@ int nvmem_cell_get_by_name(struct udevice *dev, const char *name,

return nvmem_cell_get_by_index(dev, index, cell); } + +UCLASS_DRIVER(nvmem) = { + .id = UCLASS_NVMEM, + .name = "nvmem", +}; diff --git a/drivers/rtc/rtc-uclass.c b/drivers/rtc/rtc-uclass.c index e5ae6ea4d5..bef2eefecf 100644 --- a/drivers/rtc/rtc-uclass.c +++ b/drivers/rtc/rtc-uclass.c @@ -10,6 +10,7 @@ #include <dm.h> #include <errno.h> #include <log.h> +#include <nvmem.h> #include <rtc.h>

int dm_rtc_get(struct udevice *dev, struct rtc_time *time) @@ -173,11 +174,50 @@ int rtc_write32(struct udevice *dev, unsigned int reg, u32 value) return 0; }

+static int nvmem_rtc_read(struct udevice *dev, unsigned int offset, void *buf, + size_t size) +{ + return dm_rtc_read(dev_get_parent(dev), offset, buf, size); +} + +static int nvmem_rtc_write(struct udevice *dev, unsigned int offset, + const void *buf, size_t size) +{ + return dm_rtc_write(dev_get_parent(dev), offset, buf, size); +} + +static struct __maybe_unused nvmem_ops rtc_nvmem_ops = { + .read = nvmem_rtc_read, + .write = nvmem_rtc_write, +}; + +#if CONFIG_IS_ENABLED(NVMEM) +U_BOOT_DRIVER(rtc_nvmem) = { + .name = "rtc_nvmem", + .id = UCLASS_NVMEM, + .ops = &rtc_nvmem_ops, + .flags = DM_FLAG_NAME_ALLOCED, +}; +#endif + +static int rtc_post_bind(struct udevice *dev) +{ +#if CONFIG_IS_ENABLED(OF_REAL) + int ret = dm_scan_fdt_dev(dev); + + if (ret) + return ret; +#endif +#if CONFIG_IS_ENABLED(NVMEM) + return nvmem_register(dev, DM_DRIVER_GET(rtc_nvmem)); +#else + return 0; +#endif +} + UCLASS_DRIVER(rtc) = { .name = "rtc", .id = UCLASS_RTC, .flags = DM_UC_FLAG_SEQ_ALIAS, -#if CONFIG_IS_ENABLED(OF_REAL) - .post_bind = dm_scan_fdt_dev, -#endif + .post_bind = rtc_post_bind, }; diff --git a/include/dm/uclass-id.h b/include/dm/uclass-id.h index 3ba69ad9a0..50877890b3 100644 --- a/include/dm/uclass-id.h +++ b/include/dm/uclass-id.h @@ -83,6 +83,7 @@ enum uclass_id { UCLASS_NOP, /* No-op devices */ UCLASS_NORTHBRIDGE, /* Intel Northbridge / SDRAM controller */ UCLASS_NVME, /* NVM Express device */ + UCLASS_NVMEM, /* Non-volatile memory devices */ UCLASS_P2SB, /* (x86) Primary-to-Sideband Bus */ UCLASS_PANEL, /* Display panel, such as an LCD */ UCLASS_PANEL_BACKLIGHT, /* Backlight controller for panel */ diff --git a/include/nvmem.h b/include/nvmem.h index 822e698bdd..230841499d 100644 --- a/include/nvmem.h +++ b/include/nvmem.h @@ -6,18 +6,8 @@ #ifndef NVMEM_H #define NVMEM_H

-/** - * DOC: Design - * - * The NVMEM subsystem is a "meta-uclass" in that it abstracts over several - * different uclasses all with read/write APIs. One approach to implementing - * this could be to add a new sub-device for each nvmem-style device of - * UCLASS_NVMEM. This subsystem has taken the approach of using the existing - * access methods (i2c_eeprom_write, misc_write, etc.) directly. This has the - * advantage of not requiring an extra device/driver, saving on binary size and - * runtime memory usage. On the other hand, it is not idiomatic. Similar - * efforts should generally use a new uclass. - */ +struct driver; +struct udevice;

/** * struct nvmem_cell - One datum within non-volatile memory @@ -31,8 +21,6 @@ struct nvmem_cell { size_t size; };

-struct udevice; - #if CONFIG_IS_ENABLED(NVMEM)

/** @@ -104,6 +92,22 @@ int nvmem_cell_get_by_index(struct udevice *dev, int index, int nvmem_cell_get_by_name(struct udevice *dev, const char *name, struct nvmem_cell *cell);

+/** + * nvmem_register() - Register a new nvmem device + * @dev: The nvmem device proper + * @drv: The nvmem adapter driver + * + * This registers a child device of @dev as an nvmem device using @drv. The + * child device's name will be @dev's name with ".nvmem" appended. @drv should + * contain %DM_FLAG_NAME_ALLOCED in its flags. + * + * Return: + * * 0 on success + * * -ENOMEM if no memory could be allocated for the name + * * A negative error if device_bind() failed + */ +int nvmem_register(struct udevice *dev, const struct driver* drv); + #else /* CONFIG_NVMEM */

static inline int nvmem_cell_read(struct nvmem_cell *cell, void *buf, int size) @@ -129,6 +133,53 @@ static inline int nvmem_cell_get_by_name(struct udevice *dev, const char *name, return -ENOSYS; }

+static inline int nvmem_register(struct udevice *dev, const struct driver* drv) +{ + return 0; +} #endif /* CONFIG_NVMEM */

+/** + * struct nvmem_ops - Ops implemented by nvmem devices + * read: Read a register + * write: Write a register + */ +struct nvmem_ops { + int (*read)(struct udevice *dev, unsigned int offset, void *buf, + size_t size); + int (*write)(struct udevice *dev, unsigned int offset, const void *buf, + size_t size); +}; + +#if 0 /* for docs only */ +/** + * read() - 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 + */ +int read(struct udevice *dev, unsigned int offset, void *buf, size_t size); + +/** + * write() - 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 + */ +int write(struct udevice *dev, unsigned int offset, const void *buf, + size_t size); +#endif + #endif /* NVMEM_H */