Use "dma-noncoherent" to handle Rockchip RK358x 3588001 errata

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/[email protected]/
[3] https://lore.kernel.org/linux-rockchip/[email protected]/

Cc Marc Zyngier <[email protected]>

Cc FUKAUMI Naoki <[email protected]>

Signed-off-by Dragan Simic <[email protected]>

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/[email protected]/

Cc Marc Zyngier <[email protected]>

Cc FUKAUMI Naoki <[email protected]>

Signed-off-by Dragan Simic <[email protected]>

---
 drivers/irqchip/irq-gic-v3-its.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

drivers/irqchip/irq-gic-v3-its.c +5 -0

index fdec478ba5e7..982dcbb30f39 100644
@@ -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.
+	 *

Quoted 47 lines · Dragan Simic

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/[email protected]/

Cc Marc Zyngier <[email protected]>

Cc FUKAUMI Naoki <[email protected]>

Signed-off-by Dragan Simic <[email protected]>

---
 drivers/irqchip/irq-gic-v3-its.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

drivers/irqchip/irq-gic-v3-its.c +2 -0

index fdec478ba5e7..982dcbb30f39 100644
@@ -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

drivers/irqchip/irq-gic-v3-its.c +1 -0

+	 * the "dma-noncoherent" DT property, which the older RK3588 SoC 

dts(i)

drivers/irqchip/irq-gic-v3-its.c +3 -0

+	 * files failed to specify, relying on the compatibles instead.
+	 *
+	 * Thus, check for the presence of "dma-noncoherent" DT property 

first,

drivers/irqchip/irq-gic-v3-its.c +1 -0

+	 * to let the hardware quirk be handled the preferred way, and fall 

back

drivers/irqchip/irq-gic-v3-its.c +4 -0

+	 * to checking the compatibles for backward dts(i) compatibility.
+	 */
+	if (!of_dma_is_coherent(to_of_node(its->fwnode_handle)))
+		return false;

drivers/irqchip/irq-gic-v3-its.c static bool __maybe_unused its_enable_rk3588001(void *data) · line 4764

Quoted 5 lines

drivers/irqchip/irq-gic-v3-its.c +1 -0

+
 	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.