On Thu, Jan 12, 2023 at 04:54:04PM -0800, Stephen Boyd wrote:
> From: Johan Hovold <johan+linaro@xxxxxxxxxx>
>
> commit 7a7d86d14d073dfa3429c550667a8e78b99edbd4 upstream.
>
> The PHY is powered on during phy-init by setting the SW_PWRDN bit in the
> 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@xxxxxxxxxxxxxxx # 5.10
> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@xxxxxxxxxx>
> Signed-off-by: Johan Hovold <johan+linaro@xxxxxxxxxx>
> Link: https://lore.kernel.org/r/20221114081346.5116-5-johan+linaro@xxxxxxxxxx
> Signed-off-by: Vinod Koul <vkoul@xxxxxxxxxx>
> [swboyd@xxxxxxxxxxxx: Backport to pre-split driver]
> Signed-off-by: Stephen Boyd <swboyd@xxxxxxxxxxxx>
> ---
> drivers/phy/qualcomm/phy-qcom-qmp.c | 21 ++++++++++++++-------
> 1 file changed, 14 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/phy/qualcomm/phy-qcom-qmp.c b/drivers/phy/qualcomm/phy-qcom-qmp.c
> index c6f860ce3d99..9fda6d283f20 100644
> --- a/drivers/phy/qualcomm/phy-qcom-qmp.c
> +++ b/drivers/phy/qualcomm/phy-qcom-qmp.c
> @@ -2919,6 +2919,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;
> @@ -4554,7 +4555,7 @@ static int qcom_qmp_phy_com_init(struct qmp_phy *qphy)
> struct qcom_qmp *qmp = qphy->qmp;
> const struct qmp_phy_cfg *cfg = qphy->cfg;
> void __iomem *serdes = qphy->serdes;
> - void __iomem *pcs = qphy->pcs;
> + struct qmp_phy *usb_phy = qmp->usb_phy;
> void __iomem *dp_com = qmp->dp_com;
> int ret, i;
>
> @@ -4620,13 +4621,13 @@ static int qcom_qmp_phy_com_init(struct qmp_phy *qphy)
> qphy_setbits(serdes, cfg->regs[QPHY_COM_POWER_DOWN_CONTROL],
> SW_PWRDN);
> } else {
> - if (cfg->regs[QPHY_PCS_POWER_DOWN_CONTROL])
> - qphy_setbits(pcs,
> - cfg->regs[QPHY_PCS_POWER_DOWN_CONTROL],
> - cfg->pwrdn_ctrl);
> + if (usb_phy->cfg->regs[QPHY_PCS_POWER_DOWN_CONTROL])
> + qphy_setbits(usb_phy->pcs,
> + usb_phy->cfg->regs[QPHY_PCS_POWER_DOWN_CONTROL],
> + usb_phy->cfg->pwrdn_ctrl);
> else
> - qphy_setbits(pcs, QPHY_POWER_DOWN_CONTROL,
> - cfg->pwrdn_ctrl);
> + qphy_setbits(usb_phy->pcs, QPHY_POWER_DOWN_CONTROL,
> + usb_phy->cfg->pwrdn_ctrl);
This bit looks like it requires some more work in order not to break
PCIe and UFS PHYs, which lack the USB registers.
> }
>
> mutex_unlock(&qmp->phy_mutex);
> @@ -5769,6 +5770,9 @@ static int qcom_qmp_phy_probe(struct platform_device *pdev)
> goto err_node_put;
> }
>
> + if (cfg->type == PHY_TYPE_USB3)
> + qmp->usb_phy = qmp->phys[id];
> +
> /*
> * Register the pipe clock provided by phy.
> * See function description to see details of this pipe clock.
> @@ -5791,6 +5795,9 @@ static int qcom_qmp_phy_probe(struct platform_device *pdev)
> id++;
> }
>
> + if (!qmp->usb_phy)
> + return -EINVAL;
> +
This will break probe of PCIe and UFS PHYs unless you only check this
for combo PHYs.
Perhaps you can rename the usb_phy pointer to something more generic and
set it also for PCIe and UFS so that it always points to the register
block that should be used for QPHY_PCS_POWER_DOWN_CONTROL.
> phy_provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
> if (!IS_ERR(phy_provider))
> dev_info(dev, "Registered Qcom-QMP phy\n");
Johan
