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: <CAMuHMdVVdr0R4a1WpsQ7-2fO3G_dXdwDfW367m=7VKmbEP_Xxg@mail.gmail.com>
Date: Tue, 13 May 2025 15:29:49 +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, 
	Wolfram Sang <wsa+renesas@...g-engineering.com>, linux-kernel@...r.kernel.org, 
	linux-serial@...r.kernel.org
Subject: Re: [PATCH v8 08/11] serial: sh-sci: Add support for RZ/T2H SCI

Hi Thierry,

Thanks for the update!

You forgot to CC the serial maintainers.

On Tue, 29 Apr 2025 at 10:20, Thierry Bultel
<thierry.bultel.yh@...renesas.com> wrote:
>
> Define a new RSCI port type, and the RSCI 32 bits registers set.
> The RZ/T2H SCI has a a fifo, and a quite different set of registers
> from the orginal SH SCI ones.

original

> DMA is not supported yet.
>
> Reviewed-by: Wolfram Sang <wsa+renesas@...g-engineering.com>
> Signed-off-by: Thierry Bultel <thierry.bultel.yh@...renesas.com>
> ---
> Changes v7->v8:
>   - s/rzsci/rsci/g
>   - declared SCI_PORT_RSCI as private port ID
>   - look for secondary clock
>   - report error when rsci clocks are not found

> --- a/drivers/tty/serial/Kconfig
> +++ b/drivers/tty/serial/Kconfig
> @@ -675,6 +675,13 @@ config SERIAL_SH_SCI_DMA
>         depends on SERIAL_SH_SCI && DMA_ENGINE
>         default ARCH_RENESAS
>
> +config SERIAL_RSCI
> +       tristate "Support for Renesas RZ/T2H SCI variant"
> +       depends on SERIAL_SH_SCI

As this subdriver can be a module, the relevant symbols in the sh-sci
driver need to be exported, as reported by the kernel robot.

> +       help
> +         Support for the RZ/T2H SCI variant with fifo.
> +         Say Y if you want to be able to use the RZ/T2H SCI serial port.
> +
>  config SERIAL_HS_LPC32XX
>         tristate "LPC32XX high speed serial port support"
>         depends on ARCH_LPC32XX || COMPILE_TEST

> --- /dev/null
> +++ b/drivers/tty/serial/rsci.c

> +static void rsci_prepare_console_write(struct uart_port *port, u32 ctrl)
> +{
> +       struct sci_port *s = to_sci_port(port);
> +       u32 ctrl_temp =
> +               s->params->param_bits->rxtx_enable |
> +               CCR0_TIE |
> +               s->hscif_tot;

This fits on two lines:

        u32 ctrl_temp = s->params->param_bits->rxtx_enable | CCR0_TIE |
                        s->hscif_tot;

> +       rsci_serial_out(port, CCR0, ctrl_temp);
> +}

> +static const struct uart_ops rzt2_sci_uart_ops = {

rsci_uart_ops

> +       .tx_empty       = rsci_tx_empty,
> +       .set_mctrl      = rsci_set_mctrl,
> +       .get_mctrl      = rsci_get_mctrl,
> +       .start_tx       = rsci_start_tx,
> +       .stop_tx        = rsci_stop_tx,
> +       .stop_rx        = rsci_stop_rx,
> +       .startup        = sci_startup,
> +       .shutdown       = sci_shutdown,
> +       .set_termios    = rsci_set_termios,
> +       .pm             = sci_pm,
> +       .type           = rsci_type,
> +       .release_port   = sci_release_port,
> +       .request_port   = sci_request_port,
> +       .config_port    = sci_config_port,
> +       .verify_port    = sci_verify_port,
> +};

> +struct sci_of_data of_sci_r9a09g077_data = {

of_sci_rsci_data

> +       .type = SCI_PORT_RSCI,
> +       .ops = &rsci_port_ops,
> +       .uart_ops = &rzt2_sci_uart_ops,
> +       .params = &rsci_port_params,
> +};
> +
> +#ifdef CONFIG_SERIAL_SH_SCI_EARLYCON
> +
> +static int __init rzt2hsci_early_console_setup(struct earlycon_device *device,

rsci_early_console_setup

> +                                              const char *opt)
> +{
> +       return scix_early_console_setup(device, &of_sci_r9a09g077_data);
> +}
> +
> +OF_EARLYCON_DECLARE(rsci, "renesas,r9a09g077-rsci", rzt2hsci_early_console_setup);
> +
> +#endif /* CONFIG_SERIAL_SH_SCI_EARLYCON */

> --- a/drivers/tty/serial/sh-sci-common.h
> +++ b/drivers/tty/serial/sh-sci-common.h
> @@ -5,6 +5,11 @@
>
>  #include <linux/serial_core.h>
>
> +/* Private port IDs */
> +enum SCI_PORT_TYPE {
> +       SCI_PORT_RSCI = BIT(15)|0,

Please add spaces around "|".

> +};
> +
>  enum SCI_CLKS {
>         SCI_FCK,                /* Functional Clock */
>         SCI_SCK,                /* Optional External Clock */
> diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c
> index 2abf80230a77..44066cd53e5e 100644
> --- a/drivers/tty/serial/sh-sci.c
> +++ b/drivers/tty/serial/sh-sci.c

> @@ -2977,14 +2978,26 @@ static int sci_init_clocks(struct sci_port *sci_port, struct device *dev)
>         struct clk *clk;
>         unsigned int i;
>
> -       if (sci_port->type == PORT_HSCIF)
> +       if (sci_port->type == PORT_HSCIF) {
>                 clk_names[SCI_SCK] = "hsck";
> +       } else if (sci_port->type == SCI_PORT_RSCI) {
> +               clk_names[SCI_FCK] = "async";

As per my comment on the bindings, I think you should use a different
name.  But the actual behavior (overriding [SCI_SCK]) is fine.

> +               clk_names[SCI_SCK] = "bus";

This is not OK, as on RSCI the SCK pin can serve as a clock input.
I see two options here:
  - Add a fifth entry for the RSCI bus clock,
  - Override the [SCI_BRG_INT] entry (which is the closest to a
    bus clock); RSCI will have its own set_termios implementation anyway.

> +       }

> @@ -3085,10 +3098,10 @@ static int sci_init_single(struct platform_device *dev,
>         }
>
>         /*
> -        * The fourth interrupt on SCI port is transmit end interrupt, so
> +        * The fourth interrupt on SCI and RSCI port is transmit end interrupt, so

(R)SCI?

>          * shuffle the interrupts.
>          */
> -       if (p->type == PORT_SCI)
> +       if (p->type == PORT_SCI || p->type == SCI_PORT_RSCI)
>                 swap(sci_port->irqs[SCIx_BRI_IRQ], sci_port->irqs[SCIx_TEI_IRQ]);
>
>         /* The SCI generates several interrupts. They can be muxed together or

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

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