Re: [PATCH v2 09/11] scsi: ufs: ufs-qcom: Implement vops get_rx_fom()

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

On 3/4/2026 11:39 PM, Manivannan Sadhasivam wrote:
> On Wed, Mar 04, 2026 at 05:53:11AM -0800, Can Guo wrote:
>> On some platforms, host's M-PHY RX_FOM Attribute always reads 0, meaning
>> SW cannot rely on Figure of Merit (FOM) to identify the optimal TX
>> Equalization settings for device's TX Lanes. Implement the vops
>> ufs_qcom_get_rx_fom() such that SW can utilize the UFS Eye Opening Monitor
>> (EOM) to evaluate the TX Equalization settings for device's TX Lanes.
>>
>> Signed-off-by: Can Guo <can.guo@oss.qualcomm.com>
>> ---
>>   drivers/ufs/core/ufs-txeq.c |   6 +-
>>   drivers/ufs/host/ufs-qcom.c | 315 ++++++++++++++++++++++++++++++++++++
>>   drivers/ufs/host/ufs-qcom.h |  42 +++++
>>   include/ufs/ufshcd.h        |   3 +
>>   include/ufs/unipro.h        |  16 ++
>>   5 files changed, 379 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/ufs/core/ufs-txeq.c b/drivers/ufs/core/ufs-txeq.c
>> index 2cd2d5156607..02c1a8479910 100644
>> --- a/drivers/ufs/core/ufs-txeq.c
>> +++ b/drivers/ufs/core/ufs-txeq.c
>> @@ -227,9 +227,8 @@ ufshcd_compose_tx_eq_setting(struct ufshcd_tx_eq_settings *settings,
>>    *
>>    * Returns 0 on success, negative error code otherwise
>>    */
>> -static int ufshcd_apply_tx_eq_settings(struct ufs_hba *hba,
>> -				       struct ufshcd_tx_eq_params *params,
>> -				       u32 gear)
>> +int ufshcd_apply_tx_eq_settings(struct ufs_hba *hba,
>> +				struct ufshcd_tx_eq_params *params, u32 gear)
>>   {
>>   	u32 setting;
>>   	int ret;
>> @@ -259,6 +258,7 @@ static int ufshcd_apply_tx_eq_settings(struct ufs_hba *hba,
>>   
>>   	return ret;
>>   }
>> +EXPORT_SYMBOL_GPL(ufshcd_apply_tx_eq_settings);
>>   
>>   /**
>>    * ufshcd_evaluate_fom - Update TX EQ params based on FOM results
>> diff --git a/drivers/ufs/host/ufs-qcom.c b/drivers/ufs/host/ufs-qcom.c
>> index 1e074058f23d..b8fa4670ddd6 100644
>> --- a/drivers/ufs/host/ufs-qcom.c
>> +++ b/drivers/ufs/host/ufs-qcom.c
>> @@ -2534,6 +2534,320 @@ static int ufs_qcom_change_power_mode(struct ufs_hba *hba,
>>   	return ret;
>>   }
>>   
>> +static int ufs_qcom_host_eom_config(struct ufs_hba *hba, int lane, int v_step,
>> +				    int t_step, enum ufs_eom_eye_pos eye_pos,
>> +				    u32 target_test_count)
>> +{
>> +	u32 volt_step, timing_step;
>> +	int ret;
>> +
>> +	if (v_step > 127 || v_step < -127) {
>> +		dev_err(hba->dev, "Invalid EOM Voltage Step: %d\n", v_step);
>> +		return -EINVAL;
> -ERANGE
Done.
>
>> +	}
>> +
>> +	if (t_step > 63 || t_step < -63) {
>> +		dev_err(hba->dev, "Invalid EOM Timing Step: %d\n", t_step);
>> +		return -EINVAL;
> -ERANGE
Done.
>
>> +	}
>> +
>> +	if (v_step < 0)
>> +		volt_step = BIT(6) | (u32)(-v_step);
> What does BIT(6) correspond to? Create a define maybe?
Will do.
>
>> +	else
>> +		volt_step = (u32)v_step;
>> +
>> +	if (t_step < 0)
>> +		timing_step = BIT(6) | (u32)(-t_step);
>> +	else
>> +		timing_step = (u32)t_step;
>> +
>> +	ret = ufshcd_dme_set(hba, UIC_ARG_MIB_SEL(RX_EYEMON_ENABLE,
>> +				UIC_ARG_MPHY_RX_GEN_SEL_INDEX(lane)),
>> +			     BIT(eye_pos) | BIT(6));
>> +	if (ret) {
>> +		dev_err(hba->dev, "Failed to enable Host EOM on Lane %d: %d\n",
>> +			lane, ret);
>> +		return ret;
>> +	}
>> +
>> +	ret = ufshcd_dme_set(hba, UIC_ARG_MIB_SEL(RX_EYEMON_TIMING_STEPS,
>> +				UIC_ARG_MPHY_RX_GEN_SEL_INDEX(lane)),
>> +			     timing_step);
>> +	if (ret) {
>> +		dev_err(hba->dev, "Failed to set Host EOM timing step on Lane %d: %d\n",
>> +			lane, ret);
>> +		return ret;
>> +	}
>> +
>> +	ret = ufshcd_dme_set(hba, UIC_ARG_MIB_SEL(RX_EYEMON_VOLTAGE_STEPS,
>> +				UIC_ARG_MPHY_RX_GEN_SEL_INDEX(lane)),
>> +			     volt_step);
>> +	if (ret) {
>> +		dev_err(hba->dev, "Failed to set Host EOM voltage step on Lane %d: %d\n",
>> +			lane, ret);
>> +		return ret;
>> +	}
>> +
>> +	ret = ufshcd_dme_set(hba, UIC_ARG_MIB_SEL(RX_EYEMON_TARGET_TEST_COUNT,
>> +				UIC_ARG_MPHY_RX_GEN_SEL_INDEX(lane)),
>> +			     target_test_count);
>> +	if (ret)
>> +		dev_err(hba->dev, "Failed to set Host EOM target test count on Lane %d: %d\n",
>> +			lane, ret);
>> +
>> +	return ret;
>> +}
>> +
>> +static int ufs_qcom_host_eom_may_stop(struct ufs_hba *hba, int lane,
>> +				      u32 target_test_count, u32 *err_count)
>> +{
>> +	u32 start, tested_count, error_count;
>> +	int ret;
>> +
>> +	ret = ufshcd_dme_get(hba, UIC_ARG_MIB_SEL(RX_EYEMON_START,
>> +				UIC_ARG_MPHY_RX_GEN_SEL_INDEX(lane)),
>> +			     &start);
>> +	if (ret) {
>> +		dev_err(hba->dev, "Failed to get Host EOM start status on Lane %d: %d\n",
>> +			lane, ret);
>> +		return ret;
>> +	}
>> +
>> +	if (start & 0x1)
>> +		return -EAGAIN;
>> +
>> +	ret = ufshcd_dme_get(hba, UIC_ARG_MIB_SEL(RX_EYEMON_TESTED_COUNT,
>> +				UIC_ARG_MPHY_RX_GEN_SEL_INDEX(lane)),
>> +			     &tested_count);
>> +	if (ret) {
>> +		dev_err(hba->dev, "Failed to get Host EOM tested count on Lane %d: %d\n",
>> +			lane, ret);
>> +		return ret;
>> +	}
>> +
>> +	ret = ufshcd_dme_get(hba, UIC_ARG_MIB_SEL(RX_EYEMON_ERROR_COUNT,
>> +				UIC_ARG_MPHY_RX_GEN_SEL_INDEX(lane)),
>> +			     &error_count);
>> +	if (ret) {
>> +		dev_err(hba->dev, "Failed to get Host EOM error count on Lane %d: %d\n",
>> +			lane, ret);
>> +		return ret;
>> +	}
>> +
>> +	/* EOM can stop */
>> +	if ((tested_count >= target_test_count - 3) || error_count > 0) {
>> +		*err_count = error_count;
>> +
>> +		/* Disable EOM */
>> +		ret = ufshcd_dme_set(hba, UIC_ARG_MIB_SEL(RX_EYEMON_ENABLE,
>> +					UIC_ARG_MPHY_RX_GEN_SEL_INDEX(lane)),
>> +				     0x0);
>> +		if (ret) {
>> +			dev_err(hba->dev, "Failed to disable Host EOM on Lane %d: %d\n",
>> +				lane, ret);
>> +			return ret;
>> +		}
>> +	} else {
>> +		return -EAGAIN;
>> +	}
>> +
>> +	return ret;
> return 0;
Done.
>> +}
>> +
>> +static int ufs_qcom_host_eom_scan(struct ufs_hba *hba, int num_lanes,
>> +				  const struct ufs_eom_coord *eom_coord,
>> +				  u32 target_test_count, u32 *err_count)
>> +{
>> +	bool eom_stopped[PA_MAXDATALANES] = { 0 };
>> +	enum ufs_eom_eye_pos eye_pos;
>> +	int lane, v_step, t_step, ret;
>> +	u32 setting;
>> +
>> +	if (!err_count || !eom_coord)
>> +		return -EINVAL;
>> +
>> +	if (target_test_count < 8) {
>> +		dev_err(hba->dev, "%s: Target test count (%u) too small for Host EOM\n",
>> +			__func__, target_test_count);
> No __func__ please.
Done.
>
>> +		return -EINVAL;
> -ERANGE
Done.
>
>> +	}
>> +
>> +	v_step = eom_coord->v_step;
>> +	t_step = eom_coord->t_step;
>> +	eye_pos = eom_coord->eye_pos;
>> +
>> +	for (lane = 0; lane < num_lanes; lane++) {
>> +		ret = ufs_qcom_host_eom_config(hba, lane, v_step, t_step,
>> +					       eye_pos, target_test_count);
>> +		if (ret) {
>> +			dev_err(hba->dev, "Failed to config Host EOM: %d\n",
>> +				ret);
>> +			return ret;
>> +		}
>> +	}
>> +
>> +	/*
>> +	 * Trigger a PACP_PWR_req to kick start EOM, but not to really change
>> +	 * the Power Mode.
>> +	 */
>> +	ret = ufshcd_uic_change_pwr_mode(hba, FAST_MODE << 4 | FAST_MODE);
>> +	if (ret) {
>> +		dev_err(hba->dev, "Failed to change power mode to kick start Host EOM: %d\n",
>> +			ret);
>> +		return ret;
>> +	}
>> +
>> +more_burst:
>> +	/* Create burst on Host RX Lane. */
>> +	ufshcd_dme_peer_get(hba, UIC_ARG_MIB(PA_LOCALVERINFO), &setting);
>> +
>> +	for (lane = 0; lane < num_lanes; lane++) {
>> +		if (eom_stopped[lane])
>> +			continue;
>> +
>> +		ret = ufs_qcom_host_eom_may_stop(hba, lane, target_test_count,
>> +						 &err_count[lane]);
>> +		if (!ret) {
>> +			eom_stopped[lane] = true;
>> +		} else if (ret == -EAGAIN) {
>> +			/* Need more burst to excercise EOM */
>> +			goto more_burst;
>> +		} else {
>> +			dev_err(hba->dev, "Failed to stop Host EOM: %d\n", ret);
>> +			return ret;
>> +		}
>> +
>> +		dev_dbg(hba->dev, "%s: Host RX Lane %d EOM, v_step %d, t_step %d, error count %u\n",
>> +			__func__, lane, v_step, t_step, err_count[lane]);
>> +	}
>> +
>> +	return ret;
> return 0;
>
>> +}
>> +
>> +static int ufs_qcom_host_sw_rx_fom(struct ufs_hba *hba, int num_lanes, u32 *fom)
>> +{
>> +	const struct ufs_eom_coord *eom_coord = sw_rx_fom_eom_coords_g6;
>> +	u32 eom_err_count[PA_MAXDATALANES] = { 0 };
>> +	u32 curr_ahit;
>> +	int lane, i, ret;
>> +
>> +	if (!fom)
>> +		return -EINVAL;
>> +
>> +	/* Stop the auto hibernate idle timer */
>> +	curr_ahit = ufshcd_readl(hba, REG_AUTO_HIBERNATE_IDLE_TIMER);
>> +	if (curr_ahit)
>> +		ufshcd_writel(hba, 0, REG_AUTO_HIBERNATE_IDLE_TIMER);
>> +
>> +	ret = ufshcd_dme_set(hba, UIC_ARG_MIB(PA_TXHSADAPTTYPE), PA_NO_ADAPT);
>> +	if (ret) {
>> +		dev_err(hba->dev, "%s: Failed to select NO_ADAPT before starting Host EOM: %d\n",
>> +			__func__, ret);
>> +		goto out;
>> +	}
>> +
>> +	for (i = 0; i < SW_RX_FOM_EOM_COORDS; i++, eom_coord++) {
>> +		ret = ufs_qcom_host_eom_scan(hba, num_lanes, eom_coord, 0x3F,
>> +					     eom_err_count);
>> +		if (ret) {
>> +			dev_err(hba->dev, "%s: Failed to run Host EOM scan: %d\n",
>> +				__func__, ret);
>> +			break;
>> +		}
>> +
>> +		for (lane = 0; lane < num_lanes; lane++) {
>> +			/* Bad coordinates have no weights */
>> +			if (eom_err_count[lane])
>> +				continue;
>> +			fom[lane] += SW_RX_FOM_EOM_COORDS_WEIGHT;
>> +		}
>> +	}
>> +
>> +out:
>> +	/* Restore the auto hibernate idle timer */
>> +	if (curr_ahit)
>> +		ufshcd_writel(hba, curr_ahit, REG_AUTO_HIBERNATE_IDLE_TIMER);
>> +
>> +	return ret;
>> +}
>> +
>> +static int ufs_qcom_get_rx_fom(struct ufs_hba *hba,
>> +			       struct ufs_pa_layer_attr *pwr_mode,
>> +			       struct tx_eqtr_iter *h_iter,
>> +			       struct tx_eqtr_iter *d_iter)
>> +{
>> +	struct ufshcd_tx_eq_params *params __free(kfree) =
>> +		kzalloc(sizeof(*params), GFP_KERNEL);
>> +	struct ufs_qcom_host *host = ufshcd_get_variant(hba);
>> +	struct ufs_pa_layer_attr old_pwr_info;
>> +	u32 fom[PA_MAXDATALANES] = { 0 };
>> +	u32 gear = pwr_mode->gear_tx;
>> +	u32 rate = pwr_mode->hs_rate;
>> +	int lane, ret;
>> +
>> +	if (host->hw_ver.major != 0x7 || host->hw_ver.minor > 0x1 ||
>> +	    gear <= UFS_HS_G5 || !d_iter || !d_iter->is_new)
>> +		return 0;
>> +
>> +	if (gear < UFS_HS_G1 || gear >= UFS_HS_GEAR_MAX)
>> +		return -EINVAL;
> -ERANGE
>
>> +
>> +	if (!params)
>> +		return -ENOMEM;
>> +
>> +	memcpy(&old_pwr_info, &hba->pwr_info, sizeof(struct ufs_pa_layer_attr));
>> +
>> +	memcpy(params, &hba->tx_eq_params[gear - 1], sizeof(struct ufshcd_tx_eq_params));
>> +	for (lane = 0; lane < d_iter->num_lanes; lane++) {
>> +		params->device[lane].preshoot = d_iter->preshoot;
>> +		params->device[lane].deemphasis = d_iter->deemphasis;
>> +	}
>> +
>> +	/* Use TX EQTR settings as Device's TX Equalization settings. */
>> +	ret = ufshcd_apply_tx_eq_settings(hba, params, gear);
>> +	if (ret) {
>> +		dev_err(hba->dev, "%s: Failed to apply TX EQ settings for HS-G%u: %d\n",
>> +			__func__, gear, ret);
>> +		return ret;
>> +	}
>> +
>> +	/* Force PMC to target HS Gear to use new TX Equalization settings. */
>> +	ret = ufs_qcom_change_power_mode(hba, pwr_mode, UFSHCD_PMC_POLICY_FORCE);
>> +	if (ret) {
>> +		dev_err(hba->dev, "%s: Failed to change power mode to HS-G%u, Rate-%s: %d\n",
>> +			__func__, gear, UFS_HS_RATE_STRING(rate), ret);
>> +		return ret;
>> +	}
>> +
>> +	ret = ufs_qcom_host_sw_rx_fom(hba, d_iter->num_lanes, fom);
>> +	if (ret) {
>> +		dev_err(hba->dev, "Failed to get Host SW FOM for Device TX Lane (PreShoot: %u, DeEmphasis: %u): %d\n",
>> +			d_iter->preshoot, d_iter->deemphasis, ret);
>> +		return ret;
>> +	}
>> +
>> +	/* Restore Device's TX Equalization settings. */
>> +	ret = ufshcd_apply_tx_eq_settings(hba, &hba->tx_eq_params[gear - 1], gear);
>> +	if (ret) {
>> +		dev_err(hba->dev, "%s: Failed to apply TX EQ settings for HS-G%u: %d\n",
>> +			__func__, gear, ret);
>> +		return ret;
>> +	}
>> +
>> +	/* Restore Power Mode. */
>> +	ret = ufs_qcom_change_power_mode(hba, &old_pwr_info, UFSHCD_PMC_POLICY_FORCE);
>> +	if (ret) {
>> +		dev_err(hba->dev, "%s: Failed to retore power mode to HS-G%u: %d\n",
>> +			__func__, old_pwr_info.gear_tx, ret);
>> +		return ret;
>> +	}
>> +
>> +	for (lane = 0; lane < d_iter->num_lanes; lane++)
>> +		d_iter->fom[lane] = fom[lane];
>> +
>> +	return ret;
> return 0;
Done.
> - Mani
>