| Message ID | 20210424110608.15748-1-michael@walle.cc |
|---|---|
| Headers | show |
| Series | mtd: core: OTP nvmem provider support | expand |
On Sat, 24 Apr 2021 13:06:07 +0200, Michael Walle wrote: > SPI-NOR flashes may have OTP regions and have a nvmem binding. This > binding is described in mtd.yaml. > > Signed-off-by: Michael Walle <michael@walle.cc> > --- > Changes since v1: > - none > > Changes since RFC: > - new patch > > Documentation/devicetree/bindings/mtd/jedec,spi-nor.yaml | 6 ++++++ > 1 file changed, 6 insertions(+) > Reviewed-by: Rob Herring <robh@kernel.org>
Hi, On Sat, Apr 24, 2021 at 01:06:08PM +0200, Michael Walle wrote: > Flash OTP regions can already be read via user space. Some boards have > their serial number or MAC addresses stored in the OTP regions. Add > support for them being a (read-only) nvmem provider. > > The API to read the OTP data is already in place. It distinguishes > between factory and user OTP, thus there are up to two different > providers. > > Signed-off-by: Michael Walle <michael@walle.cc> This patch causes a boot failure with one of my qemu tests. With the patch in place, the flash fails to instantiate. [ 1.156578] Creating 3 MTD partitions on "physmap-flash": [ 1.157192] 0x000000000000-0x000000040000 : "U-Boot Bootloader" [ 1.184632] 0x000000040000-0x000000060000 : "U-Boot Environment" [ 1.201767] 0x000000060000-0x000000800000 : "Flash" [ 1.222320] Deleting MTD partitions on "physmap-flash": [ 1.222744] Deleting U-Boot Bootloader MTD partition [ 1.303597] Deleting U-Boot Environment MTD partition [ 1.368751] Deleting Flash MTD partition [ 1.430619] physmap-flash: probe of physmap-flash failed with error -61 -61 is -ENODATA. Other boot tests with different flash chips can still boot. Reverting this patch (as well as the follow-up patches) fixes the problem. I do not know if this is a problem with qemu or a problem with the patch, but, as I mentioned, other flash chips do still instantiate. Do you have an idea what to look for when I try to track down the problem ? Thanks, Guenter > --- > Changes since v1: > - combine name and compatible string in mtd_otp_nvmem_register() > > Changes since RFC: > - none > > drivers/mtd/mtdcore.c | 148 ++++++++++++++++++++++++++++++++++++++++ > include/linux/mtd/mtd.h | 2 + > 2 files changed, 150 insertions(+) > > diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c > index 9aaeadd53eb4..72e7000a86fd 100644 > --- a/drivers/mtd/mtdcore.c > +++ b/drivers/mtd/mtdcore.c > @@ -777,6 +777,146 @@ static void mtd_set_dev_defaults(struct mtd_info *mtd) > mutex_init(&mtd->master.chrdev_lock); > } > > +static ssize_t mtd_otp_size(struct mtd_info *mtd, bool is_user) > +{ > + struct otp_info *info = kmalloc(PAGE_SIZE, GFP_KERNEL); > + ssize_t size = 0; > + unsigned int i; > + size_t retlen; > + int ret; > + > + if (is_user) > + ret = mtd_get_user_prot_info(mtd, PAGE_SIZE, &retlen, info); > + else > + ret = mtd_get_fact_prot_info(mtd, PAGE_SIZE, &retlen, info); > + if (ret) > + goto err; > + > + for (i = 0; i < retlen / sizeof(*info); i++) { > + size += info->length; > + info++; > + } > + > + kfree(info); > + return size; > + > +err: > + kfree(info); > + return ret; > +} > + > +static struct nvmem_device *mtd_otp_nvmem_register(struct mtd_info *mtd, > + const char *compatible, > + int size, > + nvmem_reg_read_t reg_read) > +{ > + struct nvmem_device *nvmem = NULL; > + struct nvmem_config config = {}; > + struct device_node *np; > + > + /* DT binding is optional */ > + np = of_get_compatible_child(mtd->dev.of_node, compatible); > + > + /* OTP nvmem will be registered on the physical device */ > + config.dev = mtd->dev.parent; > + /* just reuse the compatible as name */ > + config.name = compatible; > + config.id = NVMEM_DEVID_NONE; > + config.owner = THIS_MODULE; > + config.type = NVMEM_TYPE_OTP; > + config.root_only = true; > + config.reg_read = reg_read; > + config.size = size; > + config.of_node = np; > + config.priv = mtd; > + > + nvmem = nvmem_register(&config); > + /* Just ignore if there is no NVMEM support in the kernel */ > + if (IS_ERR(nvmem) && PTR_ERR(nvmem) == -EOPNOTSUPP) > + nvmem = NULL; > + > + of_node_put(np); > + > + return nvmem; > +} > + > +static int mtd_nvmem_user_otp_reg_read(void *priv, unsigned int offset, > + void *val, size_t bytes) > +{ > + struct mtd_info *mtd = priv; > + size_t retlen; > + int ret; > + > + ret = mtd_read_user_prot_reg(mtd, offset, bytes, &retlen, val); > + if (ret) > + return ret; > + > + return retlen == bytes ? 0 : -EIO; > +} > + > +static int mtd_nvmem_fact_otp_reg_read(void *priv, unsigned int offset, > + void *val, size_t bytes) > +{ > + struct mtd_info *mtd = priv; > + size_t retlen; > + int ret; > + > + ret = mtd_read_fact_prot_reg(mtd, offset, bytes, &retlen, val); > + if (ret) > + return ret; > + > + return retlen == bytes ? 0 : -EIO; > +} > + > +static int mtd_otp_nvmem_add(struct mtd_info *mtd) > +{ > + struct nvmem_device *nvmem; > + ssize_t size; > + int err; > + > + if (mtd->_get_user_prot_info && mtd->_read_user_prot_reg) { > + size = mtd_otp_size(mtd, true); > + if (size < 0) > + return size; > + > + if (size > 0) { > + nvmem = mtd_otp_nvmem_register(mtd, "user-otp", size, > + mtd_nvmem_user_otp_reg_read); > + if (IS_ERR(nvmem)) { > + dev_err(&mtd->dev, "Failed to register OTP NVMEM device\n"); > + return PTR_ERR(nvmem); > + } > + mtd->otp_user_nvmem = nvmem; > + } > + } > + > + if (mtd->_get_fact_prot_info && mtd->_read_fact_prot_reg) { > + size = mtd_otp_size(mtd, false); > + if (size < 0) { > + err = size; > + goto err; > + } > + > + if (size > 0) { > + nvmem = mtd_otp_nvmem_register(mtd, "factory-otp", size, > + mtd_nvmem_fact_otp_reg_read); > + if (IS_ERR(nvmem)) { > + dev_err(&mtd->dev, "Failed to register OTP NVMEM device\n"); > + err = PTR_ERR(nvmem); > + goto err; > + } > + mtd->otp_factory_nvmem = nvmem; > + } > + } > + > + return 0; > + > +err: > + if (mtd->otp_user_nvmem) > + nvmem_unregister(mtd->otp_user_nvmem); > + return err; > +} > + > /** > * mtd_device_parse_register - parse partitions and register an MTD device. > * > @@ -852,6 +992,8 @@ int mtd_device_parse_register(struct mtd_info *mtd, const char * const *types, > register_reboot_notifier(&mtd->reboot_notifier); > } > > + ret = mtd_otp_nvmem_add(mtd); > + > out: > if (ret && device_is_registered(&mtd->dev)) > del_mtd_device(mtd); > @@ -873,6 +1015,12 @@ int mtd_device_unregister(struct mtd_info *master) > if (master->_reboot) > unregister_reboot_notifier(&master->reboot_notifier); > > + if (master->otp_user_nvmem) > + nvmem_unregister(master->otp_user_nvmem); > + > + if (master->otp_factory_nvmem) > + nvmem_unregister(master->otp_factory_nvmem); > + > err = del_mtd_partitions(master); > if (err) > return err; > diff --git a/include/linux/mtd/mtd.h b/include/linux/mtd/mtd.h > index a89955f3cbc8..88227044fc86 100644 > --- a/include/linux/mtd/mtd.h > +++ b/include/linux/mtd/mtd.h > @@ -380,6 +380,8 @@ struct mtd_info { > int usecount; > struct mtd_debug_info dbg; > struct nvmem_device *nvmem; > + struct nvmem_device *otp_user_nvmem; > + struct nvmem_device *otp_factory_nvmem; > > /* > * Parent device from the MTD partition point of view.
Hi Guenter, Am 2021-07-01 23:34, schrieb Guenter Roeck: > Hi, > > On Sat, Apr 24, 2021 at 01:06:08PM +0200, Michael Walle wrote: >> Flash OTP regions can already be read via user space. Some boards have >> their serial number or MAC addresses stored in the OTP regions. Add >> support for them being a (read-only) nvmem provider. >> >> The API to read the OTP data is already in place. It distinguishes >> between factory and user OTP, thus there are up to two different >> providers. >> >> Signed-off-by: Michael Walle <michael@walle.cc> > > This patch causes a boot failure with one of my qemu tests. > With the patch in place, the flash fails to instantiate. > > [ 1.156578] Creating 3 MTD partitions on "physmap-flash": > [ 1.157192] 0x000000000000-0x000000040000 : "U-Boot Bootloader" > [ 1.184632] 0x000000040000-0x000000060000 : "U-Boot Environment" > [ 1.201767] 0x000000060000-0x000000800000 : "Flash" > [ 1.222320] Deleting MTD partitions on "physmap-flash": > [ 1.222744] Deleting U-Boot Bootloader MTD partition > [ 1.303597] Deleting U-Boot Environment MTD partition > [ 1.368751] Deleting Flash MTD partition > [ 1.430619] physmap-flash: probe of physmap-flash failed with error > -61 > > -61 is -ENODATA. > > Other boot tests with different flash chips can still boot. > Reverting this patch (as well as the follow-up patches) fixes > the problem. > > I do not know if this is a problem with qemu or a problem with the > patch, but, as I mentioned, other flash chips do still instantiate. > > Do you have an idea what to look for when I try to track down the > problem ? I'd start by looking at the return code of mtd_otp_size() because that should be the only function which communicates with the flash at probe time. Can you share how to reproduce that problem? Like the qemu commandline and involved images? -michael
On 7/1/21 3:10 PM, Michael Walle wrote: > Hi Guenter, > > Am 2021-07-01 23:34, schrieb Guenter Roeck: >> Hi, >> >> On Sat, Apr 24, 2021 at 01:06:08PM +0200, Michael Walle wrote: >>> Flash OTP regions can already be read via user space. Some boards have >>> their serial number or MAC addresses stored in the OTP regions. Add >>> support for them being a (read-only) nvmem provider. >>> >>> The API to read the OTP data is already in place. It distinguishes >>> between factory and user OTP, thus there are up to two different >>> providers. >>> >>> Signed-off-by: Michael Walle <michael@walle.cc> >> >> This patch causes a boot failure with one of my qemu tests. >> With the patch in place, the flash fails to instantiate. >> >> [ 1.156578] Creating 3 MTD partitions on "physmap-flash": >> [ 1.157192] 0x000000000000-0x000000040000 : "U-Boot Bootloader" >> [ 1.184632] 0x000000040000-0x000000060000 : "U-Boot Environment" >> [ 1.201767] 0x000000060000-0x000000800000 : "Flash" >> [ 1.222320] Deleting MTD partitions on "physmap-flash": >> [ 1.222744] Deleting U-Boot Bootloader MTD partition >> [ 1.303597] Deleting U-Boot Environment MTD partition >> [ 1.368751] Deleting Flash MTD partition >> [ 1.430619] physmap-flash: probe of physmap-flash failed with error -61 >> >> -61 is -ENODATA. >> >> Other boot tests with different flash chips can still boot. >> Reverting this patch (as well as the follow-up patches) fixes >> the problem. >> >> I do not know if this is a problem with qemu or a problem with the >> patch, but, as I mentioned, other flash chips do still instantiate. >> >> Do you have an idea what to look for when I try to track down the problem ? > > I'd start by looking at the return code of mtd_otp_size() because that > should be the only function which communicates with the flash at probe > time. > > Can you share how to reproduce that problem? Like the qemu commandline > and involved images? > qemu-system-arm -M z2 -kernel arch/arm/boot/zImage -no-reboot \ -snapshot -drive file=/tmp/flash,format=raw,if=pflash \ --append "root=/dev/mtdblock2 console=ttyS0" \ -nographic -monitor null -serial stdio This is with qemu v6.0 and pxa_defconfig. The actual flash image doesn't really matter (an empty file with a size of 1024*1024*8 bytes is sufficient). Debugging shows that -ENODATA is reported by cfi_intelext_otp_walk(), thanks to: [ 0.737244] #### FeatureSupport: 0x0 NumProtectionFields: 1 which seems to suggest that there are indeed flash chips which don't support OTP data. With this in mind, is it indeed appropriate to disable support for all flash chips which don't support OTP data ? Thanks, Guenter
Am 2021-07-02 03:55, schrieb Guenter Roeck: > On 7/1/21 3:10 PM, Michael Walle wrote: >> Hi Guenter, >> >> Am 2021-07-01 23:34, schrieb Guenter Roeck: >>> Hi, >>> >>> On Sat, Apr 24, 2021 at 01:06:08PM +0200, Michael Walle wrote: >>>> Flash OTP regions can already be read via user space. Some boards >>>> have >>>> their serial number or MAC addresses stored in the OTP regions. Add >>>> support for them being a (read-only) nvmem provider. >>>> >>>> The API to read the OTP data is already in place. It distinguishes >>>> between factory and user OTP, thus there are up to two different >>>> providers. >>>> >>>> Signed-off-by: Michael Walle <michael@walle.cc> >>> >>> This patch causes a boot failure with one of my qemu tests. >>> With the patch in place, the flash fails to instantiate. >>> >>> [ 1.156578] Creating 3 MTD partitions on "physmap-flash": >>> [ 1.157192] 0x000000000000-0x000000040000 : "U-Boot Bootloader" >>> [ 1.184632] 0x000000040000-0x000000060000 : "U-Boot Environment" >>> [ 1.201767] 0x000000060000-0x000000800000 : "Flash" >>> [ 1.222320] Deleting MTD partitions on "physmap-flash": >>> [ 1.222744] Deleting U-Boot Bootloader MTD partition >>> [ 1.303597] Deleting U-Boot Environment MTD partition >>> [ 1.368751] Deleting Flash MTD partition >>> [ 1.430619] physmap-flash: probe of physmap-flash failed with >>> error -61 >>> >>> -61 is -ENODATA. >>> >>> Other boot tests with different flash chips can still boot. >>> Reverting this patch (as well as the follow-up patches) fixes >>> the problem. >>> >>> I do not know if this is a problem with qemu or a problem with the >>> patch, but, as I mentioned, other flash chips do still instantiate. >>> >>> Do you have an idea what to look for when I try to track down the >>> problem ? >> >> I'd start by looking at the return code of mtd_otp_size() because that >> should be the only function which communicates with the flash at probe >> time. >> >> Can you share how to reproduce that problem? Like the qemu commandline >> and involved images? >> > > qemu-system-arm -M z2 -kernel arch/arm/boot/zImage -no-reboot \ > -snapshot -drive file=/tmp/flash,format=raw,if=pflash \ > --append "root=/dev/mtdblock2 console=ttyS0" \ > -nographic -monitor null -serial stdio > > This is with qemu v6.0 and pxa_defconfig. The actual flash image > doesn't > really matter (an empty file with a size of 1024*1024*8 bytes is > sufficient). For completeness: with pxa_defconfig, I guess. > Debugging shows that -ENODATA is reported by cfi_intelext_otp_walk(), > thanks to: Thanks for already looking into this. > > [ 0.737244] #### FeatureSupport: 0x0 NumProtectionFields: 1 > > which seems to suggest that there are indeed flash chips which don't > support > OTP data. With this in mind, is it indeed appropriate to disable > support for > all flash chips which don't support OTP data ? Yes of course. The SPI NOR drivers doesn't register the callbacks if there is no OTP support. The others return ENODATA, which I missed. I'll send a patch shortly. -michael
Some flashes provide one (or more) OTP regions which can be used to store MAC addresses or serial numbers. Implement a NVMEM provider for this storage which then can be used by a network card to fetch the MAC adress for example. This is an example DT node: flash@0 { otp { compatible = "user-otp"; #address-cells = <1>; #size-cells = <1>; serial-number@0 { reg = <0x0 0x8>; }; }; }; Michael Walle (5): nvmem: core: allow specifying of_node dt-bindings: mtd: add YAML schema for the generic MTD bindings dt-bindings: mtd: add OTP bindings dt-bindings: mtd: spi-nor: add otp property mtd: core: add OTP nvmem provider support .../devicetree/bindings/mtd/common.txt | 16 +- .../bindings/mtd/jedec,spi-nor.yaml | 6 + .../devicetree/bindings/mtd/mtd.yaml | 89 +++++++++++ drivers/mtd/mtdcore.c | 148 ++++++++++++++++++ drivers/nvmem/core.c | 4 +- include/linux/mtd/mtd.h | 2 + include/linux/nvmem-provider.h | 2 + 7 files changed, 251 insertions(+), 16 deletions(-) create mode 100644 Documentation/devicetree/bindings/mtd/mtd.yaml