Re: [PATCH] arm64: dts: rockchip: adjust SMMU interrupt type

[Niklas Cassel <cassel@kernel.org>]

On Tue, Feb 11, 2025 at 08:40:25AM +0100, Heiko Stübner wrote:
> Am Montag, 10. Februar 2025, 22:37:29 MEZ schrieb Patrick Wildt:
> > The SMMU architecture requires wired interrupts to be edge triggered,
> > which does not align with the DT description for the RK3588.  This leads
> > to interrupt storms, as the SMMU continues to hold the pin high and only
> > pulls it down for a short amount when issuing an IRQ.  Update the DT
> > description to be in line with the spec and perceived reality.
> > 
> 
> Cc'ed Niklas
> 
> This should probably also get a
> 
> Fixes: cd81d3a0695c ("arm64: dts: rockchip: add rk3588 pcie and php IOMMUs")

Agreed.


> 
> > Signed-off-by: Patrick Wildt <patrick@blueri.se>
> > ---
> >  arch/arm64/boot/dts/rockchip/rk3588-base.dtsi | 16 ++++++++--------
> >  1 file changed, 8 insertions(+), 8 deletions(-)
> > 
> > diff --git a/arch/arm64/boot/dts/rockchip/rk3588-base.dtsi b/arch/arm64/boot/dts/rockchip/rk3588-base.dtsi
> > index 8cfa30837ce7..520d0814a4de 100644
> > --- a/arch/arm64/boot/dts/rockchip/rk3588-base.dtsi
> > +++ b/arch/arm64/boot/dts/rockchip/rk3588-base.dtsi
> > @@ -549,10 +549,10 @@ usb_host2_xhci: usb@fcd00000 {
> >  	mmu600_pcie: iommu@fc900000 {
> >  		compatible = "arm,smmu-v3";
> >  		reg = <0x0 0xfc900000 0x0 0x200000>;
> > -		interrupts = <GIC_SPI 369 IRQ_TYPE_LEVEL_HIGH 0>,
> > -			     <GIC_SPI 371 IRQ_TYPE_LEVEL_HIGH 0>,
> > -			     <GIC_SPI 374 IRQ_TYPE_LEVEL_HIGH 0>,
> > -			     <GIC_SPI 367 IRQ_TYPE_LEVEL_HIGH 0>;
> > +		interrupts = <GIC_SPI 369 IRQ_TYPE_EDGE_RISING 0>,
> > +			     <GIC_SPI 371 IRQ_TYPE_EDGE_RISING 0>,
> > +			     <GIC_SPI 374 IRQ_TYPE_EDGE_RISING 0>,
> > +			     <GIC_SPI 367 IRQ_TYPE_EDGE_RISING 0>;
> >  		interrupt-names = "eventq", "gerror", "priq", "cmdq-sync";
> >  		#iommu-cells = <1>;
> >  	};
> > @@ -560,10 +560,10 @@ mmu600_pcie: iommu@fc900000 {
> >  	mmu600_php: iommu@fcb00000 {
> >  		compatible = "arm,smmu-v3";
> >  		reg = <0x0 0xfcb00000 0x0 0x200000>;
> > -		interrupts = <GIC_SPI 381 IRQ_TYPE_LEVEL_HIGH 0>,
> > -			     <GIC_SPI 383 IRQ_TYPE_LEVEL_HIGH 0>,
> > -			     <GIC_SPI 386 IRQ_TYPE_LEVEL_HIGH 0>,
> > -			     <GIC_SPI 379 IRQ_TYPE_LEVEL_HIGH 0>;
> > +		interrupts = <GIC_SPI 381 IRQ_TYPE_EDGE_RISING 0>,
> > +			     <GIC_SPI 383 IRQ_TYPE_EDGE_RISING 0>,
> > +			     <GIC_SPI 386 IRQ_TYPE_EDGE_RISING 0>,
> > +			     <GIC_SPI 379 IRQ_TYPE_EDGE_RISING 0>;
> >  		interrupt-names = "eventq", "gerror", "priq", "cmdq-sync";
> >  		#iommu-cells = <1>;
> >  		status = "disabled";
> > 

Patrick, thank you for the patch!

FWIW, they have the same bug in downstream:
https://github.com/radxa/kernel/blob/linux-6.1-stan-rkr4.1/arch/arm64/boot/dts/rockchip/rk3588s.dtsi#L2761-L2783

However, the Rockchip PCIe Virtualization Developer Guide correctly define
the IRQs as edge triggered:
https://dl.radxa.com/users/dev/Rockchip_PCIe_Virtualization_Developer_Guide_CN.pdf

Looking at the ARM SMMUv3 architecture specification:
"An implementation must support one of, or optionally both of, wired
interrupts and MSIs. Whether an implementation supports MSIs is discoverable
from SMMU_IDR0.MSI and SMMU_S_IDR0.MSI. An implementation might support wired
interrupt outputs that are edge-triggered. The discovery of support for wired
interrupts is IMPLEMENTATION DEFINED."

Thus:
Reviewed-by: Niklas Cassel <cassel@kernel.org>


Heiko, this patch should go to 6.14.

Side note: We also have another SMMU patch that should go to 6.14:
https://lore.kernel.org/linux-rockchip/20250207143900.2047949-2-cassel@kernel.org/


Kind regards,
Niklas