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

[Wolfram Sang]

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",

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

	[SCIx_RZV2H_SCIF_REGTYPE]
		 /* RZ/V2H */
		.compatible = "renesas,scif-r9a09g057",

	[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",

	[SCIx_HSCIF_REGTYPE]
		/* R-Car Gen2-5 */
		/* 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",

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

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

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

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.

So far, so good? Comments?

Happy hacking,

   Wolfram

Attachment: signature.asc
Description: PGP signature