Hi André,
Thanks for the review feedback!
On Thu, 3 Jul 2025 at 12:01, André Draszik <andre.draszik@xxxxxxxxxx> wrote:
>
> More small comments. Sorry for the drip feed, just trying to understand
> things...
>
> On Fri, 2025-06-27 at 14:08 +0100, Peter Griffin wrote:
> > [...]
> >
> > +static int gs101_cpu_pm_notify_callback(struct notifier_block *self,
> > + unsigned long action, void *v)
> > +{
> > + switch (action) {
> > + case CPU_PM_ENTER:
> > + /*
> > + * Ignore CPU_PM_ENTER event in reboot or
> > + * suspend sequence.
> > + */
> > +
> > + if (atomic_read(&pmu_context->sys_suspended) ||
> > + atomic_read(&pmu_context->sys_rebooting))
> > + return NOTIFY_OK;
> > +
> > + return gs101_cpu_pmu_offline();
> > +
> > + break;
>
> break is not needed after return, and generally there should be an empty
> line before the next case statement.
Will fix
>
> > + case CPU_PM_EXIT:
>
> Should this also handle CPU_PM_ENTER_FAILED in the same way to bring
> the CPU back up in case of failures?
I choose not to do that, mainly because the downstream production
drivers don't handle CPU_PM_ENTER_FAILED, and without access to the
firmware code it is hard to reason about.
Logically it seems like we would want to do the same code as
CPU_PM_EXIT with a CPU_PM_ENTER_FAILED, but I've never seen
CPU_PM_FAILED so far in my debugging.
>
> > +
> > + if (atomic_read(&pmu_context->sys_rebooting))
> > + return NOTIFY_OK;
> > +
> > + return gs101_cpu_pmu_online();
> > +
> > + break;
>
> No break needed.
Will fix
Thanks.
Peter
