[PATCH v4 0/2] dt-bindings: serial: samsung: style and gs101 fixes

[PATCH v4 0/2] dt-bindings: serial: samsung: style and gs101 fixes

[André Draszik]

This series started as a single patch [1] (as part of [2]) to fix a few
issues with the UART on gs101.

In [3], Rob pointed out that the binding here shouldn't be duplicating
clock-names all over the place.

So now we have a patch to do what Rob suggested, and my original patch to
address the incorrect number of clocks.

The whole series is marked as v4, because patch 2 in this series used to be
v3 in the original series.

To: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: Jiri Slaby <jirislaby@kernel.org>
To: Rob Herring <robh@kernel.org>
To: Krzysztof Kozlowski <krzk+dt@kernel.org>
To: Conor Dooley <conor+dt@kernel.org>
Cc: Peter Griffin <peter.griffin@linaro.org>
Cc: Tudor Ambarus <tudor.ambarus@linaro.org>
Cc: Will McVicker <willmcvicker@google.com>
Cc: kernel-team@android.com
Cc: linux-kernel@vger.kernel.org
Cc: linux-serial@vger.kernel.org
Cc: devicetree@vger.kernel.org
Signed-off-by: André Draszik <andre.draszik@linaro.org>

[1] https://lore.kernel.org/all/20240710-gs101-non-essential-clocks-2-v3-1-5dcb8d040d1c@linaro.org/
[2] https://lore.kernel.org/all/20240710-gs101-non-essential-clocks-2-v3-0-5dcb8d040d1c@linaro.org/
[3] https://lore.kernel.org/all/20240711212359.GA3023490-robh@kernel.org/

---
André Draszik (2):
      dt-bindings: serial: samsung: avoid duplicating permitted clock-names
      dt-bindings: serial: samsung: fix maxItems for gs101

 .../devicetree/bindings/serial/samsung_uart.yaml   | 69 ++++++++++++++++++----
 1 file changed, 56 insertions(+), 13 deletions(-)
---
base-commit: 523b23f0bee3014a7a752c9bb9f5c54f0eddae88
change-id: 20240712-gs101-uart-binding-b47f3a270c36

Best regards,
-- 
André Draszik <andre.draszik@linaro.org>

[PATCH v4 1/2] dt-bindings: serial: samsung: avoid duplicating permitted clock-names

[André Draszik]

This binding currently duplicates the permitted clock-names in various
places, and when adding more compatibles, clock-names will have to be
duplicated even more.

The reason is:
1) subschemas (-if: ...), still have to match the top-level:
       pattern: '^clk_uart_baud[0-3]$'
2) there is one compatible that doesn't follow sequential numbering for
   the clock names (samsung,s3c6400-uart)
3) when limiting the number of clock-names, we also want to enforce
   sequential names
Because of 1) and 2), the patterns can not simply be changed to
constant strings, and later overridden in a different subschema (for
samsung,s3c6400-uart only).

Since we can't populate the top-level clock-names based on the
compatible, and because when limiting the number of items we generally
want sequential numbers and not a pattern, move the permitted strings
into a subschema of its own and populate it based on the compatible:
    * 'uart clk_uart_baud2 clk_uart_baud3' for the one outlier
    * 'uart clk_uart_baud0..3' for everything else

This way we can avoid having to duplicate the permitted names
everywhere.

While at it, add blank lines as per the universal style, which is to
have blank lines between properties, except where they are booleans.

Also add another example using a compatible that uses the default
clock-names scheme, as opposed to the existing example that uses
samsung,s3c6400-uart's non-default clock-names. This allows testing
both versions of the clock-names property when running
dt_binding_check.

Signed-off-by: André Draszik <andre.draszik@linaro.org>
---
 .../devicetree/bindings/serial/samsung_uart.yaml   | 63 +++++++++++++++++-----
 1 file changed, 50 insertions(+), 13 deletions(-)

