Re: [PATCH] clk: add generic driver for fixed rate clock
On Tue, Nov 17, 2015 at 04:46:29PM +0100, Geert Uytterhoeven wrote: > Thanks, this is still very useful! > > I stumbled across this old patch while trying to instantiate a fixed rate > clock from a DT overlay. > Without this, the clock is never instantiated, as drivers/clk/clk-fixed-rate.c > uses CLK_OF_DECLARE() :-( With your driver, it works as expected > (after fixing the modpost complaint, cfr. below). > Ah, I'm glad that this old thing was of use to someone else than me ;) > However, I think that instead of creating a new driver, you should just add > the meat of clk-generic-fixed.c to clk-fixed-rate.c. Sure, that sounds reasonable, I'll take a second bat at this patch and do that. Stefan -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] clk: add generic driver for fixed rate clock
On 11/17, Geert Uytterhoeven wrote: > Hi Stefan, > > (quoting the full driver, as it predates linux-clk) > > On Sun, Sep 1, 2013 at 6:40 AM, Stefan Kristiansson >wrote: > > This adds a simple driver with the only purpose to initialise > > the fixed rate clock. > > This is useful for systems that do not wish to use seperate init > > code for the fixed rate clock init, but rather only rely on a > > device tree description of it. > > > > Signed-off-by: Stefan Kristiansson > > Thanks, this is still very useful! > > I stumbled across this old patch while trying to instantiate a fixed rate > clock from a DT overlay. > Without this, the clock is never instantiated, as drivers/clk/clk-fixed-rate.c > uses CLK_OF_DECLARE() :-( With your driver, it works as expected > (after fixing the modpost complaint, cfr. below). > > However, I think that instead of creating a new driver, you should just add > the meat of clk-generic-fixed.c to clk-fixed-rate.c. Hm... what happens when of_clk_init() runs and instantiates the clock, and then of_platform_populate() runs and creates the clock again? The platform device probe fails because the framework checks to make sure two clocks don't have the same name? I guess that's going to work, but it doesn't make me feel good. -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] clk: add generic driver for fixed rate clock
Hi Stephen, On Wed, Nov 18, 2015 at 9:34 PM, Stephen Boydwrote: > On 11/17, Geert Uytterhoeven wrote: >> (quoting the full driver, as it predates linux-clk) >> >> On Sun, Sep 1, 2013 at 6:40 AM, Stefan Kristiansson >> wrote: >> > This adds a simple driver with the only purpose to initialise >> > the fixed rate clock. >> > This is useful for systems that do not wish to use seperate init >> > code for the fixed rate clock init, but rather only rely on a >> > device tree description of it. >> > >> > Signed-off-by: Stefan Kristiansson >> >> Thanks, this is still very useful! >> >> I stumbled across this old patch while trying to instantiate a fixed rate >> clock from a DT overlay. >> Without this, the clock is never instantiated, as >> drivers/clk/clk-fixed-rate.c >> uses CLK_OF_DECLARE() :-( With your driver, it works as expected >> (after fixing the modpost complaint, cfr. below). >> >> However, I think that instead of creating a new driver, you should just add >> the meat of clk-generic-fixed.c to clk-fixed-rate.c. > > Hm... what happens when of_clk_init() runs and instantiates the > clock, and then of_platform_populate() runs and creates the clock > again? The platform device probe fails because the framework > checks to make sure two clocks don't have the same name? I guess > that's going to work, but it doesn't make me feel good. My DTS does have other clocks that are compatible with "fixed-clock" and I didn't notice any ill behavior. Perhaps the driver core knows the node has already been bound to something, and thus no longer tries to bind it to a platform driver? I'll double-check that later... Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@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 -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] clk: add generic driver for fixed rate clock
Hi Stefan, (quoting the full driver, as it predates linux-clk) On Sun, Sep 1, 2013 at 6:40 AM, Stefan Kristianssonwrote: > This adds a simple driver with the only purpose to initialise > the fixed rate clock. > This is useful for systems that do not wish to use seperate init > code for the fixed rate clock init, but rather only rely on a > device tree description of it. > > Signed-off-by: Stefan Kristiansson Thanks, this is still very useful! I stumbled across this old patch while trying to instantiate a fixed rate clock from a DT overlay. Without this, the clock is never instantiated, as drivers/clk/clk-fixed-rate.c uses CLK_OF_DECLARE() :-( With your driver, it works as expected (after fixing the modpost complaint, cfr. below). However, I think that instead of creating a new driver, you should just add the meat of clk-generic-fixed.c to clk-fixed-rate.c. > --- > drivers/clk/Kconfig | 8 ++ > drivers/clk/Makefile| 1 + > drivers/clk/clk-generic-fixed.c | 59 > + > 3 files changed, 68 insertions(+) > create mode 100644 drivers/clk/clk-generic-fixed.c > > diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig > index 51380d6..7c8ea78 100644 > --- a/drivers/clk/Kconfig > +++ b/drivers/clk/Kconfig > @@ -87,6 +87,14 @@ config CLK_PPC_CORENET > This adds the clock driver support for Freescale PowerPC corenet > platforms using common clock framework. > > +config COMMON_CLK_GENERIC_FIXED > + tristate "Generic fixed rate clock driver" > + depends on OF > + ---help--- > + Driver for systems that do not want to register their fixed rate > + clocks through init code, but rather through the device tree > + description. > + > endmenu > > source "drivers/clk/mvebu/Kconfig" > diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile > index 4038c2b..2d46647 100644 > --- a/drivers/clk/Makefile > +++ b/drivers/clk/Makefile > @@ -8,6 +8,7 @@ obj-$(CONFIG_COMMON_CLK)+= clk-fixed-rate.o > obj-$(CONFIG_COMMON_CLK) += clk-gate.o > obj-$(CONFIG_COMMON_CLK) += clk-mux.o > obj-$(CONFIG_COMMON_CLK) += clk-composite.o > +obj-$(CONFIG_COMMON_CLK_GENERIC_FIXED) += clk-generic-fixed.o > > # SoCs specific > obj-$(CONFIG_ARCH_BCM2835) += clk-bcm2835.o > diff --git a/drivers/clk/clk-generic-fixed.c b/drivers/clk/clk-generic-fixed.c > new file mode 100644 > index 000..85df8a8 > --- /dev/null > +++ b/drivers/clk/clk-generic-fixed.c > @@ -0,0 +1,59 @@ > +/* > + * Copyright 2013 Stefan Kristiansson > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License version 2 as > + * published by the Free Software Foundation. > + * > + * Generic driver for fixed rate clock > + */ > +#include > +#include > +#include > +#include > +#include > + > +static const struct of_device_id generic_fixed_clk_match[] __initconst = { Please drop the __initconst, as it causes a section mismatch. > + { .compatible = "fixed-clock",}, > + {} > +}; > + > +static int generic_fixed_clk_probe(struct platform_device *pdev) > +{ > + of_fixed_clk_setup(pdev->dev.of_node); > + > + return 0; > +} > + > +static int generic_fixed_clk_remove(struct platform_device *pdev) > +{ > + of_clk_del_provider(pdev->dev.of_node); > + > + return 0; > +} > + > +static struct platform_driver generic_fixed_clk_driver = { > + .driver = { > + .name = "generic-fixed-clk", > + .owner = THIS_MODULE, > + .of_match_table = generic_fixed_clk_match, > + }, > + .probe = generic_fixed_clk_probe, > + .remove = generic_fixed_clk_remove, > +}; > + > +static int __init generic_fixed_clk_init(void) > +{ > + return platform_driver_register(_fixed_clk_driver); > +} > +subsys_initcall(generic_fixed_clk_init); > + > +static void __exit generic_fixed_exit(void) > +{ > + platform_driver_unregister(_fixed_clk_driver); > +} > +module_exit(generic_fixed_exit); > + > +MODULE_AUTHOR("Stefan Kristiansson "); > +MODULE_DESCRIPTION("Generic driver for fixed rate clock"); > +MODULE_LICENSE("GPL v2"); Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@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 -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html