[PATCH v2] spi: dt-bindings: jcore,spi: convert spi-jcore to dtschema

[PATCH v2] spi: dt-bindings: jcore,spi: convert spi-jcore to dtschema

[Kousik Sanagavarapu]

Convert existing bindings of J-Core spi2 to dtschema.

No new properties are added.

Signed-off-by: Kousik Sanagavarapu <five231003@gmail.com>
---
Changes since v1:
- changed the subject line to conform.
- dropped desc for "clock" and "clock-names" properties.
- cleaned up stuff.

Thanks for the review Krzysztof.

 .../devicetree/bindings/spi/jcore,spi.txt     | 34 ------------
 .../devicetree/bindings/spi/jcore,spi.yaml    | 53 +++++++++++++++++++
 2 files changed, 53 insertions(+), 34 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/spi/jcore,spi.txt
 create mode 100644 Documentation/devicetree/bindings/spi/jcore,spi.yaml

diff --git a/Documentation/devicetree/bindings/spi/jcore,spi.txt b/Documentation/devicetree/bindings/spi/jcore,spi.txt
deleted file mode 100644
index 93936d16e139..000000000000
--- a/Documentation/devicetree/bindings/spi/jcore,spi.txt
+++ /dev/null
@@ -1,34 +0,0 @@
-J-Core SPI master
-
-Required properties:
-
-- compatible: Must be "jcore,spi2".
-
-- reg: Memory region for registers.
-
-- #address-cells: Must be 1.
-
-- #size-cells: Must be 0.
-
-Optional properties:
-
-- clocks: If a phandle named "ref_clk" is present, SPI clock speed
-  programming is relative to the frequency of the indicated clock.
-  Necessary only if the input clock rate is something other than a
-  fixed 50 MHz.
-
-- clock-names: Clock names, one for each phandle in clocks.
-
-See spi-bus.txt for additional properties not specific to this device.
-
-Example:
-
-spi@40 {
-	compatible = "jcore,spi2";
-	#address-cells = <1>;
-	#size-cells = <0>;
-	reg = <0x40 0x8>;
-	spi-max-frequency = <25000000>;
-	clocks = <&bus_clk>;
-	clock-names = "ref_clk";
-}
diff --git a/Documentation/devicetree/bindings/spi/jcore,spi.yaml b/Documentation/devicetree/bindings/spi/jcore,spi.yaml
new file mode 100644
index 000000000000..b8ec3adaac8d
--- /dev/null
+++ b/Documentation/devicetree/bindings/spi/jcore,spi.yaml
@@ -0,0 +1,53 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+
+$id: http://devicetree.org/schemas/spi/jcore,spi.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: J-Core SPI controller
+
+description:
+  The J-Core "spi2" device is a PIO-based SPI controller which used to
+  perform byte-at-a-time transfers between the CPU and itself.
+
+maintainers:
+  - Kousik Sanagavarapu <five231003@gmail.com>
+
+allOf:
+  - $ref: spi-controller.yaml#
+
+properties:
+  compatible:
+    const: jcore,spi2
+
+  reg:
+    maxItems: 1
+
+  clocks:
+    maxItems: 1
+
+  clock-names:
+    items:
+      - const: ref_clk
+
+  spi-max-frequency:
+    $ref: /schemas/types.yaml#/definitions/uint32
+
+required:
+  - compatible
+  - reg
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    spi@40 {
+        compatible = "jcore,spi2";
+        reg = <0x40 0x8>;
+        #address-cells = <1>;
+        #size-cells = <0>;
+        spi-max-frequency = <25000000>;
+        clocks = <&bus_clk>;
+        clock-names = "ref_clk";
+    };
-- 
2.44.0.273.g4bc5b65358.dirty

Re: [PATCH v2] spi: dt-bindings: jcore,spi: convert spi-jcore to dtschema

[Krzysztof Kozlowski]

On 21/03/2024 19:02, Kousik Sanagavarapu wrote:
> Convert existing bindings of J-Core spi2 to dtschema.
> 
> No new properties are added.
> 
> Signed-off-by: Kousik Sanagavarapu <five231003@gmail.com>
> ---
> Changes since v1:
> - changed the subject line to conform.
> - dropped desc for "clock" and "clock-names" properties.
> - cleaned up stuff.

You miss many other changes... Some unusal properties appeared.

...

