Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   Vladimir Oltean <vladimir.oltean@nxp.com>
To:     Marc Zyngier <maz@kernel.org>
CC:     "devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "linux-arm-kernel@lists.infradead.org" 
        <linux-arm-kernel@lists.infradead.org>,
        Rob Herring <robh+dt@kernel.org>,
        Shawn Guo <shawnguo@kernel.org>, Leo Li <leoyang.li@nxp.com>,
        Biwen Li <biwen.li@nxp.com>, "Z.Q. Hou" <zhiqiang.hou@nxp.com>,
        Kurt Kanzenbach <kurt@linutronix.de>,
        Rasmus Villemoes <linux@rasmusvillemoes.dk>
Subject: Re: [RFC PATCH devicetree 00/10] Do something about ls-extirq
 interrupt-map breakage
Thread-Topic: [RFC PATCH devicetree 00/10] Do something about ls-extirq
 interrupt-map breakage
Thread-Index: AQHX8ItO3xtQxxPJV0yONti9mp5846wxrpCAgAASv4CAAAYSAICdm7aAgAADHQCAAANtgA==
Date:   Thu, 24 Mar 2022 17:34:06 +0000
Message-ID: <20220324173405.nusk6247ouvek46y@skbuf>
References: <20211214013800.2703568-1-vladimir.oltean@nxp.com>
 <87ilvrk1r0.wl-maz@kernel.org> <20211214095853.4emzycaxkuqr4tun@skbuf>
 <87czlzjxmz.wl-maz@kernel.org> <20220324171041.t5yoocinj6gizcc7@skbuf>
 <87lewz5kr5.wl-maz@kernel.org>
