Re: [PATCH v2 1/1] irqchip/gic-v3: Add Altera Agilex5 address bus width limitation workaround

[Marc Zyngier <maz@kernel.org>]

On Fri, 04 Jul 2025 09:49:50 +0100,
adrianhoyin.ng@altera.com wrote:
> 
> From: Adrian Ng Ho Yin <adrianhoyin.ng@altera.com>
> 
> Agilex5 address bus width for the ACE-lite interface is only 32 bits.
> Hence the GIC600 SoC integration for Agilex5 can only access the first
> 32bit of the physical address space.
> 
> Add quirk to configure the gfp flag to allocate memory within 32bit
> addressable range. As the 0x0201743b GIC600 ID is not specific to
> Altera, of_machine_is_compatible() is added.
>
> Signed-off-by: Adrian Ng Ho Yin <adrianhoyin.ng@altera.com>
> ---
>  arch/arm64/Kconfig               | 10 ++++++++++
>  drivers/irqchip/irq-gic-v3-its.c | 18 ++++++++++++++++++
>  2 files changed, 28 insertions(+)
> 
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index 55fc331af337..2286b4d378e2 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -1348,6 +1348,16 @@ config SOCIONEXT_SYNQUACER_PREITS
>  
>  	  If unsure, say Y.
>  
> +config ALTERA_AGILEX5_ADDR_BUS_WIDTH_LIMITATION
> +	bool "Altera Agilex: GIC600 can not access physical addresses higher than 4GB"
> +	default y
> +	help
> +	  Agilex5 address bus width for the ACE-lite interface is only 32 bits. Hence
> +	  the GIC600 SoC integration for Agilex5 can only access the first 32bit of the
> +	  physical address space.

You're describing it as a feature. But really, it's a bug, and it
deserves an erratum number.

> +
> +	  If unsure, say Y.
> +
>  endmenu # "ARM errata workarounds via the alternatives framework"
>  
>  choice
> diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
> index d54fa0638dc4..a2cf401568e7 100644
> --- a/drivers/irqchip/irq-gic-v3-its.c
> +++ b/drivers/irqchip/irq-gic-v3-its.c
> @@ -4901,6 +4901,16 @@ static bool __maybe_unused its_enable_rk3568002(void *data)
>  	return true;
>  }
>  
> +static bool __maybe_unused its_enable_agilex5(void *data)
> +{
> +	if (!of_machine_is_compatible("intel,socfpga-agilex5"))
> +		return false;
> +
> +	gfp_flags_quirk |= GFP_DMA32;
> +
> +	return true;
> +}
> +
>  static const struct gic_quirk its_quirks[] = {
>  #ifdef CONFIG_CAVIUM_ERRATUM_22375
>  	{
> @@ -4975,6 +4985,14 @@ static const struct gic_quirk its_quirks[] = {
>  		.mask   = 0xffffffff,
>  		.init   = its_enable_rk3568002,
>  	},
> +#endif
> +#ifdef ALTERA_AGILEX5_ADDR_BUS_WIDTH_LIMITATION
> +	{
> +		.desc   = "ITS: Altera Agilex5 address bus width limitation",
> +		.iidr   = 0x0201743b,
> +		.mask   = 0xffffffff,
> +		.init   = its_enable_agilex5,
> +	},
>  #endif
>  	{
>  	}

Again, why do you need to reinvent the wheel? There is already *just
above* an existing workaround that implements exactly the same thing.

The whole thing could look like the hack below.

	M.

diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
index d54fa0638dc44..7047408de3e37 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -4890,10 +4890,17 @@ static bool __maybe_unused its_enable_quirk_hip09_162100801(void *data)
 	return true;
 }
 
-static bool __maybe_unused its_enable_rk3568002(void *data)
+static bool its_enable_32bit_disaster(void *data)
 {
-	if (!of_machine_is_compatible("rockchip,rk3566") &&
-	    !of_machine_is_compatible("rockchip,rk3568"))
+	static const char *broken[] = {
+#ifdef CONFIG_ROCKCHIP_ERRATUM_3568002
+		"rockchip,rk3566", "rockchip,rk3568",
+#endif
+		"intel,socfpga-agilex5",
+		NULL,
+	};
+
+	if (!of_machine_compatible_match(broken))
 		return false;
 
 	gfp_flags_quirk |= GFP_DMA32;
@@ -4968,14 +4975,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",
 		.iidr   = 0x0201743b,
 		.mask   = 0xffffffff,
-		.init   = its_enable_rk3568002,
+		.init   = its_enable_32bit_disaster,
 	},
-#endif
 	{
 	}
 };

-- 
Without deviation from the norm, progress is not possible.