Re: [PATCH v2 07/11] scsi: ufs: ufs-qcom: Fixup PAM-4 TX L0_L1_L2_L3 adaptation pattern length

[Can Guo <can.guo@oss.qualcomm.com>]

Hi Mani,

On 3/4/2026 11:10 PM, Manivannan Sadhasivam wrote:
> On Wed, Mar 04, 2026 at 05:53:09AM -0800, Can Guo wrote:
>> If HS-G6 Power Mode change handshake is successful and outbound data Lanes
>> are expected to transmit ADAPT, M-TX Lanes shall be configured as
>>
>> if (Adapt Type == REFRESH)
>>    TX_HS_ADAPT_LENGTH_L0_L1_L2_L3 = PA_PeerRxHsG6AdaptRefreshL0L1L2L3.
>> else if (Adapt Type == INITIAL)
>>    TX_HS_ADAPT_LENGTH_L0_L1_L2_L3 = PA_PeerRxHsG6AdaptInitialL0L1L2L3.
>>
>> On some platforms, the ADAPT_L0_L1_L2_L3 duration on Host TX Lanes is only
>> a half of theoretical ADAPT_L0_L1_L2_L3 duration TADAPT_L0_L1_L2_L3 (in
>> PAM-4 UI) calculated from TX_HS_ADAPT_LENGTH_L0_L1_L2_L3.
>>
>> For such platforms, the workaround is to double the ADAPT_L0_L1_L2_L3
>> duration by uplifting TX_HS_ADAPT_LENGTH_L0_L1_L2_L3. UniPro initializes
>> TX_HS_ADAPT_LENGTH_L0_L1_L2_L3 during HS-G6 Power Mode change handshake,
>> it would be too late for SW to update TX_HS_ADAPT_LENGTH_L0_L1_L2_L3 post
>> HS-G6 Power Mode change. Update PA_PeerRxHsG6AdaptRefreshL0L1L2L3 and
>> PA_PeerRxHsG6AdaptInitialL0L1L2L3 post Link Startup and before HS-G6
>> Power Mode change, so that the UniPro would use the updated value during
>> HS-G6 Power Mode change handshake.
>>
>> Signed-off-by: Can Guo <can.guo@oss.qualcomm.com>
>> ---
>>   drivers/ufs/host/ufs-qcom.c | 175 ++++++++++++++++++++++++++++++++++++
>>   1 file changed, 175 insertions(+)
>>
>> diff --git a/drivers/ufs/host/ufs-qcom.c b/drivers/ufs/host/ufs-qcom.c
>> index 5eb12a999eb1..3a9279066192 100644
>> --- a/drivers/ufs/host/ufs-qcom.c
>> +++ b/drivers/ufs/host/ufs-qcom.c
>> @@ -1079,10 +1079,185 @@ static void ufs_qcom_override_pa_tx_hsg1_sync_len(struct ufs_hba *hba)
>>   		dev_err(hba->dev, "Failed (%d) set PA_TX_HSG1_SYNC_LENGTH\n", err);
>>   }
>>   
>> +/**
>> + * ufs_qcom_double_t_adapt_l0l1l2l3 - Create a new adapt that doubles the
>> + * adaptation duration TADAPT_L0_L1_L2_L3 derived from the old adapt.
>> + *
>> + * @old_adapt: Original ADAPT_L0_L1_L2_L3 capability
>> + *
>> + * ADAPT_length_L0_L1_L2_L3 formula from M-PHY spec:
>> + * if (ADAPT_range_L0_L1_L2_L3 == COARSE) {
>> + *   ADAPT_length_L0_L1_L2_L3 = [0, 12]
>> + *   ADAPT_L0_L1_L2_L3 = 215 x 2^ADAPT_length_L0_L1_L2_L3
>> + * } else if (ADAPT_range_L0_L1_L2_L3 == FINE) {
>> + *   ADAPT_length_L0_L1_L2_L3 = [0, 127]
>> + *   TADAPT_L0_L1_L2_L3 = 215 x (ADAPT_length_L0_L1_L2_L3 + 1)
>> + * }
>> + *
>> + * To double the adaptation duration TADAPT_L0_L1_L2_L3:
>> + * 1. If adapt range is COARSE (1'b1), new adapt = old adapt + 1.
>> + * 2. If adapt range is FINE (1'b0):
>> + *   a) If old adapt length is < 64, (new adapt + 1) = 2 * (old adapt + 1).
>> + *   b) If old adapt length is >= 64, set new adapt to 0x88 using COARSE
>> + *      range, because new adapt get from equation in a) shall exceed 127.
>> + *
>> + * Examples:
>> + * ADAPT_range_L0_L1_L2_L3 | ADAPT_length_L0_L1_L2_L3 | TADAPT_L0_L1_L2_L3 (PAM-4 UI)
>> + *		0			3			131072
>> + *		0			7			262144
>> + *		0			63			2097152
>> + *		0			64			2129920
>> + *		0			127			4194304
>> + *		1			8			8388608
>> + *		1			9			16777216
>> + *		1			10			33554432
>> + *		1			11			67108864
>> + *		1			12			134217728
>> + *
>> + * Return: new adapt.
>> + */
>> +static inline u32 ufs_qcom_double_t_adapt_l0l1l2l3(u32 old_adapt)
> No need of 'inline' keyword in a .c file. Same comment to other helpers.
OK.
>
> Also, can you change the '_l0l1l2l3' suffix to something like '_level' or
> '_length'?
>
There are many Adapt length attributes in M-PHY spec, their definitions 
are similar
but used for different purposes. To make sure we are capture the correct 
one,
let's use the full name
>> +{
>> +	u32 adapt_length = old_adapt & 0x7F;
> Please add a define for 0x75
Sure.
>
>> +	u32 new_adapt;
>> +
>> +	/* Adapt range == COARSE */
>> +	if (old_adapt & 0x80) {
> This one also.
Will do.
>
>> +		new_adapt = (adapt_length + 1) | 0x80;
>> +	} else {
>> +		if (adapt_length < 64)
> And this one.
Will do.
>
>> +			new_adapt = (adapt_length << 1) + 1;
>> +		else
>> +			new_adapt = 0x88;
>> +	}
>> +
>> +	return new_adapt;
>> +}
>> +
>> +static inline void ufs_qcom_limit_max_gear(struct ufs_hba *hba,
>> +					   enum ufs_hs_gear_tag gear)
>> +{
>> +	struct ufs_qcom_host *host = ufshcd_get_variant(hba);
>> +	struct ufs_pa_layer_attr *pwr_info = &hba->max_pwr_info.info;
>> +	struct ufs_host_params *host_params = &host->host_params;
>> +
>> +	host_params->hs_tx_gear = gear;
>> +	host_params->hs_rx_gear = gear;
>> +	pwr_info->gear_tx = gear;
>> +	pwr_info->gear_rx = gear;
>> +
>> +	dev_warn(hba->dev, "Limited max gear of both sides to HS-G%d\n", gear);
> s/both sides/host and device
OK.
>
>> +}
>> +
>> +static void ufs_qcom_fixup_tx_adapt_l0l1l2l3(struct ufs_hba *hba)
>> +{
>> +	struct ufs_qcom_host *host = ufshcd_get_variant(hba);
>> +	struct ufs_pa_layer_attr *pwr_info = &hba->max_pwr_info.info;
>> +	struct ufs_host_params *host_params = &host->host_params;
>> +	u32 adapt_l0l1l2l3, new_adapt, actual_adapt;
> Can you shorten adapt_l0l1l2l3?
As I explained above, I want to capture the precise Adapt attribute.
>
>> +	bool limit_speed = false;
>> +	int err;
>> +
>> +	if (host->hw_ver.major != 0x7 || host->hw_ver.minor > 0x1 ||
>> +	    host_params->hs_tx_gear <= UFS_HS_G5 ||
>> +	    pwr_info->gear_tx <= UFS_HS_G5)
>> +		return;
>> +
>> +	err = ufshcd_dme_get(hba, UIC_ARG_MIB(PA_PEERRXHSG6ADAPTINITIALL0L1L2L3), &adapt_l0l1l2l3);
>> +	if (err)
>> +		goto out;
>> +
>> +	if (adapt_l0l1l2l3 > ADAPT_L0L1L2L3_LENGTH_MAX) {
>> +		dev_err(hba->dev, "PA_PeerRxHsG6AdaptInitialL0L1L2L3 value (0x%x) exceeds MAX.\n",
> Nit: remove full stop at the end
OK.
>
>> +			adapt_l0l1l2l3);
>> +		err = -EINVAL;
> -ERANGE
Sure.
>
>> +		goto out;
>> +	}
>> +
>> +	new_adapt = ufs_qcom_double_t_adapt_l0l1l2l3(adapt_l0l1l2l3);
>> +	dev_dbg(hba->dev, "Original PA_PeerRxHsG6AdaptInitialL0L1L2L3 value = 0x%x, new value = 0x%x\n",
>> +		adapt_l0l1l2l3, new_adapt);
>> +
>> +	/*
>> +	 * 0x8C is the max possible value allowed by UniPro v3.0 spec, some HWs
>> +	 * can accept 0x8D but some cannot.
>> +	 */
>> +	if (new_adapt <= ADAPT_L0L1L2L3_LENGTH_MAX ||
>> +	    (new_adapt == ADAPT_L0L1L2L3_LENGTH_MAX + 1 && host->hw_ver.minor == 0x1)) {
>> +		err = ufshcd_dme_set(hba, UIC_ARG_MIB(PA_PEERRXHSG6ADAPTINITIALL0L1L2L3),
>> +				     new_adapt);
>> +		if (err)
>> +			goto out;
>> +
>> +		err = ufshcd_dme_get(hba, UIC_ARG_MIB(PA_PEERRXHSG6ADAPTINITIALL0L1L2L3),
>> +				     &actual_adapt);
>> +		if (err)
>> +			goto out;
>> +
>> +		if (actual_adapt != new_adapt) {
>> +			limit_speed = true;
>> +			dev_warn(hba->dev, "Failed to update host PA_PeerRxHsG6AdaptInitialL0L1L2L3 to new value 0x%x, actual value = 0x%x\n",
> This goes beyond 100 column width. Please consider shortening up. Applies to
> other prints as well.
Will shorten them in next version.
>
>> +				 new_adapt, actual_adapt);
>> +		}
>> +	} else {
>> +		limit_speed = true;
>> +		dev_warn(hba->dev, "New PA_PeerRxHsG6AdaptInitialL0L1L2L3 value (0x%x) is too large!\n",
>> +			 new_adapt);
>> +	}
>> +
>> +	err = ufshcd_dme_get(hba, UIC_ARG_MIB(PA_PEERRXHSG6ADAPTREFRESHL0L1L2L3), &adapt_l0l1l2l3);
>> +	if (err)
>> +		goto out;
>> +
>> +	if (adapt_l0l1l2l3 > ADAPT_L0L1L2L3_LENGTH_MAX) {
>> +		dev_err(hba->dev, "PA_PeerRxHsG6AdaptRefreshL0L1L2L3 value (0x%x) exceeds MAX.\n",
>> +			adapt_l0l1l2l3);
>> +		err = -EINVAL;
> -ERANGE
>
>> +		goto out;
>> +	}
>> +
>> +	new_adapt = ufs_qcom_double_t_adapt_l0l1l2l3(adapt_l0l1l2l3);
>> +	dev_dbg(hba->dev, "Original PA_PeerRxHsG6AdaptRefreshL0L1L2L3 value = 0x%x, new value = 0x%x\n",
>> +		adapt_l0l1l2l3, new_adapt);
>> +
>> +	/*
>> +	 * 0x8C is the max possible value allowed by UniPro v3.0 spec, some HWs
>> +	 * can accept 0x8D but some cannot.
>> +	 */
>> +	if (new_adapt <= ADAPT_L0L1L2L3_LENGTH_MAX ||
>> +	    (new_adapt == ADAPT_L0L1L2L3_LENGTH_MAX + 1 && host->hw_ver.minor == 0x1)) {
>> +		err = ufshcd_dme_set(hba, UIC_ARG_MIB(PA_PEERRXHSG6ADAPTREFRESHL0L1L2L3),
>> +				     new_adapt);
>> +		if (err)
>> +			goto out;
>> +
>> +		err = ufshcd_dme_get(hba, UIC_ARG_MIB(PA_PEERRXHSG6ADAPTREFRESHL0L1L2L3),
>> +				     &actual_adapt);
>> +		if (err)
>> +			goto out;
>> +
>> +		if (actual_adapt != new_adapt) {
>> +			limit_speed = true;
>> +			dev_warn(hba->dev, "Failed to update host PA_PeerRxHsG6AdaptRefreshL0L1L2L3 to new value 0x%x, actual value = 0x%x\n",
>> +				 new_adapt, actual_adapt);
>> +		}
>> +	} else {
>> +		limit_speed = true;
>> +		dev_warn(hba->dev, "New PA_PeerRxHsG6AdaptRefreshL0L1L2L3 value (0x%x) is too large!\n",
>> +			 new_adapt);
> I'm assuming it is safe to continue despite the warnings.
Yes, warning here is to give the reason as well as heads up that it is 
going to limit the max gear.

Thanks,
Can Guo.
>
> - Mani
>