lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Tue, 30 Aug 2022 16:24:42 +0200 From: Michael Walle <michael@...le.cc> To: Srinivas Kandagatla <srinivas.kandagatla@...aro.org> Cc: Miquel Raynal <miquel.raynal@...tlin.com>, Richard Weinberger <richard@....at>, Vignesh Raghavendra <vigneshr@...com>, Rob Herring <robh+dt@...nel.org>, Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>, Shawn Guo <shawnguo@...nel.org>, Li Yang <leoyang.li@....com>, Rafał Miłecki <rafal@...ecki.pl>, "David S . Miller" <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>, Frank Rowand <frowand.list@...il.com>, linux-mtd@...ts.infradead.org, devicetree@...r.kernel.org, linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org, netdev@...r.kernel.org, Ahmad Fatoum <a.fatoum@...gutronix.de> Subject: Re: [PATCH v1 06/14] nvmem: core: introduce NVMEM layouts Am 2022-08-30 15:36, schrieb Srinivas Kandagatla: > 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@...le.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. Sure. Didn't know about that one. >> 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. Actually, that was the idea; that you can have layouts outside of layouts/. I also had a nvmem_unregister_layout() but removed it because it was dead code. Will re-add it again. >> + >> +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. good catch, will use layout && layout->add_cells. >> + >> + 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. can you elaborate on that? -michael
Powered by blists - more mailing lists