Re: [PATCH 2/3] irqchip/gic-v3: Add Renesas R-Car Gen4 erratum workaround

[Marek Vasut <marek.vasut@mailbox.org>]

On 6/17/26 9:24 AM, Marc Zyngier wrote:

Hello Marc,

>> Renesas R-Car S4/V4H/V4M GIC600 integration has address width for AXI
>> or APB interface configured to 32 bit, it can therefore access only
>> the first 4 GiB of physical address space. This information comes from
>> R-Car V4H Interface Specification sheet, there is currently no technical
>> update number assigned to this limitation. Further input from hardware
>> engineer indicates that this limitation also applies to R-Car S4 and V4M.
>> Name the limitation GEN4GICITS1, and add a driver quirk to mitigate this
>> limitation.

My concern is this ^ , I do not have an erratum number, because there 
isn't one. I am in touch with the hardware engineer and I did get a 
glimpse at internal details of the three SoC, which confirm the 
limitations. Is this sufficient ?

>> Note that the 0x0201743b GIC600 ID is not Renesas-specific, it is
>> common for many ARM GICv3 implementations. Therefore, add an extra
> 
> Not quite. It designates GIC600 unambiguously.

What I am trying to communicate is, that the 0x0201743b ID is not ID of 
the Renesas GIC implementation, but it is a generic ARM GIC600 ID. That 
is why we cannot match the quirk on the ID (it is generic ARM GIC600 
ID), and instead we have to match the quirk on the [ ID combined with 
of_machine_is_compatible("renesas,...") ].

> It is just that GIC600
> is integrated in zillions of SoCs, most of which don't have this
> problem (the machine I'm typing this from has a GIC600 *and* 96GB of
> RAM).

Right.

Shall I reword this paragraph somehow to make it clearer ?

>> of_machine_is_compatible() check.
>>
>> The GIC600 implementation in R-Car S4/V4H/V4M is r1p6.
> 
> Is this relevant?

I included it for the sake of completeness and to provide all relevant 
information, based on previous discussions about similar limitations 
that I could find on lore.k.o

[...]

>> +#ifdef CONFIG_RENESAS_ERRATUM_GEN4GICITS1
>> +	{
>> +		.desc   = "ITS: Renesas R-Car Gen4 GIC600 32-bit limit",
>> +		.iidr   = 0x0201743b,
>> +		.mask   = 0xffffffff,
>> +		.init   = its_enable_renesas_gen4,
>> +	},
>>   #endif
>>   	{
>>   	}
> 
> 
> Honestly, that's a bit too much copy-paste for my taste. Just refactor
> the erratum handling to be more generic, something like this:
> 
> diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
> index 291d7668cc8da..380c4758647d2 100644
> --- a/drivers/irqchip/irq-gic-v3-its.c
> +++ b/drivers/irqchip/irq-gic-v3-its.c
> @@ -4894,10 +4894,17 @@ static bool __maybe_unused its_enable_quirk_hip09_162100801(void *data)
>   	return true;
>   }
>   
> -static bool __maybe_unused its_enable_rk3568002(void *data)
> +static const char * const dma_impaired_platforms[] = {
> +#ifdef CONFIG_ROCKCHIP_ERRATUM_3568002
> +	"rockchip,rk3566",
> +	"rockchip,rk3568",
> +#endif
> +	NULL,
> +};
> +
> +static bool __maybe_unused its_enable_dma32(void *data)
>   {
> -	if (!of_machine_is_compatible("rockchip,rk3566") &&
> -	    !of_machine_is_compatible("rockchip,rk3568"))
> +	if (!of_machine_compatible_match(dma_impaired_platforms))
>   		return false;
>   
>   	gfp_flags_quirk |= GFP_DMA32;
> @@ -4972,14 +4979,12 @@ static const struct gic_quirk its_quirks[] = {
>   		.property = "dma-noncoherent",
>   		.init   = its_set_non_coherent,
>   	},
> -#ifdef CONFIG_ROCKCHIP_ERRATUM_3568002
>   	{
> -		.desc   = "ITS: Rockchip erratum RK3568002",
> +		.desc   = "ITS: Broken GIC600 integration limited to 32bit PA",
>   		.iidr   = 0x0201743b,
>   		.mask   = 0xffffffff,
> -		.init   = its_enable_rk3568002,
> +		.init   = its_enable_dma32,
>   	},
> -#endif
>   	{
>   	}
>   };
> 
> Then add the two lines you need in a separate patch.

Will do in V2.

> In the future, please provide a cover letter when you have more than a
> single patch (git will happily generate one for you).
OK