2022-07-04 08:59:35

by Sergiu Moga

[permalink] [raw]
Subject: [PATCH v2] spi: atmel: convert spi_atmel to json-schema

Convert SPI DT binding for Atmel/Microchip SoCs to YAML schema.

Signed-off-by: Sergiu Moga <[email protected]>
---

v1 -> v2:
- change subject headline prefix from "dt-bindings: spi" to "spi: atmel"
- change maintainer
- kept the compatbile as items (instead of switching to enums) and at91rm9200
as fallback for sam9x60, since the evolution of IP's is incremental.
- removed unnecessay "cs-gpios" property and descriptions
- added min/max for fifo-size property.


.../devicetree/bindings/spi/atmel,spi.yaml | 77 +++++++++++++++++++
.../devicetree/bindings/spi/spi_atmel.txt | 36 ---------
2 files changed, 77 insertions(+), 36 deletions(-)
create mode 100644 Documentation/devicetree/bindings/spi/atmel,spi.yaml
delete mode 100644 Documentation/devicetree/bindings/spi/spi_atmel.txt

diff --git a/Documentation/devicetree/bindings/spi/atmel,spi.yaml b/Documentation/devicetree/bindings/spi/atmel,spi.yaml
new file mode 100644
index 000000000000..d627fc9315bc
--- /dev/null
+++ b/Documentation/devicetree/bindings/spi/atmel,spi.yaml
@@ -0,0 +1,77 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+# Copyright (C) 2022 Microchip Technology, Inc. and its subsidiaries
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/spi/atmel,spi.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Atmel SPI device
+
+maintainers:
+ - Tudor Ambarus <[email protected]>
+
+allOf:
+ - $ref: spi-controller.yaml#
+
+properties:
+ compatible:
+ oneOf:
+ - items:
+ - const: atmel,at91rm9200-spi
+ - items:
+ - const: microchip,sam9x60-spi
+ - const: atmel,at91rm9200-spi
+
+ reg:
+ maxItems: 1
+
+ interrupts:
+ maxItems: 1
+
+ clock-names:
+ contains:
+ const: spi_clk
+
+ clocks:
+ maxItems: 1
+
+ atmel,fifo-size:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ description: |
+ Maximum number of data the RX and TX FIFOs can store for FIFO
+ capable SPI controllers.
+ minimum: 16
+ maximum: 32
+
+required:
+ - compatible
+ - reg
+ - interrupts
+ - clock-names
+ - clocks
+
+unevaluatedProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/gpio/gpio.h>
+ #include <dt-bindings/interrupt-controller/irq.h>
+
+ spi1: spi@fffcc000 {
+ compatible = "atmel,at91rm9200-spi";
+ reg = <0xfffcc000 0x4000>;
+ interrupts = <13 IRQ_TYPE_LEVEL_HIGH 5>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ clocks = <&spi1_clk>;
+ clock-names = "spi_clk";
+ cs-gpios = <&pioB 3 GPIO_ACTIVE_HIGH>;
+ atmel,fifo-size = <32>;
+
+ mmc@0 {
+ compatible = "mmc-spi-slot";
+ reg = <0>;
+ gpios = <&pioC 4 GPIO_ACTIVE_HIGH>; /* CD */
+ spi-max-frequency = <25000000>;
+ };
+ };
diff --git a/Documentation/devicetree/bindings/spi/spi_atmel.txt b/Documentation/devicetree/bindings/spi/spi_atmel.txt
deleted file mode 100644
index 5bb4a8f1df7a..000000000000
--- a/Documentation/devicetree/bindings/spi/spi_atmel.txt
+++ /dev/null
@@ -1,36 +0,0 @@
-Atmel SPI device
-
-Required properties:
-- compatible : should be "atmel,at91rm9200-spi" or "microchip,sam9x60-spi".
-- reg: Address and length of the register set for the device
-- interrupts: Should contain spi interrupt
-- cs-gpios: chipselects (optional for SPI controller version >= 2 with the
- Chip Select Active After Transfer feature).
-- clock-names: tuple listing input clock names.
- Required elements: "spi_clk"
-- clocks: phandles to input clocks.
-
-Optional properties:
-- atmel,fifo-size: maximum number of data the RX and TX FIFOs can store for FIFO
- capable SPI controllers.
-
-Example:
-
-spi1: spi@fffcc000 {
- compatible = "atmel,at91rm9200-spi";
- reg = <0xfffcc000 0x4000>;
- interrupts = <13 4 5>;
- #address-cells = <1>;
- #size-cells = <0>;
- clocks = <&spi1_clk>;
- clock-names = "spi_clk";
- cs-gpios = <&pioB 3 0>;
- atmel,fifo-size = <32>;
-
- mmc-slot@0 {
- compatible = "mmc-spi-slot";
- reg = <0>;
- gpios = <&pioC 4 0>; /* CD */
- spi-max-frequency = <25000000>;
- };
-};
--
2.25.1


