Re: [PATCH 2/5] phy: qcom: qmp-pcie: Add multiple power-domains support

[Qiang Yu]

On Wed, Mar 04, 2026 at 02:46:22PM -0600, Bjorn Andersson wrote:
> On Wed, Mar 04, 2026 at 12:21:56AM -0800, Qiang Yu wrote:
> > The Glymur SoC's 3rd PCIe instance supports 8-lane mode using two PHYs in
> > a bifurcated configuration. Each PHY has its own power domain (phy_gdsc)
> > that must be powered on before initialization per hardware requirements.
> > 
> > Current PHY power management assumes a single power domain per PHY,
> > preventing proper setup for this dual-PHY scenario. Add support for
> > multiple power domains by using devm_pm_domain_attach_list() to attach
> > power domains manually, while maintaining compatibility with single
> > power domain PHYs.
> > 
> > Enable runtime PM to allow power domain control when the PCIe driver
> > calls phy_power_on/phy_power_off:
> > 
> 
> Enabling runtime PM seems like a separate change that impacts all
> existing targets, while adding multiple power domains should only affect
> the specific ones.
> 
> I'm not sure if it's too picky, but it would be nice to separate this -
> so that any issues that might arise can be bisected down to one or the
> other of these two changes?

The devm_pm_domain_attach_list() function calls genpd_dev_pm_attach_by_id,
which in turn calls __genpd_dev_pm_attach() with power_on=false. This
means the attached power domains are not powered on during attachment.

> 
> > - Single power domain: QMP PHY platform device directly attaches to
> >   power domain and controls it during runtime resume/suspend
> > - Multiple power domains: devm_pm_domain_attach_list() creates virtual
> >   devices as power domain suppliers, linked to the QMP PHY platform
> >   device as consumer
> > 
> > This ensures power domains are properly attached and turned on/off
> > for both single and multiple power domain configurations.
> > 
> > Signed-off-by: Qiang Yu <qiang.yu@xxxxxxxxxxxxxxxx>
> > ---
> >  drivers/phy/qualcomm/phy-qcom-qmp-pcie.c | 13 +++++++++++++
> >  1 file changed, 13 insertions(+)
> > 
> > diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c b/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c
> > index fed2fc9bb31108d51f88d34f3379c7744681f485..7369c291be51aa1ad7a330459dcb857f5a1988f6 100644
> > --- a/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c
> > +++ b/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c
> > @@ -17,6 +17,7 @@
> >  #include <linux/phy/pcie.h>
> >  #include <linux/phy/phy.h>
> >  #include <linux/platform_device.h>
> > +#include <linux/pm_domain.h>
> >  #include <linux/regmap.h>
> >  #include <linux/regulator/consumer.h>
> >  #include <linux/reset.h>
> > @@ -3334,6 +3335,8 @@ struct qmp_pcie {
> >  
> >  	struct clk_fixed_rate pipe_clk_fixed;
> >  	struct clk_fixed_rate aux_clk_fixed;
> > +
> > +	struct dev_pm_domain_list *pd_list;
> 
> This is just an pointer to the list allocated inside
> devm_pm_domain_attach_list(), as far as I can tell you don't need to
> keep this in the qmp_pcie struct - as you're not using it anyways.

Okay, will remove this part in next version.

- Qiang Yu
> 
> Regards,
> Bjorn
> 
> >  };
> >  
> >  static bool qphy_checkbits(const void __iomem *base, u32 offset, u32 val)
> > @@ -5348,6 +5351,16 @@ static int qmp_pcie_probe(struct platform_device *pdev)
> >  	WARN_ON_ONCE(!qmp->cfg->pwrdn_ctrl);
> >  	WARN_ON_ONCE(!qmp->cfg->phy_status);
> >  
> > +	ret = devm_pm_domain_attach_list(dev, NULL, &qmp->pd_list);
> > +	if (ret < 0 && ret != -EEXIST) {
> > +		dev_err(dev, "Failed to attach power domain\n");
> > +		return ret;
> > +	}
> > +
> > +	ret = devm_pm_runtime_enable(dev);
> > +	if (ret)
> > +		return ret;
> > +
> >  	ret = qmp_pcie_clk_init(qmp);
> >  	if (ret)
> >  		return ret;
> > 
> > -- 
> > 2.34.1
> >