[PATCH v2 0/2] dt-bindings: mtd: partitions: Add binding for Sercomm parser

[PATCH v2 0/2] dt-bindings: mtd: partitions: Add binding for Sercomm parser

[Mikhail Zhilkin]

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(+)

-- 
2.25.1


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

[PATCH v2 0/2] dt-bindings: mtd: partitions: Add binding for Sercomm parser

[Mikhail Zhilkin]

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(+)

-- 
2.25.1

[PATCH v2 1/2] dt-bindings: mtd: partitions: Add binding for Sercomm parser

[Mikhail Zhilkin]

Add YAML binding for Sercomm partition parser.

Signed-off-by: Mikhail Zhilkin <csharper2005@gmail.com>
---
 .../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..07ea5596200c
--- /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 <csharper2005@gmail.com>
+
+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>;
+            };
+        };
+
+        /* ... */
+
+    };
-- 
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

[Mikhail Zhilkin]

Add YAML binding for Sercomm partition parser.

Signed-off-by: Mikhail Zhilkin <csharper2005@gmail.com>
---
 .../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..07ea5596200c
--- /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 <csharper2005@gmail.com>
+
+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>;
+            };
+        };
+
+        /* ... */
+
+    };
-- 
2.25.1

Re: [PATCH v2 1/2] dt-bindings: mtd: partitions: Add binding for Sercomm parser

[Krzysztof Kozlowski]

On 06/04/2022 21:59, Mikhail Zhilkin wrote:
> Add YAML binding for Sercomm partition parser.
> 
> Signed-off-by: Mikhail Zhilkin <csharper2005@gmail.com>
> ---
>  .../mtd/partitions/sercomm,sc-partitions.yaml | 70 +++++++++++++++++++
>  1 file changed, 70 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/mtd/partitions/sercomm,sc-partitions.yaml
> 

(...)


> +
> +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.

> +
> +required:
> +  - compatible

Missing reg.

> +  - 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

Re: [PATCH v2 1/2] dt-bindings: mtd: partitions: Add binding for Sercomm parser

[Krzysztof Kozlowski]

On 06/04/2022 21:59, Mikhail Zhilkin wrote:
> Add YAML binding for Sercomm partition parser.
> 
> Signed-off-by: Mikhail Zhilkin <csharper2005@gmail.com>
> ---
>  .../mtd/partitions/sercomm,sc-partitions.yaml | 70 +++++++++++++++++++
>  1 file changed, 70 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/mtd/partitions/sercomm,sc-partitions.yaml
> 

(...)


> +
> +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.

> +
> +required:
> +  - compatible

Missing reg.

> +  - 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

______________________________________________________
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

[Mikhail Zhilkin]

Hello Krzysztof,

On 4/7/2022 10:48 AM, Krzysztof Kozlowski wrote:

(...)

>> +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.

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(+)

diff --git a/Documentation/devicetree/bindings/vendor-prefixes.yaml
b/Documentation/devicetree/bindings/vendor-prefixes.yaml
index 01430973ecec..65ff22364fb3 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.yaml
+++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml
@@ -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,.*":
--

>> +
>> +required:
>> +  - compatible
> Missing 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).

> 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 <csharper2005@gmail.com>
+
+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>;
+            };
+        };
+
+        /* ... */
+
+    };
-- 

> Best regards,
> Krzysztof

Best regards,
Mikhail

[PATCH v2 1/2] dt-bindings: mtd: partitions: Add binding for Sercomm parser

[Mikhail Zhilkin]

Hello Krzysztof,

On 4/7/2022 10:48 AM, Krzysztof Kozlowski wrote:

(...)

>> +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.

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(+)

diff --git a/Documentation/devicetree/bindings/vendor-prefixes.yaml
b/Documentation/devicetree/bindings/vendor-prefixes.yaml
index 01430973ecec..65ff22364fb3 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.yaml
+++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml
@@ -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,.*":
--

>> +
>> +required:
>> +  - compatible
> Missing 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).

> 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 <csharper2005@gmail.com>
+
+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>;
+            };
+        };
+
+        /* ... */
+
+    };
-- 

> Best regards,
> Krzysztof

Best regards,
Mikhail


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH v2 1/2] dt-bindings: mtd: partitions: Add binding for Sercomm parser

[Krzysztof Kozlowski]

On 09/04/2022 14:26, Mikhail Zhilkin wrote:
>>
>> 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.

> 
> ---
>  Documentation/devicetree/bindings/vendor-prefixes.yaml | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/vendor-prefixes.yaml
> b/Documentation/devicetree/bindings/vendor-prefixes.yaml
> index 01430973ecec..65ff22364fb3 100644
> --- a/Documentation/devicetree/bindings/vendor-prefixes.yaml
> +++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml
> @@ -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,.*":
> --
> 
>>> +
>>> +required:
>>> +  - compatible
>> Missing 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?

>> 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.

> 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

Re: [PATCH v2 1/2] dt-bindings: mtd: partitions: Add binding for Sercomm parser

[Krzysztof Kozlowski]

On 09/04/2022 14:26, Mikhail Zhilkin wrote:
>>
>> 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.

> 
> ---
>  Documentation/devicetree/bindings/vendor-prefixes.yaml | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/vendor-prefixes.yaml
> b/Documentation/devicetree/bindings/vendor-prefixes.yaml
> index 01430973ecec..65ff22364fb3 100644
> --- a/Documentation/devicetree/bindings/vendor-prefixes.yaml
> +++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml
> @@ -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,.*":
> --
> 
>>> +
>>> +required:
>>> +  - compatible
>> Missing 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?

>> 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.

> 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

______________________________________________________
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

[Mikhail Zhilkin]

On 4/9/2022 3:43 PM, Krzysztof Kozlowski wrote:

>> 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!

>>>> +required:
>>>> +  - compatible
>>> Missing 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 <csharper2005@gmail.com>
+
+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>;
+            };
+        };
+
+        /* ... */
+
+    };
-- 

(...)

> Best regards,
> Krzysztof

-- 
Best regards,
Mikhail

[PATCH v2 1/2] dt-bindings: mtd: partitions: Add binding for Sercomm parser

[Mikhail Zhilkin]

On 4/9/2022 3:43 PM, Krzysztof Kozlowski wrote:

>> 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!

>>>> +required:
>>>> +  - compatible
>>> Missing 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 <csharper2005@gmail.com>
+
+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>;
+            };
+        };
+
+        /* ... */
+
+    };
-- 

(...)

> Best regards,
> Krzysztof

-- 
Best regards,
Mikhail


______________________________________________________
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

[Mikhail Zhilkin]

On 4/9/2022 9:04 PM, Mikhail Zhilkin wrote:
>>>>> +required:
>>>>> +  - compatible
>>>> Missing 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):


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 <csharper2005@gmail.com>
+
+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>;
+            };
+        };
+
+        /* ... */
+
+    };
-- 


> Best regards,
> Mikhail

[PATCH v2 1/2] dt-bindings: mtd: partitions: Add binding for Sercomm parser

[Mikhail Zhilkin]

On 4/9/2022 9:04 PM, Mikhail Zhilkin wrote:
>>>>> +required:
>>>>> +  - compatible
>>>> Missing 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):


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 <csharper2005@gmail.com>
+
+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>;
+            };
+        };
+
+        /* ... */
+
+    };
-- 


