[PATCH] firmware: psci: Fix return value from psci_system_suspend()

[PATCH] firmware: psci: Fix return value from psci_system_suspend()

[Sudeep Holla]

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

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

Re: [PATCH] firmware: psci: Fix return value from psci_system_suspend()

[Sudeep Holla]

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.

-- 
Regards,
Sudeep

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] firmware: psci: Fix return value from psci_system_suspend()

[Mark Rutland]

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
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] firmware: psci: Fix return value from psci_system_suspend()

[Sudeep Holla]

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

Re: [PATCH] firmware: psci: Fix return value from psci_system_suspend()

[Mark Rutland]

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
>