[RFC PATCH] mfd: ls2kbmc: Fix stop_machine() function's retvals

[RFC PATCH] mfd: ls2kbmc: Fix stop_machine() function's retvals

[Borislav Petkov (AMD)]

Hi,

see below.

It is totally untested because I don't have the hardware but the issue should
be clear.

Maybe the retvals should be tweaked, to denote even better what fails in
the function when it happens.

Thx.

---
stop_machine() takes a function to run @fn of type cpu_stop_fn_t as its
first parameter, which returns an int. That int is either 0 on a success
or non-zero on failure.

The ls2k_bmc_recover_pci_data() function returns "false" instead to
denote a failure, presumably, which gets promoted to a 0.

Add proper negative return values to denote the different failures
encountered.

Signed-off-by: Borislav Petkov (AMD) <bp@alien8.de>
---
 drivers/mfd/ls2k-bmc-core.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/mfd/ls2k-bmc-core.c b/drivers/mfd/ls2k-bmc-core.c
index 408056bfb2fe..306e444a592a 100644
--- a/drivers/mfd/ls2k-bmc-core.c
+++ b/drivers/mfd/ls2k-bmc-core.c
@@ -268,18 +268,18 @@ static int ls2k_bmc_recover_pci_data(void *data)
 	}
 
 	if (i == 0)
-		return false;
+		return -ETIMEDOUT;
 
 	ls2k_bmc_restore_bridge_pci_data(parent, ddata);
 
 	/* Check if PCI-E is connected */
 	if (!ls2k_bmc_pcie_is_connected(parent, ddata))
-		return false;
+		return -EINVAL;
 
 	/* Waiting for U-Boot and DDR ready */
 	mdelay(LS2K_BMC_RESET_WAIT_TIME);
 	if (!ls2k_bmc_bar0_addr_is_set(parent))
-		return false;
+		return -EINVAL;
 
 	/* Restore LS2K BMC PCI-E config data */
 	pci_write_config_dword(pdev, PCI_COMMAND, ddata->bmc_pci_data.pci_command);
-- 
2.53.0

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

Re: [RFC PATCH] mfd: ls2kbmc: Fix stop_machine() function's retvals

[Lee Jones]

On Wed, 07 Oct 2026, Borislav Petkov (AMD) wrote:

Please review, explain or fix the Sashiko review(s).

> Hi,
>
> see below.
>
> It is totally untested because I don't have the hardware but the issue should
> be clear.
>
> Maybe the retvals should be tweaked, to denote even better what fails in
> the function when it happens.
>
> Thx.
>
> ---

If you want to provide additional context, please add it below the ---
separator below the tags section.

> stop_machine() takes a function to run @fn of type cpu_stop_fn_t as its
> first parameter, which returns an int. That int is either 0 on a success
> or non-zero on failure.
>
> The ls2k_bmc_recover_pci_data() function returns "false" instead to
> denote a failure, presumably, which gets promoted to a 0.
>
> Add proper negative return values to denote the different failures
> encountered.
>
> Signed-off-by: Borislav Petkov (AMD) <bp@alien8.de>
> ---
>  drivers/mfd/ls2k-bmc-core.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/mfd/ls2k-bmc-core.c b/drivers/mfd/ls2k-bmc-core.c
> index 408056bfb2fe..306e444a592a 100644
> --- a/drivers/mfd/ls2k-bmc-core.c
> +++ b/drivers/mfd/ls2k-bmc-core.c
> @@ -268,18 +268,18 @@ static int ls2k_bmc_recover_pci_data(void *data)
>  	}
>
>  	if (i == 0)
> -		return false;
> +		return -ETIMEDOUT;
>
>  	ls2k_bmc_restore_bridge_pci_data(parent, ddata);
>
>  	/* Check if PCI-E is connected */
>  	if (!ls2k_bmc_pcie_is_connected(parent, ddata))
> -		return false;
> +		return -EINVAL;

Is this an invalid argument, or did the link fail to come up ('-ETIMEDOUT' or
'-ENOLINK')?

>
>  	/* Waiting for U-Boot and DDR ready */
>  	mdelay(LS2K_BMC_RESET_WAIT_TIME);
>  	if (!ls2k_bmc_bar0_addr_is_set(parent))
> -		return false;
> +		return -EINVAL;

Same here.

Wouldn't '-ETIMEDOUT' or '-EIO' be more fitting after waiting for the
hardware to become ready?

>
>  	/* Restore LS2K BMC PCI-E config data */
>  	pci_write_config_dword(pdev, PCI_COMMAND, ddata->bmc_pci_data.pci_command);
> -- 
> 2.53.0
>
> -- 
> Regards/Gruss,
>     Boris.
>
> https://people.kernel.org/tglx/notes-about-netiquette

-- 
Lee Jones