[PATCH v3 0/4] Add SCIF support for Renesas RZ/V2H(P) SoC

[PATCH v3 0/4] Add SCIF support for Renesas RZ/V2H(P) SoC

[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

[PATCH v3 1/4] dt-bindings: serial: renesas,scif: Move ref for serial.yaml at the end

[Prabhakar]

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

[PATCH v3 2/4] dt-bindings: serial: renesas,scif: Validate 'interrupts' and 'interrupt-names'

[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

Re: [PATCH v3 2/4] dt-bindings: serial: renesas,scif: Validate 'interrupts' and 'interrupt-names'

[Krzysztof Kozlowski]

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

Re: [PATCH v3 2/4] dt-bindings: serial: renesas,scif: Validate 'interrupts' and 'interrupt-names'

[Krzysztof Kozlowski]

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

Re: [PATCH v3 2/4] dt-bindings: serial: renesas,scif: Validate 'interrupts' and 'interrupt-names'

[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

Re: [PATCH v3 2/4] dt-bindings: serial: renesas,scif: Validate 'interrupts' and 'interrupt-names'

[Krzysztof Kozlowski]

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

Re: [PATCH v3 2/4] dt-bindings: serial: renesas,scif: Validate 'interrupts' and 'interrupt-names'

[Geert Uytterhoeven]

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

Re: [PATCH v3 2/4] dt-bindings: serial: renesas,scif: Validate 'interrupts' and 'interrupt-names'

[Krzysztof Kozlowski]

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

Re: [PATCH v3 2/4] dt-bindings: serial: renesas,scif: Validate 'interrupts' and 'interrupt-names'

[Geert Uytterhoeven]

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

[PATCH v3 3/4] dt-bindings: serial: renesas,scif: Document R9A09G057 support

[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

Re: [PATCH v3 3/4] dt-bindings: serial: renesas,scif: Document R9A09G057 support

[Geert Uytterhoeven]

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

Re: [PATCH v3 3/4] dt-bindings: serial: renesas,scif: Document R9A09G057 support

[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

Re: [PATCH v3 3/4] dt-bindings: serial: renesas,scif: Document R9A09G057 support

[Rob Herring]

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

Re: [PATCH v3 3/4] dt-bindings: serial: renesas,scif: Document R9A09G057 support

[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

[PATCH v3 4/4] serial: sh-sci: Add support for RZ/V2H(P) SoC

[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

Re: [PATCH v3 4/4] serial: sh-sci: Add support for RZ/V2H(P) SoC

[Geert Uytterhoeven]

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

Re: [PATCH v3 4/4] serial: sh-sci: Add support for RZ/V2H(P) SoC

[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