Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
From:   Leo Li <leoyang.li@nxp.com>
To:     Rasmus Villemoes <linux@rasmusvillemoes.dk>,
        "Biwen Li (OSS)" <biwen.li@oss.nxp.com>,
        "shawnguo@kernel.org" <shawnguo@kernel.org>,
        "robh+dt@kernel.org" <robh+dt@kernel.org>,
        "mark.rutland@arm.com" <mark.rutland@arm.com>,
        "Z.q. Hou" <zhiqiang.hou@nxp.com>,
        "tglx@linutronix.de" <tglx@linutronix.de>,
        "jason@lakedaemon.net" <jason@lakedaemon.net>,
        "maz@kernel.org" <maz@kernel.org>
CC:     "devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        Jiafei Pan <jiafei.pan@nxp.com>,
        Xiaobo Xie <xiaobo.xie@nxp.com>,
        "linux-arm-kernel@lists.infradead.org" 
        <linux-arm-kernel@lists.infradead.org>
Subject: RE: [EXT] Re: [v2 01/11] irqchip: ls-extirq: Add LS1043A, LS1088A
 external interrupt
Thread-Topic: [EXT] Re: [v2 01/11] irqchip: ls-extirq: Add LS1043A, LS1088A
 external interrupt
Thread-Index: AQHWrB1r8C3YTVD4gUGfvnOjzP6Q2amrEJ2AgAACOICAANrToIAIeRcAgAD4mMCAALfQAIAEGdlg
Date:   Thu, 5 Nov 2020 23:03:55 +0000
Message-ID: <VE1PR04MB66876B8AF6F0D3C5A583BBDF8FEE0@VE1PR04MB6687.eurprd04.prod.outlook.com>
References: <20201027044619.41879-1-biwen.li@oss.nxp.com>
 <5b1f2911-98b3-511f-404b-7d0fa44cc0c2@rasmusvillemoes.dk>
 <DB6PR0401MB2438ED1C8F629CA1E1F469768F160@DB6PR0401MB2438.eurprd04.prod.outlook.com>
 <VE1PR04MB668790304253EAEEBC1B79F48F160@VE1PR04MB6687.eurprd04.prod.outlook.com>
 <DB6PR0401MB2438DCABBF4892ECF599D4528F100@DB6PR0401MB2438.eurprd04.prod.outlook.com>
 <VE1PR04MB6687738526B56F3F0E6812C58F100@VE1PR04MB6687.eurprd04.prod.outlook.com>
 <03dc38bb-b83a-7926-60dc-61152a3ac967@rasmusvillemoes.dk>
In-Reply-To: <03dc38bb-b83a-7926-60dc-61152a3ac967@rasmusvillemoes.dk>
Accept-Language: en-US
Content-Language: en-US
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-Network-Message-Id: f914dcfe-3c47-4013-53cf-08d881df1206
X-MS-Exchange-CrossTenant-originalarrivaltime: 05 Nov 2020 23:03:55.1740
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 686ea1d3-bc2b-4c6f-a92c-d99c5c301635
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: R/c2+SN03HAJuHrsjDclAncHRTYOGJXpsewqBgfkz8C7ySME1gI8ruqpo1cZezZmaGjtW+xOKcaJMEO5j52NnQ==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: VE1PR04MB7423
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org


> -----Original Message-----
> From: Rasmus Villemoes <linux@rasmusvillemoes.dk>
> Sent: Tuesday, November 3, 2020 2:03 AM
> To: Leo Li <leoyang.li@nxp.com>; Biwen Li (OSS) <biwen.li@oss.nxp.com>;
> shawnguo@kernel.org; robh+dt@kernel.org; mark.rutland@arm.com; Z.q.
> Hou <zhiqiang.hou@nxp.com>; tglx@linutronix.de; jason@lakedaemon.net;
> maz@kernel.org
> Cc: devicetree@vger.kernel.org; linux-kernel@vger.kernel.org; Jiafei Pan
> <jiafei.pan@nxp.com>; Xiaobo Xie <xiaobo.xie@nxp.com>; linux-arm-
> kernel@lists.infradead.org
> Subject: Re: [EXT] Re: [v2 01/11] irqchip: ls-extirq: Add LS1043A, LS1088=
A
> external interrupt
>=20
> On 02/11/2020 22.22, Leo Li wrote:
> >>>
> >>> 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 << (3=
1 - 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 dr=
iver
> for good.  This will prevent future confusion.  Rasmus, what do you think=
?
>=20
> Yes, all the ls1021a-derived boards I know of do have something like
>=20
> # Initialize bit reverse of SCFG registers
> 09570200 ffffffff
>=20
> in their pre-boot-loader config file. And yes, the RM does say
>=20
>   This register must be written 0xFFFF_FFFF as a part of
>   initialization sequence before writing to any other SCFG
>   register.
>=20
> but nowhere does it say "or else...", nor a little honest addendum "becau=
se
> 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 doi=
ng it correctly in later SoCs.

>=20
> 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 fo=
r this.  But definitely it was not your fault to have introduced this in th=
e 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 dr=
iver is used on more SoCs.

Regards,
Leo