> +---
> +
> +$id: http://devicetree.org/schemas/spi/jcore,spi.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: J-Core SPI controller
> +
> +description:
> +  The J-Core "spi2" device is a PIO-based SPI controller which used to
> +  perform byte-at-a-time transfers between the CPU and itself.
> +
> +maintainers:
> +  - Kousik Sanagavarapu <five231003@gmail.com>
> +
> +allOf:
> +  - $ref: spi-controller.yaml#
> +
> +properties:
> +  compatible:
> +    const: jcore,spi2
> +
> +  reg:
> +    maxItems: 1
> +
> +  clocks:
> +    maxItems: 1
> +
> +  clock-names:
> +    items:
> +      - const: ref_clk
> +
> +  spi-max-frequency:
> +    $ref: /schemas/types.yaml#/definitions/uint32

No, drop. From which other SPI binding did you take it? I asked you to
look at existing code.


Best regards,
Krzysztof

[PATCH v2] spi: dt-bindings: jcore,spi: convert spi-jcore to dtschema

[Kousik Sanagavarapu]

On Fri, 22 Mar 2024, 11:33 Krzysztof Kozlowski,
<krzysztof.kozlowski@linaro.org> wrote:
>
> On 21/03/2024 19:02, Kousik Sanagavarapu wrote:
>>
>> +  spi-max-frequency:
>> +    $ref: /schemas/types.yaml#/definitions/uint32
>
> No, drop. From which other SPI binding did you take it? I asked you to
> look at existing code.


Without this, "make dt_binding_check" would break though, right at the
position in the example where "spi-max-frequency" is used.  That was
also the reason why additionalProperties was set to true in the last
iteration, but after reading the doc more carefully I realized that was
wrong after you pointed it out.

I followed along bindings/spi/nvidia,tegra114-spi.yaml.

