All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3 0/4] Add SCIF support for Renesas RZ/V2H(P) SoC
@ 2024-03-18 17:20 Prabhakar
  2024-03-18 17:20 ` [PATCH v3 1/4] dt-bindings: serial: renesas,scif: Move ref for serial.yaml at the end Prabhakar
                   ` (3 more replies)
  0 siblings, 4 replies; 18+ messages in thread
From: Prabhakar @ 2024-03-18 17:20 UTC (permalink / raw
  To: Geert Uytterhoeven, Greg Kroah-Hartman, Jiri Slaby, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Magnus Damm
  Cc: linux-kernel, devicetree, linux-serial, linux-renesas-soc,
	Prabhakar, Fabrizio Castro, Lad Prabhakar

From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

Hi All,

This patch series updates DT binding doc and scif driver to add support
for the Renesas RZ/V2H(P) SoC. RZ/V2H(P) SoC supports one channel SCIF
interface.

v2->v3
- Included DT validation patches
- Added a new compat string for RZ/V2H(P) SoC
- Added driver changes for RZ/V2H(P) SoC
- Listed interrupts and interrupt-names for every SoC in if check

Cheers,
Prabhakar

Lad Prabhakar (4):
  dt-bindings: serial: renesas,scif: Move ref for serial.yaml at the end
  dt-bindings: serial: renesas,scif: Validate 'interrupts' and
    'interrupt-names'
  dt-bindings: serial: renesas,scif: Document R9A09G057 support
  serial: sh-sci: Add support for RZ/V2H(P) SoC

 .../bindings/serial/renesas,scif.yaml         | 158 ++++++++++++------
 drivers/tty/serial/sh-sci.c                   |   7 +-
 2 files changed, 117 insertions(+), 48 deletions(-)

-- 
2.34.1


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH v3 1/4] dt-bindings: serial: renesas,scif: Move ref for serial.yaml at the end
  2024-03-18 17:20 [PATCH v3 0/4] Add SCIF support for Renesas RZ/V2H(P) SoC Prabhakar
