Re: [PATCH] pinctrl: amd: Steer GPIO interrupt to low power/efficiency cores

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential
issues to consider:
- [Low] pinctrl: amd: offline low-power CPUs prevent fallback in
  amd_gpio_set_irq_affinity()
- [Low] pinctrl: amd: UML build failure in amd_gpio_set_irq_affinity()
  under CONFIG_X86_64
--

commit 9979566a45b6fdd26d48ee240bec752ee33586ba
Author: Mario Limonciello <mario.limonciello@amd.com>

pinctrl: amd: Steer GPIO interrupt to low power/efficiency cores

This commit steers the AMD GPIO controller interrupt to low power or
efficiency cores on hybrid x86 systems during probe using
irq_set_affinity().

> diff --git a/drivers/pinctrl/pinctrl-amd.c b/drivers/pinctrl/pinctrl-amd.c
> index 30da75bb93b9a..3ae199cc93435 100644
> --- a/drivers/pinctrl/pinctrl-amd.c
> +++ b/drivers/pinctrl/pinctrl-amd.c
[ ... ]
> @@ -1272,6 +1274,41 @@ static void __iomem *amd_get_named_res(struct device *dev, struct platform_devic
>  	return base;
>  }
>  
> +#ifdef CONFIG_X86_64
> +/*
> + * On hybrid systems prefer to service the GPIO interrupt on low power cores,
> + * then on efficiency (dense) cores to save power.  Userspace can still
> + * override the placement.  Do nothing if neither core type exists.
> + */
> +static void amd_gpio_set_irq_affinity(struct amd_gpio *gpio_dev)
> +{
> +	static const enum x86_topology_cpu_type types[] = {
> +		TOPO_CPU_TYPE_LOW_POWER,
> +		TOPO_CPU_TYPE_EFFICIENCY,
> +	};

[Severity: Low]
Does guarding amd_gpio_set_irq_affinity() with CONFIG_X86_64 break 64-bit
ARCH=um builds when CONFIG_COMPILE_TEST is enabled?

On 64-bit User Mode Linux, arch/x86/um/Kconfig defines CONFIG_X86_64=y
without CONFIG_X86, and the UML processor headers define struct cpuinfo_um
rather than enum x86_topology_cpu_type or per-CPU cpu_data(cpu).  Guarding
this with CONFIG_X86 (or excluding CONFIG_UML) would avoid the build
failure.

> +	cpumask_var_t mask;
> +	int cpu, i;
> +
> +	if (!zalloc_cpumask_var(&mask, GFP_KERNEL))
> +		return;
> +
> +	for (i = 0; i < ARRAY_SIZE(types); i++) {
> +		for_each_possible_cpu(cpu) {
> +			if (cpu_data(cpu).topo.cpu_type == types[i])
> +				cpumask_set_cpu(cpu, mask);
> +		}
> +		if (!cpumask_empty(mask)) {
> +			irq_set_affinity(gpio_dev->irq, mask);
> +			break;
> +		}
> +	}

[Severity: Low]
Can offline low-power CPUs prevent amd_gpio_set_irq_affinity() from falling
back to efficiency cores?

When a CPU is offlined via hotplug on x86, remove_siblinginfo() leaves
cpu_data(cpu).topo.cpu_type intact.  Because the loop iterates over
for_each_possible_cpu(cpu) and breaks unconditionally whenever mask is
non-empty, if all TOPO_CPU_TYPE_LOW_POWER CPUs are offline when
amd_gpio_probe() runs, irq_do_set_affinity() rejects the offline-only mask
with -EINVAL and TOPO_CPU_TYPE_EFFICIENCY is never tried.  Iterating over
for_each_online_cpu(cpu) (or checking the return value of
irq_set_affinity()) would allow the fallback to proceed.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261111-superm1-pinctrl-power-improvement-v1-1-4bbe6cdbd2e8@amd.com?part=1