Quoting Johan Hovold (2023-01-13 00:07:32)
On Thu, Jan 12, 2023 at 04:54:04PM -0800, Stephen Boyd wrote:
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 @@ -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.
Good point. I don't have PCIe or UFS PHYs so I missed this.
} 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)
I changed this to by 'cfg->type != PHY_TYPE_DP'
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.
I worry that future backports will be complicated if they use 'usb_phy'. Maybe I'll just leave this as is.