Messages in this thread | | | Subject | Re: [PATCH v2 2/2] nvmem: add ONIE NVMEM cells support | From | Srinivas Kandagatla <> | Date | Mon, 1 Jun 2020 09:50:52 +0100 |
| |
On 30/05/2020 00:04, Vadym Kochan wrote: > ONIE is a small operating system, pre-installed on bare metal network > switches, that provides an environment for automated provisioning. > > This system requires that NVMEM (EEPROM) device holds various system > information (mac address, platform name, etc) in a special TLV layout. > > The driver registers ONIE TLV attributes as NVMEM cells which can be > accessed by other platform driver. Also it allows to use > of_get_mac_address() to retrieve mac address for the netdev. > > Signed-off-by: Vadym Kochan <vadym.kochan@plvision.eu> > --- > drivers/nvmem/Kconfig | 9 + > drivers/nvmem/Makefile | 3 + > drivers/nvmem/onie-cells.c | 332 +++++++++++++++++++++++++++++++++++++
Is there a reason why Device tree bindings are missing for this driver?
> 3 files changed, 344 insertions(+) > create mode 100644 drivers/nvmem/onie-cells.c > > diff --git a/drivers/nvmem/Kconfig b/drivers/nvmem/Kconfig > index d7b7f6d688e7..dd9298487992 100644 > --- a/drivers/nvmem/Kconfig > +++ b/drivers/nvmem/Kconfig > @@ -273,4 +273,13 @@ config SPRD_EFUSE > This driver can also be built as a module. If so, the module > will be called nvmem-sprd-efuse. > > +config NVMEM_ONIE_CELLS > + tristate "ONIE TLV cells support" > + help > + This is a driver to provide cells from ONIE TLV structure stored > + on NVME device. > + > + This driver can also be built as a module. If so, the module > + will be called nvmem-onie-cells. > + > endif > diff --git a/drivers/nvmem/Makefile b/drivers/nvmem/Makefile > index a7c377218341..2199784a489f 100644 > --- a/drivers/nvmem/Makefile > +++ b/drivers/nvmem/Makefile > @@ -55,3 +55,6 @@ obj-$(CONFIG_NVMEM_ZYNQMP) += nvmem_zynqmp_nvmem.o > nvmem_zynqmp_nvmem-y := zynqmp_nvmem.o > obj-$(CONFIG_SPRD_EFUSE) += nvmem_sprd_efuse.o > nvmem_sprd_efuse-y := sprd-efuse.o > + > +obj-$(CONFIG_NVMEM_ONIE_CELLS) += nvmem-onie-cells.o > +nvmem-onie-cells-y := onie-cells.o > diff --git a/drivers/nvmem/onie-cells.c b/drivers/nvmem/onie-cells.c > new file mode 100644 > index 000000000000..1e8b4b8d1c0d > --- /dev/null > +++ b/drivers/nvmem/onie-cells.c > @@ -0,0 +1,332 @@ > +// SPDX-License-Identifier: GPL-2.0+ > +/* > + * ONIE NVMEM cells provider > + * > + * Author: Vadym Kochan <vadym.kochan@plvision.eu> > + */ > + > +#define ONIE_NVMEM_DRVNAME "onie-nvmem-cells" > + > +#define pr_fmt(fmt) ONIE_NVMEM_DRVNAME ": " fmt > + > +#include <linux/kernel.h> > +#include <linux/module.h> > +#include <linux/slab.h> > +#include <linux/init.h> > +#include <linux/of.h> > +#include <linux/of_device.h> > +#include <linux/platform_device.h> > +#include <linux/nvmem-consumer.h> > +#include <linux/nvmem-provider.h> > + > +#define ONIE_NVMEM_TLV_MAX_LEN 2048 > + > +#define ONIE_NVMEM_HDR_ID "TlvInfo" > +
...
> + > +static int onie_nvmem_probe(struct platform_device *pdev) > +{ > + struct device *dev = &pdev->dev; > + struct device_node *np = dev->of_node; > + struct nvmem_cell_info *cells; > + struct onie_nvmem_attr *attr; > + struct nvmem_device *nvmem; > + struct onie_nvmem *onie; > + unsigned int ncells = 0; > + int err; > + > + nvmem = of_nvmem_device_get(np, NULL); > + if (IS_ERR(nvmem)) > + return PTR_ERR(nvmem); > + TBH, this looks completely incorrect way to do this and misuse of nvmem consumer interface.
Ideally nvmem provider driver should populate "cells" in struct nvmem_config after decoding them and then register nvmem provider.
That should just work!
--srini
> + onie = kmalloc(sizeof(*onie), GFP_KERNEL); > + if (!onie) { > + err = -ENOMEM; > + goto err_nvmem_alloc; > + } > + > + INIT_LIST_HEAD(&onie->attrs); > + onie->attr_count = 0; > + onie->nvmem = nvmem; > + > + err = onie_nvmem_decode(onie); > + if (err) > + goto err_nvmem_decode; > + > + if (!onie->attr_count) { > + pr_err("%s: has no ONIE attributes\n", nvmem_dev_name(nvmem)); > + err = -EINVAL; > + goto err_no_attrs; > + } > + > + cells = kmalloc_array(onie->attr_count, sizeof(*cells), GFP_KERNEL); > + if (!cells) { > + err = -ENOMEM; > + goto err_cells_alloc; > + } > + > + onie->cell_lookup = kmalloc_array(onie->attr_count, > + sizeof(struct nvmem_cell_lookup), > + GFP_KERNEL); > + if (!onie->cell_lookup) { > + err = -ENOMEM; > + goto err_lookup_alloc; > + } > + > + list_for_each_entry(attr, &onie->attrs, head) { > + struct nvmem_cell_lookup *lookup; > + struct nvmem_cell_info *cell; > + > + cell = &cells[ncells]; > + > + lookup = &onie->cell_lookup[ncells]; > + lookup->con_id = NULL; > + > + cell->offset = attr->offset; > + cell->name = attr->name; > + cell->bytes = attr->len; > + cell->bit_offset = 0; > + cell->nbits = 0; > + > + lookup->nvmem_name = nvmem_dev_name(onie->nvmem); > + lookup->dev_id = dev_name(dev); > + lookup->cell_name = cell->name; > + lookup->con_id = cell->name; > + > + ncells++; > + } > + > + onie->cell_tbl.nvmem_name = nvmem_dev_name(onie->nvmem); > + onie->cell_tbl.ncells = ncells; > + onie->cell_tbl.cells = cells; > + > + nvmem_add_cell_table(&onie->cell_tbl); > + nvmem_add_cell_lookups(onie->cell_lookup, ncells); > + > + dev_set_drvdata(dev, onie); > + > + onie_nvmem_attrs_free(onie); > + > + nvmem_device_put(nvmem); > + > + return 0; > + > +err_lookup_alloc: > + kfree(onie->cell_tbl.cells); > +err_cells_alloc: > + onie_nvmem_attrs_free(onie); > +err_no_attrs: > +err_nvmem_decode: > + kfree(onie); > +err_nvmem_alloc: > + nvmem_device_put(nvmem); > + > + return err; > +} > +
| |