Re: [RESEND PATCH v2] firmware/psci: Print a warning if PSCI doesn't accept PC mode

From: Ulf Hansson <ulf.hansson@linaro.org>
To: Johan Hovold <johan@kernel.org>
Cc: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>,
	Sudeep Holla <sudeep.holla@arm.com>,
	 Arnd Bergmann <arnd@arndb.de>,
	Mark Rutland <mark.rutland@arm.com>,
	 Lorenzo Pieralisi <lpieralisi@kernel.org>,
	"Rafael J. Wysocki" <rafael@kernel.org>,
	 Daniel Lezcano <daniel.lezcano@linaro.org>,
	linux-arm-kernel@lists.infradead.org,  soc@kernel.org,
	linux-pm@vger.kernel.org
Subject: Re: [RESEND PATCH v2] firmware/psci: Print a warning if PSCI doesn't accept PC mode
Date: Thu, 20 Oct 2022 11:21:14 +0200	[thread overview]
Message-ID: <CAPDyKFpvyrp3Lyyou8TT-KcC-_Q9NLs3wpb6qrDbz7=i0e_vhg@mail.gmail.com> (raw)
In-Reply-To: <Y1DxfjExuzfKLWDs@hovoldconsulting.com>

On Thu, 20 Oct 2022 at 08:58, Johan Hovold <johan@kernel.org> wrote:
>
> On Wed, Oct 19, 2022 at 06:02:37PM +0300, Dmitry Baryshkov wrote:
> > On Wed, 19 Oct 2022 at 17:37, Ulf Hansson <ulf.hansson@linaro.org> wrote:
> > > On Wed, 19 Oct 2022 at 16:14, Johan Hovold <johan@kernel.org> wrote:
> > > > On Mon, Sep 26, 2022 at 02:02:49PM +0300, Dmitry Baryshkov wrote:
> > > > > The function psci_pd_try_set_osi_mode() will print an error if enabling
> > > > > OSI mode fails. To ease debugging PSCI issues print corresponding
> > > > > message if switching to PC mode fails too.
> > > > >
> > > > > Acked-by: Mark Rutland <mark.rutland@arm.com>
> > > > > Reviewed-by: Sudeep Holla <sudeep.holla@arm.com>
> > > > > Reviewed-by: Ulf Hansson <ulf.hansson@linaro.org>
> > > > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
>
> > > > > diff --git a/drivers/firmware/psci/psci.c b/drivers/firmware/psci/psci.c
> > > > > index 9fdcb6bff403..42cae0ba10e2 100644
> > > > > --- a/drivers/firmware/psci/psci.c
> > > > > +++ b/drivers/firmware/psci/psci.c
> > > > > @@ -164,6 +164,8 @@ int psci_set_osi_mode(bool enable)
> > > > >                       PSCI_1_0_SUSPEND_MODE_PC;
> > > > >
> > > > >       err = invoke_psci_fn(PSCI_1_0_FN_SET_SUSPEND_MODE, suspend_mode, 0, 0);
> > > > > +     if (err < 0)
> > > > > +             pr_warn("failed to set %s mode: %d\n", enable ? "OSI" : "PC", err);
> > > > >       return psci_to_linux_errno(err);
> > > > >  }
> > > >
> > > > When booting 6.1-rc1 I now see:
> > > >
> > > >         [    0.000000] psci: OSI mode supported.
> > > >         [    0.000000] psci: failed to set PC mode: -3
> > > >
> > > > on every boot with sc8280xp, while later enabling OSI mode still works:
> > > >
> > > >         [    0.227358] CPUidle PSCI: psci_pd_try_set_osi_mode - success
> > > >         [    0.227599] CPUidle PSCI: Initialized CPU PM domain topology
> > > >
> > > > Judging from Sudeep's comment on v1:
> > > >
> > > >         The platform must boot in PC mode, so even if it fails we ignore
> > > >         so not sure if the logging is of much help here IMO.
> > > >
> > > > perhaps logging this as an error is not a good idea?
> >
> > As far as I understand, some of the Qualcomm platforms booted in the
> > OSI mode, no matter what the spec says. I'm not sure about the
> > sc8280xp, but it definitely was the case previously. This was the
> > primary reason for adding a log here.
> >
> > > I think it's useful to get information that the FW doesn't fully
> > > conform to the spec (it should support PC mode), but that doesn't
> > > really mean that it can't work.
> > >
> > > To me, it sounds like we should move the logging to the debug level!?
> >
> > I'd say, either ignore the error, or move the warning to the info or
> > debug level.
>
> "Ignore the error" as in removing the warning or as in telling every
> owner of an X13s Thinkpad to ignore it?
>
> If there's nothing that can be done about it and this is nothing to
> worry about, then moving to debug level would seem like the right thing
> to do here. At least for the PC case.

Yes.

For the OSI case, I think a better option is to extend the print of
"Initialized CPU PM domain topology\n", in psci_cpuidle_domain_probe()
with information about whether OSI is being used or not.

Kind regards
Uffe