diff mbox series

[v4,08/17] dt-bindings: fsi: ast2600-fsi-master: Convert to json-schema

Message ID 20240429210131.373487-9-eajames@linux.ibm.com
State Superseded
Headers show
Series ARM: dts: aspeed: Add IBM P11 BMC Boards | expand

Commit Message

Eddie James April 29, 2024, 9:01 p.m. UTC
Convert to json-schema for the AST2600 FSI master documentation.

Signed-off-by: Eddie James <eajames@linux.ibm.com>
---
Changes since v3:
 - Remove quotes around compatible strings
 - Re-order allOf to below required
 - Add child node in the example
 - Change commit message to match similar commits

 .../fsi/aspeed,ast2600-fsi-master.yaml        | 81 +++++++++++++++++++
 .../bindings/fsi/fsi-master-aspeed.txt        | 36 ---------
 2 files changed, 81 insertions(+), 36 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/fsi/aspeed,ast2600-fsi-master.yaml
 delete mode 100644 Documentation/devicetree/bindings/fsi/fsi-master-aspeed.txt

Comments

Krzysztof Kozlowski April 30, 2024, 7:04 a.m. UTC | #1
On 29/04/2024 23:01, Eddie James wrote:
> Convert to json-schema for the AST2600 FSI master documentation.

Please mention all the changes from pure conversion.

> 
> Signed-off-by: Eddie James <eajames@linux.ibm.com>
> ---
> Changes since v3:
>  - Remove quotes around compatible strings
>  - Re-order allOf to below required
>  - Add child node in the example
>  - Change commit message to match similar commits
> 
>  .../fsi/aspeed,ast2600-fsi-master.yaml        | 81 +++++++++++++++++++
>  .../bindings/fsi/fsi-master-aspeed.txt        | 36 ---------
>  2 files changed, 81 insertions(+), 36 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/fsi/aspeed,ast2600-fsi-master.yaml
>  delete mode 100644 Documentation/devicetree/bindings/fsi/fsi-master-aspeed.txt
> 
> diff --git a/Documentation/devicetree/bindings/fsi/aspeed,ast2600-fsi-master.yaml b/Documentation/devicetree/bindings/fsi/aspeed,ast2600-fsi-master.yaml
> new file mode 100644
> index 000000000000..fcf7c4b93b78
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/fsi/aspeed,ast2600-fsi-master.yaml
> @@ -0,0 +1,81 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/fsi/aspeed,ast2600-fsi-master.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Aspeed FSI master
> +
> +maintainers:
> +  - Eddie James <eajames@linux.ibm.com>
> +
> +description:
> +  The AST2600 and later contain two identical FSI masters. They share a
> +  clock and have a separate interrupt line and output pins.
> +
> +properties:
> +  compatible:
> +    enum:
> +      - aspeed,ast2600-fsi-master
> +      - aspeed,ast2700-fsi-master

There was no such compatible before.

How does this even validate? Where is fsi-master? You dropped a
compatible without any explanation.

> +
> +  reg:
> +    maxItems: 1
> +
> +  clocks:
> +    maxItems: 1
> +


Best regards,
Krzysztof
Eddie James May 1, 2024, 4:12 p.m. UTC | #2
On 4/30/24 02:04, Krzysztof Kozlowski wrote:
> On 29/04/2024 23:01, Eddie James wrote:
>> Convert to json-schema for the AST2600 FSI master documentation.
> Please mention all the changes from pure conversion.


Sure.


