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 <[email protected]>
+select:
+ properties:
+ compatible:
+ contains:
+ enum:
+ - fixed-partitions
+
+ required:
+ - compatible
+
properties:
compatible:
- const: fixed-partitions
+ anyOf:
+ - items:
+ - enum:
+ - sercomm,sc-partitions
+
+ - const: fixed-partitions
+
+ - contains:
+ const: fixed-partitions
+ minItems: 1
+ maxItems: 2
"#address-cells": true
@@ -27,7 +47,18 @@ properties:
patternProperties:
"@[0-9a-f]+$":
- $ref: "partition.yaml#"
+ allOf:
+ - $ref: "partition.yaml#"
+ - if:
+ properties:
+ compatible:
+ contains:
+ const: sercomm,sc-partitions
+ then:
+ properties:
+ sercomm,scpart-id:
+ description: Partition id in Sercomm partition map
+ $ref: /schemas/types.yaml#/definitions/uint32
required:
- "#address-cells"
@@ -119,3 +150,29 @@ examples:
};
};
};
+ - |
+ partitions {
+ compatible = "sercomm,sc-partitions", "fixed-partitions";
+ #address-cells = <1>;
+ #size-cells = <1>;
+
+ partition@0 {
+ label = "u-boot";
+ reg = <0x0 0x100000>;
+ sercomm,scpart-id=<0>;
+ read-only;
+ };
+
+ partition@100000 {
+ label = "dynamic partition map";
+ reg = <0x100000 0x100000>;
+ sercomm,scpart-id = <1>;
+ };
+
+ partition@200000 {
+ label = "Factory";
+ reg = <0x200000 0x100000>;
+ sercomm,scpart-id = <2>;
+ read-only;
+ };
+ };
--
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/[email protected]/
>>
>> I'm going to send it in v3:
>>
>> ---
>> dt-bindings: Add Sercomm (Suzhou) Corporation vendor prefix
>>
>> Update Documentation/devicetree/bindings/vendor-prefixes.yaml to include
>> "sercomm" as a vendor prefix for "Sercomm (Suzhou) Corporation".
>> Company website:
>> Link: https://www.sercomm.com/
>>
>> Signed-off-by: Mikhail Zhilkin <[email protected]>
>> ---
>> 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
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 <[email protected]>
>
> +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