Hi Biju,
On Thu, Mar 16, 2023 at 5:34 PM Biju Das <biju.das.jz@xxxxxxxxxxxxxx> wrote:
> > Subject: Re: [PATCH] tty: serial: sh-sci: Fix transmit end interrupt handler
> > 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")
That one added support for RZ/A2, and is also not the bad
commit?
> > > --- 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;
> +
IMHO none of these checks are needed. "make dtbs_check" takes
care of that.
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
