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

[Leo Li]

> -----Original Message-----
> From: Biwen Li <biwen.li@xxxxxxx>
> Sent: Tuesday, October 27, 2020 2:48 AM
> To: Rasmus Villemoes <linux@xxxxxxxxxxxxxxxxxx>; Biwen Li (OSS)
> <biwen.li@xxxxxxxxxxx>; shawnguo@xxxxxxxxxx; robh+dt@xxxxxxxxxx;
> mark.rutland@xxxxxxx; Leo Li <leoyang.li@xxxxxxx>; Z.q. Hou
> <zhiqiang.hou@xxxxxxx>; tglx@xxxxxxxxxxxxx; jason@xxxxxxxxxxxxxx;
> maz@xxxxxxxxxx
> Cc: devicetree@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; Jiafei Pan
> <jiafei.pan@xxxxxxx>; Xiaobo Xie <xiaobo.xie@xxxxxxx>; linux-arm-
> kernel@xxxxxxxxxxxxxxxxxxx
> 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@xxxxxxx>
> > >
> > > 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@xxxxxxx>
> > > Signed-off-by: Biwen Li <biwen.li@xxxxxxx>
> > > ---
> > > 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@xxxxxxxxxxxxxxxxxx>
> >
> > 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