| Message ID | 20250217105354.551788-10-thierry.bultel.yh@bp.renesas.com (mailing list archive) |
|---|---|
| State | Under Review |
| Delegated to: | Geert Uytterhoeven |
| Headers | show |
| Series | add initial support for Renesas RZ/T2H SoC | expand |
Hi Thierry, On Mon, 17 Feb 2025 at 12:04, Thierry Bultel <thierry.bultel.yh@bp.renesas.com> 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@bp.renesas.com> Thanks for your patch! Please rebase this on top of commit c1117a2fefbcce30 ("serial: sh-sci: Use plain struct copy in early_console_setup()") in v6.14-rc1, so it applies. Gr{oetje,eeting}s, Geert
Hi Thierry, On Mon, 17 Feb 2025 at 12:04, Thierry Bultel <thierry.bultel.yh@bp.renesas.com> 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@bp.renesas.com> Thanks for your patch! > --- a/drivers/tty/serial/sh-sci.c > +++ b/drivers/tty/serial/sh-sci.c > @@ -3009,7 +3010,6 @@ 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); > if (unlikely(sci_port->params == NULL)) > return -EINVAL; Ideally, this check can be removed here... (see below) > > @@ -3264,9 +3264,14 @@ static void sci_remove(struct platform_device *dev) > device_remove_file(&dev->dev, &dev_attr_rx_fifo_timeout); > } > > -#define SCI_OF_DATA(type, regtype) (void *)((type) << 16 | (regtype)) > -#define SCI_OF_TYPE(data) ((unsigned long)(data) >> 16) > -#define SCI_OF_REGTYPE(data) ((unsigned long)(data) & 0xffff) > +#define SCI_OF_DATA(_type, _regtype) (\ > +&(struct sci_of_data) {\ > + .type = (_type), \ > + .regtype = (_regtype),\ > + .ops = &sci_port_ops,\ > + .uart_ops = &sci_uart_ops,\ > + .params = &sci_port_params[_regtype],\ > +}) Doing it this way means each and every entry in of_sci_match[] has its own copy of struct sci_of_data, even if it is identical to one of the others. Unfortunately s/struct sci_of_data/const struct sci_of_data/ doesn't help, so I'm afraid you have to deduplicate them explicitly. > > static const struct of_device_id of_sci_match[] __maybe_unused = { > /* SoC-specific types */ > @@ -3334,7 +3339,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) > @@ -3380,8 +3385,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"); > > @@ -3471,6 +3480,7 @@ static int sci_probe(struct platform_device *dev) > } > > dev_id = dev->id; > + sp->params = sci_probe_regmap(p, &sci_ports[dev_id]); sp is still uninitialized here, so it crashes on SuperH. In adition if (!sp->params) return -ENODEV; > } > > sp = &sci_ports[dev_id]; > @@ -3560,19 +3570,23 @@ sh_early_platform_init_buffer("earlyprintk", &sci_driver, > static struct plat_sci_port port_cfg __initdata; > > int __init early_console_setup(struct earlycon_device *device, > - int type) > + const struct sci_of_data *data) > { > const struct sci_common_regs *regs; > > if (!device->port.membase) > return -ENODEV; > > - device->port.type = type; > + device->port.type = data->type; > memcpy(&sci_ports[0].port, &device->port, sizeof(struct uart_port)); > - port_cfg.type = type; > + > + port_cfg.type = data->type; > + port_cfg.regtype = data->regtype; > + > sci_ports[0].cfg = &port_cfg; > - sci_ports[0].ops = &sci_port_ops; > - sci_ports[0].params = sci_probe_regmap(&port_cfg); > + sci_ports[0].params = data->params; > + sci_ports[0].ops = data->ops; > + sci_ports[0].port.ops = data->uart_ops; > regs = sci_ports[0].params->common_regs; > > port_cfg.scscr = sci_ports[0].ops->read_reg(&sci_ports[0].port, regs->control); I think you have to do some extra setup in sci_probe_earlyprintk(), too. That function contains the second caller of sci_init_single(), and thus relied on sci_init_single() calling sci_probe_regmap() before. I haven't tested that case on actual hardware yet. Gr{oetje,eeting}s, Geert
diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c index ace56375c044..eee1c03ba5a7 100644 --- a/drivers/tty/serial/sh-sci.c +++ b/drivers/tty/serial/sh-sci.c @@ -2917,10 +2917,13 @@ static int sci_init_clocks(struct sci_port *sci_port, struct device *dev) } static const struct sci_port_params * -sci_probe_regmap(const struct plat_sci_port *cfg) +sci_probe_regmap(const struct plat_sci_port *cfg, struct sci_port *sci_port) { unsigned int regtype; + sci_port->ops = &sci_port_ops; + sci_port->port.ops = &sci_uart_ops; + if (cfg->regtype != SCIx_PROBE_REGTYPE) return &sci_port_params[cfg->regtype]; @@ -2967,9 +2970,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; port->iotype = UPIO_MEM; port->line = index; port->has_sysrq = IS_ENABLED(CONFIG_SERIAL_SH_SCI_CONSOLE); @@ -3009,7 +3010,6 @@ 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); if (unlikely(sci_port->params == NULL)) return -EINVAL; @@ -3264,9 +3264,14 @@ static void sci_remove(struct platform_device *dev) device_remove_file(&dev->dev, &dev_attr_rx_fifo_timeout); } -#define SCI_OF_DATA(type, regtype) (void *)((type) << 16 | (regtype)) -#define SCI_OF_TYPE(data) ((unsigned long)(data) >> 16) -#define SCI_OF_REGTYPE(data) ((unsigned long)(data) & 0xffff) +#define SCI_OF_DATA(_type, _regtype) (\ +&(struct sci_of_data) {\ + .type = (_type), \ + .regtype = (_regtype),\ + .ops = &sci_port_ops,\ + .uart_ops = &sci_uart_ops,\ + .params = &sci_port_params[_regtype],\ +}) static const struct of_device_id of_sci_match[] __maybe_unused = { /* SoC-specific types */ @@ -3334,7 +3339,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) @@ -3380,8 +3385,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"); @@ -3471,6 +3480,7 @@ static int sci_probe(struct platform_device *dev) } dev_id = dev->id; + sp->params = sci_probe_regmap(p, &sci_ports[dev_id]); } sp = &sci_ports[dev_id]; @@ -3560,19 +3570,23 @@ sh_early_platform_init_buffer("earlyprintk", &sci_driver, static struct plat_sci_port port_cfg __initdata; int __init early_console_setup(struct earlycon_device *device, - int type) + const struct sci_of_data *data) { const struct sci_common_regs *regs; if (!device->port.membase) return -ENODEV; - device->port.type = type; + device->port.type = data->type; memcpy(&sci_ports[0].port, &device->port, sizeof(struct uart_port)); - port_cfg.type = type; + + port_cfg.type = data->type; + port_cfg.regtype = data->regtype; + sci_ports[0].cfg = &port_cfg; - sci_ports[0].ops = &sci_port_ops; - sci_ports[0].params = sci_probe_regmap(&port_cfg); + sci_ports[0].params = data->params; + sci_ports[0].ops = data->ops; + sci_ports[0].port.ops = data->uart_ops; regs = sci_ports[0].params->common_regs; port_cfg.scscr = sci_ports[0].ops->read_reg(&sci_ports[0].port, regs->control); @@ -3586,41 +3600,39 @@ int __init early_console_setup(struct earlycon_device *device, static int __init sci_early_console_setup(struct earlycon_device *device, const char *opt) { - return early_console_setup(device, PORT_SCI); + return early_console_setup(device, SCI_OF_DATA(PORT_SCI, SCIx_SCI_REGTYPE)); } static int __init scif_early_console_setup(struct earlycon_device *device, const char *opt) { - return early_console_setup(device, PORT_SCIF); + return early_console_setup(device, SCI_OF_DATA(PORT_SCIF, SCIx_SH4_SCIF_REGTYPE)); } static int __init rzscifa_early_console_setup(struct earlycon_device *device, const char *opt) { - port_cfg.regtype = SCIx_RZ_SCIFA_REGTYPE; - return early_console_setup(device, PORT_SCIF); + return early_console_setup(device, SCI_OF_DATA(PORT_SCIF, SCIx_RZ_SCIFA_REGTYPE)); } static int __init rzv2hscif_early_console_setup(struct earlycon_device *device, const char *opt) { - port_cfg.regtype = SCIx_RZV2H_SCIF_REGTYPE; - return early_console_setup(device, PORT_SCIF); + return early_console_setup(device, SCI_OF_DATA(PORT_SCIF, SCIx_RZV2H_SCIF_REGTYPE)); } static int __init scifa_early_console_setup(struct earlycon_device *device, const char *opt) { - return early_console_setup(device, PORT_SCIFA); + return early_console_setup(device, SCI_OF_DATA(PORT_SCIFA, SCIx_SCIFA_REGTYPE)); } static int __init scifb_early_console_setup(struct earlycon_device *device, const char *opt) { - return early_console_setup(device, PORT_SCIFB); + return early_console_setup(device, SCI_OF_DATA(PORT_SCIFB, SCIx_SCIFB_REGTYPE)); } static int __init hscif_early_console_setup(struct earlycon_device *device, const char *opt) { - return early_console_setup(device, PORT_HSCIF); + return early_console_setup(device, SCI_OF_DATA(PORT_HSCIF, SCIx_HSCIF_REGTYPE)); } OF_EARLYCON_DECLARE(sci, "renesas,sci", sci_early_console_setup); diff --git a/drivers/tty/serial/sh-sci_common.h b/drivers/tty/serial/sh-sci_common.h index 0bc49ab277b5..9235a8d4c7aa 100644 --- a/drivers/tty/serial/sh-sci_common.h +++ b/drivers/tty/serial/sh-sci_common.h @@ -96,6 +96,14 @@ struct sci_port_ops { void (*prepare_console_write)(struct uart_port *port, u32 ctrl); }; +struct sci_of_data { + const struct sci_port_params *params; + const struct uart_ops *uart_ops; + const struct sci_port_ops *ops; + unsigned short regtype; + unsigned short type; +}; + struct sci_port { struct uart_port port; @@ -165,7 +173,8 @@ void sci_flush_buffer(struct uart_port *port); #define max_sr(_port) fls((_port)->sampling_rate_mask) #ifdef CONFIG_SERIAL_SH_SCI_EARLYCON -int __init early_console_setup(struct earlycon_device *device, int); +int __init early_console_setup(struct earlycon_device *device, + const struct sci_of_data *data); #endif #endif /* __SH_SCI_COMMON_H__ */
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@bp.renesas.com> --- drivers/tty/serial/sh-sci.c | 60 ++++++++++++++++++------------ drivers/tty/serial/sh-sci_common.h | 11 +++++- 2 files changed, 46 insertions(+), 25 deletions(-)