Re: [PATCH 4/6] phy: qcom-qmp-combo: fix broken power on

Dmitry Baryshkov
2022-11-12T06:15:53Z

On 11/11/2022 11:42, Johan Hovold wrote:
> The PHY is powered on during phy-init by setting the SW_PRWDN bit in the

Nit: SW_PWRDN

> COM_POWER_DOWN_CTRL register and then setting the same bit in the in the
> PCS_POWER_DOWN_CONTROL register that belongs to the USB part of the
> PHY.
>
> Currently, whether power on succeeds depends on probe order and having
> the USB part of the PHY be initialised first. In case the DP part of the
> PHY is instead initialised first, the intended power on of the USB block
> results in a corrupted DP_PHY register (e.g. DP_PHY_AUX_CFG8).
>
> Add a pointer to the USB part of the PHY to the driver data and use that
> to power on the PHY also if the DP part of the PHY is initialised first.
>
> Fixes: 52e013d0bffa ("phy: qcom-qmp: Add support for DP in USB3+DP combo phy")
> Cc: stable@vger.kernel.org # 5.10
> Signed-off-by: Johan Hovold <johan+linaro@kernel.org>

I can only hope that at some point in your cleanup this hack is going to
be removed.
Nevertheless, I don't see a good way to do this at this moment. Thus:

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>

> ---
> drivers/phy/qualcomm/phy-qcom-qmp-combo.c | 11 +++++++++--
> 1 file changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
> index 40c25a0ead23..17707f68d482 100644
> --- a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
> +++ b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
> @@ -932,6 +932,7 @@ struct qcom_qmp {
> struct regulator_bulk_data *vregs;
>
> struct qmp_phy **phys;
> + struct qmp_phy *usb_phy;
>
> struct mutex phy_mutex;
> int init_count;
> @@ -1911,7 +1912,7 @@ static int qmp_combo_com_init(struct qmp_phy *qphy)
> {
> struct qcom_qmp *qmp = qphy->qmp;
> const struct qmp_phy_cfg *cfg = qphy->cfg;
> - void __iomem *pcs = qphy->pcs;
> + struct qmp_phy *usb_phy = qmp->usb_phy;
> void __iomem *dp_com = qmp->dp_com;
> int ret;
>
> @@ -1963,7 +1964,8 @@ static int qmp_combo_com_init(struct qmp_phy *qphy)
> qphy_clrbits(dp_com, QPHY_V3_DP_COM_SWI_CTRL, 0x03);
> qphy_clrbits(dp_com, QPHY_V3_DP_COM_SW_RESET, SW_RESET);
>
> - qphy_setbits(pcs, cfg->regs[QPHY_PCS_POWER_DOWN_CONTROL], SW_PWRDN);
> + qphy_setbits(usb_phy->pcs, usb_phy->cfg->regs[QPHY_PCS_POWER_DOWN_CONTROL],
> + SW_PWRDN);
>
> mutex_unlock(&qmp->phy_mutex);
>
> @@ -2831,6 +2833,8 @@ static int qmp_combo_probe(struct platform_device *pdev)
> goto err_node_put;
> }
>
> + qmp->usb_phy = qmp->phys[id];
> +
> /*
> * Register the pipe clock provided by phy.
> * See function description to see details of this pipe clock.
> @@ -2846,6 +2850,9 @@ static int qmp_combo_probe(struct platform_device *pdev)
> id++;
> }
>
> + if (!qmp->usb_phy)
> + return -EINVAL;
> +
> phy_provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
>
> return PTR_ERR_OR_ZERO(phy_provider);

--
With best wishes
Dmitry