Re: [PATCH] cpuidle: psci: Align psci_power_state count with idle state count

From: Sudeep Holla <sudeep.holla@arm.com>
To: Ulf Hansson <ulf.hansson@linaro.org>
Cc: Lorenzo Pieralisi <Lorenzo.Pieralisi@arm.com>,
	Bjorn Andersson <bjorn.andersson@linaro.org>,
	Daniel Lezcano <daniel.lezcano@linaro.org>,
	Lina Iyer <ilina@codeaurora.org>,
	Kevin Hilman <khilman@kernel.org>,
	Linux ARM <linux-arm-kernel@lists.infradead.org>,
	linux-arm-msm <linux-arm-msm@vger.kernel.org>,
	Linux PM <linux-pm@vger.kernel.org>,
	Mark Rutland <mark.rutland@arm.com>,
	"Rafael J. Wysocki" <rjw@rjwysocki.net>,
	Rob Herring <robh+dt@kernel.org>, Stephen Boyd <sboyd@kernel.org>,
	Vincent Guittot <vincent.guittot@linaro.org>
Subject: Re: [PATCH] cpuidle: psci: Align psci_power_state count with idle state count
Date: Sun, 27 Oct 2019 02:20:56 +0000	[thread overview]
Message-ID: <20191027022056.GA18111@e107533-lin.cambridge.arm.com> (raw)
In-Reply-To: <CAPDyKFqOTpwZ_o0Z6hAFDXf9wJM82a_P8fcKkZY-ZfW-d9LFVQ@mail.gmail.com>

On Thu, Oct 24, 2019 at 06:10:09PM +0200, Ulf Hansson wrote:
> On Thu, 24 Oct 2019 at 17:18, Sudeep Holla <sudeep.holla@arm.com> wrote:
> >
> > Instead of allocating 'n-1' states in psci_power_state to manage 'n'
> > idle states which include "ARM WFI" state, it would be simpler to have
> > 1:1 mapping between psci_power_state and cpuidle driver states.
> >
> > ARM WFI state(i.e. idx == 0) is handled specially in the generic macro
> > CPU_PM_CPU_IDLE_ENTER_PARAM and hence state[-1] is not possible. However
> > for sake of code readability, it is better to have 1:1 mapping and not
> > use [idx - 1] to access psci_power_state corresponding to driver cpuidle
> > state for idx.
> >
> > psci_power_state[0] is default initialised to 0 and is never accessed
> > while entering WFI state.
> >
> > Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
>
> Reported-by: Ulf Hansson <ulf.hansson@linaro.org>
> Reviewed-by: Ulf Hansson <ulf.hansson@linaro.org>
>
> > ---
> >  drivers/cpuidle/cpuidle-psci.c | 8 +++++---
> >  1 file changed, 5 insertions(+), 3 deletions(-)
> >
> > Hi Ulf, Lorenzo,
> >
> > Just to avoid confusion, I thought I will just write this patch as I was
> > about to make reference to this in my review.
>
> As discussed with Lorenzo, I said I was going to adopt his review
> comments, which means I already have a patch for this locally.
>
> Nevermind this time, but I would appreciate if this kind of
> bikeshedding can been avoided future wise.
>

That's one of the reason I just wrote the patch as I felt describing it
it words was difficult compared to patch :). Sorry if you felt this was
bikeshedding.

--
Regards,
Sudeep