@ 2024-03-18 17:20 ` Prabhakar
  2024-03-18 17:21 ` [PATCH v3 2/4] dt-bindings: serial: renesas,scif: Validate 'interrupts' and 'interrupt-names' Prabhakar
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 18+ messages in thread
From: Prabhakar @ 2024-03-18 17:20 UTC (permalink / raw
  To: Geert Uytterhoeven, Greg Kroah-Hartman, Jiri Slaby, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Magnus Damm
  Cc: linux-kernel, devicetree, linux-serial, linux-renesas-soc,
	Prabhakar, Fabrizio Castro, Lad Prabhakar, Krzysztof Kozlowski

From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

In preparation for adding more validation checks move the ref for
'serial.yaml' to the end and also move reset check in 'allOf' block.

Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
v2->v3
- no change
---
 .../bindings/serial/renesas,scif.yaml         | 30 +++++++++----------
 1 file changed, 15 insertions(+), 15 deletions(-)

diff --git a/Documentation/devicetree/bindings/serial/renesas,scif.yaml b/Documentation/devicetree/bindings/serial/renesas,scif.yaml
index 4610a5bd580c..af72c3420453 100644
--- a/Documentation/devicetree/bindings/serial/renesas,scif.yaml
+++ b/Documentation/devicetree/bindings/serial/renesas,scif.yaml
@@ -9,9 +9,6 @@ title: Renesas Serial Communication Interface with FIFO (SCIF)
 maintainers:
   - Geert Uytterhoeven <geert+renesas@glider.be>
 
-allOf:
-  - $ref: serial.yaml#
-
 properties:
   compatible:
     oneOf:
@@ -160,18 +157,21 @@ required:
   - clock-names
   - power-domains
 
-if:
-  properties:
-    compatible:
-      contains:
-        enum:
-          - renesas,rcar-gen2-scif
-          - renesas,rcar-gen3-scif
-          - renesas,rcar-gen4-scif
-          - renesas,scif-r9a07g044
-then:
-  required:
-    - resets
+allOf:
+  - $ref: serial.yaml#
+
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - renesas,rcar-gen2-scif
+              - renesas,rcar-gen3-scif
+              - renesas,rcar-gen4-scif
+              - renesas,scif-r9a07g044
+    then:
+      required:
+        - resets
 
 unevaluatedProperties: false
 
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v3 2/4] dt-bindings: serial: renesas,scif: Validate 'interrupts' and 'interrupt-names'
  2024-03-18 17:20 [PATCH v3 0/4] Add SCIF support for Renesas RZ/V2H(P) SoC Prabhakar
  2024-03-18 17:20 ` [PATCH v3 1/4] dt-bindings: serial: renesas,scif: Move ref for serial.yaml at the end Prabhakar
@ 2024-03-18 17:21 ` Prabhakar
  2024-03-19  6:19   ` Krzysztof Kozlowski
  2024-03-18 17:21 ` [PATCH v3 3/4] dt-bindings: serial: renesas,scif: Document R9A09G057 support Prabhakar
  2024-03-18 17:21 ` [PATCH v3 4/4] serial: sh-sci: Add support for RZ/V2H(P) SoC Prabhakar
  3 siblings, 1 reply; 18+ messages in thread
From: Prabhakar @ 2024-03-18 17:21 UTC (permalink / raw
  To: Geert Uytterhoeven, Greg Kroah-Hartman, Jiri Slaby, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Magnus Damm
  Cc: linux-kernel, devicetree, linux-serial, linux-renesas-soc,
	Prabhakar, Fabrizio Castro, Lad Prabhakar

From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

Add support to validate the 'interrupts' and 'interrupt-names' properties
for every supported SoC. This ensures proper handling and configuration of
interrupt-related properties across supported platforms.

Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
v2->v3
- Listed interrupts and interrupt-names for every SoC in if check
---
 .../bindings/serial/renesas,scif.yaml         | 95 ++++++++++++-------
 1 file changed, 63 insertions(+), 32 deletions(-)

diff --git a/Documentation/devicetree/bindings/serial/renesas,scif.yaml b/Documentation/devicetree/bindings/serial/renesas,scif.yaml
index af72c3420453..53f18e9810fd 100644
--- a/Documentation/devicetree/bindings/serial/renesas,scif.yaml
+++ b/Documentation/devicetree/bindings/serial/renesas,scif.yaml
@@ -82,38 +82,6 @@ properties:
   reg:
     maxItems: 1
 
-  interrupts:
-    oneOf:
-      - items:
-          - description: A combined interrupt
-      - items:
-          - description: Error interrupt
-          - description: Receive buffer full interrupt
-          - description: Transmit buffer empty interrupt
-          - description: Break interrupt
-      - items:
-          - description: Error interrupt
-          - description: Receive buffer full interrupt
-          - description: Transmit buffer empty interrupt
-          - description: Break interrupt
-          - description: Data Ready interrupt
-          - description: Transmit End interrupt
-
-  interrupt-names:
-    oneOf:
-      - items:
-          - const: eri
-          - const: rxi
-          - const: txi
-          - const: bri
-      - items:
-          - const: eri
-          - const: rxi
-          - const: txi
-          - const: bri
-          - const: dri
-          - const: tei
-
   clocks:
     minItems: 1
     maxItems: 4
@@ -173,6 +141,69 @@ allOf:
       required:
         - resets
 
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - renesas,rcar-gen1-scif
+              - renesas,rcar-gen2-scif
+              - renesas,rcar-gen3-scif
+              - renesas,rcar-gen4-scif
+    then:
+      properties:
+        interrupts:
+          items:
+            - description: Single combined interrupt
+
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: renesas,scif-r7s72100
+    then:
+      properties:
+        interrupts:
+          items:
+            - description: Error interrupt
+            - description: Receive buffer full interrupt
+            - description: Transmit buffer empty interrupt
+            - description: Break interrupt
+
+        interrupt-names:
+          items:
+            - const: eri
+            - const: rxi
+            - const: txi
+            - const: bri
+
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - renesas,scif-r7s9210
+              - renesas,scif-r9a07g044
+    then:
+      properties:
+        interrupts:
+          items:
+            - description: Error interrupt
+            - description: Receive buffer full interrupt
+            - description: Transmit buffer empty interrupt
+            - description: Break interrupt
+            - description: Data Ready interrupt
+            - description: Transmit End interrupt
+
+        interrupt-names:
+          items:
+            - const: eri
+            - const: rxi
+            - const: txi
+            - const: bri
+            - const: dri
+            - const: tei
+
 unevaluatedProperties: false
 
 examples:
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v3 3/4] dt-bindings: serial: renesas,scif: Document R9A09G057 support
  2024-03-18 17:20 [PATCH v3 0/4] Add SCIF support for Renesas RZ/V2H(P) SoC Prabhakar
  2024-03-18 17:20 ` [PATCH v3 1/4] dt-bindings: serial: renesas,scif: Move ref for serial.yaml at the end Prabhakar
  2024-03-18 17:21 ` [PATCH v3 2/4] dt-bindings: serial: renesas,scif: Validate 'interrupts' and 'interrupt-names' Prabhakar
@ 2024-03-18 17:21 ` Prabhakar
  2024-03-19  8:12   ` Geert Uytterhoeven
  2024-03-20 14:37   ` Rob Herring
  2024-03-18 17:21 ` [PATCH v3 4/4] serial: sh-sci: Add support for RZ/V2H(P) SoC Prabhakar
  3 siblings, 2 replies; 18+ messages in thread
From: Prabhakar @ 2024-03-18 17:21 UTC (permalink / raw
  To: Geert Uytterhoeven, Greg Kroah-Hartman, Jiri Slaby, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Magnus Damm
  Cc: linux-kernel, devicetree, linux-serial, linux-renesas-soc,
	Prabhakar, Fabrizio Castro, Lad Prabhakar

From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

Document support for the Serial Communication Interface with FIFO (SCIF)
available in the Renesas RZ/V2H(P) (R9A09G057) SoC. The SCIF interface in
the Renesas RZ/V2H(P) is similar to that available in the RZ/G2L
(R9A07G044) SoC, with the only difference being that the RZ/V2H(P) SoC has
three additional interrupts: one for Tx end/Rx ready and the other two for
Rx and Tx buffer full, which are edge-triggered.

Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
v2->v3
- Added SoC specific compat string
---
 .../bindings/serial/renesas,scif.yaml         | 33 +++++++++++++++++++
 1 file changed, 33 insertions(+)

diff --git a/Documentation/devicetree/bindings/serial/renesas,scif.yaml b/Documentation/devicetree/bindings/serial/renesas,scif.yaml
index 53f18e9810fd..e4ce13e20cd7 100644
--- a/Documentation/devicetree/bindings/serial/renesas,scif.yaml
+++ b/Documentation/devicetree/bindings/serial/renesas,scif.yaml
@@ -79,6 +79,8 @@ properties:
               - renesas,scif-r9a08g045      # RZ/G3S
           - const: renesas,scif-r9a07g044   # RZ/G2{L,LC} fallback
 
+      - const: renesas,scif-r9a09g057       # RZ/V2H(P)
+
   reg:
     maxItems: 1
 
@@ -204,6 +206,37 @@ allOf:
             - const: dri
             - const: tei
 
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: renesas,scif-r9a09g057
+    then:
+      properties:
+        interrupts:
+          items:
+            - description: Error interrupt
+            - description: Receive buffer full interrupt
+            - description: Transmit buffer empty interrupt
+            - description: Break interrupt
+            - description: Data Ready interrupt
+            - description: Transmit End interrupt
+            - description: Transmit End/Data Ready interrupt
+            - description: Receive buffer full interrupt (EDGE trigger)
+            - description: Transmit buffer empty interrupt (EDGE trigger)
+
+        interrupt-names:
+          items:
+            - const: eri
+            - const: rxi
+            - const: txi
+            - const: bri
+            - const: dri
+            - const: tei
+            - const: tei-dri
+            - const: rxi-edge
+            - const: txi-edge
+
 unevaluatedProperties: false
 
 examples:
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v3 4/4] serial: sh-sci: Add support for RZ/V2H(P) SoC
  2024-03-18 17:20 [PATCH v3 0/4] Add SCIF support for Renesas RZ/V2H(P) SoC Prabhakar
                   ` (2 preceding siblings ...)
  2024-03-18 17:21 ` [PATCH v3 3/4] dt-bindings: serial: renesas,scif: Document R9A09G057 support Prabhakar
@ 2024-03-18 17:21 ` Prabhakar
  2024-03-19  8:21   ` Geert Uytterhoeven
  3 siblings, 1 reply; 18+ messages in thread
From: Prabhakar @ 2024-03-18 17:21 UTC (permalink / raw
  To: Geert Uytterhoeven, Greg Kroah-Hartman, Jiri Slaby, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Magnus Damm
  Cc: linux-kernel, devicetree, linux-serial, linux-renesas-soc,
	Prabhakar, Fabrizio Castro, Lad Prabhakar

From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

Add serial support for RZ/V2H(P) SoC with earlycon.

Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
v2 - > v3
- new patch
---
 drivers/tty/serial/sh-sci.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c
index a85e7b9a2e49..4a60d77257d6 100644
--- a/drivers/tty/serial/sh-sci.c
+++ b/drivers/tty/serial/sh-sci.c
@@ -290,7 +290,7 @@ static const struct sci_port_params sci_port_params[SCIx_NR_REGTYPES] = {
 	},
 
 	/*
-	 * The "SCIFA" that is in RZ/A2, RZ/G2L and RZ/T.
+	 * The "SCIFA" that is in RZ/A2, RZ/G2L, RZ/T and RZ/V2H.
 	 * It looks like a normal SCIF with FIFO data, but with a
 	 * compressed address space. Also, the break out of interrupts
 	 * are different: ERI/BRI, RXI, TXI, TEI, DRI.
@@ -3224,6 +3224,10 @@ static const struct of_device_id of_sci_match[] __maybe_unused = {
 		.compatible = "renesas,scif-r9a07g044",
 		.data = SCI_OF_DATA(PORT_SCIF, SCIx_RZ_SCIFA_REGTYPE),
 	},
+	{
+		.compatible = "renesas,scif-r9a09g057",
+		.data = SCI_OF_DATA(PORT_SCIF, SCIx_RZ_SCIFA_REGTYPE),
+	},
 	/* Family-specific types */
 	{
 		.compatible = "renesas,rcar-gen1-scif",
@@ -3554,6 +3558,7 @@ OF_EARLYCON_DECLARE(sci, "renesas,sci", sci_early_console_setup);
 OF_EARLYCON_DECLARE(scif, "renesas,scif", scif_early_console_setup);
 OF_EARLYCON_DECLARE(scif, "renesas,scif-r7s9210", rzscifa_early_console_setup);
 OF_EARLYCON_DECLARE(scif, "renesas,scif-r9a07g044", rzscifa_early_console_setup);
+OF_EARLYCON_DECLARE(scif, "renesas,scif-r9a09g057", rzscifa_early_console_setup);
 OF_EARLYCON_DECLARE(scifa, "renesas,scifa", scifa_early_console_setup);
 OF_EARLYCON_DECLARE(scifb, "renesas,scifb", scifb_early_console_setup);
 OF_EARLYCON_DECLARE(hscif, "renesas,hscif", hscif_early_console_setup);
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [PATCH v3 2/4] dt-bindings: serial: renesas,scif: Validate 'interrupts' and 'interrupt-names'
  2024-03-18 17:21 ` [PATCH v3 2/4] dt-bindings: serial: renesas,scif: Validate 'interrupts' and 'interrupt-names' Prabhakar
