* [PATCH v2 5/5] mtd: core: add OTP nvmem provider support
@ 2021-04-24 11:06 ` Michael Walle
0 siblings, 0 replies; 44+ messages in thread
From: Michael Walle @ 2021-04-24 11:06 UTC (permalink / raw)
To: linux-mtd, devicetree, linux-kernel
Cc: Miquel Raynal, Richard Weinberger, Vignesh Raghavendra,
Rob Herring, Srinivas Kandagatla, Michael Walle
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>
---
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.
--
2.20.1
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply related [flat|nested] 44+ messages in thread
* Re: [PATCH v2 5/5] mtd: core: add OTP nvmem provider support
2021-04-24 11:06 ` Michael Walle
@ 2021-05-10 10:43 ` Miquel Raynal
-1 siblings, 0 replies; 44+ messages in thread
From: Miquel Raynal @ 2021-05-10 10:43 UTC (permalink / raw)
To: Michael Walle, linux-mtd, devicetree, linux-kernel
Cc: Miquel Raynal, Richard Weinberger, Vignesh Raghavendra,
Rob Herring, Srinivas Kandagatla
On Sat, 2021-04-24 at 11:06:08 UTC, 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>
Applied to https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git mtd/next, thanks.
Miquel
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH v2 5/5] mtd: core: add OTP nvmem provider support
@ 2021-05-10 10:43 ` Miquel Raynal
0 siblings, 0 replies; 44+ messages in thread
From: Miquel Raynal @ 2021-05-10 10:43 UTC (permalink / raw)
To: Michael Walle, linux-mtd, devicetree, linux-kernel
Cc: Miquel Raynal, Richard Weinberger, Vignesh Raghavendra,
Rob Herring, Srinivas Kandagatla
On Sat, 2021-04-24 at 11:06:08 UTC, 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>
Applied to https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git mtd/next, thanks.
Miquel
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 44+ messages in thread
* [PATCH] mtd: core: Fix freeing of otp_info buffer
2021-04-24 11:06 ` Michael Walle
@ 2021-05-18 18:55 ` Jon Hunter
-1 siblings, 0 replies; 44+ messages in thread
From: Jon Hunter @ 2021-05-18 18:55 UTC (permalink / raw)
To: Miquel Raynal, Richard Weinberger, Vignesh Raghavendra, Michael Walle
Cc: linux-mtd, linux-kernel, linux-tegra, Jon Hunter
Commit 4b361cfa8624 ("mtd: core: add OTP nvmem provider support") is
causing the following panic ...
------------[ cut here ]------------
kernel BUG at /local/workdir/tegra/linux_next/kernel/mm/slab.c:2730!
Internal error: Oops - BUG: 0 [#1] PREEMPT SMP ARM
Modules linked in:
CPU: 3 PID: 1 Comm: swapper/0 Not tainted 5.13.0-rc2-next-20210518 #1
Hardware name: NVIDIA Tegra SoC (Flattened Device Tree)
PC is at ___cache_free+0x3f8/0x51c
...
[<c029bb1c>] (___cache_free) from [<c029c658>] (kfree+0xac/0x1bc)
[<c029c658>] (kfree) from [<c06da094>] (mtd_otp_size+0xc4/0x108)
[<c06da094>] (mtd_otp_size) from [<c06dc864>] (mtd_device_parse_register+0xe4/0x2b4)
[<c06dc864>] (mtd_device_parse_register) from [<c06e3ccc>] (spi_nor_probe+0x210/0x2c0)
[<c06e3ccc>] (spi_nor_probe) from [<c06e9578>] (spi_probe+0x88/0xac)
[<c06e9578>] (spi_probe) from [<c066891c>] (really_probe+0x214/0x3a4)
[<c066891c>] (really_probe) from [<c0668b14>] (driver_probe_device+0x68/0xc0)
[<c0668b14>] (driver_probe_device) from [<c0666cf8>] (bus_for_each_drv+0x5c/0xbc)
[<c0666cf8>] (bus_for_each_drv) from [<c0668694>] (__device_attach+0xe4/0x150)
[<c0668694>] (__device_attach) from [<c06679e0>] (bus_probe_device+0x84/0x8c)
[<c06679e0>] (bus_probe_device) from [<c06657f8>] (device_add+0x48c/0x868)
[<c06657f8>] (device_add) from [<c06eb784>] (spi_add_device+0xa0/0x168)
[<c06eb784>] (spi_add_device) from [<c06ec9a8>] (spi_register_controller+0x8b8/0xb38)
[<c06ec9a8>] (spi_register_controller) from [<c06ecc3c>] (devm_spi_register_controller+0x14/0x50)
[<c06ecc3c>] (devm_spi_register_controller) from [<c06f0510>] (tegra_spi_probe+0x33c/0x450)
[<c06f0510>] (tegra_spi_probe) from [<c066abec>] (platform_probe+0x5c/0xb8)
[<c066abec>] (platform_probe) from [<c066891c>] (really_probe+0x214/0x3a4)
[<c066891c>] (really_probe) from [<c0668b14>] (driver_probe_device+0x68/0xc0)
[<c0668b14>] (driver_probe_device) from [<c0668e30>] (device_driver_attach+0x58/0x60)
[<c0668e30>] (device_driver_attach) from [<c0668eb8>] (__driver_attach+0x80/0xc8)
[<c0668eb8>] (__driver_attach) from [<c0666c48>] (bus_for_each_dev+0x78/0xb8)
[<c0666c48>] (bus_for_each_dev) from [<c0667c44>] (bus_add_driver+0x164/0x1e8)
[<c0667c44>] (bus_add_driver) from [<c066997c>] (driver_register+0x7c/0x114)
[<c066997c>] (driver_register) from [<c010223c>] (do_one_initcall+0x50/0x2b0)
[<c010223c>] (do_one_initcall) from [<c11011f0>] (kernel_init_freeable+0x1a8/0x1fc)
[<c11011f0>] (kernel_init_freeable) from [<c0c09190>] (kernel_init+0x8/0x118)
[<c0c09190>] (kernel_init) from [<c01001b0>] (ret_from_fork+0x14/0x24)
...
---[ end trace 0f652dd222de75d7 ]---
In the function mtd_otp_size() a buffer is allocated by calling
kmalloc() and a pointer to the buffer is stored in a variable 'info'.
The pointer 'info' may then be incremented depending on the length
returned from mtd_get_user/fact_prot_info(). If 'info' is incremented,
when kfree() is called to free the buffer the above panic occurs because
we are no longer passing the original address of the buffer allocated.
Fix this by indexing through the buffer allocated to avoid incrementing
the pointer.
Fixes: 4b361cfa8624 ("mtd: core: add OTP nvmem provider support")
Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
---
drivers/mtd/mtdcore.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c
index 3ae261661eea..ffa46ccea0cf 100644
--- a/drivers/mtd/mtdcore.c
+++ b/drivers/mtd/mtdcore.c
@@ -792,10 +792,8 @@ static ssize_t mtd_otp_size(struct mtd_info *mtd, bool is_user)
if (ret)
goto err;
- for (i = 0; i < retlen / sizeof(*info); i++) {
- size += info->length;
- info++;
- }
+ for (i = 0; i < retlen / sizeof(*info); i++)
+ size += info[i].length;
kfree(info);
return size;
--
2.7.4
^ permalink raw reply related [flat|nested] 44+ messages in thread
* [PATCH] mtd: core: Fix freeing of otp_info buffer
@ 2021-05-18 18:55 ` Jon Hunter
0 siblings, 0 replies; 44+ messages in thread
From: Jon Hunter @ 2021-05-18 18:55 UTC (permalink / raw)
To: Miquel Raynal, Richard Weinberger, Vignesh Raghavendra, Michael Walle
Cc: linux-mtd, linux-kernel, linux-tegra, Jon Hunter
Commit 4b361cfa8624 ("mtd: core: add OTP nvmem provider support") is
causing the following panic ...
------------[ cut here ]------------
kernel BUG at /local/workdir/tegra/linux_next/kernel/mm/slab.c:2730!
Internal error: Oops - BUG: 0 [#1] PREEMPT SMP ARM
Modules linked in:
CPU: 3 PID: 1 Comm: swapper/0 Not tainted 5.13.0-rc2-next-20210518 #1
Hardware name: NVIDIA Tegra SoC (Flattened Device Tree)
PC is at ___cache_free+0x3f8/0x51c
...
[<c029bb1c>] (___cache_free) from [<c029c658>] (kfree+0xac/0x1bc)
[<c029c658>] (kfree) from [<c06da094>] (mtd_otp_size+0xc4/0x108)
[<c06da094>] (mtd_otp_size) from [<c06dc864>] (mtd_device_parse_register+0xe4/0x2b4)
[<c06dc864>] (mtd_device_parse_register) from [<c06e3ccc>] (spi_nor_probe+0x210/0x2c0)
[<c06e3ccc>] (spi_nor_probe) from [<c06e9578>] (spi_probe+0x88/0xac)
[<c06e9578>] (spi_probe) from [<c066891c>] (really_probe+0x214/0x3a4)
[<c066891c>] (really_probe) from [<c0668b14>] (driver_probe_device+0x68/0xc0)
[<c0668b14>] (driver_probe_device) from [<c0666cf8>] (bus_for_each_drv+0x5c/0xbc)
[<c0666cf8>] (bus_for_each_drv) from [<c0668694>] (__device_attach+0xe4/0x150)
[<c0668694>] (__device_attach) from [<c06679e0>] (bus_probe_device+0x84/0x8c)
[<c06679e0>] (bus_probe_device) from [<c06657f8>] (device_add+0x48c/0x868)
[<c06657f8>] (device_add) from [<c06eb784>] (spi_add_device+0xa0/0x168)
[<c06eb784>] (spi_add_device) from [<c06ec9a8>] (spi_register_controller+0x8b8/0xb38)
[<c06ec9a8>] (spi_register_controller) from [<c06ecc3c>] (devm_spi_register_controller+0x14/0x50)
[<c06ecc3c>] (devm_spi_register_controller) from [<c06f0510>] (tegra_spi_probe+0x33c/0x450)
[<c06f0510>] (tegra_spi_probe) from [<c066abec>] (platform_probe+0x5c/0xb8)
[<c066abec>] (platform_probe) from [<c066891c>] (really_probe+0x214/0x3a4)
[<c066891c>] (really_probe) from [<c0668b14>] (driver_probe_device+0x68/0xc0)
[<c0668b14>] (driver_probe_device) from [<c0668e30>] (device_driver_attach+0x58/0x60)
[<c0668e30>] (device_driver_attach) from [<c0668eb8>] (__driver_attach+0x80/0xc8)
[<c0668eb8>] (__driver_attach) from [<c0666c48>] (bus_for_each_dev+0x78/0xb8)
[<c0666c48>] (bus_for_each_dev) from [<c0667c44>] (bus_add_driver+0x164/0x1e8)
[<c0667c44>] (bus_add_driver) from [<c066997c>] (driver_register+0x7c/0x114)
[<c066997c>] (driver_register) from [<c010223c>] (do_one_initcall+0x50/0x2b0)
[<c010223c>] (do_one_initcall) from [<c11011f0>] (kernel_init_freeable+0x1a8/0x1fc)
[<c11011f0>] (kernel_init_freeable) from [<c0c09190>] (kernel_init+0x8/0x118)
[<c0c09190>] (kernel_init) from [<c01001b0>] (ret_from_fork+0x14/0x24)
...
---[ end trace 0f652dd222de75d7 ]---
In the function mtd_otp_size() a buffer is allocated by calling
kmalloc() and a pointer to the buffer is stored in a variable 'info'.
The pointer 'info' may then be incremented depending on the length
returned from mtd_get_user/fact_prot_info(). If 'info' is incremented,
when kfree() is called to free the buffer the above panic occurs because
we are no longer passing the original address of the buffer allocated.
Fix this by indexing through the buffer allocated to avoid incrementing
the pointer.
Fixes: 4b361cfa8624 ("mtd: core: add OTP nvmem provider support")
Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
---
drivers/mtd/mtdcore.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c
index 3ae261661eea..ffa46ccea0cf 100644
--- a/drivers/mtd/mtdcore.c
+++ b/drivers/mtd/mtdcore.c
@@ -792,10 +792,8 @@ static ssize_t mtd_otp_size(struct mtd_info *mtd, bool is_user)
if (ret)
goto err;
- for (i = 0; i < retlen / sizeof(*info); i++) {
- size += info->length;
- info++;
- }
+ for (i = 0; i < retlen / sizeof(*info); i++)
+ size += info[i].length;
kfree(info);
return size;
--
2.7.4
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply related [flat|nested] 44+ messages in thread
* Re: [PATCH] mtd: core: Fix freeing of otp_info buffer
2021-05-18 18:55 ` Jon Hunter
@ 2021-05-18 20:02 ` Michael Walle
-1 siblings, 0 replies; 44+ messages in thread
From: Michael Walle @ 2021-05-18 20:02 UTC (permalink / raw)
To: Jon Hunter
Cc: Miquel Raynal, Richard Weinberger, Vignesh Raghavendra,
linux-mtd, linux-kernel, linux-tegra
Am 2021-05-18 20:55, schrieb Jon Hunter:
> Commit 4b361cfa8624 ("mtd: core: add OTP nvmem provider support") is
> causing the following panic ...
>
> ------------[ cut here ]------------
> kernel BUG at /local/workdir/tegra/linux_next/kernel/mm/slab.c:2730!
> Internal error: Oops - BUG: 0 [#1] PREEMPT SMP ARM
> Modules linked in:
> CPU: 3 PID: 1 Comm: swapper/0 Not tainted 5.13.0-rc2-next-20210518 #1
> Hardware name: NVIDIA Tegra SoC (Flattened Device Tree)
> PC is at ___cache_free+0x3f8/0x51c
> ...
> [<c029bb1c>] (___cache_free) from [<c029c658>] (kfree+0xac/0x1bc)
> [<c029c658>] (kfree) from [<c06da094>] (mtd_otp_size+0xc4/0x108)
> [<c06da094>] (mtd_otp_size) from [<c06dc864>]
> (mtd_device_parse_register+0xe4/0x2b4)
> [<c06dc864>] (mtd_device_parse_register) from [<c06e3ccc>]
> (spi_nor_probe+0x210/0x2c0)
> [<c06e3ccc>] (spi_nor_probe) from [<c06e9578>] (spi_probe+0x88/0xac)
> [<c06e9578>] (spi_probe) from [<c066891c>] (really_probe+0x214/0x3a4)
> [<c066891c>] (really_probe) from [<c0668b14>]
> (driver_probe_device+0x68/0xc0)
> [<c0668b14>] (driver_probe_device) from [<c0666cf8>]
> (bus_for_each_drv+0x5c/0xbc)
> [<c0666cf8>] (bus_for_each_drv) from [<c0668694>]
> (__device_attach+0xe4/0x150)
> [<c0668694>] (__device_attach) from [<c06679e0>]
> (bus_probe_device+0x84/0x8c)
> [<c06679e0>] (bus_probe_device) from [<c06657f8>]
> (device_add+0x48c/0x868)
> [<c06657f8>] (device_add) from [<c06eb784>]
> (spi_add_device+0xa0/0x168)
> [<c06eb784>] (spi_add_device) from [<c06ec9a8>]
> (spi_register_controller+0x8b8/0xb38)
> [<c06ec9a8>] (spi_register_controller) from [<c06ecc3c>]
> (devm_spi_register_controller+0x14/0x50)
> [<c06ecc3c>] (devm_spi_register_controller) from [<c06f0510>]
> (tegra_spi_probe+0x33c/0x450)
> [<c06f0510>] (tegra_spi_probe) from [<c066abec>]
> (platform_probe+0x5c/0xb8)
> [<c066abec>] (platform_probe) from [<c066891c>]
> (really_probe+0x214/0x3a4)
> [<c066891c>] (really_probe) from [<c0668b14>]
> (driver_probe_device+0x68/0xc0)
> [<c0668b14>] (driver_probe_device) from [<c0668e30>]
> (device_driver_attach+0x58/0x60)
> [<c0668e30>] (device_driver_attach) from [<c0668eb8>]
> (__driver_attach+0x80/0xc8)
> [<c0668eb8>] (__driver_attach) from [<c0666c48>]
> (bus_for_each_dev+0x78/0xb8)
> [<c0666c48>] (bus_for_each_dev) from [<c0667c44>]
> (bus_add_driver+0x164/0x1e8)
> [<c0667c44>] (bus_add_driver) from [<c066997c>]
> (driver_register+0x7c/0x114)
> [<c066997c>] (driver_register) from [<c010223c>]
> (do_one_initcall+0x50/0x2b0)
> [<c010223c>] (do_one_initcall) from [<c11011f0>]
> (kernel_init_freeable+0x1a8/0x1fc)
> [<c11011f0>] (kernel_init_freeable) from [<c0c09190>]
> (kernel_init+0x8/0x118)
> [<c0c09190>] (kernel_init) from [<c01001b0>] (ret_from_fork+0x14/0x24)
> ...
> ---[ end trace 0f652dd222de75d7 ]---
>
> In the function mtd_otp_size() a buffer is allocated by calling
> kmalloc() and a pointer to the buffer is stored in a variable 'info'.
> The pointer 'info' may then be incremented depending on the length
> returned from mtd_get_user/fact_prot_info(). If 'info' is incremented,
> when kfree() is called to free the buffer the above panic occurs
> because
> we are no longer passing the original address of the buffer allocated.
> Fix this by indexing through the buffer allocated to avoid incrementing
> the pointer.
>
> Fixes: 4b361cfa8624 ("mtd: core: add OTP nvmem provider support")
> Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
uhm.. yes of course. Two fixes for this function. Not my best day :/
I'm wondering why CONFIG_SLUB_DEBUG_ON doesn't catch this, whereas
slub_debug=f (or fzpu) as commandline parameter works as expected.
Reviewed-by: Michael Walle <michael@walle.cc>
Thanks,
-michael
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH] mtd: core: Fix freeing of otp_info buffer
@ 2021-05-18 20:02 ` Michael Walle
0 siblings, 0 replies; 44+ messages in thread
From: Michael Walle @ 2021-05-18 20:02 UTC (permalink / raw)
To: Jon Hunter
Cc: Miquel Raynal, Richard Weinberger, Vignesh Raghavendra,
linux-mtd, linux-kernel, linux-tegra
Am 2021-05-18 20:55, schrieb Jon Hunter:
> Commit 4b361cfa8624 ("mtd: core: add OTP nvmem provider support") is
> causing the following panic ...
>
> ------------[ cut here ]------------
> kernel BUG at /local/workdir/tegra/linux_next/kernel/mm/slab.c:2730!
> Internal error: Oops - BUG: 0 [#1] PREEMPT SMP ARM
> Modules linked in:
> CPU: 3 PID: 1 Comm: swapper/0 Not tainted 5.13.0-rc2-next-20210518 #1
> Hardware name: NVIDIA Tegra SoC (Flattened Device Tree)
> PC is at ___cache_free+0x3f8/0x51c
> ...
> [<c029bb1c>] (___cache_free) from [<c029c658>] (kfree+0xac/0x1bc)
> [<c029c658>] (kfree) from [<c06da094>] (mtd_otp_size+0xc4/0x108)
> [<c06da094>] (mtd_otp_size) from [<c06dc864>]
> (mtd_device_parse_register+0xe4/0x2b4)
> [<c06dc864>] (mtd_device_parse_register) from [<c06e3ccc>]
> (spi_nor_probe+0x210/0x2c0)
> [<c06e3ccc>] (spi_nor_probe) from [<c06e9578>] (spi_probe+0x88/0xac)
> [<c06e9578>] (spi_probe) from [<c066891c>] (really_probe+0x214/0x3a4)
> [<c066891c>] (really_probe) from [<c0668b14>]
> (driver_probe_device+0x68/0xc0)
> [<c0668b14>] (driver_probe_device) from [<c0666cf8>]
> (bus_for_each_drv+0x5c/0xbc)
> [<c0666cf8>] (bus_for_each_drv) from [<c0668694>]
> (__device_attach+0xe4/0x150)
> [<c0668694>] (__device_attach) from [<c06679e0>]
> (bus_probe_device+0x84/0x8c)
> [<c06679e0>] (bus_probe_device) from [<c06657f8>]
> (device_add+0x48c/0x868)
> [<c06657f8>] (device_add) from [<c06eb784>]
> (spi_add_device+0xa0/0x168)
> [<c06eb784>] (spi_add_device) from [<c06ec9a8>]
> (spi_register_controller+0x8b8/0xb38)
> [<c06ec9a8>] (spi_register_controller) from [<c06ecc3c>]
> (devm_spi_register_controller+0x14/0x50)
> [<c06ecc3c>] (devm_spi_register_controller) from [<c06f0510>]
> (tegra_spi_probe+0x33c/0x450)
> [<c06f0510>] (tegra_spi_probe) from [<c066abec>]
> (platform_probe+0x5c/0xb8)
> [<c066abec>] (platform_probe) from [<c066891c>]
> (really_probe+0x214/0x3a4)
> [<c066891c>] (really_probe) from [<c0668b14>]
> (driver_probe_device+0x68/0xc0)
> [<c0668b14>] (driver_probe_device) from [<c0668e30>]
> (device_driver_attach+0x58/0x60)
> [<c0668e30>] (device_driver_attach) from [<c0668eb8>]
> (__driver_attach+0x80/0xc8)
> [<c0668eb8>] (__driver_attach) from [<c0666c48>]
> (bus_for_each_dev+0x78/0xb8)
> [<c0666c48>] (bus_for_each_dev) from [<c0667c44>]
> (bus_add_driver+0x164/0x1e8)
> [<c0667c44>] (bus_add_driver) from [<c066997c>]
> (driver_register+0x7c/0x114)
> [<c066997c>] (driver_register) from [<c010223c>]
> (do_one_initcall+0x50/0x2b0)
> [<c010223c>] (do_one_initcall) from [<c11011f0>]
> (kernel_init_freeable+0x1a8/0x1fc)
> [<c11011f0>] (kernel_init_freeable) from [<c0c09190>]
> (kernel_init+0x8/0x118)
> [<c0c09190>] (kernel_init) from [<c01001b0>] (ret_from_fork+0x14/0x24)
> ...
> ---[ end trace 0f652dd222de75d7 ]---
>
> In the function mtd_otp_size() a buffer is allocated by calling
> kmalloc() and a pointer to the buffer is stored in a variable 'info'.
> The pointer 'info' may then be incremented depending on the length
> returned from mtd_get_user/fact_prot_info(). If 'info' is incremented,
> when kfree() is called to free the buffer the above panic occurs
> because
> we are no longer passing the original address of the buffer allocated.
> Fix this by indexing through the buffer allocated to avoid incrementing
> the pointer.
>
> Fixes: 4b361cfa8624 ("mtd: core: add OTP nvmem provider support")
> Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
uhm.. yes of course. Two fixes for this function. Not my best day :/
I'm wondering why CONFIG_SLUB_DEBUG_ON doesn't catch this, whereas
slub_debug=f (or fzpu) as commandline parameter works as expected.
Reviewed-by: Michael Walle <michael@walle.cc>
Thanks,
-michael
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH] mtd: core: Fix freeing of otp_info buffer
2021-05-18 18:55 ` Jon Hunter
@ 2021-05-26 9:03 ` Miquel Raynal
-1 siblings, 0 replies; 44+ messages in thread
From: Miquel Raynal @ 2021-05-26 9:03 UTC (permalink / raw)
To: Jon Hunter, Miquel Raynal, Richard Weinberger,
Vignesh Raghavendra, Michael Walle
Cc: linux-mtd, linux-kernel, linux-tegra
On Tue, 2021-05-18 at 18:55:03 UTC, Jon Hunter wrote:
> Commit 4b361cfa8624 ("mtd: core: add OTP nvmem provider support") is
> causing the following panic ...
>
> ------------[ cut here ]------------
> kernel BUG at /local/workdir/tegra/linux_next/kernel/mm/slab.c:2730!
> Internal error: Oops - BUG: 0 [#1] PREEMPT SMP ARM
> Modules linked in:
> CPU: 3 PID: 1 Comm: swapper/0 Not tainted 5.13.0-rc2-next-20210518 #1
> Hardware name: NVIDIA Tegra SoC (Flattened Device Tree)
> PC is at ___cache_free+0x3f8/0x51c
> ...
> [<c029bb1c>] (___cache_free) from [<c029c658>] (kfree+0xac/0x1bc)
> [<c029c658>] (kfree) from [<c06da094>] (mtd_otp_size+0xc4/0x108)
> [<c06da094>] (mtd_otp_size) from [<c06dc864>] (mtd_device_parse_register+0xe4/0x2b4)
> [<c06dc864>] (mtd_device_parse_register) from [<c06e3ccc>] (spi_nor_probe+0x210/0x2c0)
> [<c06e3ccc>] (spi_nor_probe) from [<c06e9578>] (spi_probe+0x88/0xac)
> [<c06e9578>] (spi_probe) from [<c066891c>] (really_probe+0x214/0x3a4)
> [<c066891c>] (really_probe) from [<c0668b14>] (driver_probe_device+0x68/0xc0)
> [<c0668b14>] (driver_probe_device) from [<c0666cf8>] (bus_for_each_drv+0x5c/0xbc)
> [<c0666cf8>] (bus_for_each_drv) from [<c0668694>] (__device_attach+0xe4/0x150)
> [<c0668694>] (__device_attach) from [<c06679e0>] (bus_probe_device+0x84/0x8c)
> [<c06679e0>] (bus_probe_device) from [<c06657f8>] (device_add+0x48c/0x868)
> [<c06657f8>] (device_add) from [<c06eb784>] (spi_add_device+0xa0/0x168)
> [<c06eb784>] (spi_add_device) from [<c06ec9a8>] (spi_register_controller+0x8b8/0xb38)
> [<c06ec9a8>] (spi_register_controller) from [<c06ecc3c>] (devm_spi_register_controller+0x14/0x50)
> [<c06ecc3c>] (devm_spi_register_controller) from [<c06f0510>] (tegra_spi_probe+0x33c/0x450)
> [<c06f0510>] (tegra_spi_probe) from [<c066abec>] (platform_probe+0x5c/0xb8)
> [<c066abec>] (platform_probe) from [<c066891c>] (really_probe+0x214/0x3a4)
> [<c066891c>] (really_probe) from [<c0668b14>] (driver_probe_device+0x68/0xc0)
> [<c0668b14>] (driver_probe_device) from [<c0668e30>] (device_driver_attach+0x58/0x60)
> [<c0668e30>] (device_driver_attach) from [<c0668eb8>] (__driver_attach+0x80/0xc8)
> [<c0668eb8>] (__driver_attach) from [<c0666c48>] (bus_for_each_dev+0x78/0xb8)
> [<c0666c48>] (bus_for_each_dev) from [<c0667c44>] (bus_add_driver+0x164/0x1e8)
> [<c0667c44>] (bus_add_driver) from [<c066997c>] (driver_register+0x7c/0x114)
> [<c066997c>] (driver_register) from [<c010223c>] (do_one_initcall+0x50/0x2b0)
> [<c010223c>] (do_one_initcall) from [<c11011f0>] (kernel_init_freeable+0x1a8/0x1fc)
> [<c11011f0>] (kernel_init_freeable) from [<c0c09190>] (kernel_init+0x8/0x118)
> [<c0c09190>] (kernel_init) from [<c01001b0>] (ret_from_fork+0x14/0x24)
> ...
> ---[ end trace 0f652dd222de75d7 ]---
>
> In the function mtd_otp_size() a buffer is allocated by calling
> kmalloc() and a pointer to the buffer is stored in a variable 'info'.
> The pointer 'info' may then be incremented depending on the length
> returned from mtd_get_user/fact_prot_info(). If 'info' is incremented,
> when kfree() is called to free the buffer the above panic occurs because
> we are no longer passing the original address of the buffer allocated.
> Fix this by indexing through the buffer allocated to avoid incrementing
> the pointer.
>
> Fixes: 4b361cfa8624 ("mtd: core: add OTP nvmem provider support")
> Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
> Reviewed-by: Michael Walle <michael@walle.cc>
Applied to https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git mtd/next, thanks.
Miquel
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH] mtd: core: Fix freeing of otp_info buffer
@ 2021-05-26 9:03 ` Miquel Raynal
0 siblings, 0 replies; 44+ messages in thread
From: Miquel Raynal @ 2021-05-26 9:03 UTC (permalink / raw)
To: Jon Hunter, Miquel Raynal, Richard Weinberger,
Vignesh Raghavendra, Michael Walle
Cc: linux-mtd, linux-kernel, linux-tegra
On Tue, 2021-05-18 at 18:55:03 UTC, Jon Hunter wrote:
> Commit 4b361cfa8624 ("mtd: core: add OTP nvmem provider support") is
> causing the following panic ...
>
> ------------[ cut here ]------------
> kernel BUG at /local/workdir/tegra/linux_next/kernel/mm/slab.c:2730!
> Internal error: Oops - BUG: 0 [#1] PREEMPT SMP ARM
> Modules linked in:
> CPU: 3 PID: 1 Comm: swapper/0 Not tainted 5.13.0-rc2-next-20210518 #1
> Hardware name: NVIDIA Tegra SoC (Flattened Device Tree)
> PC is at ___cache_free+0x3f8/0x51c
> ...
> [<c029bb1c>] (___cache_free) from [<c029c658>] (kfree+0xac/0x1bc)
> [<c029c658>] (kfree) from [<c06da094>] (mtd_otp_size+0xc4/0x108)
> [<c06da094>] (mtd_otp_size) from [<c06dc864>] (mtd_device_parse_register+0xe4/0x2b4)
> [<c06dc864>] (mtd_device_parse_register) from [<c06e3ccc>] (spi_nor_probe+0x210/0x2c0)
> [<c06e3ccc>] (spi_nor_probe) from [<c06e9578>] (spi_probe+0x88/0xac)
> [<c06e9578>] (spi_probe) from [<c066891c>] (really_probe+0x214/0x3a4)
> [<c066891c>] (really_probe) from [<c0668b14>] (driver_probe_device+0x68/0xc0)
> [<c0668b14>] (driver_probe_device) from [<c0666cf8>] (bus_for_each_drv+0x5c/0xbc)
> [<c0666cf8>] (bus_for_each_drv) from [<c0668694>] (__device_attach+0xe4/0x150)
> [<c0668694>] (__device_attach) from [<c06679e0>] (bus_probe_device+0x84/0x8c)
> [<c06679e0>] (bus_probe_device) from [<c06657f8>] (device_add+0x48c/0x868)
> [<c06657f8>] (device_add) from [<c06eb784>] (spi_add_device+0xa0/0x168)
> [<c06eb784>] (spi_add_device) from [<c06ec9a8>] (spi_register_controller+0x8b8/0xb38)
> [<c06ec9a8>] (spi_register_controller) from [<c06ecc3c>] (devm_spi_register_controller+0x14/0x50)
> [<c06ecc3c>] (devm_spi_register_controller) from [<c06f0510>] (tegra_spi_probe+0x33c/0x450)
> [<c06f0510>] (tegra_spi_probe) from [<c066abec>] (platform_probe+0x5c/0xb8)
> [<c066abec>] (platform_probe) from [<c066891c>] (really_probe+0x214/0x3a4)
> [<c066891c>] (really_probe) from [<c0668b14>] (driver_probe_device+0x68/0xc0)
> [<c0668b14>] (driver_probe_device) from [<c0668e30>] (device_driver_attach+0x58/0x60)
> [<c0668e30>] (device_driver_attach) from [<c0668eb8>] (__driver_attach+0x80/0xc8)
> [<c0668eb8>] (__driver_attach) from [<c0666c48>] (bus_for_each_dev+0x78/0xb8)
> [<c0666c48>] (bus_for_each_dev) from [<c0667c44>] (bus_add_driver+0x164/0x1e8)
> [<c0667c44>] (bus_add_driver) from [<c066997c>] (driver_register+0x7c/0x114)
> [<c066997c>] (driver_register) from [<c010223c>] (do_one_initcall+0x50/0x2b0)
> [<c010223c>] (do_one_initcall) from [<c11011f0>] (kernel_init_freeable+0x1a8/0x1fc)
> [<c11011f0>] (kernel_init_freeable) from [<c0c09190>] (kernel_init+0x8/0x118)
> [<c0c09190>] (kernel_init) from [<c01001b0>] (ret_from_fork+0x14/0x24)
> ...
> ---[ end trace 0f652dd222de75d7 ]---
>
> In the function mtd_otp_size() a buffer is allocated by calling
> kmalloc() and a pointer to the buffer is stored in a variable 'info'.
> The pointer 'info' may then be incremented depending on the length
> returned from mtd_get_user/fact_prot_info(). If 'info' is incremented,
> when kfree() is called to free the buffer the above panic occurs because
> we are no longer passing the original address of the buffer allocated.
> Fix this by indexing through the buffer allocated to avoid incrementing
> the pointer.
>
> Fixes: 4b361cfa8624 ("mtd: core: add OTP nvmem provider support")
> Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
> Reviewed-by: Michael Walle <michael@walle.cc>
Applied to https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git mtd/next, thanks.
Miquel
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH v2 5/5] mtd: core: add OTP nvmem provider support
2021-04-24 11:06 ` Michael Walle
@ 2021-07-01 21:34 ` Guenter Roeck
-1 siblings, 0 replies; 44+ messages in thread
From: Guenter Roeck @ 2021-07-01 21:34 UTC (permalink / raw)
To: Michael Walle
Cc: linux-mtd, devicetree, linux-kernel, Miquel Raynal,
Richard Weinberger, Vignesh Raghavendra, Rob Herring,
Srinivas Kandagatla
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.
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH v2 5/5] mtd: core: add OTP nvmem provider support
@ 2021-07-01 21:34 ` Guenter Roeck
0 siblings, 0 replies; 44+ messages in thread
From: Guenter Roeck @ 2021-07-01 21:34 UTC (permalink / raw)
To: Michael Walle
Cc: linux-mtd, devicetree, linux-kernel, Miquel Raynal,
Richard Weinberger, Vignesh Raghavendra, Rob Herring,
Srinivas Kandagatla
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.
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH v2 5/5] mtd: core: add OTP nvmem provider support
2021-07-01 21:34 ` Guenter Roeck
@ 2021-07-01 22:10 ` Michael Walle
-1 siblings, 0 replies; 44+ messages in thread
From: Michael Walle @ 2021-07-01 22:10 UTC (permalink / raw)
To: Guenter Roeck
Cc: linux-mtd, devicetree, linux-kernel, Miquel Raynal,
Richard Weinberger, Vignesh Raghavendra, Rob Herring,
Srinivas Kandagatla
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
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH v2 5/5] mtd: core: add OTP nvmem provider support
@ 2021-07-01 22:10 ` Michael Walle
0 siblings, 0 replies; 44+ messages in thread
From: Michael Walle @ 2021-07-01 22:10 UTC (permalink / raw)
To: Guenter Roeck
Cc: linux-mtd, devicetree, linux-kernel, Miquel Raynal,
Richard Weinberger, Vignesh Raghavendra, Rob Herring,
Srinivas Kandagatla
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
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH v2 5/5] mtd: core: add OTP nvmem provider support
2021-07-01 22:10 ` Michael Walle
@ 2021-07-02 1:55 ` Guenter Roeck
-1 siblings, 0 replies; 44+ messages in thread
From: Guenter Roeck @ 2021-07-02 1:55 UTC (permalink / raw)
To: Michael Walle
Cc: linux-mtd, devicetree, linux-kernel, Miquel Raynal,
Richard Weinberger, Vignesh Raghavendra, Rob Herring,
Srinivas Kandagatla
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
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH v2 5/5] mtd: core: add OTP nvmem provider support
@ 2021-07-02 1:55 ` Guenter Roeck
0 siblings, 0 replies; 44+ messages in thread
From: Guenter Roeck @ 2021-07-02 1:55 UTC (permalink / raw)
To: Michael Walle
Cc: linux-mtd, devicetree, linux-kernel, Miquel Raynal,
Richard Weinberger, Vignesh Raghavendra, Rob Herring,
Srinivas Kandagatla
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
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH v2 5/5] mtd: core: add OTP nvmem provider support
2021-07-02 1:55 ` Guenter Roeck
@ 2021-07-02 9:33 ` Michael Walle
-1 siblings, 0 replies; 44+ messages in thread
From: Michael Walle @ 2021-07-02 9:33 UTC (permalink / raw)
To: Guenter Roeck
Cc: linux-mtd, devicetree, linux-kernel, Miquel Raynal,
Richard Weinberger, Vignesh Raghavendra, Rob Herring,
Srinivas Kandagatla
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
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH v2 5/5] mtd: core: add OTP nvmem provider support
@ 2021-07-02 9:33 ` Michael Walle
0 siblings, 0 replies; 44+ messages in thread
From: Michael Walle @ 2021-07-02 9:33 UTC (permalink / raw)
To: Guenter Roeck
Cc: linux-mtd, devicetree, linux-kernel, Miquel Raynal,
Richard Weinberger, Vignesh Raghavendra, Rob Herring,
Srinivas Kandagatla
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
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 44+ messages in thread