[PATCH v4 16/16] [RFC] misc: nvmem: Convert to using udevices
Sean Anderson
sean.anderson at seco.com
Thu May 5 19:11:45 CEST 2022
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 at 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 */
--
2.35.1.1320.gc452695387.dirty
More information about the U-Boot
mailing list