| Message ID | 20231113153446.82684-1-sudeep.holla@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | firmware: psci: Fix return value from psci_system_suspend() | expand |
Hi Mark/Lorenzo, On Mon, Nov 13, 2023 at 03:34:46PM +0000, Sudeep Holla wrote: > Currently we return the value from invoke_psci_fn() directly as return > value from psci_system_suspend(). It is wrong to send the PSCI interface > return value directly. psci_to_linux_errno() provide the mapping from > PSCI return value to the one that can be returned to the callers within > the kernel. > > Use psci_to_linux_errno() to convert and return the correct value from > psci_system_suspend(). > > Fixes: faf7ec4a92c0 ("drivers: firmware: psci: add system suspend support") > Signed-off-by: Sudeep Holla <sudeep.holla@arm.com> > --- > drivers/firmware/psci/psci.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > Hi, > > For some reason, this has gone unnoticed for years. I bumped into this when > I was trying to test suspend on FVP which claims to support but returns error > when called. The error was getting not communicated properly(incorrect > error code) before this patch. > Gentle ping! I had forgotten about this, if you are happy with the change, I can ask Arnd to pick it up as fix during v6.8 cycle.
On Mon, Nov 13, 2023 at 03:34:46PM +0000, Sudeep Holla wrote: > Currently we return the value from invoke_psci_fn() directly as return > value from psci_system_suspend(). It is wrong to send the PSCI interface > return value directly. psci_to_linux_errno() provide the mapping from > PSCI return value to the one that can be returned to the callers within > the kernel. > > Use psci_to_linux_errno() to convert and return the correct value from > psci_system_suspend(). > > Fixes: faf7ec4a92c0 ("drivers: firmware: psci: add system suspend support") > Signed-off-by: Sudeep Holla <sudeep.holla@arm.com> Sorry for the delay; this looks good to me: Acked-by: Mark Rutland <mark.rutland@arm.com> Arnd, are you happy to pick this up for v6.8? Mark. > --- > drivers/firmware/psci/psci.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > Hi, > > For some reason, this has gone unnoticed for years. I bumped into this when > I was trying to test suspend on FVP which claims to support but returns error > when called. The error was getting not communicated properly(incorrect > error code) before this patch. > > Regards, > Sudeep > > diff --git a/drivers/firmware/psci/psci.c b/drivers/firmware/psci/psci.c > index d9629ff87861..2328ca58bba6 100644 > --- a/drivers/firmware/psci/psci.c > +++ b/drivers/firmware/psci/psci.c > @@ -497,10 +497,12 @@ int psci_cpu_suspend_enter(u32 state) > > static int psci_system_suspend(unsigned long unused) > { > + int err; > phys_addr_t pa_cpu_resume = __pa_symbol(cpu_resume); > > - return invoke_psci_fn(PSCI_FN_NATIVE(1_0, SYSTEM_SUSPEND), > + err = invoke_psci_fn(PSCI_FN_NATIVE(1_0, SYSTEM_SUSPEND), > pa_cpu_resume, 0, 0); > + return psci_to_linux_errno(err); > } > > static int psci_system_suspend_enter(suspend_state_t state) > -- > 2.42.0 > > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel >
On Mon, 13 Nov 2023 15:34:46 +0000, Sudeep Holla wrote: > Currently we return the value from invoke_psci_fn() directly as return > value from psci_system_suspend(). It is wrong to send the PSCI interface > return value directly. psci_to_linux_errno() provide the mapping from > PSCI return value to the one that can be returned to the callers within > the kernel. > > Use psci_to_linux_errno() to convert and return the correct value from > psci_system_suspend(). > > [...] This has slipped through the cracks for long. I plan to take this via my FF-A tree as SoC team somehow missed couple of times though I cc-ed their email. Applied to sudeep.holla/linux (for-next/ffa/updates), thanks! [1/1] firmware: psci: Fix return value from psci_system_suspend() https://git.kernel.org/sudeep.holla/c/f624c7cbd155ef -- Regards, Sudeep
On Mon, Jun 17, 2024 at 11:12:55AM +0100, Sudeep Holla wrote: > On Mon, 13 Nov 2023 15:34:46 +0000, Sudeep Holla wrote: > > Currently we return the value from invoke_psci_fn() directly as return > > value from psci_system_suspend(). It is wrong to send the PSCI interface > > return value directly. psci_to_linux_errno() provide the mapping from > > PSCI return value to the one that can be returned to the callers within > > the kernel. > > > > Use psci_to_linux_errno() to convert and return the correct value from > > psci_system_suspend(). > > > > [...] > > This has slipped through the cracks for long. I plan to take this > via my FF-A tree as SoC team somehow missed couple of times though I > cc-ed their email. That works for me; thanks for picking this up! Mark. > > Applied to sudeep.holla/linux (for-next/ffa/updates), thanks! > > [1/1] firmware: psci: Fix return value from psci_system_suspend() > https://git.kernel.org/sudeep.holla/c/f624c7cbd155ef > -- > Regards, > Sudeep >
diff --git a/drivers/firmware/psci/psci.c b/drivers/firmware/psci/psci.c index d9629ff87861..2328ca58bba6 100644 --- a/drivers/firmware/psci/psci.c +++ b/drivers/firmware/psci/psci.c @@ -497,10 +497,12 @@ int psci_cpu_suspend_enter(u32 state) static int psci_system_suspend(unsigned long unused) { + int err; phys_addr_t pa_cpu_resume = __pa_symbol(cpu_resume); - return invoke_psci_fn(PSCI_FN_NATIVE(1_0, SYSTEM_SUSPEND), + err = invoke_psci_fn(PSCI_FN_NATIVE(1_0, SYSTEM_SUSPEND), pa_cpu_resume, 0, 0); + return psci_to_linux_errno(err); } static int psci_system_suspend_enter(suspend_state_t state)
Currently we return the value from invoke_psci_fn() directly as return value from psci_system_suspend(). It is wrong to send the PSCI interface return value directly. psci_to_linux_errno() provide the mapping from PSCI return value to the one that can be returned to the callers within the kernel. Use psci_to_linux_errno() to convert and return the correct value from psci_system_suspend(). Fixes: faf7ec4a92c0 ("drivers: firmware: psci: add system suspend support") Signed-off-by: Sudeep Holla <sudeep.holla@arm.com> --- drivers/firmware/psci/psci.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) Hi, For some reason, this has gone unnoticed for years. I bumped into this when I was trying to test suspend on FVP which claims to support but returns error when called. The error was getting not communicated properly(incorrect error code) before this patch. Regards, Sudeep -- 2.42.0