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

[Mikhail Zhilkin <csharper2005@xxxxxxxxx>]

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