[PATCH] dt-bindings: serial: actions,owl-uart: convert to dtschema

[PATCH] dt-bindings: serial: actions,owl-uart: convert to dtschema

[Kanak Shilledar]

From: Kanak Shilledar <kanakshilledar111@protonmail.com>

Convert the Actions Semi Owl UART to newer DT schema.

Signed-off-by: Kanak Shilledar <kanakshilledar111@protonmail.com>
---
 .../bindings/serial/actions,owl-uart.txt      | 16 -------
 .../bindings/serial/actions,owl-uart.yaml     | 43 +++++++++++++++++++
 2 files changed, 43 insertions(+), 16 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/serial/actions,owl-uart.txt
 create mode 100644 Documentation/devicetree/bindings/serial/actions,owl-uart.yaml

diff --git a/Documentation/devicetree/bindings/serial/actions,owl-uart.txt b/Documentation/devicetree/bindings/serial/actions,owl-uart.txt
deleted file mode 100644
index aa873eada02d..000000000000
--- a/Documentation/devicetree/bindings/serial/actions,owl-uart.txt
+++ /dev/null
@@ -1,16 +0,0 @@
-Actions Semi Owl UART
-
-Required properties:
-- compatible :  "actions,s500-uart", "actions,owl-uart" for S500
-                "actions,s900-uart", "actions,owl-uart" for S900
-- reg        :  Offset and length of the register set for the device.
-- interrupts :  Should contain UART interrupt.
-
-
-Example:
-
-		uart3: serial@b0126000 {
-			compatible = "actions,s500-uart", "actions,owl-uart";
-			reg = <0xb0126000 0x1000>;
-			interrupts = <GIC_SPI 32 IRQ_TYPE_LEVEL_HIGH>;
-		};
diff --git a/Documentation/devicetree/bindings/serial/actions,owl-uart.yaml b/Documentation/devicetree/bindings/serial/actions,owl-uart.yaml
new file mode 100644
index 000000000000..ee0e508255b6
--- /dev/null
+++ b/Documentation/devicetree/bindings/serial/actions,owl-uart.yaml
@@ -0,0 +1,43 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/serial/actions,owl-uart.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Actions Semi Owl UART
+
+maintainers:
+  - Kanak Shilledar <kanakshilledar111@protonmail.com>
+
+allOf:
+  - $ref: serial.yaml
+
+properties:
+  compatible:
+    items:
+      - enum:
+          - actions,s500-uart
+          - actions,s900-uart
+          - actions,owl-uart
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+required:
+  - compatible
+  - reg
+  - interrupts
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    serial@b0126000 {
+        compatible = "actions,s500-uart";
+        reg = <0xb0126000 0x1000>;
+        interrupts = <GIC_SPI 32 IRQ_TYPE_LEVEL_HIGH>;
+    };
-- 
2.34.1

Re: [PATCH] dt-bindings: serial: actions,owl-uart: convert to dtschema

[Krzysztof Kozlowski]

On 21/03/2024 09:43, Kanak Shilledar wrote:
> From: Kanak Shilledar <kanakshilledar111@protonmail.com>
> 
> Convert the Actions Semi Owl UART to newer DT schema.

You are using ProtonMail, so just be aware that it might not be suitable
for public Linux discussions:
https://www.kernel.org/doc/html/next/process/email-clients.html#proton-mail

(no need to resend just for that)

