Hi Geert,
On Monday, July 30, 2018, Geert Uytterhoeven wrote:
> > if (sci_port->irqs[0] < 0)
> > return -ENXIO;
> >
> > - if (sci_port->irqs[1] < 0) {
> > - sci_port->irqs[1] = sci_port->irqs[0];
> > - sci_port->irqs[2] = sci_port->irqs[0];
> > - sci_port->irqs[3] = sci_port->irqs[0];
> > - }
> > + if (sci_port->irqs[1] < 0)
> > + for (i = 1; i < ARRAY_SIZE(sci_port->irqs) - 1; i++)
>
> Shouldn't the "- 1" be dropped?
In reality, the last entry of the array is 'SCIx_NR_IRQS', so it's not
really used anywhere.
The original array was:
enum {
SCIx_ERI_IRQ, (the only IRQ specified in DT)
SCIx_RXI_IRQ, << copy from irqs[0]
SCIx_TXI_IRQ, << copy from irqs[0]
SCIx_BRI_IRQ, << copy from irqs[0]
SCIx_NR_IRQS, (didn’t' touch)
SCIx_MUX_IRQ = SCIx_NR_IRQS, /* special case */
};
So the new for loop used "- "1 in order to create the same outcome.
But as far as I can tell irqs[SCIx_NR_IRQS] is never used anywhere, it
doesn't really matter.
> With the above fixed:
> Reviewed-by: Geert Uytterhoeven <[email protected]>
I have no problem taking the "- 1" out.
But...here's the funny part: It was you that suggested the "- 1" ;)
On Thursday, July 26, 2018, Geert Uytterhoeven wrote:
> > @@ -2809,6 +2845,8 @@ static int sci_init_single(struct platform_device
> *dev,
> > sci_port->irqs[1] = sci_port->irqs[0];
> > sci_port->irqs[2] = sci_port->irqs[0];
> > sci_port->irqs[3] = sci_port->irqs[0];
> > + sci_port->irqs[4] = sci_port->irqs[0];
> > + sci_port->irqs[5] = sci_port->irqs[0];
>
> You may want to start using a loop from 1 to ARRAY_SIZE(sci_port->irqs) - 1
> instead.
Cheers
Chris
