Messages in this thread | | | From | Geert Uytterhoeven <> | Date | Fri, 26 Mar 2021 19:13:26 +0100 | Subject | Re: [PATCH] clk: bcm: rpi: Don't register as OF provider if !dev->np |
| |
On Fri, Mar 26, 2021 at 7:09 PM Stephen Boyd <sboyd@kernel.org> wrote: > Quoting Nicolas Saenz Julienne (2021-03-25 11:57:48) > > There are two ways clk-raspberrypi might be registered: through > > device-tree or through an explicit platform device registration. The > > latter happens after firmware/raspberrypi's probe, and it's limited to > > RPi3s, which solely use the ARM clock to scale CPU's frequency. That > > clock is matched with cpu0's device thanks to the ARM clock being > > registered as a clkdev. > > > > In that scenario, don't register the device as an OF clock provider, as > > it makes no sense and will cause trouble. > > What sort of trouble?
A crash https://lore.kernel.org/linux-acpi/d24bebc5-0f78-021f-293f-e58defa32531@samsung.com/
> > Fixes: d4b4f1b6b97e ("clk: bcm: rpi: Add DT provider for the clocks") > > Reported-by: Marek Szyprowski <m.szyprowski@samsung.com> > > Signed-off-by: Nicolas Saenz Julienne <nsaenz@kernel.org> > > --- > > drivers/clk/bcm/clk-raspberrypi.c | 10 ++++++---- > > 1 file changed, 6 insertions(+), 4 deletions(-) > > > > diff --git a/drivers/clk/bcm/clk-raspberrypi.c b/drivers/clk/bcm/clk-raspberrypi.c > > index f89b9cfc4309..27e85687326f 100644 > > --- a/drivers/clk/bcm/clk-raspberrypi.c > > +++ b/drivers/clk/bcm/clk-raspberrypi.c > > @@ -337,10 +337,12 @@ static int raspberrypi_clk_probe(struct platform_device *pdev) > > if (ret) > > return ret; > > > > - ret = devm_of_clk_add_hw_provider(dev, of_clk_hw_onecell_get, > > - clk_data); > > - if (ret) > > - return ret; > > + if (dev->of_node) { > > Can you add a comment to the code indicating the problem this is fixing? > I fear that we'll look back on this later and simply remove this if > condition because it's "redundant". Better to have some code comment so > we don't have to look up git history to figure out that we only call > this when the of node is populated. I'm not sure I understand what goes > wrong though. Won't the absence of dev->of_node mean the provider > doesn't do anything?
Gr{oetje,eeting}s,
Geert
-- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
| |