Re: [PATCH 2/2] irqchip: clocksource: fix jcore-pit irq request

On Sun, Feb 16, 2025 at 6:56 PM Artur Rojek <[email protected]> wrote:
>
> The jcore-aic irqchip does not have separate interrupt numbers reserved
> for cpu-local vs global interrupts. Instead, the task of selecting this
> property is being delegated to the device drivers requesting the given
> irq.
>
> This quirk has not been taken into account while migrating jcore-pit to
> request_percpu_irq(), resulting in a failure to register PIT interrupts.
>
> Fix this behavior by making the following changes:
> 1) Explicitly register irq_set_percpu_devid() in jcore-pit.
> 2) Provide enable_percpu_irq()/disable_percpu_irq() calls in jcore-pit.
> 3) Make jcore-aic pass the correct per-cpu cookie to the irq handler by
>    using handle_percpu_devid_irq() instead of handle_percpu_irq().
>
> Fixes: 69a9dcbd2d65 ("clocksource/drivers/jcore: Use request_percpu_irq()")
>
> Signed-off-by: Artur Rojek <[email protected]>

I can confirm that this change compiles OK with strict percpu address
space checks on x86 [1].

Acked-by: Uros Bizjak <[email protected]>

[1] https://lore.kernel.org/lkml/[email protected]/

Uros.

> ---
>  drivers/clocksource/jcore-pit.c | 15 ++++++++++++++-
>  drivers/irqchip/irq-jcore-aic.c |  2 +-
>  2 files changed, 15 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/clocksource/jcore-pit.c b/drivers/clocksource/jcore-pit.c
> index a3fe98cd3838..82815428f8f9 100644
> --- a/drivers/clocksource/jcore-pit.c
> +++ b/drivers/clocksource/jcore-pit.c
> @@ -114,6 +114,18 @@ static int jcore_pit_local_init(unsigned cpu)
>         pit->periodic_delta = DIV_ROUND_CLOSEST(NSEC_PER_SEC, HZ * buspd);
>
>         clockevents_config_and_register(&pit->ced, freq, 1, ULONG_MAX);
> +       enable_percpu_irq(pit->ced.irq, IRQ_TYPE_NONE);
> +
> +       return 0;
> +}
> +
> +static int jcore_pit_local_teardown(unsigned cpu)
> +{
> +       struct jcore_pit *pit = this_cpu_ptr(jcore_pit_percpu);
> +
> +       pr_info("Local J-Core PIT teardown on cpu %u\n", cpu);
> +
> +       disable_percpu_irq(pit->ced.irq);
>
>         return 0;
>  }
> @@ -168,6 +180,7 @@ static int __init jcore_pit_init(struct device_node *node)
>                 return -ENOMEM;
>         }
>
> +       irq_set_percpu_devid(pit_irq);
>         err = request_percpu_irq(pit_irq, jcore_timer_interrupt,
>                                  "jcore_pit", jcore_pit_percpu);
>         if (err) {
> @@ -237,7 +250,7 @@ static int __init jcore_pit_init(struct device_node *node)
>
>         cpuhp_setup_state(CPUHP_AP_JCORE_TIMER_STARTING,
>                           "clockevents/jcore:starting",
> -                         jcore_pit_local_init, NULL);
> +                         jcore_pit_local_init, jcore_pit_local_teardown);
>
>         return 0;
>  }
> diff --git a/drivers/irqchip/irq-jcore-aic.c b/drivers/irqchip/irq-jcore-aic.c
> index b9dcc8e78c75..1f613eb7b7f0 100644
> --- a/drivers/irqchip/irq-jcore-aic.c
> +++ b/drivers/irqchip/irq-jcore-aic.c
> @@ -38,7 +38,7 @@ static struct irq_chip jcore_aic;
>  static void handle_jcore_irq(struct irq_desc *desc)
>  {
>         if (irqd_is_per_cpu(irq_desc_get_irq_data(desc)))
> -               handle_percpu_irq(desc);
> +               handle_percpu_devid_irq(desc);
>         else
>                 handle_simple_irq(desc);
>  }
> --
> 2.48.1
>