clk: ignore of_device_ensure_probed error in clock lookup
Subsystems (3)
PATCH 1/3 clk: ignore of_device_ensure_probed error in clock lookup
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.
--- drivers/clk/clk.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-)
drivers/clk/clk.c +2 -4
@@ -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) {
Signature · 7 lines
2.31.1 _______________________________________________ barebox mailing list [email protected] http://lists.infradead.org/mailman/listinfo/barebox
PATCH 2/3 soc: imx: gpcv2: split PGC domain probe in two passes
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.
--- drivers/soc/imx/gpcv2.c | 22 ++++++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-)
drivers/soc/imx/gpcv2.c +20 -2
@@ -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 ; }
Signature · 7 lines
2.31.1 _______________________________________________ barebox mailing list [email protected] http://lists.infradead.org/mailman/listinfo/barebox
PATCH 3/3 ARM: mnt-reform: switch to deep-probe
In reply to Lucas StachNow that all drivers used on this platform properly handle deep-probe, we can switch it on.
--- arch/arm/boards/mnt-reform/board.c | 2 ++ 1 file changed, 2 insertions(+)
arch/arm/boards/mnt-reform/board.c +2 -0
@@ -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" ,
Signature · 7 lines
2.31.1 _______________________________________________ barebox mailing list [email protected] http://lists.infradead.org/mailman/listinfo/barebox
Hello Lucas,
Quoted 7 lines · Lucas Stach
For
CLK_OF_DECLAREclocks there is no driver that is bound to the device, so the lookup fails before even trying to find the provider, breaking theparent_ready()logic used when initializing the declared providers.Ignore the return code from
of_device_ensure_probedto allow the lookup to proceed as usual. Ifof_device_ensure_probedthe 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?
Quoted 25 lines
Signed-off-by Lucas Stach <[email protected]>--- drivers/clk/clk.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-)drivers/clk/clk.c +2
-4@@ -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) {
Signature · 9 lines
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 [email protected] http://lists.infradead.org/mailman/listinfo/barebox
Hi Ahmad,
Am Montag, dem 17.01.2022 um 06:12 +0100 schrieb Ahmad Fatoum:
Quoted 7 lines
Hello Lucas,
Quoted 4 lines · Lucas Stach
For
CLK_OF_DECLAREclocks there is no driver that is bound to the device, so the lookup fails before even trying to find the provider, breaking theparent_ready()logic used when initializing the declared providers.
Quoted 8 lines
Quoted 3 lines
Ignore the return code from
of_device_ensure_probedto allow the lookup to proceed as usual. Ifof_device_ensure_probedthe lookup will also fail, as no provider will be found.This seems to be an alternate fix for the issue addressed by:
bd516e38dd14("clk: handleCLK_OF_DECLAREin 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
Quoted 23 lines
Quoted 23 lines
Signed-off-by Lucas Stach <[email protected]>--- drivers/clk/clk.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-)drivers/clk/clk.c +2
-4@@ -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);
Quoted 5 lines
drivers/clk/clk.c struct clk *of_clk_get_from_provider(struct of_phandle_args *clkspec) · line 651
Quoted 3 lines
drivers/clk/clk.c +0
-0/* Check if we have such a provider in our array */ list_for_each_entry (provider, &of_clk_providers, link) {
_______________________________________________
barebox mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/barebox
Hi,
Quoted 24 lines · Lucas Stach
Hi Ahmad,
Am Montag, dem 17.01.2022 um 06:12 +0100 schrieb Ahmad Fatoum:
Quoted 7 lines
Hello Lucas,
Quoted 4 lines · Lucas Stach
For
CLK_OF_DECLAREclocks there is no driver that is bound to the device, so the lookup fails before even trying to find the provider, breaking theparent_ready()logic used when initializing the declared providers.Quoted 8 lines
Quoted 3 lines
Ignore the return code from
of_device_ensure_probedto allow the lookup to proceed as usual. Ifof_device_ensure_probedthe lookup will also fail, as no provider will be found.This seems to be an alternate fix for the issue addressed by:
bd516e38dd14("clk: handleCLK_OF_DECLAREin deep probe")What do you think?
Yes, it seems to be aimed at the same issue. However, your fix in
bd516e38dd14doesn't fix my specific issue, as we still don't get the correct -EPROBE_DEFER error code for the logic inparent_ready()indrivers/clk/clk.cto work.
Quoted 7 lines
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_DECLAREclock 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.
Quoted 35 lines
Regards,
LucasQuoted 28 lines
Quoted 26 lines
Signed-off-by Lucas Stach <[email protected]>--- drivers/clk/clk.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-)drivers/clk/clk.c +2
-4@@ -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) {
Signature · 9 lines
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 [email protected] http://lists.infradead.org/mailman/listinfo/barebox
Ahmad Fatoum
Tested-byQuoted 33 lines · Ahmad Fatoum
Quoted 24 lines · Lucas Stach
Hi Ahmad,
Am Montag, dem 17.01.2022 um 06:12 +0100 schrieb Ahmad Fatoum:
Quoted 7 lines
Hello Lucas,
Quoted 4 lines · Lucas Stach
For
CLK_OF_DECLAREclocks there is no driver that is bound to the device, so the lookup fails before even trying to find the provider, breaking theparent_ready()logic used when initializing the declared providers.Quoted 8 lines
Quoted 3 lines
Ignore the return code from
of_device_ensure_probedto allow the lookup to proceed as usual. Ifof_device_ensure_probedthe lookup will also fail, as no provider will be found.This seems to be an alternate fix for the issue addressed by:
bd516e38dd14("clk: handleCLK_OF_DECLAREin deep probe")What do you think?
Yes, it seems to be aimed at the same issue. However, your fix in
bd516e38dd14doesn't fix my specific issue, as we still don't get the correct -EPROBE_DEFER error code for the logic inparent_ready()indrivers/clk/clk.cto work.Quoted 7 lines
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_DECLAREclock 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.
Quoted 2 lines
Sounds logical. I'll test with my fix reverted and yours applied and report back.
"clk: handle CLK_OF_DECLARE in deep probe" can be dropped again.
Quoted 38 lines
Quoted 36 lines
Regards,
LucasQuoted 28 lines
Quoted 26 lines
Signed-off-by Lucas Stach <[email protected]>--- drivers/clk/clk.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-)drivers/clk/clk.c +2
-4@@ -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) {
Signature · 9 lines
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 [email protected] http://lists.infradead.org/mailman/listinfo/barebox
Sascha Hauer
AppliedQuoted 4 lines · Lucas Stach
For
CLK_OF_DECLAREclocks there is no driver that is bound to the device, so the lookup fails before even trying to find the provider, breaking theparent_ready()logic used when initializing the declared providers.
Quoted 8 lines
Ignore the return code from
of_device_ensure_probedto allow the lookup to proceed as usual. Ifof_device_ensure_probedthe lookup will also fail, as no provider will be found.Signed-off-by Lucas Stach <[email protected]>--- drivers/clk/clk.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-)
Applied, thanks
Sascha
Signature · 9 lines
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 [email protected] http://lists.infradead.org/mailman/listinfo/barebox