[PATCH 3/3] soc: imx: gpcv2: split PGC domain probe in two passes

[Lucas Stach]

Am Montag, dem 29.08.2022 um 01:55 +0000 schrieb Peng Fan:
> > Subject: [PATCH 3/3] soc: imx: gpcv2: split PGC domain probe in two passes
> > 
> > Since 5a46079a9645 ("PM: domains: Delete usage of
> > driver_deferred_probe_check_state()")
> > power domain consumers attached by the driver core do not support probe
> > deferral anymore, as it is assumed that they are only probed after the
> > provider is present, as a devlink should have been established between the
> > two.
> > 
> > With the GPCv2 and its slightly unusual mix between platform devices and
> > DT description for the PGC domains, devlink fails to add the neccessary
> > probe dependency. Now that probe deferral is not an option anymore, the
> > domain drivers for nested GPC domains simply fail to probe, leaving parts of
> > the SoC unusable. Rather than trying to teach devlink about our one-off
> > usage of DT and platform devices, just split the registration of the nested
> > power domains into a second pass, so that we never need any dependency
> > handling.
> > 
> > Fixes: 5a46079a9645 ("PM: domains: Delete usage of
> > driver_deferred_probe_check_state()")
> 
> This regression has been fixed, the patch author reverted that patch.
> 
Thanks, didn't know this.

> Do we still need this fix?
> 
We certainly do not need it as a fix then. Depending on how people feel
about the code change, I would still like to land it, as we can avoid a
probe deferral cycle for some GPC power domains this way and thus avoid
unnecessary work during kernel boot.

Regards,
Lucas

> Thanks,
> Peng.
> > Signed-off-by: Lucas Stach <l.stach at pengutronix.de>
> > ---
> >  drivers/soc/imx/gpcv2.c | 19 ++++++++++++++++++-
> >  1 file changed, 18 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/soc/imx/gpcv2.c b/drivers/soc/imx/gpcv2.c index
> > 6383a4edc360..d1bbadbcb034 100644
> > --- a/drivers/soc/imx/gpcv2.c
> > +++ b/drivers/soc/imx/gpcv2.c
> > @@ -1446,7 +1446,7 @@ static int imx_gpcv2_probe(struct
> > platform_device *pdev)
> >  	struct device_node *pgc_np, *np;
> >  	struct regmap *regmap;
> >  	void __iomem *base;
> > -	int ret;
> > +	int ret, pass = 0;
> > 
> >  	pgc_np = of_get_child_by_name(dev->of_node, "pgc");
> >  	if (!pgc_np) {
> > @@ -1465,7 +1465,16 @@ static int imx_gpcv2_probe(struct
> > platform_device *pdev)
> >  		return ret;
> >  	}
> > 
> > +	/*
> > +	 * Run two passes for the registration of the PGC domain platform
> > +	 * devices: first all devices that are not part of a power-domain
> > +	 * themselves, then all the others. This avoids -EPROBE_DEFER being
> > +	 * returned for nested domains, that need their parent PGC domains
> > +	 * to be present on probe.
> > +	 */
> > +again:
> >  	for_each_child_of_node(pgc_np, np) {
> > +		bool child_domain = of_property_read_bool(np, "power-
> > domains");
> >  		struct platform_device *pd_pdev;
> >  		struct imx_pgc_domain *domain;
> >  		u32 domain_index;
> > @@ -1473,6 +1482,9 @@ static int imx_gpcv2_probe(struct
> > platform_device *pdev)
> >  		if (!of_device_is_available(np))
> >  			continue;
> > 
> > +		if ((pass == 0 && child_domain) || (pass == 1
> > && !child_domain))
> > +			continue;
> > +
> >  		ret = of_property_read_u32(np, "reg", &domain_index);
> >  		if (ret) {
> >  			dev_err(dev, "Failed to read 'reg' property\n"); @@
> > -1522,6 +1534,11 @@ static int imx_gpcv2_probe(struct platform_device
> > *pdev)
> >  		}
> >  	}
> > 
> > +	if (pass == 0) {
> > +		pass++;
> > +		goto again;
> > +	}
> > +
> >  	return 0;
> >  }
> > 
> > --
> > 2.30.2
>