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]

 



On 2024-12-23 10:29, Marc Zyngier wrote:
On Mon, 23 Dec 2024 06:10:21 +0000,
Dragan Simic <dsimic@xxxxxxxxxxx> wrote:
On 2024-12-23 00:16, Marc Zyngier wrote:
> On Sun, 22 Dec 2024 18:25:02 +0000,
> Dragan Simic <dsimic@xxxxxxxxxxx> wrote:
>> 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.
>
> It is. My email makes it plain what needs doing.
>
>> 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         },
>
> Nothing tickles me more than having my own work being thrown back at
> me.

I'm sorry, that wasn't my intention.  I just wanted to make
referencing to what I was talking about a bit easier.  Though,
I now see that I was wrong, and I apologize for the noise.

No need to apologise. Just understand that the way you approached the
discussion was suboptimal. Next time, just ask how the proposed
solution works, rather than asserting that it doesn't.

Thanks.  Indeed, the way I approached it was waaay suboptimal.
I just wanted to clarify that it was an honest mistake resulting
from not looking at the code carefully enough, nothing else.

Hopefully we can move on and you and Naoki can come up with a set of
patches that does the right thing.

Of course.  I've already prepared a small patch series that,
hopefully, does the right thing when it comes to the Rockchip
3588001 errata.  I'll submit it soon, after I check the patches
a bit further.




[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