>
>> Signed-off-by: Eddie James <eajames@linux.ibm.com>
>> ---
>> Changes since v3:
>>   - Remove quotes around compatible strings
>>   - Re-order allOf to below required
>>   - Add child node in the example
>>   - Change commit message to match similar commits
>>
>>   .../fsi/aspeed,ast2600-fsi-master.yaml        | 81 +++++++++++++++++++
>>   .../bindings/fsi/fsi-master-aspeed.txt        | 36 ---------
>>   2 files changed, 81 insertions(+), 36 deletions(-)
>>   create mode 100644 Documentation/devicetree/bindings/fsi/aspeed,ast2600-fsi-master.yaml
>>   delete mode 100644 Documentation/devicetree/bindings/fsi/fsi-master-aspeed.txt
>>
>> diff --git a/Documentation/devicetree/bindings/fsi/aspeed,ast2600-fsi-master.yaml b/Documentation/devicetree/bindings/fsi/aspeed,ast2600-fsi-master.yaml
>> new file mode 100644
>> index 000000000000..fcf7c4b93b78
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/fsi/aspeed,ast2600-fsi-master.yaml
>> @@ -0,0 +1,81 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/fsi/aspeed,ast2600-fsi-master.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Aspeed FSI master
>> +
>> +maintainers:
>> +  - Eddie James <eajames@linux.ibm.com>
>> +
>> +description:
>> +  The AST2600 and later contain two identical FSI masters. They share a
>> +  clock and have a separate interrupt line and output pins.
>> +
>> +properties:
>> +  compatible:
>> +    enum:
>> +      - aspeed,ast2600-fsi-master
>> +      - aspeed,ast2700-fsi-master
> There was no such compatible before.
>
> How does this even validate? Where is fsi-master? You dropped a
> compatible without any explanation.


I can make it a separate change to add ast2700.


I suppose I don't understand having two compatibles... Aspeed master 
shouldn't use "fsi-master" as that is too generic, right? Why wouldn't 
it validate? Devicetrees using "fsi-master" also use 
"aspeed,ast2600-fsi-master" so they should be OK...


>
>> +
>> +  reg:
>> +    maxItems: 1
>> +
>> +  clocks:
>> +    maxItems: 1
>> +
>
> Best regards,
> Krzysztof
>
Krzysztof Kozlowski May 4, 2024, 11:55 a.m. UTC | #3
On 01/05/2024 18:12, Eddie James wrote:
> 
> On 4/30/24 02:04, Krzysztof Kozlowski wrote:
>> On 29/04/2024 23:01, Eddie James wrote:
>>> Convert to json-schema for the AST2600 FSI master documentation.
>> Please mention all the changes from pure conversion.
> 
> 
> Sure.
> 
> 
>>
>>> Signed-off-by: Eddie James <eajames@linux.ibm.com>
>>> ---
>>> Changes since v3:
>>>   - Remove quotes around compatible strings
>>>   - Re-order allOf to below required
>>>   - Add child node in the example
>>>   - Change commit message to match similar commits
>>>
>>>   .../fsi/aspeed,ast2600-fsi-master.yaml        | 81 +++++++++++++++++++
>>>   .../bindings/fsi/fsi-master-aspeed.txt        | 36 ---------
>>>   2 files changed, 81 insertions(+), 36 deletions(-)
>>>   create mode 100644 Documentation/devicetree/bindings/fsi/aspeed,ast2600-fsi-master.yaml
>>>   delete mode 100644 Documentation/devicetree/bindings/fsi/fsi-master-aspeed.txt
>>>
>>> diff --git a/Documentation/devicetree/bindings/fsi/aspeed,ast2600-fsi-master.yaml b/Documentation/devicetree/bindings/fsi/aspeed,ast2600-fsi-master.yaml
>>> new file mode 100644
>>> index 000000000000..fcf7c4b93b78
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/fsi/aspeed,ast2600-fsi-master.yaml
>>> @@ -0,0 +1,81 @@
>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>>> +%YAML 1.2
>>> +---
>>> +$id: http://devicetree.org/schemas/fsi/aspeed,ast2600-fsi-master.yaml#
>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>> +
>>> +title: Aspeed FSI master
>>> +
>>> +maintainers:
>>> +  - Eddie James <eajames@linux.ibm.com>
>>> +
>>> +description:
>>> +  The AST2600 and later contain two identical FSI masters. They share a
>>> +  clock and have a separate interrupt line and output pins.
>>> +
>>> +properties:
>>> +  compatible:
>>> +    enum:
>>> +      - aspeed,ast2600-fsi-master
>>> +      - aspeed,ast2700-fsi-master
>> There was no such compatible before.
>>
>> How does this even validate? Where is fsi-master? You dropped a
>> compatible without any explanation.
> 
> 
> I can make it a separate change to add ast2700.
> 
> 
> I suppose I don't understand having two compatibles... Aspeed master 
> shouldn't use "fsi-master" as that is too generic, right? Why wouldn't 

Not necessarily, depends. Dropping it silently is confusing. What about
other users? firmware, bootloaders, out-of-tree, other OS? Did you
investigate all of them?

> it validate? Devicetrees using "fsi-master" also use 
> "aspeed,ast2600-fsi-master" so they should be OK...

No, because the compatibles do not match. Run validation and you will
see the errors.

I am fine with dropping such compatible, which is not used by current
kernel ABI, but first DTS must be fixed and second some explanation and
justification is needed.

Best regards,
Krzysztof
Eddie James May 14, 2024, 3:38 p.m. UTC | #4
On 5/4/24 06:55, Krzysztof Kozlowski wrote:
> On 01/05/2024 18:12, Eddie James wrote:
>> On 4/30/24 02:04, Krzysztof Kozlowski wrote:
>>> On 29/04/2024 23:01, Eddie James wrote:
>>>> Convert to json-schema for the AST2600 FSI master documentation.
>>> Please mention all the changes from pure conversion.
>>
>> Sure.
>>
>>
>>>> Signed-off-by: Eddie James <eajames@linux.ibm.com>
>>>> ---
>>>> Changes since v3:
>>>>    - Remove quotes around compatible strings
>>>>    - Re-order allOf to below required
>>>>    - Add child node in the example
>>>>    - Change commit message to match similar commits
>>>>
>>>>    .../fsi/aspeed,ast2600-fsi-master.yaml        | 81 +++++++++++++++++++
>>>>    .../bindings/fsi/fsi-master-aspeed.txt        | 36 ---------
>>>>    2 files changed, 81 insertions(+), 36 deletions(-)
>>>>    create mode 100644 Documentation/devicetree/bindings/fsi/aspeed,ast2600-fsi-master.yaml
>>>>    delete mode 100644 Documentation/devicetree/bindings/fsi/fsi-master-aspeed.txt
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/fsi/aspeed,ast2600-fsi-master.yaml b/Documentation/devicetree/bindings/fsi/aspeed,ast2600-fsi-master.yaml
>>>> new file mode 100644
>>>> index 000000000000..fcf7c4b93b78
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/fsi/aspeed,ast2600-fsi-master.yaml
>>>> @@ -0,0 +1,81 @@
>>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>>>> +%YAML 1.2
>>>> +---
>>>> +$id: http://devicetree.org/schemas/fsi/aspeed,ast2600-fsi-master.yaml#
>>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>>> +
>>>> +title: Aspeed FSI master
>>>> +
>>>> +maintainers:
>>>> +  - Eddie James <eajames@linux.ibm.com>
>>>> +
>>>> +description:
>>>> +  The AST2600 and later contain two identical FSI masters. They share a
>>>> +  clock and have a separate interrupt line and output pins.
>>>> +
>>>> +properties:
>>>> +  compatible:
>>>> +    enum:
>>>> +      - aspeed,ast2600-fsi-master
>>>> +      - aspeed,ast2700-fsi-master
>>> There was no such compatible before.
>>>
>>> How does this even validate? Where is fsi-master? You dropped a
>>> compatible without any explanation.
>>
>> I can make it a separate change to add ast2700.
>>
>>
>> I suppose I don't understand having two compatibles... Aspeed master
>> shouldn't use "fsi-master" as that is too generic, right? Why wouldn't
> Not necessarily, depends. Dropping it silently is confusing. What about
> other users? firmware, bootloaders, out-of-tree, other OS? Did you
> investigate all of them?


The old format file actually only used fsi-master in the example, not in 
the actual properties description. So I didn't really drop a compatible. 
Device trees using "fsi-master" are just buggy and should be fixed 
eventually, but I don't think it needs to be part of this series.


Thanks,

Eddie



