RE: [PATCH 4/4] serial: sh-sci: Improve support for separate TEI and DRI interrupts

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

 



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 <geert+renesas@xxxxxxxxx>

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

��.n��������+%������w��{.n�����{��ǫ����{ay�ʇڙ���f���h������_�(�階�ݢj"��������G����?���&��




[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