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
--
2.20.1
SPI-NOR flashes may have OTP regions and have a nvmem binding. This
binding is described in mtd.yaml.
Signed-off-by: Michael Walle <[email protected]>
---
Changes since v1:
- none
Changes since RFC:
- new patch
Documentation/devicetree/bindings/mtd/jedec,spi-nor.yaml | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/Documentation/devicetree/bindings/mtd/jedec,spi-nor.yaml b/Documentation/devicetree/bindings/mtd/jedec,spi-nor.yaml
index 5e7e5349f9a1..ed590d7c6e37 100644
--- a/Documentation/devicetree/bindings/mtd/jedec,spi-nor.yaml
+++ b/Documentation/devicetree/bindings/mtd/jedec,spi-nor.yaml
@@ -9,6 +9,9 @@ title: SPI NOR flash ST M25Pxx (and similar) serial flash chips
maintainers:
- Rob Herring <[email protected]>
+allOf:
+ - $ref: "mtd.yaml#"
+
properties:
compatible:
oneOf:
@@ -82,6 +85,9 @@ patternProperties:
'^partition@':
type: object
+ "^otp(-[0-9]+)?$":
+ type: object
+
additionalProperties: false
examples:
--
2.20.1
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 <[email protected]>
---
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
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 <[email protected]>
> ---
> 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 <[email protected]>
On Sat, 2021-04-24 at 11:06:07 UTC, 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 <[email protected]>
> Reviewed-by: Rob Herring <[email protected]>
Applied to https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git mtd/next, thanks.
Miquel
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 <[email protected]>
Applied to https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git mtd/next, thanks.
Miquel
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 <[email protected]>
---
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
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 <[email protected]>
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 <[email protected]>
Thanks,
-michael
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 <[email protected]>
> Reviewed-by: Michael Walle <[email protected]>
Applied to https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git mtd/next, thanks.
Miquel
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 <[email protected]>
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 <[email protected]>
>
> 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 <[email protected]>
>>
>> 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 <[email protected]>
>>>
>>> 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