> Best regards,
> Mikhail

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH v2 1/2] dt-bindings: mtd: partitions: Add binding for Sercomm parser

[Krzysztof Kozlowski]

On 09/04/2022 20:17, Mikhail Zhilkin wrote:
> 
> On 4/9/2022 9:04 PM, Mikhail Zhilkin wrote:
>>>>>> +required:
>>>>>> +  - compatible
>>>>> Missing 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):
> 
> 
> 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...

> 
> ---
>  .../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 <csharper2005@gmail.com>
> +
> +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"

Still missing reg.

> +
> +additionalProperties: true

This is wrong, why it became true?


Best regards,
Krzysztof

Re: [PATCH v2 1/2] dt-bindings: mtd: partitions: Add binding for Sercomm parser

[Krzysztof Kozlowski]

On 09/04/2022 20:17, Mikhail Zhilkin wrote:
> 
> On 4/9/2022 9:04 PM, Mikhail Zhilkin wrote:
>>>>>> +required:
>>>>>> +  - compatible
>>>>> Missing 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):
> 
> 
> 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...

> 
> ---
>  .../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 <csharper2005@gmail.com>
> +
> +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"

Still missing reg.

> +
> +additionalProperties: true

This is wrong, why it became true?


Best regards,
Krzysztof

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH v2 1/2] dt-bindings: mtd: partitions: Add binding for Sercomm parser

[Mikhail Zhilkin]

Hi, Krzysztof,

On 4/10/2022 11:18 AM, Krzysztof Kozlowski wrote:
> 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:

Link:
https://github.com/openwrt/openwrt/blob/edcc1a9a734bb3fcdc9242025290d3f173e71b78/target/linux/ramips/dts/mt7621_beeline_smartbox-giga.dts#L79

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
--- a/Documentation/devicetree/bindings/mtd/partitions/fixed-partitions.yaml
+++ b/Documentation/devicetree/bindings/mtd/partitions/fixed-partitions.yaml
@@ -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;
+        };
+    };
-- 
2.25.1


>> +
>> +additionalProperties: true
> This is wrong, why it became true?

You're right. Got it


> Best regards,
> Krzysztof

-- 
Best regards,
Mikhail

Re: [PATCH v2 1/2] dt-bindings: mtd: partitions: Add binding for Sercomm parser

[Mikhail Zhilkin]

Hi, Krzysztof,

On 4/10/2022 11:18 AM, Krzysztof Kozlowski wrote:
> 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:

Link:
https://github.com/openwrt/openwrt/blob/edcc1a9a734bb3fcdc9242025290d3f173e71b78/target/linux/ramips/dts/mt7621_beeline_smartbox-giga.dts#L79

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
--- a/Documentation/devicetree/bindings/mtd/partitions/fixed-partitions.yaml
+++ b/Documentation/devicetree/bindings/mtd/partitions/fixed-partitions.yaml
@@ -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;
+        };
+    };
-- 
2.25.1


>> +
>> +additionalProperties: true
> This is wrong, why it became true?

You're right. Got it


> Best regards,
> Krzysztof

-- 
Best regards,
Mikhail


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH v2 1/2] dt-bindings: mtd: partitions: Add binding for Sercomm parser

[Krzysztof Kozlowski]

On 28/04/2022 17:24, Mikhail Zhilkin wrote:
> 
> Hi, Krzysztof,
> 
> On 4/10/2022 11:18 AM, Krzysztof Kozlowski wrote:
>> 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.

Yes, the fixed-partitions.yaml would have to be changed to allow extension.

> 
> Real dts:
> 
> Link:
> https://github.com/openwrt/openwrt/blob/edcc1a9a734bb3fcdc9242025290d3f173e71b78/target/linux/ramips/dts/mt7621_beeline_smartbox-giga.dts#L79
> 
> 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?

Not really, I don't understand why do you need it and it does not
include our previous talks.

> 
> diff --git
> a/Documentation/devicetree/bindings/mtd/partitions/fixed-partitions.yaml
> b/Documentation/devicetree/bindings/mtd/partitions/fixed-partitions.yaml
> index ea4cace6a955..9eebe39a57fb 100644
> --- a/Documentation/devicetree/bindings/mtd/partitions/fixed-partitions.yaml
> +++ b/Documentation/devicetree/bindings/mtd/partitions/fixed-partitions.yaml
> @@ -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:

It still misses vendor prefix and we agreed you don't need it, didn't we?


Best regards,
Krzysztof

Re: [PATCH v2 1/2] dt-bindings: mtd: partitions: Add binding for Sercomm parser

[Krzysztof Kozlowski]

On 28/04/2022 17:24, Mikhail Zhilkin wrote:
> 
> Hi, Krzysztof,
> 
> On 4/10/2022 11:18 AM, Krzysztof Kozlowski wrote:
>> 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.

Yes, the fixed-partitions.yaml would have to be changed to allow extension.

> 
> Real dts:
> 
> Link:
> https://github.com/openwrt/openwrt/blob/edcc1a9a734bb3fcdc9242025290d3f173e71b78/target/linux/ramips/dts/mt7621_beeline_smartbox-giga.dts#L79
> 
> 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?

Not really, I don't understand why do you need it and it does not
include our previous talks.

> 
> diff --git
> a/Documentation/devicetree/bindings/mtd/partitions/fixed-partitions.yaml
> b/Documentation/devicetree/bindings/mtd/partitions/fixed-partitions.yaml
> index ea4cace6a955..9eebe39a57fb 100644
> --- a/Documentation/devicetree/bindings/mtd/partitions/fixed-partitions.yaml
> +++ b/Documentation/devicetree/bindings/mtd/partitions/fixed-partitions.yaml
> @@ -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:

It still misses vendor prefix and we agreed you don't need it, didn't we?


Best regards,
Krzysztof

______________________________________________________
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

[Mikhail Zhilkin]

On 4/29/2022 9:46 AM, Krzysztof Kozlowski wrote:

>>> 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.
> Yes, the fixed-partitions.yaml would have to be changed to allow extension.

Well.

>> Real dts:
>>
>> Link:
>> https://github.com/openwrt/openwrt/blob/edcc1a9a734bb3fcdc9242025290d3f173e71b78/target/linux/ramips/dts/mt7621_beeline_smartbox-giga.dts#L79
>>
>> 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?
> 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.

> 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.

>> diff --git
>> a/Documentation/devicetree/bindings/mtd/partitions/fixed-partitions.yaml
>> b/Documentation/devicetree/bindings/mtd/partitions/fixed-partitions.yaml
>> index ea4cace6a955..9eebe39a57fb 100644
>> --- a/Documentation/devicetree/bindings/mtd/partitions/fixed-partitions.yaml
>> +++ b/Documentation/devicetree/bindings/mtd/partitions/fixed-partitions.yaml
>> @@ -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:
> 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/1b391399-984b-7a63-3265-62ef09caec39@gmail.com/

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:
Link: https://www.sercomm.com/

Signed-off-by: Mikhail Zhilkin <csharper2005@gmail.com>
---
 Documentation/devicetree/bindings/vendor-prefixes.yaml | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/vendor-prefixes.yaml
