dt-bindings: mtd: partitions: Add binding for Sercomm parser
Subsystems (2)
v2 cover dt-bindings: mtd: partitions: Add binding for Sercomm parser
MTD partition parser for the Sercomm partition table that is used in some Beeline, Netgear and Sercomm routers.
The Sercomm partition map table contains real partition offsets, which may differ from device to device depending on the number and location of bad blocks on NAND.
Changes since:
v1:
- Add dt-binding in a separate patch
- Remove redundant braces and logical NOT operator
- Define pr_fmt
- Replace kcalloc by kzalloc
- Use of_get_child_count() and alloc big enough array before the for_each_child_of_node()
Mikhail Zhilkin (2):
dt-bindings: mtd: partitions: Add binding for Sercomm parser
mtd: parsers: add support for Sercomm partitions
.../devicetree/bindings/mtd/partitions/sercomm,sc-partitions.yaml | 70 ++++++++++ drivers/mtd/parsers/Kconfig | 9 ++ drivers/mtd/parsers/Makefile | 1 + drivers/mtd/parsers/scpart.c | 240 ++++++++++++++++++++++++++++++++++ 4 files changed, 320 insertions(+)
Signature · 6 lines
2.25.1 ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/
PATCH v2 1/2 dt-bindings: mtd: partitions: Add binding for Sercomm parser
Add YAML binding for Sercomm partition parser.
--- .../mtd/partitions/sercomm,sc-partitions.yaml | 70 +++++++++++++++++++ 1 file changed, 70 insertions(+) create mode 100644 Documentation/devicetree/bindings/mtd/partitions/sercomm,sc-partitions.yaml
Documentation/devicetree/bindings/mtd/partitions/sercomm,sc-partitions.yaml new file +70 -0
@@ -0,0 +1,70 @@ +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/mtd/partitions/sercomm,sc-partitions.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Sercomm Partitions + +description: | + Sercomm is one of hardware manufacturers providing SoCs used in home routers. + The Sercomm partition map table contains information about non-standard + partition offsets and sizes (depending on the bad blocks presence and their + locations). Partition map is used by many Sercomm-based Ralink devices + (e.g. Beeline, Netgear). + +maintainers: + - Mikhail Zhilkin <[email protected]> + +properties: + compatible: + const: sercomm,sc-partitions + + scpart-id: + $ref: /schemas/types.yaml#/definitions/uint32 + description: Partition id in Sercomm partition map + +required: + - compatible + - scpart-id + +additionalProperties: false + +examples: + - | + partitions + compatible = "sercomm,sc-partitions", "fixed-partitions"; + #address-cells = <1>; + #size-cells = <1>; + + partition@0 { + label = "u-boot"; + reg = <0x0 0x100000>; + scpart-id = <0>; + read-only; + }; + + partition@100000 { + label = "dynamic partition map"; + reg = <0x100000 0x100000>; + scpart-id = <1>; + }; + + factory: partition@200000 { + label = "Factory"; + reg = <0x200000 0x100000>; + scpart-id = <2>; + read-only; + + compatible = "nvmem-cells"; + #address-cells = <1>; + #size-cells = <1>; + + macaddr_factory_21000: macaddr@21000 { + reg = <0x21000 0x6>; + }; + }; + + /* ... */ + + };
Signature · 6 lines
2.25.1 ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/
Quoted 8 lines · Mikhail Zhilkin
Add YAML binding for Sercomm partition parser.
Signed-off-by Mikhail Zhilkin <[email protected]>--- .../mtd/partitions/sercomm,sc-partitions.yaml | 70 +++++++++++++++++++ 1 file changed, 70 insertions(+) create mode 100644 Documentation/devicetree/bindings/mtd/partitions/sercomm,sc-partitions.yaml
(...)
Documentation/devicetree/bindings/mtd/partitions/sercomm,sc-partitions.yaml line 25
Quoted 8 lines
Documentation/devicetree/bindings/mtd/partitions/sercomm,sc-partitions.yaml new file +8
-0+ +properties: + compatible: + const: sercomm,sc-partitions + + scpart-id: + $ref: /schemas/types.yaml#/definitions/uint32 + description: Partition id in Sercomm partition map
Do you really need it? The reg should define the order, unless you expect some incomplete partition list?
In any case this requires vendor prefix.
Documentation/devicetree/bindings/mtd/partitions/sercomm,sc-partitions.yaml line 28
Quoted 3 lines
Documentation/devicetree/bindings/mtd/partitions/sercomm,sc-partitions.yaml new file +3
-0+ +required: + - compatible
Missing reg.
Documentation/devicetree/bindings/mtd/partitions/sercomm,sc-partitions.yaml line 31
Quoted 3 lines
Documentation/devicetree/bindings/mtd/partitions/sercomm,sc-partitions.yaml new file +3
-0+ - scpart-id + +additionalProperties: false
Are you sure that you tested your bindings? You miss here address/size cells and children, so you should have big fat warning.
Plus your DTS example has error and does not compile...
Best regards,
Krzysztof
PATCH v2 1/2 dt-bindings: mtd: partitions: Add binding for Sercomm parser
Hello Krzysztof,
On 4/7/2022 10:48 AM, Krzysztof Kozlowski wrote:
(...)
Quoted 11 lines
Review
Quoted 7 lines
(quoted diff) +7
-0+properties: + compatible: + const: sercomm,sc-partitions + + scpart-id: + $ref: /schemas/types.yaml#/definitions/uint32 + description: Partition id in Sercomm partition mapDo you really need it? The reg should define the order, unless you expect some incomplete partition list?
In any case this requires vendor prefix.
I'm not sure that "scpart-id" is necessary here. "sercomm,sc-partitions" is necessary. I'm going to add vendor prefix in a separate patch. Is this ok?
---
Documentation/devicetree/bindings/vendor-prefixes.yaml | 2 ++
1 file changed, 2 insertions(+)
(quoted diff) +0 -0
b/Documentation/devicetree/bindings/vendor-prefixes.yaml index 01430973ecec..65ff22364fb3 100644
Documentation/devicetree/bindings/vendor-prefixes.yaml +0 -0
@@ -1082,6 +1082,8 @@ patternProperties:
description: Sensirion AG "^sensortek,.*":
description: Sensortek Technology Corporation + "^sercomm,.*":
+ description: Sercomm (Suzhou) Corporation "^sff,.*":
description: Small Form Factor Committee "^sgd,.*":
--
Quoted 4 lines
Review
Quoted 3 lines
(quoted diff) +3
-0+ +required: + - compatibleMissing reg.
reg isn't required. Parser can read partition offsets and sizes from SC PART MAP table. Or do you mean something else? All is ok without reg definition in "Example" (except the warns that reg property is missing).
Quoted 4 lines
Are you sure that you tested your bindings? You miss here address/size cells and children, so you should have big fat warning.
Plus your DTS example has error and does not compile...
Whole dts, for the real device (not for example), was tested many times. Thank you for your feedback! I checked the another examples and there are no any warnings now. But I'm not yet sure that "properties" and "required" are correct.
What do you think (or what else I have to read / check)?
---
.../mtd/partitions/sercomm,sc-partitions.yaml | 70 +++++++++++++++++++
1 file changed, 70 insertions(+)
create mode 100644 Documentation/devicetree/bindings/mtd/partitions/sercomm,sc-partitions.yaml
diff --git a/Documentation/devicetree/bindings/mtd/partitions/sercomm,sc-partitions.yaml b/Documentation/devicetree/bindings/mtd/partitions/sercomm,sc-partitions.yaml new file mode 100644
index 000000000000..cb171a0383aa
--- /dev/null
+++b/Documentation/devicetree/bindings/mtd/partitions/sercomm,sc-partitions.yaml @@ -0,0 +1,70 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id:
http://devicetree.org/schemas/mtd/partitions/sercomm,sc-partitions.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Sercomm Partitions
+
+description: |
+ Sercomm is one of hardware manufacturers providing SoCs used in home routers.
+ The Sercomm partition map table contains information about non-standard + partition offsets and sizes (depending on the bad blocks presence and their
+ locations). Partition map is used by many Sercomm-based Ralink devices + (e.g. Beeline, Netgear).
+
+maintainers:
+ - Mikhail Zhilkin <[email protected]>
+
+properties:
+ compatible:
+ const: sercomm,sc-partitions
+
+ "#address-cells": true
+
+ "#size-cells": true
+
+required:
+ - "#address-cells"
+ - "#size-cells"
+
+additionalProperties: true
+
+examples:
+ - |
+ partitions {
+ compatible = "sercomm,sc-partitions";
+ #address-cells = <1>;
+ #size-cells = <1>;
+
+ partition@0 {
+ label = "u-boot";
+ reg = <0x0 0x100000>;
+ scpart-id = <0>;
+ read-only;
+ };
+
+ partition@100000 {
+ label = "dynamic partition map";
+ reg = <0x100000 0x100000>;
+ scpart-id = <1>;
+ };
+
+ factory: partition@200000 {
+ label = "Factory";
+ reg = <0x200000 0x100000>;
+ scpart-id = <2>;
+ read-only;
+
+ compatible = "nvmem-cells";
+ #address-cells = <1>;
+ #size-cells = <1>;
+
+ macaddr_factory_21000: macaddr@21000 {
+ reg = <0x21000 0x6>;
+ };
+ };
+
+ /* ... */
+
+ };
Quoted 2 lines
Best regards,
Krzysztof
Best regards,
Mikhail
Quoted 6 lines · Mikhail Zhilkin
Quoted 2 lines
In any case this requires vendor prefix.
I'm not sure that "scpart-id" is necessary here. "sercomm,sc-partitions" is necessary. I'm going to add vendor prefix in a separate patch. Is this ok?
Yes.
Quoted 26 lines
---
Documentation/devicetree/bindings/vendor-prefixes.yaml | 2 ++1 file changed, 2 insertions(+)Documentation/devicetree/bindings/vendor-prefixes.yaml +0
-0@@ -1082,6 +1082,8 @@ patternProperties:description: Sensirion AG "^sensortek,.*":
description: Sensortek Technology Corporation + "^sercomm,.*":
+ description: Sercomm (Suzhou) Corporation "^sff,.*":
description: Small Form Factor Committee "^sgd,.*":
--Quoted 4 lines
Review
Quoted 3 lines
(quoted diff) +3
-0+ +required: + - compatibleMissing reg.
Quoted 4 lines
reg isn't required. Parser can read partition offsets and sizes from SC PART MAP table. Or do you mean something else? All is ok without reg definition in "Example" (except the warns that reg property is missing).
reg might not be required for current implementation but it is required by devicetree for every node with unit address. Do you expect here nodes without unit addresses?
Quoted 6 lines
Quoted 4 lines
Are you sure that you tested your bindings? You miss here address/size cells and children, so you should have big fat warning.
Plus your DTS example has error and does not compile...
Whole dts, for the real device (not for example), was tested many times.
Yeah, I did not speak about whole DTS, but bindings and example in the bindings.
Quoted 4 lines
Thank you for your feedback! I checked the another examples and there are no any warnings now. But I'm not yet sure that "properties" and "required" are correct.
What do you think (or what else I have to read / check)?
There is no way you tested the bindings. There are for sure warnings because it simply cannot be even compiled. The writing-schema.rst describes how to test it.
Best regards,
Krzysztof
PATCH v2 1/2 dt-bindings: mtd: partitions: Add binding for Sercomm parser
Quoted 4 lines · Krzysztof Kozlowski
Quoted 3 lines
I'm not sure that "scpart-id" is necessary here. "sercomm,sc-partitions" is necessary. I'm going to add vendor prefix in a separate patch. Is this ok?
Yes.
Thanks!
Quoted 10 lines
Quoted 7 lines
Quoted 3 lines
Review
Quoted 2 lines
(quoted diff) +2
-0+required: + - compatibleMissing reg.
reg isn't required. Parser can read partition offsets and sizes from SC PART MAP table. Or do you mean something else? All is ok without reg definition in "Example" (except the warns that reg property is missing).
reg might not be required for current implementation but it is required by devicetree for every node with unit address. Do you expect here nodes without unit addresses?
Only "partitions" node has no unit address. All subnodes have unit addresses and therefore have to have reg property. I've just realized that "fixed-partitions.yaml" is almost my case. It looks like I can copy'n'paste "required" and "*properties".
Do you mind if I don't reinvent the wheel and reuse this good practice?
Here's what I got (no any warnings appears):
---
.../mtd/partitions/sercomm,sc-partitions.yaml | 70 +++++++++++++++++++
1 file changed, 70 insertions(+)
create mode 100644 Documentation/devicetree/bindings/mtd/partitions/sercomm,sc-partitions.yaml
diff --git a/Documentation/devicetree/bindings/mtd/partitions/sercomm,sc-partitions.yaml b/Documentation/devicetree/bindings/mtd/partitions/sercomm,sc-partitions.yaml new file mode 100644
index 000000000000..cb171a0383aa
--- /dev/null
+++b/Documentation/devicetree/bindings/mtd/partitions/sercomm,sc-partitions.yaml @@ -0,0 +1,70 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id:
http://devicetree.org/schemas/mtd/partitions/sercomm,sc-partitions.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Sercomm Partitions
+
+description: |
+ Sercomm is one of hardware manufacturers providing SoCs used in home routers.
+ The Sercomm partition map table contains information about non-standard + partition offsets and sizes (depending on the bad blocks presence and their
+ locations). Partition map is used by many Sercomm-based Ralink devices + (e.g. Beeline, Netgear).
+
+maintainers:
+ - Mikhail Zhilkin <[email protected]>
+
+properties:
+ compatible:
+ const: sercomm,sc-partitions
+
+ "#address-cells": true
+
+ "#size-cells": true
+
+required:
+ - "#address-cells"
+ - "#size-cells"
+
+additionalProperties: true
+
+examples:
+ - |
+ partitions {
+ compatible = "sercomm,sc-partitions";
+ #address-cells = <1>;
+ #size-cells = <1>;
+
+ partition@0 {
+ label = "u-boot";
+ reg = <0x0 0x100000>;
+ scpart-id = <0>;
+ read-only;
+ };
+
+ partition@100000 {
+ label = "dynamic partition map";
+ reg = <0x100000 0x100000>;
+ scpart-id = <1>;
+ };
+
+ factory: partition@200000 {
+ label = "Factory";
+ reg = <0x200000 0x100000>;
+ scpart-id = <2>;
+ read-only;
+
+ compatible = "nvmem-cells";
+ #address-cells = <1>;
+ #size-cells = <1>;
+
+ macaddr_factory_21000: macaddr@21000 {
+ reg = <0x21000 0x6>;
+ };
+ };
+
+ /* ... */
+
+ };
Signature · 1 line
(...)
Quoted 2 lines
Best regards,
Krzysztof
Signature · 2 lines
Best regards, Mikhail
PATCH v2 1/2 dt-bindings: mtd: partitions: Add binding for Sercomm parser
Quoted 10 lines · Mikhail Zhilkin
Quoted 10 lines
Quoted 7 lines
Quoted 3 lines
Review
Quoted 2 lines
(quoted diff) +2
-0+required: + - compatibleMissing reg.
reg isn't required. Parser can read partition offsets and sizes from SC PART MAP table. Or do you mean something else? All is ok without reg definition in "Example" (except the warns that reg property is missing).
reg might not be required for current implementation but it is required by devicetree for every node with unit address. Do you expect here nodes without unit addresses?
Quoted 8 lines
Only "partitions" node has no unit address. All subnodes have unit addresses and therefore have to have reg property. I've just realized that "
fixed-partitions.yaml" is almost my case. It looks like I can copy'n'paste "required" and "*properties".
Do you mind if I don't reinvent the wheel and reuse this good practice?Here's what I got (no any warnings appears):
I'm sorry, Krzysztof & All. Here is the final one.
---
.../mtd/partitions/sercomm,sc-partitions.yaml | 78 +++++++++++++++++++
1 file changed, 78 insertions(+)
create mode 100644 Documentation/devicetree/bindings/mtd/partitions/sercomm,sc-partitions.yaml
diff --git a/Documentation/devicetree/bindings/mtd/partitions/sercomm,sc-partitions.yaml b/Documentation/devicetree/bindings/mtd/partitions/sercomm,sc-partitions.yaml new file mode 100644
index 000000000000..33172f0be92a
--- /dev/null
+++b/Documentation/devicetree/bindings/mtd/partitions/sercomm,sc-partitions.yaml @@ -0,0 +1,78 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id:
http://devicetree.org/schemas/mtd/partitions/sercomm,sc-partitions.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Sercomm Partitions
+
+description: |
+ Sercomm is one of hardware manufacturers providing SoCs used in home routers.
+ The Sercomm partition map table contains information about non-standard + partition offsets and sizes (depending on the bad blocks presence and their
+ locations). Partition map is used by many Sercomm-based Ralink devices (e.g.
+ Beeline, Netgear).
+
+ The partition table should be a node named "partitions". Partitions are then
+ defined as subnodes.
+
+maintainers:
+ - Mikhail Zhilkin <[email protected]>
+
+properties:
+ compatible:
+ const: sercomm,sc-partitions
+
+ "#address-cells": true
+
+ "#size-cells": true
+
+patternProperties:
+ "@[0-9a-f]+$":
+ $ref: "partition.yaml#"
+
+required:
+ - compatible
+ - "#address-cells"
+ - "#size-cells"
+
+additionalProperties: true
+
+examples:
+ - |
+ partitions {
+ compatible = "sercomm,sc-partitions";
+ #address-cells = <1>;
+ #size-cells = <1>;
+
+ partition@0 {
+ label = "u-boot";
+ reg = <0x0 0x100000>;
+ scpart-id = <0>;
+ read-only;
+ };
+
+ partition@100000 {
+ label = "dynamic partition map";
+ reg = <0x100000 0x100000>;
+ scpart-id = <1>;
+ };
+
+ factory: partition@200000 {
+ label = "Factory";
+ reg = <0x200000 0x100000>;
+ scpart-id = <2>;
+ read-only;
+
+ compatible = "nvmem-cells";
+ #address-cells = <1>;
+ #size-cells = <1>;
+
+ macaddr_factory_21000: macaddr@21000 {
+ reg = <0x21000 0x6>;
+ };
+ };
+
+ /* ... */
+
+ };
Quoted 2 lines
Best regards,
Mikhail
Quoted 19 lines · Mikhail Zhilkin
Quoted 17 lines · Mikhail Zhilkin
Quoted 10 lines
Quoted 7 lines
Quoted 3 lines
Review
Quoted 2 lines
(quoted diff) +2
-0+required: + - compatibleMissing reg.
reg isn't required. Parser can read partition offsets and sizes from SC PART MAP table. Or do you mean something else? All is ok without reg definition in "Example" (except the warns that reg property is missing).
reg might not be required for current implementation but it is required by devicetree for every node with unit address. Do you expect here nodes without unit addresses?
Only "partitions" node has no unit address. All subnodes have unit addresses and therefore have to have reg property. I've just realized that "
fixed-partitions.yaml" is almost my case. It looks like I can copy'n'paste "required" and "*properties".
Do you mind if I don't reinvent the wheel and reuse this good practice?
Quoted 4 lines
Quoted 1 line
Here's what I got (no any warnings appears):
I'm sorry, Krzysztof & All. Here is the final one.
I am sorry, but you changed now a lot in the bindings and it looks entirely different. Things previously being correct now are wrong, so rather start from your old bindings...
Quoted 48 lines
---
.../mtd/partitions/sercomm,sc-partitions.yaml | 78 +++++++++++++++++++1 file changed, 78 insertions(+)create mode 100644
Documentation/devicetree/bindings/mtd/partitions/sercomm,sc-partitions.yamldiff --git
a/Documentation/devicetree/bindings/mtd/partitions/sercomm,sc-partitions.yamlb/Documentation/devicetree/bindings/mtd/partitions/sercomm,sc-partitions.yamlnew file mode 100644
index 000000000000..33172f0be92a
---/dev/null
+++b/Documentation/devicetree/bindings/mtd/partitions/sercomm,sc-partitions.yaml(quoted diff) +4
-0@@ -0,0 +1,78 @@ +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause +%YAML 1.2 +--- +$id:http://devicetree.org/schemas/mtd/partitions/sercomm,sc-partitions.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#(quoted diff) +5
-0+ +title: Sercomm Partitions + +description: | + Sercomm is one of hardware manufacturers providing SoCs used in homerouters.
+ The Sercomm partition map table contains information about non-standard + partition offsets and sizes (depending on the bad blocks presence and their
+ locations). Partition map is used by many Sercomm-based Ralink devices (e.g.
+ Beeline, Netgear).
+
+ The partition table should be a node named "partitions". Partitions are then
+ defined as subnodes.(quoted diff) +10
-0+ +maintainers: + - Mikhail Zhilkin <[email protected]> + +properties: + compatible: + const: sercomm,sc-partitions + + "#address-cells": true +
Review
Quoted 10 lines
(quoted diff) +10
-0+ "#size-cells": true + +patternProperties: + "@[0-9a-f]+$": + $ref: "partition.yaml#" + +required: + - compatible + - "#address-cells" + - "#size-cells"
Still missing reg.
Review
Quoted 2 lines
(quoted diff) +2
-0+ +additionalProperties: true
This is wrong, why it became true?
Best regards,
Krzysztof
Hi, Krzysztof,
Quoted 3 lines · Krzysztof Kozlowski
I am sorry, but you changed now a lot in the bindings and it looks entirely different. Things previously being correct now are wrong, so rather start from your old bindings...
Looks like I'm a bit confused... I use dual "compatible" in my real dts and I realized that:
1. Therefore I have to use dual "compatible" in example too:
compatible = "sercomm,sc-partitions", "fixed-partitions";
2. When I'm trying to reuse "fixed-partitions" compatible from fixed-partitions.yaml in my new .yaml I get "too long" errors.
Real dts:
So, I currently found another solution - to extend fixed-partitions.yaml with "sercomm,sc-partitions". Is It ok from your side? Can I use this code in v3?
diff --git a/Documentation/devicetree/bindings/mtd/partitions/fixed-partitions.yaml b/Documentation/devicetree/bindings/mtd/partitions/fixed-partitions.yaml index ea4cace6a955..9eebe39a57fb 100644
Documentation/devicetree/bindings/mtd/partitions/fixed-partitions.yaml +0 -0
@@ -19,7 +19,11 @@ maintainers:
properties:
compatible:
- const: fixed-partitions
+ oneOf:
+ - const: fixed-partitions
+ - items:
+ - const: sercomm,sc-partitions
+ - const: fixed-partitions
"#address-cells": true
@@ -27,7 +31,18 @@ properties:
patternProperties:
"@[0-9a-f]+$":
- $ref: "partition.yaml#"
+ allOf:
+ - $ref: "partition.yaml#"
+ - if:
+ properties:
+ compatible:
+ contains:
+ const: sercomm,sc-partitions
+ then:
+ properties:
+ scpart-id:
+ description: Partition id in Sercomm partition map + $ref: /schemas/types.yaml#/definitions/uint32
required:
- "#address-cells"
@@ -119,3 +134,29 @@ examples:
};
};
};
+ - |
+ partitions {
+ compatible = "sercomm,sc-partitions", "fixed-partitions"; + #address-cells = <1>;
+ #size-cells = <1>;
+
+ partition@0 {
+ label = "u-boot";
+ reg = <0x0 0x100000>;
+ scpart-id=<0>;
+ read-only;
+ };
+
+ partition@100000 {
+ label = "dynamic partition map";
+ reg = <0x100000 0x100000>;
+ scpart-id = <1>;
+ };
+
+ partition@200000 {
+ label = "Factory";
+ reg = <0x200000 0x100000>;
+ scpart-id = <2>;
+ read-only;
+ };
+ };
Signature · 1 line
2.25.1
Quoted 3 lines
Review
Quoted 2 lines
(quoted diff) +2
-0+ +additionalProperties: trueThis is wrong, why it became true?
You're right. Got it
Quoted 2 lines
Best regards,
Krzysztof
Signature · 2 lines
Best regards, Mikhail
Quoted 12 lines · Mikhail Zhilkin
Hi, Krzysztof,
Quoted 3 lines · Krzysztof Kozlowski
I am sorry, but you changed now a lot in the bindings and it looks entirely different. Things previously being correct now are wrong, so rather start from your old bindings...
Looks like I'm a bit confused... I use dual "compatible" in my real dts and I realized that:
Quoted 6 lines
1. Therefore I have to use dual "compatible" in example too:
compatible = "sercomm,sc-partitions", "fixed-partitions";
2. When I'm trying to reuse "fixed-partitions" compatible from
fixed-partitions.yamlin my new .yaml I get "too long" errors.
Yes, the fixed-partitions.yaml would have to be changed to allow extension.
Quoted 9 lines
Real dts:
So, I currently found another solution - to extend
fixed-partitions.yamlwith "sercomm,sc-partitions". Is It ok from your side? Can I use this code in v3?
Not really, I don't understand why do you need it and it does not include our previous talks.
Quoted 27 lines
diff --git
a/Documentation/devicetree/bindings/mtd/partitions/fixed-partitions.yamlb/Documentation/devicetree/bindings/mtd/partitions/fixed-partitions.yamlindexea4cace6a955..9eebe39a57fb100644Documentation/devicetree/bindings/mtd/partitions/fixed-partitions.yaml +0
-0@@ -19,7 +19,11 @@ maintainers:
properties:
compatible:
- const: fixed-partitions
+ oneOf:
+ - const: fixed-partitions
+ - items:
+ - const: sercomm,sc-partitions
+ - const: fixed-partitions
"#address-cells": trueDocumentation/devicetree/bindings/mtd/partitions/fixed-partitions.yaml +0
-0@@ -27,7 +31,18 @@ properties:
patternProperties:
"@[0-9a-f]+$":
- $ref: "partition.yaml#"
+ allOf:
+ - $ref: "partition.yaml#"
Quoted 8 lines
+ - if:
+ properties:
+ compatible:
+ contains:
+ const: sercomm,sc-partitions
+ then:
+ properties:
+ scpart-id:
It still misses vendor prefix and we agreed you don't need it, didn't we?
Best regards,
Krzysztof
PATCH v2 1/2 dt-bindings: mtd: partitions: Add binding for Sercomm parser
Quoted 7 lines · Krzysztof Kozlowski
Quoted 7 lines
Quoted 3 lines
I am sorry, but you changed now a lot in the bindings and it looks entirely different. Things previously being correct now are wrong, so rather start from your old bindings...
Looks like I'm a bit confused... I use dual "compatible" in my real dts and I realized that:
Quoted 7 lines
Quoted 6 lines
1. Therefore I have to use dual "compatible" in example too:
compatible = "sercomm,sc-partitions", "fixed-partitions";
2. When I'm trying to reuse "fixed-partitions" compatible from
fixed-partitions.yamlin my new .yaml I get "too long" errors.Yes, the
fixed-partitions.yamlwould have to be changed to allow extension.
Well.
Quoted 9 lines
Quoted 8 lines
Real dts:
So, I currently found another solution - to extend
fixed-partitions.yamlwith "sercomm,sc-partitions". Is It ok from your side? Can I use this code in v3?Not really, I don't understand why do you need it
The main idea is keeping original Sercomm firmware behavior:
1. If dynamic partition map found then use offsets and mtd sizes stored in partition map. It's provided by "sercomm,sc-partitions" compatible.
2. If dynamic partition map doesn't exist or broken then default values (from dts) are used. It's provided by "fixed-partitions" compatible.
Quoted 2 lines
and it does not
include our previous talks.
At the time, I didn't realize how important is it. Understanding began to come after dozens of experiments and checking the similar Linux patches.
Quoted 27 lines
Quoted 27 lines
diff --git
a/Documentation/devicetree/bindings/mtd/partitions/fixed-partitions.yamlb/Documentation/devicetree/bindings/mtd/partitions/fixed-partitions.yamlindexea4cace6a955..9eebe39a57fb100644Documentation/devicetree/bindings/mtd/partitions/fixed-partitions.yaml +0
-0@@ -19,7 +19,11 @@ maintainers:
properties:
compatible:
- const: fixed-partitions
+ oneOf:
+ - const: fixed-partitions
+ - items:
+ - const: sercomm,sc-partitions
+ - const: fixed-partitions
"#address-cells": trueDocumentation/devicetree/bindings/mtd/partitions/fixed-partitions.yaml +0
-0@@ -27,7 +31,18 @@ properties:
patternProperties:
"@[0-9a-f]+$":
- $ref: "partition.yaml#"
+ allOf:
+ - $ref: "partition.yaml#"
+ - if:
Quoted 8 lines
Quoted 7 lines
+ properties:
+ compatible:
+ contains:
+ const: sercomm,sc-partitions
+ then:
+ properties:
+ scpart-id:It still misses vendor prefix and we agreed you don't need it, didn't we?
Do you mean "sercomm" vendor prefix? If so then we agreed that I include it in a separate patch:
Link:
https://lore.kernel.org/linux-mtd/[email protected]/
I'm going to send it in v3:
---
dt-bindings: Add Sercomm (Suzhou) Corporation vendor prefix
Update Documentation/devicetree/bindings/vendor-prefixes.yaml to include "sercomm" as a vendor prefix for "Sercomm (Suzhou) Corporation". Company website:
---
Documentation/devicetree/bindings/vendor-prefixes.yaml | 2 ++
1 file changed, 2 insertions(+)
(quoted diff) +0 -0
b/Documentation/devicetree/bindings/vendor-prefixes.yaml index 01430973ecec..65ff22364fb3 100644
Documentation/devicetree/bindings/vendor-prefixes.yaml +0 -0
@@ -1082,6 +1082,8 @@ patternProperties:
description: Sensirion AG "^sensortek,.*":
description: Sensortek Technology Corporation + "^sercomm,.*":
+ description: Sercomm (Suzhou) Corporation "^sff,.*":
description: Small Form Factor Committee "^sgd,.*":
Signature · 1 line
2.25.1
Quoted 2 lines
Best regards,
Krzysztof
Signature · 2 lines
Best regards, Mikhail
Quoted 29 lines · Mikhail Zhilkin
Quoted 7 lines · Krzysztof Kozlowski
Quoted 7 lines
Quoted 3 lines
I am sorry, but you changed now a lot in the bindings and it looks entirely different. Things previously being correct now are wrong, so rather start from your old bindings...
Looks like I'm a bit confused... I use dual "compatible" in my real dts and I realized that:
Quoted 7 lines
Quoted 6 lines
1. Therefore I have to use dual "compatible" in example too:
compatible = "sercomm,sc-partitions", "fixed-partitions";
2. When I'm trying to reuse "fixed-partitions" compatible from
fixed-partitions.yamlin my new .yaml I get "too long" errors.Yes, the
fixed-partitions.yamlwould have to be changed to allow extension.Well.
Quoted 9 lines
Quoted 8 lines
Real dts:
So, I currently found another solution - to extend
fixed-partitions.yamlwith "sercomm,sc-partitions". Is It ok from your side? Can I use this code in v3?Not really, I don't understand why do you need it
Quoted 7 lines
The main idea is keeping original Sercomm firmware behavior:
1. If dynamic partition map found then use offsets and mtd sizes stored in partition map. It's provided by "sercomm,sc-partitions" compatible.
2. If dynamic partition map doesn't exist or broken then default values (from dts) are used. It's provided by "fixed-partitions" compatible.
Then you need to adjust fixed-partitions for such case. See syscon case (all over the tree and Documentation/devicetree/bindings/mfd/syscon.yaml).
Quoted 43 lines
Quoted 3 lines
and it does not
include our previous talks.At the time, I didn't realize how important is it. Understanding began to come after dozens of experiments and checking the similar Linux patches.
Quoted 27 lines
Quoted 27 lines
diff --git
a/Documentation/devicetree/bindings/mtd/partitions/fixed-partitions.yamlb/Documentation/devicetree/bindings/mtd/partitions/fixed-partitions.yamlindexea4cace6a955..9eebe39a57fb100644Documentation/devicetree/bindings/mtd/partitions/fixed-partitions.yaml +0
-0@@ -19,7 +19,11 @@ maintainers:
properties:
compatible:
- const: fixed-partitions
+ oneOf:
+ - const: fixed-partitions
+ - items:
+ - const: sercomm,sc-partitions
+ - const: fixed-partitions
"#address-cells": trueDocumentation/devicetree/bindings/mtd/partitions/fixed-partitions.yaml +0
-0@@ -27,7 +31,18 @@ properties:
patternProperties:
"@[0-9a-f]+$":
- $ref: "partition.yaml#"
+ allOf:
+ - $ref: "partition.yaml#"
+ - if:Quoted 8 lines
Quoted 7 lines
+ properties:
+ compatible:
+ contains:
+ const: sercomm,sc-partitions
+ then:
+ properties:
+ scpart-id:It still misses vendor prefix and we agreed you don't need it, didn't we?
Quoted 2 lines
Do you mean "sercomm" vendor prefix? If so then we agreed that I include it in a separate patch:
There was some misunderstanding then. We talk here about scpart-id name. Adding vendor prefix cannot be a separate patch because it does not make much sense. You add new property with wrong name and immediately change/fix it in next patch.
No, it should have proper name since beginning. The property is not used in the kernel.
Quoted 25 lines
Link:
https://lore.kernel.org/linux-mtd/[email protected]/I'm going to send it in v3:
---
dt-bindings: Add Sercomm (Suzhou) Corporation vendor prefixUpdate
Documentation/devicetree/bindings/vendor-prefixes.yamlto include "sercomm" as a vendor prefix for "Sercomm (Suzhou) Corporation". Company website:Signed-off-by Mikhail Zhilkin <[email protected]>---
Documentation/devicetree/bindings/vendor-prefixes.yaml | 2 ++1 file changed, 2 insertions(+)Documentation/devicetree/bindings/vendor-prefixes.yaml +0
-0@@ -1082,6 +1082,8 @@ patternProperties:
Quoted 5 lines
description: Sensirion AG "^sensortek,.*":
description: Sensortek Technology Corporation + "^sercomm,.*":
+ description: Sercomm (Suzhou) Corporation
This can be separate patch, but it's separate issue...
Best regards,
Krzysztof
PATCH v2 1/2 dt-bindings: mtd: partitions: Add binding for Sercomm parser
Quoted 11 lines · Krzysztof Kozlowski
Quoted 11 lines
Quoted 9 lines
Quoted 8 lines
Real dts:
So, I currently found another solution - to extend
fixed-partitions.yamlwith "sercomm,sc-partitions". Is It ok from your side? Can I use this code in v3?Not really, I don't understand why do you need it
The main idea is keeping original Sercomm firmware behavior:
Quoted 7 lines
Quoted 5 lines
1. If dynamic partition map found then use offsets and mtd sizes stored in partition map. It's provided by "sercomm,sc-partitions" compatible.
2. If dynamic partition map doesn't exist or broken then default values (from dts) are used. It's provided by "fixed-partitions" compatible.
Then you need to adjust fixed-partitions for such case. See syscon case (all over the tree and
Documentation/devicetree/bindings/mfd/syscon.yaml).
Thanks! Here's what I got (neither errors nor warnings). I also updated the parser itself by adding the vendor prefix and tested on a real device.
.../mtd/partitions/fixed-partitions.yaml | 61 ++++++++++++++++++-
1 file changed, 59 insertions(+), 2 deletions(-)
diff --git a/Documentation/devicetree/bindings/mtd/partitions/fixed-partitions.yaml b/Documentation/devicetree/bindings/mtd/partitions/fixed-partitions.yaml index ea4cace6a955..fa457d55559b 100644
Documentation/devicetree/bindings/mtd/partitions/fixed-partitions.yaml +0 -0
@@ -17,9 +17,29 @@ description: |
maintainers:
- Rafał Miłecki <[email protected]>
+select:
+ properties:
+ compatible:
+ contains:
+ enum:
+ - fixed-partitions
+
+ required:
+ - compatible
+
properties:
compatible:
- const: fixed-partitions
+ anyOf:
+ - items:
+ - enum:
+ - sercomm,sc-partitions
+
+ - const: fixed-partitions
+
+ - contains:
+ const: fixed-partitions
+ minItems: 1
+ maxItems: 2
"#address-cells": true
@@ -27,7 +47,18 @@ properties:
patternProperties:
"@[0-9a-f]+$":
- $ref: "partition.yaml#"
+ allOf:
+ - $ref: "partition.yaml#"
+ - if:
+ properties:
+ compatible:
+ contains:
+ const: sercomm,sc-partitions
+ then:
+ properties:
+ sercomm,scpart-id:
+ description: Partition id in Sercomm partition map + $ref: /schemas/types.yaml#/definitions/uint32
required:
- "#address-cells"
@@ -119,3 +150,29 @@ examples:
};
};
};
+ - |
+ partitions {
+ compatible = "sercomm,sc-partitions", "fixed-partitions"; + #address-cells = <1>;
+ #size-cells = <1>;
+
+ partition@0 {
+ label = "u-boot";
+ reg = <0x0 0x100000>;
+ sercomm,scpart-id=<0>;
+ read-only;
+ };
+
+ partition@100000 {
+ label = "dynamic partition map";
+ reg = <0x100000 0x100000>;
+ sercomm,scpart-id = <1>;
+ };
+
+ partition@200000 {
+ label = "Factory";
+ reg = <0x200000 0x100000>;
+ sercomm,scpart-id = <2>;
+ read-only;
+ };
+ };
Signature · 1 line
2.25.1
Quoted 42 lines
Quoted 40 lines
Quoted 2 lines
and it does not
include our previous talks.At the time, I didn't realize how important is it. Understanding began to come after dozens of experiments and checking the similar Linux patches.
Quoted 27 lines
Quoted 27 lines
diff --git
a/Documentation/devicetree/bindings/mtd/partitions/fixed-partitions.yamlb/Documentation/devicetree/bindings/mtd/partitions/fixed-partitions.yamlindexea4cace6a955..9eebe39a57fb100644Documentation/devicetree/bindings/mtd/partitions/fixed-partitions.yaml +0
-0@@ -19,7 +19,11 @@ maintainers:
properties:
compatible:
- const: fixed-partitions
+ oneOf:
+ - const: fixed-partitions
+ - items:
+ - const: sercomm,sc-partitions
+ - const: fixed-partitions
"#address-cells": trueDocumentation/devicetree/bindings/mtd/partitions/fixed-partitions.yaml +0
-0@@ -27,7 +31,18 @@ properties:
patternProperties:
"@[0-9a-f]+$":
- $ref: "partition.yaml#"
+ allOf:
+ - $ref: "partition.yaml#"
+ - if:Quoted 8 lines
Quoted 7 lines
+ properties:
+ compatible:
+ contains:
+ const: sercomm,sc-partitions
+ then:
+ properties:
+ scpart-id:It still misses vendor prefix and we agreed you don't need it, didn't we?
Quoted 2 lines
Do you mean "sercomm" vendor prefix? If so then we agreed that I include it in a separate patch:
Quoted 7 lines
There was some misunderstanding then. We talk here about scpart-id name. Adding vendor prefix cannot be a separate patch because it does not make much sense. You add new property with wrong name and immediately change/fix it in next patch.
No, it should have proper name since beginning. The property is not used in the kernel.
Eureka! Thank you for your patience. I've never seen vendor prefixes for properties and didn't know that it's possible too.
Quoted 34 lines
Quoted 24 lines
Link:
https://lore.kernel.org/linux-mtd/[email protected]/I'm going to send it in v3:
---
dt-bindings: Add Sercomm (Suzhou) Corporation vendor prefixUpdate
Documentation/devicetree/bindings/vendor-prefixes.yamlto include "sercomm" as a vendor prefix for "Sercomm (Suzhou) Corporation". Company website:Signed-off-by Mikhail Zhilkin <[email protected]>---
Documentation/devicetree/bindings/vendor-prefixes.yaml | 2 ++1 file changed, 2 insertions(+)Documentation/devicetree/bindings/vendor-prefixes.yaml +0
-0@@ -1082,6 +1082,8 @@ patternProperties:Quoted 5 lines
description: Sensirion AG "^sensortek,.*":
description: Sensortek Technology Corporation + "^sercomm,.*":
+ description: Sercomm (Suzhou) CorporationThis can be separate patch, but it's separate issue...
Best regards,
Krzysztof
Signature · 2 lines
Best regards, Mikhail
Quoted 11 lines · Mikhail Zhilkin
diff --git
a/Documentation/devicetree/bindings/mtd/partitions/fixed-partitions.yamlb/Documentation/devicetree/bindings/mtd/partitions/fixed-partitions.yamlindexea4cace6a955..fa457d55559b100644Documentation/devicetree/bindings/mtd/partitions/fixed-partitions.yaml +0
-0@@ -17,9 +17,29 @@ description: |maintainers:
- Rafał Miłecki <[email protected]>
Documentation/devicetree/bindings/mtd/partitions/fixed-partitions.yaml
Quoted 9 lines
Documentation/devicetree/bindings/mtd/partitions/fixed-partitions.yaml +9
-0+select: + properties: + compatible: + contains: + enum: + - fixed-partitions + + required: + - compatible
With your approach you do not need this entire select. I pointed out to you if you wanted to take the syscon approach.
Quoted 5 lines
+
properties:
compatible:
- const: fixed-partitions
+ anyOf:
oneOf
Documentation/devicetree/bindings/mtd/partitions/fixed-partitions.yaml
Quoted 10 lines
+ - items:
+ - enum:
+ - sercomm,sc-partitionsDocumentation/devicetree/bindings/mtd/partitions/fixed-partitions.yaml +7
-0+ + - const: fixed-partitions + + - contains: + const: fixed-partitions + minItems: 1 + maxItems: 2
This is also not needed if you do no take the syscon approach.
Quoted 12 lines
"#address-cells": trueDocumentation/devicetree/bindings/mtd/partitions/fixed-partitions.yaml +0
-0@@ -27,7 +47,18 @@ properties:
patternProperties:
"@[0-9a-f]+$":
- $ref: "partition.yaml#"
+ allOf:
+ - $ref: "partition.yaml#"
+ - if:
+ properties:
Quoted 8 lines
+ compatible:
+ contains:
+ const: sercomm,sc-partitions
+ then:
+ properties:
+ sercomm,scpart-id:
+ description: Partition id in Sercomm partition map + $ref: /schemas/types.yaml#/definitions/uint32
I think we still did not clarify why do you need this ID which in all your examples increments by one. The description basically is a copy of property name, so it does not explain anything.
Documentation/devicetree/bindings/mtd/partitions/fixed-partitions.yaml examples: · line 152
Quoted 7 lines
required:
- "#address-cells"Documentation/devicetree/bindings/mtd/partitions/fixed-partitions.yaml +0
-0@@ -119,3 +150,29 @@ examples: }; }; };
Blank line.
Documentation/devicetree/bindings/mtd/partitions/fixed-partitions.yaml examples: · line 162
Quoted 10 lines
Documentation/devicetree/bindings/mtd/partitions/fixed-partitions.yaml +10
-0+ - | + partitions { + compatible = "sercomm,sc-partitions", "fixed-partitions"; + #address-cells = <1>; + #size-cells = <1>; + + partition@0 { + label = "u-boot"; + reg = <0x0 0x100000>; + sercomm,scpart-id=<0>;
Missing spaces around =.
Best regards,
Krzysztof
PATCH v2 1/2 dt-bindings: mtd: partitions: Add binding for Sercomm parser
Quoted 29 lines · Krzysztof Kozlowski
Quoted 10 lines
diff --git
a/Documentation/devicetree/bindings/mtd/partitions/fixed-partitions.yamlb/Documentation/devicetree/bindings/mtd/partitions/fixed-partitions.yamlindexea4cace6a955..fa457d55559b100644Documentation/devicetree/bindings/mtd/partitions/fixed-partitions.yaml +0
-0@@ -17,9 +17,29 @@ description: |maintainers:
- Rafał Miłecki <[email protected]>Documentation/devicetree/bindings/mtd/partitions/fixed-partitions.yaml
Quoted 9 lines
Documentation/devicetree/bindings/mtd/partitions/fixed-partitions.yaml +9
-0+select: + properties: + compatible: + contains: + enum: + - fixed-partitions + + required: + - compatibleWith your approach you do not need this entire select. I pointed out to you if you wanted to take the syscon approach.
Quoted 5 lines
+
properties:
compatible:
- const: fixed-partitions
+ anyOf:oneOf
Quoted 11 lines
Documentation/devicetree/bindings/mtd/partitions/fixed-partitions.yaml
Quoted 10 lines
+ - items:
+ - enum:
+ - sercomm,sc-partitionsDocumentation/devicetree/bindings/mtd/partitions/fixed-partitions.yaml +7
-0+ + - const: fixed-partitions + + - contains: + const: fixed-partitions + minItems: 1 + maxItems: 2This is also not needed if you do no take the syscon approach.
I tried to take into account all of your comments:
diff --git a/Documentation/devicetree/bindings/mtd/partitions/fixed-partitions.yaml b/Documentation/devicetree/bindings/mtd/partitions/fixed-partitions.yaml index ea4cace6a955..45d6a3971514 100644
Documentation/devicetree/bindings/mtd/partitions/fixed-partitions.yaml +0 -0
@@ -19,7 +19,11 @@ maintainers:
properties:
compatible:
- const: fixed-partitions
+ oneOf:
+ - const: fixed-partitions
+ - items:
+ - const: sercomm,sc-partitions
+ - const: fixed-partitions
"#address-cells": true
@@ -27,7 +31,20 @@ properties:
patternProperties:
"@[0-9a-f]+$":
- $ref: "partition.yaml#"
+ allOf:
+ - $ref: "partition.yaml#"
+ - if:
+ properties:
+ compatible:
+ contains:
+ const: sercomm,sc-partitions
+ then:
+ properties:
+ sercomm,scpart-id:
+ description: Partition id in Sercomm partition map. Parser + uses this id to get partition offset and size values from + dynamic partition map.
+ $ref: /schemas/types.yaml#/definitions/uint32
required:
- "#address-cells"
@@ -52,6 +69,7 @@ examples:
reg = <0x0100000 0x200000>;
};
};
+
- |
partitions {
compatible = "fixed-partitions";
@@ -64,6 +82,7 @@ examples:
reg = <0x00000000 0x1 0x00000000>;
};
};
+
- |
partitions {
compatible = "fixed-partitions";
@@ -82,6 +101,7 @@ examples:
reg = <0x2 0x00000000 0x1 0x00000000>;
};
};
+
- |
partitions {
compatible = "fixed-partitions";
@@ -119,3 +139,30 @@ examples:
};
};
};
+
+ - |
+ partitions {
+ compatible = "sercomm,sc-partitions", "fixed-partitions"; + #address-cells = <1>;
+ #size-cells = <1>;
+
+ partition@0 {
+ label = "u-boot";
+ reg = <0x0 0x100000>;
+ sercomm,scpart-id = <0>;
+ read-only;
+ };
+
+ partition@100000 {
+ label = "dynamic partition map";
+ reg = <0x100000 0x100000>;
+ sercomm,scpart-id = <1>;
+ };
+
+ partition@200000 {
+ label = "Factory";
+ reg = <0x200000 0x100000>;
+ sercomm,scpart-id = <2>;
+ read-only;
+ };
+ };
Signature · 1 line
2.25.1
Quoted 14 lines
"#address-cells": true
(quoted diff) +0
-0@@ -27,7 +47,18 @@ properties:
patternProperties:
"@[0-9a-f]+$":
- $ref: "partition.yaml#"
+ allOf:
+ - $ref: "partition.yaml#"
+ - if:
+ properties:
+ compatible:
+ contains:
+ const: sercomm,sc-partitions
Quoted 8 lines
+ then:
+ properties:
+ sercomm,scpart-id:
+ description: Partition id in Sercomm partition map + $ref: /schemas/types.yaml#/definitions/uint32 I think we still did not clarify why do you need this ID which in all your examples increments by one. The description basically is a copy of property name, so it does not explain anything.
I added more detailed description.
Quoted 8 lines
required:
- "#address-cells"(quoted diff) +0
-0@@ -119,3 +150,29 @@ examples: }; }; };Blank line.
Fixed. And I added blank lines between already existing examples.
Quoted 11 lines
Quoted 10 lines
+ - |
+ partitions {
+ compatible = "sercomm,sc-partitions", "fixed-partitions"; + #address-cells = <1>;
+ #size-cells = <1>;
+
+ partition@0 {
+ label = "u-boot";
+ reg = <0x00x100000>;
+ sercomm,scpart-id=<0>;Missing spaces around =.
Thanks. Fixed.
Quoted 2 lines
Best regards,
Krzysztof
Signature · 2 lines
Best regards, Mikhail
Quoted 8 lines · Mikhail Zhilkin
patternProperties:
"@[0-9a-f]+$":
- $ref: "partition.yaml#"
+ allOf:
+ - $ref: "partition.yaml#"
+ - if:
+ properties:
+ compatible:
Quoted 8 lines
+ contains:
+ const: sercomm,sc-partitions
+ then:
+ properties:
+ sercomm,scpart-id:
+ description: Partition id in Sercomm partition map. Parser + uses this id to get partition offset and size values from + dynamic partition map.
Partition offset and size values are not derived from scpart-id. I am sorry but after all these questions - it's the third time now - you never answer why do you need this property and what is it used for. From all the examples it could be simply removed and the partition map will be exactly the same.
Best regards,
Krzysztof
PATCH v2 1/2 dt-bindings: mtd: partitions: Add binding for Sercomm parser
Quoted 17 lines · Krzysztof Kozlowski
Quoted 8 lines · Mikhail Zhilkin
patternProperties:
"@[0-9a-f]+$":
- $ref: "partition.yaml#"
+ allOf:
+ - $ref: "partition.yaml#"
+ - if:
+ properties:
+ compatible:Quoted 8 lines
+ contains:
+ const: sercomm,sc-partitions
+ then:
+ properties:
+ sercomm,scpart-id:
+ description: Partition id in Sercomm partition map. Parser + uses this id to get partition offset and size values from + dynamic partition map.
Quoted 5 lines
Partition offset and size values are not derived from scpart-id. I am sorry but after all these questions - it's the third time now - you never answer why do you need this property and what is it used for. From all the examples it could be simply removed and the partition map will be exactly the same.
scpart-id is necessary to get (using mtd parser) partition offset and size from dynamic partition map (NOT from the reg property):
❯ xxd -e -c 12 -s $((0x800)) -l $((0x78)) mtd1 00000800: 00000000 00000000 00100000 ............ 0000080c: 00000001 00100000 00100000 ............ 00000818: 00000002 00200000 00100000 ...... ..... 00000824: 00000003 00300000 00100000 ......0..... 00000830: 00000004 00400000 00600000 ......@.... 0000083c: 00000005 00a00000 00600000 ........... 00000848: 00000006 01000000 02000000 ............ 00000854: 00000007 03000000 02000000 ............ 00000860: 00000008 05000000 01400000 ..........@. 0000086c: 00000009 06400000 01b80000 ......@..... scpart-id offset size
With sercomm,sc-partitions the reg property will be ignored (offset = 0x200000, size = 0x100000) and the values will be taken from partition map.
For example we have this is dts:
partition@200000 {
label = "Factory";
reg = <0x200000 0x100000>;
sercomm,scpart-id = <2>;
read-only;
};
Dynamic partition map:
scpart-id = 2; offset = 0x00200000; size = 0x00100000
00000002 00200000 00100000 ...... .....
In this example the offset and size are the same in reg and dynamic partition map. If device have bad blocks on NAND the values will be a little different. And we have to take partition offsets from partition map to avoid boot loops, wrong eeprom location and other bad things.
Is there anything that needs to be explained in more detail?
Quoted 2 lines
Best regards,
Krzysztof
Signature · 2 lines
Best regards, Mikhail
Quoted 56 lines · Mikhail Zhilkin
Quoted 17 lines · Krzysztof Kozlowski
Quoted 8 lines · Mikhail Zhilkin
patternProperties:
"@[0-9a-f]+$":
- $ref: "partition.yaml#"
+ allOf:
+ - $ref: "partition.yaml#"
+ - if:
+ properties:
+ compatible:Quoted 8 lines
+ contains:
+ const: sercomm,sc-partitions
+ then:
+ properties:
+ sercomm,scpart-id:
+ description: Partition id in Sercomm partition map. Parser + uses this id to get partition offset and size values from + dynamic partition map.Quoted 5 lines
Partition offset and size values are not derived from scpart-id. I am sorry but after all these questions - it's the third time now - you never answer why do you need this property and what is it used for. From all the examples it could be simply removed and the partition map will be exactly the same.
scpart-id is necessary to get (using mtd parser) partition offset and size from dynamic partition map (NOT from the reg property):
❯ xxd -e -c 12 -s $((0x800)) -l $((0x78)) mtd1 00000800: 00000000 00000000 00100000 ............
0000080c: 00000001 00100000 00100000 ............ 00000818: 00000002 00200000 00100000 ...... ..... 00000824: 00000003 00300000 00100000 ......0..... 00000830: 00000004 00400000 00600000 ......@.... 0000083c: 00000005 00a00000 00600000 ........... 00000848: 00000006 01000000 02000000 ............ 00000854: 00000007 03000000 02000000 ............ 00000860: 00000008 05000000 01400000 ..........@.0000086c: 00000009 06400000 01b80000 ......@..... scpart-id offset sizeWith sercomm,sc-partitions the reg property will be ignored (offset =
0x200000, size =0x100000) and the values will be taken from partition map.For example we have this is dts:
partition@200000 {
label = "Factory";
reg = <0x2000000x100000>;
sercomm,scpart-id = <2>;
read-only;
};Dynamic partition map:
scpart-id = 2; offset =
0x00200000; size =0x00100000
Quoted 8 lines
00000002 00200000 00100000 ...... .....
In this example the offset and size are the same in reg and dynamic partition map. If device have bad blocks on NAND the values will be a little different. And we have to take partition offsets from partition map to avoid boot loops, wrong eeprom location and other bad things.
Is there anything that needs to be explained in more detail?
Thanks a lot, this clarifies the topic. Looks good. Maybe you could put parts of this into the scpart-id field description?
Best regards,
Krzysztof
PATCH v2 1/2 dt-bindings: mtd: partitions: Add binding for Sercomm parser
Quoted 59 lines · Krzysztof Kozlowski
Quoted 58 lines · Mikhail Zhilkin
Quoted 17 lines · Krzysztof Kozlowski
Quoted 8 lines · Mikhail Zhilkin
patternProperties:
"@[0-9a-f]+$":
- $ref: "partition.yaml#"
+ allOf:
+ - $ref: "partition.yaml#"
+ - if:
+ properties:
+ compatible:Quoted 8 lines
+ contains:
+ const: sercomm,sc-partitions
+ then:
+ properties:
+ sercomm,scpart-id:
+ description: Partition id in Sercomm partition map. Parser + uses this id to get partition offset and size values from + dynamic partition map.Quoted 5 lines
Partition offset and size values are not derived from scpart-id. I am sorry but after all these questions - it's the third time now - you never answer why do you need this property and what is it used for. From all the examples it could be simply removed and the partition map will be exactly the same.
scpart-id is necessary to get (using mtd parser) partition offset and size from dynamic partition map (NOT from the reg property):
❯ xxd -e -c 12 -s $((0x800)) -l $((0x78)) mtd1 00000800: 00000000 00000000 00100000 ............
0000080c: 00000001 00100000 00100000 ............ 00000818: 00000002 00200000 00100000 ...... ..... 00000824: 00000003 00300000 00100000 ......0..... 00000830: 00000004 00400000 00600000 ......@.... 0000083c: 00000005 00a00000 00600000 ........... 00000848: 00000006 01000000 02000000 ............ 00000854: 00000007 03000000 02000000 ............ 00000860: 00000008 05000000 01400000 ..........@.0000086c: 00000009 06400000 01b80000 ......@..... scpart-id offset sizeWith sercomm,sc-partitions the reg property will be ignored (offset =
0x200000, size =0x100000) and the values will be taken from partition map.For example we have this is dts:
partition@200000 {
label = "Factory";
reg = <0x2000000x100000>;
sercomm,scpart-id = <2>;
read-only;
};Dynamic partition map:
scpart-id = 2; offset =
0x00200000; size =0x0010000000000002 00200000 00100000 ...... .....
Quoted 8 lines
Quoted 6 lines
In this example the offset and size are the same in reg and dynamic partition map. If device have bad blocks on NAND the values will be a little different. And we have to take partition offsets from partition map to avoid boot loops, wrong eeprom location and other bad things.
Is there anything that needs to be explained in more detail?
Thanks a lot, this clarifies the topic. Looks good. Maybe you could put parts of this into the scpart-id field description?
Thank you for you support! I updated the scpart-id description and hope this should be clear enough. If so, I'll prepare PATCH v3.
patternProperties:
"@[0-9a-f]+$":
- $ref: "partition.yaml#"
+ allOf:
+ - $ref: "partition.yaml#"
+ - if:
+ properties:
+ compatible:
+ contains:
+ const: sercomm,sc-partitions
+ then:
+ properties:
+ sercomm,scpart-id:
+ description: Partition id in Sercomm partition map. Mtd + parser uses this id to find a record in the partiton map + containing offset and size of the current partition. The + values from partition map overrides partition offset and + size defined in reg property of the dts. Frequently these + values are the same, but may differ if device has bad + eraseblocks on a flash.
+ $ref: /schemas/types.yaml#/definitions/uint32
Quoted 2 lines
Best regards,
Krzysztof
Signature · 2 lines
Best regards, Mikhail
Quoted 12 lines · Mikhail Zhilkin
Quoted 4 lines · Krzysztof Kozlowski
Quoted 3 lines
I'm not sure that "scpart-id" is necessary here. "sercomm,sc-partitions" is necessary. I'm going to add vendor prefix in a separate patch. Is this ok?
Yes.
Thanks!
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Review
Quoted 2 lines
(quoted diff) +2
-0+required: + - compatibleMissing reg.
Quoted 8 lines
Quoted 7 lines
Quoted 4 lines
reg isn't required. Parser can read partition offsets and sizes from SC PART MAP table. Or do you mean something else? All is ok without reg definition in "Example" (except the warns that reg property is missing).
reg might not be required for current implementation but it is required by devicetree for every node with unit address. Do you expect here nodes without unit addresses?
Only "partitions" node has no unit address.
This confused me. Do you have a child node named "partitions"?
Quoted 2 lines
All subnodes have unit
addresses and therefore have to have reg property.
So all of them need reg.
Quoted 5 lines
I've just realized
that "fixed-partitions.yaml" is almost my case. It looks like I can copy'n'paste "required" and "*properties".
Do you mind if I don't reinvent the wheel and reuse this good practice?
Re-using proper stuff is good, but just don't blindly copy. You need still compatible and reg in required properties.
Best regards,
Krzysztof
Rob Herring
maintainerQuoted 8 lines · Mikhail Zhilkin
Add YAML binding for Sercomm partition parser.
Signed-off-by Mikhail Zhilkin <[email protected]>--- .../mtd/partitions/sercomm,sc-partitions.yaml | 70 +++++++++++++++++++ 1 file changed, 70 insertions(+) create mode 100644 Documentation/devicetree/bindings/mtd/partitions/sercomm,sc-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:
Error: Documentation/devicetree/bindings/mtd/partitions/sercomm,sc-partitions.example.dts:21.13-14 syntax error FATAL ERROR: Unable to parse input tree
make[1]: *** [scripts/Makefile.lib:364: Documentation/devicetree/bindings/mtd/partitions/sercomm,sc-partitions.example.dtb] Error 1 make[1]: *** Waiting for unfinished jobs....
make: *** [Makefile:1401: dt_binding_check] Error 2
doc reference errors (make refcheckdocs):
See https://patchwork.ozlabs.org/patch/
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.
PATCH v2 1/2 dt-bindings: mtd: partitions: Add binding for Sercomm parser
Hello Rob,
Quoted 7 lines · Rob Herring
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.
Thanks for your great explanation how to test! I found and fixed a mistake. How I have only one WARNING:
"added, moved or deleted file(s), does MAINTAINERS need updating?"
I hope it doesn't require additional change. What do you think?
Best regards,
Mikhail
Quoted 7 lines · Mikhail Zhilkin
Hello Rob,
Quoted 4 lines · Rob Herring
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:
Quoted 5 lines
Quoted 3 lines
pip3 install dtschema --upgrade
Please check and re-submit.
Thanks for your great explanation how to test! I found and fixed a mistake.
One? Apart of broken compilation, there were other mistakes. When you run the tests, you will see all of them.
Quoted 4 lines
How I have only one WARNING:
"added, moved or deleted file(s), doesMAINTAINERSneed updating?"I hope it doesn't require additional change. What do you think?
This is not related to dt_binding_check. if you ask about checkpatch, then no, this does not require fixing.
Best regards,
Krzysztof
PATCH v2 1/2 dt-bindings: mtd: partitions: Add binding for Sercomm parser
Quoted 2 lines · Krzysztof Kozlowski
One? Apart of broken compilation, there were other mistakes. When you run the tests, you will see all of them.
I checked the first version again. It was:
- One "FATAL ERROR" (missing '{' in dts example)
- 5 warning / errors (severity not specified) "From schema"
Fixed version is here (not a single warning):
https://lore.kernel.org/linux-mtd/[email protected]/T/#ma43afb59fd1f0fab8899951005ae9ce011fbb0cc
Is it ok if I send it in PATCH v3?
Quoted 2 lines
This is not related to
dt_binding_check. if you ask about checkpatch, then no, this does not require fixing.
Yeah, it's about checkpatch. Thanks.
Quoted 2 lines
Best regards,
Krzysztof
Best regards,
Mikhail
PATCH v2 2/2 mtd: parsers: add support for Sercomm partitions
In reply to Mikhail ZhilkinThis adds an MTD partition parser for the Sercomm partition table that is used in some Beeline, Netgear and Sercomm routers.
The Sercomm partition map table contains real partition offsets, which may differ from device to device depending on the number and location of bad blocks on NAND.
This is essentially the same code as proposed by NOGUCHI Hiroshi:
--- drivers/mtd/parsers/Kconfig | 9 ++ drivers/mtd/parsers/Makefile | 1 + drivers/mtd/parsers/scpart.c | 240 +++++++++++++++++++++++++++++++++++ 3 files changed, 250 insertions(+) create mode 100644 drivers/mtd/parsers/scpart.c
drivers/mtd/parsers/Kconfig +9 -0
@@ -180,6 +180,15 @@ config MTD_REDBOOT_PARTS_READONLY endif # MTD_REDBOOT_PARTS +config MTD_SERCOMM_PARTS + tristate "Sercomm partition table parser" + depends on MTD + help + This provides partitions table parser for devices with Sercomm + partition map. This partition table contains real partition + offsets, which may differ from device to device depending on the + number and location of bad blocks on NAND. + config MTD_QCOMSMEM_PARTS tristate "Qualcomm SMEM flash partition parser" depends on QCOM_SMEM
drivers/mtd/parsers/Makefile +1 -0
@@ -10,6 +10,7 @@ ofpart-$(CONFIG_MTD_OF_PARTS_LINKSYS_NS)+= ofpart_linksys_ns.o obj-$(CONFIG_MTD_PARSER_IMAGETAG) += parser_imagetag.o obj-$(CONFIG_MTD_AFS_PARTS) += afs.o obj-$(CONFIG_MTD_PARSER_TRX) += parser_trx.o +obj-$(CONFIG_MTD_SERCOMM_PARTS) += scpart.o obj-$(CONFIG_MTD_SHARPSL_PARTS) += sharpslpart.o obj-$(CONFIG_MTD_REDBOOT_PARTS) += redboot.o obj-$(CONFIG_MTD_QCOMSMEM_PARTS) += qcomsmempart.o
drivers/mtd/parsers/scpart.c new file +240 -0
240 lines not on this page · raw message
Signature · 6 lines
2.25.1 ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/