Re: [PATCH v4 6/9] phy: qualcomm: qmp-combo: Extract common DP PHY init sequence

[Nabige Aala <nabige.aala@oss.qualcomm.com>]

On 9/11/2026 11:00 AM, Manivannan Sadhasivam wrote:
> On Tue, Sep 08, 2026 at 07:30:59PM +0530, Nabige Aala wrote:
>> From: Ritesh Kumar <ritesh.kumar@oss.qualcomm.com>
>>
>> Extract the common DP PHY initialization sequence shared between
>> qmp_v456_configure_dp_phy() and qmp_v8_configure_dp_phy() into a new
>> qmp_combo_configure_dp_phy_common() function.
>>
>> The common sequence covers:
>> - Validation that dp_aux_cfg2 is configured for the platform
>> - Writing dp_phy_cfg1 and dp_aux_cfg2 hardware-specific register values
>> - Calling configure_dp_mode() for TypeC lane orientation
>> - Programming AUX_CFG1, TX lane control registers
>> - Invoking configure_dp_clocks() callback
>> - PHY_CFG reset/enable sequence
>> - Polling COM_C_READY_STATUS and COM_CMN_STATUS for PLL lock
>>
>> Refactor qmp_v456_configure_dp_phy() to call the common function,
>> removing the duplicated initialization code. Update
>> qmp_v8_configure_dp_phy() to call qmp_combo_configure_dp_phy_common()
>> directly instead of going through qmp_v456_configure_dp_phy().
>>
>> Signed-off-by: Ritesh Kumar <ritesh.kumar@oss.qualcomm.com>
>> Signed-off-by: Mahadevan P <mahadevan.p@oss.qualcomm.com>
>> Signed-off-by: Nabige Aala <nabige.aala@oss.qualcomm.com>
> One comment below. With that addressed,
>
> Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
>
>> ---
>>   drivers/phy/qualcomm/phy-qcom-qmp-combo.c | 99 ++++++++++++++++++-------------
>>   1 file changed, 59 insertions(+), 40 deletions(-)
>>
>> diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
>> index 0ec382aebe2b..79006469f3d4 100644
>> --- a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
>> +++ b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
>> @@ -3459,6 +3459,7 @@ static const struct qmp_phy_cfg glymur_usb3dpphy_cfg = {
>>   	.configure_dp_tx	= qmp_v4_configure_dp_tx,
>>   	.configure_dp_clocks	= qmp_v8_configure_dp_clocks,
>>   	.configure_dp_phy	= qmp_v8_configure_dp_phy,
>> +
>>   	.dp_aux_cfg2		= QSERDES_DP_PHY_AUX_CFG2_V8,
>>   	.dp_phy_cfg1		= QSERDES_DP_PHY_CFG1_V8,
>>   	.dp_mode_ignore_reverse	= true,
>> @@ -3766,6 +3767,62 @@ static void qmp_v4_dp_aux_init(struct qmp_combo *qmp)
>>   	       qmp->dp_dp_phy + QSERDES_V4_DP_PHY_AUX_INTERRUPT_MASK);
>>   }
>>   
>> +static int qmp_combo_configure_dp_phy_common(struct qmp_combo *qmp)
>> +{
>> +	const struct qmp_phy_cfg *cfg = qmp->cfg;
>> +	u32 status;
>> +	int ret;
>> +
>> +	if (!cfg->dp_aux_cfg2) {
>> +		dev_err(qmp->dev, "DP AUX CFG2 value not configured\n");
>> +		return -EINVAL;
>> +	}
>> +
>> +	writel(cfg->dp_phy_cfg1, qmp->dp_dp_phy + QSERDES_DP_PHY_CFG_1);
>> +
>> +	qmp_combo_configure_dp_mode(qmp);
>> +
>> +	writel(0x13, qmp->dp_dp_phy + QSERDES_DP_PHY_AUX_CFG1);
>> +	writel(cfg->dp_aux_cfg2, qmp->dp_dp_phy + QSERDES_DP_PHY_AUX_CFG2);
>> +
>> +	writel(0x05, qmp->dp_dp_phy + QSERDES_V4_DP_PHY_TX0_TX1_LANE_CTL);
>> +	writel(0x05, qmp->dp_dp_phy + QSERDES_V4_DP_PHY_TX2_TX3_LANE_CTL);
>> +
>> +	ret = qmp->cfg->configure_dp_clocks(qmp);
>> +	if (ret)
>> +		return ret;
>> +
>> +	writel(0x01, qmp->dp_dp_phy + QSERDES_DP_PHY_CFG);
>> +	writel(0x05, qmp->dp_dp_phy + QSERDES_DP_PHY_CFG);
>> +	writel(0x01, qmp->dp_dp_phy + QSERDES_DP_PHY_CFG);
>> +	writel(0x09, qmp->dp_dp_phy + QSERDES_DP_PHY_CFG);
>> +
>> +	writel(0x20, qmp->dp_serdes + cfg->regs[QPHY_COM_RESETSM_CNTRL]);
>> +
>> +	if (readl_poll_timeout(qmp->dp_serdes + cfg->regs[QPHY_COM_C_READY_STATUS],
>> +			status,
>> +			((status & BIT(0)) > 0),
>> +			500,
>> +			10000))
>> +		return -ETIMEDOUT;
> Didn't I suggest that you should just return what readl_poll_timeout() returns?

Ack, I will update in the next revision.

Regards,

Nabige

>
> - Mani
>

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy