lkml.org 
[lkml]   [2022]   [Aug]   [30]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH v1 06/14] nvmem: core: introduce NVMEM layouts
From


On 25/08/2022 22:44, Michael Walle wrote:
> NVMEM layouts are used to generate NVMEM cells during runtime. Think of
> an EEPROM with a well-defined conent. For now, the content can be
> described by a device tree or a board file. But this only works if the
> offsets and lengths are static and don't change. One could also argue
> that putting the layout of the EEPROM in the device tree is the wrong
> place. Instead, the device tree should just have a specific compatible
> string.
>
> Right now there are two use cases:
> (1) The NVMEM cell needs special processing. E.g. if it only specifies
> a base MAC address offset and you need to add an offset, or it
> needs to parse a MAC from ASCII format or some proprietary format.
> (Post processing of cells is added in a later commit).
> (2) u-boot environment parsing. The cells don't have a particular
> offset but it needs parsing the content to determine the offsets
> and length.
>
> Signed-off-by: Michael Walle <michael@walle.cc>
> ---
> drivers/nvmem/Kconfig | 2 ++
> drivers/nvmem/Makefile | 1 +
> drivers/nvmem/core.c | 57 ++++++++++++++++++++++++++++++++++
> drivers/nvmem/layouts/Kconfig | 5 +++
> drivers/nvmem/layouts/Makefile | 4 +++
> include/linux/nvmem-provider.h | 38 +++++++++++++++++++++++
> 6 files changed, 107 insertions(+)
> create mode 100644 drivers/nvmem/layouts/Kconfig
> create mode 100644 drivers/nvmem/layouts/Makefile

update to ./Documentation/driver-api/nvmem.rst would help others.

