[PATCH v3 0/2] processing of asymmetric connected lanes

[PATCH v3 0/2] processing of asymmetric connected lanes

[SEO HOYOUNG]

Performance problems may occur if there is a problem with the
asymmetric connected lane such as h/w failure.
Currently, only check connected lane for rx/tx is checked if it is not 0.
But it should also be checked if it is asymmetrically connected.
So if it is an asymmetric connected lane, an error occurs.

v1 -> v2: add error routine.
ufs initialization error occurs in case of asymmetic connected

v2 -> v3: split two patches

SEO HOYOUNG (2):
  scsi: ufs: core: check asymmetric connected lanes
  scsi: ufs: core: reflect function execution result in return

 drivers/ufs/core/ufshcd.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

-- 
2.26.0

[PATCH v3 1/2] scsi: ufs: core: check asymmetric connected lanes

[SEO HOYOUNG]

Performance problems may occur if there is a problem with the
asymmetric connected lane such as h/w failure.
Currently, only check connected lane for rx/tx is checked if it is not 0.
But it should also be checked if it is asymmetrically connected.

Signed-off-by: SEO HOYOUNG <hy50.seo@samsung.com>
---
 drivers/ufs/core/ufshcd.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
index 24a32e2fd75e..387eec6f19ef 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -4540,6 +4540,14 @@ static int ufshcd_get_max_pwr_mode(struct ufs_hba *hba)
 		return -EINVAL;
 	}
 
+	if (pwr_info->lane_rx != pwr_info->lane_tx) {
+		dev_err(hba->dev, "%s: asymmetric connected lanes. rx=%d, tx=%d\n",
+			__func__,
+				pwr_info->lane_rx,
+				pwr_info->lane_tx);
+		return -EINVAL;
+	}
+
 	/*
 	 * First, get the maximum gears of HS speed.
 	 * If a zero value, it means there is no HSGEAR capability.
-- 
2.26.0

[PATCH v3 2/2] scsi: ufs: core: reflect function execution result in return

[SEO HOYOUNG]

If an error is returned in the power mode function, it is returned and
modified to cause failure in the UFS linkup.
If it is an asymmetric connected lane, the UFS init can fail because it is
an incorrect situation.

Signed-off-by: SEO HOYOUNG <hy50.seo@samsung.com>
---
 drivers/ufs/core/ufshcd.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
index 387eec6f19ef..1381eb7d506a 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -8587,7 +8587,8 @@ static int ufshcd_device_params_init(struct ufs_hba *hba)
 		hba->dev_info.f_power_on_wp_en = flag;
 
 	/* Probe maximum power mode co-supported by both UFS host and device */
-	if (ufshcd_get_max_pwr_mode(hba))
+	ret = ufshcd_get_max_pwr_mode(hba);
+	if (ret)
 		dev_err(hba->dev,
 			"%s: Failed getting max supported power mode\n",
 			__func__);
-- 
2.26.0

Re: [PATCH v3 2/2] scsi: ufs: core: reflect function execution result in return

[Bart Van Assche]

On 10/10/24 12:52 AM, SEO HOYOUNG wrote:
> If an error is returned in the power mode function, it is returned and
> modified to cause failure in the UFS linkup.
> If it is an asymmetric connected lane, the UFS init can fail because it is
> an incorrect situation.

Why is this an incorrect situation? If ufshcd_get_max_pwr_mode() fails,
won't communication succeed with a lower power mode than maximum power?

Thanks,

Bart.

Re: [PATCH v3 1/2] scsi: ufs: core: check asymmetric connected lanes

[Bart Van Assche]

On 10/10/24 12:52 AM, SEO HOYOUNG wrote:
> Performance problems may occur if there is a problem with the
> asymmetric connected lane such as h/w failure.
> Currently, only check connected lane for rx/tx is checked if it is not 0.
> But it should also be checked if it is asymmetrically connected.
> 
> Signed-off-by: SEO HOYOUNG <hy50.seo@samsung.com>
> ---
>   drivers/ufs/core/ufshcd.c | 8 ++++++++
>   1 file changed, 8 insertions(+)
> 
> diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
> index 24a32e2fd75e..387eec6f19ef 100644
> --- a/drivers/ufs/core/ufshcd.c
> +++ b/drivers/ufs/core/ufshcd.c
> @@ -4540,6 +4540,14 @@ static int ufshcd_get_max_pwr_mode(struct ufs_hba *hba)
>   		return -EINVAL;
>   	}
>   
> +	if (pwr_info->lane_rx != pwr_info->lane_tx) {
> +		dev_err(hba->dev, "%s: asymmetric connected lanes. rx=%d, tx=%d\n",
> +			__func__,
> +				pwr_info->lane_rx,
> +				pwr_info->lane_tx);
> +		return -EINVAL;
> +	}
> +
>   	/*
>   	 * First, get the maximum gears of HS speed.
>   	 * If a zero value, it means there is no HSGEAR capability.

It seems to me that symmetry of the number of lanes is required by the
UFS standard? From the UFS standard: "An equal number of downstream and
upstream lanes shall be provided in each link." Hence:

Reviewed-by: Bart Van Assche <bvanassche@acm.org>

Re: [PATCH v3 1/2] scsi: ufs: core: check asymmetric connected lanes

[Martin K. Petersen]

> Performance problems may occur if there is a problem with the
> asymmetric connected lane such as h/w failure. Currently, only check
> connected lane for rx/tx is checked if it is not 0. But it should also
> be checked if it is asymmetrically connected.

Applied to 6.13/scsi-staging, thanks!

-- 
Martin K. Petersen	Oracle Linux Engineering

Re: [PATCH v3 0/2] processing of asymmetric connected lanes

[Martin K. Petersen]

On Thu, 10 Oct 2024 16:52:27 +0900, SEO HOYOUNG wrote:

> Performance problems may occur if there is a problem with the
> asymmetric connected lane such as h/w failure.
> Currently, only check connected lane for rx/tx is checked if it is not 0.
> But it should also be checked if it is asymmetrically connected.
> So if it is an asymmetric connected lane, an error occurs.
> 
> v1 -> v2: add error routine.
> ufs initialization error occurs in case of asymmetic connected
> 
> [...]

Applied to 6.13/scsi-queue, thanks!

[1/2] scsi: ufs: core: check asymmetric connected lanes
      https://git.kernel.org/mkp/scsi/c/10c58d7eea44

-- 
Martin K. Petersen	Oracle Linux Engineering