Hello,
this patchset does several things:
1) It fixes unstable behaviour of Meson driver, for example random ECC
errors during reads. It is done by changing 'meson_nfc_queue_rb()'
implementation. Sequence of commands inside this function is updated.
This patch is suggested by Liang Yang <[email protected]>.
Here is link to discussion:
https://lore.kernel.org/linux-mtd/[email protected]/T/#ma8097bad0228f81a3d11a14389cdec44f45b86c8
2) It moves OOB free bytes to non-protected by ECC area. Here are some
details:
Current OOB free bytes are 4 bytes (2 x 2 user bytes) under ECC engine.
Here is how it looks like in the current implementation:
[ 2B user bytes ][ 14B ECC codes ]
[ 2B user bytes ][ 14B ECC codes ]
[ 16B unused area, not protected by ECC ]
[ 16B unused area, not protected by ECC ]
All 4 user bytes are protected by ECC. This patch changes OOB free
bytes in this way:
[ 2B unused area ][ 14B ECC codes ]
[ 2B unused area ][ 14B ECC codes ]
[ 16B user bytes, not protected by ECC ]
[ 16B user bytes, not protected by ECC ]
Now OOB user bytes are 32 bytes instead of 4 bytes and not protected
by ECC.
Motivation of this layout comes from problem with JFFS2. It uses OOB
free bytes for cleanmarkers. Each cleanmarker is 4 bytes and written
by JFFS2 driver (small remark - cleanmarkers are always written in
case of NAND storage for JFFS2).
We have two ways to write this data to OOB (e.g. user bytes):
1) ECC mode. In this case it will be ECC covered user bytes, e.g.
writing this bytes will update ECC codes. Problem fires, when
JFFS2 tries to write this page later - this write makes controller
to update ECC codes again, but it is impossible to do it correctly,
because we can't update bits from 0 to 1 (only from 1 to 0).
2) Raw mode. In this case ECC codes won't be updated. But later, it
will be impossible to read this page in ECC mode, because we have
some user bytes, but ECC codes are missed.
So let's move OOB free bytes out of ECC area. In this case we can
read/write OOB separately in raw mode and at the same time work with
data in ECC mode. JFFS2 is happy now. User bytes are untouched - all
of them are ignored during non-OOB access.
I've tested this with mount/unmount/read/write cases for JFFS2 and
nanddump/nandwrite utlities on AXG family (A113X SoC).
Here is link to discussion:
https://lore.kernel.org/linux-mtd/[email protected]/T/#m3087bd06386a7f430cd5e343e22b25d724d3e2d7
3) Changes size of OOB read access - now, in both ECC and raw modes whole
OOB (e.g. 64 bytes) is returned to the caller.
4) Checks buffer length on accesses to NAND controller.
5) Removes useless bitwise OR with zeroes.
6) Renames device tree node's name for chip selection from "reg" to
"cs". See commit message.
Link to v1:
https://lore.kernel.org/linux-mtd/[email protected]/
Link to v2:
https://lore.kernel.org/linux-mtd/[email protected]/
Arseniy Krasnov (6):
mtd: rawnand: meson: fix command sequence for read/write
mtd: rawnand: meson: move OOB to non-protected ECC area
mtd: rawnand: meson: always read whole OOB bytes
mtd: rawnand: meson: check buffer length
mtd: rawnand: meson: remove unneeded bitwise OR with zeroes
mtd: rawnand: meson: rename node for chip select
drivers/mtd/nand/raw/meson_nand.c | 274 ++++++++++++++++++++++++------
1 file changed, 222 insertions(+), 52 deletions(-)
--
2.35.0
This renames node with values for chip select from "reg" to "cs". It is
needed because when OTP access is enabled on the attached storage, MTD
subsystem registers this storage in the NVMEM subsystem. NVMEM in turn
tries to use "reg" node in its own manner, supposes that it has another
layout. All of this leads to device initialization failure.
Example:
[...] nvmem mtd0-user-otp: nvmem: invalid reg on /soc/bus@ffe00000/...
[...] mtd mtd0: Failed to register OTP NVMEM device
[...] meson-nand ffe07800.nfc: failed to register MTD device: -22
[...] meson-nand ffe07800.nfc: failed to init NAND chips
[...] meson-nand: probe of ffe07800.nfc failed with error -22
Signed-off-by: Arseniy Krasnov <[email protected]>
---
drivers/mtd/nand/raw/meson_nand.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/mtd/nand/raw/meson_nand.c b/drivers/mtd/nand/raw/meson_nand.c
index bc99a098f3ca..28371919a6c5 100644
--- a/drivers/mtd/nand/raw/meson_nand.c
+++ b/drivers/mtd/nand/raw/meson_nand.c
@@ -1419,7 +1419,7 @@ meson_nfc_nand_chip_init(struct device *dev,
int ret, i;
u32 tmp, nsels;
- nsels = of_property_count_elems_of_size(np, "reg", sizeof(u32));
+ nsels = of_property_count_elems_of_size(np, "cs", sizeof(u32));
if (!nsels || nsels > MAX_CE_NUM) {
dev_err(dev, "invalid register property size\n");
return -EINVAL;
@@ -1433,7 +1433,7 @@ meson_nfc_nand_chip_init(struct device *dev,
meson_chip->nsels = nsels;
for (i = 0; i < nsels; i++) {
- ret = of_property_read_u32_index(np, "reg", i, &tmp);
+ ret = of_property_read_u32_index(np, "cs", i, &tmp);
if (ret) {
dev_err(dev, "could not retrieve register property: %d\n",
ret);
--
2.35.0
This changes size of read access to OOB area by reading all bytes of
OOB (free bytes + ECC engine bytes).
Signed-off-by: Arseniy Krasnov <[email protected]>
---
drivers/mtd/nand/raw/meson_nand.c | 24 ++++++++++++++++++++++++
1 file changed, 24 insertions(+)
diff --git a/drivers/mtd/nand/raw/meson_nand.c b/drivers/mtd/nand/raw/meson_nand.c
index d7c81408cfa2..331377a2c5dc 100644
--- a/drivers/mtd/nand/raw/meson_nand.c
+++ b/drivers/mtd/nand/raw/meson_nand.c
@@ -755,6 +755,30 @@ static int __meson_nfc_read_oob(struct nand_chip *nand, int page,
u32 oob_bytes;
u32 page_size;
int ret;
+ int i;
+
+ /* Read ECC codes and user bytes. */
+ for (i = 0; i < nand->ecc.steps; i++) {
+ u32 ecc_offs = nand->ecc.size * (i + 1) +
+ NFC_OOB_PER_ECC(nand) * i;
+
+ ret = nand_read_page_op(nand, page, 0, NULL, 0);
+ if (ret)
+ return ret;
+
+ /* Use temporary buffer, because 'nand_change_read_column_op()'
+ * seems work with some alignment, so we can't read data to
+ * 'oob_buf' directly.
+ */
+ ret = nand_change_read_column_op(nand, ecc_offs, meson_chip->oob_buf,
+ NFC_OOB_PER_ECC(nand), false);
+ if (ret)
+ return ret;
+
+ memcpy(oob_buf + i * NFC_OOB_PER_ECC(nand),
+ meson_chip->oob_buf,
+ NFC_OOB_PER_ECC(nand));
+ }
oob_bytes = meson_nfc_get_oob_bytes(nand);
--
2.35.0
Hello Arseniy,
On Wed, May 10, 2023 at 1:13 PM Arseniy Krasnov
<[email protected]> wrote:
>
> This renames node with values for chip select from "reg" to "cs". It is
> needed because when OTP access is enabled on the attached storage, MTD
> subsystem registers this storage in the NVMEM subsystem. NVMEM in turn
> tries to use "reg" node in its own manner, supposes that it has another
> layout. All of this leads to device initialization failure.
In general: if we change the device-tree interface (in this case:
replacing a "reg" with a "cs" property) the dt-bindings have to be
updated as well.
Documentation/devicetree/bindings/mtd/nand-controller.yaml and
Documentation/devicetree/bindings/mtd/amlogic,meson-nand.yaml show
that the chip select of a NAND chip is specified with a "reg"
property.
Also the code has to be backwards compatible with old .dtbs.
> Example:
>
> [...] nvmem mtd0-user-otp: nvmem: invalid reg on /soc/bus@ffe00000/...
> [...] mtd mtd0: Failed to register OTP NVMEM device
> [...] meson-nand ffe07800.nfc: failed to register MTD device: -22
> [...] meson-nand ffe07800.nfc: failed to init NAND chips
> [...] meson-nand: probe of ffe07800.nfc failed with error -22
This is odd - can you please share your definition of the &nfc node?
&nfc {
nand_chip0: nand@0 {
reg = <0>;
};
};
This should result in nand_set_flash_node() being called with
&nand_chip0 (if it's called with &nfc then something is buggy in our
driver).
If there's no child nodes within &nand_chip0 then why would the
MTD-to-NVMEM code think that it has to parse something?
If you do have child nodes and those are partitions, then make sure
that the structure is correct (see the extra "partitions" node inside
which all partitions are nested):
&nand_chip0 {
partitions {
compatible = "fixed-partitions";
#address-cells = <1>;
#size-cells = <1>;
partition@0 {
label = "u-boot";
reg = <0x0000000 0x4000>;
read-only;
};
};
};
Best regards,,
Martin
Hi Martin & Arseniy,
[email protected] wrote on Wed, 10 May 2023 22:40:37
+0200:
> Hello Arseniy,
>
> On Wed, May 10, 2023 at 1:13 PM Arseniy Krasnov
> <[email protected]> wrote:
> >
> > This renames node with values for chip select from "reg" to "cs". It is
> > needed because when OTP access is enabled on the attached storage, MTD
> > subsystem registers this storage in the NVMEM subsystem. NVMEM in turn
> > tries to use "reg" node in its own manner, supposes that it has another
> > layout. All of this leads to device initialization failure.
> In general: if we change the device-tree interface (in this case:
> replacing a "reg" with a "cs" property) the dt-bindings have to be
> updated as well.
True, and I would add, bindings should not be broken.
> Documentation/devicetree/bindings/mtd/nand-controller.yaml and
> Documentation/devicetree/bindings/mtd/amlogic,meson-nand.yaml show
> that the chip select of a NAND chip is specified with a "reg"
> property.
All NAND controller binding expect the chip-select to be in the
'reg' property, very much like a spi device would use reg to store the
cs as well: the reg property tells you how you address the device.
I also fully agree with Martin's comments below. Changing reg is likely
a wrong approach :)
> Also the code has to be backwards compatible with old .dtbs.
>
> > Example:
> >
> > [...] nvmem mtd0-user-otp: nvmem: invalid reg on /soc/bus@ffe00000/...
> > [...] mtd mtd0: Failed to register OTP NVMEM device
> > [...] meson-nand ffe07800.nfc: failed to register MTD device: -22
> > [...] meson-nand ffe07800.nfc: failed to init NAND chips
> > [...] meson-nand: probe of ffe07800.nfc failed with error -22
> This is odd - can you please share your definition of the &nfc node?
>
> &nfc {
> nand_chip0: nand@0 {
> reg = <0>;
> };
> };
>
> This should result in nand_set_flash_node() being called with
> &nand_chip0 (if it's called with &nfc then something is buggy in our
> driver).
> If there's no child nodes within &nand_chip0 then why would the
> MTD-to-NVMEM code think that it has to parse something?
> If you do have child nodes and those are partitions, then make sure
> that the structure is correct (see the extra "partitions" node inside
> which all partitions are nested):
> &nand_chip0 {
> partitions {
> compatible = "fixed-partitions";
> #address-cells = <1>;
> #size-cells = <1>;
>
> partition@0 {
> label = "u-boot";
> reg = <0x0000000 0x4000>;
> read-only;
> };
> };
> };
>
>
> Best regards,,
> Martin
Thanks,
Miquèl
On 10.05.2023 23:53, Miquel Raynal wrote:
Hello Martin, Miquel
> Hi Martin & Arseniy,
>
> [email protected] wrote on Wed, 10 May 2023 22:40:37
> +0200:
>
>> Hello Arseniy,
>>
>> On Wed, May 10, 2023 at 1:13 PM Arseniy Krasnov
>> <[email protected]> wrote:
>>>
>>> This renames node with values for chip select from "reg" to "cs". It is
>>> needed because when OTP access is enabled on the attached storage, MTD
>>> subsystem registers this storage in the NVMEM subsystem. NVMEM in turn
>>> tries to use "reg" node in its own manner, supposes that it has another
>>> layout. All of this leads to device initialization failure.
>> In general: if we change the device-tree interface (in this case:
>> replacing a "reg" with a "cs" property) the dt-bindings have to be
>> updated as well.
>
> True, and I would add, bindings should not be broken.
I see, that's true. That is bad way to change bindings.
>
>> Documentation/devicetree/bindings/mtd/nand-controller.yaml and
>> Documentation/devicetree/bindings/mtd/amlogic,meson-nand.yaml show
>> that the chip select of a NAND chip is specified with a "reg"
>> property.
>
> All NAND controller binding expect the chip-select to be in the
> 'reg' property, very much like a spi device would use reg to store the
> cs as well: the reg property tells you how you address the device.
>
> I also fully agree with Martin's comments below. Changing reg is likely
> a wrong approach :)
>
>> Also the code has to be backwards compatible with old .dtbs.
>>
>>> Example:
>>>
>>> [...] nvmem mtd0-user-otp: nvmem: invalid reg on /soc/bus@ffe00000/...
>>> [...] mtd mtd0: Failed to register OTP NVMEM device
>>> [...] meson-nand ffe07800.nfc: failed to register MTD device: -22
>>> [...] meson-nand ffe07800.nfc: failed to init NAND chips
>>> [...] meson-nand: probe of ffe07800.nfc failed with error -22
>> This is odd - can you please share your definition of the &nfc node?
Sure, here it is:
mtd_nand: nfc@7800 {
compatible = "amlogic,meson-axg-nfc";
...
nand@0 {
reg = <0>;
};
}
I checked, that 'nand_set_flash_node()' is called with 'nand@0' and i suppose
that it is correct (as You mentioned below). But, 'nvmem_add_cells_from_of()' is called
with parent: 'nfc@7800', then it iterates over its childs, e.g. 'nand@0' and thus i get such
situation. I guess, that 'nvmem_add_cells_from_of()' must be called with 'nand@0' ?
>>
>> &nfc {
>> nand_chip0: nand@0 {
>> reg = <0>;
>> };
>> };
>>
>> This should result in nand_set_flash_node() being called with
>> &nand_chip0 (if it's called with &nfc then something is buggy in our
>> driver).
>> If there's no child nodes within &nand_chip0 then why would the
>> MTD-to-NVMEM code think that it has to parse something?
>> If you do have child nodes and those are partitions, then make sure
>> that the structure is correct (see the extra "partitions" node inside
>> which all partitions are nested):
>> &nand_chip0 {
>> partitions {
>> compatible = "fixed-partitions";
>> #address-cells = <1>;
>> #size-cells = <1>;
>>
>> partition@0 {
>> label = "u-boot";
>> reg = <0x0000000 0x4000>;
>> read-only;
>> };
>> };
>> };
No, partition nodes are disabled in this case.
Thanks, Arseniy
>>
>>
>> Best regards,,
>> Martin
>
>
> Thanks,
> Miquèl
Hi Arseniy,
[email protected] wrote on Thu, 11 May 2023 11:59:07 +0300:
> On 10.05.2023 23:53, Miquel Raynal wrote:
>
> Hello Martin, Miquel
>
> > Hi Martin & Arseniy,
> >
> > [email protected] wrote on Wed, 10 May 2023 22:40:37
> > +0200:
> >
> >> Hello Arseniy,
> >>
> >> On Wed, May 10, 2023 at 1:13 PM Arseniy Krasnov
> >> <[email protected]> wrote:
> >>>
> >>> This renames node with values for chip select from "reg" to "cs". It is
> >>> needed because when OTP access is enabled on the attached storage, MTD
> >>> subsystem registers this storage in the NVMEM subsystem. NVMEM in turn
> >>> tries to use "reg" node in its own manner, supposes that it has another
> >>> layout. All of this leads to device initialization failure.
> >> In general: if we change the device-tree interface (in this case:
> >> replacing a "reg" with a "cs" property) the dt-bindings have to be
> >> updated as well.
> >
> > True, and I would add, bindings should not be broken.
>
> I see, that's true. That is bad way to change bindings.
>
> >
> >> Documentation/devicetree/bindings/mtd/nand-controller.yaml and
> >> Documentation/devicetree/bindings/mtd/amlogic,meson-nand.yaml show
> >> that the chip select of a NAND chip is specified with a "reg"
> >> property.
> >
> > All NAND controller binding expect the chip-select to be in the
> > 'reg' property, very much like a spi device would use reg to store the
> > cs as well: the reg property tells you how you address the device.
> >
> > I also fully agree with Martin's comments below. Changing reg is likely
> > a wrong approach :)
> >
> >> Also the code has to be backwards compatible with old .dtbs.
> >>
> >>> Example:
> >>>
> >>> [...] nvmem mtd0-user-otp: nvmem: invalid reg on /soc/bus@ffe00000/...
> >>> [...] mtd mtd0: Failed to register OTP NVMEM device
> >>> [...] meson-nand ffe07800.nfc: failed to register MTD device: -22
> >>> [...] meson-nand ffe07800.nfc: failed to init NAND chips
> >>> [...] meson-nand: probe of ffe07800.nfc failed with error -22
> >> This is odd - can you please share your definition of the &nfc node?
>
> Sure, here it is:
>
> mtd_nand: nfc@7800 {
> compatible = "amlogic,meson-axg-nfc";
> ...
> nand@0 {
> reg = <0>;
> };
> }
>
> I checked, that 'nand_set_flash_node()' is called with 'nand@0' and i suppose
> that it is correct (as You mentioned below). But, 'nvmem_add_cells_from_of()' is called
> with parent: 'nfc@7800', then it iterates over its childs, e.g. 'nand@0' and thus i get such
> situation. I guess, that 'nvmem_add_cells_from_of()' must be called with 'nand@0' ?
We recently had issues with nvmem parsing, but I believe a mainline
kernel should now be perfectly working on this regard. What version of
the Linux kernel are you using?
Thanks,
Miquèl
On 11.05.2023 12:12, Miquel Raynal wrote:
> Hi Arseniy,
>
> [email protected] wrote on Thu, 11 May 2023 11:59:07 +0300:
>
>> On 10.05.2023 23:53, Miquel Raynal wrote:
>>
>> Hello Martin, Miquel
>>
>>> Hi Martin & Arseniy,
>>>
>>> [email protected] wrote on Wed, 10 May 2023 22:40:37
>>> +0200:
>>>
>>>> Hello Arseniy,
>>>>
>>>> On Wed, May 10, 2023 at 1:13 PM Arseniy Krasnov
>>>> <[email protected]> wrote:
>>>>>
>>>>> This renames node with values for chip select from "reg" to "cs". It is
>>>>> needed because when OTP access is enabled on the attached storage, MTD
>>>>> subsystem registers this storage in the NVMEM subsystem. NVMEM in turn
>>>>> tries to use "reg" node in its own manner, supposes that it has another
>>>>> layout. All of this leads to device initialization failure.
>>>> In general: if we change the device-tree interface (in this case:
>>>> replacing a "reg" with a "cs" property) the dt-bindings have to be
>>>> updated as well.
>>>
>>> True, and I would add, bindings should not be broken.
>>
>> I see, that's true. That is bad way to change bindings.
>>
>>>
>>>> Documentation/devicetree/bindings/mtd/nand-controller.yaml and
>>>> Documentation/devicetree/bindings/mtd/amlogic,meson-nand.yaml show
>>>> that the chip select of a NAND chip is specified with a "reg"
>>>> property.
>>>
>>> All NAND controller binding expect the chip-select to be in the
>>> 'reg' property, very much like a spi device would use reg to store the
>>> cs as well: the reg property tells you how you address the device.
>>>
>>> I also fully agree with Martin's comments below. Changing reg is likely
>>> a wrong approach :)
>>>
>>>> Also the code has to be backwards compatible with old .dtbs.
>>>>
>>>>> Example:
>>>>>
>>>>> [...] nvmem mtd0-user-otp: nvmem: invalid reg on /soc/bus@ffe00000/...
>>>>> [...] mtd mtd0: Failed to register OTP NVMEM device
>>>>> [...] meson-nand ffe07800.nfc: failed to register MTD device: -22
>>>>> [...] meson-nand ffe07800.nfc: failed to init NAND chips
>>>>> [...] meson-nand: probe of ffe07800.nfc failed with error -22
>>>> This is odd - can you please share your definition of the &nfc node?
>>
>> Sure, here it is:
>>
>> mtd_nand: nfc@7800 {
>> compatible = "amlogic,meson-axg-nfc";
>> ...
>> nand@0 {
>> reg = <0>;
>> };
>> }
>>
>> I checked, that 'nand_set_flash_node()' is called with 'nand@0' and i suppose
>> that it is correct (as You mentioned below). But, 'nvmem_add_cells_from_of()' is called
>> with parent: 'nfc@7800', then it iterates over its childs, e.g. 'nand@0' and thus i get such
>> situation. I guess, that 'nvmem_add_cells_from_of()' must be called with 'nand@0' ?
>
> We recently had issues with nvmem parsing, but I believe a mainline
> kernel should now be perfectly working on this regard. What version of
> the Linux kernel are you using?
My current version is:
VERSION = 6
PATCHLEVEL = 2
SUBLEVEL = 0
EXTRAVERSION = -rc8
Fix was in drivers/nvmem/* ?
Thanks, Arseniy
>
> Thanks,
> Miquèl
On 11.05.2023 12:17, Arseniy Krasnov wrote:
>
>
> On 11.05.2023 12:12, Miquel Raynal wrote:
>> Hi Arseniy,
>>
>> [email protected] wrote on Thu, 11 May 2023 11:59:07 +0300:
>>
>>> On 10.05.2023 23:53, Miquel Raynal wrote:
>>>
>>> Hello Martin, Miquel
>>>
>>>> Hi Martin & Arseniy,
>>>>
>>>> [email protected] wrote on Wed, 10 May 2023 22:40:37
>>>> +0200:
>>>>
>>>>> Hello Arseniy,
>>>>>
>>>>> On Wed, May 10, 2023 at 1:13 PM Arseniy Krasnov
>>>>> <[email protected]> wrote:
>>>>>>
>>>>>> This renames node with values for chip select from "reg" to "cs". It is
>>>>>> needed because when OTP access is enabled on the attached storage, MTD
>>>>>> subsystem registers this storage in the NVMEM subsystem. NVMEM in turn
>>>>>> tries to use "reg" node in its own manner, supposes that it has another
>>>>>> layout. All of this leads to device initialization failure.
>>>>> In general: if we change the device-tree interface (in this case:
>>>>> replacing a "reg" with a "cs" property) the dt-bindings have to be
>>>>> updated as well.
>>>>
>>>> True, and I would add, bindings should not be broken.
>>>
>>> I see, that's true. That is bad way to change bindings.
>>>
>>>>
>>>>> Documentation/devicetree/bindings/mtd/nand-controller.yaml and
>>>>> Documentation/devicetree/bindings/mtd/amlogic,meson-nand.yaml show
>>>>> that the chip select of a NAND chip is specified with a "reg"
>>>>> property.
>>>>
>>>> All NAND controller binding expect the chip-select to be in the
>>>> 'reg' property, very much like a spi device would use reg to store the
>>>> cs as well: the reg property tells you how you address the device.
>>>>
>>>> I also fully agree with Martin's comments below. Changing reg is likely
>>>> a wrong approach :)
>>>>
>>>>> Also the code has to be backwards compatible with old .dtbs.
>>>>>
>>>>>> Example:
>>>>>>
>>>>>> [...] nvmem mtd0-user-otp: nvmem: invalid reg on /soc/bus@ffe00000/...
>>>>>> [...] mtd mtd0: Failed to register OTP NVMEM device
>>>>>> [...] meson-nand ffe07800.nfc: failed to register MTD device: -22
>>>>>> [...] meson-nand ffe07800.nfc: failed to init NAND chips
>>>>>> [...] meson-nand: probe of ffe07800.nfc failed with error -22
>>>>> This is odd - can you please share your definition of the &nfc node?
>>>
>>> Sure, here it is:
>>>
>>> mtd_nand: nfc@7800 {
>>> compatible = "amlogic,meson-axg-nfc";
>>> ...
>>> nand@0 {
>>> reg = <0>;
>>> };
>>> }
>>>
>>> I checked, that 'nand_set_flash_node()' is called with 'nand@0' and i suppose
>>> that it is correct (as You mentioned below). But, 'nvmem_add_cells_from_of()' is called
>>> with parent: 'nfc@7800', then it iterates over its childs, e.g. 'nand@0' and thus i get such
>>> situation. I guess, that 'nvmem_add_cells_from_of()' must be called with 'nand@0' ?
>>
>> We recently had issues with nvmem parsing, but I believe a mainline
>> kernel should now be perfectly working on this regard. What version of
>> the Linux kernel are you using?
>
> My current version is:
>
> VERSION = 6
> PATCHLEVEL = 2
> SUBLEVEL = 0
> EXTRAVERSION = -rc8
>
> Fix was in drivers/nvmem/* ?
>
> Thanks, Arseniy
Upd: I resolved problem in the following way:
nand@0 {
reg = <0>;//chip select
otp@0 {
#address-cells = <2>;
#size-cells = <0>;
compatible = "user-otp";
reg = <A B>;
};
otp@1 {
#address-cells = <2>;
#size-cells = <0>;
compatible = "factory-otp";
reg = <C D>;
};
};
Now nvmem subsystem parses 'otp@0' and 'otp@1' and error is gone. 'compatible' values are
the same as in drivers/mtd/mtdcore.c:mtd_otp_nvmem_add(). 'reg' in 'nand@0' is used as
chip select as supposed.
I think, this patch should be abandoned in the next version.
Thanks, Arseniy
>
>>
>> Thanks,
>> Miquèl
Hi Arseniy,
[email protected] wrote on Thu, 11 May 2023 13:16:59 +0300:
> On 11.05.2023 12:17, Arseniy Krasnov wrote:
> >
> >
> > On 11.05.2023 12:12, Miquel Raynal wrote:
> >> Hi Arseniy,
> >>
> >> [email protected] wrote on Thu, 11 May 2023 11:59:07 +0300:
> >>
> >>> On 10.05.2023 23:53, Miquel Raynal wrote:
> >>>
> >>> Hello Martin, Miquel
> >>>
> >>>> Hi Martin & Arseniy,
> >>>>
> >>>> [email protected] wrote on Wed, 10 May 2023 22:40:37
> >>>> +0200:
> >>>>
> >>>>> Hello Arseniy,
> >>>>>
> >>>>> On Wed, May 10, 2023 at 1:13 PM Arseniy Krasnov
> >>>>> <[email protected]> wrote:
> >>>>>>
> >>>>>> This renames node with values for chip select from "reg" to "cs". It is
> >>>>>> needed because when OTP access is enabled on the attached storage, MTD
> >>>>>> subsystem registers this storage in the NVMEM subsystem. NVMEM in turn
> >>>>>> tries to use "reg" node in its own manner, supposes that it has another
> >>>>>> layout. All of this leads to device initialization failure.
> >>>>> In general: if we change the device-tree interface (in this case:
> >>>>> replacing a "reg" with a "cs" property) the dt-bindings have to be
> >>>>> updated as well.
> >>>>
> >>>> True, and I would add, bindings should not be broken.
> >>>
> >>> I see, that's true. That is bad way to change bindings.
> >>>
> >>>>
> >>>>> Documentation/devicetree/bindings/mtd/nand-controller.yaml and
> >>>>> Documentation/devicetree/bindings/mtd/amlogic,meson-nand.yaml show
> >>>>> that the chip select of a NAND chip is specified with a "reg"
> >>>>> property.
> >>>>
> >>>> All NAND controller binding expect the chip-select to be in the
> >>>> 'reg' property, very much like a spi device would use reg to store the
> >>>> cs as well: the reg property tells you how you address the device.
> >>>>
> >>>> I also fully agree with Martin's comments below. Changing reg is likely
> >>>> a wrong approach :)
> >>>>
> >>>>> Also the code has to be backwards compatible with old .dtbs.
> >>>>>
> >>>>>> Example:
> >>>>>>
> >>>>>> [...] nvmem mtd0-user-otp: nvmem: invalid reg on /soc/bus@ffe00000/...
> >>>>>> [...] mtd mtd0: Failed to register OTP NVMEM device
> >>>>>> [...] meson-nand ffe07800.nfc: failed to register MTD device: -22
> >>>>>> [...] meson-nand ffe07800.nfc: failed to init NAND chips
> >>>>>> [...] meson-nand: probe of ffe07800.nfc failed with error -22
> >>>>> This is odd - can you please share your definition of the &nfc node?
> >>>
> >>> Sure, here it is:
> >>>
> >>> mtd_nand: nfc@7800 {
> >>> compatible = "amlogic,meson-axg-nfc";
> >>> ...
> >>> nand@0 {
> >>> reg = <0>;
> >>> };
> >>> }
> >>>
> >>> I checked, that 'nand_set_flash_node()' is called with 'nand@0' and i suppose
> >>> that it is correct (as You mentioned below). But, 'nvmem_add_cells_from_of()' is called
> >>> with parent: 'nfc@7800', then it iterates over its childs, e.g. 'nand@0' and thus i get such
> >>> situation. I guess, that 'nvmem_add_cells_from_of()' must be called with 'nand@0' ?
> >>
> >> We recently had issues with nvmem parsing, but I believe a mainline
> >> kernel should now be perfectly working on this regard. What version of
> >> the Linux kernel are you using?
> >
> > My current version is:
> >
> > VERSION = 6
> > PATCHLEVEL = 2
> > SUBLEVEL = 0
> > EXTRAVERSION = -rc8
> >
> > Fix was in drivers/nvmem/* ?
> >
> > Thanks, Arseniy
>
> Upd: I resolved problem in the following way:
>
> nand@0 {
> reg = <0>;//chip select
>
partitions {
compatible = ...
> otp@0 {
> #address-cells = <2>;
> #size-cells = <0>;
#address/size-cells is not needed here
> compatible = "user-otp";
> reg = <A B>;
> };
> otp@1 {
> #address-cells = <2>;
> #size-cells = <0>;
Ditto
> compatible = "factory-otp";
> reg = <C D>;
> };
};
> };
>
> Now nvmem subsystem parses 'otp@0' and 'otp@1' and error is gone. 'compatible' values are
> the same as in drivers/mtd/mtdcore.c:mtd_otp_nvmem_add(). 'reg' in 'nand@0' is used as
> chip select as supposed.
I don't fully get it. The parsing on the nvmem side should not fail if
there is no subpartition/otp-region defined. Can you confirm an empty
NAND device node works? Because your last e-mail suggested the opposite.
>
> I think, this patch should be abandoned in the next version.
>
> Thanks, Arseniy
>
> >
> >>
> >> Thanks,
> >> Miquèl
Thanks,
Miquèl
On 11.05.2023 15:11, Miquel Raynal wrote:
> Hi Arseniy,
>
> [email protected] wrote on Thu, 11 May 2023 13:16:59 +0300:
>
>> On 11.05.2023 12:17, Arseniy Krasnov wrote:
>>>
>>>
>>> On 11.05.2023 12:12, Miquel Raynal wrote:
>>>> Hi Arseniy,
>>>>
>>>> [email protected] wrote on Thu, 11 May 2023 11:59:07 +0300:
>>>>
>>>>> On 10.05.2023 23:53, Miquel Raynal wrote:
>>>>>
>>>>> Hello Martin, Miquel
>>>>>
>>>>>> Hi Martin & Arseniy,
>>>>>>
>>>>>> [email protected] wrote on Wed, 10 May 2023 22:40:37
>>>>>> +0200:
>>>>>>
>>>>>>> Hello Arseniy,
>>>>>>>
>>>>>>> On Wed, May 10, 2023 at 1:13 PM Arseniy Krasnov
>>>>>>> <[email protected]> wrote:
>>>>>>>>
>>>>>>>> This renames node with values for chip select from "reg" to "cs". It is
>>>>>>>> needed because when OTP access is enabled on the attached storage, MTD
>>>>>>>> subsystem registers this storage in the NVMEM subsystem. NVMEM in turn
>>>>>>>> tries to use "reg" node in its own manner, supposes that it has another
>>>>>>>> layout. All of this leads to device initialization failure.
>>>>>>> In general: if we change the device-tree interface (in this case:
>>>>>>> replacing a "reg" with a "cs" property) the dt-bindings have to be
>>>>>>> updated as well.
>>>>>>
>>>>>> True, and I would add, bindings should not be broken.
>>>>>
>>>>> I see, that's true. That is bad way to change bindings.
>>>>>
>>>>>>
>>>>>>> Documentation/devicetree/bindings/mtd/nand-controller.yaml and
>>>>>>> Documentation/devicetree/bindings/mtd/amlogic,meson-nand.yaml show
>>>>>>> that the chip select of a NAND chip is specified with a "reg"
>>>>>>> property.
>>>>>>
>>>>>> All NAND controller binding expect the chip-select to be in the
>>>>>> 'reg' property, very much like a spi device would use reg to store the
>>>>>> cs as well: the reg property tells you how you address the device.
>>>>>>
>>>>>> I also fully agree with Martin's comments below. Changing reg is likely
>>>>>> a wrong approach :)
>>>>>>
>>>>>>> Also the code has to be backwards compatible with old .dtbs.
>>>>>>>
>>>>>>>> Example:
>>>>>>>>
>>>>>>>> [...] nvmem mtd0-user-otp: nvmem: invalid reg on /soc/bus@ffe00000/...
>>>>>>>> [...] mtd mtd0: Failed to register OTP NVMEM device
>>>>>>>> [...] meson-nand ffe07800.nfc: failed to register MTD device: -22
>>>>>>>> [...] meson-nand ffe07800.nfc: failed to init NAND chips
>>>>>>>> [...] meson-nand: probe of ffe07800.nfc failed with error -22
>>>>>>> This is odd - can you please share your definition of the &nfc node?
>>>>>
>>>>> Sure, here it is:
>>>>>
>>>>> mtd_nand: nfc@7800 {
>>>>> compatible = "amlogic,meson-axg-nfc";
>>>>> ...
>>>>> nand@0 {
>>>>> reg = <0>;
>>>>> };
>>>>> }
>>>>>
>>>>> I checked, that 'nand_set_flash_node()' is called with 'nand@0' and i suppose
>>>>> that it is correct (as You mentioned below). But, 'nvmem_add_cells_from_of()' is called
>>>>> with parent: 'nfc@7800', then it iterates over its childs, e.g. 'nand@0' and thus i get such
>>>>> situation. I guess, that 'nvmem_add_cells_from_of()' must be called with 'nand@0' ?
>>>>
>>>> We recently had issues with nvmem parsing, but I believe a mainline
>>>> kernel should now be perfectly working on this regard. What version of
>>>> the Linux kernel are you using?
>>>
>>> My current version is:
>>>
>>> VERSION = 6
>>> PATCHLEVEL = 2
>>> SUBLEVEL = 0
>>> EXTRAVERSION = -rc8
>>>
>>> Fix was in drivers/nvmem/* ?
>>>
>>> Thanks, Arseniy
>>
>> Upd: I resolved problem in the following way:
>>
>> nand@0 {
>> reg = <0>;//chip select
>>
> partitions {
> compatible = ...
>
>> otp@0 {
>> #address-cells = <2>;
>> #size-cells = <0>;
>
> #address/size-cells is not needed here
>
>> compatible = "user-otp";
>> reg = <A B>;
>> };
>> otp@1 {
>> #address-cells = <2>;
>> #size-cells = <0>;
>
> Ditto
>
>> compatible = "factory-otp";
>> reg = <C D>;
>> };
>
> };
>
>> };
>>
>> Now nvmem subsystem parses 'otp@0' and 'otp@1' and error is gone. 'compatible' values are
>> the same as in drivers/mtd/mtdcore.c:mtd_otp_nvmem_add(). 'reg' in 'nand@0' is used as
>> chip select as supposed.
>
> I don't fully get it. The parsing on the nvmem side should not fail if
> there is no subpartition/otp-region defined. Can you confirm an empty
> NAND device node works? Because your last e-mail suggested the opposite.
Ok, so i'll describe what happens in my case. Let's NAND node be like this (IIUC this is
considered as empty NAND device):
mtd_nand: nfc@7800 {
compatible = "amlogic,meson-axg-nfc";
...
nand@0 {
reg = <0>;
};
}
I see, that
1) 'mtd_otp_nvmem_add()' calls 'mtd_otp_nvmem_register()' twice for two types of
OTP memory "user-otp" and "factory-otp". Let's take a look only on "user-otp".
2) 'mtd_otp_nvmem_register()' tries to lookup for node in 'nand@0' which is compatible with
"user-otp" and then passes found (or not found, e.g. NULL) node to 'nvmem_register()'.
3) 'nvmem_register()' uses this node iterating over its childs and searching value "reg" in
each child. If "user-otp" node is not found in 2), 'nvmem_register()' uses node 'nfc@7800'
also looking for "reg" value in each of its child. In this case it found "reg" in 'nand@0'
and fails.
Now, if i add "compatible = "user-otp";" to 'nand@0', in step 2) search will be successful,
and "reg" value will be used from this new node (or we remove "reg" from it - nothing happens
as You wrote). So, problem is that nvmem tries to parse node with invalid "reg" value.
Also I see, that 'nvmem_register()' is called earlier in 'mtd_nvmem_add()', but with no effect.
I think, that it is not related with enabled OTP feature.
Thanks, Arseniy
>
>>
>> I think, this patch should be abandoned in the next version.
>>
>> Thanks, Arseniy
>>
>>>
>>>>
>>>> Thanks,
>>>> Miquèl
>
>
> Thanks,
> Miquèl
Hi Arseniy,
I'm adding Rafał & Michael: any idea what could be wrong? The behavior
below does not look expected at all, but I thought we (= Rafał, mainly)
already sorted this out?
> >>>>>>> On Wed, May 10, 2023 at 1:13 PM Arseniy Krasnov
> >>>>>>> <[email protected]> wrote:
> >>>>>>>>
> >>>>>>>> This renames node with values for chip select from "reg" to "cs". It is
> >>>>>>>> needed because when OTP access is enabled on the attached storage, MTD
> >>>>>>>> subsystem registers this storage in the NVMEM subsystem. NVMEM in turn
> >>>>>>>> tries to use "reg" node in its own manner, supposes that it has another
> >>>>>>>> layout. All of this leads to device initialization failure.
> >>>>>>> In general: if we change the device-tree interface (in this case:
> >>>>>>> replacing a "reg" with a "cs" property) the dt-bindings have to be
> >>>>>>> updated as well.
> >>>>>>
> >>>>>> True, and I would add, bindings should not be broken.
> >>>>>
> >>>>> I see, that's true. That is bad way to change bindings.
> >>>>>
> >>>>>>
> >>>>>>> Documentation/devicetree/bindings/mtd/nand-controller.yaml and
> >>>>>>> Documentation/devicetree/bindings/mtd/amlogic,meson-nand.yaml show
> >>>>>>> that the chip select of a NAND chip is specified with a "reg"
> >>>>>>> property.
> >>>>>>
> >>>>>> All NAND controller binding expect the chip-select to be in the
> >>>>>> 'reg' property, very much like a spi device would use reg to store the
> >>>>>> cs as well: the reg property tells you how you address the device.
> >>>>>>
> >>>>>> I also fully agree with Martin's comments below. Changing reg is likely
> >>>>>> a wrong approach :)
> >>>>>>
> >>>>>>> Also the code has to be backwards compatible with old .dtbs.
> >>>>>>>
> >>>>>>>> Example:
> >>>>>>>>
> >>>>>>>> [...] nvmem mtd0-user-otp: nvmem: invalid reg on /soc/bus@ffe00000/...
> >>>>>>>> [...] mtd mtd0: Failed to register OTP NVMEM device
> >>>>>>>> [...] meson-nand ffe07800.nfc: failed to register MTD device: -22
> >>>>>>>> [...] meson-nand ffe07800.nfc: failed to init NAND chips
> >>>>>>>> [...] meson-nand: probe of ffe07800.nfc failed with error -22
> >>>>>>> This is odd - can you please share your definition of the &nfc node?
> >>>>>
> >>>>> Sure, here it is:
> >>>>>
> >>>>> mtd_nand: nfc@7800 {
> >>>>> compatible = "amlogic,meson-axg-nfc";
> >>>>> ...
> >>>>> nand@0 {
> >>>>> reg = <0>;
> >>>>> };
> >>>>> }
> >>>>>
> >>>>> I checked, that 'nand_set_flash_node()' is called with 'nand@0' and i suppose
> >>>>> that it is correct (as You mentioned below). But, 'nvmem_add_cells_from_of()' is called
> >>>>> with parent: 'nfc@7800', then it iterates over its childs, e.g. 'nand@0' and thus i get such
> >>>>> situation. I guess, that 'nvmem_add_cells_from_of()' must be called with 'nand@0' ?
> >>>>
> >>>> We recently had issues with nvmem parsing, but I believe a mainline
> >>>> kernel should now be perfectly working on this regard. What version of
> >>>> the Linux kernel are you using?
> >>>
> >>> My current version is:
> >>>
> >>> VERSION = 6
> >>> PATCHLEVEL = 2
> >>> SUBLEVEL = 0
> >>> EXTRAVERSION = -rc8
> >>>
> >>> Fix was in drivers/nvmem/* ?
> >>>
> >>> Thanks, Arseniy
> >>
> >> Upd: I resolved problem in the following way:
> >>
> >> nand@0 {
> >> reg = <0>;//chip select
> >>
> > partitions {
> > compatible = ...
> >
> >> otp@0 {
> >> #address-cells = <2>;
> >> #size-cells = <0>;
> >
> > #address/size-cells is not needed here
> >
> >> compatible = "user-otp";
> >> reg = <A B>;
> >> };
> >> otp@1 {
> >> #address-cells = <2>;
> >> #size-cells = <0>;
> >
> > Ditto
> >
> >> compatible = "factory-otp";
> >> reg = <C D>;
> >> };
> >
> > };
> >
> >> };
> >>
> >> Now nvmem subsystem parses 'otp@0' and 'otp@1' and error is gone. 'compatible' values are
> >> the same as in drivers/mtd/mtdcore.c:mtd_otp_nvmem_add(). 'reg' in 'nand@0' is used as
> >> chip select as supposed.
> >
> > I don't fully get it. The parsing on the nvmem side should not fail if
> > there is no subpartition/otp-region defined. Can you confirm an empty
> > NAND device node works? Because your last e-mail suggested the opposite.
>
> Ok, so i'll describe what happens in my case. Let's NAND node be like this (IIUC this is
> considered as empty NAND device):
>
> mtd_nand: nfc@7800 {
> compatible = "amlogic,meson-axg-nfc";
> ...
> nand@0 {
> reg = <0>;
> };
> }
>
> I see, that
>
> 1) 'mtd_otp_nvmem_add()' calls 'mtd_otp_nvmem_register()' twice for two types of
> OTP memory "user-otp" and "factory-otp". Let's take a look only on "user-otp".
> 2) 'mtd_otp_nvmem_register()' tries to lookup for node in 'nand@0' which is compatible with
> "user-otp" and then passes found (or not found, e.g. NULL) node to 'nvmem_register()'.
> 3) 'nvmem_register()' uses this node iterating over its childs and searching value "reg" in
> each child. If "user-otp" node is not found in 2), 'nvmem_register()' uses node 'nfc@7800'
> also looking for "reg" value in each of its child. In this case it found "reg" in 'nand@0'
> and fails.
>
> Now, if i add "compatible = "user-otp";" to 'nand@0', in step 2) search will be successful,
> and "reg" value will be used from this new node (or we remove "reg" from it - nothing happens
> as You wrote). So, problem is that nvmem tries to parse node with invalid "reg" value.
>
> Also I see, that 'nvmem_register()' is called earlier in 'mtd_nvmem_add()', but with no effect.
> I think, that it is not related with enabled OTP feature.
>
> Thanks, Arseniy
On 12.05.2023 17:49, Miquel Raynal wrote:
> Hi Arseniy,
>
> I'm adding Rafał & Michael: any idea what could be wrong? The behavior
> below does not look expected at all, but I thought we (= Rafał, mainly)
> already sorted this out?
>
Hi Miquel, thanks for help!
just to clarify an expected behaviour: if i have the following layout in the device tree
mtd_nand: nfc@7800 {
compatible = "amlogic,meson-axg-nfc";
...
nand@0 {
reg = <0>;
};
}
node used by 'nvmem_add_cells_from_of()' must be NULL? or 'nand@0'?
I guess, that in above dts I have node 'nfc@7800' in use, because 'mtd_otp_nvmem_register()' uses
the following device before passing 'config' to 'nvmem_register()':
/* OTP nvmem will be registered on the physical device */
config.dev = mtd->dev.parent;
'mtd->dev.parent' is 'nfc@7800'.
May be 'mtd_otp_nvmem_register()' must initialize 'no_of_node' field of 'config' like in 'mtd_nvmem_add()' ?
This field is documented as:
* @no_of_node: Device should not use the parent's of_node even if it's !NULL.
In this case node passed to 'nvmem_add_cells_from_of()' will be NULL.
Thanks, Arseniy
>>>>>>>>> On Wed, May 10, 2023 at 1:13 PM Arseniy Krasnov
>>>>>>>>> <[email protected]> wrote:
>>>>>>>>>>
>>>>>>>>>> This renames node with values for chip select from "reg" to "cs". It is
>>>>>>>>>> needed because when OTP access is enabled on the attached storage, MTD
>>>>>>>>>> subsystem registers this storage in the NVMEM subsystem. NVMEM in turn
>>>>>>>>>> tries to use "reg" node in its own manner, supposes that it has another
>>>>>>>>>> layout. All of this leads to device initialization failure.
>>>>>>>>> In general: if we change the device-tree interface (in this case:
>>>>>>>>> replacing a "reg" with a "cs" property) the dt-bindings have to be
>>>>>>>>> updated as well.
>>>>>>>>
>>>>>>>> True, and I would add, bindings should not be broken.
>>>>>>>
>>>>>>> I see, that's true. That is bad way to change bindings.
>>>>>>>
>>>>>>>>
>>>>>>>>> Documentation/devicetree/bindings/mtd/nand-controller.yaml and
>>>>>>>>> Documentation/devicetree/bindings/mtd/amlogic,meson-nand.yaml show
>>>>>>>>> that the chip select of a NAND chip is specified with a "reg"
>>>>>>>>> property.
>>>>>>>>
>>>>>>>> All NAND controller binding expect the chip-select to be in the
>>>>>>>> 'reg' property, very much like a spi device would use reg to store the
>>>>>>>> cs as well: the reg property tells you how you address the device.
>>>>>>>>
>>>>>>>> I also fully agree with Martin's comments below. Changing reg is likely
>>>>>>>> a wrong approach :)
>>>>>>>>
>>>>>>>>> Also the code has to be backwards compatible with old .dtbs.
>>>>>>>>>
>>>>>>>>>> Example:
>>>>>>>>>>
>>>>>>>>>> [...] nvmem mtd0-user-otp: nvmem: invalid reg on /soc/bus@ffe00000/...
>>>>>>>>>> [...] mtd mtd0: Failed to register OTP NVMEM device
>>>>>>>>>> [...] meson-nand ffe07800.nfc: failed to register MTD device: -22
>>>>>>>>>> [...] meson-nand ffe07800.nfc: failed to init NAND chips
>>>>>>>>>> [...] meson-nand: probe of ffe07800.nfc failed with error -22
>>>>>>>>> This is odd - can you please share your definition of the &nfc node?
>>>>>>>
>>>>>>> Sure, here it is:
>>>>>>>
>>>>>>> mtd_nand: nfc@7800 {
>>>>>>> compatible = "amlogic,meson-axg-nfc";
>>>>>>> ...
>>>>>>> nand@0 {
>>>>>>> reg = <0>;
>>>>>>> };
>>>>>>> }
>>>>>>>
>>>>>>> I checked, that 'nand_set_flash_node()' is called with 'nand@0' and i suppose
>>>>>>> that it is correct (as You mentioned below). But, 'nvmem_add_cells_from_of()' is called
>>>>>>> with parent: 'nfc@7800', then it iterates over its childs, e.g. 'nand@0' and thus i get such
>>>>>>> situation. I guess, that 'nvmem_add_cells_from_of()' must be called with 'nand@0' ?
>>>>>>
>>>>>> We recently had issues with nvmem parsing, but I believe a mainline
>>>>>> kernel should now be perfectly working on this regard. What version of
>>>>>> the Linux kernel are you using?
>>>>>
>>>>> My current version is:
>>>>>
>>>>> VERSION = 6
>>>>> PATCHLEVEL = 2
>>>>> SUBLEVEL = 0
>>>>> EXTRAVERSION = -rc8
>>>>>
>>>>> Fix was in drivers/nvmem/* ?
>>>>>
>>>>> Thanks, Arseniy
>>>>
>>>> Upd: I resolved problem in the following way:
>>>>
>>>> nand@0 {
>>>> reg = <0>;//chip select
>>>>
>>> partitions {
>>> compatible = ...
>>>
>>>> otp@0 {
>>>> #address-cells = <2>;
>>>> #size-cells = <0>;
>>>
>>> #address/size-cells is not needed here
>>>
>>>> compatible = "user-otp";
>>>> reg = <A B>;
>>>> };
>>>> otp@1 {
>>>> #address-cells = <2>;
>>>> #size-cells = <0>;
>>>
>>> Ditto
>>>
>>>> compatible = "factory-otp";
>>>> reg = <C D>;
>>>> };
>>>
>>> };
>>>
>>>> };
>>>>
>>>> Now nvmem subsystem parses 'otp@0' and 'otp@1' and error is gone. 'compatible' values are
>>>> the same as in drivers/mtd/mtdcore.c:mtd_otp_nvmem_add(). 'reg' in 'nand@0' is used as
>>>> chip select as supposed.
>>>
>>> I don't fully get it. The parsing on the nvmem side should not fail if
>>> there is no subpartition/otp-region defined. Can you confirm an empty
>>> NAND device node works? Because your last e-mail suggested the opposite.
>>
>> Ok, so i'll describe what happens in my case. Let's NAND node be like this (IIUC this is
>> considered as empty NAND device):
>>
>> mtd_nand: nfc@7800 {
>> compatible = "amlogic,meson-axg-nfc";
>> ...
>> nand@0 {
>> reg = <0>;
>> };
>> }
>>
>> I see, that
>>
>> 1) 'mtd_otp_nvmem_add()' calls 'mtd_otp_nvmem_register()' twice for two types of
>> OTP memory "user-otp" and "factory-otp". Let's take a look only on "user-otp".
>> 2) 'mtd_otp_nvmem_register()' tries to lookup for node in 'nand@0' which is compatible with
>> "user-otp" and then passes found (or not found, e.g. NULL) node to 'nvmem_register()'.
>> 3) 'nvmem_register()' uses this node iterating over its childs and searching value "reg" in
>> each child. If "user-otp" node is not found in 2), 'nvmem_register()' uses node 'nfc@7800'
>> also looking for "reg" value in each of its child. In this case it found "reg" in 'nand@0'
>> and fails.
>>
>> Now, if i add "compatible = "user-otp";" to 'nand@0', in step 2) search will be successful,
>> and "reg" value will be used from this new node (or we remove "reg" from it - nothing happens
>> as You wrote). So, problem is that nvmem tries to parse node with invalid "reg" value.
>>
>> Also I see, that 'nvmem_register()' is called earlier in 'mtd_nvmem_add()', but with no effect.
>> I think, that it is not related with enabled OTP feature.
>>
>> Thanks, Arseniy