Hi Thierry,
On Thu, 15 May 2025 at 16:19, Thierry Bultel
<thierry.bultel.yh@xxxxxxxxxxxxxx> 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 original SH SCI ones.
> DMA is not supported yet.
>
> Reviewed-by: Wolfram Sang <wsa+renesas@xxxxxxxxxxxxxxxxxxxx>
> Signed-off-by: Thierry Bultel <thierry.bultel.yh@xxxxxxxxxxxxxx>
> ---
> Changes v8->v9:
> - Fixed some code formatting
> - Renamed rzt2_sci_uart_ops to rsci_uart_ops
> - Renamed of_sci_r9a09g077_data to of_sci_rsci_data
> - Added EXPORT_SYMBOL for public functions
> - Added MODULE_LICENSE & MODULE_DESCRIPTION
> - Fixed RSCI clock names
> - Fixed SCI_PORT_RSCI using BIT(7)
Thanks for the update!
> --- /dev/null
> +++ b/drivers/tty/serial/rsci.h
> @@ -0,0 +1,12 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +
> +#ifndef __RSCI_H__
> +#define __RSCI_H__
> +
> +#include "sh-sci-common.h"
> +
> +#ifdef CONFIG_SERIAL_RSCI
> +extern struct sci_of_data of_sci_rsci_data;
> +#endif
The #ifdef isn't really needed.
> +
> +#endif /* __RSCI_H__ */
> --- a/drivers/tty/serial/sh-sci.c
> +++ b/drivers/tty/serial/sh-sci.c
> @@ -2977,14 +2987,27 @@ 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] = "operation";
> + clk_names[SCI_BRG_INT] = "bus";
> + }
>
> for (i = 0; i < SCI_NUM_CLKS; i++) {
> - clk = devm_clk_get_optional(dev, clk_names[i]);
> + const char *name = clk_names[i];
> +
> + clk = devm_clk_get_optional(dev, name);
> if (IS_ERR(clk))
> return PTR_ERR(clk);
>
> + if (!clk && sci_port->type == SCI_PORT_RSCI &&
> + (i == SCI_FCK || i == SCI_BRG_INT)) {
> + return dev_err_probe(dev, -ENODEV,
> + "failed to get '%s' clock\n",
I would make the error message identical to the other cases below,
so the format string can be shared by the compiler.
> + name);
> + }
> +
> if (!clk && i == SCI_FCK) {
> /*
> * Not all SH platforms declare a clock lookup entry
> @@ -2995,13 +3018,13 @@ static int sci_init_clocks(struct sci_port *sci_port, struct device *dev)
> if (IS_ERR(clk))
> return dev_err_probe(dev, PTR_ERR(clk),
> "failed to get %s\n",
> - clk_names[i]);
> + name);
> }
>
> if (!clk)
> - dev_dbg(dev, "failed to get %s\n", clk_names[i]);
> + dev_dbg(dev, "failed to get %s\n", name);
> else
> - dev_dbg(dev, "clk %s is %pC rate %lu\n", clk_names[i],
> + dev_dbg(dev, "clk %s is %pC rate %lu\n", name,
> clk, clk_get_rate(clk));
> sci_port->clks[i] = clk;
> }
The rest of the (generic; I didn't look at the RSCI low-level details)
changes LGTM.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx
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
