[PATCH] clk: add generic driver for fixed rate clock

[PATCH] clk: add generic driver for fixed rate clock

[Stefan Kristiansson]

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 <stefan.kristiansson@saunalahti.fi>
---
 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 0000000..85df8a8
--- /dev/null
+++ b/drivers/clk/clk-generic-fixed.c
@@ -0,0 +1,59 @@
+/*
+ * Copyright 2013 Stefan Kristiansson <stefan.kristiansson@saunalahti.fi>
+ *
+ * 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 <linux/platform_device.h>
+#include <linux/clk-provider.h>
+#include <linux/clk.h>
+#include <linux/of.h>
+#include <linux/module.h>
+
+static const struct of_device_id generic_fixed_clk_match[] __initconst = {
+	{ .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(&generic_fixed_clk_driver);
+}
+subsys_initcall(generic_fixed_clk_init);
+
+static void __exit generic_fixed_exit(void)
+{
+	platform_driver_unregister(&generic_fixed_clk_driver);
+}
+module_exit(generic_fixed_exit);
+
+MODULE_AUTHOR("Stefan Kristiansson <stefan.kristiansson@saunalahti.fi>");
+MODULE_DESCRIPTION("Generic driver for fixed rate clock");
+MODULE_LICENSE("GPL v2");
-- 
1.8.1.2

Re: [PATCH] clk: add generic driver for fixed rate clock

[Stefan Kristiansson]

Ping and adding Mike Turquette to CC

On Sun, Sep 01, 2013 at 07:40:20AM +0300, 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 <stefan.kristiansson@saunalahti.fi>
> ---
>  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 0000000..85df8a8
> --- /dev/null
> +++ b/drivers/clk/clk-generic-fixed.c
> @@ -0,0 +1,59 @@
> +/*
> + * Copyright 2013 Stefan Kristiansson <stefan.kristiansson@saunalahti.fi>
> + *
> + * 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 <linux/platform_device.h>
> +#include <linux/clk-provider.h>
> +#include <linux/clk.h>
> +#include <linux/of.h>
> +#include <linux/module.h>
> +
> +static const struct of_device_id generic_fixed_clk_match[] __initconst = {
> +	{ .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(&generic_fixed_clk_driver);
> +}
> +subsys_initcall(generic_fixed_clk_init);
> +
> +static void __exit generic_fixed_exit(void)
> +{
> +	platform_driver_unregister(&generic_fixed_clk_driver);
> +}
> +module_exit(generic_fixed_exit);
> +
> +MODULE_AUTHOR("Stefan Kristiansson <stefan.kristiansson@saunalahti.fi>");
> +MODULE_DESCRIPTION("Generic driver for fixed rate clock");
> +MODULE_LICENSE("GPL v2");
> -- 
> 1.8.1.2
> 

Re: [PATCH] clk: add generic driver for fixed rate clock

[Geert Uytterhoeven]

Hi Stefan,

(quoting the full driver, as it predates linux-clk)

On Sun, Sep 1, 2013 at 6:40 AM, Stefan Kristiansson
<stefan.kristiansson@saunalahti.fi> 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 <stefan.kristiansson@saunalahti.fi>

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 0000000..85df8a8
> --- /dev/null
> +++ b/drivers/clk/clk-generic-fixed.c
> @@ -0,0 +1,59 @@
> +/*
> + * Copyright 2013 Stefan Kristiansson <stefan.kristiansson@saunalahti.fi>
> + *
> + * 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 <linux/platform_device.h>
> +#include <linux/clk-provider.h>
> +#include <linux/clk.h>
> +#include <linux/of.h>
> +#include <linux/module.h>
> +
> +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(&generic_fixed_clk_driver);
> +}
> +subsys_initcall(generic_fixed_clk_init);
> +
> +static void __exit generic_fixed_exit(void)
> +{
> +       platform_driver_unregister(&generic_fixed_clk_driver);
> +}
> +module_exit(generic_fixed_exit);
> +
> +MODULE_AUTHOR("Stefan Kristiansson <stefan.kristiansson@saunalahti.fi>");
> +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 -- 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

Re: [PATCH] clk: add generic driver for fixed rate clock

[Stephen Boyd]

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
> <stefan.kristiansson@saunalahti.fi> 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 <stefan.kristiansson@saunalahti.fi>
> 
> 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

Re: [PATCH] clk: add generic driver for fixed rate clock

[Geert Uytterhoeven]

Hi Stephen,

On Wed, Nov 18, 2015 at 9:34 PM, Stephen Boyd <sboyd@codeaurora.org> wrote:
> 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
>> <stefan.kristiansson@saunalahti.fi> 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 <stefan.kristiansson@saunalahti.fi>
>>
>> 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 -- 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

Re: [PATCH] clk: add generic driver for fixed rate clock

[Stefan Kristiansson]

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