Hi Thierry, On Wed, 29 Jan 2025 at 17:56, Thierry Bultel <thierry.bultel.yh@xxxxxxxxxxxxxx> wrote: > The aim here is to provide an easier support to more different SCI > controllers, like the RZ/T2H one. > > The existing .data field of_sci_match is changed to a structure containing > all what that can be statically initialized, and avoid a call to > 'sci_probe_regmap', in both 'sci_init_single', and 'early_console_setup'. > > 'sci_probe_regmap' is now assumed to be called in the only case where the > device description is from a board file instead of a dts. > > In this way, there is no need to patch 'sci_probe_regmap' for adding new > SCI type, and also, the specific sci_port_params for a new SCI type can be > provided by an external file. > > Signed-off-by: Thierry Bultel <thierry.bultel.yh@xxxxxxxxxxxxxx> Thanks for your patch! > --- a/drivers/tty/serial/sh-sci.c > +++ b/drivers/tty/serial/sh-sci.c > @@ -2968,9 +2968,7 @@ static int sci_init_single(struct platform_device *dev, > int ret; > > sci_port->cfg = p; > - sci_port->ops = &sci_port_ops; > > - port->ops = &sci_uart_ops; This relies on sci_parse_dt() having filled in both ops, which is not done in the non-DT case (i.e. legacy SuperH). > port->iotype = UPIO_MEM; > port->line = index; > port->has_sysrq = IS_ENABLED(CONFIG_SERIAL_SH_SCI_CONSOLE); > @@ -3010,7 +3008,16 @@ static int sci_init_single(struct platform_device *dev, > for (i = 1; i < ARRAY_SIZE(sci_port->irqs); i++) > sci_port->irqs[i] = sci_port->irqs[0]; > > - sci_port->params = sci_probe_regmap(p); > + /* > + * sci_port->params params can be NULL when using a board file instead > + * of a dts. > + */ > + if (sci_port->params == NULL) { > + sci_port->params = sci_probe_regmap(p); ... hence sci_probe_regmap() should fill in the ops. > + if (unlikely(sci_port->params == NULL)) > + return -EINVAL; This case is already handled below. > + } > + > if (unlikely(sci_port->params == NULL)) > return -EINVAL; > > @@ -3336,7 +3348,7 @@ static struct plat_sci_port *sci_parse_dt(struct platform_device *pdev, > struct reset_control *rstc; > struct plat_sci_port *p; > struct sci_port *sp; > - const void *data; > + const struct sci_of_data *data; > int id, ret; > > if (!IS_ENABLED(CONFIG_OF) || !np) > @@ -3382,8 +3394,12 @@ static struct plat_sci_port *sci_parse_dt(struct platform_device *pdev, > sp = &sci_ports[id]; > *dev_id = id; > > - p->type = SCI_OF_TYPE(data); > - p->regtype = SCI_OF_REGTYPE(data); > + p->type = data->type; > + p->regtype = data->regtype; > + > + sp->ops = data->ops; > + sp->port.ops = data->uart_ops; > + sp->params = data->params; > > sp->has_rtscts = of_property_read_bool(np, "uart-has-rtscts"); > > --- a/drivers/tty/serial/sh-sci_common.h > +++ b/drivers/tty/serial/sh-sci_common.h "struct sci_of_data" should be introduced here, instead of in the previous patch. > @@ -172,7 +172,8 @@ extern void sci_flush_buffer(struct uart_port *port); > #define max_sr(_port) fls((_port)->sampling_rate_mask) > > #ifdef CONFIG_SERIAL_SH_SCI_EARLYCON > -extern int __init early_console_setup(struct earlycon_device *device, int); > +extern int __init early_console_setup(struct earlycon_device *device, > + const struct sci_of_data *data); > #endif > > #endif /* __SH_SCI_COMMON_H__ */ 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