Re: [PATCH v3 10/14] phy: qcom: qmp-usbc: Add TCSR parsing and USB/DP mode mutex handling

[Xiangxu Yin <xiangxu.yin@oss.qualcomm.com>]

On 8/20/2025 7:24 PM, Dmitry Baryshkov wrote:
> On Wed, Aug 20, 2025 at 05:34:52PM +0800, Xiangxu Yin wrote:
>> Parse TCSR registers to support DP mode signaling via dp_phy_mode_reg.
>> Introduce mutual exclusion between USB and DP PHY modes to prevent
>> simultaneous activation. Also update com_init/com_exit to reflect DP
>> mode initialization and cleanup.
>>
>> Signed-off-by: Xiangxu Yin <xiangxu.yin@oss.qualcomm.com>
>> ---
>>  drivers/phy/qualcomm/phy-qcom-qmp-usbc.c | 60 +++++++++++++++++++++++++-------
>>  1 file changed, 47 insertions(+), 13 deletions(-)
>>
>> diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-usbc.c b/drivers/phy/qualcomm/phy-qcom-qmp-usbc.c
>> index a1495a2029cf038bb65c36e42d0a4f633e544558..821398653bef23e1915d9d3a3a2950b0bfbefb9a 100644
>> --- a/drivers/phy/qualcomm/phy-qcom-qmp-usbc.c
>> +++ b/drivers/phy/qualcomm/phy-qcom-qmp-usbc.c
>> @@ -674,7 +674,7 @@ static const struct qmp_phy_cfg qcs615_usb3dp_phy_cfg = {
>>  	.num_vregs		= ARRAY_SIZE(qmp_phy_usbdp_vreg_l),
>>  };
>>  
>> -static int qmp_usbc_com_init(struct phy *phy)
>> +static int qmp_usbc_com_init(struct phy *phy, bool is_dp)
>>  {
>>  	struct qmp_usbc *qmp = phy_get_drvdata(phy);
>>  	const struct qmp_phy_cfg *cfg = qmp->cfg;
>> @@ -704,15 +704,20 @@ static int qmp_usbc_com_init(struct phy *phy)
>>  	if (ret)
>>  		goto err_assert_reset;
>>  
>> -	qphy_setbits(pcs, cfg->regs[QPHY_PCS_POWER_DOWN_CONTROL], SW_PWRDN);
>> +	if (!is_dp) {
>> +		qphy_setbits(pcs, cfg->regs[QPHY_PCS_POWER_DOWN_CONTROL], SW_PWRDN);
> Why? Don't we need to program those bits for DP PHY too?
>
> If not, move them to USB init call.


For these USB3DP PHY series, USB registers in pcs_misc will don’t affect DP. 

I’ll move them to USB init call.


>>  
>>  #define SW_PORTSELECT_VAL			BIT(0)
>>  #define SW_PORTSELECT_MUX			BIT(1)
>> -	/* Use software based port select and switch on typec orientation */
>> -	val = SW_PORTSELECT_MUX;
>> -	if (qmp->orientation == TYPEC_ORIENTATION_REVERSE)
>> -		val |= SW_PORTSELECT_VAL;
>> -	writel(val, qmp->pcs_misc);
>> +		/* Use software based port select and switch on typec orientation */
>> +		val = SW_PORTSELECT_MUX;
>> +		if (qmp->orientation == TYPEC_ORIENTATION_REVERSE)
>> +			val |= SW_PORTSELECT_VAL;
>> +		writel(val, qmp->pcs_misc);
>> +	}
>> +
>> +	if (qmp->tcsr_map && qmp->dp_phy_mode_reg)
>> +		regmap_write(qmp->tcsr_map, qmp->dp_phy_mode_reg, is_dp);
> Write this reg directly from USB / DP init.


Ok.


>>  
>>  	return 0;
>>  
>> @@ -733,6 +738,9 @@ static int qmp_usbc_com_exit(struct phy *phy)
>>  
>>  	clk_bulk_disable_unprepare(qmp->num_clks, qmp->clks);
>>  
>> +	if (qmp->tcsr_map && qmp->dp_phy_mode_reg)
>> +		regmap_write(qmp->tcsr_map, qmp->dp_phy_mode_reg, 0);
> Why?


Since this is a switchable PHY, it makes more sense to set it only during enable.
I’ll remove the TCSR handling from com_exit in the next patch.


>> +
>>  	regulator_bulk_disable(cfg->num_vregs, qmp->vregs);
>>  
>>  	return 0;
>> @@ -1045,6 +1053,17 @@ static int qmp_usbc_usb_power_off(struct phy *phy)
>>  	return 0;
>>  }
>>  
>> +static int qmp_check_mutex_phy(struct qmp_usbc *qmp, bool is_dp)
>> +{
>> +	if ((is_dp && qmp->usb_init_count) ||
>> +	    (!is_dp && qmp->dp_init_count)) {
>> +		dev_err(qmp->dev, "%s PHY busy\n", is_dp ? "USB" : "DP");
> "PHY is configured for %s, can not enable %s\n"


Ack.


>> +		return -EBUSY;
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>>  static int qmp_usbc_usb_enable(struct phy *phy)
>>  {
>>  	struct qmp_usbc *qmp = phy_get_drvdata(phy);
>> @@ -1052,7 +1071,11 @@ static int qmp_usbc_usb_enable(struct phy *phy)
>>  
>>  	mutex_lock(&qmp->phy_mutex);
>>  
>> -	ret = qmp_usbc_com_init(phy);
>> +	ret = qmp_check_mutex_phy(qmp, false);
>> +	if (ret)
>> +		goto out_unlock;
>> +
>> +	ret = qmp_usbc_com_init(phy, false);
>>  	if (ret)
>>  		goto out_unlock;
>>  
>> @@ -1103,7 +1126,11 @@ static int qmp_usbc_dp_enable(struct phy *phy)
>>  
>>  	mutex_lock(&qmp->phy_mutex);
>>  
>> -	ret = qmp_usbc_com_init(phy);
>> +	ret = qmp_check_mutex_phy(qmp, true);
>> +	if (ret)
>> +		goto dp_init_unlock;
>> +
>> +	ret = qmp_usbc_com_init(phy, true);
>>  	if (ret)
>>  		goto dp_init_unlock;
>>  
>> @@ -1467,7 +1494,7 @@ static int qmp_usbc_typec_switch_set(struct typec_switch_dev *sw,
>>  		qmp_usbc_usb_power_off(qmp->usb_phy);
>>  		qmp_usbc_com_exit(qmp->usb_phy);
>>  
>> -		qmp_usbc_com_init(qmp->usb_phy);
>> +		qmp_usbc_com_init(qmp->usb_phy, false);
>>  		qmp_usbc_usb_power_on(qmp->usb_phy);
>>  	}
>>  
>> @@ -1602,13 +1629,13 @@ static int qmp_usbc_parse_usb_dt(struct qmp_usbc *qmp)
>>  	return 0;
>>  }
>>  
>> -static int qmp_usbc_parse_vls_clamp(struct qmp_usbc *qmp)
>> +static int qmp_usbc_parse_tcsr(struct qmp_usbc *qmp)
>>  {
>>  	struct of_phandle_args tcsr_args;
>>  	struct device *dev = qmp->dev;
>>  	int ret;
>>  
>> -	/*  for backwards compatibility ignore if there is no property */
>> +	/*  for backwards compatibility ignore if there is 1 or no property */
>>  	ret = of_parse_phandle_with_fixed_args(dev->of_node, "qcom,tcsr-reg", 1, 0,
>>  					       &tcsr_args);
>>  	if (ret == -ENOENT)
>> @@ -1623,6 +1650,13 @@ static int qmp_usbc_parse_vls_clamp(struct qmp_usbc *qmp)
>>  
>>  	qmp->vls_clamp_reg = tcsr_args.args[0];
>>  
>> +	ret = of_parse_phandle_with_fixed_args(dev->of_node, "qcom,tcsr-reg", 1, 1,
>> +					       &tcsr_args);
>> +	if (ret == -ENOENT)
>> +		return 0;
>> +
>> +	qmp->dp_phy_mode_reg = tcsr_args.args[0];
>> +
>>  	return 0;
>>  }
>>  
>> @@ -1665,7 +1699,7 @@ static int qmp_usbc_probe(struct platform_device *pdev)
>>  	if (ret)
>>  		return ret;
>>  
>> -	ret = qmp_usbc_parse_vls_clamp(qmp);
>> +	ret = qmp_usbc_parse_tcsr(qmp);
>>  	if (ret)
>>  		return ret;
>>  
>>
>> -- 
>> 2.34.1
>>