| Message ID | 20220826191305.3215706-3-l.stach@pengutronix.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/3] soc: imx: imx8mp-blk-ctrl: don't set power device name in | expand |
> 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. Do we still need this fix? Thanks, Peng. > Signed-off-by: Lucas Stach <l.stach@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
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@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 >
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; }
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()") Signed-off-by: Lucas Stach <l.stach@pengutronix.de> --- drivers/soc/imx/gpcv2.c | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-)