> 
> Signed-off-by: Kanak Shilledar <kanakshilledar111@protonmail.com>
> ---
>  .../bindings/serial/actions,owl-uart.txt      | 16 -------
>  .../bindings/serial/actions,owl-uart.yaml     | 43 +++++++++++++++++++
>  2 files changed, 43 insertions(+), 16 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/serial/actions,owl-uart.txt
>  create mode 100644 Documentation/devicetree/bindings/serial/actions,owl-uart.yaml
> 
> diff --git a/Documentation/devicetree/bindings/serial/actions,owl-uart.txt b/Documentation/devicetree/bindings/serial/actions,owl-uart.txt
> deleted file mode 100644
> index aa873eada02d..000000000000
> --- a/Documentation/devicetree/bindings/serial/actions,owl-uart.txt
> +++ /dev/null
> @@ -1,16 +0,0 @@
> -Actions Semi Owl UART
> -
> -Required properties:
> -- compatible :  "actions,s500-uart", "actions,owl-uart" for S500
> -                "actions,s900-uart", "actions,owl-uart" for S900
> -- reg        :  Offset and length of the register set for the device.
> -- interrupts :  Should contain UART interrupt.
> -
> -
> -Example:
> -
> -		uart3: serial@b0126000 {
> -			compatible = "actions,s500-uart", "actions,owl-uart";
> -			reg = <0xb0126000 0x1000>;
> -			interrupts = <GIC_SPI 32 IRQ_TYPE_LEVEL_HIGH>;
> -		};
> diff --git a/Documentation/devicetree/bindings/serial/actions,owl-uart.yaml b/Documentation/devicetree/bindings/serial/actions,owl-uart.yaml
> new file mode 100644
> index 000000000000..ee0e508255b6
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/serial/actions,owl-uart.yaml
> @@ -0,0 +1,43 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/serial/actions,owl-uart.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Actions Semi Owl UART
> +
> +maintainers:
> +  - Kanak Shilledar <kanakshilledar111@protonmail.com>
> +
> +allOf:
> +  - $ref: serial.yaml
> +
> +properties:
> +  compatible:
> +    items:
> +      - enum:
> +          - actions,s500-uart
> +          - actions,s900-uart
> +          - actions,owl-uart

That's not what old binding said and you did not explain differences
from conversion in commit msg.


> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts:
> +    maxItems: 1
> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupts
> +
> +unevaluatedProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +    serial@b0126000 {
> +        compatible = "actions,s500-uart";

That's not what was in old binding.

That's not what is being used. It does not look like you tested the DTS
against bindings. Please run `make dtbs_check W=1` (see
Documentation/devicetree/bindings/writing-schema.rst or
https://www.linaro.org/blog/tips-and-tricks-for-validating-devicetree-sources-with-the-devicetree-schema/
for instructions).


Best regards,
Krzysztof

Re: [PATCH] dt-bindings: serial: actions,owl-uart: convert to dtschema

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 908 bytes --]

On Thu, Mar 21, 2024 at 10:22:21AM +0100, Krzysztof Kozlowski wrote:
> On 21/03/2024 09:43, Kanak Shilledar wrote:
> > From: Kanak Shilledar <kanakshilledar111@protonmail.com>
> > 
> > Convert the Actions Semi Owl UART to newer DT schema.
> 
> You are using ProtonMail, so just be aware that it might not be suitable
> for public Linux discussions:
> https://www.kernel.org/doc/html/next/process/email-clients.html#proton-mail
> 
> (no need to resend just for that)

I think this might actually have been fixed/worked around for kernel.org
addresses. There was a thread a while back on the fediverse stuff with
Konstantin and some Proton people IIRC. I've not deleted the document
section though, because nobody has sent me a mail from proton in ages
so I have not been able to check whether or not it has been fixed.
I think they might've hacked in an "if (kernel.org) {disble_wdk=true}"!

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[PATCH v2] dt-bindings: serial: actions,owl-uart: convert to dtschema

[Kanak Shilledar]

From: Kanak Shilledar <kanakshilledar111@protonmail.com>

Convert the Actions Semi Owl UART to newer DT schema.
Created DT schema based on the .txt file which had
`compatible`, `reg` and `interrupts` as the required properties.
This binding is used by Actions S500 and S700 SoC.