>
> diff --git a/drivers/nvmem/Kconfig b/drivers/nvmem/Kconfig
> index bab8a29c9861..1416837b107b 100644
> --- a/drivers/nvmem/Kconfig
> +++ b/drivers/nvmem/Kconfig
> @@ -357,4 +357,6 @@ config NVMEM_U_BOOT_ENV
>
> If compiled as module it will be called nvmem_u-boot-env.
>
> +source "drivers/nvmem/layouts/Kconfig"
> +
> endif
> diff --git a/drivers/nvmem/Makefile b/drivers/nvmem/Makefile
> index 399f9972d45b..cd5a5baa2f3a 100644
> --- a/drivers/nvmem/Makefile
> +++ b/drivers/nvmem/Makefile
> @@ -5,6 +5,7 @@
>
> obj-$(CONFIG_NVMEM) += nvmem_core.o
> nvmem_core-y := core.o
> +obj-y += layouts/
>
> # Devices
> obj-$(CONFIG_NVMEM_BCM_OCOTP) += nvmem-bcm-ocotp.o
> diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
> index 3dfd149374a8..5357fc378700 100644
> --- a/drivers/nvmem/core.c
> +++ b/drivers/nvmem/core.c
> @@ -74,6 +74,9 @@ static LIST_HEAD(nvmem_lookup_list);
>
> static BLOCKING_NOTIFIER_HEAD(nvmem_notifier);
>
> +static DEFINE_SPINLOCK(nvmem_layout_lock);
> +static LIST_HEAD(nvmem_layouts);
> +
> static int __nvmem_reg_read(struct nvmem_device *nvmem, unsigned int offset,
> void *val, size_t bytes)
> {
> @@ -744,6 +747,56 @@ static int nvmem_add_cells_from_of(struct nvmem_device *nvmem)
> return 0;
> }
>
> +int nvmem_register_layout(struct nvmem_layout *layout)
> +{
> + spin_lock(&nvmem_layout_lock);
> + list_add(&layout->node, &nvmem_layouts);
> + spin_unlock(&nvmem_layout_lock);
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(nvmem_register_layout);

we should provide nvmem_unregister_layout too, so that providers can add
them if they can in there respective drivers.

> +
> +static struct nvmem_layout *nvmem_get_compatible_layout(struct device_node *np)
> +{
> + struct nvmem_layout *p, *ret = NULL;
> +
> + spin_lock(&nvmem_layout_lock);
> +
> + list_for_each_entry(p, &nvmem_layouts, node) {
> + if (of_match_node(p->of_match_table, np)) {
> + ret = p;
> + break;
> + }
> + }
> +
> + spin_unlock(&nvmem_layout_lock);
> +
> + return ret;
> +}
> +
> +static int nvmem_add_cells_from_layout(struct nvmem_device *nvmem)
> +{
> + struct nvmem_layout *layout;
> +
> + layout = nvmem_get_compatible_layout(nvmem->dev.of_node);
> + if (layout)
> + layout->add_cells(&nvmem->dev, nvmem, layout);

access to add_cells can crash hear as we did not check it before adding
in to list.
Or
we could relax add_cells callback for usecases like imx-octop.


> +
> + return 0;
> +}
> +
> +const void *nvmem_layout_get_match_data(struct nvmem_device *nvmem,
> + struct nvmem_layout *layout)
> +{
> + const struct of_device_id *match;
> +
> + match = of_match_node(layout->of_match_table, nvmem->dev.of_node);
> +
> + return match ? match->data : NULL;
> +}
> +EXPORT_SYMBOL_GPL(nvmem_layout_get_match_data);
> +
> /**
> * nvmem_register() - Register a nvmem device for given nvmem_config.
> * Also creates a binary entry in /sys/bus/nvmem/devices/dev-name/nvmem
> @@ -872,6 +925,10 @@ struct nvmem_device *nvmem_register(const struct nvmem_config *config)
> if (rval)
> goto err_remove_cells;
>
> + rval = nvmem_add_cells_from_layout(nvmem);
> + if (rval)
> + goto err_remove_cells;
> +
> blocking_notifier_call_chain(&nvmem_notifier, NVMEM_ADD, nvmem);
>
> return nvmem;
> diff --git a/drivers/nvmem/layouts/Kconfig b/drivers/nvmem/layouts/Kconfig
> new file mode 100644
> index 000000000000..9ad3911d1605
> --- /dev/null
> +++ b/drivers/nvmem/layouts/Kconfig
> @@ -0,0 +1,5 @@
> +# SPDX-License-Identifier: GPL-2.0
> +
> +menu "Layout Types"
> +
> +endmenu
> diff --git a/drivers/nvmem/layouts/Makefile b/drivers/nvmem/layouts/Makefile
> new file mode 100644
> index 000000000000..6fdb3c60a4fa
> --- /dev/null
> +++ b/drivers/nvmem/layouts/Makefile
> @@ -0,0 +1,4 @@
> +# SPDX-License-Identifier: GPL-2.0
> +#
> +# Makefile for nvmem layouts.
> +#
> diff --git a/include/linux/nvmem-provider.h b/include/linux/nvmem-provider.h
> index e710404959e7..323685841e9f 100644
> --- a/include/linux/nvmem-provider.h
> +++ b/include/linux/nvmem-provider.h
> @@ -127,6 +127,28 @@ struct nvmem_cell_table {
> struct list_head node;
> };
>
> +/**
> + * struct nvmem_layout - NVMEM layout definitions
> + *
> + * @name: Layout name.
> + * @of_match_table: Open firmware match table.
> + * @add_cells: Will be called if a nvmem device is found which
> + * has this layout. The function will add layout
> + * specific cells with nvmem_add_one_cell().
> + * @node: List node.
> + *
> + * A nvmem device can hold a well defined structure which can just be
> + * evaluated during runtime. For example a TLV list, or a list of "name=val"
> + * pairs. A nvmem layout can parse the nvmem device and add appropriate
> + * cells.
> + */
> +struct nvmem_layout {
> + const char *name;
> + const struct of_device_id *of_match_table;

looking at this, I think its doable to convert the existing
cell_post_process callback to layouts by adding a layout specific
callback here.


--srini
> + int (*add_cells)(struct nvmem_device *nvmem, struct nvmem_layout *layout);
> + struct list_head node;
> +};
> +
> #if IS_ENABLED(CONFIG_NVMEM)
>
> struct nvmem_device *nvmem_register(const struct nvmem_config *cfg);
> @@ -141,6 +163,10 @@ void nvmem_del_cell_table(struct nvmem_cell_table *table);
> int nvmem_add_one_cell(struct nvmem_device *nvmem,
> const struct nvmem_cell_info *info);
>
> +int nvmem_register_layout(struct nvmem_layout *layout);
> +const void *nvmem_layout_get_match_data(struct nvmem_device *nvmem,
> + struct nvmem_layout *layout);
> +
> #else
>
> static inline struct nvmem_device *nvmem_register(const struct nvmem_config *c)
> @@ -161,5 +187,17 @@ static inline void nvmem_del_cell_table(struct nvmem_cell_table *table) {}
> static inline int nvmem_add_one_cell(struct nvmem_device *nvmem,
> const struct nvmem_cell_info *info) {}
>
> +static inline int nvmem_register_layout(struct nvmem_layout *layout)
> +{
> + return -EOPNOTSUPP;
> +}
> +
> +static inline const void *
> +nvmem_layout_get_match_data(struct nvmem_device *nvmem,
> + struct nvmem_layout *layout)
> +{
> + return NULL;
> +}
> +
> #endif /* CONFIG_NVMEM */
> #endif /* ifndef _LINUX_NVMEM_PROVIDER_H */

\
 
 \ /
  Last update: 2022-08-30 15:37    [W:0.270 / U:0.048 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site