diff mbox series

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

Message ID 20240318172102.45549-5-prabhakar.mahadev-lad.rj@bp.renesas.com (mailing list archive)
State Superseded
Delegated to: Geert Uytterhoeven
Headers show
Series Add SCIF support for Renesas RZ/V2H(P) SoC | expand

Commit Message

Lad, Prabhakar March 18, 2024, 5:21 p.m. UTC
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(-)

Comments

Geert Uytterhoeven March 19, 2024, 8:21 a.m. UTC | #1
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
Lad, Prabhakar March 21, 2024, 9:56 a.m. UTC | #2
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
diff mbox series

Patch

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);