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

[Biwen Li (OSS)]

> > > >>> 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.
> > > >> Hi Leo,
> > > >>
> > > >> I directly use the same logic to write the bit(field IRQ0~11INTP)
> > > >> of the register SCFG_INTPCR in LS1043A and LS1046A.
> > > >> Such as,
> > > >> if I want to control the polarity of IRQ0(field IRQ0INTP, IRQ0 is
> > > >> active low) of LS1043A/LS1046A, then I just need write a value 1
> > > >> << (31 - 0)
> > > to it.
> > > >> The logic depends on register's definition in LS1043A/LS1046A's RM.
> > > >
> > > > Ok.  The SCFG_SCFGREVCR seems to be a one-off fixup only existed
> > > > on
> > > LS1021.  And it is mandatory to be bit_reversed according to the RM
> > > which is already taken care of in the RCW.  So the bit reversed case
> > > should be the only case supported otherwise a lot of other places
> > > for SCFG access should be failed.
> > > >
> > > > I think we should remove the bit_reverse thing all together from
> > > > the driver
> > > for good.  This will prevent future confusion.  Rasmus, what do you think?
> > >
> > > Yes, all the ls1021a-derived boards I know of do have something like
> > >
> > > # Initialize bit reverse of SCFG registers
> > > 09570200 ffffffff
> > >
> > > in their pre-boot-loader config file. And yes, the RM does say
> > >
> > >   This register must be written 0xFFFF_FFFF as a part of
> > >   initialization sequence before writing to any other SCFG
> > >   register.
> > >
> > > but nowhere does it say "or else...", nor a little honest addendum
> > > "because we accidentally released broken silicon with this
> > > misfeature _and_ wrong POR value".
> >
> > Yeah.  I do think they messed up at the beginning when trying to integrate
> the big endian registers on little endian core.  It is good that we are doing it
> correctly in later SoCs.
> >
> > >
> > > Can we have an official statement from NXP stating that SCFGREVCR is
> > > a hardware design bug? And can you send it through a time-machine so
> > > I had it three years ago avoiding the whole "fsl,bit-reverse
> > > device-tree-property, no, read the register if you're on a ls1021a and decide"
> hullabaloo.
> >
> > I'm not sure if it is possible to update the related documents right now for this.
> But definitely it was not your fault to have introduced this in the driver due to
> the confusion from document.  My suggestion to remove it is just to prevent
> this from causing more confusions in the future as this driver is used on more
> SoCs.
> 
> Hi Biwen,
> 
> Would you send a new version of this patch?  Thanks.
Hi Leo, sure, np.
> 
> Regards,
> Leo