diff --git a/Documentation/devicetree/bindings/serial/samsung_uart.yaml b/Documentation/devicetree/bindings/serial/samsung_uart.yaml
index 0f0131026911..cfa1c0de946f 100644
--- a/Documentation/devicetree/bindings/serial/samsung_uart.yaml
+++ b/Documentation/devicetree/bindings/serial/samsung_uart.yaml
@@ -58,12 +58,7 @@ properties:
   clock-names:
     description: N = 0 is allowed for SoCs without internal baud clock mux.
     minItems: 2
-    items:
-      - const: uart
-      - pattern: '^clk_uart_baud[0-3]$'
-      - pattern: '^clk_uart_baud[0-3]$'
-      - pattern: '^clk_uart_baud[0-3]$'
-      - pattern: '^clk_uart_baud[0-3]$'
+    maxItems: 5
 
   dmas:
     items:
@@ -103,18 +98,45 @@ allOf:
         compatible:
           contains:
             enum:
-              - samsung,s5pv210-uart
+              - samsung,s3c6400-uart
     then:
       properties:
         clocks:
-          minItems: 2
+          minItems: 3
           maxItems: 3
+
+        clock-names:
+          items:
+            - const: uart
+            - const: clk_uart_baud2
+            - const: clk_uart_baud3
+
+    else:
+      properties:
         clock-names:
           minItems: 2
           items:
             - const: uart
-            - pattern: '^clk_uart_baud[0-1]$'
-            - pattern: '^clk_uart_baud[0-1]$'
+            - const: clk_uart_baud0
+            - const: clk_uart_baud1
+            - const: clk_uart_baud2
+            - const: clk_uart_baud3
+
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - samsung,s5pv210-uart
+    then:
+      properties:
+        clocks:
+          minItems: 3
+          maxItems: 3
+
+        clock-names:
+          minItems: 3
+          maxItems: 3
 
   - if:
       properties:
@@ -129,10 +151,9 @@ allOf:
       properties:
         clocks:
           maxItems: 2
+
         clock-names:
-          items:
-            - const: uart
-            - const: clk_uart_baud0
+          maxItems: 2
 
   - if:
       properties:
@@ -163,3 +184,19 @@ examples:
                  <&clocks SCLK_UART>;
         samsung,uart-fifosize = <16>;
     };
+  - |
+    #include <dt-bindings/clock/google,gs101.h>
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    #include <dt-bindings/interrupt-controller/irq.h>
+
+    serial_0: serial@10a00000 {
+      compatible = "google,gs101-uart";
+      reg = <0x10a00000 0xc0>;
+      clocks = <&cmu_peric0 CLK_GOUT_PERIC0_PERIC0_TOP1_PCLK_0>,
+               <&cmu_peric0 CLK_GOUT_PERIC0_PERIC0_TOP1_IPCLK_0>;
+      clock-names = "uart", "clk_uart_baud0";
+      interrupts = <GIC_SPI 634 IRQ_TYPE_LEVEL_HIGH 0>;
+      pinctrl-0 = <&uart0_bus>;
+      pinctrl-names = "default";
+      samsung,uart-fifosize = <256>;
+    };

-- 
2.45.2.993.g49e7a77208-goog

Re: [PATCH v4 1/2] dt-bindings: serial: samsung: avoid duplicating permitted clock-names

[Rob Herring]

