mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Lee Jones <lee@kernel.org>
To: Borislav Petkov <bp@alien8.de>
Cc: Binbin Zhou <zhoubinbin@loongson.cn>,
	Chong Qiao <qiaochong@loongson.cn>,
	mfd@lists.linux.dev, LKML <linux-kernel@vger.kernel.org>
Subject: Re: [RFC PATCH] mfd: ls2kbmc: Fix stop_machine() function's retvals
Date: Thu, 15 Oct 2026 16:57:08 +0100	[thread overview]
Message-ID: <179198419934.839288.12713994676834542149@kernel.org> (raw)
In-Reply-To: <20261007235635.GAasbcM53RNcTVfnu3@fat_crate.local>

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

      reply	other threads:[~2026-10-15 15:57 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-07 23:56 Borislav Petkov (AMD)
2026-10-15 15:57 ` Lee Jones [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=179198419934.839288.12713994676834542149@kernel.org \
    --to=lee@kernel.org \
    --cc=bp@alien8.de \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mfd@lists.linux.dev \
    --cc=qiaochong@loongson.cn \
    --cc=zhoubinbin@loongson.cn \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®