[PATCH 1/3] clk: ignore of_device_ensure_probed error in clock lookup

[PATCH 1/3] clk: ignore of_device_ensure_probed error in clock lookup

[Lucas Stach]

For CLK_OF_DECLARE clocks there is no driver that is bound to the device,
so the lookup fails before even trying to find the provider, breaking
the parent_ready() logic used when initializing the declared providers.

Ignore the return code from of_device_ensure_probed to allow the lookup
to proceed as usual. If of_device_ensure_probed the lookup will also fail,
as no provider will be found.

Signed-off-by: Lucas Stach <dev@lynxeye.de>
---
 drivers/clk/clk.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index 189c9c62df5c..a1d1d7f1a467 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -643,11 +643,9 @@ struct clk *of_clk_get_from_provider(struct of_phandle_args *clkspec)
 {
 	struct of_clk_provider *provider;
 	struct clk *clk = ERR_PTR(-EPROBE_DEFER);
-	int ret;
 
-	ret = of_device_ensure_probed(clkspec->np);
-	if (ret)
-		return ERR_PTR(ret);
+	/* Ignore error, as CLK_OF_DECLARE clocks have no proper driver. */
+	of_device_ensure_probed(clkspec->np);
 
 	/* Check if we have such a provider in our array */
 	list_for_each_entry(provider, &of_clk_providers, link) {
-- 
2.31.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

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

[Lucas Stach]

Currently it is possible that the platform device for a nested PGC
domain is added before the parent PGC device is there, which leads to
-EPROBE_DEFER when probing the driver. With normal probe this isn't
an issue, as the probe will be retried. With deep-probe this is fatal,
as the PGC domain devices aren't probed from DT, but via registration
of platform devices from the GPC driver, so the usual deep-probe
approach to ensure the devices are probed before the lookup isn't
working in this case.

Make sure to register the PGC domain platform devices in the correct
order to avoid the EPROBE_DEFER altogether.

Signed-off-by: Lucas Stach <dev@lynxeye.de>
---
 drivers/soc/imx/gpcv2.c | 22 ++++++++++++++++++++--
 1 file changed, 20 insertions(+), 2 deletions(-)

diff --git a/drivers/soc/imx/gpcv2.c b/drivers/soc/imx/gpcv2.c
index bc373ecf40cc..8abeb15d0378 100644
--- a/drivers/soc/imx/gpcv2.c
+++ b/drivers/soc/imx/gpcv2.c
@@ -429,7 +429,7 @@ static int imx_gpcv2_probe(struct device_d *dev)
 	struct device_node *pgc_np, *np;
 	struct resource *res;
 	void __iomem *base;
-	int ret;	
+	int ret, pass = 0;
 
 	pgc_np = of_get_child_by_name(dev->device_node, "pgc");
 	if (!pgc_np) {
@@ -445,10 +445,23 @@ static int imx_gpcv2_probe(struct device_d *dev)
 
 	domain_data = of_device_get_match_data(dev);
 
+	/*
+	 * 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) {
-		struct device_d *pd_dev;
+		bool child_domain = of_property_read_bool(np, "power-domains");
 		struct imx_pgc_domain *domain;
+		struct device_d *pd_dev;
 		u32 domain_index;
+
+		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");
@@ -481,6 +494,11 @@ static int imx_gpcv2_probe(struct device_d *dev)
 			return ret;
 	}
 
+	if (pass == 0) {
+		pass++;
+		goto again;
+	}
+
 	return 0;
 }
 
-- 
2.31.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 3/3] ARM: mnt-reform: switch to deep-probe

[Lucas Stach]

Now that all drivers used on this platform properly handle deep-probe,
we can switch it on.

Signed-off-by: Lucas Stach <dev@lynxeye.de>
---
 arch/arm/boards/mnt-reform/board.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm/boards/mnt-reform/board.c b/arch/arm/boards/mnt-reform/board.c
index feb874c0a0ef..010690ecbd6c 100644
--- a/arch/arm/boards/mnt-reform/board.c
+++ b/arch/arm/boards/mnt-reform/board.c
@@ -5,6 +5,7 @@
 
 #include <bootsource.h>
 #include <common.h>
+#include <deep-probe.h>
 #include <init.h>
 #include <mach/bbu.h>
 
@@ -31,6 +32,7 @@ static const struct of_device_id mnt_reform_of_match[] = {
 	{ .compatible = "mntre,reform2"},
 	{ /* sentinel */ },
 };
+BAREBOX_DEEP_PROBE_ENABLE(mnt_reform_of_match);
 
 static struct driver_d mnt_reform_board_driver = {
 	.name = "board-mnt-reform",
-- 
2.31.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 1/3] clk: ignore of_device_ensure_probed error in clock lookup

[Ahmad Fatoum]

Hello Lucas,

On 16.01.22 22:32, Lucas Stach wrote:
> For CLK_OF_DECLARE clocks there is no driver that is bound to the device,
> so the lookup fails before even trying to find the provider, breaking
> the parent_ready() logic used when initializing the declared providers.
> 
> Ignore the return code from of_device_ensure_probed to allow the lookup
> to proceed as usual. If of_device_ensure_probed the lookup will also fail,
> as no provider will be found.

This seems to be an alternate fix for the issue addressed by:
bd516e38dd14 ("clk: handle CLK_OF_DECLARE in deep probe")

What do you think?

> 
> Signed-off-by: Lucas Stach <dev@lynxeye.de>
> ---
>  drivers/clk/clk.c | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
> index 189c9c62df5c..a1d1d7f1a467 100644
> --- a/drivers/clk/clk.c
> +++ b/drivers/clk/clk.c
> @@ -643,11 +643,9 @@ struct clk *of_clk_get_from_provider(struct of_phandle_args *clkspec)
>  {
>  	struct of_clk_provider *provider;
>  	struct clk *clk = ERR_PTR(-EPROBE_DEFER);
> -	int ret;
>  
> -	ret = of_device_ensure_probed(clkspec->np);
> -	if (ret)
> -		return ERR_PTR(ret);
> +	/* Ignore error, as CLK_OF_DECLARE clocks have no proper driver. */
> +	of_device_ensure_probed(clkspec->np);
>  
>  	/* Check if we have such a provider in our array */
>  	list_for_each_entry(provider, &of_clk_providers, link) {
> 


-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 1/3] clk: ignore of_device_ensure_probed error in clock lookup

[Lucas Stach]

Hi Ahmad,

Am Montag, dem 17.01.2022 um 06:12 +0100 schrieb Ahmad Fatoum:
> Hello Lucas,
> 
> On 16.01.22 22:32, Lucas Stach wrote:
> > For CLK_OF_DECLARE clocks there is no driver that is bound to the device,
> > so the lookup fails before even trying to find the provider, breaking
> > the parent_ready() logic used when initializing the declared providers.
> > 
> > Ignore the return code from of_device_ensure_probed to allow the lookup
> > to proceed as usual. If of_device_ensure_probed the lookup will also fail,
> > as no provider will be found.
> 
> This seems to be an alternate fix for the issue addressed by:
> bd516e38dd14 ("clk: handle CLK_OF_DECLARE in deep probe")
> 
> What do you think?

Yes, it seems to be aimed at the same issue. However, your fix in
bd516e38dd14 doesn't fix my specific issue, as we still don't get the
correct -EPROBE_DEFER error code for the logic in parent_ready() in
drivers/clk/clk.c to work.

I guess we could even revert your fix if we agree that ignoring the
return code here is the right thing to do? Attaching a dummy driver 
doesn't seem to be totally correct, as I think it isn't prohibited to
have both a CLK_OF_DECLARE clock controller and a real platform driver
for other uses attached to the same OF node. While I'm not aware of any
clock controllers where this would be the case, I have seen such
constructs with IRQ controllers.

Regards,
Lucas

> 
> > 
> > Signed-off-by: Lucas Stach <dev@lynxeye.de>
> > ---
> >  drivers/clk/clk.c | 6 ++----
> >  1 file changed, 2 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
> > index 189c9c62df5c..a1d1d7f1a467 100644
> > --- a/drivers/clk/clk.c
> > +++ b/drivers/clk/clk.c
> > @@ -643,11 +643,9 @@ struct clk *of_clk_get_from_provider(struct of_phandle_args *clkspec)
> >  {
> >  	struct of_clk_provider *provider;
> >  	struct clk *clk = ERR_PTR(-EPROBE_DEFER);
> > -	int ret;
> >  
> > -	ret = of_device_ensure_probed(clkspec->np);
> > -	if (ret)
> > -		return ERR_PTR(ret);
> > +	/* Ignore error, as CLK_OF_DECLARE clocks have no proper driver. */
> > +	of_device_ensure_probed(clkspec->np);
> >  
> >  	/* Check if we have such a provider in our array */
> >  	list_for_each_entry(provider, &of_clk_providers, link) {
> > 
> 
> 



_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 1/3] clk: ignore of_device_ensure_probed error in clock lookup

[Ahmad Fatoum]

Hi,

On 17.01.22 10:16, Lucas Stach wrote:
> Hi Ahmad,
> 
> Am Montag, dem 17.01.2022 um 06:12 +0100 schrieb Ahmad Fatoum:
>> Hello Lucas,
>>
>> On 16.01.22 22:32, Lucas Stach wrote:
>>> For CLK_OF_DECLARE clocks there is no driver that is bound to the device,
>>> so the lookup fails before even trying to find the provider, breaking
>>> the parent_ready() logic used when initializing the declared providers.
>>>
>>> Ignore the return code from of_device_ensure_probed to allow the lookup
>>> to proceed as usual. If of_device_ensure_probed the lookup will also fail,
>>> as no provider will be found.
>>
>> This seems to be an alternate fix for the issue addressed by:
>> bd516e38dd14 ("clk: handle CLK_OF_DECLARE in deep probe")
>>
>> What do you think?
> 
> Yes, it seems to be aimed at the same issue. However, your fix in
> bd516e38dd14 doesn't fix my specific issue, as we still don't get the
> correct -EPROBE_DEFER error code for the logic in parent_ready() in
> drivers/clk/clk.c to work.
> 
> I guess we could even revert your fix if we agree that ignoring the
> return code here is the right thing to do? Attaching a dummy driver 
> doesn't seem to be totally correct, as I think it isn't prohibited to
> have both a CLK_OF_DECLARE clock controller and a real platform driver
> for other uses attached to the same OF node. While I'm not aware of any
> clock controllers where this would be the case, I have seen such
> constructs with IRQ controllers.

Sounds logical. I'll test with my fix reverted and yours applied
and report back.

> 
> Regards,
> Lucas
> 
>>
>>>
>>> Signed-off-by: Lucas Stach <dev@lynxeye.de>
>>> ---
>>>  drivers/clk/clk.c | 6 ++----
>>>  1 file changed, 2 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
>>> index 189c9c62df5c..a1d1d7f1a467 100644
>>> --- a/drivers/clk/clk.c
>>> +++ b/drivers/clk/clk.c
>>> @@ -643,11 +643,9 @@ struct clk *of_clk_get_from_provider(struct of_phandle_args *clkspec)
>>>  {
>>>  	struct of_clk_provider *provider;
>>>  	struct clk *clk = ERR_PTR(-EPROBE_DEFER);
>>> -	int ret;
>>>  
>>> -	ret = of_device_ensure_probed(clkspec->np);
>>> -	if (ret)
>>> -		return ERR_PTR(ret);
>>> +	/* Ignore error, as CLK_OF_DECLARE clocks have no proper driver. */
>>> +	of_device_ensure_probed(clkspec->np);
>>>  
>>>  	/* Check if we have such a provider in our array */
>>>  	list_for_each_entry(provider, &of_clk_providers, link) {
>>>
>>
>>
> 
> 
> 


-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 1/3] clk: ignore of_device_ensure_probed error in clock lookup

[Ahmad Fatoum]

On 17.01.22 11:20, Ahmad Fatoum wrote:
> On 17.01.22 10:16, Lucas Stach wrote:
>> Hi Ahmad,
>>
>> Am Montag, dem 17.01.2022 um 06:12 +0100 schrieb Ahmad Fatoum:
>>> Hello Lucas,
>>>
>>> On 16.01.22 22:32, Lucas Stach wrote:
>>>> For CLK_OF_DECLARE clocks there is no driver that is bound to the device,
>>>> so the lookup fails before even trying to find the provider, breaking
>>>> the parent_ready() logic used when initializing the declared providers.
>>>>
>>>> Ignore the return code from of_device_ensure_probed to allow the lookup
>>>> to proceed as usual. If of_device_ensure_probed the lookup will also fail,
>>>> as no provider will be found.
>>>
>>> This seems to be an alternate fix for the issue addressed by:
>>> bd516e38dd14 ("clk: handle CLK_OF_DECLARE in deep probe")
>>>
>>> What do you think?
>>
>> Yes, it seems to be aimed at the same issue. However, your fix in
>> bd516e38dd14 doesn't fix my specific issue, as we still don't get the
>> correct -EPROBE_DEFER error code for the logic in parent_ready() in
>> drivers/clk/clk.c to work.
>>
>> I guess we could even revert your fix if we agree that ignoring the
>> return code here is the right thing to do? Attaching a dummy driver 
>> doesn't seem to be totally correct, as I think it isn't prohibited to
>> have both a CLK_OF_DECLARE clock controller and a real platform driver
>> for other uses attached to the same OF node. While I'm not aware of any
>> clock controllers where this would be the case, I have seen such
>> constructs with IRQ controllers.
> 
> Sounds logical. I'll test with my fix reverted and yours applied
> and report back.

Tested-by: Ahmad Fatoum <a.fatoum@pengutronix.de>

"clk: handle CLK_OF_DECLARE in deep probe" can be dropped again.

> 
>>
>> Regards,
>> Lucas
>>
>>>
>>>>
>>>> Signed-off-by: Lucas Stach <dev@lynxeye.de>
>>>> ---
>>>>  drivers/clk/clk.c | 6 ++----
>>>>  1 file changed, 2 insertions(+), 4 deletions(-)
>>>>
>>>> diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
>>>> index 189c9c62df5c..a1d1d7f1a467 100644
>>>> --- a/drivers/clk/clk.c
>>>> +++ b/drivers/clk/clk.c
>>>> @@ -643,11 +643,9 @@ struct clk *of_clk_get_from_provider(struct of_phandle_args *clkspec)
>>>>  {
>>>>  	struct of_clk_provider *provider;
>>>>  	struct clk *clk = ERR_PTR(-EPROBE_DEFER);
>>>> -	int ret;
>>>>  
>>>> -	ret = of_device_ensure_probed(clkspec->np);
>>>> -	if (ret)
>>>> -		return ERR_PTR(ret);
>>>> +	/* Ignore error, as CLK_OF_DECLARE clocks have no proper driver. */
>>>> +	of_device_ensure_probed(clkspec->np);
>>>>  
>>>>  	/* Check if we have such a provider in our array */
>>>>  	list_for_each_entry(provider, &of_clk_providers, link) {
>>>>
>>>
>>>
>>
>>
>>
> 
> 


-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 1/3] clk: ignore of_device_ensure_probed error in clock lookup

[Sascha Hauer]

On Sun, Jan 16, 2022 at 10:32:19PM +0100, Lucas Stach wrote:
> For CLK_OF_DECLARE clocks there is no driver that is bound to the device,
> so the lookup fails before even trying to find the provider, breaking
> the parent_ready() logic used when initializing the declared providers.
> 
> Ignore the return code from of_device_ensure_probed to allow the lookup
> to proceed as usual. If of_device_ensure_probed the lookup will also fail,
> as no provider will be found.
> 
> Signed-off-by: Lucas Stach <dev@lynxeye.de>
> ---
>  drivers/clk/clk.c | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)

Applied, thanks

Sascha

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox