1. Patchew
  2. linux
  3. [PATCH v1 0/2] Add post change sequence for link start notify
  4. View patch

[PATCH v1 1/2] ufs: core: Configure only active lanes during link

palash.kambar@oss.qualcomm.com posted 2 patches 3 weeks, 6 days ago
There is a newer version of this series
[PATCH v1 1/2] ufs: core: Configure only active lanes during link
Posted by palash.kambar@oss.qualcomm.com 3 weeks, 6 days ago 
From: Palash Kambar <palash.kambar@oss.qualcomm.com>

The number of active lanes detected during UFS link startup can be
fewer than the lanes specified in the device tree. The current driver
logic attempts to configure all lanes defined in the device tree,
regardless of their actual availability. This mismatch may cause
failures during power mode changes.

Hence, add check to identify only the lanes that were successfully
discovered during link startup, to warn on power mode change errors
caused by mismatched lane counts.

Signed-off-by: Palash Kambar <palash.kambar@oss.qualcomm.com>
---
 drivers/ufs/core/ufshcd.c | 39 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 39 insertions(+)

diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
index 31950fc51a4c..c956fab32932 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -5035,6 +5035,42 @@ void ufshcd_update_evt_hist(struct ufs_hba *hba, u32 id, u32 val)
 }
 EXPORT_SYMBOL_GPL(ufshcd_update_evt_hist);
 
+static int ufshcd_get_connected_tx_lanes(struct ufs_hba *hba, u32 *tx_lanes)
+{
+	return ufshcd_dme_get(hba,
+			      UIC_ARG_MIB(PA_CONNECTEDTXDATALANES), tx_lanes);
+}
+
+static int ufshcd_get_connected_rx_lanes(struct ufs_hba *hba, u32 *rx_lanes)
+{
+	return ufshcd_dme_get(hba,
+			      UIC_ARG_MIB(PA_CONNECTEDRXDATALANES), rx_lanes);
+}
+
+static void ufshcd_validate_link_params(struct ufs_hba *hba)
+{
+	int val = 0;
+
+	if (ufshcd_get_connected_tx_lanes(hba, &val))
+		return;
+
+	if (val != hba->lanes_per_direction) {
+		dev_err(hba->dev, "Tx lane mismatch [config,reported] [%d,%d]\n",
+			hba->lanes_per_direction, val);
+		return;
+	}
+
+	val = 0;
+
+	if (ufshcd_get_connected_rx_lanes(hba, &val))
+		return;
+
+	if (val != hba->lanes_per_direction) {
+		dev_err(hba->dev, "Rx lane mismatch [config,reported] [%d,%d]\n",
+			hba->lanes_per_direction, val);
+	}
+}
+
 /**
  * ufshcd_link_startup - Initialize unipro link startup
  * @hba: per adapter instance
@@ -5108,6 +5144,9 @@ static int ufshcd_link_startup(struct ufs_hba *hba)
 			goto out;
 	}
 
+	/* Check successfully detected lanes */
+	ufshcd_validate_link_params(hba);
+
 	/* Include any host controller configuration via UIC commands */
 	ret = ufshcd_vops_link_startup_notify(hba, POST_CHANGE);
 	if (ret)
-- 
2.34.1
Re: [PATCH v1 1/2] ufs: core: Configure only active lanes during link
Posted by Bart Van Assche 2 weeks, 1 day ago 
On 3/10/26 11:09 PM, palash.kambar@oss.qualcomm.com wrote:
 > [ ... ]
 > +static int ufshcd_get_connected_tx_lanes(struct ufs_hba *hba, u32 
*tx_lanes)
> +{
> +	return ufshcd_dme_get(hba,
> +			      UIC_ARG_MIB(PA_CONNECTEDTXDATALANES), tx_lanes);
> +}
> +
> +static int ufshcd_get_connected_rx_lanes(struct ufs_hba *hba, u32 *rx_lanes)
> +{
> +	return ufshcd_dme_get(hba,
> +			      UIC_ARG_MIB(PA_CONNECTEDRXDATALANES), rx_lanes);
> +}

The body of the above two functions is very short. Please remove these
functions and instead inline these function into their only caller.