b/Documentation/devicetree/bindings/vendor-prefixes.yaml
index 01430973ecec..65ff22364fb3 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.yaml
+++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml
@@ -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,.*":
-- 
2.25.1

> Best regards,
> Krzysztof

-- 
Best regards,
Mikhail

[PATCH v2 1/2] dt-bindings: mtd: partitions: Add binding for Sercomm parser

[Mikhail Zhilkin]

On 4/29/2022 9:46 AM, Krzysztof Kozlowski wrote:

>>> 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.
> Yes, the fixed-partitions.yaml would have to be changed to allow extension.

Well.

>> Real dts:
>>
>> Link:
>> https://github.com/openwrt/openwrt/blob/edcc1a9a734bb3fcdc9242025290d3f173e71b78/target/linux/ramips/dts/mt7621_beeline_smartbox-giga.dts#L79
>>
>> 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?
> 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.

> 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.

>> diff --git
>> a/Documentation/devicetree/bindings/mtd/partitions/fixed-partitions.yaml
>> b/Documentation/devicetree/bindings/mtd/partitions/fixed-partitions.yaml
>> index ea4cace6a955..9eebe39a57fb 100644
>> --- a/Documentation/devicetree/bindings/mtd/partitions/fixed-partitions.yaml
>> +++ b/Documentation/devicetree/bindings/mtd/partitions/fixed-partitions.yaml
>> @@ -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:
> 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/1b391399-984b-7a63-3265-62ef09caec39@gmail.com/

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:
Link: https://www.sercomm.com/

Signed-off-by: Mikhail Zhilkin <csharper2005@gmail.com>
---
 Documentation/devicetree/bindings/vendor-prefixes.yaml | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/vendor-prefixes.yaml
b/Documentation/devicetree/bindings/vendor-prefixes.yaml
index 01430973ecec..65ff22364fb3 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.yaml
+++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml
@@ -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,.*":
-- 
2.25.1

> Best regards,
> Krzysztof

-- 
Best regards,
Mikhail


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH v2 1/2] dt-bindings: mtd: partitions: Add binding for Sercomm parser

[Krzysztof Kozlowski]

On 29/04/2022 17:26, Mikhail Zhilkin wrote:
> On 4/29/2022 9:46 AM, Krzysztof Kozlowski wrote:
> 
>>>> 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.
>> Yes, the fixed-partitions.yaml would have to be changed to allow extension.
> 
> Well.
> 
>>> Real dts:
>>>
>>> Link:
>>> https://github.com/openwrt/openwrt/blob/edcc1a9a734bb3fcdc9242025290d3f173e71b78/target/linux/ramips/dts/mt7621_beeline_smartbox-giga.dts#L79
>>>
>>> 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?
>> 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.

Then you need to adjust fixed-partitions for such case. See syscon case
(all over the tree and Documentation/devicetree/bindings/mfd/syscon.yaml).

> 
>> 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.
> 
>>> diff --git
>>> a/Documentation/devicetree/bindings/mtd/partitions/fixed-partitions.yaml
>>> b/Documentation/devicetree/bindings/mtd/partitions/fixed-partitions.yaml
>>> index ea4cace6a955..9eebe39a57fb 100644
>>> --- a/Documentation/devicetree/bindings/mtd/partitions/fixed-partitions.yaml
>>> +++ b/Documentation/devicetree/bindings/mtd/partitions/fixed-partitions.yaml
>>> @@ -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:
>> 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:

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.

> 
> Link:
> https://lore.kernel.org/linux-mtd/1b391399-984b-7a63-3265-62ef09caec39@gmail.com/
> 
> 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:
> Link: https://www.sercomm.com/
> 
> Signed-off-by: Mikhail Zhilkin <csharper2005@gmail.com>
> ---
>  Documentation/devicetree/bindings/vendor-prefixes.yaml | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/vendor-prefixes.yaml
> b/Documentation/devicetree/bindings/vendor-prefixes.yaml
> index 01430973ecec..65ff22364fb3 100644
> --- a/Documentation/devicetree/bindings/vendor-prefixes.yaml
> +++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml
> @@ -1082,6 +1082,8 @@ patternProperties:
>      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

Re: [PATCH v2 1/2] dt-bindings: mtd: partitions: Add binding for Sercomm parser

[Krzysztof Kozlowski]

On 29/04/2022 17:26, Mikhail Zhilkin wrote:
> On 4/29/2022 9:46 AM, Krzysztof Kozlowski wrote:
> 
>>>> 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.
>> Yes, the fixed-partitions.yaml would have to be changed to allow extension.
> 
> Well.
> 
>>> Real dts:
>>>
>>> Link:
>>> https://github.com/openwrt/openwrt/blob/edcc1a9a734bb3fcdc9242025290d3f173e71b78/target/linux/ramips/dts/mt7621_beeline_smartbox-giga.dts#L79
>>>
>>> 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?
>> 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.

Then you need to adjust fixed-partitions for such case. See syscon case
(all over the tree and Documentation/devicetree/bindings/mfd/syscon.yaml).

> 
>> 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.
> 
>>> diff --git
>>> a/Documentation/devicetree/bindings/mtd/partitions/fixed-partitions.yaml
>>> b/Documentation/devicetree/bindings/mtd/partitions/fixed-partitions.yaml
>>> index ea4cace6a955..9eebe39a57fb 100644
>>> --- a/Documentation/devicetree/bindings/mtd/partitions/fixed-partitions.yaml
>>> +++ b/Documentation/devicetree/bindings/mtd/partitions/fixed-partitions.yaml
>>> @@ -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:
>> 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:

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.

> 
> Link:
> https://lore.kernel.org/linux-mtd/1b391399-984b-7a63-3265-62ef09caec39@gmail.com/
> 
> 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:
> Link: https://www.sercomm.com/
> 
> Signed-off-by: Mikhail Zhilkin <csharper2005@gmail.com>
> ---
>  Documentation/devicetree/bindings/vendor-prefixes.yaml | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/vendor-prefixes.yaml
> b/Documentation/devicetree/bindings/vendor-prefixes.yaml
> index 01430973ecec..65ff22364fb3 100644
> --- a/Documentation/devicetree/bindings/vendor-prefixes.yaml
> +++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml
> @@ -1082,6 +1082,8 @@ patternProperties:
>      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

______________________________________________________
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

[Mikhail Zhilkin]

On 4/29/2022 11:22 PM, Krzysztof Kozlowski wrote:

>>>> Real dts:
>>>>
>>>> Link:
>>>> https://github.com/openwrt/openwrt/blob/edcc1a9a734bb3fcdc9242025290d3f173e71b78/target/linux/ramips/dts/mt7621_beeline_smartbox-giga.dts#L79
>>>>
>>>> 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?
>>> 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.
> 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
--- a/Documentation/devicetree/bindings/mtd/partitions/fixed-partitions.yaml
+++ b/Documentation/devicetree/bindings/mtd/partitions/fixed-partitions.yaml
@@ -17,9 +17,29 @@ description: |
 maintainers:
   - Rafał Miłecki <rafal@milecki.pl>
 
+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;
+        };
+    };
-- 
2.25.1