In-Reply-To: <87lewz5kr5.wl-maz@kernel.org>
Accept-Language: en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?8ErhAYid6TZFd9ypg++d7IBllx2Vu9sf60iLUMyPWNpwXsPTYDI0X8lsMb1f?=
 =?us-ascii?Q?rEA0jq358b6bXGou3CZs5ImuitmqHczqV6jnbtXAn9nKGcTJHVX/2hXCVr1T?=
 =?us-ascii?Q?FzNZl1Et7DWabDxCY8tvwXZB4iU8nCO/Yjw+2VV8GOXNZyBVqoBKeFTycFqB?=
 =?us-ascii?Q?A41WLmxBcLo/tc0mbQG4s4PGu8FZzrKSjnAwIraAIun3ZoIgHCLMQehwrTVI?=
 =?us-ascii?Q?W6pEuxEiDJ1gvPHYwqCd3MzO15E7PUcDjiZBTf+YcKKApYsYs7kIYcldQGjc?=
 =?us-ascii?Q?HDLEg42RFlJtMx+EeOUPYUw/dZABHAUN154iph0HeLaL2b2YrSvAbSYnYTQf?=
 =?us-ascii?Q?uoaRprFkcB0o7zLefeT7JHViRqheCVXO0bN4wWJawxmOsQdCCqzi5OBaU/mC?=
 =?us-ascii?Q?BFULUKvgXPYlK8pfz7f3f+cc2Dw/lKW8+t9rtbuVQ0LPc4uEJ2iii36+UnaQ?=
 =?us-ascii?Q?CTxsw9DIapCd9AOyAwDdnLstQuCPK9hp+nMNEUx0K4XMuqWuazZ6BETooAg9?=
 =?us-ascii?Q?LQQ5vygVPOgrC7ESjQXh3T6JAaPmZ0DDEqdpdTkItG77JOfLmvee/AFgcCrp?=
 =?us-ascii?Q?KnNqfKflT5qNRMs7fajI6LzTQ5DRbQqhatcVTmXCdckSBusxSQI7dpPZZ+Bz?=
 =?us-ascii?Q?wYzKa05iYgwLfq+r6+Li/FKRNJepCtjjBwI86Maul/y8hZxd5x2zZJgPmlpT?=
 =?us-ascii?Q?xPDRS/00xz8s/I6L0vJPMe6l1YEyY2xi3HxkR4uuOYwGns2ArQHitS6l+bmf?=
 =?us-ascii?Q?CSQymxw1UR7xHQ+ZjgDrev3HbfXTj+HUWzpc7kB67DN8ylxVID72gh6TBq4R?=
 =?us-ascii?Q?ptCBlR/4GN6hY8LVFVPOHR/FOt1hUgQ/kyetLnM+QtXhrI9qb6VARanFhu6n?=
 =?us-ascii?Q?3R6xFZgUOvu9eQ0ZuhBwqvkaTOQdZAaVOonxz3k7BChlhxrNag+cXZD+xW5A?=
 =?us-ascii?Q?Xq9ONvgu3p7NJ0LimldGG3lBMa2jxxFHcU/FKv3I0mn6J2gki6EV8ZkWDb2A?=
 =?us-ascii?Q?vTIgvrPfLjEz2YSSRtkPnQcy6p6KVsXwTsF2fdT5nrx/0BSSzikNfrM5vn0r?=
 =?us-ascii?Q?nzHKoLicJkXbi04Q/pTNqPAq3F5Oxkup9Q+UEyIFRU79tJg4u1ZGxXfIKyRu?=
 =?us-ascii?Q?931Cs1oOwg/F4+nIhKszL7ARujMFoKsyfqVQRt1qIEXxHD/YeSIq7AehGP5I?=
 =?us-ascii?Q?NmlrHURN2HnMIdaMDpB+uVocIbWXdJ8sFTn5rjrK55libCq+kwfnCgYWdRww?=
 =?us-ascii?Q?4Wl5pO0sO/TFtkf+F5FHregTJg3kxMtJL5jPeOtGTeS8NzafZ7D+z8fD1Lh2?=
 =?us-ascii?Q?iCWRbPGq/pMulbNXuFcVD2mO2ti7D/DfTj9TgpgOpoY5w3Bjz9wZQGsu686I?=
 =?us-ascii?Q?I0HB0Nx5nzB03fXPdCguHfs393cHCEmDMZwsBMSrAylPFXhugKfCsj/0GxqG?=
 =?us-ascii?Q?rAMJVdf3+XMj3qAUsD2yA/ACMEVG3fEZOgP0qxqq0zdtFYm7h1l5+k6y53ST?=
 =?us-ascii?Q?AWVH1yS3QOSkTDa09TNM2UZodr8qIP2H8cOQlSOaSHO9V1951fzWa43SWqR+?=
 =?us-ascii?Q?RqhV7GPsXUfw7dkwTiIKlkXMZe97CV+yWebF85c9tH2Otbyra0IApxtbFdpj?=
 =?us-ascii?Q?/FfJt7KJLHPNXxmFeKLa9fko6cAAX1PBLwMphTwX0FDJuyqhBOsVjO6pJyNy?=
 =?us-ascii?Q?N4dq2WY4NfLgb82kMB9/OUxTNmI3ea0XzIwK4ttah1yCMlsz860DAVjqYsbA?=
 =?us-ascii?Q?xmxuL9uOSH6wmfInsWtzZ5K/DvUoUl4=3D?=
Content-Type: text/plain; charset="us-ascii"
Content-ID: <C72A2E8862BB9244922AD30026681452@eurprd04.prod.outlook.com>
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-OriginatorOrg: nxp.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: VI1PR04MB5136.eurprd04.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 535b111d-954f-45b5-9cae-08da0dbc7f3b
X-MS-Exchange-CrossTenant-originalarrivaltime: 24 Mar 2022 17:34:06.3626
 (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: /pNmh7uYvYUY7s6AKPu1IoYx1xzZ0+4qK4/h+5mfbb1zqvIrO1ua2DoWLqTQ4Yx3+0Epc+pkMb82hQ4QBZ4sSw==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: AM0PR04MB7140
X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_NONE,
        RCVD_IN_MSPIKE_H2,SPF_HELO_PASS,SPF_PASS,T_SCC_BODY_TEXT_LINE
        autolearn=ham autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On Thu, Mar 24, 2022 at 05:21:50PM +0000, Marc Zyngier wrote:
> On Thu, 24 Mar 2022 17:10:42 +0000,
> Vladimir Oltean <vladimir.oltean@nxp.com> wrote:
> >=20
> > Hello Marc,
> >=20
> > On Tue, Dec 14, 2021 at 10:20:36AM +0000, Marc Zyngier wrote:
> > > On Tue, 14 Dec 2021 09:58:54 +0000,
> > > Vladimir Oltean <vladimir.oltean@nxp.com> wrote:
> > > >=20
> > > > Hi Marc (with a c),
> > > >=20
> > > > I wish the firmware for these SoCs was smart enough to be compatibl=
e
> > > > with the bindings that are in the kernel and provide a blob that th=
e
> > > > kernel could actually use. Some work has been started there and thi=
s is
> > > > work in progress. True, I don't know what other OF-based firmware s=
ome
> > > > other customers may use, but I trust it isn't a lot more advanced t=
han
> > > > what U-Boot currently has :)
> > > >=20
> > > > Also, the machines may have been in the wild for years, but the
> > > > ls-extirq driver was added in November 2019. So not with the
> > > > introduction of the SoC device trees themselves. That isn't so long=
 ago.
