2021-02-16 21:29:55

by Christian Marangi

[permalink] [raw]
Subject: [PATCH v2 0/3] Implement nvmem support for mtd

The mtd support for the nvmem api has been stalled from 2018 with a patch
half pushed hoping that a scheme is found for the mtd name later. This
patchset try to address this.

The solution is simple.
New partitions scheme should always have the partitions {} structure and
declare subnodes as partitions is deprecated and should not be used anymore.
Fixed-partitions parser is changed to parse direct subnode as partitions
only if the appropriate compatible is used. This change make possible
the use of nvmem-partitions compatible and the entire partition node can
be parsed by the nvmem of framework.
The current code register the partition to the nvmem framework every time
but skip actually of_node parting. The new nvmem-partitions compatible is
used to enable of_node parsing on the desired partitions.

Ansuel Smith (3):
mtd: partitions: ofpart: skip subnodes parse with compatible
mtd: core: add nvmem-partitions compatible to parse mtd as nvmem cells
dt-bindings: mtd: Document use of nvmem-partitions compatible

.../mtd/partitions/nvmem-partitions.yaml | 105 ++++++++++++++++++
drivers/mtd/mtdcore.c | 3 +-
drivers/mtd/parsers/ofpart.c | 5 +
3 files changed, 112 insertions(+), 1 deletion(-)
create mode 100644 Documentation/devicetree/bindings/mtd/partitions/nvmem-partitions.yaml

--
2.30.0


2021-02-16 21:29:56

by Christian Marangi

[permalink] [raw]
Subject: [PATCH v2 3/3] dt-bindings: mtd: Document use of nvmem-partitions compatible

Document nvmem-partitions compatible used to treat mtd partitions as a
nvmem provider.

Signed-off-by: Ansuel Smith <[email protected]>
---
.../mtd/partitions/nvmem-partitions.yaml | 105 ++++++++++++++++++
1 file changed, 105 insertions(+)
create mode 100644 Documentation/devicetree/bindings/mtd/partitions/nvmem-partitions.yaml

diff --git a/Documentation/devicetree/bindings/mtd/partitions/nvmem-partitions.yaml b/Documentation/devicetree/bindings/mtd/partitions/nvmem-partitions.yaml
new file mode 100644
index 000000000000..1ff283febcaa
--- /dev/null
+++ b/Documentation/devicetree/bindings/mtd/partitions/nvmem-partitions.yaml
@@ -0,0 +1,105 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/mtd/partitions/nvmem-partitions.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Nvmem partitions
+
+description: |
+ This binding can be used to treat the specific partition as a nvmem provider.
+ Each direct subnodes represents the nvmem cells and won't be parsed as fixed-partitions.
+ Fixed-partitions bindings described in fixed-partitions.yaml apply to the nvmem provider node.
+
+maintainers:
+ - Ansuel Smith <[email protected]>
+
+properties:
+ compatible:
+ const: nvmem-partitions
+
+ "#address-cells": true
+
+ "#size-cells": true
+
+ reg:
+ description: partition's offset and size within the flash
+ maxItems: 1
+
+required:
+ - compatible
+ - "#address-cells"
+ - "#size-cells"
+ - reg
+
+additionalProperties: true
+
+examples:
+ - |
+ partitions {
+ compatible = "fixed-partitions";
+ #address-cells = <1>;
+ #size-cells = <1>;
+
+ /* ... */
+
+ };
+ art: art@1200000 {
+ compatible = "nvmem-partitions";
+ reg = <0x1200000 0x0140000>;
+ label = "art";
+ read-only;
+ #address-cells = <1>;
+ #size-cells = <1>;
+
+ macaddr_gmac1: macaddr_gmac1@0 {
+ reg = <0x0 0x6>;
+ };
+
+ macaddr_gmac2: macaddr_gmac2@6 {
+ reg = <0x6 0x6>;
+ };
+
+ pre_cal_24g: pre_cal_24g@1000 {
+ reg = <0x1000 0x2f20>;
+ };
+
+ pre_cal_5g: pre_cal_5g@5000{
+ reg = <0x5000 0x2f20>;
+ };
+ };
+ - |
+ partitions {
+ compatible = "fixed-partitions";
+ #address-cells = <1>;
+ #size-cells = <1>;
+
+ partition@0 {
+ label = "bootloader";
+ reg = <0x000000 0x100000>;
+ read-only;
+ };
+
+ firmware@100000 {
+ compatible = "brcm,trx";
+ label = "firmware";
+ reg = <0x100000 0xe00000>;
+ };
+
+ calibration@f00000 {
+ compatible = "nvmem-partitions";
+ label = "calibration";
+ reg = <0xf00000 0x100000>;
+ ranges = <0 0xf00000 0x100000>;
+ #address-cells = <1>;
+ #size-cells = <1>;
+
+ wifi0@0 {
+ reg = <0x000000 0x080000>;
+ };
+
+ wifi1@80000 {
+ reg = <0x080000 0x080000>;
+ };
+ };
+ };
--
2.30.0