>>> 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.
>>
>>>> diff --git
>>>> a/Documentation/devicetree/bindings/mtd/partitions/fixed-partitions.yaml
>>>> b/Documentation/devicetree/bindings/mtd/partitions/fixed-partitions.yaml
>>>> index ea4cace6a955..9eebe39a57fb 100644
>>>> --- a/Documentation/devicetree/bindings/mtd/partitions/fixed-partitions.yaml
>>>> +++ b/Documentation/devicetree/bindings/mtd/partitions/fixed-partitions.yaml
>>>> @@ -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:
>>> 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:
> 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.

>> Link:
>> https://lore.kernel.org/linux-mtd/1b391399-984b-7a63-3265-62ef09caec39@gmail.com/
>>
>> 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:
>> Link: https://www.sercomm.com/
>>
>> Signed-off-by: Mikhail Zhilkin <csharper2005@gmail.com>
>> ---
>>  Documentation/devicetree/bindings/vendor-prefixes.yaml | 2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/vendor-prefixes.yaml
>> b/Documentation/devicetree/bindings/vendor-prefixes.yaml
>> index 01430973ecec..65ff22364fb3 100644
>> --- a/Documentation/devicetree/bindings/vendor-prefixes.yaml
>> +++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml
>> @@ -1082,6 +1082,8 @@ patternProperties:
>>      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

-- 
Best regards,
Mikhail

[PATCH v2 1/2] dt-bindings: mtd: partitions: Add binding for Sercomm parser

[Mikhail Zhilkin]

On 4/29/2022 11:22 PM, Krzysztof Kozlowski wrote:

>>>> Real dts:
>>>>
>>>> Link:
>>>> https://github.com/openwrt/openwrt/blob/edcc1a9a734bb3fcdc9242025290d3f173e71b78/target/linux/ramips/dts/mt7621_beeline_smartbox-giga.dts#L79
>>>>
>>>> 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?
>>> 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.
> 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
--- a/Documentation/devicetree/bindings/mtd/partitions/fixed-partitions.yaml
+++ b/Documentation/devicetree/bindings/mtd/partitions/fixed-partitions.yaml
@@ -17,9 +17,29 @@ description: |
 maintainers:
   - Rafał Miłecki <rafal@milecki.pl>
 
+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;
+        };
+    };
-- 
2.25.1


>>> 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.
>>
>>>> diff --git
>>>> a/Documentation/devicetree/bindings/mtd/partitions/fixed-partitions.yaml
>>>> b/Documentation/devicetree/bindings/mtd/partitions/fixed-partitions.yaml
>>>> index ea4cace6a955..9eebe39a57fb 100644
>>>> --- a/Documentation/devicetree/bindings/mtd/partitions/fixed-partitions.yaml
>>>> +++ b/Documentation/devicetree/bindings/mtd/partitions/fixed-partitions.yaml
>>>> @@ -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:
>>> 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:
> 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.

>> Link:
>> https://lore.kernel.org/linux-mtd/1b391399-984b-7a63-3265-62ef09caec39@gmail.com/
>>
>> 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:
>> Link: https://www.sercomm.com/
>>
>> Signed-off-by: Mikhail Zhilkin <csharper2005@gmail.com>
>> ---
>>  Documentation/devicetree/bindings/vendor-prefixes.yaml | 2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/vendor-prefixes.yaml
>> b/Documentation/devicetree/bindings/vendor-prefixes.yaml
>> index 01430973ecec..65ff22364fb3 100644
>> --- a/Documentation/devicetree/bindings/vendor-prefixes.yaml
>> +++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml
>> @@ -1082,6 +1082,8 @@ patternProperties:
>>      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

-- 
Best regards,
Mikhail


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH v2 1/2] dt-bindings: mtd: partitions: Add binding for Sercomm parser

[Krzysztof Kozlowski]

On 30/04/2022 10:04, Mikhail Zhilkin wrote:
> 
> diff --git
> a/Documentation/devicetree/bindings/mtd/partitions/fixed-partitions.yaml
> b/Documentation/devicetree/bindings/mtd/partitions/fixed-partitions.yaml
> index ea4cace6a955..fa457d55559b 100644
> --- a/Documentation/devicetree/bindings/mtd/partitions/fixed-partitions.yaml
> +++ b/Documentation/devicetree/bindings/mtd/partitions/fixed-partitions.yaml
> @@ -17,9 +17,29 @@ description: |
>  maintainers:
>    - Rafał Miłecki <rafal@milecki.pl>
>  
> +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.

> +
>  properties:
>    compatible:
> -    const: fixed-partitions
> +    anyOf:

oneOf

> +      - items:
> +          - enum:
> +              - sercomm,sc-partitions
> +
> +          - const: fixed-partitions
> +
> +      - contains:
> +          const: fixed-partitions
> +        minItems: 1
> +        maxItems: 2

This is also not needed if you do no take the syscon approach.

>  
>    "#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

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.

>  
>  required:
>    - "#address-cells"
> @@ -119,3 +150,29 @@ examples:
>              };
>          };
>      };

Blank line.

> +  - |
> +    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

Re: [PATCH v2 1/2] dt-bindings: mtd: partitions: Add binding for Sercomm parser

[Krzysztof Kozlowski]

On 30/04/2022 10:04, Mikhail Zhilkin wrote:
> 
> diff --git
> a/Documentation/devicetree/bindings/mtd/partitions/fixed-partitions.yaml
> b/Documentation/devicetree/bindings/mtd/partitions/fixed-partitions.yaml
> index ea4cace6a955..fa457d55559b 100644
> --- a/Documentation/devicetree/bindings/mtd/partitions/fixed-partitions.yaml
> +++ b/Documentation/devicetree/bindings/mtd/partitions/fixed-partitions.yaml
> @@ -17,9 +17,29 @@ description: |
>  maintainers:
>    - Rafał Miłecki <rafal@milecki.pl>
>  
> +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.

> +
>  properties:
>    compatible:
> -    const: fixed-partitions
> +    anyOf:

oneOf

> +      - items:
> +          - enum:
> +              - sercomm,sc-partitions
> +
> +          - const: fixed-partitions
> +
> +      - contains:
> +          const: fixed-partitions
> +        minItems: 1
> +        maxItems: 2

This is also not needed if you do no take the syscon approach.

>  
>    "#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

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.

>  
>  required:
>    - "#address-cells"
> @@ -119,3 +150,29 @@ examples:
>              };
>          };
>      };

Blank line.

> +  - |
> +    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

______________________________________________________
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

[Mikhail Zhilkin]

On 4/30/2022 5:35 PM, Krzysztof Kozlowski wrote:

