linux-kernel - Re: [PATCH 10/14] serial: sh-sci: Introduced sci_of

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: <CAMuHMdUCY=PyfcJKMu=WGAzHWfGZVctL3-e=UdAd-tCOQTP-Mw@mail.gmail.com>
Date: Mon, 10 Feb 2025 16:48:19 +0100
From: Geert Uytterhoeven <geert@...ux-m68k.org>
To: Thierry Bultel <thierry.bultel.yh@...renesas.com>
Cc: Greg Kroah-Hartman <gregkh@...uxfoundation.org>, Jiri Slaby <jirislaby@...nel.org>, 
	linux-kernel@...r.kernel.org, linux-serial@...r.kernel.org, 
	Linux-Renesas <linux-renesas-soc@...r.kernel.org>
Subject: Re: [PATCH 10/14] serial: sh-sci: Introduced sci_of_data

Hi Thierry,

On Wed, 29 Jan 2025 at 17:56, Thierry Bultel
<thierry.bultel.yh@...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@...renesas.com>

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@...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