2024-03-21 18:14:05

by Kousik Sanagavarapu

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

Convert existing bindings of J-Core spi2 to dtschema.

No new properties are added.

Signed-off-by: Kousik Sanagavarapu <[email protected]>
---
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 <[email protected]>
+
+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



2024-03-22 06:03:47

by Krzysztof Kozlowski

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

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 <[email protected]>
> ---
> 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 <[email protected]>
> +
> +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


2024-03-22 06:33:56

by Kousik Sanagavarapu

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

On Fri, 22 Mar 2024, 11:33 Krzysztof Kozlowski,
<[email protected]> 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)

2024-03-22 06:34:38

by Krzysztof Kozlowski

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

On 22/03/2024 07:23, Kousik Sanagavarapu wrote:
> On Fri, 22 Mar 2024, 11:33 Krzysztof Kozlowski, <
> [email protected]> 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


2024-03-22 06:50:10

by Krzysztof Kozlowski

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

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, <
>> [email protected]> 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


2024-03-22 15:05:41

by Rob Herring (Arm)

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

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, <
> >> [email protected]> 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

2024-03-22 16:48:29

by Kousik Sanagavarapu

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

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.