Signed-off-by: Kanak Shilledar <kanakshilledar111@protonmail.com>
---
Changes in v2
- Added Clock property to fix the warning of `dts/actions/s700-cubieboard7.dtb: 
serial@e0126000: Unevaluated properties are not allowed 
('clocks' was unexpected)` thrown by `make dtbs_check`
- Changed the compatible identifier for having both the compatible names
earlier was throwing `owl-s500-cubieboard6.dtb: serial@b0120000: compatible:
['actions,s500-uart', 'actions,owl-uart'] is too long` warning.
- Updated commit message


 .../bindings/serial/actions,owl-uart.txt      | 16 -------
 .../bindings/serial/actions,owl-uart.yaml     | 46 +++++++++++++++++++
 2 files changed, 46 insertions(+), 16 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/serial/actions,owl-uart.txt
 create mode 100644 Documentation/devicetree/bindings/serial/actions,owl-uart.yaml

diff --git a/Documentation/devicetree/bindings/serial/actions,owl-uart.txt b/Documentation/devicetree/bindings/serial/actions,owl-uart.txt
deleted file mode 100644
index aa873eada02d..000000000000
--- a/Documentation/devicetree/bindings/serial/actions,owl-uart.txt
+++ /dev/null
@@ -1,16 +0,0 @@
-Actions Semi Owl UART
-
-Required properties:
-- compatible :  "actions,s500-uart", "actions,owl-uart" for S500
-                "actions,s900-uart", "actions,owl-uart" for S900
-- reg        :  Offset and length of the register set for the device.
-- interrupts :  Should contain UART interrupt.
-
-
-Example:
-
-		uart3: serial@b0126000 {
-			compatible = "actions,s500-uart", "actions,owl-uart";
-			reg = <0xb0126000 0x1000>;
-			interrupts = <GIC_SPI 32 IRQ_TYPE_LEVEL_HIGH>;
-		};
diff --git a/Documentation/devicetree/bindings/serial/actions,owl-uart.yaml b/Documentation/devicetree/bindings/serial/actions,owl-uart.yaml
new file mode 100644
index 000000000000..095f299a851d
--- /dev/null
+++ b/Documentation/devicetree/bindings/serial/actions,owl-uart.yaml
@@ -0,0 +1,46 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/serial/actions,owl-uart.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Actions Semi Owl UART
+
+maintainers:
+  - Kanak Shilledar <kanakshilledar111@protonmail.com>
+
+allOf:
+  - $ref: serial.yaml
+
+properties:
+  compatible:
+    items:
+      - enum:
+          - actions,s500-uart
+          - actions,s900-uart
+      - const: actions,owl-uart
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  clocks:
+    maxItems: 1
+
+required:
+  - compatible
+  - reg
+  - interrupts
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    serial@b0126000 {
+        compatible = "actions,s500-uart", "actions,owl-uart";
+        reg = <0xb0126000 0x1000>;
+        interrupts = <GIC_SPI 32 IRQ_TYPE_LEVEL_HIGH>;
+    };
-- 
2.34.1

Re: [PATCH v2] dt-bindings: serial: actions,owl-uart: convert to dtschema

[Krzysztof Kozlowski]

On 26/03/2024 09:58, Kanak Shilledar wrote:
> From: Kanak Shilledar <kanakshilledar111@protonmail.com>
> 
> Convert the Actions Semi Owl UART to newer DT schema.
> Created DT schema based on the .txt file which had
> `compatible`, `reg` and `interrupts` as the required properties.
> This binding is used by Actions S500 and S700 SoC.

Please mention changes from pure conversion. You added clocks.

> 
> Signed-off-by: Kanak Shilledar <kanakshilledar111@protonmail.com>
> ---
> Changes in v2
> - Added Clock property to fix the warning of `dts/actions/s700-cubieboard7.dtb: 
> serial@e0126000: Unevaluated properties are not allowed 
> ('clocks' was unexpected)` thrown by `make dtbs_check`
> - Changed the compatible identifier for having both the compatible names
> earlier was throwing `owl-s500-cubieboard6.dtb: serial@b0120000: compatible:
> ['actions,s500-uart', 'actions,owl-uart'] is too long` warning.
> - Updated commit message
> 

