Re: [PATCH 2/2] cpufreq: CPPC: Fix error handling in cppc_scale_freq_workfn()

[Beata Michalska <beata.michalska@arm.com>]

On Wed, Aug 13, 2025 at 03:15:12PM +0800, Jie Zhan wrote:
> 
> 
> On 05/08/2025 12:58, Prashant Malani wrote:
> > On Mon, 4 Aug 2025 at 18:12, Prashant Malani <pmalani@google.com> wrote:
> >>
> >> On Sun, 3 Aug 2025 at 23:21, Jie Zhan <zhanjie9@hisilicon.com> wrote
> >>> On 01/08/2025 16:58, Prashant Malani wrote:
> >>>> This begs the question: why is this work function being scheduled
> >>>> for CPUs that are in reset or offline/powered-down at all?
> >>>> IANAE but it sounds like it would be better to add logic to ensure this
> >>>> work function doesn't get scheduled/executed for CPUs that
> >>>> are truly offline/powered-down or in reset.
> >>> Yeah good question.  We may discuss that on your thread.
> >>
> >> OK.
> >> Quickly looking around, it sounds having in the CPPC tick function [1]
> >> might be a better option (one probably doesn't want to lift it beyond the
> >> CPPC layer, since other drivers might have different behaviour).
> >> One can add a cpu_online/cpu_enabled check there.
> > 
> > Fixed link:
> > [1] https://elixir.bootlin.com/linux/v6.13/source/drivers/cpufreq/cppc_cpufreq.c#L125
> I don't think a cpu_online/cpu_enabled check there would help.
> 
> Offlined CPUs don't make cppc_scale_freq_workfn() fail because they won't
> have FIE triggered.  It fails from accessing perf counters on powered-down
> CPUs.
> 
> Perhaps the CPPC FIE needs a bit rework.  AFAICS, FIE is meant to run in
> ticks, but currently the CPPC FIE eventually runs in a thread due to the
> possible PCC path when reading CPC regs I guess.
Just for my benefit: the tick is being fired on a given CPU which is when an
irq_work is being queued. Then before this goes through the kworker and finally
ends up in 'cppc_scale_freq_workfn' that CPU is entering a deeper idle state ?
Could the cppc driver register for pm notifications and cancel any pending work
for a CPU that is actually going down, directly or by setting some flag or smth
so that the final worker function is either not triggered or knows it has to
bail out early ?
(Note this is a rough idea and needs verification)

---
BR
Beata