Received-SPF: pass (google.com: domain of linux-kernel+bounces-194344-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
Date: Wed, 29 May 2024 16:17:16 +0100
From: Conor Dooley <conor@kernel.org>
To: Marc Zyngier <maz@kernel.org>
Cc: Anup Patel <apatel@ventanamicro.com>, devicetree@vger.kernel.org,
	linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	linux-riscv@lists.infradead.org,
	Saravana Kannan <saravanak@google.com>,
	Rob Herring <robh@kernel.org>
Subject: Re: [PATCH] of: property: Fix fw_devlink handling of interrupt-map
Message-ID: <20240529-rust-tile-a05517a6260f@spud>
References: <20240528164132.2451685-1-maz@kernel.org>
 <CAK9=C2XNPJP0X=pg5TSrQbsuouDD3jP-gy2Sm4BXNJp0ZiWp+A@mail.gmail.com>
 <86bk4pm8j1.wl-maz@kernel.org>
 <CAK9=C2XRx==OTPoGW_AHmjq4Th0bv4okwcq6-3L5JYwHwQp97A@mail.gmail.com>
 <86a5k8nbh1.wl-maz@kernel.org>
 <CAK9=C2Ugq=0y8M86CD75mQccBo=TBLBomb4rqC4i1naKy2LyWQ@mail.gmail.com>
 <868qzsn7zs.wl-maz@kernel.org>
Precedence: bulk
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha256;
	protocol="application/pgp-signature"; boundary="QuT3/USFLVPqyc21"
Content-Disposition: inline
In-Reply-To: <868qzsn7zs.wl-maz@kernel.org>


--QuT3/USFLVPqyc21
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On Wed, May 29, 2024 at 01:00:07PM +0100, Marc Zyngier wrote:
> > > > In the RISC-V world, there have been quite a few QEMU releases
> > > > where the generated DT node of the interrupt controller does not
> > > > have the "#address-cells" property. This patch breaks the kernel
> > > > for all such QEMU releases.
> > >
> > > Congratulations, you've forked DT. News at 11.
> >=20
> > Can you elaborate how ?
>=20
> You've stated it yourself. You are relying on a behaviour that
> deviates from the standard by having DTs with missing properties
>=20
> And since we can't travel back it time to fix this, the only solution
> I can see is to support both behaviours by quirking it.

I'm not convinced that there is any actual production hardware that
would get broken by your patch, just QEMU, so I think it should get
fixed to output devicetrees that are spec compliant rather than add some
riscv-specific hacks that we can't even gate on the "qemu,aplic"
compatible because QEMU doesn't use the compatible created for it...

Spec violations aside, the QEMU aplic nodes in the DT contain a bunch
of other issues, including using properties that changed in the
upstreaming process. Here's the issues with Alistair's current riscv
tree for QEMU w/ -smp 4 -M virt,aia=3Daplic,dumpdtb=3D$(qemu_dtb) -cpu max =
-m 1G -nographic

qemu.dtb: aplic@d000000: $nodename:0: 'aplic@d000000' does not match '^inte=
rrupt-controller(@[0-9a-f,]+)*$'
        from schema $id: http://devicetree.org/schemas/interrupt-controller=
/riscv,aplic.yaml#
qemu.dtb: aplic@d000000: compatible:0: 'riscv,aplic' is not one of ['qemu,a=
plic']
        from schema $id: http://devicetree.org/schemas/interrupt-controller=
/riscv,aplic.yaml#
qemu.dtb: aplic@d000000: compatible: ['riscv,aplic'] is too short
        from schema $id: http://devicetree.org/schemas/interrupt-controller=
/riscv,aplic.yaml#
qemu.dtb: aplic@d000000: Unevaluated properties are not allowed ('compatibl=
e' was unexpected)
        from schema $id: http://devicetree.org/schemas/interrupt-controller=
/riscv,aplic.yaml#
qemu.dtb: aplic@c000000: $nodename:0: 'aplic@c000000' does not match '^inte=
rrupt-controller(@[0-9a-f,]+)*$'
        from schema $id: http://devicetree.org/schemas/interrupt-controller=
/riscv,aplic.yaml#
qemu.dtb: aplic@c000000: compatible:0: 'riscv,aplic' is not one of ['qemu,a=
plic']
        from schema $id: http://devicetree.org/schemas/interrupt-controller=
/riscv,aplic.yaml#
qemu.dtb: aplic@c000000: compatible: ['riscv,aplic'] is too short
        from schema $id: http://devicetree.org/schemas/interrupt-controller=
/riscv,aplic.yaml#
qemu.dtb: aplic@c000000: Unevaluated properties are not allowed ('compatibl=
e', 'riscv,delegate' were unexpected)
        from schema $id: http://devicetree.org/schemas/interrupt-controller=
/riscv,aplic.yaml#

I guess noone updated QEMU to comply with the bindings that actually got
upstreamed for the aplic?

--QuT3/USFLVPqyc21
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iHUEABYIAB0WIQRh246EGq/8RLhDjO14tDGHoIJi0gUCZldG/AAKCRB4tDGHoIJi
0oYWAPoDsUoaVQGx27axveY1n1lIrXkUGHlGo31YRIruXNo+ngD/R9/DjC94q7t/
JpAE9JiPE1EcGTOVFoRzpEHCL0sh8wg=
=qPe7
-----END PGP SIGNATURE-----

--QuT3/USFLVPqyc21--