@ 2024-03-19  6:19   ` Krzysztof Kozlowski
  2024-03-19  6:22     ` Krzysztof Kozlowski
  0 siblings, 1 reply; 18+ messages in thread
From: Krzysztof Kozlowski @ 2024-03-19  6:19 UTC (permalink / raw
  To: Prabhakar, Geert Uytterhoeven, Greg Kroah-Hartman, Jiri Slaby,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Magnus Damm
  Cc: linux-kernel, devicetree, linux-serial, linux-renesas-soc,
	Fabrizio Castro, Lad Prabhakar

On 18/03/2024 18:21, Prabhakar wrote:
> From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> 
> Add support to validate the 'interrupts' and 'interrupt-names' properties
> for every supported SoC. This ensures proper handling and configuration of
> interrupt-related properties across supported platforms.
> 
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> ---
> v2->v3
> - Listed interrupts and interrupt-names for every SoC in if check
> ---
>  .../bindings/serial/renesas,scif.yaml         | 95 ++++++++++++-------
>  1 file changed, 63 insertions(+), 32 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/serial/renesas,scif.yaml b/Documentation/devicetree/bindings/serial/renesas,scif.yaml
> index af72c3420453..53f18e9810fd 100644
> --- a/Documentation/devicetree/bindings/serial/renesas,scif.yaml
> +++ b/Documentation/devicetree/bindings/serial/renesas,scif.yaml
> @@ -82,38 +82,6 @@ properties:
>    reg:
>      maxItems: 1
>  
> -  interrupts:

I don't understand what is happening with this patchset. Interrupts must
stay here. Where did you receive any different feedback?


Best regards,
Krzysztof


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v3 2/4] dt-bindings: serial: renesas,scif: Validate 'interrupts' and 'interrupt-names'
  2024-03-19  6:19   ` Krzysztof Kozlowski
@ 2024-03-19  6:22     ` Krzysztof Kozlowski
  2024-03-19 12:43       ` Lad, Prabhakar
  0 siblings, 1 reply; 18+ messages in thread
From: Krzysztof Kozlowski @ 2024-03-19  6:22 UTC (permalink / raw
  To: Prabhakar, Geert Uytterhoeven, Greg Kroah-Hartman, Jiri Slaby,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Magnus Damm
  Cc: linux-kernel, devicetree, linux-serial, linux-renesas-soc,
	Fabrizio Castro, Lad Prabhakar

On 19/03/2024 07:19, Krzysztof Kozlowski wrote:
> On 18/03/2024 18:21, Prabhakar wrote:
>> From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
>>
>> Add support to validate the 'interrupts' and 'interrupt-names' properties
>> for every supported SoC. This ensures proper handling and configuration of
>> interrupt-related properties across supported platforms.
>>
>> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
>> ---
>> v2->v3
>> - Listed interrupts and interrupt-names for every SoC in if check
>> ---
>>  .../bindings/serial/renesas,scif.yaml         | 95 ++++++++++++-------
>>  1 file changed, 63 insertions(+), 32 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/serial/renesas,scif.yaml b/Documentation/devicetree/bindings/serial/renesas,scif.yaml
>> index af72c3420453..53f18e9810fd 100644
>> --- a/Documentation/devicetree/bindings/serial/renesas,scif.yaml
>> +++ b/Documentation/devicetree/bindings/serial/renesas,scif.yaml
>> @@ -82,38 +82,6 @@ properties:
>>    reg:
>>      maxItems: 1
>>  
>> -  interrupts:
> 
> I don't understand what is happening with this patchset. Interrupts must
> stay here. Where did you receive any different feedback?

Look how it is done:
https://elixir.bootlin.com/linux/v6.8/source/Documentation/devicetree/bindings/ufs/qcom,ufs.yaml#L44


Best regards,
Krzysztof


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v3 3/4] dt-bindings: serial: renesas,scif: Document R9A09G057 support
  2024-03-18 17:21 ` [PATCH v3 3/4] dt-bindings: serial: renesas,scif: Document R9A09G057 support Prabhakar