> > > >=20
> > > > As for compatibility between old kernel and new DT: I guess you'll =
hear
> > > > various opinions on this one.
> > > > https://www.spinics.net/lists/linux-mips/msg07778.html
> > > >=20
> > > > | > Are we okay with the new device tree blobs breaking the old ker=
nel?
> > > > |
> > > > | From my point of view, newer device trees are not required to wor=
k on
> > > > | older kernel, this would impose an unreasonable limitation and th=
e use
> > > > | case is very limited.
> > >=20
> > > My views are on the opposite side. DT is an ABI, full stop. If you
> > > change something, you *must* guarantee forward *and* backward
> > > compatibility. That's because:
> > >=20
> > > - you don't control how updatable the firmware is
> > >=20
> > > - people may need to revert to other versions of the kernel because
> > >   the new one is broken
> > >=20
> > > - there are plenty of DT users beyond Linux, and we are not creating
> > >   bindings for Linux only.
> > >=20
> > > You may disagree with this, but for the subsystems I maintain, this i=
s
> > > the rule I intent to stick to.
> > >=20
> > > 	M.
> > >=20
> > > --=20
> > > Without deviation from the norm, progress is not possible.
> >=20
> > I was just debugging an interesting issue with an old kernel not workin=
g
> > with a new DT blob, and after figuring out what the problem was (is),
> > I remembered this message and I'm curious what you have to say about it=
.
> >=20
> > I have this DT layout:
> >=20
> > 	ethernet-phy@1 {
> > 		reg =3D <0x1>;
> > 		interrupts-extended =3D <&extirq 2 IRQ_TYPE_LEVEL_LOW>;
> > 	};
> >=20
> > 	extirq: interrupt-controller@1ac {
> > 		compatible =3D "fsl,ls1021a-extirq";
> > 		<bla bla>
> > 	};
> >=20
> > I booted the new DT blob (which has "interrupts-extended") on a kernel
> > where the ls-extirq driver did not exist. This had the result of
> > of_mdiobus_phy_device_register() -> of_irq_get() returning -EPROBE_DEFE=
R
> > forever and ever. So the PHY driver in turn never probed, and Ethernet
> > was broken. So I had to delete the interrupts OF property to let the PH=
Y
> > at least work in poll mode.
> >=20
> > What went wrong here in your opinion?
>=20
> I'm not sure what you expect me to say here. You have a device that
> references an interrupt. The DT seems sound (I don't get why you think
> "interrupt-extended" is a problem here, but hey...).
>=20
> If your kernel doesn't have a driver for the interrupt controller
> referenced here, what do you expect, other than things not working?
>=20
> 	M.
>=20
> --=20
> Without deviation from the norm, progress is not possible.

I was just raising this as what I thought would be a simple and
non-controversial counter example to your remark "If you change something,
you *must* guarantee forward *and* backward compatibility."

Practically speaking, what has happened is that the board DT appeared in
kernel N, the ls-extirq driver in kernel N+1, and the DT was updated to
enable PHY interrupts in kernel N+2. That DT update practically broke
kernel N from running correctly on DTs taken from kernel N+2 onwards.
This is the observable behavior, we can find as many justifications for
it as we wish.

(as to what I expect, Ethernet PHYs work without an interrupt too, but
 of_mdiobus_phy_device_register() treats -EPROBE_DEFER from of_irq_get()
 as special, because it assumes the IRQ domain will eventually come up.
 The IRQ is optional, as evidenced by the fact that kernel N worked)=