On Sun, Nov 19, 2023 at 2:54 AM Youngmin Nam <youngmin.nam@xxxxxxxxxxx> wrote:
>
> To support affinity setting for non wake up external gpio interrupt,
> we add a new irq_set_affinity callback using irq number which is in pinctrl
> driver data.
>
> Before applying this patch, we couldn't change irq affinity of gpio interrupt.
> * before
> erd9945:/proc/irq/418 # cat smp_affinity
> 3ff
> erd9945:/proc/irq/418 # echo 00f > smp_affinity
> erd9945:/proc/irq/418 # cat smp_affinity
> 3ff
> erd9945:/proc/irq/418 # cat /proc/interrupts
> CPU0 CPU1 CPU2 CPU3 CPU4 CPU5 CPU6 CPU7 CPU8 CPU9
> 418: 3631 0 0 0 0 0 0 0 0 0 gpg2 0 Edge 19100000.drmdecon
>
> After applying this patch, we can change irq affinity of gpio interrupt as below.
> * after
> erd9945:/proc/irq/418 # cat smp_affinity
> 3ff
> erd9945:/proc/irq/418 # echo 00f > smp_affinity
> erd9945:/proc/irq/418 # cat smp_affinity
> 00f
> erd9945:/proc/irq/418 # cat /proc/interrupts
> CPU0 CPU1 CPU2 CPU3 CPU4 CPU5 CPU6 CPU7 CPU8 CPU9
> 418: 3893 201 181 188 0 0 0 0 0 0 gpg2 0 Edge 19100000.drmdecon
>
Suggest formatting the commit message as follows, to make it more readable:
8<-------------------------------------------------------------------------->8
To support affinity setting for non wake up external gpio interrupt,
add irq_set_affinity callback using irq number from pinctrl driver
data.
Before this patch, changing the irq affinity of gpio interrupt is not
possible:
# cat /proc/irq/418/smp_affinity
3ff
# echo 00f > /proc/irq/418/smp_affinity
# cat /proc/irq/418/smp_affinity
3ff
# cat /proc/interrupts
CPU0 CPU1 CPU2 CPU3 ...
418: 3631 0 0 0 ...
With this patch applied, it's possible to change irq affinity of gpio
interrupt:
# cat /proc/irq/418/smp_affinity
3ff
# echo 00f > /proc/irq/418/smp_affinity
# cat /proc/irq/418/smp_affinity
00f
# cat /proc/interrupts
CPU0 CPU1 CPU2 CPU3 ...
418: 3893 201 181 188 ...
8<-------------------------------------------------------------------------->8
> Signed-off-by: Youngmin Nam <youngmin.nam@xxxxxxxxxxx>
> ---
> drivers/pinctrl/samsung/pinctrl-exynos.c | 14 ++++++++++++++
> 1 file changed, 14 insertions(+)
>
> diff --git a/drivers/pinctrl/samsung/pinctrl-exynos.c b/drivers/pinctrl/samsung/pinctrl-exynos.c
> index 6b58ec84e34b..5d7b788282e9 100644
> --- a/drivers/pinctrl/samsung/pinctrl-exynos.c
> +++ b/drivers/pinctrl/samsung/pinctrl-exynos.c
> @@ -147,6 +147,19 @@ static int exynos_irq_set_type(struct irq_data *irqd, unsigned int type)
> return 0;
> }
>
> +static int exynos_irq_set_affinity(struct irq_data *irqd,
> + const struct cpumask *dest, bool force)
> +{
> + struct samsung_pin_bank *bank = irq_data_get_irq_chip_data(irqd);
> + struct samsung_pinctrl_drv_data *d = bank->drvdata;
> + struct irq_data *parent = irq_get_irq_data(d->irq);
I'm probably missing something, but: why not just use "irqd" parameter
and avoid declaring "bank" and "d"? Is "d->irq" somehow different from
"irqd"?
> +
> + if (parent)
> + return parent->chip->irq_set_affinity(parent, dest, force);
> +
Why not use irq_chip_set_affinity_parent() API?
> + return -EINVAL;
Maybe use something like this instead:
if (!irqd->parent_data)
return -EINVAL;
return irq_chip_set_affinity_parent(irqd, dest, force);
Can you please test if this code works?
> +}
> +
> static int exynos_irq_request_resources(struct irq_data *irqd)
> {
> struct samsung_pin_bank *bank = irq_data_get_irq_chip_data(irqd);
> @@ -212,6 +225,7 @@ static const struct exynos_irq_chip exynos_gpio_irq_chip __initconst = {
> .irq_mask = exynos_irq_mask,
> .irq_ack = exynos_irq_ack,
> .irq_set_type = exynos_irq_set_type,
> + .irq_set_affinity = exynos_irq_set_affinity,
What happens if we just assign irq_chip_set_affinity_parent() here?
Would it work, or Exynos case is more complicated than this?
> .irq_request_resources = exynos_irq_request_resources,
> .irq_release_resources = exynos_irq_release_resources,
> },
> --
> 2.39.2
>