2021-02-16 21:30:35

by Christian Marangi

[permalink] [raw]
Subject: [PATCH v2 2/3] mtd: core: add nvmem-partitions compatible to parse mtd as nvmem cells

Partitions that contains the nvmem-partitions compatible will register
their direct subonodes as nvmem cells and the node will be treated as a
nvmem provider.

Signed-off-by: Ansuel Smith <[email protected]>
---
drivers/mtd/mtdcore.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c
index 2d6423d89a17..29d257678a86 100644
--- a/drivers/mtd/mtdcore.c
+++ b/drivers/mtd/mtdcore.c
@@ -531,6 +531,7 @@ static int mtd_nvmem_reg_read(void *priv, unsigned int offset,

static int mtd_nvmem_add(struct mtd_info *mtd)
{
+ struct device_node *node = mtd_get_of_node(mtd);
struct nvmem_config config = {};

config.id = -1;
@@ -543,7 +544,7 @@ static int mtd_nvmem_add(struct mtd_info *mtd)
config.stride = 1;
config.read_only = true;
config.root_only = true;
- config.no_of_node = true;
+ config.no_of_node = !of_device_is_compatible(node, "nvmem-partitions");
config.priv = mtd;

mtd->nvmem = nvmem_register(&config);
--
2.30.0

2021-03-04 21:40:31

by Rafał Miłecki

[permalink] [raw]
Subject: Re: [PATCH v2 2/3] mtd: core: add nvmem-partitions compatible to parse mtd as nvmem cells

On 16.02.2021 22:26, Ansuel Smith wrote:
> Partitions that contains the nvmem-partitions compatible will register
> their direct subonodes as nvmem cells and the node will be treated as a
> nvmem provider.
>
> Signed-off-by: Ansuel Smith <[email protected]>

Tested-by: Rafał Miłecki <[email protected]>


I applied this patch on top of the:
[PATCH] mtd: parsers: ofpart: limit parsing of deprecated DT syntax

I succesfully used NVMEM cell defined in bootloader mtd partition for
reading device MAC address.

partitions {
compatible = "fixed-partitions";
#address-cells = <1>;
#size-cells = <1>;

partition@0 {
compatible = "nvmem-partitions";
label = "bootloader";
reg = <0x0 0x100000>;
#address-cells = <1>;
#size-cells = <1>;
ranges = <0 0x0 0x100000>;

base_mac_addr: mac@106a0 {
reg = <0x106a0 0x6>;
};
};
}

2021-03-04 21:44:29

by Rafał Miłecki

[permalink] [raw]
Subject: Re: [PATCH v2 3/3] dt-bindings: mtd: Document use of nvmem-partitions compatible

[Rob: please advise]

On 16.02.2021 22:26, Ansuel Smith wrote:
> Document nvmem-partitions compatible used to treat mtd partitions as a
> nvmem provider.

Until now we were using "compatible" string in partition node only for
parsers (looking for subpartitions). We need to think if this change can
break anything from DT / Linux perspective.

Compatible strings should be unique, so there is no risk of conflict
between NVMEM and parsers.

Now: can we ever need mtd partition to:
1. Contain subpartitions
2. Provide NVMEM
at the same time?

Let's say:

partition@0 {
compatible = "vendor,dynamic-firmware-partitions", "nvmem-partitions";
label = "firmware";
reg = <0x0 0x100000>;
#address-cells = <1>;
#size-cells = <1>;
ranges = <0 0x0 0x100000>;

firmware-version@10 {
reg = <0x10 0x4>;
};

firmware-date@10 {
reg = <0x20 0x4>;
};
};

Is that allowed to respect both "compatible" strings and have:
1. Linux parser parse "firmware" for subpartitions
2. Linux MTD register "firmware" as NVMEM device
?

If not, what other options do we have? Is that allowed to have a
dangling MTD NVMEM node with phandle to MTD partition?

firmware: partition@0 {
compatible = "vendor,dynamic-firmware-partitions";
label = "firmware";
reg = <0x0 0x100000>;
};

(...)

firmware-version@10 {
compatible = "mtd-nvmem";
reg = <0x10 0x4>;
mtd = <&firmware>;
};

firmware-date@10 {
compatible = "mtd-nvmem";
reg = <0x20 0x4>;
mtd = <&firmware>;
};


Rob: I'd really appreciate your input & help here.

2021-03-05 22:24:40

by Rob Herring

[permalink] [raw]
Subject: Re: [PATCH v2 3/3] dt-bindings: mtd: Document use of nvmem-partitions compatible

On Wed, Mar 03, 2021 at 11:01:55AM +0100, Rafał Miłecki wrote:
> [Rob: please advise]
>
> On 16.02.2021 22:26, Ansuel Smith wrote:
> > Document nvmem-partitions compatible used to treat mtd partitions as a
> > nvmem provider.
>
> Until now we were using "compatible" string in partition node only for
> parsers (looking for subpartitions). We need to think if this change can
> break anything from DT / Linux perspective.
>
> Compatible strings should be unique, so there is no risk of conflict
> between NVMEM and parsers.
>
> Now: can we ever need mtd partition to:
> 1. Contain subpartitions
> 2. Provide NVMEM
> at the same time?
>
> Let's say:
>
> partition@0 {
> compatible = "vendor,dynamic-firmware-partitions", "nvmem-partitions";

I think you'd want the "vendor,dynamic-firmware-partitions" parser/code
to serve up any nvmem regions. Whether you have a fallback here depends
if an OS could make use of the regions knowing nothing about
"vendor,dynamic-firmware-partitions".

> label = "firmware";
> reg = <0x0 0x100000>;
> #address-cells = <1>;
> #size-cells = <1>;
> ranges = <0 0x0 0x100000>;
>
> firmware-version@10 {
> reg = <0x10 0x4>;
> };
>
> firmware-date@10 {
> reg = <0x20 0x4>;
> };
> };
>
> Is that allowed to respect both "compatible" strings and have:
> 1. Linux parser parse "firmware" for subpartitions
> 2. Linux MTD register "firmware" as NVMEM device
> ?
>
> If not, what other options do we have? Is that allowed to have a
> dangling MTD NVMEM node with phandle to MTD partition?
>
> firmware: partition@0 {
> compatible = "vendor,dynamic-firmware-partitions";
> label = "firmware";
> reg = <0x0 0x100000>;
> };
>
> (...)
>
> firmware-version@10 {
> compatible = "mtd-nvmem";
> reg = <0x10 0x4>;
> mtd = <&firmware>;
> };
>
> firmware-date@10 {
> compatible = "mtd-nvmem";
> reg = <0x20 0x4>;
> mtd = <&firmware>;
> };

This, I would not like to see.

Rob

2021-03-08 10:16:10

by Rafał Miłecki

[permalink] [raw]
Subject: Re: [PATCH v2 3/3] dt-bindings: mtd: Document use of nvmem-partitions compatible

On 05.03.2021 23:23, Rob Herring wrote:
> On Wed, Mar 03, 2021 at 11:01:55AM +0100, Rafał Miłecki wrote:
>> [Rob: please advise]
>>
>> On 16.02.2021 22:26, Ansuel Smith wrote:
>>> Document nvmem-partitions compatible used to treat mtd partitions as a
>>> nvmem provider.
>>
>> Until now we were using "compatible" string in partition node only for
>> parsers (looking for subpartitions). We need to think if this change can
>> break anything from DT / Linux perspective.
>>
>> Compatible strings should be unique, so there is no risk of conflict
>> between NVMEM and parsers.
>>
>> Now: can we ever need mtd partition to:
>> 1. Contain subpartitions
>> 2. Provide NVMEM
>> at the same time?
>>
>> Let's say:
>>
>> partition@0 {
>> compatible = "vendor,dynamic-firmware-partitions", "nvmem-partitions";
>
> I think you'd want the "vendor,dynamic-firmware-partitions" parser/code
> to serve up any nvmem regions. Whether you have a fallback here depends
> if an OS could make use of the regions knowing nothing about
> "vendor,dynamic-firmware-partitions".

Perfect! I didn't think that driver handling
"vendor,dynamic-firmware-partitions" may also take care of NVMEM.

Thank you.

2021-03-08 11:21:56

by Rafał Miłecki

[permalink] [raw]
Subject: Re: [PATCH v2 3/3] dt-bindings: mtd: Document use of nvmem-partitions compatible

On 16.02.2021 22:26, Ansuel Smith wrote:
> Document nvmem-partitions compatible used to treat mtd partitions as a
> nvmem provider.

I'm just wondering if "nvmem-partitions" is accurate enough. Partitions
bit sounds a bit ambiguous in the mtd context.

What do you think about "mtd-nvmem-cells" or just "nvmem-cells"?

2021-03-08 12:23:25

by Christian Marangi

[permalink] [raw]
Subject: Re: [PATCH v2 3/3] dt-bindings: mtd: Document use of nvmem-partitions compatible

On Mon, Mar 08, 2021 at 10:48:32AM +0100, Rafał Miłecki wrote:
> On 16.02.2021 22:26, Ansuel Smith wrote:
> > Document nvmem-partitions compatible used to treat mtd partitions as a
> > nvmem provider.
>
> I'm just wondering if "nvmem-partitions" is accurate enough. Partitions
> bit sounds a bit ambiguous in the mtd context.
>
> What do you think about "mtd-nvmem-cells" or just "nvmem-cells"?

I read somewhere that mtd is not so standard so "nvmem-cells" should be the
right compatible.
To sum up, with v2 I should change the compatible name and just push the
2 and 3 patch. (waiting your fix to be accepted) Correct?

2021-03-08 13:32:44

by Rafał Miłecki

[permalink] [raw]
Subject: Re: [PATCH v2 3/3] dt-bindings: mtd: Document use of nvmem-partitions compatible

On 07.03.2021 18:04, Ansuel Smith wrote:
> On Mon, Mar 08, 2021 at 10:48:32AM +0100, Rafał Miłecki wrote:
>> On 16.02.2021 22:26, Ansuel Smith wrote:
>>> Document nvmem-partitions compatible used to treat mtd partitions as a
>>> nvmem provider.
>>
>> I'm just wondering if "nvmem-partitions" is accurate enough. Partitions
>> bit sounds a bit ambiguous in the mtd context.
>>
>> What do you think about "mtd-nvmem-cells" or just "nvmem-cells"?
>
> I read somewhere that mtd is not so standard so "nvmem-cells" should be the
> right compatible.
> To sum up, with v2 I should change the compatible name and just push the
> 2 and 3 patch. (waiting your fix to be accepted) Correct?

Yes, I believe so

2021-03-08 13:34:48

by Rafał Miłecki

[permalink] [raw]
Subject: Re: [PATCH v2 3/3] dt-bindings: mtd: Document use of nvmem-partitions compatible

On 07.03.2021 18:04, Ansuel Smith wrote:
> On Mon, Mar 08, 2021 at 10:48:32AM +0100, Rafał Miłecki wrote:
>> On 16.02.2021 22:26, Ansuel Smith wrote:
>>> Document nvmem-partitions compatible used to treat mtd partitions as a
>>> nvmem provider.
>>
>> I'm just wondering if "nvmem-partitions" is accurate enough. Partitions
>> bit sounds a bit ambiguous in the mtd context.
>>
>> What do you think about "mtd-nvmem-cells" or just "nvmem-cells"?
>
> I read somewhere that mtd is not so standard so "nvmem-cells" should be the
> right compatible.
> To sum up, with v2 I should change the compatible name and just push the
> 2 and 3 patch. (waiting your fix to be accepted) Correct?

I'm also quite sure you're fine to send V2 now, if you just let
maintainers know (e.g. in a comment below a --- tear line) that it
depends on the:
[PATCH] mtd: parsers: ofpart: limit parsing of deprecated DT syntax

In other words: you don't need to wait for my patch to get accepted.