* [PATCH 0/2] Use "dma-noncoherent" to handle Rockchip RK358x 3588001 errata @ 2024-12-27 15:42 Dragan Simic 2024-12-27 15:42 ` [PATCH 1/2] arm64: dts: rockchip: Use "dma-noncoherent" in base RK3588 SoC dtsi Dragan Simic 2024-12-27 15:42 ` [PATCH 2/2] irqchip/gic-v3-its: Make "dma-noncoherent" preferred for RK358x errata Dragan Simic 0 siblings, 2 replies; 6+ messages in thread From: Dragan Simic @ 2024-12-27 15:42 UTC (permalink / raw) To: linux-rockchip Cc: heiko, maz, tglx, linux-arm-kernel, linux-kernel, devicetree, robh, krzk+dt, conor+dt, FUKAUMI Naoki This is a small series that makes the "dma-noncoherent" DT property used for handling the 3588001 errata for Rockchip RK3588, RK3588S and RK3582 SoCs, which is the preferred way over checking the compatibles. [1] Older versions of the Rockchip RK3588 and RK3588S SoC dts(i) files failed to specify the "dma-noncoherent" DT property, so checking the compatibles remains required for backward SoC dts(i) compatibility. [1] https://lore.kernel.org/linux-rockchip/86msgoozqa.wl-maz@kernel.org/ Cc: Marc Zyngier <maz@kernel.org> Cc: FUKAUMI Naoki <naoki@radxa.com> Dragan Simic (2): arm64: dts: rockchip: Use "dma-noncoherent" in base RK3588 SoC dtsi irqchip/gic-v3-its: Make "dma-noncoherent" preferred for RK358x errata arch/arm64/boot/dts/rockchip/rk3588-base.dtsi | 3 +++ drivers/irqchip/irq-gic-v3-its.c | 12 ++++++++++++ 2 files changed, 15 insertions(+) ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 1/2] arm64: dts: rockchip: Use "dma-noncoherent" in base RK3588 SoC dtsi 2024-12-27 15:42 [PATCH 0/2] Use "dma-noncoherent" to handle Rockchip RK358x 3588001 errata Dragan Simic @ 2024-12-27 15:42 ` Dragan Simic 2024-12-27 16:47 ` Marc Zyngier 2024-12-27 15:42 ` [PATCH 2/2] irqchip/gic-v3-its: Make "dma-noncoherent" preferred for RK358x errata Dragan Simic 1 sibling, 1 reply; 6+ messages in thread From: Dragan Simic @ 2024-12-27 15:42 UTC (permalink / raw) To: linux-rockchip Cc: heiko, maz, tglx, linux-arm-kernel, linux-kernel, devicetree, robh, krzk+dt, conor+dt, FUKAUMI Naoki The preferred way to denote hardware with non-coherent DMA is to use the "dma-noncoherent" DT property, at both the GIC redistributor and the GIC ITS levels, [1] instead of relying on the compatibles to handle hardware errata, in this case the Rockchip 3588001 errata. [2] Let's have the preferred way employed in the base Rockchip RK3588 SoC dtsi, which also goes along with adding initial support for the Rockchip RK3582 SoC variant, with its separate compatible. [2][3] [1] Documentation/devicetree/bindings/interrupt-controller/arm,gic-v3.yaml [2] https://lore.kernel.org/linux-rockchip/86msgoozqa.wl-maz@kernel.org/ [3] https://lore.kernel.org/linux-rockchip/20241222030355.2246-4-naoki@radxa.com/ Cc: Marc Zyngier <maz@kernel.org> Cc: FUKAUMI Naoki <naoki@radxa.com> Signed-off-by: Dragan Simic <dsimic@manjaro.org> --- arch/arm64/boot/dts/rockchip/rk3588-base.dtsi | 3 +++ 1 file changed, 3 insertions(+) diff --git a/arch/arm64/boot/dts/rockchip/rk3588-base.dtsi b/arch/arm64/boot/dts/rockchip/rk3588-base.dtsi index d97d84b88837..bd2385b6bd7f 100644 --- a/arch/arm64/boot/dts/rockchip/rk3588-base.dtsi +++ b/arch/arm64/boot/dts/rockchip/rk3588-base.dtsi @@ -1972,27 +1972,30 @@ &i2s3_sdi gic: interrupt-controller@fe600000 { compatible = "arm,gic-v3"; + dma-noncoherent; reg = <0x0 0xfe600000 0 0x10000>, /* GICD */ <0x0 0xfe680000 0 0x100000>; /* GICR */ interrupts = <GIC_PPI 9 IRQ_TYPE_LEVEL_HIGH 0>; interrupt-controller; mbi-alias = <0x0 0xfe610000>; mbi-ranges = <424 56>; msi-controller; ranges; #address-cells = <2>; #interrupt-cells = <4>; #size-cells = <2>; its0: msi-controller@fe640000 { compatible = "arm,gic-v3-its"; + dma-noncoherent; reg = <0x0 0xfe640000 0x0 0x20000>; msi-controller; #msi-cells = <1>; }; its1: msi-controller@fe660000 { compatible = "arm,gic-v3-its"; + dma-noncoherent; reg = <0x0 0xfe660000 0x0 0x20000>; msi-controller; #msi-cells = <1>; ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] arm64: dts: rockchip: Use "dma-noncoherent" in base RK3588 SoC dtsi 2024-12-27 15:42 ` [PATCH 1/2] arm64: dts: rockchip: Use "dma-noncoherent" in base RK3588 SoC dtsi Dragan Simic @ 2024-12-27 16:47 ` Marc Zyngier 0 siblings, 0 replies; 6+ messages in thread From: Marc Zyngier @ 2024-12-27 16:47 UTC (permalink / raw) To: Dragan Simic Cc: linux-rockchip, heiko, tglx, linux-arm-kernel, linux-kernel, devicetree, robh, krzk+dt, conor+dt, FUKAUMI Naoki On Fri, 27 Dec 2024 15:42:23 +0000, Dragan Simic <dsimic@manjaro.org> wrote: > > The preferred way to denote hardware with non-coherent DMA is to use the > "dma-noncoherent" DT property, at both the GIC redistributor and the GIC ITS > levels, [1] instead of relying on the compatibles to handle hardware errata, > in this case the Rockchip 3588001 errata. [2] > > Let's have the preferred way employed in the base Rockchip RK3588 SoC dtsi, > which also goes along with adding initial support for the Rockchip RK3582 SoC > variant, with its separate compatible. [2][3] > > [1] Documentation/devicetree/bindings/interrupt-controller/arm,gic-v3.yaml > [2] https://lore.kernel.org/linux-rockchip/86msgoozqa.wl-maz@kernel.org/ > [3] https://lore.kernel.org/linux-rockchip/20241222030355.2246-4-naoki@radxa.com/ > > Cc: Marc Zyngier <maz@kernel.org> > Cc: FUKAUMI Naoki <naoki@radxa.com> > Signed-off-by: Dragan Simic <dsimic@manjaro.org> Acked-by: Marc Zyngier <maz@kernel.org> M. -- Without deviation from the norm, progress is not possible. ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 2/2] irqchip/gic-v3-its: Make "dma-noncoherent" preferred for RK358x errata 2024-12-27 15:42 [PATCH 0/2] Use "dma-noncoherent" to handle Rockchip RK358x 3588001 errata Dragan Simic 2024-12-27 15:42 ` [PATCH 1/2] arm64: dts: rockchip: Use "dma-noncoherent" in base RK3588 SoC dtsi Dragan Simic @ 2024-12-27 15:42 ` Dragan Simic 2024-12-27 16:46 ` Marc Zyngier 1 sibling, 1 reply; 6+ messages in thread From: Dragan Simic @ 2024-12-27 15:42 UTC (permalink / raw) To: linux-rockchip Cc: heiko, maz, tglx, linux-arm-kernel, linux-kernel, devicetree, robh, krzk+dt, conor+dt, FUKAUMI Naoki The preferred way to denote hardware with non-coherent DMA is to use the "dma-noncoherent" DT property, [1] instead of relying on the compatibles. [2] Alas, older versions of the Rockchip RK3588 and RK3588S SoC dts(i) files failed to specify this DT property, which means that checking the compatibles remains required for backward SoC dts(i) compatibility. Let's have the Rockchip 3588001 hardware errata handled the preferred way, with newer versions of the Rockchip RK3588, RK3588S and RK3582 SoC dts(i) files that properly specify the "dma-noncoherent" DT properties at both the GIC redistributor and the GIC ITS levels, while falling back to checking the compatibles for backward RK3588 and RK3588S SoC dts(i) compatibility. [1] Documentation/devicetree/bindings/interrupt-controller/arm,gic-v3.yaml [2] https://lore.kernel.org/linux-rockchip/86msgoozqa.wl-maz@kernel.org/ Cc: Marc Zyngier <maz@kernel.org> Cc: FUKAUMI Naoki <naoki@radxa.com> Signed-off-by: Dragan Simic <dsimic@manjaro.org> --- drivers/irqchip/irq-gic-v3-its.c | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c index fdec478ba5e7..982dcbb30f39 100644 --- a/drivers/irqchip/irq-gic-v3-its.c +++ b/drivers/irqchip/irq-gic-v3-its.c @@ -4747,6 +4747,18 @@ static bool __maybe_unused its_enable_rk3588001(void *data) { struct its_node *its = data; + /* + * The preferred way to denote hardware with non-coherent DMA is to use + * the "dma-noncoherent" DT property, which the older RK3588 SoC dts(i) + * files failed to specify, relying on the compatibles instead. + * + * Thus, check for the presence of "dma-noncoherent" DT property first, + * to let the hardware quirk be handled the preferred way, and fall back + * to checking the compatibles for backward dts(i) compatibility. + */ + if (!of_dma_is_coherent(to_of_node(its->fwnode_handle))) + return false; + if (!of_machine_is_compatible("rockchip,rk3588") && !of_machine_is_compatible("rockchip,rk3588s")) return false; ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] irqchip/gic-v3-its: Make "dma-noncoherent" preferred for RK358x errata 2024-12-27 15:42 ` [PATCH 2/2] irqchip/gic-v3-its: Make "dma-noncoherent" preferred for RK358x errata Dragan Simic @ 2024-12-27 16:46 ` Marc Zyngier 2024-12-27 17:08 ` Dragan Simic 0 siblings, 1 reply; 6+ messages in thread From: Marc Zyngier @ 2024-12-27 16:46 UTC (permalink / raw) To: Dragan Simic Cc: linux-rockchip, heiko, tglx, linux-arm-kernel, linux-kernel, devicetree, robh, krzk+dt, conor+dt, FUKAUMI Naoki On Fri, 27 Dec 2024 15:42:24 +0000, Dragan Simic <dsimic@manjaro.org> wrote: > > The preferred way to denote hardware with non-coherent DMA is to use the > "dma-noncoherent" DT property, [1] instead of relying on the compatibles. [2] > Alas, older versions of the Rockchip RK3588 and RK3588S SoC dts(i) files > failed to specify this DT property, which means that checking the compatibles > remains required for backward SoC dts(i) compatibility. > > Let's have the Rockchip 3588001 hardware errata handled the preferred way, > with newer versions of the Rockchip RK3588, RK3588S and RK3582 SoC dts(i) > files that properly specify the "dma-noncoherent" DT properties at both the > GIC redistributor and the GIC ITS levels, while falling back to checking the > compatibles for backward RK3588 and RK3588S SoC dts(i) compatibility. > > [1] Documentation/devicetree/bindings/interrupt-controller/arm,gic-v3.yaml > [2] https://lore.kernel.org/linux-rockchip/86msgoozqa.wl-maz@kernel.org/ > > Cc: Marc Zyngier <maz@kernel.org> > Cc: FUKAUMI Naoki <naoki@radxa.com> > Signed-off-by: Dragan Simic <dsimic@manjaro.org> > --- > drivers/irqchip/irq-gic-v3-its.c | 12 ++++++++++++ > 1 file changed, 12 insertions(+) > > diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c > index fdec478ba5e7..982dcbb30f39 100644 > --- a/drivers/irqchip/irq-gic-v3-its.c > +++ b/drivers/irqchip/irq-gic-v3-its.c > @@ -4747,6 +4747,18 @@ static bool __maybe_unused its_enable_rk3588001(void *data) > { > struct its_node *its = data; > > + /* > + * The preferred way to denote hardware with non-coherent DMA is to use > + * the "dma-noncoherent" DT property, which the older RK3588 SoC dts(i) > + * files failed to specify, relying on the compatibles instead. > + * > + * Thus, check for the presence of "dma-noncoherent" DT property first, > + * to let the hardware quirk be handled the preferred way, and fall back > + * to checking the compatibles for backward dts(i) compatibility. > + */ > + if (!of_dma_is_coherent(to_of_node(its->fwnode_handle))) > + return false; > + > if (!of_machine_is_compatible("rockchip,rk3588") && > !of_machine_is_compatible("rockchip,rk3588s")) > return false; > I'm sorry, but this patch serves no purpose. We already have a workaround in place, and we don't need to handle it *twice*. Worse, it is actively harmful. A DT that would only advertise the property on the ITS would result in a broken system, while it would work correctly with the current code. Additionally, look at what of_dma_is_coherent() does, and how it will fetch a property that is potentially completely unrelated to the GIC node, or eventually return some random default. The binding documents the dma-noncoherent property as part of the relevant nodes, and not anywhere else. In the end, this is just dead code, and potentially worse. So while I'm rather supportive of your first patch in this series as it allows OSs other than Linux to *maybe* avoid this hack, the Linux driver doesn't need any change. Thanks, M. -- Without deviation from the norm, progress is not possible. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] irqchip/gic-v3-its: Make "dma-noncoherent" preferred for RK358x errata 2024-12-27 16:46 ` Marc Zyngier @ 2024-12-27 17:08 ` Dragan Simic 0 siblings, 0 replies; 6+ messages in thread From: Dragan Simic @ 2024-12-27 17:08 UTC (permalink / raw) To: Marc Zyngier Cc: linux-rockchip, heiko, tglx, linux-arm-kernel, linux-kernel, devicetree, robh, krzk+dt, conor+dt, FUKAUMI Naoki Hello Marc, On 2024-12-27 17:46, Marc Zyngier wrote: > On Fri, 27 Dec 2024 15:42:24 +0000, > Dragan Simic <dsimic@manjaro.org> wrote: >> >> The preferred way to denote hardware with non-coherent DMA is to use >> the >> "dma-noncoherent" DT property, [1] instead of relying on the >> compatibles. [2] >> Alas, older versions of the Rockchip RK3588 and RK3588S SoC dts(i) >> files >> failed to specify this DT property, which means that checking the >> compatibles >> remains required for backward SoC dts(i) compatibility. >> >> Let's have the Rockchip 3588001 hardware errata handled the preferred >> way, >> with newer versions of the Rockchip RK3588, RK3588S and RK3582 SoC >> dts(i) >> files that properly specify the "dma-noncoherent" DT properties at >> both the >> GIC redistributor and the GIC ITS levels, while falling back to >> checking the >> compatibles for backward RK3588 and RK3588S SoC dts(i) compatibility. >> >> [1] >> Documentation/devicetree/bindings/interrupt-controller/arm,gic-v3.yaml >> [2] >> https://lore.kernel.org/linux-rockchip/86msgoozqa.wl-maz@kernel.org/ >> >> Cc: Marc Zyngier <maz@kernel.org> >> Cc: FUKAUMI Naoki <naoki@radxa.com> >> Signed-off-by: Dragan Simic <dsimic@manjaro.org> >> --- >> drivers/irqchip/irq-gic-v3-its.c | 12 ++++++++++++ >> 1 file changed, 12 insertions(+) >> >> diff --git a/drivers/irqchip/irq-gic-v3-its.c >> b/drivers/irqchip/irq-gic-v3-its.c >> index fdec478ba5e7..982dcbb30f39 100644 >> --- a/drivers/irqchip/irq-gic-v3-its.c >> +++ b/drivers/irqchip/irq-gic-v3-its.c >> @@ -4747,6 +4747,18 @@ static bool __maybe_unused >> its_enable_rk3588001(void *data) >> { >> struct its_node *its = data; >> >> + /* >> + * The preferred way to denote hardware with non-coherent DMA is to >> use >> + * the "dma-noncoherent" DT property, which the older RK3588 SoC >> dts(i) >> + * files failed to specify, relying on the compatibles instead. >> + * >> + * Thus, check for the presence of "dma-noncoherent" DT property >> first, >> + * to let the hardware quirk be handled the preferred way, and fall >> back >> + * to checking the compatibles for backward dts(i) compatibility. >> + */ >> + if (!of_dma_is_coherent(to_of_node(its->fwnode_handle))) >> + return false; >> + >> if (!of_machine_is_compatible("rockchip,rk3588") && >> !of_machine_is_compatible("rockchip,rk3588s")) >> return false; >> > > I'm sorry, but this patch serves no purpose. We already have a > workaround in place, and we don't need to handle it *twice*. Thanks for reviewing this series so quickly! Regarding handling the workaround twice, I agree that this patch is somewhat redundant, but without this patch and with the "dma- noncoherent" DT properties in the proper places we'll actually end up with some kind of double hanling. More precisely, RDIST_FLAGS_FORCE_NON_SHAREABLE will be set twice, which doesn't do any harm, but it's something that made me twitch a bit and produce this patch. > Worse, it is actively harmful. A DT that would only advertise the > property on the ITS would result in a broken system, while it would > work correctly with the current code. Indeed, that possible issue bugged me as well. Though, we can probably rest assured that the RK3588 dtsi files are changed to include the "dma-noncoherent" DT properties correctly. > Additionally, look at what of_dma_is_coherent() does, and how it will > fetch a property that is potentially completely unrelated to the GIC > node, or eventually return some random default. The binding documents > the dma-noncoherent property as part of the relevant nodes, and not > anywhere else. That's a possibility, but I think that the same argument about the RK3588 dtsi correctness may also apply here. > In the end, this is just dead code, and potentially worse. So while > I'm rather supportive of your first patch in this series as it allows > OSs other than Linux to *maybe* avoid this hack, the Linux driver > doesn't need any change. I'm happy to drop this patch, but I just wanted to describe the rationale behind it, as explained above. ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2024-12-27 17:28 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2024-12-27 15:42 [PATCH 0/2] Use "dma-noncoherent" to handle Rockchip RK358x 3588001 errata Dragan Simic 2024-12-27 15:42 ` [PATCH 1/2] arm64: dts: rockchip: Use "dma-noncoherent" in base RK3588 SoC dtsi Dragan Simic 2024-12-27 16:47 ` Marc Zyngier 2024-12-27 15:42 ` [PATCH 2/2] irqchip/gic-v3-its: Make "dma-noncoherent" preferred for RK358x errata Dragan Simic 2024-12-27 16:46 ` Marc Zyngier 2024-12-27 17:08 ` Dragan Simic
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).