* [PATCH V4 0/2] nvmem: add and use generic MMIO NVMEM
@ 2023-02-28 7:29 Rafał Miłecki
2023-02-28 7:29 ` [PATCH V4 1/2] dt-bindings: nvmem: mmio: new binding for MMIO accessible NVMEM devices Rafał Miłecki
2023-02-28 7:29 ` [PATCH V4 2/2] nvmem: add generic driver for devices with MMIO access Rafał Miłecki
0 siblings, 2 replies; 6+ messages in thread
From: Rafał Miłecki @ 2023-02-28 7:29 UTC (permalink / raw)
To: Srinivas Kandagatla, Rob Herring, Krzysztof Kozlowski
Cc: Matthias Brugger, Kunihiko Hayashi, Masami Hiramatsu,
linux-mediatek, linux-arm-kernel, devicetree, linux-kernel,
Rafał Miłecki
From: Rafał Miłecki <rafal@milecki.pl>
MMIO accessible NVMEM devices should be simple enough to allow using a
single binding & driver for them.
In V3 I didn't decide to modify existing Mediatek & UniPhier drivers as
there are some concerns about adding writing support to them. If needed
that can be done later.
Once we get layouts support NVRAM driver will get migrated into one and
generic MMIO driver will take over its binding.
Rafał Miłecki (2):
dt-bindings: nvmem: mmio: new binding for MMIO accessible NVMEM
devices
nvmem: add generic driver for devices with MMIO access
.../devicetree/bindings/nvmem/mmio.yaml | 50 ++++++
drivers/nvmem/Kconfig | 10 ++
drivers/nvmem/Makefile | 2 +
drivers/nvmem/mmio.c | 148 ++++++++++++++++++
4 files changed, 210 insertions(+)
create mode 100644 Documentation/devicetree/bindings/nvmem/mmio.yaml
create mode 100644 drivers/nvmem/mmio.c
--
2.34.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH V4 1/2] dt-bindings: nvmem: mmio: new binding for MMIO accessible NVMEM devices 2023-02-28 7:29 [PATCH V4 0/2] nvmem: add and use generic MMIO NVMEM Rafał Miłecki @ 2023-02-28 7:29 ` Rafał Miłecki 2023-02-28 7:29 ` [PATCH V4 2/2] nvmem: add generic driver for devices with MMIO access Rafał Miłecki 1 sibling, 0 replies; 6+ messages in thread From: Rafał Miłecki @ 2023-02-28 7:29 UTC (permalink / raw) To: Srinivas Kandagatla, Rob Herring, Krzysztof Kozlowski Cc: Matthias Brugger, Kunihiko Hayashi, Masami Hiramatsu, linux-mediatek, linux-arm-kernel, devicetree, linux-kernel, Rafał Miłecki, Rob Herring From: Rafał Miłecki <rafal@milecki.pl> Content of some NVMEM devices can be read using MMIO. Some of them (probably very few though) may be also programmable that way. Add generic binding to allow describing such hardware. This *doesn't* apply to any more complicated devices that need more complex interface e.g. for writing. While such devices could be supported for reading purposes by the same driver - they should get their own binding. This binding will gain even more usability once we fully support NVMEM layouts (describing content of NVMEM devices in an independent way). Signed-off-by: Rafał Miłecki <rafal@milecki.pl> Reviewed-by: Rob Herring <robh@kernel.org> --- V3: Make it clear this binding should NOT be used for more complex devices --- .../devicetree/bindings/nvmem/mmio.yaml | 50 +++++++++++++++++++ 1 file changed, 50 insertions(+) create mode 100644 Documentation/devicetree/bindings/nvmem/mmio.yaml diff --git a/Documentation/devicetree/bindings/nvmem/mmio.yaml b/Documentation/devicetree/bindings/nvmem/mmio.yaml new file mode 100644 index 000000000000..9ca96b7a4856 --- /dev/null +++ b/Documentation/devicetree/bindings/nvmem/mmio.yaml @@ -0,0 +1,50 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/nvmem/mmio.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: MMIO access based NVMEM + +description: | + This binding describes simple NVMEM devices with content that can be accessed + using MMIO (memory-mapped I/O access). + + More complex devices that support any other access than a simple memory + mapping should use a custom binding. In such cases this binding's compatible + should NOT be used even as a fallback. + + This binding is designed to describe just an NVMEM content access method. The + way of handling actual content should be described independently (on top of + this binding). + +maintainers: + - Rafał Miłecki <rafal@milecki.pl> + +allOf: + - $ref: nvmem.yaml# + +properties: + compatible: + const: mmio-nvmem + + reg: + maxItems: 1 + + reg-io-width: + description: | + The size (in bytes) of the IO accesses that should be performed + on the device. + enum: [1, 2, 4, 8] + +required: + - reg + +unevaluatedProperties: false + +examples: + - | + nvmem@10000 { + compatible = "mmio-nvmem"; + reg = <0x10000000 0x10000>; + }; -- 2.34.1 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH V4 2/2] nvmem: add generic driver for devices with MMIO access 2023-02-28 7:29 [PATCH V4 0/2] nvmem: add and use generic MMIO NVMEM Rafał Miłecki 2023-02-28 7:29 ` [PATCH V4 1/2] dt-bindings: nvmem: mmio: new binding for MMIO accessible NVMEM devices Rafał Miłecki @ 2023-02-28 7:29 ` Rafał Miłecki 2023-03-08 13:31 ` Srinivas Kandagatla 1 sibling, 1 reply; 6+ messages in thread From: Rafał Miłecki @ 2023-02-28 7:29 UTC (permalink / raw) To: Srinivas Kandagatla, Rob Herring, Krzysztof Kozlowski Cc: Matthias Brugger, Kunihiko Hayashi, Masami Hiramatsu, linux-mediatek, linux-arm-kernel, devicetree, linux-kernel, Rafał Miłecki From: Rafał Miłecki <rafal@milecki.pl> Some NVMEM devices can be accessed by simply mapping memory and reading from / writing to it. This driver adds support for a generic "mmio-nvmem" DT binding used by such devices. One of such devices is Broadcom's NVRAM. It's already supported (see NVMEM_BRCM_NVRAM) but existing driver covers both: 1. NVMEM device access 2. NVMEM content parsing Once we get support for NVMEM layouts then existing NVRAM driver will get converted into a layout and generic driver will take over responsibility for data access. Signed-off-by: Rafał Miłecki <rafal@milecki.pl> --- V3: Support "reg-io-width", basic writing & "brcm,nvram" string V3: Don't duplicate core checks, add 64 b support, complete writing support, don't add confusing conditional "brcm,nvram" support (it will be handled with layouts migration) --- drivers/nvmem/Kconfig | 10 +++ drivers/nvmem/Makefile | 2 + drivers/nvmem/mmio.c | 148 +++++++++++++++++++++++++++++++++++++++++ 3 files changed, 160 insertions(+) create mode 100644 drivers/nvmem/mmio.c diff --git a/drivers/nvmem/Kconfig b/drivers/nvmem/Kconfig index 6dec38805041..189ea85bd67d 100644 --- a/drivers/nvmem/Kconfig +++ b/drivers/nvmem/Kconfig @@ -166,6 +166,16 @@ config NVMEM_MICROCHIP_OTPC This driver enable the OTP controller available on Microchip SAMA7G5 SoCs. It controls the access to the OTP memory connected to it. +config NVMEM_MMIO + tristate "MMIO access based NVMEM support" + depends on HAS_IOMEM + help + This driver provides support for NVMEM devices that can be accessed + using MMIO. + + This driver can also be built as a module. If so, the module + will be called nvmem-mmio. + config NVMEM_MTK_EFUSE tristate "Mediatek SoCs EFUSE support" depends on ARCH_MEDIATEK || COMPILE_TEST diff --git a/drivers/nvmem/Makefile b/drivers/nvmem/Makefile index 6a1efffa88f0..767a9db2bfc1 100644 --- a/drivers/nvmem/Makefile +++ b/drivers/nvmem/Makefile @@ -35,6 +35,8 @@ obj-$(CONFIG_NVMEM_MESON_MX_EFUSE) += nvmem_meson_mx_efuse.o nvmem_meson_mx_efuse-y := meson-mx-efuse.o obj-$(CONFIG_NVMEM_MICROCHIP_OTPC) += nvmem-microchip-otpc.o nvmem-microchip-otpc-y := microchip-otpc.o +obj-$(CONFIG_NVMEM_MMIO) += nvmem-mmio.o +nvmem-mmio-y := mmio.o obj-$(CONFIG_NVMEM_MTK_EFUSE) += nvmem_mtk-efuse.o nvmem_mtk-efuse-y := mtk-efuse.o obj-$(CONFIG_NVMEM_MXS_OCOTP) += nvmem-mxs-ocotp.o diff --git a/drivers/nvmem/mmio.c b/drivers/nvmem/mmio.c new file mode 100644 index 000000000000..ce51648bb321 --- /dev/null +++ b/drivers/nvmem/mmio.c @@ -0,0 +1,148 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* + * Copyright (C) 2023 Rafał Miłecki <rafal@milecki.pl> + */ + +#include <linux/io.h> +#include <linux/mod_devicetable.h> +#include <linux/module.h> +#include <linux/nvmem-provider.h> +#include <linux/of.h> +#include <linux/platform_device.h> +#include <linux/property.h> +#include <linux/slab.h> + +struct mmio_nvmem { + void __iomem *base; + u32 io_width; +}; + +static int mmio_nvmem_read(void *context, unsigned int offset, void *val, size_t bytes) +{ + struct mmio_nvmem *priv = context; + u64 __maybe_unused *dst64; + u32 *dst32; + u16 *dst16; + u8 *dst8; + + switch (priv->io_width) { + case 0: + memcpy_fromio(val, priv->base + offset, bytes); + break; + case 1: + for (dst8 = val; bytes; bytes -= 1, offset += 1) + *dst8++ = readb(priv->base + offset); + break; + case 2: + for (dst16 = val; bytes; bytes -= 2, offset += 2) + *dst16++ = readw(priv->base + offset); + break; + case 4: + for (dst32 = val; bytes; bytes -= 4, offset += 4) + *dst32++ = readl(priv->base + offset); + break; +#ifdef CONFIG_64BIT + case 8: + for (dst64 = val; bytes; bytes -= 8, offset += 8) + *dst64++ = readq(priv->base + offset); + break; +#endif + default: + return -EINVAL; + } + + return 0; +} + +static int mmio_nvmem_write(void *context, unsigned int offset, void *val, size_t bytes) +{ + struct mmio_nvmem *priv = context; + u64 __maybe_unused *dst64; + u32 *dst32; + u16 *dst16; + u8 *dst8; + + switch (priv->io_width) { + case 0: + memcpy_toio(priv->base + offset, val, bytes); + break; + case 1: + for (dst8 = val; bytes; bytes -= 1, offset += 1) + writeb(*dst8++, priv->base + offset); + break; + case 2: + for (dst16 = val; bytes; bytes -= 2, offset += 2) + writew(*dst16++, priv->base + offset); + break; + case 4: + for (dst32 = val; bytes; bytes -= 4, offset += 4) + writel(*dst32++, priv->base + offset); + break; +#ifdef CONFIG_64BIT + case 8: + for (dst64 = val; bytes; bytes -= 8, offset += 8) + writeq(*dst64++, priv->base + offset); + break; +#endif + default: + return -EINVAL; + } + + return 0; +} + +static int mmio_nvmem_probe(struct platform_device *pdev) +{ + struct nvmem_config config = { + .name = "mmio-nvmem", + .id = NVMEM_DEVID_AUTO, + .read_only = true, + .reg_read = mmio_nvmem_read, + .reg_write = mmio_nvmem_write, + }; + struct device *dev = &pdev->dev; + struct mmio_nvmem *priv; + struct resource *res; + + priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL); + if (!priv) + return -ENOMEM; + + priv->base = devm_platform_get_and_ioremap_resource(pdev, 0, &res); + if (IS_ERR(priv->base)) + return PTR_ERR(priv->base); + + of_property_read_u32(dev->of_node, "reg-io-width", &priv->io_width); + + config.dev = dev; + config.size = resource_size(res); + config.word_size = priv->io_width; + config.stride = priv->io_width; + config.priv = priv; + + return PTR_ERR_OR_ZERO(devm_nvmem_register(dev, &config)); +} + +static const struct of_device_id mmio_nvmem_of_match_table[] = { + { .compatible = "mmio-nvmem", }, + {}, +}; + +static struct platform_driver mmio_nvmem_driver = { + .probe = mmio_nvmem_probe, + .driver = { + .name = "mmio_nvmem", + .of_match_table = mmio_nvmem_of_match_table, + }, +}; + +static int __init mmio_nvmem_init(void) +{ + return platform_driver_register(&mmio_nvmem_driver); +} + +subsys_initcall_sync(mmio_nvmem_init); + +MODULE_AUTHOR("Rafał Miłecki"); +MODULE_LICENSE("GPL"); +MODULE_DEVICE_TABLE(of, mmio_nvmem_of_match_table); -- 2.34.1 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH V4 2/2] nvmem: add generic driver for devices with MMIO access 2023-02-28 7:29 ` [PATCH V4 2/2] nvmem: add generic driver for devices with MMIO access Rafał Miłecki @ 2023-03-08 13:31 ` Srinivas Kandagatla 2023-03-08 15:42 ` Rafał Miłecki 0 siblings, 1 reply; 6+ messages in thread From: Srinivas Kandagatla @ 2023-03-08 13:31 UTC (permalink / raw) To: Rafał Miłecki, Rob Herring, Krzysztof Kozlowski Cc: Matthias Brugger, Kunihiko Hayashi, Masami Hiramatsu, linux-mediatek, linux-arm-kernel, devicetree, linux-kernel, Rafał Miłecki Hi Rafał, Thanks for doing this, On 28/02/2023 07:29, Rafał Miłecki wrote: > From: Rafał Miłecki <rafal@milecki.pl> > > Some NVMEM devices can be accessed by simply mapping memory and reading > from / writing to it. This driver adds support for a generic > "mmio-nvmem" DT binding used by such devices. > > One of such devices is Broadcom's NVRAM. It's already supported (see > NVMEM_BRCM_NVRAM) but existing driver covers both: What will happen to the old "brcm,nvram" compatible and the dt firmware that already have this node? If there is only one user for this then one would object that why do we need this DT level of abstraction to start with? If this is not the case please consider adding those patches to this series. > 1. NVMEM device access > 2. NVMEM content parsing > > Once we get support for NVMEM layouts then existing NVRAM driver will > get converted into a layout and generic driver will take over > responsibility for data access. > Even though this series is simple, but it is really confusing for two reasons. 1> Generic mmio nvmem bindings are incomplete and potentially change/evolve on every new user. Ex clks, regulators, endianess ... So it looks really fragile and incomplete to me as a generic bindings. Is this want you are expecting? 2> As you mentioned that this will replace broadcom NVMRAM, but this patch does nothing in relation to updating that driver, so the code is dead as it is. If you are considering to use it for Broadcom NVMRAM, please add those patches to this series so that we could see the real user for this code. --srini > Signed-off-by: Rafał Miłecki <rafal@milecki.pl> > --- > V3: Support "reg-io-width", basic writing & "brcm,nvram" string > V3: Don't duplicate core checks, add 64 b support, complete writing > support, don't add confusing conditional "brcm,nvram" support (it > will be handled with layouts migration) > --- > drivers/nvmem/Kconfig | 10 +++ > drivers/nvmem/Makefile | 2 + > drivers/nvmem/mmio.c | 148 +++++++++++++++++++++++++++++++++++++++++ > 3 files changed, 160 insertions(+) > create mode 100644 drivers/nvmem/mmio.c > > diff --git a/drivers/nvmem/Kconfig b/drivers/nvmem/Kconfig > index 6dec38805041..189ea85bd67d 100644 > --- a/drivers/nvmem/Kconfig > +++ b/drivers/nvmem/Kconfig > @@ -166,6 +166,16 @@ config NVMEM_MICROCHIP_OTPC > This driver enable the OTP controller available on Microchip SAMA7G5 > SoCs. It controls the access to the OTP memory connected to it. > > +config NVMEM_MMIO > + tristate "MMIO access based NVMEM support" > + depends on HAS_IOMEM > + help > + This driver provides support for NVMEM devices that can be accessed > + using MMIO. > + > + This driver can also be built as a module. If so, the module > + will be called nvmem-mmio. > + > config NVMEM_MTK_EFUSE > tristate "Mediatek SoCs EFUSE support" > depends on ARCH_MEDIATEK || COMPILE_TEST > diff --git a/drivers/nvmem/Makefile b/drivers/nvmem/Makefile > index 6a1efffa88f0..767a9db2bfc1 100644 > --- a/drivers/nvmem/Makefile > +++ b/drivers/nvmem/Makefile > @@ -35,6 +35,8 @@ obj-$(CONFIG_NVMEM_MESON_MX_EFUSE) += nvmem_meson_mx_efuse.o > nvmem_meson_mx_efuse-y := meson-mx-efuse.o > obj-$(CONFIG_NVMEM_MICROCHIP_OTPC) += nvmem-microchip-otpc.o > nvmem-microchip-otpc-y := microchip-otpc.o > +obj-$(CONFIG_NVMEM_MMIO) += nvmem-mmio.o > +nvmem-mmio-y := mmio.o > obj-$(CONFIG_NVMEM_MTK_EFUSE) += nvmem_mtk-efuse.o > nvmem_mtk-efuse-y := mtk-efuse.o > obj-$(CONFIG_NVMEM_MXS_OCOTP) += nvmem-mxs-ocotp.o > diff --git a/drivers/nvmem/mmio.c b/drivers/nvmem/mmio.c > new file mode 100644 > index 000000000000..ce51648bb321 > --- /dev/null > +++ b/drivers/nvmem/mmio.c > @@ -0,0 +1,148 @@ > +// SPDX-License-Identifier: GPL-2.0-only > +/* > + * Copyright (C) 2023 Rafał Miłecki <rafal@milecki.pl> > + */ > + > +#include <linux/io.h> > +#include <linux/mod_devicetable.h> > +#include <linux/module.h> > +#include <linux/nvmem-provider.h> > +#include <linux/of.h> > +#include <linux/platform_device.h> > +#include <linux/property.h> > +#include <linux/slab.h> > + > +struct mmio_nvmem { > + void __iomem *base; > + u32 io_width; > +}; > + > +static int mmio_nvmem_read(void *context, unsigned int offset, void *val, size_t bytes) > +{ > + struct mmio_nvmem *priv = context; > + u64 __maybe_unused *dst64; > + u32 *dst32; > + u16 *dst16; > + u8 *dst8; > + > + switch (priv->io_width) { > + case 0: > + memcpy_fromio(val, priv->base + offset, bytes); > + break; > + case 1: > + for (dst8 = val; bytes; bytes -= 1, offset += 1) > + *dst8++ = readb(priv->base + offset); > + break; > + case 2: > + for (dst16 = val; bytes; bytes -= 2, offset += 2) > + *dst16++ = readw(priv->base + offset); > + break; > + case 4: > + for (dst32 = val; bytes; bytes -= 4, offset += 4) > + *dst32++ = readl(priv->base + offset); > + break; > +#ifdef CONFIG_64BIT > + case 8: > + for (dst64 = val; bytes; bytes -= 8, offset += 8) > + *dst64++ = readq(priv->base + offset); > + break; > +#endif > + default: > + return -EINVAL; > + } > + > + return 0; > +} > + > +static int mmio_nvmem_write(void *context, unsigned int offset, void *val, size_t bytes) > +{ > + struct mmio_nvmem *priv = context; > + u64 __maybe_unused *dst64; > + u32 *dst32; > + u16 *dst16; > + u8 *dst8; > + > + switch (priv->io_width) { > + case 0: > + memcpy_toio(priv->base + offset, val, bytes); > + break; > + case 1: > + for (dst8 = val; bytes; bytes -= 1, offset += 1) > + writeb(*dst8++, priv->base + offset); > + break; > + case 2: > + for (dst16 = val; bytes; bytes -= 2, offset += 2) > + writew(*dst16++, priv->base + offset); > + break; > + case 4: > + for (dst32 = val; bytes; bytes -= 4, offset += 4) > + writel(*dst32++, priv->base + offset); > + break; > +#ifdef CONFIG_64BIT > + case 8: > + for (dst64 = val; bytes; bytes -= 8, offset += 8) > + writeq(*dst64++, priv->base + offset); > + break; > +#endif > + default: > + return -EINVAL; > + } > + > + return 0; > +} > + > +static int mmio_nvmem_probe(struct platform_device *pdev) > +{ > + struct nvmem_config config = { > + .name = "mmio-nvmem", > + .id = NVMEM_DEVID_AUTO, > + .read_only = true, > + .reg_read = mmio_nvmem_read, > + .reg_write = mmio_nvmem_write, > + }; > + struct device *dev = &pdev->dev; > + struct mmio_nvmem *priv; > + struct resource *res; > + > + priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL); > + if (!priv) > + return -ENOMEM; > + > + priv->base = devm_platform_get_and_ioremap_resource(pdev, 0, &res); > + if (IS_ERR(priv->base)) > + return PTR_ERR(priv->base); > + > + of_property_read_u32(dev->of_node, "reg-io-width", &priv->io_width); > + > + config.dev = dev; > + config.size = resource_size(res); > + config.word_size = priv->io_width; > + config.stride = priv->io_width; > + config.priv = priv; > + > + return PTR_ERR_OR_ZERO(devm_nvmem_register(dev, &config)); > +} > + > +static const struct of_device_id mmio_nvmem_of_match_table[] = { > + { .compatible = "mmio-nvmem", }, > + {}, > +}; > + > +static struct platform_driver mmio_nvmem_driver = { > + .probe = mmio_nvmem_probe, > + .driver = { > + .name = "mmio_nvmem", > + .of_match_table = mmio_nvmem_of_match_table, > + }, > +}; > + > +static int __init mmio_nvmem_init(void) > +{ > + return platform_driver_register(&mmio_nvmem_driver); > +} > + > +subsys_initcall_sync(mmio_nvmem_init); > + > +MODULE_AUTHOR("Rafał Miłecki"); > +MODULE_LICENSE("GPL"); > +MODULE_DEVICE_TABLE(of, mmio_nvmem_of_match_table); _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH V4 2/2] nvmem: add generic driver for devices with MMIO access 2023-03-08 13:31 ` Srinivas Kandagatla @ 2023-03-08 15:42 ` Rafał Miłecki 2023-03-09 9:56 ` Srinivas Kandagatla 0 siblings, 1 reply; 6+ messages in thread From: Rafał Miłecki @ 2023-03-08 15:42 UTC (permalink / raw) To: Srinivas Kandagatla, Rob Herring, Krzysztof Kozlowski Cc: Matthias Brugger, Kunihiko Hayashi, Masami Hiramatsu, linux-mediatek, linux-arm-kernel, devicetree, linux-kernel, Rafał Miłecki On 8.03.2023 14:31, Srinivas Kandagatla wrote: > Thanks for doing this, Thank you for reviewing. Sadly it seems it still isn't clear if we can have this generic driver. I guess I missed some important questions or comments. In previous series we were discussing implementation details so I thought it's OK to have this driver after all. Not sure if I didn't waste time working on V4. I'll see if I can I address your concerns (see below). > On 28/02/2023 07:29, Rafał Miłecki wrote: >> From: Rafał Miłecki <rafal@milecki.pl> >> >> Some NVMEM devices can be accessed by simply mapping memory and reading >> from / writing to it. This driver adds support for a generic >> "mmio-nvmem" DT binding used by such devices. >> >> One of such devices is Broadcom's NVRAM. It's already supported (see >> NVMEM_BRCM_NVRAM) but existing driver covers both: > > What will happen to the old "brcm,nvram" compatible and the dt firmware that already have this node? I treat backward compatibility with previouly used bindings very seriously. I'm going to keep it. I may make an attempt to drop it in few years if it's very unlikely to break any setups. > If there is only one user for this then one would object that why do we need this DT level of abstraction to start with? > If this is not the case please consider adding those patches to this series. Existing Linux drivers prove that there is more hardware with MMIO based read access: brcm_nvram, mtk-efuse, uniphier-efuse. Migration of other drivers (mtk, unipher) is on hold as apparently there may be support for writing support soon. In any case this MMIO solution isn't completely unique to Broadcom. I don't have other patches to add to it right now. >> 1. NVMEM device access >> 2. NVMEM content parsing >> >> Once we get support for NVMEM layouts then existing NVRAM driver will >> get converted into a layout and generic driver will take over >> responsibility for data access. >> > > Even though this series is simple, but it is really confusing for two reasons. > > 1> Generic mmio nvmem bindings are incomplete and potentially change/evolve on every new user. Ex clks, regulators, endianess ... So it looks really fragile and incomplete to me as a generic bindings. > Is this want you are expecting? All 3 existing hardware support MMIO reads without extra clocks or regulators. I'm not sure if endianess belongs to this layer. Isn't that NVMEM content thing? I'm not claiming this driver is in its final and perfect state. For simple hardware that needs minor fixups we can add those later to this generic driver. Adding clocks should be possible, fine and easy. I'm sure there will be more complex hardware that we will not be able to support with this driver. It's require another driver and I'm fine with that. > 2> As you mentioned that this will replace broadcom NVMRAM, but this patch does nothing in relation to updating that driver, so the code is dead as it is. If you are considering to use it for Broadcom NVMRAM, please add those patches to this series so that we could see the real user for this code. Of course it does nothing because there are no layouts yet. I could migrate brcm_nvram into layout once there is layouts support. I don't agree this code is dead. It support new binding. It works. Every new binding and its driver are "dead" until you add first DT users. Here is real use: nvmem@1eff0000 { compatible = "mmio-nvmem"; reg = <0x1eff0000 0x10000>; }; _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH V4 2/2] nvmem: add generic driver for devices with MMIO access 2023-03-08 15:42 ` Rafał Miłecki @ 2023-03-09 9:56 ` Srinivas Kandagatla 0 siblings, 0 replies; 6+ messages in thread From: Srinivas Kandagatla @ 2023-03-09 9:56 UTC (permalink / raw) To: Rafał Miłecki, Rob Herring, Krzysztof Kozlowski Cc: Matthias Brugger, Kunihiko Hayashi, Masami Hiramatsu, linux-mediatek, linux-arm-kernel, devicetree, linux-kernel, Rafał Miłecki On 08/03/2023 15:42, Rafał Miłecki wrote: > On 8.03.2023 14:31, Srinivas Kandagatla wrote: >> Thanks for doing this, > > Thank you for reviewing. Sadly it seems it still isn't clear if we can > have this generic driver. I don't mean to be rude, but TBH, I don't see any value for this ATM, it is going to add something that we need to keep updating for every user. Unless anyone thinks otherwise. > > I guess I missed some important questions or comments. In previous > series we were discussing implementation details so I thought it's OK to > have this driver after all. Not sure if I didn't waste time working on > V4. I'll see if I can I address your concerns (see below). Lets not waste your time for now, we can revist this once we have more users. thanks, srini > > >> On 28/02/2023 07:29, Rafał Miłecki wrote: >>> From: Rafał Miłecki <rafal@milecki.pl> >>> >>> Some NVMEM devices can be accessed by simply mapping memory and reading >>> from / writing to it. This driver adds support for a generic >>> "mmio-nvmem" DT binding used by such devices. >>> >>> One of such devices is Broadcom's NVRAM. It's already supported (see >>> NVMEM_BRCM_NVRAM) but existing driver covers both: >> >> What will happen to the old "brcm,nvram" compatible and the dt >> firmware that already have this node? > > I treat backward compatibility with previouly used bindings very > seriously. I'm going to keep it. I may make an attempt to drop it in > few years if it's very unlikely to break any setups. > > >> If there is only one user for this then one would object that why do >> we need this DT level of abstraction to start with? >> If this is not the case please consider adding those patches to this >> series. > > Existing Linux drivers prove that there is more hardware with MMIO based > read access: brcm_nvram, mtk-efuse, uniphier-efuse. Migration of other > drivers (mtk, unipher) is on hold as apparently there may be support for > writing support soon. In any case this MMIO solution isn't completely > unique to Broadcom. > I don't have other patches to add to it right now. > > >>> 1. NVMEM device access >>> 2. NVMEM content parsing >>> >>> Once we get support for NVMEM layouts then existing NVRAM driver will >>> get converted into a layout and generic driver will take over >>> responsibility for data access. >>> >> >> Even though this series is simple, but it is really confusing for two >> reasons. >> >> 1> Generic mmio nvmem bindings are incomplete and potentially >> change/evolve on every new user. Ex clks, regulators, endianess ... So >> it looks really fragile and incomplete to me as a generic bindings. >> Is this want you are expecting? > > All 3 existing hardware support MMIO reads without extra clocks or > regulators. I'm not sure if endianess belongs to this layer. Isn't that > NVMEM content thing? > > I'm not claiming this driver is in its final and perfect state. For > simple hardware that needs minor fixups we can add those later to this > generic driver. Adding clocks should be possible, fine and easy. > > I'm sure there will be more complex hardware that we will not be able > to support with this driver. It's require another driver and I'm fine > with that. > > >> 2> As you mentioned that this will replace broadcom NVMRAM, but this >> patch does nothing in relation to updating that driver, so the code is >> dead as it is. If you are considering to use it for Broadcom NVMRAM, >> please add those patches to this series so that we could see the real >> user for this code. > > Of course it does nothing because there are no layouts yet. I could > migrate brcm_nvram into layout once there is layouts support. > > I don't agree this code is dead. It support new binding. It works. > Every new binding and its driver are "dead" until you add first DT > users. > > Here is real use: > > nvmem@1eff0000 { > compatible = "mmio-nvmem"; > reg = <0x1eff0000 0x10000>; > }; > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2023-03-09 9:57 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2023-02-28 7:29 [PATCH V4 0/2] nvmem: add and use generic MMIO NVMEM Rafał Miłecki 2023-02-28 7:29 ` [PATCH V4 1/2] dt-bindings: nvmem: mmio: new binding for MMIO accessible NVMEM devices Rafał Miłecki 2023-02-28 7:29 ` [PATCH V4 2/2] nvmem: add generic driver for devices with MMIO access Rafał Miłecki 2023-03-08 13:31 ` Srinivas Kandagatla 2023-03-08 15:42 ` Rafał Miłecki 2023-03-09 9:56 ` Srinivas Kandagatla
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).