[EXT] Re: [v2 01/11] irqchip: ls-extirq: Add LS1043A, LS1088A external interrupt

[Leo Li]

> -----Original Message-----
> From: Biwen Li <biwen.li at nxp.com>
> Sent: Tuesday, October 27, 2020 2:48 AM
> To: Rasmus Villemoes <linux at rasmusvillemoes.dk>; Biwen Li (OSS)
> <biwen.li at oss.nxp.com>; shawnguo at kernel.org; robh+dt at kernel.org;
> mark.rutland at arm.com; Leo Li <leoyang.li at nxp.com>; Z.q. Hou
> <zhiqiang.hou at nxp.com>; tglx at linutronix.de; jason at lakedaemon.net;
> maz at kernel.org
> Cc: devicetree at vger.kernel.org; linux-kernel at vger.kernel.org; Jiafei Pan
> <jiafei.pan at nxp.com>; Xiaobo Xie <xiaobo.xie at nxp.com>; linux-arm-
> kernel at lists.infradead.org
> Subject: RE: [EXT] Re: [v2 01/11] irqchip: ls-extirq: Add LS1043A, LS1088A
> external interrupt
> 
> >
> > Caution: EXT Email
> >
> > On 27/10/2020 05.46, Biwen Li wrote:
> > > From: Hou Zhiqiang <Zhiqiang.Hou at nxp.com>
> > >
> > > Add an new IRQ chip declaration for LS1043A and LS1088A
> > > - compatible "fsl,ls1043a-extirq" for LS1043A, LS1046A. SCFG_INTPCR[31:0]
> > >   of these SoCs is stored/read as SCFG_INTPCR[0:31] defaultly(bit
> > >   reverse)
> >
> > s/defaultly/by default/ I suppose. But what does that mean? Is it
> > still configurable, just now through some undocumented register? If
> > that register still exists, does it now have a reset value of all-ones
> > as opposed to the ls1021 case? If it's not configurable, then
> > describing the situation as "by default" is confusing and wrong, it
> > should just say "On LS1043A, LS1046A, SCFG_INTPCR is stored/read bit-
> reversed."
> Okay, got it. Will update it in v3. Thanks.

Hi Biwen,

Where did you get this information that the register on LS1043 and LS1046 is bit reversed?  I cannot find such information in the RM.  And does this mean all other SCFG registers are also bit reversed?  If this is some information that is not covered by the RM, we probably should clarify it in the code and the commit message.

Regards,
Leo

> >
> >
> > > - compatible "fsl,ls1088a-extirq" for LS1088A, LS208xA, LX216xA
> > >
> > > Signed-off-by: Hou Zhiqiang <Zhiqiang.Hou at nxp.com>
> > > Signed-off-by: Biwen Li <biwen.li at nxp.com>
> > > ---
> > > Change in v2:
> > >       - add despcription of bit reverse
> > >       - update copyright
> > >
> > >  drivers/irqchip/irq-ls-extirq.c | 10 +++++++++-
> > >  1 file changed, 9 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/irqchip/irq-ls-extirq.c
> > > b/drivers/irqchip/irq-ls-extirq.c index 4d1179fed77c..9587bc2607fc
> > > 100644
> > > --- a/drivers/irqchip/irq-ls-extirq.c
> > > +++ b/drivers/irqchip/irq-ls-extirq.c
> > > @@ -1,5 +1,8 @@
> > >  // SPDX-License-Identifier: GPL-2.0
> > > -
> > > +/*
> > > + * Author: Rasmus Villemoes <linux at rasmusvillemoes.dk>
> >
> > If I wanted my name splattered all over the files I touch or add, I'd
> > add it myself, TYVM. The git history is plenty fine for recording
> > authorship as far as I'm concerned, and I absolutely abhor having to
> > skip over any kind of legalese boilerplate when opening a file.
> Okay, got it. Will drop it in v3. Thanks.
> >
> > Rasmus