@ 2024-03-19  8:12   ` Geert Uytterhoeven
  2024-03-19 12:48     ` Lad, Prabhakar
  2024-03-20 14:37   ` Rob Herring
  1 sibling, 1 reply; 18+ messages in thread
From: Geert Uytterhoeven @ 2024-03-19  8:12 UTC (permalink / raw
  To: Prabhakar
  Cc: Geert Uytterhoeven, Greg Kroah-Hartman, Jiri Slaby, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Magnus Damm, linux-kernel,
	devicetree, linux-serial, linux-renesas-soc, Fabrizio Castro,
	Lad Prabhakar

Hi Prabhakar,

On Mon, Mar 18, 2024 at 6:22 PM Prabhakar <prabhakar.csengg@gmail.com> wrote:
> From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
>
> Document support for the Serial Communication Interface with FIFO (SCIF)
> available in the Renesas RZ/V2H(P) (R9A09G057) SoC. The SCIF interface in
> the Renesas RZ/V2H(P) is similar to that available in the RZ/G2L
> (R9A07G044) SoC, with the only difference being that the RZ/V2H(P) SoC has
> three additional interrupts: one for Tx end/Rx ready and the other two for
> Rx and Tx buffer full, which are edge-triggered.
>
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> ---
> v2->v3
> - Added SoC specific compat string

Thanks for the update!

> --- a/Documentation/devicetree/bindings/serial/renesas,scif.yaml
> +++ b/Documentation/devicetree/bindings/serial/renesas,scif.yaml
> @@ -79,6 +79,8 @@ properties:
>                - renesas,scif-r9a08g045      # RZ/G3S
>            - const: renesas,scif-r9a07g044   # RZ/G2{L,LC} fallback
>
> +      - const: renesas,scif-r9a09g057       # RZ/V2H(P)
> +
>    reg:
>      maxItems: 1
>
> @@ -204,6 +206,37 @@ allOf:
>              - const: dri
>              - const: tei
>
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: renesas,scif-r9a09g057
> +    then:
> +      properties:
> +        interrupts:
> +          items:
> +            - description: Error interrupt

[...]

These descriptions should be at the top level.  The SoC-specific rules
should just restrict the lower limit (interrupts/minItems).

> +
> +        interrupt-names:
> +          items:
> +            - const: eri

[...]

Likewise.

In addition, you should restrict clocks/maxItems to 1, as on RZ/V2H
only the UART functional clock is supported.

Gr{oetje,eeting}s,

                        Geert


--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v3 4/4] serial: sh-sci: Add support for RZ/V2H(P) SoC
  2024-03-18 17:21 ` [PATCH v3 4/4] serial: sh-sci: Add support for RZ/V2H(P) SoC Prabhakar
@ 2024-03-19  8:21   ` Geert Uytterhoeven
  2024-03-21  9:56     ` Lad, Prabhakar
  0 siblings, 1 reply; 18+ messages in thread