>> diff --git
>> a/Documentation/devicetree/bindings/mtd/partitions/fixed-partitions.yaml
>> b/Documentation/devicetree/bindings/mtd/partitions/fixed-partitions.yaml
>> index ea4cace6a955..fa457d55559b 100644
>> --- a/Documentation/devicetree/bindings/mtd/partitions/fixed-partitions.yaml
>> +++ b/Documentation/devicetree/bindings/mtd/partitions/fixed-partitions.yaml
>> @@ -17,9 +17,29 @@ description: |
>>  maintainers:
>>    - Rafał Miłecki <rafal@milecki.pl>
>>  
>> +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.
>
>> +
>>  properties:
>>    compatible:
>> -    const: fixed-partitions
>> +    anyOf:
> oneOf
>
>> +      - items:
>> +          - enum:
>> +              - sercomm,sc-partitions
>> +
>> +          - const: fixed-partitions
>> +
>> +      - contains:
>> +          const: fixed-partitions
>> +        minItems: 1
>> +        maxItems: 2
> This 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
--- a/Documentation/devicetree/bindings/mtd/partitions/fixed-partitions.yaml
+++ b/Documentation/devicetree/bindings/mtd/partitions/fixed-partitions.yaml
@@ -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;
+        };
+    };
-- 
2.25.1


>    "#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
> 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.

>  
>  required:
>    - "#address-cells"
> @@ -119,3 +150,29 @@ examples:
>              };
>          };
>      };
> Blank line.

Fixed. And I added blank lines between already existing 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>;
> Missing spaces around =.

Thanks. Fixed.

> Best regards,
> Krzysztof

-- 
Best regards,
Mikhail

[PATCH v2 1/2] dt-bindings: mtd: partitions: Add binding for Sercomm parser

[Mikhail Zhilkin]

On 4/30/2022 5:35 PM, Krzysztof Kozlowski wrote:

>> diff --git
>> a/Documentation/devicetree/bindings/mtd/partitions/fixed-partitions.yaml
>> b/Documentation/devicetree/bindings/mtd/partitions/fixed-partitions.yaml
>> index ea4cace6a955..fa457d55559b 100644
>> --- a/Documentation/devicetree/bindings/mtd/partitions/fixed-partitions.yaml
>> +++ b/Documentation/devicetree/bindings/mtd/partitions/fixed-partitions.yaml
>> @@ -17,9 +17,29 @@ description: |
>>  maintainers:
>>    - Rafał Miłecki <rafal@milecki.pl>
>>  
>> +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.
>
>> +
>>  properties:
>>    compatible:
>> -    const: fixed-partitions
>> +    anyOf:
> oneOf
>
>> +      - items:
>> +          - enum:
>> +              - sercomm,sc-partitions
>> +
>> +          - const: fixed-partitions
>> +
>> +      - contains:
>> +          const: fixed-partitions
>> +        minItems: 1
>> +        maxItems: 2
> This 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
--- a/Documentation/devicetree/bindings/mtd/partitions/fixed-partitions.yaml
+++ b/Documentation/devicetree/bindings/mtd/partitions/fixed-partitions.yaml
@@ -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;
+        };
+    };
-- 
2.25.1


>    "#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
> 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.

>  
>  required:
>    - "#address-cells"
> @@ -119,3 +150,29 @@ examples:
>              };
>          };
>      };
> Blank line.

Fixed. And I added blank lines between already existing 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>;
> Missing spaces around =.

Thanks. Fixed.

> Best regards,
> Krzysztof

-- 
Best regards,
Mikhail


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH v2 1/2] dt-bindings: mtd: partitions: Add binding for Sercomm parser

[Krzysztof Kozlowski]

On 30/04/2022 20:54, Mikhail Zhilkin wrote:
>  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.

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

Re: [PATCH v2 1/2] dt-bindings: mtd: partitions: Add binding for Sercomm parser

[Krzysztof Kozlowski]

On 30/04/2022 20:54, Mikhail Zhilkin wrote:
>  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.

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

______________________________________________________
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

[Mikhail Zhilkin]

On 5/1/2022 11:17 AM, Krzysztof Kozlowski wrote:

> On 30/04/2022 20:54, Mikhail Zhilkin wrote:
>>  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.
> 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?

> Best regards,
> Krzysztof

-- 
Best regards,
Mikhail

[PATCH v2 1/2] dt-bindings: mtd: partitions: Add binding for Sercomm parser

[Mikhail Zhilkin]

On 5/1/2022 11:17 AM, Krzysztof Kozlowski wrote:

> On 30/04/2022 20:54, Mikhail Zhilkin wrote:
>>  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.
> 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?

> Best regards,
> Krzysztof

-- 
Best regards,
Mikhail


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH v2 1/2] dt-bindings: mtd: partitions: Add binding for Sercomm parser

[Krzysztof Kozlowski]

On 01/05/2022 16:51, Mikhail Zhilkin wrote:
> On 5/1/2022 11:17 AM, Krzysztof Kozlowski wrote:
> 
>> On 30/04/2022 20:54, Mikhail Zhilkin wrote:
>>>  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.
>> 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?

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

Re: [PATCH v2 1/2] dt-bindings: mtd: partitions: Add binding for Sercomm parser

[Krzysztof Kozlowski]

On 01/05/2022 16:51, Mikhail Zhilkin wrote:
> On 5/1/2022 11:17 AM, Krzysztof Kozlowski wrote:
> 
>> On 30/04/2022 20:54, Mikhail Zhilkin wrote:
>>>  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.
>> 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?

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

______________________________________________________
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

[Mikhail Zhilkin]

On 5/1/2022 7:17 PM, Krzysztof Kozlowski wrote:

> On 01/05/2022 16:51, Mikhail Zhilkin wrote:
>> On 5/1/2022 11:17 AM, Krzysztof Kozlowski wrote:
>>
>>> On 30/04/2022 20:54, Mikhail Zhilkin wrote:
>>>>  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.
>>> 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?
> 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

> Best regards,
> Krzysztof

-- 
Best regards,
Mikhail

[PATCH v2 1/2] dt-bindings: mtd: partitions: Add binding for Sercomm parser

[Mikhail Zhilkin]

On 5/1/2022 7:17 PM, Krzysztof Kozlowski wrote:

> On 01/05/2022 16:51, Mikhail Zhilkin wrote:
>> On 5/1/2022 11:17 AM, Krzysztof Kozlowski wrote:
>>
>>> On 30/04/2022 20:54, Mikhail Zhilkin wrote:
>>>>  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.
>>> 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?
> 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

> Best regards,
> Krzysztof

-- 
Best regards,
Mikhail


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH v2 1/2] dt-bindings: mtd: partitions: Add binding for Sercomm parser

[Krzysztof Kozlowski]

On 09/04/2022 20:04, Mikhail Zhilkin wrote:
> On 4/9/2022 3:43 PM, Krzysztof Kozlowski wrote:
> 
>>> 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!
> 
>>>>> +required:
>>>>> +  - compatible
>>>> Missing 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.

This confused me. Do you have a child node named "partitions"?

> All subnodes  have unit
> addresses and therefore have to have reg property. 

So all of them need reg.

> 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

Re: [PATCH v2 1/2] dt-bindings: mtd: partitions: Add binding for Sercomm parser

[Krzysztof Kozlowski]

On 09/04/2022 20:04, Mikhail Zhilkin wrote:
> On 4/9/2022 3:43 PM, Krzysztof Kozlowski wrote:
> 
>>> 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!
> 
>>>>> +required:
>>>>> +  - compatible
>>>> Missing 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.

This confused me. Do you have a child node named "partitions"?

> All subnodes  have unit
> addresses and therefore have to have reg property. 

So all of them need reg.

> 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

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH v2 1/2] dt-bindings: mtd: partitions: Add binding for Sercomm parser

