Hi Geert,
Thanks for the feedback.
> -----Original Message-----
> From: Geert Uytterhoeven <geert@xxxxxxxxxxxxxx>
> Sent: Monday, March 20, 2023 7:12 PM
> To: Biju Das <biju.das.jz@xxxxxxxxxxxxxx>
> Cc: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>; Jiri Slaby
> <jirislaby@xxxxxxxxxx>; Geert Uytterhoeven <geert+renesas@xxxxxxxxx>;
> Yoshinori Sato <ysato@xxxxxxxxxxxxxxxxxxxx>; linux-serial@xxxxxxxxxxxxxxx;
> Prabhakar Mahadev Lad <prabhakar.mahadev-lad.rj@xxxxxxxxxxxxxx>; linux-
> renesas-soc@xxxxxxxxxxxxxxx; stable@xxxxxxxxxxxxxxx
> Subject: Re: [PATCH v3 2/5] tty: serial: sh-sci: Fix Rx on RZ/G2L SCI
>
> Hi Biju,
>
> On Mon, Mar 20, 2023 at 11:53 AM Biju Das <biju.das.jz@xxxxxxxxxxxxxx>
> wrote:
> > SCI IP on RZ/G2L alike SoCs do not need regshift compared to other SCI
> > IPs on the SH platform. Currently, it does regshift and configuring Rx
> > wrongly. Drop adding regshift for RZ/G2L alike SoCs.
> >
> > Fixes: f9a2adcc9e90 ("arm64: dts: renesas: r9a07g044: Add SCI[0-1]
> > nodes")
> > Cc: stable@xxxxxxxxxxxxxxx
> > Signed-off-by: Biju Das <biju.das.jz@xxxxxxxxxxxxxx>
> > ---
> > v3:
> > * New patch.
>
> Thanks for your patch!
>
> > --- a/drivers/tty/serial/sh-sci.c
> > +++ b/drivers/tty/serial/sh-sci.c
> > @@ -158,6 +158,7 @@ struct sci_port {
> >
> > bool has_rtscts;
> > bool autorts;
> > + bool is_rz_sci;
> > };
> >
> > #define SCI_NPORTS CONFIG_SERIAL_SH_SCI_NR_UARTS @@ -2937,7 +2938,7
> > @@ static int sci_init_single(struct platform_device *dev,
> > port->flags = UPF_FIXED_PORT | UPF_BOOT_AUTOCONF | p-
> >flags;
> > port->fifosize = sci_port->params->fifosize;
> >
> > - if (port->type == PORT_SCI) {
> > + if (port->type == PORT_SCI && !sci_port->is_rz_sci) {
>
> Perhaps check for !dev->dev.of_node instead? Values of 1 or 2 for regshift
> are only needed for sh770x/sh7750/sh7760, which don't use DT yet. When they
> are converted to DT, we can extend the driver to support the more-or-less
> standard "reg-shift" DT property.
Agreed.
>
> > if (sci_port->reg_size >= 0x20)
> > port->regshift = 2;
> > else
> > @@ -3353,6 +3354,11 @@ static int sci_probe(struct platform_device *dev)
> > sp = &sci_ports[dev_id];
> > platform_set_drvdata(dev, sp);
> >
> > + if (of_device_is_compatible(dev->dev.of_node, "renesas,r9a07g043-
> sci") ||
> > + of_device_is_compatible(dev->dev.of_node, "renesas,r9a07g044-
> sci") ||
> > + of_device_is_compatible(dev->dev.of_node,
> > + "renesas,r9a07g054-sci"))
>
> Please, no of_device_is_compatible() checks in a driver that uses proper DT
> matching.
OK, will drop this.
Cheers,
Biju
>
> > + sp->is_rz_sci = 1;
> > +
> > ret = sci_probe_single(dev, dev_id, p, sp);
> > if (ret)
> > return ret;
>
> Gr{oetje,eeting}s,
>
> Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-
> 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