From: Geert Uytterhoeven @ 2024-03-19  8:21 UTC (permalink / raw
  To: Prabhakar
  Cc: Greg Kroah-Hartman, Jiri Slaby, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Magnus Damm, linux-kernel, devicetree, linux-serial,
	linux-renesas-soc, Fabrizio Castro, Lad Prabhakar

Hi Prabhakar,

On Mon, Mar 18, 2024 at 6:22 PM Prabhakar <prabhakar.csengg@gmail.com> wrote:
> From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
>
> Add serial support for RZ/V2H(P) SoC with earlycon.
>
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> ---
> v2 - > v3
> - new patch

Thanks for your patch!

> --- a/drivers/tty/serial/sh-sci.c
> +++ b/drivers/tty/serial/sh-sci.c
> @@ -290,7 +290,7 @@ static const struct sci_port_params sci_port_params[SCIx_NR_REGTYPES] = {
>         },
>
>         /*
> -        * The "SCIFA" that is in RZ/A2, RZ/G2L and RZ/T.
> +        * The "SCIFA" that is in RZ/A2, RZ/G2L, RZ/T and RZ/V2H.
>          * It looks like a normal SCIF with FIFO data, but with a
>          * compressed address space. Also, the break out of interrupts
>          * are different: ERI/BRI, RXI, TXI, TEI, DRI.

and RZ/V2H has more interrupts than RZ/A1, RZ/G2L and RZ/T...

In addition, RZ/V2H does not support synchronous mode (does not matter
for the driver) and modem control signals.

Currently, sci_init_pins() does write ones to the SCPTR bits that are
reserved and marked as "write zero" on RZ/V2H. I am not sure how bad
that is.  You could avoid that by adding a check for .hasrtscts, but
that may have impact on other SoCs/boards, where currently e.g. RTS#
is always programmed for output and active low...

So if you really need to avoid writing to these bits, the only safe
way may be to add a new SCIF type.  But perhaps I'm over-cautious? ;-)

> @@ -3224,6 +3224,10 @@ static const struct of_device_id of_sci_match[] __maybe_unused = {
>                 .compatible = "renesas,scif-r9a07g044",
>                 .data = SCI_OF_DATA(PORT_SCIF, SCIx_RZ_SCIFA_REGTYPE),
>         },
> +       {
> +               .compatible = "renesas,scif-r9a09g057",
> +               .data = SCI_OF_DATA(PORT_SCIF, SCIx_RZ_SCIFA_REGTYPE),
> +       },
>         /* Family-specific types */
>         {
>                 .compatible = "renesas,rcar-gen1-scif",
> @@ -3554,6 +3558,7 @@ OF_EARLYCON_DECLARE(sci, "renesas,sci", sci_early_console_setup);
>  OF_EARLYCON_DECLARE(scif, "renesas,scif", scif_early_console_setup);
>  OF_EARLYCON_DECLARE(scif, "renesas,scif-r7s9210", rzscifa_early_console_setup);
>  OF_EARLYCON_DECLARE(scif, "renesas,scif-r9a07g044", rzscifa_early_console_setup);
> +OF_EARLYCON_DECLARE(scif, "renesas,scif-r9a09g057", rzscifa_early_console_setup);
>  OF_EARLYCON_DECLARE(scifa, "renesas,scifa", scifa_early_console_setup);
>  OF_EARLYCON_DECLARE(scifb, "renesas,scifb", scifb_early_console_setup);
>  OF_EARLYCON_DECLARE(hscif, "renesas,hscif", hscif_early_console_setup);

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v3 2/4] dt-bindings: serial: renesas,scif: Validate 'interrupts' and 'interrupt-names'
  2024-03-19  6:22     ` Krzysztof Kozlowski
@ 2024-03-19 12:43       ` Lad, Prabhakar
  2024-03-19 13:04         ` Krzysztof Kozlowski
  0 siblings, 1 reply; 18+ messages in thread
From: Lad, Prabhakar @ 2024-03-19 12:43 UTC (permalink / raw
  To: Krzysztof Kozlowski
  Cc: Geert Uytterhoeven, Greg Kroah-Hartman, Jiri Slaby, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Magnus Damm, linux-kernel,
	devicetree, linux-serial, linux-renesas-soc, Fabrizio Castro,
	Lad Prabhakar

Hi Krzysztof,

On Tue, Mar 19, 2024 at 6:22 AM Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
>
> On 19/03/2024 07:19, Krzysztof Kozlowski wrote:
> > On 18/03/2024 18:21, Prabhakar wrote:
> >> From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> >>
> >> Add support to validate the 'interrupts' and 'interrupt-names' properties
> >> for every supported SoC. This ensures proper handling and configuration of
> >> interrupt-related properties across supported platforms.
> >>
> >> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> >> ---
> >> v2->v3
> >> - Listed interrupts and interrupt-names for every SoC in if check
> >> ---
> >>  .../bindings/serial/renesas,scif.yaml         | 95 ++++++++++++-------
> >>  1 file changed, 63 insertions(+), 32 deletions(-)
> >>
> >> diff --git a/Documentation/devicetree/bindings/serial/renesas,scif.yaml b/Documentation/devicetree/bindings/serial/renesas,scif.yaml
> >> index af72c3420453..53f18e9810fd 100644
> >> --- a/Documentation/devicetree/bindings/serial/renesas,scif.yaml
> >> +++ b/Documentation/devicetree/bindings/serial/renesas,scif.yaml
> >> @@ -82,38 +82,6 @@ properties:
> >>    reg:
> >>      maxItems: 1
> >>
> >> -  interrupts:
> >
> > I don't understand what is happening with this patchset. Interrupts must
> > stay here. Where did you receive any different feedback?
>
> Look how it is done:
> https://elixir.bootlin.com/linux/v6.8/source/Documentation/devicetree/bindings/ufs/qcom,ufs.yaml#L44
>
Thanks for the pointer, as the above binding doesn't have any
description items as compared to our case, to clarify I have updated
the binding is below. Is this the correct approach?

option #1
---------------
  interrupts:
    minItems: 1
    maxItems: 6

 interrupt-names:
    minItems: 4
    maxItems: 6

  - if:
      properties:
        compatible:
          contains:
            enum:
              - renesas,rcar-gen1-scif
              - renesas,rcar-gen2-scif
              - renesas,rcar-gen3-scif
              - renesas,rcar-gen4-scif
    then:
      properties:
        interrupts:
          items:
            - description: Single combined interrupt

        interrupt-names: false

  - if:
      properties:
        compatible:
          contains:
            const: renesas,scif-r7s72100
    then:
      properties:
        interrupts:
          items:
            - description: Error interrupt
            - description: Receive buffer full interrupt
            - description: Transmit buffer empty interrupt
            - description: Break interrupt

        interrupt-names:
          items:
            - const: eri
            - const: rxi
            - const: txi
            - const: bri
  - if:
      properties:
        compatible:
          contains:
            enum:
              - renesas,scif-r7s9210
              - renesas,scif-r9a07g044
    then:
      properties:
        interrupts:
          items:
            - description: Error interrupt
            - description: Receive buffer full interrupt
            - description: Transmit buffer empty interrupt
            - description: Break interrupt
            - description: Data Ready interrupt
            - description: Transmit End interrupt

        interrupt-names:
          items:
            - const: eri
            - const: rxi
            - const: txi
            - const: bri
            - const: dri
            - const: tei

Cheers,
Prabhakar

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v3 3/4] dt-bindings: serial: renesas,scif: Document R9A09G057 support
  2024-03-19  8:12   ` Geert Uytterhoeven
@ 2024-03-19 12:48     ` Lad, Prabhakar
  0 siblings, 0 replies; 18+ messages in thread
From: Lad, Prabhakar @ 2024-03-19 12:48 UTC (permalink / raw
  To: Geert Uytterhoeven
  Cc: Geert Uytterhoeven, Greg Kroah-Hartman, Jiri Slaby, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Magnus Damm, linux-kernel,
	devicetree, linux-serial, linux-renesas-soc, Fabrizio Castro,
	Lad Prabhakar

Hi Geert,

Thank you for the review.

On Tue, Mar 19, 2024 at 8:12 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>
> Hi Prabhakar,
>
> On Mon, Mar 18, 2024 at 6:22 PM Prabhakar <prabhakar.csengg@gmail.com> wrote:
> > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> >
> > Document support for the Serial Communication Interface with FIFO (SCIF)
> > available in the Renesas RZ/V2H(P) (R9A09G057) SoC. The SCIF interface in
> > the Renesas RZ/V2H(P) is similar to that available in the RZ/G2L
> > (R9A07G044) SoC, with the only difference being that the RZ/V2H(P) SoC has
> > three additional interrupts: one for Tx end/Rx ready and the other two for
> > Rx and Tx buffer full, which are edge-triggered.
> >
> > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > ---
> > v2->v3
> > - Added SoC specific compat string
>
> Thanks for the update!
>
> > --- a/Documentation/devicetree/bindings/serial/renesas,scif.yaml
> > +++ b/Documentation/devicetree/bindings/serial/renesas,scif.yaml
> > @@ -79,6 +79,8 @@ properties:
> >                - renesas,scif-r9a08g045      # RZ/G3S
> >            - const: renesas,scif-r9a07g044   # RZ/G2{L,LC} fallback
> >
> > +      - const: renesas,scif-r9a09g057       # RZ/V2H(P)
> > +
> >    reg:
> >      maxItems: 1
> >
> > @@ -204,6 +206,37 @@ allOf:
> >              - const: dri
> >              - const: tei
> >
> > +  - if:
> > +      properties:
> > +        compatible:
> > +          contains:
> > +            const: renesas,scif-r9a09g057
> > +    then:
> > +      properties:
> > +        interrupts:
> > +          items:
> > +            - description: Error interrupt
>
> [...]
>
> These descriptions should be at the top level.  The SoC-specific rules
> should just restrict the lower limit (interrupts/minItems).
>
I think I'm misunderstanding here. As per patch 2/4 the DT maintainer
wants properties at top level with just minItems/maxItems and have SoC
specific listed in the checks (as pointed out to me like [0])

[0] https://elixir.bootlin.com/linux/v6.8/source/Documentation/devicetree/bindings/ufs/qcom,ufs.yaml#L48

> > +
> > +        interrupt-names:
> > +          items:
> > +            - const: eri
>
> [...]
>
> Likewise.
>
> In addition, you should restrict clocks/maxItems to 1, as on RZ/V2H
> only the UART functional clock is supported.
>
Agreed.

Cheers,
Prabhakar

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v3 2/4] dt-bindings: serial: renesas,scif: Validate 'interrupts' and 'interrupt-names'
  2024-03-19 12:43       ` Lad, Prabhakar
@ 2024-03-19 13:04         ` Krzysztof Kozlowski
  2024-03-19 13:25           ` Geert Uytterhoeven
  0 siblings, 1 reply; 18+ messages in thread
From: Krzysztof Kozlowski @ 2024-03-19 13:04 UTC (permalink / raw
  To: Lad, Prabhakar
  Cc: Geert Uytterhoeven, Greg Kroah-Hartman, Jiri Slaby, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Magnus Damm, linux-kernel,
	devicetree, linux-serial, linux-renesas-soc, Fabrizio Castro,
	Lad Prabhakar

On 19/03/2024 13:43, Lad, Prabhakar wrote:
>>>> diff --git a/Documentation/devicetree/bindings/serial/renesas,scif.yaml b/Documentation/devicetree/bindings/serial/renesas,scif.yaml
>>>> index af72c3420453..53f18e9810fd 100644
>>>> --- a/Documentation/devicetree/bindings/serial/renesas,scif.yaml
>>>> +++ b/Documentation/devicetree/bindings/serial/renesas,scif.yaml
>>>> @@ -82,38 +82,6 @@ properties:
>>>>    reg:
>>>>      maxItems: 1
>>>>
>>>> -  interrupts:
>>>
>>> I don't understand what is happening with this patchset. Interrupts must
>>> stay here. Where did you receive any different feedback?
>>
>> Look how it is done:
>> https://elixir.bootlin.com/linux/v6.8/source/Documentation/devicetree/bindings/ufs/qcom,ufs.yaml#L44
>>
> Thanks for the pointer, as the above binding doesn't have any

Yeah, that's just an example to point you the concept: top level
property comes with widest constraints (or widest matching items
description) and each variant narrows the choice.

> description items as compared to our case, to clarify I have updated
> the binding is below. Is this the correct approach?
> 
> option #1
> ---------------


Yes, it looks correct.

Best regards,
Krzysztof


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v3 2/4] dt-bindings: serial: renesas,scif: Validate 'interrupts' and 'interrupt-names'
  2024-03-19 13:04         ` Krzysztof Kozlowski
@ 2024-03-19 13:25           ` Geert Uytterhoeven
  2024-03-20  8:06             ` Krzysztof Kozlowski
  0 siblings, 1 reply; 18+ messages in thread
From: Geert Uytterhoeven @ 2024-03-19 13:25 UTC (permalink / raw
  To: Krzysztof Kozlowski
  Cc: Lad, Prabhakar, Geert Uytterhoeven, Greg Kroah-Hartman,
	Jiri Slaby, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Magnus Damm, linux-kernel, devicetree, linux-serial,
	linux-renesas-soc, Fabrizio Castro, Lad Prabhakar

Hi Krzysztof,

On Tue, Mar 19, 2024 at 2:04 PM Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
> On 19/03/2024 13:43, Lad, Prabhakar wrote:
> >>>> diff --git a/Documentation/devicetree/bindings/serial/renesas,scif.yaml b/Documentation/devicetree/bindings/serial/renesas,scif.yaml
> >>>> index af72c3420453..53f18e9810fd 100644
> >>>> --- a/Documentation/devicetree/bindings/serial/renesas,scif.yaml
> >>>> +++ b/Documentation/devicetree/bindings/serial/renesas,scif.yaml
> >>>> @@ -82,38 +82,6 @@ properties:
> >>>>    reg:
> >>>>      maxItems: 1
> >>>>
> >>>> -  interrupts:
> >>>
> >>> I don't understand what is happening with this patchset. Interrupts must
> >>> stay here. Where did you receive any different feedback?
> >>
> >> Look how it is done:
> >> https://elixir.bootlin.com/linux/v6.8/source/Documentation/devicetree/bindings/ufs/qcom,ufs.yaml#L44
> >>
> > Thanks for the pointer, as the above binding doesn't have any
>
> Yeah, that's just an example to point you the concept: top level
> property comes with widest constraints (or widest matching items
> description) and each variant narrows the choice.
>
> > description items as compared to our case, to clarify I have updated
> > the binding is below. Is this the correct approach?
> >
> > option #1
> > ---------------
>
>
> Yes, it looks correct.

Why duplicate all the descriptions? The only differences are the number
of valid interrupts?
What was wrong with "[PATCH v2 2/2] dt-bindings: serial: renesas,scif:
Validate 'interrupts' and 'interrupt-names'"[1]?

https://lore.kernel.org/r/20240307114217.34784-3-prabhakar.mahadev-lad.rj@bp.renesas.com/

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v3 2/4] dt-bindings: serial: renesas,scif: Validate 'interrupts' and 'interrupt-names'
  2024-03-19 13:25           ` Geert Uytterhoeven
@ 2024-03-20  8:06             ` Krzysztof Kozlowski
  2024-03-20  8:10               ` Geert Uytterhoeven
  0 siblings, 1 reply; 18+ messages in thread
From: Krzysztof Kozlowski @ 2024-03-20  8:06 UTC (permalink / raw
  To: Geert Uytterhoeven
  Cc: Lad, Prabhakar, Geert Uytterhoeven, Greg Kroah-Hartman,
	Jiri Slaby, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Magnus Damm, linux-kernel, devicetree, linux-serial,
	linux-renesas-soc, Fabrizio Castro, Lad Prabhakar

On 19/03/2024 14:25, Geert Uytterhoeven wrote:
> Hi Krzysztof,
> 
> On Tue, Mar 19, 2024 at 2:04 PM Krzysztof Kozlowski
> <krzysztof.kozlowski@linaro.org> wrote:
>> On 19/03/2024 13:43, Lad, Prabhakar wrote:
>>>>>> diff --git a/Documentation/devicetree/bindings/serial/renesas,scif.yaml b/Documentation/devicetree/bindings/serial/renesas,scif.yaml
>>>>>> index af72c3420453..53f18e9810fd 100644
>>>>>> --- a/Documentation/devicetree/bindings/serial/renesas,scif.yaml
>>>>>> +++ b/Documentation/devicetree/bindings/serial/renesas,scif.yaml
>>>>>> @@ -82,38 +82,6 @@ properties:
>>>>>>    reg:
>>>>>>      maxItems: 1
>>>>>>
>>>>>> -  interrupts:
>>>>>
>>>>> I don't understand what is happening with this patchset. Interrupts must
>>>>> stay here. Where did you receive any different feedback?
>>>>
>>>> Look how it is done:
>>>> https://elixir.bootlin.com/linux/v6.8/source/Documentation/devicetree/bindings/ufs/qcom,ufs.yaml#L44
>>>>
>>> Thanks for the pointer, as the above binding doesn't have any
>>
>> Yeah, that's just an example to point you the concept: top level
>> property comes with widest constraints (or widest matching items
>> description) and each variant narrows the choice.
>>
>>> description items as compared to our case, to clarify I have updated
>>> the binding is below. Is this the correct approach?
>>>
>>> option #1
>>> ---------------
>>
>>
>> Yes, it looks correct.
> 
> Why duplicate all the descriptions? The only differences are the number
> of valid interrupts?
> What was wrong with "[PATCH v2 2/2] dt-bindings: serial: renesas,scif:
> Validate 'interrupts' and 'interrupt-names'"[1]?
> 
> https://lore.kernel.org/r/20240307114217.34784-3-prabhakar.mahadev-lad.rj@bp.renesas.com/

I have impression that only two variants out of three have same
descriptions... but now I see mistake I made in above. I read that first
interrupt is "Error interrupt" but it is "error or combined". Sorry for
that, I think most of my comment there is not correct.

It could be made oneOf?

    oneOf:
     - items:
          - description: A combined interrupt
     - items:
         - ....
       minItems: 4
?



Best regards,
Krzysztof


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v3 2/4] dt-bindings: serial: renesas,scif: Validate 'interrupts' and 'interrupt-names'
  2024-03-20  8:06             ` Krzysztof Kozlowski
@ 2024-03-20  8:10               ` Geert Uytterhoeven
  0 siblings, 0 replies; 18+ messages in thread
From: Geert Uytterhoeven @ 2024-03-20  8:10 UTC (permalink / raw
  To: Krzysztof Kozlowski
  Cc: Lad, Prabhakar, Geert Uytterhoeven, Greg Kroah-Hartman,
	Jiri Slaby, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Magnus Damm, linux-kernel, devicetree, linux-serial,
	linux-renesas-soc, Fabrizio Castro, Lad Prabhakar

Hi Krzysztof,

On Wed, Mar 20, 2024 at 9:06 AM Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
> On 19/03/2024 14:25, Geert Uytterhoeven wrote:
> > On Tue, Mar 19, 2024 at 2:04 PM Krzysztof Kozlowski
> > <krzysztof.kozlowski@linaro.org> wrote:
> >> On 19/03/2024 13:43, Lad, Prabhakar wrote:
> >>>>>> diff --git a/Documentation/devicetree/bindings/serial/renesas,scif.yaml b/Documentation/devicetree/bindings/serial/renesas,scif.yaml
> >>>>>> index af72c3420453..53f18e9810fd 100644
> >>>>>> --- a/Documentation/devicetree/bindings/serial/renesas,scif.yaml
> >>>>>> +++ b/Documentation/devicetree/bindings/serial/renesas,scif.yaml
> >>>>>> @@ -82,38 +82,6 @@ properties:
> >>>>>>    reg:
> >>>>>>      maxItems: 1
> >>>>>>
> >>>>>> -  interrupts:
> >>>>>
> >>>>> I don't understand what is happening with this patchset. Interrupts must
> >>>>> stay here. Where did you receive any different feedback?
> >>>>
> >>>> Look how it is done:
> >>>> https://elixir.bootlin.com/linux/v6.8/source/Documentation/devicetree/bindings/ufs/qcom,ufs.yaml#L44
> >>>>
> >>> Thanks for the pointer, as the above binding doesn't have any
> >>
> >> Yeah, that's just an example to point you the concept: top level
> >> property comes with widest constraints (or widest matching items
> >> description) and each variant narrows the choice.
> >>
> >>> description items as compared to our case, to clarify I have updated
> >>> the binding is below. Is this the correct approach?
> >>>
> >>> option #1
> >>> ---------------
> >>
> >>
> >> Yes, it looks correct.
> >
> > Why duplicate all the descriptions? The only differences are the number
> > of valid interrupts?
> > What was wrong with "[PATCH v2 2/2] dt-bindings: serial: renesas,scif:
> > Validate 'interrupts' and 'interrupt-names'"[1]?
> >
> > https://lore.kernel.org/r/20240307114217.34784-3-prabhakar.mahadev-lad.rj@bp.renesas.com/
>
> I have impression that only two variants out of three have same
> descriptions... but now I see mistake I made in above. I read that first
> interrupt is "Error interrupt" but it is "error or combined". Sorry for
> that, I think most of my comment there is not correct.
>
> It could be made oneOf?
>
>     oneOf:
>      - items:
>           - description: A combined interrupt
>      - items:
>          - ....
>        minItems: 4
> ?

Yes, that would be even better.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v3 3/4] dt-bindings: serial: renesas,scif: Document R9A09G057 support
  2024-03-18 17:21 ` [PATCH v3 3/4] dt-bindings: serial: renesas,scif: Document R9A09G057 support Prabhakar
  2024-03-19  8:12   ` Geert Uytterhoeven
@ 2024-03-20 14:37   ` Rob Herring
  2024-03-21 10:48     ` Lad, Prabhakar
  1 sibling, 1 reply; 18+ messages in thread
From: Rob Herring @ 2024-03-20 14:37 UTC (permalink / raw
  To: Prabhakar
  Cc: Geert Uytterhoeven, Greg Kroah-Hartman, Jiri Slaby,
	Krzysztof Kozlowski, Conor Dooley, Magnus Damm, linux-kernel,
	devicetree, linux-serial, linux-renesas-soc, Fabrizio Castro,
	Lad Prabhakar

On Mon, Mar 18, 2024 at 05:21:01PM +0000, Prabhakar wrote:
> From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> 
> Document support for the Serial Communication Interface with FIFO (SCIF)
> available in the Renesas RZ/V2H(P) (R9A09G057) SoC. The SCIF interface in
> the Renesas RZ/V2H(P) is similar to that available in the RZ/G2L
> (R9A07G044) SoC, with the only difference being that the RZ/V2H(P) SoC has
> three additional interrupts: one for Tx end/Rx ready and the other two for
> Rx and Tx buffer full, which are edge-triggered.
> 
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> ---
> v2->v3
> - Added SoC specific compat string
> ---
>  .../bindings/serial/renesas,scif.yaml         | 33 +++++++++++++++++++
>  1 file changed, 33 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/serial/renesas,scif.yaml b/Documentation/devicetree/bindings/serial/renesas,scif.yaml
> index 53f18e9810fd..e4ce13e20cd7 100644
> --- a/Documentation/devicetree/bindings/serial/renesas,scif.yaml
> +++ b/Documentation/devicetree/bindings/serial/renesas,scif.yaml
> @@ -79,6 +79,8 @@ properties:
>                - renesas,scif-r9a08g045      # RZ/G3S
>            - const: renesas,scif-r9a07g044   # RZ/G2{L,LC} fallback
>  
> +      - const: renesas,scif-r9a09g057       # RZ/V2H(P)

I don't understand why there's not a fallback. Looks like the existing 
driver would work if you had one. It should be fine to ignore the new 
interrupts. Though with Geert's comments, it seems there are more 
differences than you say. 

Rob

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v3 4/4] serial: sh-sci: Add support for RZ/V2H(P) SoC
  2024-03-19  8:21   ` Geert Uytterhoeven
@ 2024-03-21  9:56     ` Lad, Prabhakar
  0 siblings, 0 replies; 18+ messages in thread
From: Lad, Prabhakar @ 2024-03-21  9:56 UTC (permalink / raw
  To: Geert Uytterhoeven
  Cc: Greg Kroah-Hartman, Jiri Slaby, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Magnus Damm, linux-kernel, devicetree, linux-serial,
	linux-renesas-soc, Fabrizio Castro, Lad Prabhakar

Hi Geert,

Thank you for the review.

On Tue, Mar 19, 2024 at 8:21 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>
> Hi Prabhakar,
>
> On Mon, Mar 18, 2024 at 6:22 PM Prabhakar <prabhakar.csengg@gmail.com> wrote:
> > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> >
> > Add serial support for RZ/V2H(P) SoC with earlycon.
> >
> > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > ---
> > v2 - > v3
> > - new patch
>
> Thanks for your patch!
>
> > --- a/drivers/tty/serial/sh-sci.c
> > +++ b/drivers/tty/serial/sh-sci.c
> > @@ -290,7 +290,7 @@ static const struct sci_port_params sci_port_params[SCIx_NR_REGTYPES] = {
> >         },
> >
> >         /*
> > -        * The "SCIFA" that is in RZ/A2, RZ/G2L and RZ/T.
> > +        * The "SCIFA" that is in RZ/A2, RZ/G2L, RZ/T and RZ/V2H.
> >          * It looks like a normal SCIF with FIFO data, but with a
> >          * compressed address space. Also, the break out of interrupts
> >          * are different: ERI/BRI, RXI, TXI, TEI, DRI.
>
> and RZ/V2H has more interrupts than RZ/A1, RZ/G2L and RZ/T...
>
> In addition, RZ/V2H does not support synchronous mode (does not matter
> for the driver) and modem control signals.
>
> Currently, sci_init_pins() does write ones to the SCPTR bits that are
> reserved and marked as "write zero" on RZ/V2H. I am not sure how bad
> that is.  You could avoid that by adding a check for .hasrtscts, but
> that may have impact on other SoCs/boards, where currently e.g. RTS#
> is always programmed for output and active low...
>
Oops I had totally missed this difference, thanks for catching that.

> So if you really need to avoid writing to these bits, the only safe
> way may be to add a new SCIF type.  But perhaps I'm over-cautious? ;-)
>
As we are adding a SoC specific compat string we can update this if we
see an issue, but I'm happy to do that change now too. Please let me
know what would you prefer.

Cheers,
Prabhakar
> > @@ -3224,6 +3224,10 @@ static const struct of_device_id of_sci_match[] __maybe_unused = {
> >                 .compatible = "renesas,scif-r9a07g044",
> >                 .data = SCI_OF_DATA(PORT_SCIF, SCIx_RZ_SCIFA_REGTYPE),
> >         },
> > +       {
> > +               .compatible = "renesas,scif-r9a09g057",
> > +               .data = SCI_OF_DATA(PORT_SCIF, SCIx_RZ_SCIFA_REGTYPE),
> > +       },
> >         /* Family-specific types */
> >         {
> >                 .compatible = "renesas,rcar-gen1-scif",
> > @@ -3554,6 +3558,7 @@ OF_EARLYCON_DECLARE(sci, "renesas,sci", sci_early_console_setup);
> >  OF_EARLYCON_DECLARE(scif, "renesas,scif", scif_early_console_setup);
> >  OF_EARLYCON_DECLARE(scif, "renesas,scif-r7s9210", rzscifa_early_console_setup);
> >  OF_EARLYCON_DECLARE(scif, "renesas,scif-r9a07g044", rzscifa_early_console_setup);
> > +OF_EARLYCON_DECLARE(scif, "renesas,scif-r9a09g057", rzscifa_early_console_setup);
> >  OF_EARLYCON_DECLARE(scifa, "renesas,scifa", scifa_early_console_setup);
> >  OF_EARLYCON_DECLARE(scifb, "renesas,scifb", scifb_early_console_setup);
> >  OF_EARLYCON_DECLARE(hscif, "renesas,hscif", hscif_early_console_setup);
>
> Gr{oetje,eeting}s,
>
>                         Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
>
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
>                                 -- Linus Torvalds

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v3 3/4] dt-bindings: serial: renesas,scif: Document R9A09G057 support
  2024-03-20 14:37   ` Rob Herring
@ 2024-03-21 10:48     ` Lad, Prabhakar
  0 siblings, 0 replies; 18+ messages in thread
From: Lad, Prabhakar @ 2024-03-21 10:48 UTC (permalink / raw
  To: Rob Herring
  Cc: Geert Uytterhoeven, Greg Kroah-Hartman, Jiri Slaby,
	Krzysztof Kozlowski, Conor Dooley, Magnus Damm, linux-kernel,
	devicetree, linux-serial, linux-renesas-soc, Fabrizio Castro,
	Lad Prabhakar

Hi Rob,

Thank you for the review.

On Wed, Mar 20, 2024 at 2:37 PM Rob Herring <robh@kernel.org> wrote:
>
> On Mon, Mar 18, 2024 at 05:21:01PM +0000, Prabhakar wrote:
> > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> >
> > Document support for the Serial Communication Interface with FIFO (SCIF)
> > available in the Renesas RZ/V2H(P) (R9A09G057) SoC. The SCIF interface in
> > the Renesas RZ/V2H(P) is similar to that available in the RZ/G2L
> > (R9A07G044) SoC, with the only difference being that the RZ/V2H(P) SoC has
> > three additional interrupts: one for Tx end/Rx ready and the other two for
> > Rx and Tx buffer full, which are edge-triggered.
> >
> > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > ---
> > v2->v3
> > - Added SoC specific compat string
> > ---
> >  .../bindings/serial/renesas,scif.yaml         | 33 +++++++++++++++++++
> >  1 file changed, 33 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/serial/renesas,scif.yaml b/Documentation/devicetree/bindings/serial/renesas,scif.yaml
> > index 53f18e9810fd..e4ce13e20cd7 100644
> > --- a/Documentation/devicetree/bindings/serial/renesas,scif.yaml
> > +++ b/Documentation/devicetree/bindings/serial/renesas,scif.yaml
> > @@ -79,6 +79,8 @@ properties:
> >                - renesas,scif-r9a08g045      # RZ/G3S
> >            - const: renesas,scif-r9a07g044   # RZ/G2{L,LC} fallback
> >
> > +      - const: renesas,scif-r9a09g057       # RZ/V2H(P)
>
> I don't understand why there's not a fallback. Looks like the existing
> driver would work if you had one. It should be fine to ignore the new
> interrupts. Though with Geert's comments, it seems there are more
> differences than you say.
>
Apart from the interrupt differences there are some register bit
differences too (as pointed by Geert in patch 4/4).

Cheers,
Prabhakar

^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2024-03-21 10:48 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-03-18 17:20 [PATCH v3 0/4] Add SCIF support for Renesas RZ/V2H(P) SoC Prabhakar
2024-03-18 17:20 ` [PATCH v3 1/4] dt-bindings: serial: renesas,scif: Move ref for serial.yaml at the end Prabhakar
2024-03-18 17:21 ` [PATCH v3 2/4] dt-bindings: serial: renesas,scif: Validate 'interrupts' and 'interrupt-names' Prabhakar
2024-03-19  6:19   ` Krzysztof Kozlowski
2024-03-19  6:22     ` Krzysztof Kozlowski
2024-03-19 12:43       ` Lad, Prabhakar
2024-03-19 13:04         ` Krzysztof Kozlowski
2024-03-19 13:25           ` Geert Uytterhoeven
2024-03-20  8:06             ` Krzysztof Kozlowski
2024-03-20  8:10               ` Geert Uytterhoeven
2024-03-18 17:21 ` [PATCH v3 3/4] dt-bindings: serial: renesas,scif: Document R9A09G057 support Prabhakar
2024-03-19  8:12   ` Geert Uytterhoeven
2024-03-19 12:48     ` Lad, Prabhakar
2024-03-20 14:37   ` Rob Herring
2024-03-21 10:48     ` Lad, Prabhakar
2024-03-18 17:21 ` [PATCH v3 4/4] serial: sh-sci: Add support for RZ/V2H(P) SoC Prabhakar
2024-03-19  8:21   ` Geert Uytterhoeven
2024-03-21  9:56     ` Lad, Prabhakar

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.