>
>> it validate? Devicetrees using "fsi-master" also use
>> "aspeed,ast2600-fsi-master" so they should be OK...
> No, because the compatibles do not match. Run validation and you will
> see the errors.
>
> I am fine with dropping such compatible, which is not used by current
> kernel ABI, but first DTS must be fixed and second some explanation and
> justification is needed.
>
> Best regards,
> Krzysztof
>
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/fsi/aspeed,ast2600-fsi-master.yaml b/Documentation/devicetree/bindings/fsi/aspeed,ast2600-fsi-master.yaml
new file mode 100644
index 000000000000..fcf7c4b93b78
--- /dev/null
+++ b/Documentation/devicetree/bindings/fsi/aspeed,ast2600-fsi-master.yaml
@@ -0,0 +1,81 @@ 
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/fsi/aspeed,ast2600-fsi-master.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Aspeed FSI master
+
+maintainers:
+  - Eddie James <eajames@linux.ibm.com>
+
+description:
+  The AST2600 and later contain two identical FSI masters. They share a
+  clock and have a separate interrupt line and output pins.
+
+properties:
+  compatible:
+    enum:
+      - aspeed,ast2600-fsi-master
+      - aspeed,ast2700-fsi-master
+
+  reg:
+    maxItems: 1
+
+  clocks:
+    maxItems: 1
+
+  cfam-reset-gpios:
+    maxItems: 1
+    description:
+      Output GPIO pin for CFAM reset
+
+  fsi-routing-gpios:
+    maxItems: 1
+    description:
+      Output GPIO pin for setting the FSI mux (internal or cabled)
+
+  fsi-mux-gpios:
+    maxItems: 1
+    description:
+      Input GPIO pin for detecting the desired FSI mux state
+
+  interrupts:
+    maxItems: 1
+
+required:
+  - compatible
+  - reg
+  - clocks
+  - interrupts
+
+allOf:
+  - $ref: fsi-controller.yaml#
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/clock/ast2600-clock.h>
+    #include <dt-bindings/gpio/aspeed-gpio.h>
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    fsi-master@1e79b000 {
+        compatible = "aspeed,ast2600-fsi-master";
+        reg = <0x1e79b000 0x94>;
+        #address-cells = <2>;
+        #size-cells = <0>;
+        interrupts = <GIC_SPI 100 IRQ_TYPE_LEVEL_HIGH>;
+        pinctrl-names = "default";
+        pinctrl-0 = <&pinctrl_fsi1_default>;
+        clocks = <&syscon ASPEED_CLK_GATE_FSICLK>;
+        fsi-routing-gpios = <&gpio0 ASPEED_GPIO(Q, 7) GPIO_ACTIVE_HIGH>;
+        fsi-mux-gpios = <&gpio0 ASPEED_GPIO(B, 0) GPIO_ACTIVE_HIGH>;
+        cfam-reset-gpios = <&gpio0 ASPEED_GPIO(Q, 0) GPIO_ACTIVE_LOW>;
+
+        cfam@0,0 {
+            reg = <0 0>;
+            #address-cells = <1>;
+            #size-cells = <1>;
+            chip-id = <0>;
+        };
+    };
diff --git a/Documentation/devicetree/bindings/fsi/fsi-master-aspeed.txt b/Documentation/devicetree/bindings/fsi/fsi-master-aspeed.txt
deleted file mode 100644
index 9853fefff5d8..000000000000
--- a/Documentation/devicetree/bindings/fsi/fsi-master-aspeed.txt
+++ /dev/null
@@ -1,36 +0,0 @@ 
-Device-tree bindings for AST2600 FSI master
--------------------------------------------
-
-The AST2600 contains two identical FSI masters. They share a clock and have a
-separate interrupt line and output pins.
-
-Required properties:
- - compatible: "aspeed,ast2600-fsi-master"
- - reg: base address and length
- - clocks: phandle and clock number
- - interrupts: platform dependent interrupt description
- - pinctrl-0: phandle to pinctrl node
- - pinctrl-names: pinctrl state
-
-Optional properties:
- - cfam-reset-gpios: GPIO for CFAM reset
-
- - fsi-routing-gpios: GPIO for setting the FSI mux (internal or cabled)
- - fsi-mux-gpios: GPIO for detecting the desired FSI mux state
-
-
-Examples:
-
-    fsi-master {
-        compatible = "aspeed,ast2600-fsi-master", "fsi-master";
-        reg = <0x1e79b000 0x94>;
-	interrupts = <GIC_SPI 100 IRQ_TYPE_LEVEL_HIGH>;
-	pinctrl-names = "default";
-	pinctrl-0 = <&pinctrl_fsi1_default>;
-	clocks = <&syscon ASPEED_CLK_GATE_FSICLK>;
-
-	fsi-routing-gpios = <&gpio0 ASPEED_GPIO(Q, 7) GPIO_ACTIVE_HIGH>;
-	fsi-mux-gpios = <&gpio0 ASPEED_GPIO(B, 0) GPIO_ACTIVE_HIGH>;
-
-	cfam-reset-gpios = <&gpio0 ASPEED_GPIO(Q, 0) GPIO_ACTIVE_LOW>;
-    };