Hi, Stephen, On 2/9/21 9:55 AM, Stephen Boyd wrote: > EXTERNAL EMAIL: Do not click links or open attachments unless you know the > content is safe > > Quoting Saravana Kannan (2021-01-28 09:01:41) >> On Thu, Jan 28, 2021 at 2:45 AM Tudor Ambarus >> <tudor.amba...@microchip.com> wrote: >>> >>> The sama5d2 requires the clock provider initialized before timers. >>> We can't use a platform driver for the sama5d2-pmc driver, as the >>> platform_bus_init() is called later on, after time_init(). >>> >>> As fw_devlink considers only devices, it does not know that the >>> pmc is ready. Hence probing of devices that depend on it fail: >>> probe deferral - supplier f0014000.pmc not ready >>> >>> Fix this by setting the OF_POPULATED flag for the sama5d2_pmc >>> device node after successful setup. This will make >>> of_link_to_phandle() ignore the sama5d2_pmc device node as a >>> dependency, and consumer devices will be probed again. >>> >>> Fixes: e590474768f1cc04 ("driver core: Set fw_devlink=on by default") >>> Signed-off-by: Tudor Ambarus <tudor.amba...@microchip.com> >>> --- >>> I'll be out of office, will check the rest of the at91 SoCs >>> at the begining of next week. >>> >>> drivers/clk/at91/sama5d2.c | 2 ++ >>> 1 file changed, 2 insertions(+) >>> >>> diff --git a/drivers/clk/at91/sama5d2.c b/drivers/clk/at91/sama5d2.c >>> index 9a5cbc7cd55a..5eea2b4a63dd 100644 >>> --- a/drivers/clk/at91/sama5d2.c >>> +++ b/drivers/clk/at91/sama5d2.c >>> @@ -367,6 +367,8 @@ static void __init sama5d2_pmc_setup(struct device_node >>> *np) >>> >>> of_clk_add_hw_provider(np, of_clk_hw_pmc_get, sama5d2_pmc); >>> >>> + of_node_set_flag(np, OF_POPULATED); >>> + >>> return; >> >> Hi Tudor, >> >> Thanks for looking into this. >> >> I already accounted for early clocks like this when I designed >> fw_devlink. Each driver shouldn't need to set OF_POPULATED. >> drivers/clk/clk.c already does this for you. >> >> I think the problem is that your driver is using >> CLK_OF_DECLARE_DRIVER() instead of CLK_OF_DECLARE(). The comments for >> CLK_OF_DECLARE_DRIVER() says: >> /* >> * Use this macro when you have a driver that requires two initialization >> * routines, one at of_clk_init(), and one at platform device probe >> */ >> >> In your case, you are explicitly NOT having a driver bind to this >> clock later. So you shouldn't be using CLK_OF_DECLARE() instead. >> > > I see > > drivers/power/reset/at91-sama5d2_shdwc.c: { .compatible = > "atmel,sama5d2-pmc" }, > > so isn't that the driver that wants to bind to the same device node > again? First at of_clk_init() time here and then second for the reset > driver?
No, it isn't. at91_shdwc_driver binds the compatibles from at91_shdwc_of_match: static const struct of_device_id at91_shdwc_of_match[] = { { .compatible = "atmel,sama5d2-shdwc", .data = &sama5d2_reg_config, }, { .compatible = "microchip,sam9x60-shdwc", .data = &sam9x60_reg_config, }, { /*sentinel*/ } }; MODULE_DEVICE_TABLE(of, at91_shdwc_of_match); The pmc compatibles are later on used in the probe function in order to get with of_iomap() the pmc_base, that is later used in at91_poweroff() method. Just for the reference, this patch is superseded by the following: https://lore.kernel.org/lkml/20210203154332.470587-1-tudor.amba...@microchip.com/ Cheers, ta