This driver adds a DT test clock consumer that exposes debugfs files to
enable/disable and set/get rate of the attached programmable clock.
During development of a i2c-attached clock generator I found it useful
to debug the clock generator's internal pll settings by enforcing clock
rates through debugfs.
Signed-off-by: Sebastian Hesselbarth <[email protected]>
---
Cc: Grant Likely <[email protected]>
Cc: Rob Herring <[email protected]>
Cc: Rob Landley <[email protected]>
Cc: Mike Turquette <[email protected]>
Cc: Linus Walleij <[email protected]>
Cc: Arnd Bergmann <[email protected]>
Cc: Andrew Morton <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
---
.../bindings/clock/test-clock-consumer.txt | 19 ++
drivers/clk/Kconfig | 6 +
drivers/clk/Makefile | 3 +
drivers/clk/clk-test.c | 183 ++++++++++++++++++++
4 files changed, 211 insertions(+)
create mode 100644 Documentation/devicetree/bindings/clock/test-clock-consumer.txt
create mode 100644 drivers/clk/clk-test.c
diff --git a/Documentation/devicetree/bindings/clock/test-clock-consumer.txt b/Documentation/devicetree/bindings/clock/test-clock-consumer.txt
new file mode 100644
index 0000000..3e228ed
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/test-clock-consumer.txt
@@ -0,0 +1,19 @@
+* Test Clock Consumer
+
+The test clock consumer allows to debug clock providers by exposing
+debugfs files to enable/disable and set/get rate of attached clock
+respectively. It is especially useful for checking programmable clock
+generators, e.g. enforcing target clock rates to debug clock generator's
+pll settings. Currently, there is only one clock per test clock consumer
+supported but more than one test clock consumer can be added.
+
+Required properties:
+- compatible : shall be "test-clock-consumer"
+- clocks : phandle of the clock to debug
+
+Example:
+
+clock-consumer {
+ compatible = "test-clock-consumer";
+ clocks = <&programmable_clock>;
+};
diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig
index a47e6ee..e23e471 100644
--- a/drivers/clk/Kconfig
+++ b/drivers/clk/Kconfig
@@ -33,6 +33,12 @@ config COMMON_CLK_DEBUG
clk_flags, clk_prepare_count, clk_enable_count &
clk_notifier_count.
+config COMMON_CLK_TEST_CONSUMER
+ tristate "Clock consumer test driver"
+ ---help---
+ This is a clock consumer test driver to allow testing clock
+ provider through user space.
+
config COMMON_CLK_WM831X
tristate "Clock driver for WM831x/2x PMICs"
depends on MFD_WM831X
diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
index 300d477..095899e 100644
--- a/drivers/clk/Makefile
+++ b/drivers/clk/Makefile
@@ -34,3 +34,6 @@ obj-$(CONFIG_X86) += x86/
obj-$(CONFIG_COMMON_CLK_WM831X) += clk-wm831x.o
obj-$(CONFIG_COMMON_CLK_MAX77686) += clk-max77686.o
obj-$(CONFIG_CLK_TWL6040) += clk-twl6040.o
+
+# Debug drivers
+obj-$(CONFIG_COMMON_CLK_TEST_CONSUMER) += clk-test.o
diff --git a/drivers/clk/clk-test.c b/drivers/clk/clk-test.c
new file mode 100644
index 0000000..08dd66b
--- /dev/null
+++ b/drivers/clk/clk-test.c
@@ -0,0 +1,183 @@
+/*
+ * clk-test.c: Common Clock Framework Test Clock Consumer
+ *
+ * (c) 2012 Sebastian Hesselbarth <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the
+ * Free Software Foundation; either version 2 of the License, or (at your
+ * option) any later version.
+ */
+
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/clk.h>
+#include <linux/clk-provider.h>
+#include <linux/debugfs.h>
+#include <linux/err.h>
+#include <linux/errno.h>
+#include <linux/of_platform.h>
+
+struct clock_consumer_data {
+ struct clk *clk;
+ struct dentry *clkdir;
+ const char *name;
+ unsigned int enabled;
+};
+
+static struct dentry *rootdir;
+
+static int clock_consumer_debug_rate_set(void *data, u64 val)
+{
+ struct clock_consumer_data *ccdata = (struct clock_consumer_data *)data;
+
+ if (!ccdata->enabled)
+ return -EBUSY;
+
+ return clk_set_rate(ccdata->clk, (unsigned long)val);
+}
+
+static int clock_consumer_debug_rate_get(void *data, u64 *val)
+{
+ struct clock_consumer_data *ccdata = (struct clock_consumer_data *)data;
+
+ if (!ccdata->enabled)
+ return -EBUSY;
+
+ *val = clk_get_rate(ccdata->clk);
+
+ return 0;
+}
+
+static int clock_consumer_debug_enable_set(void *data, u64 val)
+{
+ struct clock_consumer_data *ccdata = (struct clock_consumer_data *)data;
+
+ if ((val == ccdata->enabled) || (!val && !ccdata->enabled))
+ return 0;
+
+ ccdata->enabled = (val) ? 1 : 0;
+
+ if (ccdata->enabled)
+ return clk_prepare_enable(ccdata->clk);
+
+ clk_disable_unprepare(ccdata->clk);
+ return 0;
+}
+
+static int clock_consumer_debug_enable_get(void *data, u64 *val)
+{
+ struct clock_consumer_data *ccdata = (struct clock_consumer_data *)data;
+ *val = ccdata->enabled;
+ return 0;
+}
+
+DEFINE_SIMPLE_ATTRIBUTE(clock_consumer_debug_ops_enable,
+ clock_consumer_debug_enable_get,
+ clock_consumer_debug_enable_set, "%llu\n");
+DEFINE_SIMPLE_ATTRIBUTE(clock_consumer_debug_ops_rate,
+ clock_consumer_debug_rate_get,
+ clock_consumer_debug_rate_set, "%llu\n");
+
+static int clock_consumer_debug_create_one(struct platform_device *pdev)
+{
+ struct clock_consumer_data *ccdata = platform_get_drvdata(pdev);
+ struct dentry *d;
+
+ if (!rootdir) {
+ rootdir = debugfs_create_dir("clk_consumer", NULL);
+ if (WARN_ON(IS_ERR_OR_NULL(rootdir)))
+ return -ENOMEM;
+ }
+
+ ccdata->clkdir = debugfs_create_dir(ccdata->name, rootdir);
+ if (WARN_ON(IS_ERR_OR_NULL(ccdata->clkdir)))
+ return -ENOMEM;
+
+ d = debugfs_create_file("enable", S_IRUGO | S_IWUGO, ccdata->clkdir,
+ ccdata, &clock_consumer_debug_ops_enable);
+ if (WARN_ON(IS_ERR_OR_NULL(d)))
+ goto err_out;
+
+ d = debugfs_create_file("rate", S_IRUGO | S_IWUGO, ccdata->clkdir,
+ ccdata, &clock_consumer_debug_ops_rate);
+ if (WARN_ON(IS_ERR_OR_NULL(d)))
+ goto err_out;
+
+ return 0;
+
+err_out:
+ debugfs_remove(ccdata->clkdir);
+ return -ENOMEM;
+}
+
+static struct of_device_id clock_consumer_of_match[] = {
+ { .compatible = "test-clock-consumer", },
+ { }
+};
+
+static int clock_consumer_probe(struct platform_device *pdev)
+{
+ struct clock_consumer_data *ccdata;
+ int ret;
+
+ ccdata = devm_kzalloc(&pdev->dev, sizeof(struct clock_consumer_data),
+ GFP_KERNEL);
+ if (WARN_ON(!ccdata))
+ return -ENOMEM;
+
+ ccdata->clk = devm_clk_get(&pdev->dev, NULL);
+ if (IS_ERR(ccdata->clk)) {
+ dev_err(&pdev->dev, "unable to get parent clock");
+ return PTR_RET(ccdata->clk);
+ }
+
+ platform_set_drvdata(pdev, ccdata);
+ ccdata->name = pdev->dev.of_node->name;
+
+ ret = clock_consumer_debug_create_one(pdev);
+ if (ret)
+ return ret;
+
+ dev_info(&pdev->dev, "installed clock consumer %s\n",
+ ccdata->name);
+
+ return 0;
+}
+
+static int clock_consumer_remove(struct platform_device *pdev)
+{
+ struct clock_consumer_data *ccdata = platform_get_drvdata(pdev);
+
+ if (!ccdata || !ccdata->clkdir)
+ return 0;
+
+ debugfs_remove_recursive(ccdata->clkdir);
+ if (!IS_ERR(ccdata->clk) && ccdata->enabled)
+ clk_disable_unprepare(ccdata->clk);
+
+ dev_info(&pdev->dev, "removed clock consumer %s\n", ccdata->name);
+
+ if (rootdir && list_empty(&rootdir->d_subdirs))
+ debugfs_remove_recursive(rootdir);
+
+ platform_set_drvdata(pdev, NULL);
+
+ return 0;
+}
+
+static struct platform_driver clock_consumer_driver = {
+ .driver = {
+ .name = "clock-consumer",
+ .owner = THIS_MODULE,
+ .of_match_table = of_match_ptr(clock_consumer_of_match),
+ },
+ .probe = clock_consumer_probe,
+ .remove = clock_consumer_remove,
+};
+
+module_platform_driver(clock_consumer_driver);
+
+MODULE_AUTHOR("Sebastian Hesselbarth <[email protected]>");
+MODULE_DESCRIPTION("Common clock framwork test clock consumer driver");
+MODULE_LICENSE("GPL v2");
--
1.7.10.4
On Saturday 16 March 2013, Sebastian Hesselbarth wrote:
> This driver adds a DT test clock consumer that exposes debugfs files to
> enable/disable and set/get rate of the attached programmable clock.
> During development of a i2c-attached clock generator I found it useful
> to debug the clock generator's internal pll settings by enforcing clock
> rates through debugfs.
>
> Signed-off-by: Sebastian Hesselbarth <[email protected]>
It sounds a little clumsy to have a device driver to match a device that
you create just for matching the driver.
Would it be possible to separate the debugging logic from the platform
device logic? I think it may be useful to have a debugfs or sysfs
inteface for all clocks in the system, even if that is disabled by
default or only available after manually loading a module implementing
that functionality.
Arnd
Quoting Arnd Bergmann (2013-03-16 07:56:54)
> On Saturday 16 March 2013, Sebastian Hesselbarth wrote:
> > This driver adds a DT test clock consumer that exposes debugfs files to
> > enable/disable and set/get rate of the attached programmable clock.
> > During development of a i2c-attached clock generator I found it useful
> > to debug the clock generator's internal pll settings by enforcing clock
> > rates through debugfs.
> >
> > Signed-off-by: Sebastian Hesselbarth <[email protected]>
>
> It sounds a little clumsy to have a device driver to match a device that
> you create just for matching the driver.
>
> Would it be possible to separate the debugging logic from the platform
> device logic? I think it may be useful to have a debugfs or sysfs
> inteface for all clocks in the system, even if that is disabled by
> default or only available after manually loading a module implementing
> that functionality.
>
I agree that a generic approach is needed here. I have been meaning to
break the existing debugfs stuff out into clk-debug.c. I'll do that
soon and maybe you can add a new Kconfig entry for
COMMON_CLK_DEBUG_USERSPACE (or something like that) which implements
this?
On the other hand this sort of stuff really scares me. I know for a
fact that a debug interface to enable/disable clocks and set clock rate
would ship on real devices. Quite likely some android phones out there
would be controlling hardware clocks from some horrible userspace
utility.
*shudder*
Sebastian, another small nitpick, can you change the "enable" attribute
to be named "prepare_enable"? This more accurately reflects what is
going on.
I also wonder how simple it would be to add a "parent" attribute here
that allows one to call clk_set_parent from the debugfs interface? To
make it easy on you, the interface could accept an integer as the index
of the clk->parents[] array. This is a bad interface design as it
requires the user to look into the code to know which index corresponds
to which parent clock; however I do not want people to use this
interface for anything other than debug/testing, so I am ok with this
interface being a PITA to use.
Regards,
Mike
> Arnd
>
> _______________________________________________
> linux-arm-kernel mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
On 03/19/2013 02:54 AM, Mike Turquette wrote:
> Quoting Arnd Bergmann (2013-03-16 07:56:54)
>> On Saturday 16 March 2013, Sebastian Hesselbarth wrote:
>>> This driver adds a DT test clock consumer that exposes debugfs files to
>>> enable/disable and set/get rate of the attached programmable clock.
>>> During development of a i2c-attached clock generator I found it useful
>>> to debug the clock generator's internal pll settings by enforcing clock
>>> rates through debugfs.
>>>
>>> Signed-off-by: Sebastian Hesselbarth<[email protected]>
>>
>> It sounds a little clumsy to have a device driver to match a device that
>> you create just for matching the driver.
>>
>> Would it be possible to separate the debugging logic from the platform
>> device logic? I think it may be useful to have a debugfs or sysfs
>> inteface for all clocks in the system, even if that is disabled by
>> default or only available after manually loading a module implementing
>> that functionality.
>>
>
> I agree that a generic approach is needed here. I have been meaning to
> break the existing debugfs stuff out into clk-debug.c. I'll do that
> soon and maybe you can add a new Kconfig entry for
> COMMON_CLK_DEBUG_USERSPACE (or something like that) which implements
> this?
Mike,
I agree with you and Arnd about clumsiness and a generic approach, but
this driver is a little different from controlling _all_ clocks within
the tree. It just adds one consumer that can _request_ a new rate.
Nevertheless, I can have a look at clk-debug and adding the functionality.
> On the other hand this sort of stuff really scares me. I know for a
> fact that a debug interface to enable/disable clocks and set clock rate
> would ship on real devices. Quite likely some android phones out there
> would be controlling hardware clocks from some horrible userspace
> utility.
>
> *shudder*
This will happen for sure.
> Sebastian, another small nitpick, can you change the "enable" attribute
> to be named "prepare_enable"? This more accurately reflects what is
> going on.
On a generic approach I would rather have a look at the actual ops that
are provided and name the files accordingly. That will also allow us
_not_ to set the rate of crystal oscillators ;)
> I also wonder how simple it would be to add a "parent" attribute here
> that allows one to call clk_set_parent from the debugfs interface? To
> make it easy on you, the interface could accept an integer as the index
> of the clk->parents[] array. This is a bad interface design as it
> requires the user to look into the code to know which index corresponds
> to which parent clock; however I do not want people to use this
> interface for anything other than debug/testing, so I am ok with this
> interface being a PITA to use.
Sure, but it will not help much against userspace hardware clock
utilities ;)
Sebastian
On Sat, 16 Mar 2013 14:09:36 +0100, Sebastian Hesselbarth <[email protected]> wrote:
> This driver adds a DT test clock consumer that exposes debugfs files to
> enable/disable and set/get rate of the attached programmable clock.
> During development of a i2c-attached clock generator I found it useful
> to debug the clock generator's internal pll settings by enforcing clock
> rates through debugfs.
>
> Signed-off-by: Sebastian Hesselbarth <[email protected]>
Rather that using a DT binding to enable this, would it not be better to
have the debug interface bound entirely at runtime, and be able to
attach to pretty much any clock. It is less usable if it requires
modifying the dtb to use a debug feature.
g.