Re: [PATCH RFT 4/6] serial: sh-sci: Do not probe the serial port if its slot in sci_ports[] is in use

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