RE: [PATCH] tty: serial: sh-sci: Fix transmit end interrupt handler

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

 



Hi Geert,

Thanks for the feedback.

> Subject: Re: [PATCH] tty: serial: sh-sci: Fix transmit end interrupt handler
> 
> Hi Biju,
> 
> On Thu, Mar 16, 2023 at 5:01 PM Biju Das <biju.das.jz@xxxxxxxxxxxxxx> wrote:
> > The RZ SCI/ RZ/A1 SCIF has only 4 interrupts. The fourth interrupt is
> > transmit end interrupt, so shuffle the interrupts to fix the transmit
> > end interrupt handler for these IPs.
> >
> > Fixes: 392fb8df528b ("serial: sh-sci: Use platform_get_irq_optional()
> > for optional interrupts")
> 
> I don't think that's the right bad commit.

OK. I will use below commit as fixes one, 
that is the commit which added RZ/A1 SCIF with 4 interrupts.

commit 8b0bbd956228ae87 ("serial: sh-sci: Add support for R7S9210")


> 
> > Suggested-by: Geert Uytterhoeven <geert+renesas@xxxxxxxxx>
> > Signed-off-by: Biju Das <biju.das.jz@xxxxxxxxxxxxxx>
> 
> Thanks for your patch!
> 
> > --- a/drivers/tty/serial/sh-sci.c
> > +++ b/drivers/tty/serial/sh-sci.c
> > @@ -31,6 +31,7 @@
> >  #include <linux/ioport.h>
> >  #include <linux/ktime.h>
> >  #include <linux/major.h>
> > +#include <linux/minmax.h>
> >  #include <linux/module.h>
> >  #include <linux/mm.h>
> >  #include <linux/of.h>
> > @@ -2841,6 +2842,7 @@ static int sci_init_single(struct platform_device
> *dev,
> >         struct uart_port *port = &sci_port->port;
> >         const struct resource *res;
> >         unsigned int i;
> > +       int irq_cnt;
> >         int ret;
> >
> >         sci_port->cfg   = p;
> > @@ -2864,6 +2866,14 @@ static int sci_init_single(struct platform_device
> *dev,
> >                         sci_port->irqs[i] = platform_get_irq(dev, i);
> >         }
> >
> > +       /*
> > +        * RZ SCI/ RZ/A1 SCIF has only 4 interrupts. The fourth interrupt
> > +        * is transmit end interrupt, so shuffle the interrupts.
> > +        */
> > +       irq_cnt = platform_irq_count(dev);
> > +       if (irq_cnt == 4)
> > +               swap(sci_port->irqs[SCIx_BRI_IRQ],
> > + sci_port->irqs[SCIx_TEI_IRQ]);
> > +
> 
> I think it's simpler to just check if SCIx_TEI_IRQ is missing:
> 
>     if (sci_port->irqs[SCIx_TEI_IRQ] < 0)
>             swap(sci_port->irqs[SCIx_BRI_IRQ], sci_port-
> >irqs[SCIx_TEI_IRQ]);

OK.

> 
> We already rely on "make dtbs_check" to catch invalid combinations (anything
> different from 1/4/6 interrupts).
> 
> And please move that code below, together with the other checks for non-
> existing interrupts.

OK, Will add below code in probe

+       irq_cnt = platform_irq_count(dev);
+       if (irq_cnt != 1 && irq_cnt != 4 && irq_cnt != 6)
+               return -EINVAL;
+

Cheers,
Biju

> 
> >         /* The SCI generates several interrupts. They can be muxed
> together or
> >          * connected to different interrupt lines. In the muxed case only
> one
> >          * interrupt resource is specified as there is only one interrupt
> ID.
> 
> 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]     [Linux Samsung SOC]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Samba]     [Device Mapper]

  Powered by Linux