Re: [PATCH v5 01/12] scsi: ufs: dwc: Return a negative error if the link is down

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] scsi: ufs: dwc: ufshcd_dwc_link_is_up() propagates positive error values
--

commit bbaf6092d07a94b81e19b83041bbd45bc5f8771f
Author: Larisa Grigore <larisa.grigore@oss.nxp.com>

scsi: ufs: dwc: Return a negative error if the link is down

This commit updates ufshcd_dwc_link_is_up() to return -ENOLINK when the link
is down, rather than returning 1. It also propagates any errors returned by
ufshcd_dme_get() to the caller.

> diff --git a/drivers/ufs/host/ufshcd-dwc.c b/drivers/ufs/host/ufshcd-dwc.c
> index 21b1cf912dcc6..ccf785fce6cc6 100644
> --- a/drivers/ufs/host/ufshcd-dwc.c
> +++ b/drivers/ufs/host/ufshcd-dwc.c
> @@ -48,20 +48,23 @@ static void ufshcd_dwc_program_clk_div(struct ufs_hba *hba, u32 divider_val)
>   * ufshcd_dwc_link_is_up() - check if link is up.
>   * @hba: private structure pointer
>   *
> - * Return: 0 on success, non-zero value on failure.
> + * Return: 0 on success, a negative errno value on failure.
>   */
>  static int ufshcd_dwc_link_is_up(struct ufs_hba *hba)
>  {
>  	int dme_result = 0;
> +	int ret;
>  
> -	ufshcd_dme_get(hba, UIC_ARG_MIB(VS_POWERSTATE), &dme_result);
> +	ret = ufshcd_dme_get(hba, UIC_ARG_MIB(VS_POWERSTATE), &dme_result);
> +	if (ret)
> +		return ret;

[Severity: Low]
Are we propagating a positive error code here?

Looking at ufshcd_wait_for_uic_cmd() in drivers/ufs/core/ufshcd.c, when a DME
hardware command fails, it extracts the positive error code from the UIC
registers:

	ret = uic_cmd->argument2 & MASK_UIC_COMMAND_RESULT;

Since this value propagates up through ufshcd_dme_get(), returning it directly
means ufshcd_dwc_link_is_up() can still return positive error codes. Does this
violate the updated API contract to return a negative errno value on failure?

While upstream callers evaluate the result as a truthy value (if (err)) and the
probe safely fails without crashing, returning positive error codes violates
standard Linux driver conventions.

>  
>  	if (dme_result == UFSHCD_LINK_IS_UP) {
>  		ufshcd_set_link_active(hba);
>  		return 0;
>  	}
>  
> -	return 1;
> +	return -ENOLINK;
>  }
>  
>  /**

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260922073009.1449688-1-larisa.grigore@oss.nxp.com?part=1