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