2022-07-04 13:25:36

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH v2] spi: atmel: convert spi_atmel to json-schema

On 04/07/2022 15:13, [email protected] wrote:
> On 04.07.2022 15:52, Krzysztof Kozlowski wrote:
>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>>
>> On 04/07/2022 10:31, Sergiu Moga wrote:
>>> Convert SPI DT binding for Atmel/Microchip SoCs to YAML schema.
>>>
>>> Signed-off-by: Sergiu Moga <[email protected]>
>>> ---
>>>
>>> v1 -> v2:
>>> - change subject headline prefix from "dt-bindings: spi" to "spi: atmel"
>>
>> Should be:
>> spi: dt-bindings: atmel,spi: convert to json-schema
>> (or to DT schema)
>>
>>
>>> - change maintainer
>>> - kept the compatbile as items (instead of switching to enums) and at91rm9200
>>> as fallback for sam9x60, since the evolution of IP's is incremental.
>>> - removed unnecessay "cs-gpios" property and descriptions
>>> - added min/max for fifo-size property.
>>>
>>>
>>> .../devicetree/bindings/spi/atmel,spi.yaml | 77 +++++++++++++++++++
>
> Regarding file name, shouldn't it be atmel,at91rm9200-spi.yaml or something
> similar?

Yes, that would be better.


Best regards,
Krzysztof

2022-07-04 13:49:55

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH v2] spi: atmel: convert spi_atmel to json-schema

On 04/07/2022 10:31, Sergiu Moga wrote:
> Convert SPI DT binding for Atmel/Microchip SoCs to YAML schema.
>
> Signed-off-by: Sergiu Moga <[email protected]>
> ---
>
> v1 -> v2:
> - change subject headline prefix from "dt-bindings: spi" to "spi: atmel"

Should be:
spi: dt-bindings: atmel,spi: convert to json-schema
(or to DT schema)


> - change maintainer
> - kept the compatbile as items (instead of switching to enums) and at91rm9200
> as fallback for sam9x60, since the evolution of IP's is incremental.
> - removed unnecessay "cs-gpios" property and descriptions
> - added min/max for fifo-size property.
>
>
> .../devicetree/bindings/spi/atmel,spi.yaml | 77 +++++++++++++++++++
> .../devicetree/bindings/spi/spi_atmel.txt | 36 ---------
> 2 files changed, 77 insertions(+), 36 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/spi/atmel,spi.yaml
> delete mode 100644 Documentation/devicetree/bindings/spi/spi_atmel.txt
>
> diff --git a/Documentation/devicetree/bindings/spi/atmel,spi.yaml b/Documentation/devicetree/bindings/spi/atmel,spi.yaml
> new file mode 100644
> index 000000000000..d627fc9315bc
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/spi/atmel,spi.yaml
> @@ -0,0 +1,77 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +# Copyright (C) 2022 Microchip Technology, Inc. and its subsidiaries
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/spi/atmel,spi.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Atmel SPI device
> +
> +maintainers:
> + - Tudor Ambarus <[email protected]>
> +
> +allOf:
> + - $ref: spi-controller.yaml#
> +
> +properties:
> + compatible:
> + oneOf:
> + - items:
> + - const: atmel,at91rm9200-spi
> + - items:
> + - const: microchip,sam9x60-spi
> + - const: atmel,at91rm9200-spi
> +
> + reg:
> + maxItems: 1
> +
> + interrupts:
> + maxItems: 1
> +
> + clock-names:
> + contains:
> + const: spi_clk
> +
> + clocks:
> + maxItems: 1
> +
> + atmel,fifo-size:
> + $ref: /schemas/types.yaml#/definitions/uint32
> + description: |
> + Maximum number of data the RX and TX FIFOs can store for FIFO
> + capable SPI controllers.
> + minimum: 16
> + maximum: 32
> +
> +required:
> + - compatible
> + - reg
> + - interrupts
> + - clock-names
> + - clocks

In properties it's clock-names followed by clocks, so better to keep
same order here.

> +
> +unevaluatedProperties: false
> +



