| Message ID | 20241204155806.3781200-5-claudiu.beznea.uj@bp.renesas.com |
|---|---|
| State | New |
| Headers | show |
| Series | serial: sh-sci: Fixes for earlycon and keep_bootcon | expand |
Hi Claudiu, On Wed, Dec 4, 2024 at 4:58 PM Claudiu <claudiu.beznea@tuxon.dev> wrote: > From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> > > In the sh-sci driver, sci_ports[0] is used by earlycon. If the earlycon is > still active when sci_probe() is called and the new serial port is supposed > to map to sci_ports[0], return -EBUSY to prevent breaking the earlycon. > > This situation should occurs in debug scenarios, and users should be > aware of the potential conflict. > > Fixes: 0b0cced19ab1 ("serial: sh-sci: Add CONFIG_SERIAL_EARLYCON support") > Cc: stable@vger.kernel.org > Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> Thanks for your patch! > --- a/drivers/tty/serial/sh-sci.c > +++ b/drivers/tty/serial/sh-sci.c > @@ -158,6 +158,7 @@ struct sci_port { > bool has_rtscts; > bool autorts; > bool tx_occurred; > + bool earlycon; This is only used in sci_ports[0], so it can be a single global flag, instead of a flag embedded in each sci_port structure. > }; > > #define SCI_NPORTS CONFIG_SERIAL_SH_SCI_NR_UARTS > @@ -3443,6 +3444,7 @@ static int sci_probe_single(struct platform_device *dev, > static int sci_probe(struct platform_device *dev) > { > struct plat_sci_port *p; > + struct resource *res; > struct sci_port *sp; > unsigned int dev_id; > int ret; > @@ -3472,6 +3474,26 @@ static int sci_probe(struct platform_device *dev) > } > > sp = &sci_ports[dev_id]; > + > + /* > + * In case: > + * - the probed port alias is zero (as the one used by earlycon), and > + * - the earlycon is still active (e.g., "earlycon keep_bootcon" in > + * bootargs) This is even true without "keep_bootcon", as nothing ever clears the sci_port.earlycon flag once it is set. > + * > + * defer the probe of this serial. This is a debug scenario and the user > + * must be aware of it. > + * > + * Except when the probed port is the same as the earlycon port. > + */ > + > + res = platform_get_resource(dev, IORESOURCE_MEM, 0); > + if (!res) > + return -ENODEV; > + > + if (sp->earlycon && res->start != sp->port.mapbase) > + return dev_err_probe(&dev->dev, -EBUSY, "sci_port[0] is used by earlycon!\n"); > + > platform_set_drvdata(dev, sp); > > ret = sci_probe_single(dev, dev_id, p, sp); > @@ -3568,6 +3590,7 @@ static int __init early_console_setup(struct earlycon_device *device, > port_cfg.type = type; > sci_ports[0].cfg = &port_cfg; > sci_ports[0].params = sci_probe_regmap(&port_cfg); > + sci_ports[0].earlycon = true; > port_cfg.scscr = sci_serial_in(&sci_ports[0].port, SCSCR); > sci_serial_out(&sci_ports[0].port, SCSCR, > SCSCR_RE | SCSCR_TE | port_cfg.scscr); 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 --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c index 373195995d3b..e12fbc71082a 100644 --- a/drivers/tty/serial/sh-sci.c +++ b/drivers/tty/serial/sh-sci.c @@ -158,6 +158,7 @@ struct sci_port { bool has_rtscts; bool autorts; bool tx_occurred; + bool earlycon; }; #define SCI_NPORTS CONFIG_SERIAL_SH_SCI_NR_UARTS @@ -3443,6 +3444,7 @@ static int sci_probe_single(struct platform_device *dev, static int sci_probe(struct platform_device *dev) { struct plat_sci_port *p; + struct resource *res; struct sci_port *sp; unsigned int dev_id; int ret; @@ -3472,6 +3474,26 @@ static int sci_probe(struct platform_device *dev) } sp = &sci_ports[dev_id]; + + /* + * In case: + * - the probed port alias is zero (as the one used by earlycon), and + * - the earlycon is still active (e.g., "earlycon keep_bootcon" in + * bootargs) + * + * defer the probe of this serial. This is a debug scenario and the user + * must be aware of it. + * + * Except when the probed port is the same as the earlycon port. + */ + + res = platform_get_resource(dev, IORESOURCE_MEM, 0); + if (!res) + return -ENODEV; + + if (sp->earlycon && res->start != sp->port.mapbase) + return dev_err_probe(&dev->dev, -EBUSY, "sci_port[0] is used by earlycon!\n"); + platform_set_drvdata(dev, sp); ret = sci_probe_single(dev, dev_id, p, sp); @@ -3568,6 +3590,7 @@ static int __init early_console_setup(struct earlycon_device *device, port_cfg.type = type; sci_ports[0].cfg = &port_cfg; sci_ports[0].params = sci_probe_regmap(&port_cfg); + sci_ports[0].earlycon = true; port_cfg.scscr = sci_serial_in(&sci_ports[0].port, SCSCR); sci_serial_out(&sci_ports[0].port, SCSCR, SCSCR_RE | SCSCR_TE | port_cfg.scscr);