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.
