| Message ID | 20210216212638.28382-4-ansuelsmth@gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v2,1/3] mtd: partitions: ofpart: skip subnodes parse with compatible | expand |
[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.
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
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?
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.
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"?
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
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 <ansuelsmth@gmail.com> + +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>; + }; + }; + };
Document nvmem-partitions compatible used to treat mtd partitions as a nvmem provider. Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com> --- .../mtd/partitions/nvmem-partitions.yaml | 105 ++++++++++++++++++ 1 file changed, 105 insertions(+) create mode 100644 Documentation/devicetree/bindings/mtd/partitions/nvmem-partitions.yaml