Some board designers, when running out of clock output pads, decide to
(mis)use PWM output pads to provide a clock to external components.
This driver supports this practice by providing an adapter between the
PWM and clock bindings in the device tree. As the PWM bindings specify
the period in the device tree, this is a fixed clock.
Signed-off-by: Philipp Zabel <[email protected]>
---
Changes since v3:
- Call pwm_enable/disable from prepare/unprepare callbacks
- Use NSEC_PER_SEC
- Add CLK_IS_BASIC flag
- Register clk_pwm_remove function
- Since this is a platform_device, remove driver.owner = THIS_MODULE
---
.../devicetree/bindings/clock/pwm-clock.txt | 26 +++++
drivers/clk/Kconfig | 7 ++
drivers/clk/Makefile | 1 +
drivers/clk/clk-pwm.c | 129 +++++++++++++++++++++
4 files changed, 163 insertions(+)
create mode 100644 Documentation/devicetree/bindings/clock/pwm-clock.txt
create mode 100644 drivers/clk/clk-pwm.c
diff --git a/Documentation/devicetree/bindings/clock/pwm-clock.txt b/Documentation/devicetree/bindings/clock/pwm-clock.txt
new file mode 100644
index 0000000..751fff5
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/pwm-clock.txt
@@ -0,0 +1,26 @@
+Binding for an external clock signal driven by a PWM pin.
+
+This binding uses the common clock binding[1] and the common PWM binding[2].
+
+[1] Documentation/devicetree/bindings/clock/clock-bindings.txt
+[2] Documentation/devicetree/bindings/pwm/pwm.txt
+
+Required properties:
+- compatible : shall be "pwm-clock".
+- #clock-cells : from common clock binding; shall be set to 0.
+- pwms : from common PWM binding; this determines the clock frequency
+ via the PWM period given in the pwm-specifier.
+
+Optional properties:
+- clock-output-names : From common clock binding.
+- clock-frequency : Exact output frequency, in case the pwm period
+ is not exact but was rounded to nanoseconds.
+
+Example:
+ clock {
+ compatible = "pwm-clock";
+ #clock-cells = <0>;
+ clock-frequency = <25000000>;
+ clock-output-names = "mipi_mclk";
+ pwms = <&pwm2 0 40>; /* 1 / 40 ns = 25 MHz */
+ };
diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig
index 455fd17..36a6918a 100644
--- a/drivers/clk/Kconfig
+++ b/drivers/clk/Kconfig
@@ -129,6 +129,13 @@ config COMMON_CLK_PALMAS
This driver supports TI Palmas devices 32KHz output KG and KG_AUDIO
using common clock framework.
+config COMMON_CLK_PWM
+ bool "Clock driver for PWMs used as clock outputs"
+ depends on PWM
+ ---help---
+ Adapter driver so that any PWM output can be (mis)used as clock signal
+ at 50% duty cycle.
+
config COMMON_CLK_PXA
def_bool COMMON_CLK && ARCH_PXA
---help---
diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
index d5fba5b..6a0c5cf 100644
--- a/drivers/clk/Makefile
+++ b/drivers/clk/Makefile
@@ -40,6 +40,7 @@ obj-$(CONFIG_ARCH_U300) += clk-u300.o
obj-$(CONFIG_ARCH_VT8500) += clk-vt8500.o
obj-$(CONFIG_COMMON_CLK_WM831X) += clk-wm831x.o
obj-$(CONFIG_COMMON_CLK_XGENE) += clk-xgene.o
+obj-$(CONFIG_COMMON_CLK_PWM) += clk-pwm.o
obj-$(CONFIG_COMMON_CLK_AT91) += at91/
obj-$(CONFIG_ARCH_BCM_MOBILE) += bcm/
obj-$(CONFIG_ARCH_BERLIN) += berlin/
diff --git a/drivers/clk/clk-pwm.c b/drivers/clk/clk-pwm.c
new file mode 100644
index 0000000..c7d5652
--- /dev/null
+++ b/drivers/clk/clk-pwm.c
@@ -0,0 +1,129 @@
+/*
+ * Copyright (C) 2014 Philipp Zabel, Pengutronix
+ *
+ * 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.
+ *
+ * PWM (mis)used as clock output
+ */
+#include <linux/clk-provider.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/pwm.h>
+
+struct clk_pwm {
+ struct clk_hw hw;
+ struct pwm_device *pwm;
+ u32 fixed_rate;
+};
+
+#define to_clk_pwm(_hw) container_of(_hw, struct clk_pwm, hw)
+
+static int clk_pwm_prepare(struct clk_hw *hw)
+{
+ struct clk_pwm *clk_pwm = to_clk_pwm(hw);
+
+ return pwm_enable(clk_pwm->pwm);
+}
+
+static void clk_pwm_unprepare(struct clk_hw *hw)
+{
+ struct clk_pwm *clk_pwm = to_clk_pwm(hw);
+
+ pwm_disable(clk_pwm->pwm);
+}
+
+static unsigned long clk_pwm_recalc_rate(struct clk_hw *hw,
+ unsigned long parent_rate)
+{
+ struct clk_pwm *clk_pwm = to_clk_pwm(hw);
+
+ return clk_pwm->fixed_rate;
+}
+
+const struct clk_ops clk_pwm_ops = {
+ .prepare = clk_pwm_prepare,
+ .unprepare = clk_pwm_unprepare,
+ .recalc_rate = clk_pwm_recalc_rate,
+};
+
+int clk_pwm_probe(struct platform_device *pdev)
+{
+ struct device_node *node = pdev->dev.of_node;
+ struct clk_init_data init;
+ struct clk_pwm *clk_pwm;
+ struct pwm_device *pwm;
+ const char *clk_name;
+ struct clk *clk;
+ int ret;
+
+ clk_pwm = devm_kzalloc(&pdev->dev, sizeof(*clk_pwm), GFP_KERNEL);
+ if (!clk_pwm)
+ return -ENOMEM;
+
+ pwm = devm_pwm_get(&pdev->dev, NULL);
+ if (IS_ERR(pwm))
+ return PTR_ERR(pwm);
+
+ if (!pwm || !pwm->period) {
+ dev_err(&pdev->dev, "invalid pwm period\n");
+ return -EINVAL;
+ }
+
+ if (of_property_read_u32(node, "clock-frequency", &clk_pwm->fixed_rate))
+ clk_pwm->fixed_rate = NSEC_PER_SEC / pwm->period;
+
+ if (pwm->period != NSEC_PER_SEC / clk_pwm->fixed_rate &&
+ pwm->period != DIV_ROUND_UP(NSEC_PER_SEC, clk_pwm->fixed_rate)) {
+ dev_err(&pdev->dev,
+ "clock-frequency does not match pwm period\n");
+ return -EINVAL;
+ }
+
+ ret = pwm_config(pwm, (pwm->period + 1) >> 1, pwm->period);
+ if (ret < 0)
+ return ret;
+
+ clk_name = node->name;
+ of_property_read_string(node, "clock-output-names", &clk_name);
+
+ init.name = clk_name;
+ init.ops = &clk_pwm_ops;
+ init.flags = CLK_IS_BASIC | CLK_IS_ROOT;
+ init.num_parents = 0;
+
+ clk_pwm->pwm = pwm;
+ clk_pwm->hw.init = &init;
+ clk = devm_clk_register(&pdev->dev, &clk_pwm->hw);
+ if (IS_ERR(clk))
+ return PTR_ERR(clk);
+
+ return of_clk_add_provider(node, of_clk_src_simple_get, clk);
+}
+
+int clk_pwm_remove(struct platform_device *pdev)
+{
+ of_clk_del_provider(pdev->dev.of_node);
+
+ return 0;
+}
+
+static const struct of_device_id clk_pwm_dt_ids[] = {
+ { .compatible = "pwm-clock" },
+ { }
+};
+MODULE_DEVICE_TABLE(of, clk_pwm_dt_ids);
+
+static struct platform_driver clk_pwm_driver = {
+ .probe = clk_pwm_probe,
+ .remove = clk_pwm_remove,
+ .driver = {
+ .name = "pwm-clock",
+ .of_match_table = of_match_ptr(clk_pwm_dt_ids),
+ },
+};
+
+module_platform_driver(clk_pwm_driver);
--
2.1.3
W dniu 2014-12-12 o 11:47, Philipp Zabel pisze:
> Some board designers, when running out of clock output pads, decide to
> (mis)use PWM output pads to provide a clock to external components.
> This driver supports this practice by providing an adapter between the
> PWM and clock bindings in the device tree. As the PWM bindings specify
> the period in the device tree, this is a fixed clock.
>
> Signed-off-by: Philipp Zabel <[email protected]>
> ---
> Changes since v3:
> - Call pwm_enable/disable from prepare/unprepare callbacks
> - Use NSEC_PER_SEC
> - Add CLK_IS_BASIC flag
> - Register clk_pwm_remove function
> - Since this is a platform_device, remove driver.owner = THIS_MODULE
> ---
> .../devicetree/bindings/clock/pwm-clock.txt | 26 +++++
> drivers/clk/Kconfig | 7 ++
> drivers/clk/Makefile | 1 +
> drivers/clk/clk-pwm.c | 129 +++++++++++++++++++++
> 4 files changed, 163 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/clock/pwm-clock.txt
> create mode 100644 drivers/clk/clk-pwm.c
>
> diff --git a/Documentation/devicetree/bindings/clock/pwm-clock.txt b/Documentation/devicetree/bindings/clock/pwm-clock.txt
> new file mode 100644
> index 0000000..751fff5
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/pwm-clock.txt
> @@ -0,0 +1,26 @@
> +Binding for an external clock signal driven by a PWM pin.
> +
> +This binding uses the common clock binding[1] and the common PWM binding[2].
> +
> +[1] Documentation/devicetree/bindings/clock/clock-bindings.txt
> +[2] Documentation/devicetree/bindings/pwm/pwm.txt
> +
> +Required properties:
> +- compatible : shall be "pwm-clock".
> +- #clock-cells : from common clock binding; shall be set to 0.
> +- pwms : from common PWM binding; this determines the clock frequency
> + via the PWM period given in the pwm-specifier.
> +
> +Optional properties:
> +- clock-output-names : From common clock binding.
> +- clock-frequency : Exact output frequency, in case the pwm period
> + is not exact but was rounded to nanoseconds.
> +
> +Example:
> + clock {
> + compatible = "pwm-clock";
> + #clock-cells = <0>;
> + clock-frequency = <25000000>;
> + clock-output-names = "mipi_mclk";
> + pwms = <&pwm2 0 40>; /* 1 / 40 ns = 25 MHz */
> + };
> diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig
> index 455fd17..36a6918a 100644
> --- a/drivers/clk/Kconfig
> +++ b/drivers/clk/Kconfig
> @@ -129,6 +129,13 @@ config COMMON_CLK_PALMAS
> This driver supports TI Palmas devices 32KHz output KG and KG_AUDIO
> using common clock framework.
>
> +config COMMON_CLK_PWM
> + bool "Clock driver for PWMs used as clock outputs"
> + depends on PWM
> + ---help---
> + Adapter driver so that any PWM output can be (mis)used as clock signal
> + at 50% duty cycle.
> +
> config COMMON_CLK_PXA
> def_bool COMMON_CLK && ARCH_PXA
> ---help---
> diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
> index d5fba5b..6a0c5cf 100644
> --- a/drivers/clk/Makefile
> +++ b/drivers/clk/Makefile
> @@ -40,6 +40,7 @@ obj-$(CONFIG_ARCH_U300) += clk-u300.o
> obj-$(CONFIG_ARCH_VT8500) += clk-vt8500.o
> obj-$(CONFIG_COMMON_CLK_WM831X) += clk-wm831x.o
> obj-$(CONFIG_COMMON_CLK_XGENE) += clk-xgene.o
> +obj-$(CONFIG_COMMON_CLK_PWM) += clk-pwm.o
> obj-$(CONFIG_COMMON_CLK_AT91) += at91/
> obj-$(CONFIG_ARCH_BCM_MOBILE) += bcm/
> obj-$(CONFIG_ARCH_BERLIN) += berlin/
> diff --git a/drivers/clk/clk-pwm.c b/drivers/clk/clk-pwm.c
> new file mode 100644
> index 0000000..c7d5652
> --- /dev/null
> +++ b/drivers/clk/clk-pwm.c
> @@ -0,0 +1,129 @@
> +/*
> + * Copyright (C) 2014 Philipp Zabel, Pengutronix
> + *
> + * 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.
> + *
> + * PWM (mis)used as clock output
> + */
> +#include <linux/clk-provider.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +#include <linux/pwm.h>
> +
> +struct clk_pwm {
> + struct clk_hw hw;
> + struct pwm_device *pwm;
> + u32 fixed_rate;
> +};
> +
> +#define to_clk_pwm(_hw) container_of(_hw, struct clk_pwm, hw)
> +
> +static int clk_pwm_prepare(struct clk_hw *hw)
> +{
> + struct clk_pwm *clk_pwm = to_clk_pwm(hw);
> +
> + return pwm_enable(clk_pwm->pwm);
> +}
> +
> +static void clk_pwm_unprepare(struct clk_hw *hw)
> +{
> + struct clk_pwm *clk_pwm = to_clk_pwm(hw);
> +
> + pwm_disable(clk_pwm->pwm);
> +}
> +
> +static unsigned long clk_pwm_recalc_rate(struct clk_hw *hw,
> + unsigned long parent_rate)
> +{
> + struct clk_pwm *clk_pwm = to_clk_pwm(hw);
> +
> + return clk_pwm->fixed_rate;
> +}
> +
> +const struct clk_ops clk_pwm_ops = {
> + .prepare = clk_pwm_prepare,
> + .unprepare = clk_pwm_unprepare,
> + .recalc_rate = clk_pwm_recalc_rate,
> +};
> +
> +int clk_pwm_probe(struct platform_device *pdev)
> +{
> + struct device_node *node = pdev->dev.of_node;
> + struct clk_init_data init;
> + struct clk_pwm *clk_pwm;
> + struct pwm_device *pwm;
> + const char *clk_name;
> + struct clk *clk;
> + int ret;
> +
> + clk_pwm = devm_kzalloc(&pdev->dev, sizeof(*clk_pwm), GFP_KERNEL);
> + if (!clk_pwm)
> + return -ENOMEM;
> +
> + pwm = devm_pwm_get(&pdev->dev, NULL);
> + if (IS_ERR(pwm))
> + return PTR_ERR(pwm);
> +
> + if (!pwm || !pwm->period) {
> + dev_err(&pdev->dev, "invalid pwm period\n");
> + return -EINVAL;
> + }
> +
> + if (of_property_read_u32(node, "clock-frequency", &clk_pwm->fixed_rate))
> + clk_pwm->fixed_rate = NSEC_PER_SEC / pwm->period;
> +
> + if (pwm->period != NSEC_PER_SEC / clk_pwm->fixed_rate &&
> + pwm->period != DIV_ROUND_UP(NSEC_PER_SEC, clk_pwm->fixed_rate)) {
> + dev_err(&pdev->dev,
> + "clock-frequency does not match pwm period\n");
> + return -EINVAL;
> + }
> +
> + ret = pwm_config(pwm, (pwm->period + 1) >> 1, pwm->period);
> + if (ret < 0)
> + return ret;
> +
> + clk_name = node->name;
> + of_property_read_string(node, "clock-output-names", &clk_name);
> +
> + init.name = clk_name;
> + init.ops = &clk_pwm_ops;
> + init.flags = CLK_IS_BASIC | CLK_IS_ROOT;
> + init.num_parents = 0;
> +
> + clk_pwm->pwm = pwm;
> + clk_pwm->hw.init = &init;
> + clk = devm_clk_register(&pdev->dev, &clk_pwm->hw);
> + if (IS_ERR(clk))
> + return PTR_ERR(clk);
> +
> + return of_clk_add_provider(node, of_clk_src_simple_get, clk);
> +}
> +
> +int clk_pwm_remove(struct platform_device *pdev)
> +{
> + of_clk_del_provider(pdev->dev.of_node);
> +
> + return 0;
> +}
> +
> +static const struct of_device_id clk_pwm_dt_ids[] = {
> + { .compatible = "pwm-clock" },
> + { }
> +};
> +MODULE_DEVICE_TABLE(of, clk_pwm_dt_ids);
> +
> +static struct platform_driver clk_pwm_driver = {
> + .probe = clk_pwm_probe,
> + .remove = clk_pwm_remove,
> + .driver = {
> + .name = "pwm-clock",
> + .of_match_table = of_match_ptr(clk_pwm_dt_ids),
> + },
> +};
> +
> +module_platform_driver(clk_pwm_driver);
rebased to 3.14.17 and then:
Tested-by: Janusz Uzycki <[email protected]>
On Fri, Dec 12, 2014 at 11:47:49AM +0100, Philipp Zabel wrote:
> Some board designers, when running out of clock output pads, decide to
> (mis)use PWM output pads to provide a clock to external components.
> This driver supports this practice by providing an adapter between the
> PWM and clock bindings in the device tree. As the PWM bindings specify
> the period in the device tree, this is a fixed clock.
Typically the period is specified in DT because it is a board-level
characteristic. In this case where you emulate a clock using the PWM
channel you could simply ignore the period. After all you can freely
choose it during pwm_config() irrespective of the period specified in
DT.
Other than that looks mostly good to me, just a few nits below.
[...]
> diff --git a/Documentation/devicetree/bindings/clock/pwm-clock.txt b/Documentation/devicetree/bindings/clock/pwm-clock.txt
> new file mode 100644
> index 0000000..751fff5
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/pwm-clock.txt
> @@ -0,0 +1,26 @@
> +Binding for an external clock signal driven by a PWM pin.
> +
> +This binding uses the common clock binding[1] and the common PWM binding[2].
> +
> +[1] Documentation/devicetree/bindings/clock/clock-bindings.txt
> +[2] Documentation/devicetree/bindings/pwm/pwm.txt
> +
> +Required properties:
> +- compatible : shall be "pwm-clock".
> +- #clock-cells : from common clock binding; shall be set to 0.
> +- pwms : from common PWM binding; this determines the clock frequency
> + via the PWM period given in the pwm-specifier.
Perhaps: "the period given in the PWM specifier"?
> +Optional properties:
> +- clock-output-names : From common clock binding.
> +- clock-frequency : Exact output frequency, in case the pwm period
"PWM period"
> + is not exact but was rounded to nanoseconds.
Does this make sense? For one it's now easy to specify two different
values for the frequency, one using the PWM specifier, the other with
clock-frequency. According to the above, clock-frequency takes
precedence, but in that case, what use is there in having the PWM
specifier?
> diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig
> index 455fd17..36a6918a 100644
> --- a/drivers/clk/Kconfig
> +++ b/drivers/clk/Kconfig
> @@ -129,6 +129,13 @@ config COMMON_CLK_PALMAS
> This driver supports TI Palmas devices 32KHz output KG and KG_AUDIO
> using common clock framework.
>
> +config COMMON_CLK_PWM
> + bool "Clock driver for PWMs used as clock outputs"
> + depends on PWM
> + ---help---
> + Adapter driver so that any PWM output can be (mis)used as clock signal
> + at 50% duty cycle.
Any reason why this isn't tristate?
> config COMMON_CLK_PXA
> def_bool COMMON_CLK && ARCH_PXA
> ---help---
[...]
> diff --git a/drivers/clk/clk-pwm.c b/drivers/clk/clk-pwm.c
> new file mode 100644
> index 0000000..c7d5652
> --- /dev/null
> +++ b/drivers/clk/clk-pwm.c
> @@ -0,0 +1,129 @@
> +/*
> + * Copyright (C) 2014 Philipp Zabel, Pengutronix
> + *
> + * 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.
> + *
> + * PWM (mis)used as clock output
> + */
> +#include <linux/clk-provider.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +#include <linux/pwm.h>
> +
> +struct clk_pwm {
> + struct clk_hw hw;
> + struct pwm_device *pwm;
> + u32 fixed_rate;
> +};
> +
> +#define to_clk_pwm(_hw) container_of(_hw, struct clk_pwm, hw)
Perhaps use a static inline so you get proper type checking?
> +int clk_pwm_probe(struct platform_device *pdev)
> +{
> + struct device_node *node = pdev->dev.of_node;
> + struct clk_init_data init;
> + struct clk_pwm *clk_pwm;
> + struct pwm_device *pwm;
> + const char *clk_name;
> + struct clk *clk;
> + int ret;
> +
> + clk_pwm = devm_kzalloc(&pdev->dev, sizeof(*clk_pwm), GFP_KERNEL);
> + if (!clk_pwm)
> + return -ENOMEM;
> +
> + pwm = devm_pwm_get(&pdev->dev, NULL);
> + if (IS_ERR(pwm))
> + return PTR_ERR(pwm);
> +
> + if (!pwm || !pwm->period) {
I don't think there's a case where pwm can be NULL here.
> + dev_err(&pdev->dev, "invalid pwm period\n");
"PWM"
> + return -EINVAL;
> + }
> +
> + if (of_property_read_u32(node, "clock-frequency", &clk_pwm->fixed_rate))
> + clk_pwm->fixed_rate = NSEC_PER_SEC / pwm->period;
> +
> + if (pwm->period != NSEC_PER_SEC / clk_pwm->fixed_rate &&
> + pwm->period != DIV_ROUND_UP(NSEC_PER_SEC, clk_pwm->fixed_rate)) {
> + dev_err(&pdev->dev,
> + "clock-frequency does not match pwm period\n");
"PWM"
> +static struct platform_driver clk_pwm_driver = {
> + .probe = clk_pwm_probe,
> + .remove = clk_pwm_remove,
> + .driver = {
> + .name = "pwm-clock",
> + .of_match_table = of_match_ptr(clk_pwm_dt_ids),
> + },
> +};
> +
> +module_platform_driver(clk_pwm_driver);
This is missing MODULE_AUTHOR, MODULE_DESCRIPTION and MODULE_LICENSE.
Thierry
Hi Thierry,
thank you for the comments.
Am Donnerstag, den 12.02.2015, 23:29 +0100 schrieb Thierry Reding:
> On Fri, Dec 12, 2014 at 11:47:49AM +0100, Philipp Zabel wrote:
> > Some board designers, when running out of clock output pads, decide to
> > (mis)use PWM output pads to provide a clock to external components.
> > This driver supports this practice by providing an adapter between the
> > PWM and clock bindings in the device tree. As the PWM bindings specify
> > the period in the device tree, this is a fixed clock.
>
> Typically the period is specified in DT because it is a board-level
> characteristic. In this case where you emulate a clock using the PWM
> channel you could simply ignore the period. After all you can freely
> choose it during pwm_config() irrespective of the period specified in
> DT.
The problem with with dynamic rate changes is that they go through the
PWM API, and due to rounding issues it is often impossible to obtain the
correct clock rate or upper and lower limits of the clock without
knowledge about the reference clock of the PWM itself, for example.
> Other than that looks mostly good to me, just a few nits below.
>
> [...]
> > diff --git a/Documentation/devicetree/bindings/clock/pwm-clock.txt b/Documentation/devicetree/bindings/clock/pwm-clock.txt
> > new file mode 100644
> > index 0000000..751fff5
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/clock/pwm-clock.txt
> > @@ -0,0 +1,26 @@
> > +Binding for an external clock signal driven by a PWM pin.
> > +
> > +This binding uses the common clock binding[1] and the common PWM binding[2].
> > +
> > +[1] Documentation/devicetree/bindings/clock/clock-bindings.txt
> > +[2] Documentation/devicetree/bindings/pwm/pwm.txt
> > +
> > +Required properties:
> > +- compatible : shall be "pwm-clock".
> > +- #clock-cells : from common clock binding; shall be set to 0.
> > +- pwms : from common PWM binding; this determines the clock frequency
> > + via the PWM period given in the pwm-specifier.
>
> Perhaps: "the period given in the PWM specifier"?
Yes, I'll change that.
> > +Optional properties:
> > +- clock-output-names : From common clock binding.
> > +- clock-frequency : Exact output frequency, in case the pwm period
>
> "PWM period"
Ok.
> > + is not exact but was rounded to nanoseconds.
>
> Does this make sense?
The PWM binding specifies the period value, but it is not good enough
for pwm-clock's purpose. Due to the rounding issue:
For the Nitrogen6X board I want to produce a 22 MHz 'clock' from a PWM
with 66 MHz reference clock, with a 33% duty cycle.
The period time for this rate is 45.4545 ns, but in the PWM bindings it
is only possible to request 45 ns or 46 ns and hope the PWM driver will
round into the right direction.
> For one it's now easy to specify two different
> values for the frequency, one using the PWM specifier, the other with
> clock-frequency.
I agree this is suboptimal. At least there is a warning for this case.
> According to the above, clock-frequency takes
> precedence, but in that case, what use is there in having the PWM
> specifier?
Are you suggesting the period given int the PWM specifier should be set
to 0?
> > diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig
> > index 455fd17..36a6918a 100644
> > --- a/drivers/clk/Kconfig
> > +++ b/drivers/clk/Kconfig
> > @@ -129,6 +129,13 @@ config COMMON_CLK_PALMAS
> > This driver supports TI Palmas devices 32KHz output KG and KG_AUDIO
> > using common clock framework.
> >
> > +config COMMON_CLK_PWM
> > + bool "Clock driver for PWMs used as clock outputs"
> > + depends on PWM
> > + ---help---
> > + Adapter driver so that any PWM output can be (mis)used as clock signal
> > + at 50% duty cycle.
>
> Any reason why this isn't tristate?
No, I'll change that.
[...]
> > +#define to_clk_pwm(_hw) container_of(_hw, struct clk_pwm, hw)
>
> Perhaps use a static inline so you get proper type checking?
Ok.
[...]
> > + pwm = devm_pwm_get(&pdev->dev, NULL);
> > + if (IS_ERR(pwm))
> > + return PTR_ERR(pwm);
> > +
> > + if (!pwm || !pwm->period) {
>
> I don't think there's a case where pwm can be NULL here.
I think you are right.
> > + dev_err(&pdev->dev, "invalid pwm period\n");
>
> "PWM"
Ok.
[...]
> > + if (pwm->period != NSEC_PER_SEC / clk_pwm->fixed_rate &&
> > + pwm->period != DIV_ROUND_UP(NSEC_PER_SEC, clk_pwm->fixed_rate)) {
> > + dev_err(&pdev->dev,
> > + "clock-frequency does not match pwm period\n");
>
> "PWM"
>
> > +static struct platform_driver clk_pwm_driver = {
> > + .probe = clk_pwm_probe,
> > + .remove = clk_pwm_remove,
> > + .driver = {
> > + .name = "pwm-clock",
> > + .of_match_table = of_match_ptr(clk_pwm_dt_ids),
> > + },
> > +};
> > +
> > +module_platform_driver(clk_pwm_driver);
>
> This is missing MODULE_AUTHOR, MODULE_DESCRIPTION and MODULE_LICENSE.
Will add them in the next round.
regards
Philipp
On 12/12/14 02:47, Philipp Zabel wrote:
> +
> +const struct clk_ops clk_pwm_ops = {
static?
> + .prepare = clk_pwm_prepare,
> + .unprepare = clk_pwm_unprepare,
> + .recalc_rate = clk_pwm_recalc_rate,
> +};
> +
> +int clk_pwm_probe(struct platform_device *pdev)
static?
> +{
> + struct device_node *node = pdev->dev.of_node;
> + struct clk_init_data init;
> + struct clk_pwm *clk_pwm;
> + struct pwm_device *pwm;
> + const char *clk_name;
> + struct clk *clk;
> + int ret;
[...]
> +}
> +
> +int clk_pwm_remove(struct platform_device *pdev)
static?
> +{
> + of_clk_del_provider(pdev->dev.of_node);
> +
> + return 0;
> +}
>
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
Hi Stephen,
Am Freitag, den 13.02.2015, 09:16 -0800 schrieb Stephen Boyd:
> On 12/12/14 02:47, Philipp Zabel wrote:
> > +
> > +const struct clk_ops clk_pwm_ops = {
>
> static?
[...]
> > +int clk_pwm_probe(struct platform_device *pdev)
>
> static?
[...]
> > +int clk_pwm_remove(struct platform_device *pdev)
>
> static?
Yes indeed. Next round will have this fixed, thank you.
regards
Philipp