[PATCH v4] mtd: limit OTP NVMEM Cell parse to non Nand devices

[PATCH v4] mtd: limit OTP NVMEM Cell parse to non Nand devices

[Christian Marangi]

MTD OTP logic is very fragile on parsing NVMEM Cell and can be
problematic with some specific kind of devices.

The problem was discovered by e87161321a40 ("mtd: rawnand: macronix:
OTP access for MX30LFxG18AC") where OTP support was added to a NAND
device. With the case of NAND devices, it does require a node where ECC
info are declared and all the fixed partitions, and this cause the OTP
codepath to parse this node as OTP NVMEM Cells, making probe fail and
the NAND device registration fail.

MTD OTP parsing should have been limited to always using compatible to
prevent this error by using node with compatible "otp-user" or
"otp-factory".

NVMEM across the years had various iteration on how Cells could be
declared in DT, in some old implementation, no_of_node should have been
enabled but now add_legacy_fixed_of_cells should be used to disable
NVMEM to parse child node as NVMEM Cell.

To fix this and limit any regression with other MTD that makes use of
declaring OTP as direct child of the dev node, disable
add_legacy_fixed_of_cells if we detect the MTD type is Nand.

With the following logic, the OTP NVMEM entry is correctly created with
no Cells and the MTD Nand is correctly probed and partitions are
correctly exposed.

Fixes: 4b361cfa8624 ("mtd: core: add OTP nvmem provider support")
Cc: <stable@vger.kernel.org> # v6.7+
Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
---

To backport this to v6.6 and previous,

config.no_of_node = mtd_type_is_nand(mtd);

should be used as it does pose the same usage of
add_legacy_fixed_of_cells.

Changes v4:
- Add info on how to backport this to previous kernel
- Fix Fixes tag
- Reformat commit description as it was unprecise and
  had false statement
Changes v3:
- Fix commit description
Changes v2:
- Use mtd_type_is_nand instead of node name check

 drivers/mtd/mtdcore.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c
index 5887feb347a4..0de87bc63840 100644
--- a/drivers/mtd/mtdcore.c
+++ b/drivers/mtd/mtdcore.c
@@ -900,7 +900,7 @@ static struct nvmem_device *mtd_otp_nvmem_register(struct mtd_info *mtd,
 	config.name = compatible;
 	config.id = NVMEM_DEVID_AUTO;
 	config.owner = THIS_MODULE;
-	config.add_legacy_fixed_of_cells = true;
+	config.add_legacy_fixed_of_cells = !mtd_type_is_nand(mtd);
 	config.type = NVMEM_TYPE_OTP;
 	config.root_only = true;
 	config.ignore_wp = true;
-- 
2.43.0

Re: [PATCH v4] mtd: limit OTP NVMEM Cell parse to non Nand devices

[Miquel Raynal]

Hi Christian,

ansuelsmth@gmail.com wrote on Tue,  2 Apr 2024 23:23:19 +0200:

> MTD OTP logic is very fragile on parsing NVMEM Cell and can be
> problematic with some specific kind of devices.
> 
> The problem was discovered by e87161321a40 ("mtd: rawnand: macronix:
> OTP access for MX30LFxG18AC") where OTP support was added to a NAND
> device. With the case of NAND devices, it does require a node where ECC
> info are declared and all the fixed partitions, and this cause the OTP
> codepath to parse this node as OTP NVMEM Cells, making probe fail and
> the NAND device registration fail.
> 
> MTD OTP parsing should have been limited to always using compatible to
> prevent this error by using node with compatible "otp-user" or
> "otp-factory".
> 
> NVMEM across the years had various iteration on how Cells could be
> declared in DT, in some old implementation, no_of_node should have been
> enabled but now add_legacy_fixed_of_cells should be used to disable
> NVMEM to parse child node as NVMEM Cell.
> 
> To fix this and limit any regression with other MTD that makes use of
> declaring OTP as direct child of the dev node, disable
> add_legacy_fixed_of_cells if we detect the MTD type is Nand.
> 
> With the following logic, the OTP NVMEM entry is correctly created with
> no Cells and the MTD Nand is correctly probed and partitions are
> correctly exposed.
> 
> Fixes: 4b361cfa8624 ("mtd: core: add OTP nvmem provider support")
> Cc: <stable@vger.kernel.org> # v6.7+
> Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>

Feels okay to me, but I'd like to get validation from Rafał as well who
extensively worked on this aspect and must have a sharpened eyed for
this kind of issue :-)

> ---
> 
> To backport this to v6.6 and previous,
> 
> config.no_of_node = mtd_type_is_nand(mtd);
> 
> should be used as it does pose the same usage of
> add_legacy_fixed_of_cells.
> 
> Changes v4:
> - Add info on how to backport this to previous kernel
> - Fix Fixes tag
> - Reformat commit description as it was unprecise and
>   had false statement
> Changes v3:
> - Fix commit description
> Changes v2:
> - Use mtd_type_is_nand instead of node name check
> 
>  drivers/mtd/mtdcore.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c
> index 5887feb347a4..0de87bc63840 100644
> --- a/drivers/mtd/mtdcore.c
> +++ b/drivers/mtd/mtdcore.c
> @@ -900,7 +900,7 @@ static struct nvmem_device
> *mtd_otp_nvmem_register(struct mtd_info *mtd, config.name =
> compatible; config.id = NVMEM_DEVID_AUTO;
>  	config.owner = THIS_MODULE;
> -	config.add_legacy_fixed_of_cells = true;
> +	config.add_legacy_fixed_of_cells = !mtd_type_is_nand(mtd);
>  	config.type = NVMEM_TYPE_OTP;
>  	config.root_only = true;
>  	config.ignore_wp = true;


Thanks,
Miquèl

Re: [PATCH v4] mtd: limit OTP NVMEM Cell parse to non Nand devices

[Michael Walle]

[-- Attachment #1: Type: text/plain, Size: 270 bytes --]

Hi,

just a quick (non-technical) nitpick if you do a new version or
maybe Miquel will fix it during applying:

> Subject: [PATCH v4] mtd: limit OTP NVMEM Cell parse to non Nand devices

subject should be "non-NAND", also cell could be lower case :)

-michael

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 297 bytes --]

Re: [PATCH v4] mtd: limit OTP NVMEM Cell parse to non Nand devices

[Miquel Raynal]

Hi Michael,

michael@walle.cc wrote on Tue, 09 Apr 2024 11:41:06 +0200:

> Hi,
> 
> just a quick (non-technical) nitpick if you do a new version or
> maybe Miquel will fix it during applying:
> 
> > Subject: [PATCH v4] mtd: limit OTP NVMEM Cell parse to non Nand devices  
> 
> subject should be "non-NAND", also cell could be lower case :)

Yes I can. I was waiting for some feedback from Rafał but he must be
busy. I'll fix this. We need to move forward and send the fixes PR now.

Thanks,
Miquèl