RE: [PATCH v3 2/5] tty: serial: sh-sci: Fix Rx on RZ/G2L SCI

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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




[Index of Archives]     [Kernel Newbies]     [Security]     [Netfilter]     [Bugtraq]     [Linux PPP]     [Linux FS]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Linmodem]     [Device Mapper]     [Linux Kernel for ARM]

  Powered by Linux