Re: [PATCH v4 08/13] serial: sh-sci: Introduced function pointers

[Geert Uytterhoeven]

On Mon, 24 Mar 2025 at 10:25, Wolfram Sang
<wsa+renesas@xxxxxxxxxxxxxxxxxxxx> wrote:
> On Thu, Mar 06, 2025 at 04:24:42PM +0100, Thierry Bultel wrote:
> > The aim here is to prepare support for new sci controllers like
> > the T2H/RSCI whose registers are too much different for being
> > handled in common code.
> >
> > This named serial controller also has 32 bits register,
> > so some return types had to be changed.
> >
> > The needed generic functions are no longer static, with prototypes
> > defined in sh-sci-common.h so that they can be used from specific
> > implementation in a separate file, to keep this driver as little
> > changed as possible.
> >
> > For doing so, a set of 'ops' is added to struct sci_port.
> >
> > Signed-off-by: Thierry Bultel <thierry.bultel.yh@xxxxxxxxxxxxxx>
>
> Okay, the discussion about the general approach convinced me that we can
> go this road. I will not do a line-by-line review of these patches, but
> just check that it looks good to me in general. This patch here merely
> shuffles code around and adds some inderection. If it works, it seems
> good enough for me and we can improve on it incrementally:
>
> Reviewed-by: Wolfram Sang <wsa+renesas@xxxxxxxxxxxxxxxxxxxx>
>
> That means, though, that testing this series on a variety of SoCs is
> especially important and I'd like to get confirmed that you did these
> tests on SCI variations which are available on RZ hardware. According to
> my research it would be those:
>
>         [SCIx_SCI_REGTYPE]
>                 /* RZ/Five, RZ/G2UL, RZ/V2L */
>                 .compatible = "renesas,sci",

Tested as console on RZ/Five...

>         [SCIx_RZ_SCIFA_REGTYPE]
>                  /* The "SCIFA" that is in RZ/A2, RZ/G2L and RZ/T1 */
>                 .compatible = "renesas,scif-r7s9210",
>                 .compatible = "renesas,scif-r9a07g044",

...RZA2MEVB...

>         [SCIx_SH4_SCIF_BRG_REGTYPE]
>                 /* a lot of RZ, too */
>                 .compatible = "renesas,rcar-gen1-scif",
>                 .compatible = "renesas,rcar-gen2-scif",
>                 .compatible = "renesas,rcar-gen3-scif",
>                 .compatible = "renesas,rcar-gen4-scif",

...R-Car Gen1/2/3...

>
>         [SCIx_HSCIF_REGTYPE]
>                 /* R-Car Gen2-5 */

... R-Car Gen3/4.

>                 /* a lot of RZ */
>                 .compatible = "renesas,hscif",
>
> Please double check that I did not make a mistake. I'd think Geert tests
> these on in his board farm anyway:
>
>         [SCIx_SH4_SCIF_REGTYPE]
>                 /* landisk */
>                 .compatible = "renesas,scif",

Tested as console on Landisk and QEMU RTS7751R2D (both without DT,
though)...

>
>         [SCIx_SCIFA_REGTYPE]
>                 /* R-Car Gen2 */
>                 .compatible = "renesas,scifa",

... SH/R-Mobile...

>         [SCIx_SCIFB_REGTYPE]
>                 /* R-Car Gen2 */
>                 .compatible = "renesas,scifb",

Not tested. SCIFB is very similar to SCIFA, so I am confident it is OK
(all tests for PORT_SCIFA and PORT_SCIFB come in pairs).

>         [SCIx_SH2_SCIF_FIFODATA_REGTYPE]
>                 /* RZ/A1 */
>                 .compatible = "renesas,scif-r7s72100",

Tested as console on RSK+RZA1.

> We maybe can get hold of the next board. I will figure this out
> internally (not super important for this series, but nice to have):
>
>         [SCIx_SH4_SCIF_NO_SCSPTR_REGTYPE]
>         /* SH Ecovec */
>         arch/sh/kernel/cpu/sh4a/setup-sh7723.c: .regtype        = SCIx_SH4_SCIF_NO_SCSPTR_REGTYPE,
>
> That leaves some older SH boards out of the loop, but I think this is
> OK. A quick research didn't let me obtain boards for these anymore.

Tested-by: Geert Uytterhoeven <geert+renesas@xxxxxxxxx>

Reviewed-by: Geert Uytterhoeven <geert+renesas@xxxxxxxxx>

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx

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