Reviewed-by: Krzysztof Kozlowski <[email protected]>

Best regards,
Krzysztof

2022-07-04 13:51:01

by Sergiu Moga

[permalink] [raw]
Subject: Re: [PATCH v2] spi: atmel: convert spi_atmel to json-schema

On 04.07.2022 15:52, Krzysztof Kozlowski wrote:
> On 04/07/2022 10:31, Sergiu Moga wrote:
>> Convert SPI DT binding for Atmel/Microchip SoCs to YAML schema.
>>
>> Signed-off-by: Sergiu Moga <[email protected]>
>> ---
>>
>> v1 -> v2:
>> - change subject headline prefix from "dt-bindings: spi" to "spi: atmel"
> Should be:
> spi: dt-bindings: atmel,spi: convert to json-schema
> (or to DT schema)
>
Noted.
>> - change maintainer
>> - kept the compatbile as items (instead of switching to enums) and at91rm9200
>> as fallback for sam9x60, since the evolution of IP's is incremental.
>> - removed unnecessay "cs-gpios" property and descriptions
>> - added min/max for fifo-size property.
>>
>>
>> .../devicetree/bindings/spi/atmel,spi.yaml | 77 +++++++++++++++++++
>> .../devicetree/bindings/spi/spi_atmel.txt | 36 ---------
>> 2 files changed, 77 insertions(+), 36 deletions(-)
>> create mode 100644 Documentation/devicetree/bindings/spi/atmel,spi.yaml
>> delete mode 100644 Documentation/devicetree/bindings/spi/spi_atmel.txt
>>
>> diff --git a/Documentation/devicetree/bindings/spi/atmel,spi.yaml b/Documentation/devicetree/bindings/spi/atmel,spi.yaml
>> new file mode 100644
>> index 000000000000..d627fc9315bc
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/spi/atmel,spi.yaml
>> @@ -0,0 +1,77 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +# Copyright (C) 2022 Microchip Technology, Inc. and its subsidiaries
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/spi/atmel,spi.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Atmel SPI device
>> +
>> +maintainers:
>> + - Tudor Ambarus <[email protected]>
>> +
>> +allOf:
>> + - $ref: spi-controller.yaml#
>> +
>> +properties:
>> + compatible:
>> + oneOf:
>> + - items:
>> + - const: atmel,at91rm9200-spi
>> + - items:
>> + - const: microchip,sam9x60-spi
>> + - const: atmel,at91rm9200-spi
>> +
>> + reg:
>> + maxItems: 1
>> +
>> + interrupts:
>> + maxItems: 1
>> +
>> + clock-names:
>> + contains:
>> + const: spi_clk
>> +
>> + clocks:
>> + maxItems: 1
>> +
>> + atmel,fifo-size:
>> + $ref: /schemas/types.yaml#/definitions/uint32
>> + description: |
>> + Maximum number of data the RX and TX FIFOs can store for FIFO
>> + capable SPI controllers.
>> + minimum: 16
>> + maximum: 32
>> +
>> +required:
>> + - compatible
>> + - reg
>> + - interrupts
>> + - clock-names
>> + - clocks
> In properties it's clock-names followed by clocks, so better to keep
> same order here.
I am sorry, but I don't quite understand. Isn't it the same already (the
order in properties and the order in required)?
>> +
>> +unevaluatedProperties: false
>> +
>
>
> Reviewed-by: Krzysztof Kozlowski <[email protected]>
>
> Best regards,
> Krzysztof

Regards,

    Sergiu

2022-07-04 13:52:32

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH v2] spi: atmel: convert spi_atmel to json-schema

On 04/07/2022 15:32, [email protected] wrote:
>>> +required:
>>> + - compatible
>>> + - reg
>>> + - interrupts
>>> + - clock-names
>>> + - clocks
>> In properties it's clock-names followed by clocks, so better to keep
>> same order here.
> I am sorry, but I don't quite understand. Isn't it the same already (the
> order in properties and the order in required)?
>>> +

Apologies, ignore my comment. I saw something else... It is fine.

Best regards,
Krzysztof

2022-07-04 13:59:28

by Tudor Ambarus

[permalink] [raw]
Subject: Re: [PATCH v2] spi: atmel: convert spi_atmel to json-schema

On 7/4/22 11:31, Sergiu Moga wrote:
> + atmel,fifo-size:
> + $ref: /schemas/types.yaml#/definitions/uint32
> + description: |
> + Maximum number of data the RX and TX FIFOs can store for FIFO
> + capable SPI controllers.
> + minimum: 16
> + maximum: 32

