Re: [PATCH v2 3/7] phy: qcom: qmp-combo: introduce QPHY_MODE

[Neil Armstrong <neil.armstrong@linaro.org>]

On 04/06/2025 13:25, Udipto Goswami wrote:
> 
> 
> On 5/27/2024 2:12 PM, Neil Armstrong wrote:
>> Introduce an enum for the QMP Combo PHY modes, use it in the
>> QMP commmon phy init function and default to COMBO mode.
>>
>> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
>> ---
>>   drivers/phy/qualcomm/phy-qcom-qmp-combo.c | 41 +++++++++++++++++++++++++++----
>>   1 file changed, 36 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
>> index 183cd9cd1884..788e4c05eaf2 100644
>> --- a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
>> +++ b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
>> @@ -61,6 +61,12 @@
>>   #define PHY_INIT_COMPLETE_TIMEOUT        10000
>> +enum qphy_mode {
>> +    QPHY_MODE_COMBO = 0,
> 
> Hi Neil,
> 
> I have a doubt here, shouldn't this be aligned with what typec_altmode has ?
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/linux/usb/typec_altmode.h?h=v6.15#n113
> 
> This patch marks COMBO mode as 0
> when the mux_set when called from pmic_glink_altmode.c
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/soc/qcom/pmic_glink_altmode.c?h=v6.15#n160
> 
> the state will start from 2 if i'm not wrong ?
> 
> For the similar implmentation I referring to fsa4480.c which seems to be using the enums from typec_altmode.c therefore asking.

Those enums are local to the driver, not related to altmode bits at all,
they represent the 3 possible states of the combo phy.

Neil

> 
> 
> Thanks,
> -Udipto
> 
>> +    QPHY_MODE_DP_ONLY,
>> +    QPHY_MODE_USB_ONLY,
>> +};
>> +
>>   /* set of registers with offsets different per-PHY */
>>   enum qphy_reg_layout {
>>       /* PCS registers */
>> @@ -1503,6 +1509,7 @@ struct qmp_combo {
>>       struct mutex phy_mutex;
>>       int init_count;
>> +    enum qphy_mode init_mode;
>>       struct phy *usb_phy;
>>       enum phy_mode mode;
>> @@ -2589,12 +2596,33 @@ static int qmp_combo_com_init(struct qmp_combo *qmp, bool force)
>>       if (qmp->orientation == TYPEC_ORIENTATION_REVERSE)
>>           val |= SW_PORTSELECT_VAL;
>>       writel(val, com + QPHY_V3_DP_COM_TYPEC_CTRL);
>> -    writel(USB3_MODE | DP_MODE, com + QPHY_V3_DP_COM_PHY_MODE_CTRL);
>> -    /* bring both QMP USB and QMP DP PHYs PCS block out of reset */
>> -    qphy_clrbits(com, QPHY_V3_DP_COM_RESET_OVRD_CTRL,
>> -            SW_DPPHY_RESET_MUX | SW_DPPHY_RESET |
>> -            SW_USB3PHY_RESET_MUX | SW_USB3PHY_RESET);
>> +    switch (qmp->init_mode) {
>> +    case QPHY_MODE_COMBO:
>> +        writel(USB3_MODE | DP_MODE, com + QPHY_V3_DP_COM_PHY_MODE_CTRL);
>> +
>> +        /* bring both QMP USB and QMP DP PHYs PCS block out of reset */
>> +        qphy_clrbits(com, QPHY_V3_DP_COM_RESET_OVRD_CTRL,
>> +                SW_DPPHY_RESET_MUX | SW_DPPHY_RESET |
>> +                SW_USB3PHY_RESET_MUX | SW_USB3PHY_RESET);
>> +        break;
>> +
>> +    case QPHY_MODE_DP_ONLY:
>> +        writel(DP_MODE, com + QPHY_V3_DP_COM_PHY_MODE_CTRL);
>> +
>> +        /* bring QMP DP PHY PCS block out of reset */
>> +        qphy_clrbits(com, QPHY_V3_DP_COM_RESET_OVRD_CTRL,
>> +                SW_DPPHY_RESET_MUX | SW_DPPHY_RESET);
>> +        break;
>> +
>> +    case QPHY_MODE_USB_ONLY:
>> +        writel(USB3_MODE, com + QPHY_V3_DP_COM_PHY_MODE_CTRL);
>> +
>> +        /* bring QMP USB PHY PCS block out of reset */
>> +        qphy_clrbits(com, QPHY_V3_DP_COM_RESET_OVRD_CTRL,
>> +                SW_USB3PHY_RESET_MUX | SW_USB3PHY_RESET);
>> +        break;
>> +    }
>>       qphy_clrbits(com, QPHY_V3_DP_COM_SWI_CTRL, 0x03);
>>       qphy_clrbits(com, QPHY_V3_DP_COM_SW_RESET, SW_RESET);
>> @@ -3603,6 +3631,9 @@ static int qmp_combo_probe(struct platform_device *pdev)
>>       if (ret)
>>           goto err_node_put;
>> +    /* Set PHY_MODE as combo by default */
>> +    qmp->init_mode = QPHY_MODE_COMBO;
>> +
>>       qmp->usb_phy = devm_phy_create(dev, usb_np, &qmp_combo_usb_phy_ops);
>>       if (IS_ERR(qmp->usb_phy)) {
>>           ret = PTR_ERR(qmp->usb_phy);
>>
>