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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Mon, 30 Nov 2020 01:38:17 +0000
From:   "Biwen Li (OSS)" <biwen.li@....nxp.com>
To:     Leo Li <leoyang.li@....com>,
        Rasmus Villemoes <linux@...musvillemoes.dk>,
        "Biwen Li (OSS)" <biwen.li@....nxp.com>,
        "shawnguo@...nel.org" <shawnguo@...nel.org>,
        "robh+dt@...nel.org" <robh+dt@...nel.org>,
        "mark.rutland@....com" <mark.rutland@....com>,
        "Z.q. Hou" <zhiqiang.hou@....com>,
        "tglx@...utronix.de" <tglx@...utronix.de>,
        "jason@...edaemon.net" <jason@...edaemon.net>,
        "maz@...nel.org" <maz@...nel.org>
CC:     "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Jiafei Pan <jiafei.pan@....com>,
        Xiaobo Xie <xiaobo.xie@....com>,
        "linux-arm-kernel@...ts.infradead.org" 
        <linux-arm-kernel@...ts.infradead.org>
Subject: RE: [EXT] Re: [v2 01/11] irqchip: ls-extirq: Add LS1043A, LS1088A
 external interrupt


> > > >>> 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