Shouldn't we have an enum instead of a range? Can the FIFO size have a value of 24?

2022-07-04 14:04:39

by Claudiu Beznea

[permalink] [raw]
Subject: Re: [PATCH v2] spi: atmel: convert spi_atmel to json-schema

On 04.07.2022 15:52, Krzysztof Kozlowski wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>
> On 04/07/2022 10:31, Sergiu Moga wrote:
>> Convert SPI DT binding for Atmel/Microchip SoCs to YAML schema.
>>
>> Signed-off-by: Sergiu Moga <[email protected]>
>> ---
>>
>> v1 -> v2:
>> - change subject headline prefix from "dt-bindings: spi" to "spi: atmel"
>
> Should be:
> spi: dt-bindings: atmel,spi: convert to json-schema
> (or to DT schema)
>
>
>> - change maintainer
>> - kept the compatbile as items (instead of switching to enums) and at91rm9200
>> as fallback for sam9x60, since the evolution of IP's is incremental.
>> - removed unnecessay "cs-gpios" property and descriptions
>> - added min/max for fifo-size property.
>>
>>
>> .../devicetree/bindings/spi/atmel,spi.yaml | 77 +++++++++++++++++++

Regarding file name, shouldn't it be atmel,at91rm9200-spi.yaml or something
similar?

>> .../devicetree/bindings/spi/spi_atmel.txt | 36 ---------
>> 2 files changed, 77 insertions(+), 36 deletions(-)
>> create mode 100644 Documentation/devicetree/bindings/spi/atmel,spi.yaml
>> delete mode 100644 Documentation/devicetree/bindings/spi/spi_atmel.txt
>>
>> diff --git a/Documentation/devicetree/bindings/spi/atmel,spi.yaml b/Documentation/devicetree/bindings/spi/atmel,spi.yaml
>> new file mode 100644
>> index 000000000000..d627fc9315bc
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/spi/atmel,spi.yaml
>> @@ -0,0 +1,77 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +# Copyright (C) 2022 Microchip Technology, Inc. and its subsidiaries
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/spi/atmel,spi.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Atmel SPI device
>> +
>> +maintainers:
>> + - Tudor Ambarus <[email protected]>
>> +
>> +allOf:
>> + - $ref: spi-controller.yaml#
>> +
>> +properties:
>> + compatible:
>> + oneOf:
>> + - items:
>> + - const: atmel,at91rm9200-spi
>> + - items:
>> + - const: microchip,sam9x60-spi
>> + - const: atmel,at91rm9200-spi
>> +
>> + reg:
>> + maxItems: 1
>> +
>> + interrupts:
>> + maxItems: 1
>> +
>> + clock-names:
>> + contains:
>> + const: spi_clk
>> +
>> + clocks:
>> + maxItems: 1
>> +
>> + atmel,fifo-size:
>> + $ref: /schemas/types.yaml#/definitions/uint32
>> + description: |
>> + Maximum number of data the RX and TX FIFOs can store for FIFO
>> + capable SPI controllers.
>> + minimum: 16
>> + maximum: 32
>> +
>> +required:
>> + - compatible
>> + - reg
>> + - interrupts
>> + - clock-names
>> + - clocks
>
> In properties it's clock-names followed by clocks, so better to keep
> same order here.
>
>> +
>> +unevaluatedProperties: false
>> +
>
>
>
> Reviewed-by: Krzysztof Kozlowski <[email protected]>
>
> Best regards,
> Krzysztof

2022-07-04 14:20:21

by Tudor Ambarus

[permalink] [raw]
Subject: Re: [PATCH v2] spi: atmel: convert spi_atmel to json-schema

On 7/4/22 16:24, Tudor Ambarus wrote:
> On 7/4/22 11:31, Sergiu Moga wrote:
>> + atmel,fifo-size:
>> + $ref: /schemas/types.yaml#/definitions/uint32
>> + description: |
>> + Maximum number of data the RX and TX FIFOs can store for FIFO
>> + capable SPI controllers.
>> + minimum: 16
>> + maximum: 32
>
> Shouldn't we have an enum instead of a range? Can the FIFO size have a value of 24?

I looked into the driver, when max is 32, one can set 24 for example.
I expect it will work, as the FIFO can handle data that is not multiple with
the FIFO size.

But I can't think of reasons why one would use a smaller FIFO size, so I lean
towards making this an enum instead of a range.

Cheers,
ta