(Sorry for the HTML mail ping, I'm replying from mobile)

Re: [PATCH v2] spi: dt-bindings: jcore,spi: convert spi-jcore to dtschema

[Krzysztof Kozlowski]

On 22/03/2024 07:23, Kousik Sanagavarapu wrote:
> On Fri, 22 Mar 2024, 11:33 Krzysztof Kozlowski, <
> krzysztof.kozlowski@linaro.org> wrote:
> 
>> On 21/03/2024 19:02, Kousik Sanagavarapu wrote:
>>
>>> +  spi-max-frequency:
>>> +    $ref: /schemas/types.yaml#/definitions/uint32
>>
>> No, drop. From which other SPI binding did you take it? I asked you to
>> look at existing code.
>>
> 
> Without this, "make dt_binding_check" would break though, right at the
> position in the example where "spi-max-frequency" is used.  That was
> also the reason why additionalProperties was set to true in the last
> iteration, but after reading the doc more carefully I realized that was
> wrong after you pointed it out.
> 
> I followed along bindings/spi/nvidia,tegra114-spi.yaml.

OK, you are right, the property is used here in controller node, however
Linux driver never parsed it. It was never used, so I propose to drop it
from the binding and example. You can mention in commit msg that
spi-max-frequency was not documented thus you drop it from the example.

DTS should be fixed as well. I'll send a patch for it.

Best regards,
Krzysztof

Re: [PATCH v2] spi: dt-bindings: jcore,spi: convert spi-jcore to dtschema

[Krzysztof Kozlowski]

On 22/03/2024 07:34, Krzysztof Kozlowski wrote:
> On 22/03/2024 07:23, Kousik Sanagavarapu wrote:
>> On Fri, 22 Mar 2024, 11:33 Krzysztof Kozlowski, <
>> krzysztof.kozlowski@linaro.org> wrote:
>>
>>> On 21/03/2024 19:02, Kousik Sanagavarapu wrote:
>>>
>>>> +  spi-max-frequency:
>>>> +    $ref: /schemas/types.yaml#/definitions/uint32
>>>
>>> No, drop. From which other SPI binding did you take it? I asked you to
>>> look at existing code.
>>>
>>
>> Without this, "make dt_binding_check" would break though, right at the
>> position in the example where "spi-max-frequency" is used.  That was
>> also the reason why additionalProperties was set to true in the last
>> iteration, but after reading the doc more carefully I realized that was
>> wrong after you pointed it out.
>>
>> I followed along bindings/spi/nvidia,tegra114-spi.yaml.
> 
> OK, you are right, the property is used here in controller node, however
> Linux driver never parsed it. It was never used, so I propose to drop it
> from the binding and example. You can mention in commit msg that
> spi-max-frequency was not documented thus you drop it from the example.
> 
> DTS should be fixed as well. I'll send a patch for it.

Cc Daniel,

BTW, J2 core is rather odd platform to work on... Even cross compiling
and building that DTB is tricky. If I failed, I have doubts that you
tested the DTS with your binding.

This applies to all GSoC or some Linux Mentorship programs: I suggest to
choose for conversion bindings with more users and bigger possible
impact. So first I would look at ARM64 and ARMv7 platforms. We still
have around 1000 and 3500 unique warnings about undocumented compatibles
for ARM64 defconfig and ARM multi_v7! That's the platforms you should
choose.

Not SuperH, ARC, or whatever with only one DTS which is difficult to
build for regular developer.

Best regards,
Krzysztof

Re: [PATCH v2] spi: dt-bindings: jcore,spi: convert spi-jcore to dtschema

[Rob Herring]

On Fri, Mar 22, 2024 at 07:49:57AM +0100, Krzysztof Kozlowski wrote:
> On 22/03/2024 07:34, Krzysztof Kozlowski wrote:
> > On 22/03/2024 07:23, Kousik Sanagavarapu wrote:
> >> On Fri, 22 Mar 2024, 11:33 Krzysztof Kozlowski, <
> >> krzysztof.kozlowski@linaro.org> wrote:
> >>
> >>> On 21/03/2024 19:02, Kousik Sanagavarapu wrote:
> >>>
> >>>> +  spi-max-frequency:
> >>>> +    $ref: /schemas/types.yaml#/definitions/uint32
> >>>
> >>> No, drop. From which other SPI binding did you take it? I asked you to
> >>> look at existing code.
> >>>
> >>
> >> Without this, "make dt_binding_check" would break though, right at the
> >> position in the example where "spi-max-frequency" is used.  That was
> >> also the reason why additionalProperties was set to true in the last
> >> iteration, but after reading the doc more carefully I realized that was
> >> wrong after you pointed it out.
> >>
> >> I followed along bindings/spi/nvidia,tegra114-spi.yaml.
> > 
> > OK, you are right, the property is used here in controller node, however
> > Linux driver never parsed it. It was never used, so I propose to drop it
> > from the binding and example. You can mention in commit msg that
> > spi-max-frequency was not documented thus you drop it from the example.
> > 
> > DTS should be fixed as well. I'll send a patch for it.
> 
> Cc Daniel,
> 
> BTW, J2 core is rather odd platform to work on... Even cross compiling
> and building that DTB is tricky. If I failed, I have doubts that you
> tested the DTS with your binding.
> 
> This applies to all GSoC or some Linux Mentorship programs: I suggest to
> choose for conversion bindings with more users and bigger possible
> impact. So first I would look at ARM64 and ARMv7 platforms. We still
> have around 1000 and 3500 unique warnings about undocumented compatibles
> for ARM64 defconfig and ARM multi_v7! That's the platforms you should
> choose.
> 
> Not SuperH, ARC, or whatever with only one DTS which is difficult to
> build for regular developer.

To add to this, either ask DT maintainers what would be useful to work 
on or you can look here[1][2] to see what are the top occurring 
undocumented (by schema) compatibles.

Rob

[1] https://gitlab.com/robherring/linux-dt/-/jobs/6453674734
[2] https://gitlab.com/robherring/linux-dt/-/jobs/6453674732

Re: [PATCH v2] spi: dt-bindings: jcore,spi: convert spi-jcore to dtschema

[Kousik Sanagavarapu]

On Fri, Mar 22, 2024 at 10:05:24AM -0500, Rob Herring wrote:
> On Fri, Mar 22, 2024 at 07:49:57AM +0100, Krzysztof Kozlowski wrote:
> > This applies to all GSoC or some Linux Mentorship programs: I suggest to
> > choose for conversion bindings with more users and bigger possible
> > impact. So first I would look at ARM64 and ARMv7 platforms. We still
> > have around 1000 and 3500 unique warnings about undocumented compatibles
> > for ARM64 defconfig and ARM multi_v7! That's the platforms you should
> > choose.
> > 
> > Not SuperH, ARC, or whatever with only one DTS which is difficult to
> > build for regular developer.
> 
> To add to this, either ask DT maintainers what would be useful to work 
> on or you can look here[1][2] to see what are the top occurring 
> undocumented (by schema) compatibles.
> 
> Rob
> 
> [1] https://gitlab.com/robherring/linux-dt/-/jobs/6453674734
> [2] https://gitlab.com/robherring/linux-dt/-/jobs/6453674732

Thank you for the valuable pieces of advice and links Krzysztof and Rob.