[Rob Herring]

On Wed, 06 Apr 2022 19:59:46 +0000, Mikhail Zhilkin wrote:
> Add YAML binding for Sercomm partition parser.
> 
> Signed-off-by: Mikhail Zhilkin <csharper2005@gmail.com>
> ---
>  .../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.

Re: [PATCH v2 1/2] dt-bindings: mtd: partitions: Add binding for Sercomm parser

[Rob Herring]

On Wed, 06 Apr 2022 19:59:46 +0000, Mikhail Zhilkin wrote:
> Add YAML binding for Sercomm partition parser.
> 
> Signed-off-by: Mikhail Zhilkin <csharper2005@gmail.com>
> ---
>  .../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.


______________________________________________________
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

[Mikhail Zhilkin]

Hello Rob,

On 4/7/2022 4:50 PM, Rob Herring wrote:
> 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

[PATCH v2 1/2] dt-bindings: mtd: partitions: Add binding for Sercomm parser

[Mikhail Zhilkin]

Hello Rob,

On 4/7/2022 4:50 PM, Rob Herring wrote:
> 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


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH v2 1/2] dt-bindings: mtd: partitions: Add binding for Sercomm parser

[Krzysztof Kozlowski]

On 09/04/2022 14:35, Mikhail Zhilkin wrote:
> Hello Rob,
> 
> On 4/7/2022 4:50 PM, Rob Herring wrote:
>> 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.

One? Apart of broken compilation, there were other mistakes. When you
run the tests, you will see all of them.

> 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?

This is not related to dt_binding_check. if you ask about checkpatch,
then no, this does not require fixing.

Best regards,
Krzysztof

Re: [PATCH v2 1/2] dt-bindings: mtd: partitions: Add binding for Sercomm parser

[Krzysztof Kozlowski]

On 09/04/2022 14:35, Mikhail Zhilkin wrote:
> Hello Rob,
> 
> On 4/7/2022 4:50 PM, Rob Herring wrote:
>> 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.

One? Apart of broken compilation, there were other mistakes. When you
run the tests, you will see all of them.

> 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?

This is not related to dt_binding_check. if you ask about checkpatch,
then no, this does not require fixing.

Best regards,
Krzysztof

______________________________________________________
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

[Mikhail Zhilkin]

On 4/9/2022 3:49 PM, Krzysztof Kozlowski wrote:

> 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/20220406195557.1956-1-csharper2005@gmail.com/T/#ma43afb59fd1f0fab8899951005ae9ce011fbb0cc

Is it ok if I send it in PATCH v3?


> 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.

> Best regards,
> Krzysztof

 
Best regards,
Mikhail

[PATCH v2 1/2] dt-bindings: mtd: partitions: Add binding for Sercomm parser

[Mikhail Zhilkin]

On 4/9/2022 3:49 PM, Krzysztof Kozlowski wrote:

> 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/20220406195557.1956-1-csharper2005@gmail.com/T/#ma43afb59fd1f0fab8899951005ae9ce011fbb0cc

Is it ok if I send it in PATCH v3?


> 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.

> Best regards,
> Krzysztof

 
Best regards,
Mikhail


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

[PATCH v2 2/2] mtd: parsers: add support for Sercomm partitions

[Mikhail Zhilkin]

This 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:
Link: https://github.com/openwrt/openwrt/pull/1318#issuecomment-420607394

Signed-off-by: NOGUCHI Hiroshi <drvlabo@gmail.com>
Signed-off-by: Mikhail Zhilkin <csharper2005@gmail.com>
---
 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

