Re: [PATCH] firmware/psci: demote suspend-mode warning to debug level

[Mark Rutland]

On Tue, Oct 25, 2022 at 04:32:52PM +0200, Johan Hovold wrote:
> On Tue, Oct 25, 2022 at 02:26:55PM +0100, Sudeep Holla wrote:
> > On Tue, Oct 25, 2022 at 02:56:00PM +0200, Johan Hovold wrote:
> > > On Tue, Oct 25, 2022 at 12:53:55PM +0100, Sudeep Holla wrote:
> > > > On Mon, Oct 24, 2022 at 04:34:17PM +0200, Johan Hovold wrote:
> > > > > On some Qualcomm platform, like SC8280XP, the attempt to set PC mode
> > > > > during boot fails with PSCI_RET_DENIED and since commit 998fcd001feb
> > > > > ("firmware/psci: Print a warning if PSCI doesn't accept PC mode") this
> > > > > is now logged at warning level:
> > > > >
> > > > > 	psci: failed to set PC mode: -3
> > > > >
> > > > > As there is nothing users can do about the firmware behaving this way,
> > > > > demote the warning to debug level.
> > > > >
> > > >
> > > > As mentioned in the other thread I prefer to keep this as error as we
> > > > shouldn't mask this error and enable more/newer platforms to ignore it
> > > > when they can go and fix it. So I don't agree with this.
> > >
> > > But now every owner of an X13s laptop will see this not very informative
> > > error at every boot and wonder what it means. Has something gone broken?
> > > Should they be worried? Can something be done about it?
> > >
> > 
> > I understand that but I have expressed why I am concerned on generalising
> > it. As long as we inform the concerned owners running Linux(which is quite
> > small at the moment), keeping it will help to get these fixed on platforms
> > that are running Linux today for validation and get it fixed if their
> > platform firmware suffers from the same.
> 
> Trying to inform every user that a warning during boot is actually
> benign and nothing to worry about generally seems backwards to me and is
> not something that is likely to scale.

It's not *entirely* beningn; we still have to bodge around this not being to
spec, and there are things that won't work (e.g. if we kexec to a kernel that
expects the FW to actually follow the spec it claims to).

I agree with Sudeep that we should log something here, but I do appreciate that
argumetn that for 90% of users this is not interesting.

I would suggest we make this:

	pr_info(FW_BUG "failed to set PC mode: %d\n", ...);

... so that it's always logged for those who care, but as it's INFO rather than
WARNING, it'll easily be filtered out of logs (e.g. for users booting with
"quiet"). Adding FW_BUG will also make this clear we're complaining about a FW
bug rather than this being a kernel-internal error.

Thanks,
Mark.