> +static void ufshcd_validate_link_params(struct ufs_hba *hba)
> +{
> +	int val = 0;
> +
> +	if (ufshcd_get_connected_tx_lanes(hba, &val))
> +		return;

Shouldn't it be reported if ufshcd_get_connected_tx_lanes() fails?

> +	if (ufshcd_get_connected_rx_lanes(hba, &val))
> +		return;

Same question here - shouldn't it be reported if
ufshcd_get_connected_rx_lanes() fails?

Why does this patch only call dev_err() in case of a mismatch instead of 
adjusting hba->lanes_per_direction or making initialization fail?

Thanks,

Bart.
Re: [PATCH v1 1/2] ufs: core: Configure only active lanes during link
Posted by Palash Kambar 1 week, 5 days ago 

On 3/23/2026 9:13 PM, Bart Van Assche wrote:
> On 3/10/26 11:09 PM, palash.kambar@oss.qualcomm.com wrote:
>> [ ... ]
>> +static int ufshcd_get_connected_tx_lanes(struct ufs_hba *hba, u32 *tx_lanes)
>> +{
>> +    return ufshcd_dme_get(hba,
>> +                  UIC_ARG_MIB(PA_CONNECTEDTXDATALANES), tx_lanes);
>> +}
>> +
>> +static int ufshcd_get_connected_rx_lanes(struct ufs_hba *hba, u32 *rx_lanes)
>> +{
>> +    return ufshcd_dme_get(hba,
>> +                  UIC_ARG_MIB(PA_CONNECTEDRXDATALANES), rx_lanes);
>> +}
> 
> The body of the above two functions is very short. Please remove these
> functions and instead inline these function into their only caller.

Sure Bart, will address this.
> 
>> +static void ufshcd_validate_link_params(struct ufs_hba *hba)
>> +{
>> +    int val = 0;
>> +
>> +    if (ufshcd_get_connected_tx_lanes(hba, &val))
>> +        return;
> 
> Shouldn't it be reported if ufshcd_get_connected_tx_lanes() fails?
> 
>> +    if (ufshcd_get_connected_rx_lanes(hba, &val))
>> +        return;
> 
> Same question here - shouldn't it be reported if
> ufshcd_get_connected_rx_lanes() fails?
> 
> Why does this patch only call dev_err() in case of a mismatch instead of adjusting hba->lanes_per_direction or making initialization fail?

Idea was just to warn and catch the failure scenario. But I agree on your feedback. Will fix it.

> 
> Thanks,
> 
> Bart.
Re: [PATCH v1 1/2] ufs: core: Configure only active lanes during link
Posted by Shawn Lin 3 weeks, 6 days ago 
Hi Palash

在 2026/03/11 星期三 14:09, palash.kambar@oss.qualcomm.com 写道:
> From: Palash Kambar <palash.kambar@oss.qualcomm.com>
> 
> The number of active lanes detected during UFS link startup can be

connected lanes(which is used in the code blow) or active lanes? There
are different primitives in UniPro context.

> fewer than the lanes specified in the device tree. The current driver
> logic attempts to configure all lanes defined in the device tree,
> regardless of their actual availability. This mismatch may cause
> failures during power mode changes.
> 

It sounds vague, how it causes failures, could you quote some clue from
spec?

> Hence, add check to identify only the lanes that were successfully
> discovered during link startup, to warn on power mode change errors
> caused by mismatched lane counts.
> 
> Signed-off-by: Palash Kambar <palash.kambar@oss.qualcomm.com>
> ---
>   drivers/ufs/core/ufshcd.c | 39 +++++++++++++++++++++++++++++++++++++++
>   1 file changed, 39 insertions(+)
> 
> diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
> index 31950fc51a4c..c956fab32932 100644
> --- a/drivers/ufs/core/ufshcd.c
> +++ b/drivers/ufs/core/ufshcd.c
> @@ -5035,6 +5035,42 @@ void ufshcd_update_evt_hist(struct ufs_hba *hba, u32 id, u32 val)
>   }
>   EXPORT_SYMBOL_GPL(ufshcd_update_evt_hist);
>   
> +static int ufshcd_get_connected_tx_lanes(struct ufs_hba *hba, u32 *tx_lanes)
> +{
> +	return ufshcd_dme_get(hba,
> +			      UIC_ARG_MIB(PA_CONNECTEDTXDATALANES), tx_lanes);
> +}
> +
> +static int ufshcd_get_connected_rx_lanes(struct ufs_hba *hba, u32 *rx_lanes)
> +{
> +	return ufshcd_dme_get(hba,
> +			      UIC_ARG_MIB(PA_CONNECTEDRXDATALANES), rx_lanes);
> +}
> +
> +static void ufshcd_validate_link_params(struct ufs_hba *hba)
> +{
> +	int val = 0;
> +
> +	if (ufshcd_get_connected_tx_lanes(hba, &val))
> +		return;
> +
> +	if (val != hba->lanes_per_direction) {
> +		dev_err(hba->dev, "Tx lane mismatch [config,reported] [%d,%d]\n",
> +			hba->lanes_per_direction, val);
> +		return;
> +	}
> +
> +	val = 0;
> +
> +	if (ufshcd_get_connected_rx_lanes(hba, &val))
> +		return;
> +
> +	if (val != hba->lanes_per_direction) {
> +		dev_err(hba->dev, "Rx lane mismatch [config,reported] [%d,%d]\n",
> +			hba->lanes_per_direction, val);
> +	}
> +}
> +
>   /**
>    * ufshcd_link_startup - Initialize unipro link startup
>    * @hba: per adapter instance
> @@ -5108,6 +5144,9 @@ static int ufshcd_link_startup(struct ufs_hba *hba)
>   			goto out;
>   	}
>   
> +	/* Check successfully detected lanes */
> +	ufshcd_validate_link_params(hba);
> +
>   	/* Include any host controller configuration via UIC commands */
>   	ret = ufshcd_vops_link_startup_notify(hba, POST_CHANGE);
>   	if (ret)
>
Re: [PATCH v1 1/2] ufs: core: Configure only active lanes during link
Posted by Palash Kambar 2 weeks, 1 day ago 

On 3/11/2026 12:23 PM, Shawn Lin wrote:
> Hi Palash
> 
> 在 2026/03/11 星期三 14:09, palash.kambar@oss.qualcomm.com 写道:
>> From: Palash Kambar <palash.kambar@oss.qualcomm.com>
>>
>> The number of active lanes detected during UFS link startup can be
> 
> connected lanes(which is used in the code blow) or active lanes? There
> are different primitives in UniPro context.

Yes, I meant connected lanes, will fix comment.

> 
>> fewer than the lanes specified in the device tree. The current driver
>> logic attempts to configure all lanes defined in the device tree,
>> regardless of their actual availability. This mismatch may cause
>> failures during power mode changes.
>>
> 
> It sounds vague, how it causes failures, could you quote some clue from
> spec?

There was a negotiation mismatch for connected lanes during
link startup phase. So when host sends power mode change(PMC) after
UFS link startup, device returns a failure for PMC command with WR_ERROR_CAP
due to mismatch in the connected lane capability between host and device. 

> 
>> Hence, add check to identify only the lanes that were successfully
>> discovered during link startup, to warn on power mode change errors
>> caused by mismatched lane counts.
>>
>> Signed-off-by: Palash Kambar <palash.kambar@oss.qualcomm.com>
>> ---
>>   drivers/ufs/core/ufshcd.c | 39 +++++++++++++++++++++++++++++++++++++++
>>   1 file changed, 39 insertions(+)
>>
>> diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
>> index 31950fc51a4c..c956fab32932 100644
>> --- a/drivers/ufs/core/ufshcd.c
>> +++ b/drivers/ufs/core/ufshcd.c
>> @@ -5035,6 +5035,42 @@ void ufshcd_update_evt_hist(struct ufs_hba *hba, u32 id, u32 val)
>>   }
>>   EXPORT_SYMBOL_GPL(ufshcd_update_evt_hist);
>>   +static int ufshcd_get_connected_tx_lanes(struct ufs_hba *hba, u32 *tx_lanes)
>> +{
>> +    return ufshcd_dme_get(hba,
>> +                  UIC_ARG_MIB(PA_CONNECTEDTXDATALANES), tx_lanes);
>> +}
>> +
>> +static int ufshcd_get_connected_rx_lanes(struct ufs_hba *hba, u32 *rx_lanes)
>> +{
>> +    return ufshcd_dme_get(hba,
>> +                  UIC_ARG_MIB(PA_CONNECTEDRXDATALANES), rx_lanes);
>> +}
>> +
>> +static void ufshcd_validate_link_params(struct ufs_hba *hba)
>> +{
>> +    int val = 0;
>> +
>> +    if (ufshcd_get_connected_tx_lanes(hba, &val))
>> +        return;
>> +
>> +    if (val != hba->lanes_per_direction) {
>> +        dev_err(hba->dev, "Tx lane mismatch [config,reported] [%d,%d]\n",
>> +            hba->lanes_per_direction, val);
>> +        return;
>> +    }
>> +
>> +    val = 0;
>> +
>> +    if (ufshcd_get_connected_rx_lanes(hba, &val))
>> +        return;
>> +
>> +    if (val != hba->lanes_per_direction) {
>> +        dev_err(hba->dev, "Rx lane mismatch [config,reported] [%d,%d]\n",
>> +            hba->lanes_per_direction, val);
>> +    }
>> +}
>> +
>>   /**
>>    * ufshcd_link_startup - Initialize unipro link startup
>>    * @hba: per adapter instance
>> @@ -5108,6 +5144,9 @@ static int ufshcd_link_startup(struct ufs_hba *hba)
>>               goto out;
>>       }
>>   +    /* Check successfully detected lanes */
>> +    ufshcd_validate_link_params(hba);
>> +
>>       /* Include any host controller configuration via UIC commands */
>>       ret = ufshcd_vops_link_startup_notify(hba, POST_CHANGE);
>>       if (ret)
>>

Patchew 2.1-devel-ea86937

© 2016 - 2026 Red Hat, Inc.