diff --git a/drivers/mtd/parsers/Kconfig b/drivers/mtd/parsers/Kconfig
index 23763d16e4f9..fea0b9f70e04 100644
--- a/drivers/mtd/parsers/Kconfig
+++ b/drivers/mtd/parsers/Kconfig
@@ -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
diff --git a/drivers/mtd/parsers/Makefile b/drivers/mtd/parsers/Makefile
index 2e98aa048278..2fcf0ab9e7da 100644
--- a/drivers/mtd/parsers/Makefile
+++ b/drivers/mtd/parsers/Makefile
@@ -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
diff --git a/drivers/mtd/parsers/scpart.c b/drivers/mtd/parsers/scpart.c
new file mode 100644
index 000000000000..620a465cf808
--- /dev/null
+++ b/drivers/mtd/parsers/scpart.c
@@ -0,0 +1,240 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ *    drivers/mtd/scpart.c: Sercomm Partition Parser
+ *
+ *    Copyright (C) 2018 NOGUCHI Hiroshi
+ *    Copyright (C) 2022 Mikhail Zhilkin
+ */
+
+#include <linux/kernel.h>
+#include <linux/slab.h>
+#include <linux/mtd/mtd.h>
+#include <linux/mtd/partitions.h>
+#include <linux/module.h>
+
+#define	MOD_NAME	"scpart"
+
+#ifdef pr_fmt
+#undef pr_fmt
+#endif
+
+#define pr_fmt(fmt) MOD_NAME ": " fmt
+
+static const char sc_part_magic[] = {
+	'S', 'C', 'F', 'L', 'M', 'A', 'P', 'O', 'K', '\0',
+};
+#define	PART_MAGIC_LEN	sizeof(sc_part_magic)
+
+/* assumes that all fields are set by CPU native endian */
+struct sc_part_desc {
+	uint32_t	part_id;
+	uint32_t	part_offs;
+	uint32_t	part_bytes;
+};
+#define	ID_ALREADY_FOUND	0xFFFFFFFFUL
+
+#define	MAP_OFFS_IN_BLK	0x800
+
+#define	MAP_MIRROR_NUM	2
+
+static int scpart_desc_is_valid(struct sc_part_desc *pdesc)
+{
+	return ((pdesc->part_id != 0xFFFFFFFFUL) &&
+		(pdesc->part_offs != 0xFFFFFFFFUL) &&
+		(pdesc->part_bytes != 0xFFFFFFFFUL));
+}
+
+static int scpart_scan_partmap(struct mtd_info *master, loff_t partmap_offs,
+			       struct sc_part_desc **ppdesc)
+{
+	uint8_t *buf;
+	loff_t offs;
+	size_t retlen;
+	struct sc_part_desc *pdesc = NULL;
+	struct sc_part_desc *tmpdesc;
+	int cnt = 0;
+	int res2;
+	int res = 0;
+
+	buf = kzalloc(master->erasesize, GFP_KERNEL);
+	if (!buf) {
+		res = -ENOMEM;
+		goto out;
+	}
+
+	res2 = mtd_read(master, partmap_offs, master->erasesize, &retlen, buf);
+	if (res2 || retlen != master->erasesize) {
+		res = -EIO;
+		goto free;
+	}
+
+	offs = MAP_OFFS_IN_BLK;
+	while (offs < (master->erasesize - sizeof(*tmpdesc))) {
+		tmpdesc = (struct sc_part_desc *)&(buf[offs]);
+		if (!scpart_desc_is_valid(tmpdesc))
+			break;
+		cnt++;
+		offs += sizeof(*tmpdesc);
+	}
+
+	if (cnt > 0) {
+		int bytes = cnt * sizeof(*pdesc);
+
+		pdesc = kcalloc(cnt, sizeof(*pdesc), GFP_KERNEL);
+		if (!pdesc) {
+			res = -ENOMEM;
+			goto free;
+		}
+		memcpy(pdesc, &(buf[MAP_OFFS_IN_BLK]), bytes);
+
+		*ppdesc = pdesc;
+		res = cnt;
+	}
+
+free:
+	kfree(buf);
+
+out:
+	return res;
+}
+
+static int scpart_find_partmap(struct mtd_info *master,
+			       struct sc_part_desc **ppdesc)
+{
+	loff_t offs;
+	uint8_t rdbuf[PART_MAGIC_LEN];
+	size_t retlen;
+	int magic_found = 0;
+	int res2;
+	int res = 0;
+
+	offs = 0;
+	while ((magic_found < MAP_MIRROR_NUM) &&
+			(offs < master->size) && !mtd_block_isbad(master, offs)) {
+		res2 = mtd_read(master, offs, PART_MAGIC_LEN, &retlen, rdbuf);
+		if (res2 || (retlen != PART_MAGIC_LEN)) {
+			res = -EIO;
+			goto out;
+		}
+		if (!memcmp(rdbuf, sc_part_magic, PART_MAGIC_LEN)) {
+			pr_debug("Signature found at 0x%llx\n", offs);
+			magic_found++;
+			res = scpart_scan_partmap(master, offs, ppdesc);
+			if (res > 0)
+				goto out;
+		}
+		offs += master->erasesize;
+	}
+
+out:
+	if (res > 0)
+		pr_info("Valid 'SC PART MAP' (%d partitions) found at 0x%llx\n", res, offs);
+	else
+		pr_info("No valid 'SC PART MAP'\n");
+
+	return res;
+}
+
+static int scpart_parse(struct mtd_info *master,
+			const struct mtd_partition **pparts,
+			struct mtd_part_parser_data *data)
+{
+	struct sc_part_desc *scpart_map = NULL;
+	struct mtd_partition *parts = NULL;
+	struct device_node *mtd_node;
+	struct device_node *ofpart_node;
+	struct device_node *pp;
+	const char *partname;
+	int nr_scparts;
+	int nr_parts = 0;
+	int n;
+	int res = 0;
+
+	mtd_node = mtd_get_of_node(master);
+	if (!mtd_node)
+		goto out;
+
+	ofpart_node = of_get_child_by_name(mtd_node, "partitions");
+	if (!ofpart_node)
+		goto out;
+
+	nr_scparts = scpart_find_partmap(master, &scpart_map);
+	if (nr_scparts <= 0) {
+		res = nr_scparts;
+		goto free;
+	}
+
+	parts = kcalloc(of_get_child_count(ofpart_node), sizeof(*parts),
+		GFP_KERNEL);
+	if (!parts) {
+		res = -ENOMEM;
+		goto out;
+	}
+
+	for_each_child_of_node(ofpart_node, pp) {
+		u32 scpart_id;
+
+		if (of_property_read_u32(pp, "scpart-id", &scpart_id))
+			continue;
+
+		for (n = 0 ; n < nr_scparts ; n++)
+			if ((scpart_map[n].part_id != ID_ALREADY_FOUND) &&
+					(scpart_id == scpart_map[n].part_id))
+				break;
+		if (n >= nr_scparts)
+			/* not match */
+			continue;
+
+		/* add the partition found in OF into MTD partition array */
+		parts[nr_parts].offset = scpart_map[n].part_offs;
+		parts[nr_parts].size = scpart_map[n].part_bytes;
+		parts[nr_parts].of_node = pp;
+
+		if (!of_property_read_string(pp, "label", &partname))
+			parts[nr_parts].name = partname;
+		if (of_property_read_bool(pp, "read-only"))
+			parts[nr_parts].mask_flags |= MTD_WRITEABLE;
+		if (of_property_read_bool(pp, "lock"))
+			parts[nr_parts].mask_flags |= MTD_POWERUP_LOCK;
+
+		/* mark as 'done' */
+		scpart_map[n].part_id = ID_ALREADY_FOUND;
+
+		nr_parts++;
+	}
+
+	if (nr_parts > 0) {
+		*pparts = parts;
+		res = nr_parts;
+	} else
+		pr_info("No partition in OF matches partition ID with 'SC PART MAP'.\n");
+
+	of_node_put(pp);
+
+free:
+	kfree(scpart_map);
+	if (res <= 0)
+		kfree(parts);
+
+out:
+	return res;
+}
+
+static const struct of_device_id scpart_parser_of_match_table[] = {
+	{ .compatible = "sercomm,sc-partitions" },
+	{},
+};
+MODULE_DEVICE_TABLE(of, scpart_parser_of_match_table);
+
+static struct mtd_part_parser scpart_parser = {
+	.parse_fn = scpart_parse,
+	.name = "scpart",
+	.of_match_table = scpart_parser_of_match_table,
+};
+module_mtd_part_parser(scpart_parser);
+
+/* mtd parsers will request the module by parser name */
+MODULE_ALIAS("scpart");
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("NOGUCHI Hiroshi <drvlabo@gmail.com>");
+MODULE_DESCRIPTION("Sercomm partition parser");
-- 
2.25.1


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

[PATCH v2 2/2] mtd: parsers: add support for Sercomm partitions

[Mikhail Zhilkin]

This 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:
Link: https://github.com/openwrt/openwrt/pull/1318#issuecomment-420607394

Signed-off-by: NOGUCHI Hiroshi <drvlabo@gmail.com>
Signed-off-by: Mikhail Zhilkin <csharper2005@gmail.com>
---
 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