Do not attach (thread) your patchsets to some other threads (unrelated
or older versions). This buries them deep in the mailbox and might
interfere with applying entire sets.

> 
>  .../bindings/serial/actions,owl-uart.txt      | 16 -------
>  .../bindings/serial/actions,owl-uart.yaml     | 46 +++++++++++++++++++
>  2 files changed, 46 insertions(+), 16 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/serial/actions,owl-uart.txt
>  create mode 100644 Documentation/devicetree/bindings/serial/actions,owl-uart.yaml
> 

...

> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupts
> +
> +unevaluatedProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +    serial@b0126000 {
> +        compatible = "actions,s500-uart", "actions,owl-uart";
> +        reg = <0xb0126000 0x1000>;
> +        interrupts = <GIC_SPI 32 IRQ_TYPE_LEVEL_HIGH>;


Please extend the example and make it complete by adding clocks (based
on DTS).

With two changes above:

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>


---

This is an automated instruction, just in case, because many review tags
are being ignored. If you know the process, you can skip it (please do
not feel offended by me posting it here - no bad intentions intended).
If you do not know the process, here is a short explanation:

Please add Acked-by/Reviewed-by/Tested-by tags when posting new
versions, under or above your Signed-off-by tag. Tag is "received", when
provided in a message replied to you on the mailing list. Tools like b4
can help here. However, there's no need to repost patches *only* to add
the tags. The upstream maintainer will do that for tags received on the
version they apply.

https://elixir.bootlin.com/linux/v6.5-rc3/source/Documentation/process/submitting-patches.rst#L577



Best regards,
Krzysztof

[PATCH v3] dt-bindings: serial: actions,owl-uart: convert to dtschema

[Kanak Shilledar]

From: Kanak Shilledar <kanakshilledar111@protonmail.com>

Convert the Actions Semi Owl UART to newer DT schema.
Created DT schema based on the .txt file which had
`compatible`, `reg`, `clocks` and `interrupts` as the
required properties. This binding is used by Actions S500, S700
and S900 SoC. S700 and S900 use the same UART compatible string.
Implemented a full node example for the UART, referenced S500 SoC
devicetree include.

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Signed-off-by: Kanak Shilledar <kanakshilledar111@protonmail.com>
---
Changes in v3
- Updated commit message to make it more verbose
- Extended the DTS example by adding clocks. Referred the S500.dtsi 
for adding clocks property.
- Added Reviewed-by tag

 .../bindings/serial/actions,owl-uart.txt      | 16 ------
 .../bindings/serial/actions,owl-uart.yaml     | 50 +++++++++++++++++++
 2 files changed, 50 insertions(+), 16 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/serial/actions,owl-uart.txt
 create mode 100644 Documentation/devicetree/bindings/serial/actions,owl-uart.yaml

diff --git a/Documentation/devicetree/bindings/serial/actions,owl-uart.txt b/Documentation/devicetree/bindings/serial/actions,owl-uart.txt
deleted file mode 100644
index aa873eada02d..000000000000
--- a/Documentation/devicetree/bindings/serial/actions,owl-uart.txt
+++ /dev/null
@@ -1,16 +0,0 @@
-Actions Semi Owl UART
-
-Required properties:
-- compatible :  "actions,s500-uart", "actions,owl-uart" for S500
-                "actions,s900-uart", "actions,owl-uart" for S900
-- reg        :  Offset and length of the register set for the device.
-- interrupts :  Should contain UART interrupt.
-
-
-Example:
-
-		uart3: serial@b0126000 {
-			compatible = "actions,s500-uart", "actions,owl-uart";
-			reg = <0xb0126000 0x1000>;
-			interrupts = <GIC_SPI 32 IRQ_TYPE_LEVEL_HIGH>;
-		};
diff --git a/Documentation/devicetree/bindings/serial/actions,owl-uart.yaml b/Documentation/devicetree/bindings/serial/actions,owl-uart.yaml
new file mode 100644
index 000000000000..b163060cdd33
--- /dev/null
+++ b/Documentation/devicetree/bindings/serial/actions,owl-uart.yaml
@@ -0,0 +1,50 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/serial/actions,owl-uart.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Actions Semi Owl UART
+
+maintainers:
+  - Kanak Shilledar <kanakshilledar111@protonmail.com>
+
+allOf:
+  - $ref: serial.yaml
+
+properties:
+  compatible:
+    items:
+      - enum:
+          - actions,s500-uart
+          - actions,s900-uart
+      - const: actions,owl-uart
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  clocks:
+    maxItems: 1
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - clocks
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/clock/actions,s500-cmu.h>
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    uart0: serial@b0126000 {
+        compatible = "actions,s500-uart", "actions,owl-uart";
+        reg = <0xb0126000 0x1000>;
+        clocks = <&cmu CLK_UART0>;
+        interrupts = <GIC_SPI 32 IRQ_TYPE_LEVEL_HIGH>;
+        status = "disabled";
+    };
-- 
2.34.1

Re: [PATCH v2] dt-bindings: serial: actions,owl-uart: convert to dtschema

[Krzysztof Kozlowski]

On 26/03/2024 10:15, Krzysztof Kozlowski wrote:
>>
>> Signed-off-by: Kanak Shilledar <kanakshilledar111@protonmail.com>
>> ---
>> Changes in v2
>> - Added Clock property to fix the warning of `dts/actions/s700-cubieboard7.dtb: 
>> serial@e0126000: Unevaluated properties are not allowed 
>> ('clocks' was unexpected)` thrown by `make dtbs_check`
>> - Changed the compatible identifier for having both the compatible names
>> earlier was throwing `owl-s500-cubieboard6.dtb: serial@b0120000: compatible:
>> ['actions,s500-uart', 'actions,owl-uart'] is too long` warning.
>> - Updated commit message
>>
> 
> Do not attach (thread) your patchsets to some other threads (unrelated
> or older versions). This buries them deep in the mailbox and might
> interfere with applying entire sets.

You ignored this comment.

Best regards,
Krzysztof

Re: [PATCH v3] dt-bindings: serial: actions,owl-uart: convert to dtschema

[Krzysztof Kozlowski]

On 26/03/2024 12:05, Kanak Shilledar wrote:
> From: Kanak Shilledar <kanakshilledar111@protonmail.com>
> 
> Convert the Actions Semi Owl UART to newer DT schema.
> Created DT schema based on the .txt file which had
> `compatible`, `reg`, `clocks` and `interrupts` as the

That's not true. Old binding file did not have clocks. You did more than
conversion and I asked you to describe changes which are different than
pure conversion.

Instead of sending another version the same day, can you please read
carefully comments you receive?

> required properties. This binding is used by Actions S500, S700
> and S900 SoC. S700 and S900 use the same UART compatible string.
> Implemented a full node example for the UART, referenced S500 SoC
> devicetree include.
> 
> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Drop, you made MANY MORE changes than I asked, up to the point of making
it broken.

I don't understand. I asked three things. One you ignored, two
implemented and then added more.

> Signed-off-by: Kanak Shilledar <kanakshilledar111@protonmail.com>
> ---


...


> +  interrupts:
> +    maxItems: 1
> +
> +  clocks:
> +    maxItems: 1
> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupts
> +  - clocks

That's surprising. Nothing in the changelog explains this.

> +
> +unevaluatedProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/clock/actions,s500-cmu.h>
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +    uart0: serial@b0126000 {
> +        compatible = "actions,s500-uart", "actions,owl-uart";
> +        reg = <0xb0126000 0x1000>;
> +        clocks = <&cmu CLK_UART0>;
> +        interrupts = <GIC_SPI 32 IRQ_TYPE_LEVEL_HIGH>;
> +        status = "disabled";

NAK.

Examples are not disabled.

Best regards,
Krzysztof