linux-kernel - Re: [PATCH v3] soc: samsung: exynos-pmu: Enable CPU Idle for gs101

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CADrjBPrPcYZPFuRqwm0OkFU+CQtinFUtNXHzt+MQpEy=Q-Xnww@mail.gmail.com>
Date: Wed, 9 Jul 2025 11:04:29 +0100
From: Peter Griffin <peter.griffin@...aro.org>
To: André Draszik <andre.draszik@...aro.org>
Cc: Tudor Ambarus <tudor.ambarus@...aro.org>, Alim Akhtar <alim.akhtar@...sung.com>, 
	Krzysztof Kozlowski <krzk@...nel.org>, William Mcvicker <willmcvicker@...gle.com>, 
	Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>, linux-arm-kernel@...ts.infradead.org, 
	linux-samsung-soc@...r.kernel.org, linux-kernel@...r.kernel.org, 
	kernel-team@...roid.com
Subject: Re: [PATCH v3] soc: samsung: exynos-pmu: Enable CPU Idle for gs101

Hi André,

Thanks for the review feedback!

On Thu, 3 Jul 2025 at 12:01, André Draszik <andre.draszik@...aro.org> 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