| Message ID | 20230220174930.7440-3-zajec5@gmail.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | nvmem: add and use generic MMIO NVMEM | expand |
Hi, On 2/20/23 09:49, Rafał Miłecki wrote: > +static int mmio_nvmem_write(void *context, unsigned int offset, void *val, size_t bytes) > +{ > + struct mmio_nvmem *priv = context; > + > + switch (priv->io_width) { > + case 1: > + case 2: > + case 4: > + return -EOPNOTSUPP; I'm just curious: (since read supports those cases) what size writes are typically used here? And what value for priv->io_width? Thanks. > + default: > + memcpy_toio(priv->base + offset, val, bytes); > + } > + > + return 0; > +}
On 2023-02-20 20:55, Randy Dunlap wrote: > On 2/20/23 09:49, Rafał Miłecki wrote: >> +static int mmio_nvmem_write(void *context, unsigned int offset, void >> *val, size_t bytes) >> +{ >> + struct mmio_nvmem *priv = context; >> + >> + switch (priv->io_width) { >> + case 1: >> + case 2: >> + case 4: >> + return -EOPNOTSUPP; > > I'm just curious: (since read supports those cases) > > what size writes are typically used here? > And what value for priv->io_width? Sorry, I really don't have any statistics on this or any real overview of hardware that supports MMIO writes. It's probably a minority of devices that allows it.
Thanks Rafal for the patch, On 20/02/2023 17:49, 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: > 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. That's why it claims "brcm,nvram" > (conditionally for now). > > Signed-off-by: Rafał Miłecki <rafal@milecki.pl> > --- > V3: Support "reg-io-width", basic writing & "brcm,nvram" string > --- > drivers/nvmem/Kconfig | 10 ++++ > drivers/nvmem/Makefile | 2 + > drivers/nvmem/mmio.c | 125 +++++++++++++++++++++++++++++++++++++++++ > 3 files changed, 137 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..e247c943eea2 > --- /dev/null > +++ b/drivers/nvmem/mmio.c > @@ -0,0 +1,125 @@ > +// 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; > + u32 *dst32; > + u16 *dst16; > + u8 *dst8; > + > + if (priv->io_width && WARN_ON(bytes % priv->io_width)) > + return -EINVAL; > + If there is a code path in core that would allow this to happen then it needs to be fixed at core level rather than handling it in provider. > + switch (priv->io_width) { > + 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; > + default: > + memcpy_fromio(val, priv->base + offset, bytes); > + } > + > + return 0; > +} > + > +static int mmio_nvmem_write(void *context, unsigned int offset, void *val, size_t bytes) > +{ > + struct mmio_nvmem *priv = context; > + > + switch (priv->io_width) { If io_width is 0 core considers defaults it to 1. But in this patch its possible to still have priv->io_width 0. This is the behavior that you want? > + case 1: > + case 2: > + case 4: > + return -EOPNOTSUPP; > + default: > + memcpy_toio(priv->base + offset, val, bytes); so, are we saying that we only support writes to 8 bytes io_width? Any reason for doing this way? > + } > + > + 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, > + }; > + 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; > + if (!device_property_present(dev, "read-only")) > + config.reg_write = mmio_nvmem_write; core should have already marked it readonly based on this property. > + > + return PTR_ERR_OR_ZERO(devm_nvmem_register(dev, &config)); > +} > + > +static const struct of_device_id mmio_nvmem_of_match_table[] = { > + { .compatible = "mmio-nvmem", }, > + /* Custom bindings */ > +#if !IS_ENABLED(CONFIG_NVMEM_BRCM_NVRAM) > + { .compatible = "brcm,nvram", }, > +#endif If CONFIG_NVMEM_BRCM_NVRAM is not enabled we want to add "brcm,nvram" to this driver compatibles, that is something confusing to be honest atleast at this transition stage. How about making nvmem-mmio export functions for drivers like brcm to call directly. > + {}, > +}; > + > +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);
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..e247c943eea2 --- /dev/null +++ b/drivers/nvmem/mmio.c @@ -0,0 +1,125 @@ +// 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; + u32 *dst32; + u16 *dst16; + u8 *dst8; + + if (priv->io_width && WARN_ON(bytes % priv->io_width)) + return -EINVAL; + + switch (priv->io_width) { + 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; + default: + memcpy_fromio(val, priv->base + offset, bytes); + } + + return 0; +} + +static int mmio_nvmem_write(void *context, unsigned int offset, void *val, size_t bytes) +{ + struct mmio_nvmem *priv = context; + + switch (priv->io_width) { + case 1: + case 2: + case 4: + return -EOPNOTSUPP; + default: + memcpy_toio(priv->base + offset, val, bytes); + } + + 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, + }; + 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; + if (!device_property_present(dev, "read-only")) + config.reg_write = mmio_nvmem_write; + + return PTR_ERR_OR_ZERO(devm_nvmem_register(dev, &config)); +} + +static const struct of_device_id mmio_nvmem_of_match_table[] = { + { .compatible = "mmio-nvmem", }, + /* Custom bindings */ +#if !IS_ENABLED(CONFIG_NVMEM_BRCM_NVRAM) + { .compatible = "brcm,nvram", }, +#endif + {}, +}; + +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);