linux-kernel - Re: [PATCH v8 07/11] serial: sh-sci: Use private port ID

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CAMuHMdWa1_UtOEgKHoOfaN4aYvpVobjX_QK=auND_cs_uwxjnQ@mail.gmail.com>
Date: Tue, 13 May 2025 14:59:47 +0200
From: Geert Uytterhoeven <geert@...ux-m68k.org>
To: Thierry Bultel <thierry.bultel.yh@...renesas.com>
Cc: thierry.bultel@...atsea.fr, linux-renesas-soc@...r.kernel.org, 
	paul.barker.ct@...renesas.com, linux-kernel@...r.kernel.org, 
	linux-serial@...r.kernel.org
Subject: Re: [PATCH v8 07/11] serial: sh-sci: Use private port ID

Hi Thierry,

Thanks for your patch!

You forgot to CC the serial maintainers.

On Tue, 29 Apr 2025 at 10:20, Thierry Bultel
<thierry.bultel.yh@...renesas.com> wrote:
> From: Thierry Bultel <thierry.bultel@...atsea.fr>
>
> New port types cannot be added in serial_core.h, which is shared with
> userspace.
> In order to support new port types, the coming new ones will have
> BIT(15) set in the id value, and in this case, uartport->type is
> set to PORT_GENERIC.
> This commit therefore changes all the places where the port type is
> read, by not relying on uartport->type but on the private
> value stored in struct sci_port.
>
> Signed-off-by: Thierry Bultel <thierry.bultel.yh@...renesas.com>

scripts/checkpatch.pl:
WARNING: From:/Signed-off-by: email address mismatch: 'From: Thierry
Bultel <thierry.bultel@...atsea.fr>' != 'Signed-off-by: Thierry Bultel
<thierry.bultel.yh@...renesas.com>'

> --- a/drivers/tty/serial/sh-sci-common.h
> +++ b/drivers/tty/serial/sh-sci-common.h
> @@ -142,6 +142,9 @@ struct sci_port {
>         int                             rx_fifo_timeout;
>         u16                             hscif_tot;
>
> +       unsigned int                    type;

u16 should be sufficient, as you store the new info in bit 15.
Even u8 is sufficient (but then you have to move the new bit to bit
7), and has the advantage of not increasing the size of the structure,
as there was still a 2-byte hole.

> +       unsigned int                    regtype;

u8?

> +
>         const struct sci_port_ops *ops;
>
>         bool has_rtscts;
> diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c
> index ff1986dc6af3..2abf80230a77 100644
> --- a/drivers/tty/serial/sh-sci.c
> +++ b/drivers/tty/serial/sh-sci.c
> @@ -75,6 +75,8 @@
>
>  #define SCI_NPORTS CONFIG_SERIAL_SH_SCI_NR_UARTS
>
> +#define SCI_PUBLIC_PORT_ID(port) (!((port) & BIT(15)) ? (port) : PORT_GENERIC)

Please simplify by inverting the check:

    ((port) & BIT(15)) ? PORT_GENERIC : port

> +
>  static struct sci_port sci_ports[SCI_NPORTS];
>  static unsigned long sci_ports_in_use;
>  static struct uart_driver sci_uart_driver;

> @@ -3050,6 +3063,9 @@ static int sci_init_single(struct platform_device *dev,
>
>         sci_port->cfg   = p;
>
> +       sci_port->type  = p->type;
> +       sci_port->regtype = p->regtype;
> +
>         port->iotype    = UPIO_MEM;
>         port->line      = index;
>         port->has_sysrq = IS_ENABLED(CONFIG_SERIAL_SH_SCI_CONSOLE);

> @@ -3799,7 +3815,8 @@ int __init scix_early_console_setup(struct earlycon_device *device,
>         if (!device->port.membase)
>                 return -ENODEV;
>
> -       device->port.type = data->type;
> +       device->port.type = SCI_PUBLIC_PORT_ID(data->type);
> +
>         sci_ports[0].port = device->port;

Here you have to fill in sci_ports[0].type and sci_ports[0].regtype,
like in sci_init_single(), else they are always zero when using
earlycon.

>
>         port_cfg.type = data->type;

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@...ux-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