[<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