Re: [PATCH 1/3] irqchip/gic-v3: Enable Rockchip 3588001 erratum workaround for RK3582

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hello Marc,

On 2024-12-22 10:04, Marc Zyngier wrote:
On Sun, 22 Dec 2024 03:03:53 +0000,
FUKAUMI Naoki <naoki@xxxxxxxxx> wrote:

Rockchip RK3582 is a scaled down version of Rockchip RK3588(S). Apply
Rockchip 3588001 erratum workaround to RK3582.

Signed-off-by: FUKAUMI Naoki <naoki@xxxxxxxxx>
---
 drivers/irqchip/irq-gic-v3-its.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
index 92244cfa0464..c59ce9332dc0 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -4861,7 +4861,8 @@ static bool __maybe_unused its_enable_rk3588001(void *data)
 {
 	struct its_node *its = data;

-	if (!of_machine_is_compatible("rockchip,rk3588") &&
+	if (!of_machine_is_compatible("rockchip,rk3582") &&
+	    !of_machine_is_compatible("rockchip,rk3588") &&
 	    !of_machine_is_compatible("rockchip,rk3588s"))
 		return false;


Please use the relevant property for that purpose ("dma-noncoherent")
at the distributor and ITS levels. We're not adding extra compatibles
for this anymore, and you might as well fix the core dtsi to expose
such property.

Thanks for your response.

After a more detailed look into drivers/irqchip/irq-gic-v3-its.c,
it seems that relying on the "dma-noncoherent" DT property may not
be equivalent to adding another compatible check.  Here are a few
quotations from irq-gic-v3-its.c, to illustrate this better:

4746 static bool __maybe_unused its_enable_rk3588001(void *data)
4747 {
4748         struct its_node *its = data;
4749
4750         if (!of_machine_is_compatible("rockchip,rk3588") &&
4751             !of_machine_is_compatible("rockchip,rk3588s"))
4752                 return false;
4753
4754         its->flags |= ITS_FLAGS_FORCE_NON_SHAREABLE;
4755         gic_rdists->flags |= RDIST_FLAGS_FORCE_NON_SHAREABLE;
4756
4757         return true;
4758 }
4759
4760 static bool its_set_non_coherent(void *data)
4761 {
4762         struct its_node *its = data;
4763
4764         its->flags |= ITS_FLAGS_FORCE_NON_SHAREABLE;
4765         return true;
4766 }

4814 #ifdef CONFIG_ROCKCHIP_ERRATUM_3588001
4815         {
4816                 .desc   = "ITS: Rockchip erratum RK3588001",
4817                 .iidr   = 0x0201743b,
4818                 .mask   = 0xffffffff,
4819                 .init   = its_enable_rk3588001,
4820         },
4821 #endif
4822         {
4823                 .desc   = "ITS: non-coherent attribute",
4824                 .property = "dma-noncoherent",
4825                 .init   = its_set_non_coherent,
4826         },

As visible above, using the "dma-noncoherent" DT property results
in not setting the RDIST_FLAGS_FORCE_NON_SHAREABLE flag, which the
its_enable_rk3588001() function does.  In other words, it doesn't
seem that "dma-noncoherent" is a "drop-in" replacement for adding
yet another compatible for the RK3582.

Modifying the current behavior of the "dma-noncoherent" DT property
doesn't seem like an option, because it's already used in a couple
of board dts(i) files.  Should we introduce another DT property,
perhaps "dma-noncoherent-rdist" or something similar?

Could you, please, advise on how to move forward with this?  I'm
willing to implement the required patches, but I'd prefer to reduce
the possible back-and-forth on them, to save everyone's time.




[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux