Add nvmem support for dynamic partitions
Subsystems (2)
cover Add nvmem support for dynamic partitions
This very small series comes to fix the very annyoing problem of partitions declared by parser at runtime NOT supporting nvmem cells definition.
The current implementation is very generic. The idea is to provide an of node if defined for everyone and not strictly limit this to nvmem stuff. But still the actual change is done only for nvmem-cells mtd. (just to make sure) This can totally change and be limited to restore the old mtd_get_of_node and add a function just to fix the problem with nvmem.
The idea here is that a user can still use these dynamic parsers instead of declaring a fixed-partition and also declare how nvmem-cells are defined for the partition.
This live with the assumption that dynamic partition have always the same name and they are known. (this is the case for smem partition that would require a bootloader reflash to change and for parsers like cmdlinepart where the name is always the same.)
With this assumption, it's easy to fix this problem. Just introduce a new partition node that will declare just these special partition. Mtdcore then will check if these special declaration are present and connect the dynamic partition with the of node present in the dts. Nvmem will automagically fin the ofnode and cells will be works based on the data provided by the parser.
The initial idea was to create a special nvmem driver with a special compatible where a user would declare the mtd partition name and this driver would search it and register the nvmem cells but that became difficult really fast, mtd notifier system is problematic for this kind of stuff. So here is the better implementation. A variant of this is already tested on openwrt where we have devices that use cmdlinepart. (that current variant have defined in the dts the exact copy of cmdlinepart in the fixed-partition scheme and we patched the cmdlinepart parser to scan this fixed-partition node (that is ignored as cmdlinepart have priority) and connect the dynamic partition with the dts node)
I provided an example of this in the documentation commit. In short the dynamic-partitions node is defined at the same level of the partitions node. The partition in the "dynamic-partitions" node require to have the same label of the dynamic-partition that will be created at runtime and then a user can use the usual nvmem-cells way to declare nvmem cells. Only the needed partition has to be declared, all the other dynamic-partitions can be ignored.
I currently tested this on my device that have smem and the gmac driver use nvmem to get the mac-address. This works correctly and the same address is provided.
I hope we find a solution to this problem as currently we are trying to transition to nvmem and we found this limitation. This seems to be a good solution that doesn't looks to be too much of an hack.
Ansuel Smith (2):
dt-bindings: mtd: partitions: Document new dynamic-partitions node
mtd: core: introduce of support for dynamic partitions
.../mtd/partitions/dynamic-partitions.yaml | 59 +++++++++++++++++++ drivers/mtd/mtdcore.c | 45 ++++++++++++++ include/linux/mtd/mtd.h | 6 +- 3 files changed, 105 insertions(+), 5 deletions(-) create mode 100644 Documentation/devicetree/bindings/mtd/partitions/dynamic-partitions.yaml
Signature · 1 line
2.33.1
RFC PATCH 1/2 dt-bindings: mtd: partitions: Document new dynamic-partitions node
Document new dynamic-partitions node used to provide an of node for partition registred at runtime by parsers. This is required for nvmem system to declare and detect nvmem-cells.
--- .../mtd/partitions/dynamic-partitions.yaml | 59 +++++++++++++++++++ 1 file changed, 59 insertions(+) create mode 100644 Documentation/devicetree/bindings/mtd/partitions/dynamic-partitions.yaml
Documentation/devicetree/bindings/mtd/partitions/dynamic-partitions.yaml new file +59 -0
@@ -0,0 +1,59 @@ +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/mtd/partitions/dynamic-partitions.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Dynamic partitions + +description: | + This binding can be used on platforms which have partitions registered at + runtime by parsers or partition table present on the flash. Example are + partitions declared from smem parser or cmdlinepart. This will create an + of node for these dynamic partition where systems like Nvmem can get a + reference to register nvmem-cells. + + The partition table should be a node named "dynamic-partitions". + Partitions are then defined as subnodes. Only the label is required + as any other data will be taken from the parser. + +maintainers: + - Ansuel Smith <[email protected]> + +properties: + compatible: + const: dynamic-partitions + +patternProperties: + "@[0-9a-f]+$": + $ref: "partition.yaml#" + +additionalProperties: true + +examples: + - | + partitions { + compatible = "qcom,smem"; + #address-cells = <1>; + #size-cells = <1>; + }; + + dynamic-partitions { + compatible = "dynamic-partitions"; + + art: art { + label = "0:art"; + read-only; + compatible = "nvmem-cells"; + #address-cells = <1>; + #size-cells = <1>; + + macaddr_art_0: macaddr@0 { + reg = <0x0 0x6>; + }; + + macaddr_art_6: macaddr@6 { + reg = <0x6 0x6>; + }; + }; + };
Signature · 1 line
2.33.1
Rob Herring
maintainerRe: [RFC PATCH 1/2] dt-bindings: mtd: partitions: Document new dynamic-partitions node
Quoted 10 lines · Ansuel Smith
Document new dynamic-partitions node used to provide an of node for partition registred at runtime by parsers. This is required for nvmem system to declare and detect nvmem-cells.
Signed-off-by Ansuel Smith <[email protected]>--- .../mtd/partitions/dynamic-partitions.yaml | 59 +++++++++++++++++++ 1 file changed, 59 insertions(+) create mode 100644 Documentation/devicetree/bindings/mtd/partitions/dynamic-partitions.yaml
My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check' on your patch (DT_CHECKER_FLAGS is new in v5.13):
yamllint warnings/errors:
dtschema/dtc warnings/errors:
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/mtd/partitions/dynamic-partitions.example.dt.yaml: partitions: 'hwlocks' is a required property
From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/soc/qcom/qcom,smem.yaml
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/mtd/partitions/dynamic-partitions.example.dt.yaml: partitions: 'oneOf' conditional failed, one must be fixed:
'reg' is a required property
'no-map' is a required property
'memory-region' is a required property
From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/soc/qcom/qcom,smem.yaml
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/mtd/partitions/dynamic-partitions.example.dt.yaml: partitions: '#address-cells', '#size-cells' do not match any of the regexes: 'pinctrl-[0-9]+'
From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/soc/qcom/qcom,smem.yaml
doc reference errors (make refcheckdocs):
See https://patchwork.ozlabs.org/patch/1582321
This check can fail if there are any dependencies. The base for a patch series is generally the most recent rc1.
If you already ran 'make dt_binding_check' and didn't see the above error(s), then make sure 'yamllint' is installed and dt-schema is up to date:
pip3 install dtschema --upgrade
Please check and re-submit.
Rob Herring
maintainerRe: [RFC PATCH 1/2] dt-bindings: mtd: partitions: Document new dynamic-partitions node
In reply to Ansuel SmithQuoted 3 lines · Ansuel Smith
Document new dynamic-partitions node used to provide an of node for partition registred at runtime by parsers. This is required for nvmem system to declare and detect nvmem-cells.
So you have some discoverable way to find all the partitions and the nvmem cells are at an unknown (to the DT) location, but still need to be described in DT?
Quoted 14 lines
Signed-off-by Ansuel Smith <[email protected]>--- .../mtd/partitions/dynamic-partitions.yaml | 59 +++++++++++++++++++ 1 file changed, 59 insertions(+) create mode 100644 Documentation/devicetree/bindings/mtd/partitions/dynamic-partitions.yamlDocumentation/devicetree/bindings/mtd/partitions/dynamic-partitions.yaml new file +2
-0@@ -0,0 +1,59 @@ +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause +%YAML 1.2
Documentation/devicetree/bindings/mtd/partitions/dynamic-partitions.yaml line 12
Quoted 10 lines
Documentation/devicetree/bindings/mtd/partitions/dynamic-partitions.yaml new file +10
-0+--- +$id: http://devicetree.org/schemas/mtd/partitions/dynamic-partitions.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Dynamic partitions + +description: | + This binding can be used on platforms which have partitions registered at + runtime by parsers or partition table present on the flash. Example are + partitions declared from smem parser or cmdlinepart. This will create an
Some information in DT and some on the cmdline seems broken to me. Pick one or the other.
Documentation/devicetree/bindings/mtd/partitions/dynamic-partitions.yaml line 25
Quoted 13 lines
Documentation/devicetree/bindings/mtd/partitions/dynamic-partitions.yaml new file +13
-0+ of node for these dynamic partition where systems like Nvmem can get a + reference to register nvmem-cells. + + The partition table should be a node named "dynamic-partitions". + Partitions are then defined as subnodes. Only the label is required + as any other data will be taken from the parser. + +maintainers: + - Ansuel Smith <[email protected]> + +properties: + compatible: + const: dynamic-partitions
This is useless. This tells me nothing about the what's in the partitions.
Quoted 38 lines
Documentation/devicetree/bindings/mtd/partitions/dynamic-partitions.yaml new file +34
-0+ +patternProperties: + "@[0-9a-f]+$": + $ref: "partition.yaml#" + +additionalProperties: true + +examples: + - | + partitions { + compatible = "qcom,smem"; + #address-cells = <1>; + #size-cells = <1>; + }; + + dynamic-partitions { + compatible = "dynamic-partitions"; + + art: art { + label = "0:art"; + read-only; + compatible = "nvmem-cells"; + #address-cells = <1>; + #size-cells = <1>; + + macaddr_art_0: macaddr@0 { + reg = <0x0 0x6>; + }; + + macaddr_art_6: macaddr@6 { + reg = <0x6 0x6>; + }; + }; + };Signature · 1 line
2.33.1
Re: [RFC PATCH 1/2] dt-bindings: mtd: partitions: Document new dynamic-partitions node
Quoted 9 lines · Rob Herring
Quoted 3 lines · Ansuel Smith
Document new dynamic-partitions node used to provide an of node for partition registred at runtime by parsers. This is required for nvmem system to declare and detect nvmem-cells.
So you have some discoverable way to find all the partitions and the nvmem cells are at an unknown (to the DT) location, but still need to be described in DT?
Example: smem partition layout is saved in the bootloader and static. To know the layout just boot the device and extract it. Aside from this the naming convention is ""standard"" (example the standard nvmem location for this is named 0:art)
What needs to be described in the DT is the cell offset and the partition name (the location)
NVMEM doesn't support this and honestly I can't think of a simple and direct way to solve this. Considering partition in this case are just filled at runtime and they doesn't change (or at worst the partition offset change but NEVER the name) it seems a good way to fix this by describing a nvmem cells partition name and assign a of node to the runtime filled partition.
Quoted 19 lines
Quoted 19 lines
Signed-off-by Ansuel Smith <[email protected]>--- .../mtd/partitions/dynamic-partitions.yaml | 59 +++++++++++++++++++ 1 file changed, 59 insertions(+) create mode 100644 Documentation/devicetree/bindings/mtd/partitions/dynamic-partitions.yamlDocumentation/devicetree/bindings/mtd/partitions/dynamic-partitions.yaml new file +7
-0@@ -0,0 +1,59 @@ +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/mtd/partitions/dynamic-partitions.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Dynamic partitions
Quoted 9 lines
Documentation/devicetree/bindings/mtd/partitions/dynamic-partitions.yaml line 12
Quoted 5 lines
Documentation/devicetree/bindings/mtd/partitions/dynamic-partitions.yaml new file +5
-0+ +description: | + This binding can be used on platforms which have partitions registered at + runtime by parsers or partition table present on the flash. Example are + partitions declared from smem parser or cmdlinepart. This will create anSome information in DT and some on the cmdline seems broken to me. Pick one or the other.
Converting a system from cmdline to fixedpartition is problematic if the cmdline is dynamic. Example some system have dual partition and are handled by editing the cmdline partition description. In this cmdline tho the nvmem cell of our interest doesn't change and we can use this new implemenation to add support for nvmem cells.
So really there are some case where nvmem won't work and it's not possible to provide a correct configuration for nvmem to work correctly.
Is it that bad to have information in the DT about nvmem cells for a partition named with a particular label that won't change.
Quoted 55 lines
Documentation/devicetree/bindings/mtd/partitions/dynamic-partitions.yaml line 25
Quoted 13 lines
Documentation/devicetree/bindings/mtd/partitions/dynamic-partitions.yaml new file +13
-0+ of node for these dynamic partition where systems like Nvmem can get a + reference to register nvmem-cells. + + The partition table should be a node named "dynamic-partitions". + Partitions are then defined as subnodes. Only the label is required + as any other data will be taken from the parser. + +maintainers: + - Ansuel Smith <[email protected]> + +properties: + compatible: + const: dynamic-partitionsThis is useless. This tells me nothing about the what's in the partitions.
Quoted 38 lines
Documentation/devicetree/bindings/mtd/partitions/dynamic-partitions.yaml new file +34
-0+ +patternProperties: + "@[0-9a-f]+$": + $ref: "partition.yaml#" + +additionalProperties: true + +examples: + - | + partitions { + compatible = "qcom,smem"; + #address-cells = <1>; + #size-cells = <1>; + }; + + dynamic-partitions { + compatible = "dynamic-partitions"; + + art: art { + label = "0:art"; + read-only; + compatible = "nvmem-cells"; + #address-cells = <1>; + #size-cells = <1>; + + macaddr_art_0: macaddr@0 { + reg = <0x0 0x6>; + }; + + macaddr_art_6: macaddr@6 { + reg = <0x6 0x6>; + }; + }; + };Signature · 1 line
2.33.1
Signature · 1 line
Ansuel
Re: [RFC PATCH 1/2] dt-bindings: mtd: partitions: Document new dynamic-partitions node
In reply to Ansuel SmithQuoted 65 lines · Ansuel Smith
Document new dynamic-partitions node used to provide an of node for partition registred at runtime by parsers. This is required for nvmem system to declare and detect nvmem-cells.
Signed-off-by Ansuel Smith <[email protected]>--- .../mtd/partitions/dynamic-partitions.yaml | 59 +++++++++++++++++++ 1 file changed, 59 insertions(+)create mode 100644
Documentation/devicetree/bindings/mtd/partitions/dynamic-partitions.yamlDocumentation/devicetree/bindings/mtd/partitions/dynamic-partitions.yaml new file +49
-0@@ -0,0 +1,59 @@ +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/mtd/partitions/dynamic-partitions.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Dynamic partitions + +description: | + This binding can be used on platforms which have partitions registered at + runtime by parsers or partition table present on the flash. Example are + partitions declared from smem parser or cmdlinepart. This will create an + of node for these dynamic partition where systems like Nvmem can get a + reference to register nvmem-cells. + + The partition table should be a node named "dynamic-partitions". + Partitions are then defined as subnodes. Only the label is required + as any other data will be taken from the parser. + +maintainers: + - Ansuel Smith <[email protected]> + +properties: + compatible: + const: dynamic-partitions + +patternProperties: + "@[0-9a-f]+$": + $ref: "partition.yaml#" + +additionalProperties: true + +examples: + - | + partitions { + compatible = "qcom,smem"; + #address-cells = <1>; + #size-cells = <1>; + }; + + dynamic-partitions { + compatible = "dynamic-partitions"; + + art: art { + label = "0:art"; + read-only; + compatible = "nvmem-cells"; + #address-cells = <1>; + #size-cells = <1>;
Documentation/devicetree/bindings/mtd/partitions/dynamic-partitions.yaml line 59
Quoted 10 lines
Documentation/devicetree/bindings/mtd/partitions/dynamic-partitions.yaml new file +10
-0+ + macaddr_art_0: macaddr@0 { + reg = <0x0 0x6>; + }; + + macaddr_art_6: macaddr@6 { + reg = <0x6 0x6>; + }; + }; + };
First of all: I fully support such a feature. I need it for Broadom platforms that use "brcm,bcm947xx-cfe-partitions" dynamic partitions. In my case bootloader partition is created dynamically (it doesn't have const offset and size). It contains NVMEM data however that needs to be described in DT.
This binding however looks loose and confusing to me.
First of all did you really mean to use "qcom,smem"? My first guess is you meant "qcom,smem-part".
Secondly can't we have partitions defined just as subnodes of the partitions { ... }; node?
I think sth like below would make more sense:
partitions {
compatible = "qcom,smem-part";
art {
label = "0:art";
read-only;
compatible = "nvmem-cells";
#address-cells = <1>;
#size-cells = <1>;
macaddr_art_0: macaddr@0 {
reg = <0x0 0x6>;
};
macaddr_art_6: macaddr@6 {
reg = <0x6 0x6>;
};
};
};Then I could also reuse that for something like:
partitions {
compatible = "brcm,bcm947xx-cfe-partitions";
partition-0 {
compatible = "nvmem-cells";
label = "boot";
#address-cells = <1>;
#size-cells = <1>;
mac: macaddr@0 {
reg = <0x100 0x6>;
};
}
};______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
Re: [RFC PATCH 1/2] dt-bindings: mtd: partitions: Document new dynamic-partitions node
Quoted 83 lines · Rafał Miłecki
Quoted 65 lines · Ansuel Smith
Document new dynamic-partitions node used to provide an of node for partition registred at runtime by parsers. This is required for nvmem system to declare and detect nvmem-cells.
Signed-off-by Ansuel Smith <[email protected]>--- .../mtd/partitions/dynamic-partitions.yaml | 59 +++++++++++++++++++ 1 file changed, 59 insertions(+)create mode 100644
Documentation/devicetree/bindings/mtd/partitions/dynamic-partitions.yamlDocumentation/devicetree/bindings/mtd/partitions/dynamic-partitions.yaml new file +49
-0@@ -0,0 +1,59 @@ +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/mtd/partitions/dynamic-partitions.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Dynamic partitions + +description: | + This binding can be used on platforms which have partitions registered at + runtime by parsers or partition table present on the flash. Example are + partitions declared from smem parser or cmdlinepart. This will create an + of node for these dynamic partition where systems like Nvmem can get a + reference to register nvmem-cells. + + The partition table should be a node named "dynamic-partitions". + Partitions are then defined as subnodes. Only the label is required + as any other data will be taken from the parser. + +maintainers: + - Ansuel Smith <[email protected]> + +properties: + compatible: + const: dynamic-partitions + +patternProperties: + "@[0-9a-f]+$": + $ref: "partition.yaml#" + +additionalProperties: true + +examples: + - | + partitions { + compatible = "qcom,smem"; + #address-cells = <1>; + #size-cells = <1>; + }; + + dynamic-partitions { + compatible = "dynamic-partitions"; + + art: art { + label = "0:art"; + read-only; + compatible = "nvmem-cells"; + #address-cells = <1>; + #size-cells = <1>;Documentation/devicetree/bindings/mtd/partitions/dynamic-partitions.yaml line 59
Quoted 10 lines
Documentation/devicetree/bindings/mtd/partitions/dynamic-partitions.yaml new file +10
-0+ + macaddr_art_0: macaddr@0 { + reg = <0x0 0x6>; + }; + + macaddr_art_6: macaddr@6 { + reg = <0x6 0x6>; + }; + }; + };First of all: I fully support such a feature. I need it for Broadom platforms that use "brcm,bcm947xx-cfe-partitions" dynamic partitions. In my case bootloader partition is created dynamically (it doesn't have const offset and size). It contains NVMEM data however that needs to be described in DT.
Quoted 2 lines
This binding however looks loose and confusing to me.
I agree.
Quoted 3 lines
First of all did you really mean to use "qcom,smem"? My first guess is you meant "qcom,smem-part".
Yes sorry, I was referring to the smem parser qcom,smem-part
Quoted 3 lines
Secondly can't we have partitions defined just as subnodes of the partitions { ... }; node?
I would love to use it. My only concern is that due to the fact that we have to support legacy partition declaring, wonder if this could create some problem. I'm referring to declaring fixed partition without using any compatible/standard binding name.
I remember we improved that with the introduction of the nvmem binding by making the fixed-partition compatible mandatory. But I would like to have extra check. Wonder if to be on the safe part we can consider appending to the "dynamic parser" a compatible like "dynamic-partitions" and use your way to declare them (aka keeping the dynamic-partition and removing the extra parallel partitions list)
Feel free to tell me it's just a stupid and unnecessary idea. I just have fear to introduce regression in the partition parsing logic.
Quoted 41 lines
I think sth like below would make more sense:
partitions { compatible = "qcom,smem-part"; art { label = "0:art"; read-only; compatible = "nvmem-cells"; #address-cells = <1>; #size-cells = <1>; macaddr_art_0: macaddr@0 { reg = <0x0 0x6>; }; macaddr_art_6: macaddr@6 { reg = <0x6 0x6>; }; }; };Then I could also reuse that for something like:
partitions { compatible = "brcm,bcm947xx-cfe-partitions"; partition-0 { compatible = "nvmem-cells"; label = "boot"; #address-cells = <1>; #size-cells = <1>; mac: macaddr@0 { reg = <0x100 0x6>; }; } };
Signature · 5 lines
Ansuel ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/
Re: [RFC PATCH 1/2] dt-bindings: mtd: partitions: Document new dynamic-partitions node
Quoted 95 lines · Ansuel Smith
Quoted 83 lines · Rafał Miłecki
Quoted 65 lines · Ansuel Smith
Document new dynamic-partitions node used to provide an of node for partition registred at runtime by parsers. This is required for nvmem system to declare and detect nvmem-cells.
Signed-off-by Ansuel Smith <[email protected]>--- .../mtd/partitions/dynamic-partitions.yaml | 59 +++++++++++++++++++ 1 file changed, 59 insertions(+)create mode 100644
Documentation/devicetree/bindings/mtd/partitions/dynamic-partitions.yamlDocumentation/devicetree/bindings/mtd/partitions/dynamic-partitions.yaml new file +49
-0@@ -0,0 +1,59 @@ +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/mtd/partitions/dynamic-partitions.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Dynamic partitions + +description: | + This binding can be used on platforms which have partitions registered at + runtime by parsers or partition table present on the flash. Example are + partitions declared from smem parser or cmdlinepart. This will create an + of node for these dynamic partition where systems like Nvmem can get a + reference to register nvmem-cells. + + The partition table should be a node named "dynamic-partitions". + Partitions are then defined as subnodes. Only the label is required + as any other data will be taken from the parser. + +maintainers: + - Ansuel Smith <[email protected]> + +properties: + compatible: + const: dynamic-partitions + +patternProperties: + "@[0-9a-f]+$": + $ref: "partition.yaml#" + +additionalProperties: true + +examples: + - | + partitions { + compatible = "qcom,smem"; + #address-cells = <1>; + #size-cells = <1>; + }; + + dynamic-partitions { + compatible = "dynamic-partitions"; + + art: art { + label = "0:art"; + read-only; + compatible = "nvmem-cells"; + #address-cells = <1>; + #size-cells = <1>;Documentation/devicetree/bindings/mtd/partitions/dynamic-partitions.yaml line 59
Quoted 10 lines
Documentation/devicetree/bindings/mtd/partitions/dynamic-partitions.yaml new file +10
-0+ + macaddr_art_0: macaddr@0 { + reg = <0x0 0x6>; + }; + + macaddr_art_6: macaddr@6 { + reg = <0x6 0x6>; + }; + }; + };First of all: I fully support such a feature. I need it for Broadom platforms that use "brcm,bcm947xx-cfe-partitions" dynamic partitions. In my case bootloader partition is created dynamically (it doesn't have const offset and size). It contains NVMEM data however that needs to be described in DT.
Quoted 2 lines
This binding however looks loose and confusing to me.
I agree.
Quoted 3 lines
First of all did you really mean to use "qcom,smem"? My first guess is you meant "qcom,smem-part".
Yes sorry, I was referring to the smem parser qcom,smem-part
Quoted 8 lines
Quoted 3 lines
Secondly can't we have partitions defined just as subnodes of the partitions { ... }; node?
I would love to use it. My only concern is that due to the fact that we have to support legacy partition declaring, wonder if this could create some problem. I'm referring to declaring fixed partition without using any compatible/standard binding name.
Legacy partitioning won't kick in if you have "partitions" with "compatible" string. We're safe here. Just checked to be sure.
Quoted 9 lines
I remember we improved that with the introduction of the nvmem binding by making the fixed-partition compatible mandatory. But I would like to have extra check. Wonder if to be on the safe part we can consider appending to the "dynamic parser" a compatible like "dynamic-partitions" and use your way to declare them (aka keeping the dynamic-partition and removing the extra parallel partitions list)
Feel free to tell me it's just a stupid and unnecessary idea. I just have fear to introduce regression in the partition parsing logic.
I'm confused. I think all dynamic partitioners already have a "compatible" set.
Can you post an example of DT binging you described above, please?
Re: [RFC PATCH 1/2] dt-bindings: mtd: partitions: Document new dynamic-partitions node
Quoted 100 lines · Rafał Miłecki
Quoted 99 lines · Ansuel Smith
Quoted 83 lines · Rafał Miłecki
Quoted 65 lines · Ansuel Smith
Document new dynamic-partitions node used to provide an of node for partition registred at runtime by parsers. This is required for nvmem system to declare and detect nvmem-cells.
Signed-off-by Ansuel Smith <[email protected]>--- .../mtd/partitions/dynamic-partitions.yaml | 59 +++++++++++++++++++ 1 file changed, 59 insertions(+)create mode 100644
Documentation/devicetree/bindings/mtd/partitions/dynamic-partitions.yamlDocumentation/devicetree/bindings/mtd/partitions/dynamic-partitions.yaml new file +49
-0@@ -0,0 +1,59 @@ +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/mtd/partitions/dynamic-partitions.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Dynamic partitions + +description: | + This binding can be used on platforms which have partitions registered at + runtime by parsers or partition table present on the flash. Example are + partitions declared from smem parser or cmdlinepart. This will create an + of node for these dynamic partition where systems like Nvmem can get a + reference to register nvmem-cells. + + The partition table should be a node named "dynamic-partitions". + Partitions are then defined as subnodes. Only the label is required + as any other data will be taken from the parser. + +maintainers: + - Ansuel Smith <[email protected]> + +properties: + compatible: + const: dynamic-partitions + +patternProperties: + "@[0-9a-f]+$": + $ref: "partition.yaml#" + +additionalProperties: true + +examples: + - | + partitions { + compatible = "qcom,smem"; + #address-cells = <1>; + #size-cells = <1>; + }; + + dynamic-partitions { + compatible = "dynamic-partitions"; + + art: art { + label = "0:art"; + read-only; + compatible = "nvmem-cells"; + #address-cells = <1>; + #size-cells = <1>;Documentation/devicetree/bindings/mtd/partitions/dynamic-partitions.yaml line 59
Quoted 10 lines
Documentation/devicetree/bindings/mtd/partitions/dynamic-partitions.yaml new file +10
-0+ + macaddr_art_0: macaddr@0 { + reg = <0x0 0x6>; + }; + + macaddr_art_6: macaddr@6 { + reg = <0x6 0x6>; + }; + }; + };First of all: I fully support such a feature. I need it for Broadom platforms that use "brcm,bcm947xx-cfe-partitions" dynamic partitions. In my case bootloader partition is created dynamically (it doesn't have const offset and size). It contains NVMEM data however that needs to be described in DT.
Quoted 2 lines
This binding however looks loose and confusing to me.
I agree.
Quoted 3 lines
First of all did you really mean to use "qcom,smem"? My first guess is you meant "qcom,smem-part".
Yes sorry, I was referring to the smem parser qcom,smem-part
Quoted 3 lines
Secondly can't we have partitions defined just as subnodes of the partitions { ... }; node?
Quoted 8 lines
Quoted 4 lines
I would love to use it. My only concern is that due to the fact that we have to support legacy partition declaring, wonder if this could create some problem. I'm referring to declaring fixed partition without using any compatible/standard binding name.
Legacy partitioning won't kick in if you have "partitions" with "compatible" string. We're safe here. Just checked to be sure.
Oh ok then the dynamic partition compatible stuff is not needed. To make sure I will change the "connect" function part and skip the of_node assign if a compatible is not present. (The of_node assign should be done only with the nvmem-cell compatible currently.)
Quoted 8 lines
Quoted 8 lines
I remember we improved that with the introduction of the nvmem binding by making the fixed-partition compatible mandatory. But I would like to have extra check. Wonder if to be on the safe part we can consider appending to the "dynamic parser" a compatible like "dynamic-partitions" and use your way to declare them (aka keeping the dynamic-partition and removing the extra parallel partitions list)
Quoted 5 lines
Quoted 2 lines
Feel free to tell me it's just a stupid and unnecessary idea. I just have fear to introduce regression in the partition parsing logic.
I'm confused. I think all dynamic partitioners already have a "compatible" set.
Now that I think about it you are right. If a dynamic partition is present in the system, a compatible must be present to use the correct parser. And as I said up, all the nvmem cells should have the correct compatible.
Quoted 2 lines
Can you post an example of DT binging you described above, please?
Was thinking something like this. But not needed.
partitions {
compatible = "brcm,bcm947xx-cfe-partitions", "dynamic-partitions";
partition-0 {
compatible = "nvmem-cells";
label = "boot";
#address-cells = <1>;
#size-cells = <1>;
mac: macaddr@0 {
reg = <0x100 0x6>;
};
}
};So in short, a scheme like this should NOT be handled/should not have of_node assigned. (and is actually very wrong)
partitions {
compatible = "brcm,bcm947xx-cfe-partitions";
partition-0 {
label = "boot";
#address-cells = <1>;
#size-cells = <1>;
mac: macaddr@0 {
reg = <0x100 0x6>;
};
}
};Signature · 1 line
Ansuel
RFC PATCH 2/2 mtd: core: introduce of support for dynamic partitions
In reply to Ansuel SmithWe have many parser that register mtd partitions at runtime. One example is the cmdlinepart or the smem partition where the compatible is defined in the dts and the partitions gets detected and registered by the parser. This is problematic for the Nvmem system that requires an of node to detect nvmem cells. To fix this problem, introduce an additional node called "dynamic-partitions" that must be defined at the same level of the "partitions" node that will contain all the required partitions where a nvmem cell has to be declared. When a mtd_get_of_node() is called, the function will first check the default dev_of_node() and then check this alternative partitions node and optionally if a "nvmem-cells" compatible is detected, sets the of node for the mtd.
The "dynamic-partitions" requires the label set to the mtd name from the dynamic partition. All the nvmem-cells will be declared in this node and nvmem will use this node to register the nvmem cells.
--- drivers/mtd/mtdcore.c | 45 +++++++++++++++++++++++++++++++++++++++++ include/linux/mtd/mtd.h | 6 +----- 2 files changed, 46 insertions(+), 5 deletions(-)
drivers/mtd/mtdcore.c +45 -0
@@ -563,6 +563,51 @@ static int mtd_nvmem_add(struct mtd_info *mtd)return 0 ; } +struct device_node *mtd_get_of_node (struct mtd_info *mtd) +{ +struct device_node *dynamic_partitions, *parent_dn, *dn, *mtd_dn =NULL ; +struct mtd_info *parent; +const char *mtd_name; +int plen; + +/* Check if mtd has a device node */ + dn =dev_of_node (&mtd->dev); +if (dn) +return dn; + +/* Check if a dynamic-partitions node exist */ + parent = mtd->parent; + parent_dn =dev_of_node (&parent->dev); +if (!parent_dn) +return NULL ; + + dynamic_partitions =of_get_compatible_child (parent_dn,"dynamic-partitions" ); +if (!dynamic_partitions) + goto exit_parent; + +/* Search if a dynamic partition is defined with the same name */ +for_each_child_of_node (dynamic_partitions, dn) { + mtd_name =of_get_property (dn,"label" , &plen); +if (!strncmp (mtd->name, mtd_name, plen)) { + mtd_dn = dn; +break ; + } + } + +if (!mtd_dn) + goto exit_partitions; + +/* Set of_node only for nvmem */ +if (of_device_is_compatible (mtd_dn,"nvmem-cells" )) +mtd_set_of_node (mtd, mtd_dn); + +exit_partitions: +of_node_put (dynamic_partitions); +exit_parent: +of_node_put (parent_dn); +return mtd_dn; +} +/** * add_mtd_device - register an MTD device * @mtd: pointer to new MTD device info structure
include/linux/mtd/mtd.h +1 -5
@@ -464,11 +464,6 @@ static inline void mtd_set_of_node(struct mtd_info *mtd,of_property_read_string (np,"label" , &mtd->name); } -static inline struct device_node *mtd_get_of_node (struct mtd_info *mtd) -{ -return dev_of_node (&mtd->dev); -} -static inline u32 mtd_oobavail (struct mtd_info *mtd,struct mtd_oob_ops *ops) {return ops->mode ==MTD_OPS_AUTO_OOB ? mtd->oobavail : mtd->oobsize;
@@ -489,6 +484,7 @@ static inline int mtd_max_bad_blocks(struct mtd_info *mtd, len); } +struct device_node *mtd_get_of_node (struct mtd_info *mtd);int mtd_wunit_to_pairing_info (struct mtd_info *mtd,int wunit,struct mtd_pairing_info *info); int mtd_pairing_info_to_wunit(struct mtd_info *mtd,
Signature · 1 line
2.33.1
kernel test robot
build botRe: [RFC PATCH 2/2] mtd: core: introduce of support for dynamic partitions
[If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch]
Hi Ansuel,
[FYI, it's a private test report for your RFC patch.]
[auto build test ERROR on mtd/mtd/next]
[also build test ERROR on mtd/mtd/fixes robh/for-next v5.16 next-20220120] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Ansuel-Smith/Add-nvmem-support-for-dynamic-partitions/20220121-042801 base: https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git mtd/next config: arc-randconfig-r043-20220120 (https://download.01.org/0day-ci/archive/20220121/202201211003.6NTRXP9H-lkp(a)intel.com/config) compiler: arc-elf-gcc (GCC) 11.2.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross
# https://github.com/0day-ci/linux/commit/e0b2803ed83327a84dc5e92fbb2f2d704748e08f
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Ansuel-Smith/Add-nvmem-support-for-dynamic-partitions/20220121-042801 git checkout e0b2803ed83327a84dc5e92fbb2f2d704748e08f
# save the config file to linux build tree
mkdir build_dir
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross O=build_dir ARCH=arc SHELL=/bin/bash
If you fix the issue, kindly add following tag as appropriate
All errors (new ones prefixed by >>, old ones prefixed by <<):
>> ERROR: modpost: "mtd_get_of_node" [drivers/mtd/nand/nandcore.ko] undefined! >> ERROR: modpost: "mtd_get_of_node" [drivers/mtd/parsers/ofpart.ko] undefined!
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org