diff --git a/drivers/mtd/parsers/Kconfig b/drivers/mtd/parsers/Kconfig
index 23763d16e4f9..fea0b9f70e04 100644
--- a/drivers/mtd/parsers/Kconfig
+++ b/drivers/mtd/parsers/Kconfig
@@ -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
diff --git a/drivers/mtd/parsers/Makefile b/drivers/mtd/parsers/Makefile
index 2e98aa048278..2fcf0ab9e7da 100644
--- a/drivers/mtd/parsers/Makefile
+++ b/drivers/mtd/parsers/Makefile
@@ -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
diff --git a/drivers/mtd/parsers/scpart.c b/drivers/mtd/parsers/scpart.c
new file mode 100644
index 000000000000..620a465cf808
--- /dev/null
+++ b/drivers/mtd/parsers/scpart.c
@@ -0,0 +1,240 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ *    drivers/mtd/scpart.c: Sercomm Partition Parser
+ *
+ *    Copyright (C) 2018 NOGUCHI Hiroshi
+ *    Copyright (C) 2022 Mikhail Zhilkin
+ */
+
+#include <linux/kernel.h>
+#include <linux/slab.h>
+#include <linux/mtd/mtd.h>
+#include <linux/mtd/partitions.h>
+#include <linux/module.h>
+
+#define	MOD_NAME	"scpart"
+
+#ifdef pr_fmt
+#undef pr_fmt
+#endif
+
+#define pr_fmt(fmt) MOD_NAME ": " fmt
+
+static const char sc_part_magic[] = {
+	'S', 'C', 'F', 'L', 'M', 'A', 'P', 'O', 'K', '\0',
+};
+#define	PART_MAGIC_LEN	sizeof(sc_part_magic)
+
+/* assumes that all fields are set by CPU native endian */
+struct sc_part_desc {
+	uint32_t	part_id;
+	uint32_t	part_offs;
+	uint32_t	part_bytes;
+};
+#define	ID_ALREADY_FOUND	0xFFFFFFFFUL
+
+#define	MAP_OFFS_IN_BLK	0x800
+
+#define	MAP_MIRROR_NUM	2
+
+static int scpart_desc_is_valid(struct sc_part_desc *pdesc)
+{
+	return ((pdesc->part_id != 0xFFFFFFFFUL) &&
+		(pdesc->part_offs != 0xFFFFFFFFUL) &&
+		(pdesc->part_bytes != 0xFFFFFFFFUL));
+}
+
+static int scpart_scan_partmap(struct mtd_info *master, loff_t partmap_offs,
+			       struct sc_part_desc **ppdesc)
+{
+	uint8_t *buf;
+	loff_t offs;
+	size_t retlen;
+	struct sc_part_desc *pdesc = NULL;
+	struct sc_part_desc *tmpdesc;
+	int cnt = 0;
+	int res2;
+	int res = 0;
+
+	buf = kzalloc(master->erasesize, GFP_KERNEL);
+	if (!buf) {
+		res = -ENOMEM;
+		goto out;
+	}
+
+	res2 = mtd_read(master, partmap_offs, master->erasesize, &retlen, buf);
+	if (res2 || retlen != master->erasesize) {
+		res = -EIO;
+		goto free;
+	}
+
+	offs = MAP_OFFS_IN_BLK;
+	while (offs < (master->erasesize - sizeof(*tmpdesc))) {
+		tmpdesc = (struct sc_part_desc *)&(buf[offs]);
+		if (!scpart_desc_is_valid(tmpdesc))
+			break;
+		cnt++;
+		offs += sizeof(*tmpdesc);
+	}
+
+	if (cnt > 0) {
+		int bytes = cnt * sizeof(*pdesc);
+
+		pdesc = kcalloc(cnt, sizeof(*pdesc), GFP_KERNEL);
+		if (!pdesc) {
+			res = -ENOMEM;
+			goto free;
+		}
+		memcpy(pdesc, &(buf[MAP_OFFS_IN_BLK]), bytes);
+
+		*ppdesc = pdesc;
+		res = cnt;
+	}
+
+free:
+	kfree(buf);
+
+out:
+	return res;
+}
+
+static int scpart_find_partmap(struct mtd_info *master,
+			       struct sc_part_desc **ppdesc)
+{
+	loff_t offs;
+	uint8_t rdbuf[PART_MAGIC_LEN];
+	size_t retlen;
+	int magic_found = 0;
+	int res2;
+	int res = 0;
+
+	offs = 0;
+	while ((magic_found < MAP_MIRROR_NUM) &&
+			(offs < master->size) && !mtd_block_isbad(master, offs)) {
+		res2 = mtd_read(master, offs, PART_MAGIC_LEN, &retlen, rdbuf);
+		if (res2 || (retlen != PART_MAGIC_LEN)) {
+			res = -EIO;
+			goto out;
+		}
+		if (!memcmp(rdbuf, sc_part_magic, PART_MAGIC_LEN)) {
+			pr_debug("Signature found at 0x%llx\n", offs);
+			magic_found++;
+			res = scpart_scan_partmap(master, offs, ppdesc);
+			if (res > 0)
+				goto out;
+		}
+		offs += master->erasesize;
+	}
+
+out:
+	if (res > 0)
+		pr_info("Valid 'SC PART MAP' (%d partitions) found at 0x%llx\n", res, offs);
+	else
+		pr_info("No valid 'SC PART MAP'\n");
+
+	return res;
+}
+
+static int scpart_parse(struct mtd_info *master,
+			const struct mtd_partition **pparts,
+			struct mtd_part_parser_data *data)
+{
+	struct sc_part_desc *scpart_map = NULL;
+	struct mtd_partition *parts = NULL;
+	struct device_node *mtd_node;
+	struct device_node *ofpart_node;
+	struct device_node *pp;
+	const char *partname;
+	int nr_scparts;
+	int nr_parts = 0;
+	int n;
+	int res = 0;
+
+	mtd_node = mtd_get_of_node(master);
+	if (!mtd_node)
+		goto out;
+
+	ofpart_node = of_get_child_by_name(mtd_node, "partitions");
+	if (!ofpart_node)
+		goto out;
+
+	nr_scparts = scpart_find_partmap(master, &scpart_map);
+	if (nr_scparts <= 0) {
+		res = nr_scparts;
+		goto free;
+	}
+
+	parts = kcalloc(of_get_child_count(ofpart_node), sizeof(*parts),
+		GFP_KERNEL);
+	if (!parts) {
+		res = -ENOMEM;
+		goto out;
+	}
+
+	for_each_child_of_node(ofpart_node, pp) {
+		u32 scpart_id;
+
+		if (of_property_read_u32(pp, "scpart-id", &scpart_id))
+			continue;
+
+		for (n = 0 ; n < nr_scparts ; n++)
+			if ((scpart_map[n].part_id != ID_ALREADY_FOUND) &&
+					(scpart_id == scpart_map[n].part_id))
+				break;
+		if (n >= nr_scparts)
+			/* not match */
+			continue;
+
+		/* add the partition found in OF into MTD partition array */
+		parts[nr_parts].offset = scpart_map[n].part_offs;
+		parts[nr_parts].size = scpart_map[n].part_bytes;
+		parts[nr_parts].of_node = pp;
+
+		if (!of_property_read_string(pp, "label", &partname))
+			parts[nr_parts].name = partname;
+		if (of_property_read_bool(pp, "read-only"))
+			parts[nr_parts].mask_flags |= MTD_WRITEABLE;
+		if (of_property_read_bool(pp, "lock"))
+			parts[nr_parts].mask_flags |= MTD_POWERUP_LOCK;
+
+		/* mark as 'done' */
+		scpart_map[n].part_id = ID_ALREADY_FOUND;
+
+		nr_parts++;
+	}
+
+	if (nr_parts > 0) {
+		*pparts = parts;
+		res = nr_parts;
+	} else
+		pr_info("No partition in OF matches partition ID with 'SC PART MAP'.\n");
+
+	of_node_put(pp);
+
+free:
+	kfree(scpart_map);
+	if (res <= 0)
+		kfree(parts);
+
+out:
+	return res;
+}
+
+static const struct of_device_id scpart_parser_of_match_table[] = {
+	{ .compatible = "sercomm,sc-partitions" },
+	{},
+};
+MODULE_DEVICE_TABLE(of, scpart_parser_of_match_table);
+
+static struct mtd_part_parser scpart_parser = {
+	.parse_fn = scpart_parse,
+	.name = "scpart",
+	.of_match_table = scpart_parser_of_match_table,
+};
+module_mtd_part_parser(scpart_parser);
+
+/* mtd parsers will request the module by parser name */
+MODULE_ALIAS("scpart");
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("NOGUCHI Hiroshi <drvlabo@gmail.com>");
+MODULE_DESCRIPTION("Sercomm partition parser");
-- 
2.25.1