On Fri, Jul 12, 2024 at 03:51:17PM +0100, André Draszik wrote:
> This binding currently duplicates the permitted clock-names in various
> places, and when adding more compatibles, clock-names will have to be
> duplicated even more.
> 
> The reason is:
> 1) subschemas (-if: ...), still have to match the top-level:
>        pattern: '^clk_uart_baud[0-3]$'
> 2) there is one compatible that doesn't follow sequential numbering for
>    the clock names (samsung,s3c6400-uart)
> 3) when limiting the number of clock-names, we also want to enforce
>    sequential names
> Because of 1) and 2), the patterns can not simply be changed to
> constant strings, and later overridden in a different subschema (for
> samsung,s3c6400-uart only).
> 
> Since we can't populate the top-level clock-names based on the
> compatible, and because when limiting the number of items we generally
> want sequential numbers and not a pattern, move the permitted strings
> into a subschema of its own and populate it based on the compatible:
>     * 'uart clk_uart_baud2 clk_uart_baud3' for the one outlier
>     * 'uart clk_uart_baud0..3' for everything else
> 
> This way we can avoid having to duplicate the permitted names
> everywhere.
> 
> While at it, add blank lines as per the universal style, which is to
> have blank lines between properties, except where they are booleans.
> 
> Also add another example using a compatible that uses the default
> clock-names scheme, as opposed to the existing example that uses
> samsung,s3c6400-uart's non-default clock-names. This allows testing
> both versions of the clock-names property when running
> dt_binding_check.
> 
> Signed-off-by: André Draszik <andre.draszik@linaro.org>
> ---
>  .../devicetree/bindings/serial/samsung_uart.yaml   | 63 +++++++++++++++++-----
>  1 file changed, 50 insertions(+), 13 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/serial/samsung_uart.yaml b/Documentation/devicetree/bindings/serial/samsung_uart.yaml
> index 0f0131026911..cfa1c0de946f 100644
> --- a/Documentation/devicetree/bindings/serial/samsung_uart.yaml
> +++ b/Documentation/devicetree/bindings/serial/samsung_uart.yaml
> @@ -58,12 +58,7 @@ properties:
>    clock-names:
>      description: N = 0 is allowed for SoCs without internal baud clock mux.

The description doesn't really make sense on its own. I'd drop it.

With that,

Reviewed-by: Rob Herring (Arm) <robh@kernel.org>

>      minItems: 2
> -    items:
> -      - const: uart
> -      - pattern: '^clk_uart_baud[0-3]$'
> -      - pattern: '^clk_uart_baud[0-3]$'
> -      - pattern: '^clk_uart_baud[0-3]$'
> -      - pattern: '^clk_uart_baud[0-3]$'
> +    maxItems: 5

[PATCH v4 2/2] dt-bindings: serial: samsung: fix maxItems for gs101

[André Draszik]

While gs101 needs exactly two clocks for the UART, the schema doesn't
currently limit the maximum number to this and instead the default of
five from this schema is applied.

Update the schema accordingly.

Signed-off-by: André Draszik <andre.draszik@linaro.org>

---
v4:
* drop description from clocks:, it was Linux-specific and a we'll
  implement something that makes it obsolete anyway
* no need to duplicate clock-names anymore
---
 Documentation/devicetree/bindings/serial/samsung_uart.yaml | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/Documentation/devicetree/bindings/serial/samsung_uart.yaml b/Documentation/devicetree/bindings/serial/samsung_uart.yaml
index cfa1c0de946f..37ffa953b064 100644
--- a/Documentation/devicetree/bindings/serial/samsung_uart.yaml
+++ b/Documentation/devicetree/bindings/serial/samsung_uart.yaml
@@ -167,6 +167,12 @@ allOf:
       properties:
         reg-io-width: false
 
+        clocks:
+          maxItems: 2
+
+        clock-names:
+          maxItems: 2
+
 unevaluatedProperties: false
 
 examples:

-- 
2.45.2.993.g49e7a77208-goog

Re: [PATCH v4 2/2] dt-bindings: serial: samsung: fix maxItems for gs101

[Rob Herring (Arm)]

On Fri, 12 Jul 2024 15:51:18 +0100, André Draszik wrote:
> While gs101 needs exactly two clocks for the UART, the schema doesn't
> currently limit the maximum number to this and instead the default of
> five from this schema is applied.
> 
> Update the schema accordingly.
> 
> Signed-off-by: André Draszik <andre.draszik@linaro.org>
> 
> ---
> v4:
> * drop description from clocks:, it was Linux-specific and a we'll
>   implement something that makes it obsolete anyway
> * no need to duplicate clock-names anymore
> ---
>  Documentation/devicetree/bindings/serial/samsung_uart.yaml | 6 ++++++
>  1 file changed, 6 insertions(+)
> 

Reviewed-by: Rob Herring (Arm) <robh@kernel.org>