| Message ID | 20240318172102.45549-4-prabhakar.mahadev-lad.rj@bp.renesas.com |
|---|---|
| State | New |
| Headers | show |
| Series | Add SCIF support for Renesas RZ/V2H(P) SoC | expand |
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
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
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
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
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: