LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH v2] clk: Add PWM clock driver
@ 2014-11-03  9:31 Philipp Zabel
  2014-11-25  7:07 ` Mike Turquette
  2014-12-04 18:00 ` Janusz Użycki
  0 siblings, 2 replies; 7+ messages in thread
From: Philipp Zabel @ 2014-11-03  9:31 UTC (permalink / raw)
  To: Mike Turquette, Thierry Reding
  Cc: Janusz Użycki, Rob Herring, Pawel Moll, Mark Rutland,
	Ian Campbell, Kumar Gala, devicetree, linux-kernel, linux-pwm,
	Philipp Zabel

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 <p.zabel@pengutronix.de>
---
I'm resending this because last time the linux-pwm list was not in Cc:
So far I have not received any comments on the patch itself. I have used
it on a BoundaryDevices Nitrogen6X board to produce a master clock for
the OV5640 MIPI CSI-2 camera module.

Changes since v1:
 - none (rebased onto v3.18-rc3)
---
 .../devicetree/bindings/clock/pwm-clock.txt        |  23 +++++
 drivers/clk/Kconfig                                |   7 ++
 drivers/clk/Makefile                               |   1 +
 drivers/clk/clk-pwm.c                              | 110 +++++++++++++++++++++
 4 files changed, 141 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..d127d17
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/pwm-clock.txt
@@ -0,0 +1,23 @@
+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.
+
+Example:
+	clock {
+		compatible = "pwm-clock";
+		#clock-cells = <0>;
+		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..8f747b3
--- /dev/null
+++ b/drivers/clk/clk-pwm.c
@@ -0,0 +1,110 @@
+/*
+ * 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;
+};
+
+#define to_clk_pwm(_hw) container_of(_hw, struct clk_pwm, hw)
+
+static int clk_pwm_enable(struct clk_hw *hw)
+{
+	struct clk_pwm *clk_pwm = to_clk_pwm(hw);
+
+	return pwm_enable(clk_pwm->pwm);
+}
+
+static void clk_pwm_disable(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);
+	unsigned int period_ns = pwm_get_period(clk_pwm->pwm);
+
+	return period_ns ? (1000000000 / period_ns) : 0;
+}
+
+const struct clk_ops clk_pwm_ops = {
+	.enable = clk_pwm_enable,
+	.disable = clk_pwm_disable,
+	.recalc_rate = clk_pwm_recalc_rate,
+};
+
+int clk_pwm_probe(struct platform_device *pdev)
+{
+	struct clk_init_data init;
+	struct clk_pwm *clk_pwm;
+	struct pwm_device *pwm;
+	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);
+
+	ret = pwm_config(pwm, (pwm->period + 1) >> 1, pwm->period);
+	if (ret < 0)
+		return ret;
+
+	init.name = "pwm-clock";
+	init.ops = &clk_pwm_ops;
+	init.flags = 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(pdev->dev.of_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,
+	.driver = {
+		.name = "pwm-clock",
+		.owner = THIS_MODULE,
+		.of_match_table = of_match_ptr(clk_pwm_dt_ids),
+	},
+};
+
+module_platform_driver(clk_pwm_driver);
-- 
2.1.1


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2] clk: Add PWM clock driver
  2014-11-03  9:31 [PATCH v2] clk: Add PWM clock driver Philipp Zabel
@ 2014-11-25  7:07 ` Mike Turquette
  2014-11-25  7:56   ` Philipp Zabel
  2014-12-04 18:00 ` Janusz Użycki
  1 sibling, 1 reply; 7+ messages in thread
From: Mike Turquette @ 2014-11-25  7:07 UTC (permalink / raw)
  To: Philipp Zabel, Thierry Reding
  Cc: Janusz Użycki, Rob Herring, Pawel Moll, Mark Rutland,
	Ian Campbell, Kumar Gala, devicetree, linux-kernel, linux-pwm,
	Philipp Zabel

Quoting Philipp Zabel (2014-11-03 01:31:18)
> 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 <p.zabel@pengutronix.de>
> ---
> I'm resending this because last time the linux-pwm list was not in Cc:
> So far I have not received any comments on the patch itself. I have used
> it on a BoundaryDevices Nitrogen6X board to produce a master clock for
> the OV5640 MIPI CSI-2 camera module.
> 
> Changes since v1:
>  - none (rebased onto v3.18-rc3)

Hi Philipp,

Thanks for testing this. Is the Nitrogen6X board merged upstream? I'm OK
with the patch but prefer to merge things when a consumer is ready to
use it. I guess you are missing a patch to add the pwm-clock bits to the
Nitrogen6X dts?

Regards,
Mike

> ---
>  .../devicetree/bindings/clock/pwm-clock.txt        |  23 +++++
>  drivers/clk/Kconfig                                |   7 ++
>  drivers/clk/Makefile                               |   1 +
>  drivers/clk/clk-pwm.c                              | 110 +++++++++++++++++++++
>  4 files changed, 141 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..d127d17
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/pwm-clock.txt
> @@ -0,0 +1,23 @@
> +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.
> +
> +Example:
> +       clock {
> +               compatible = "pwm-clock";
> +               #clock-cells = <0>;
> +               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..8f747b3
> --- /dev/null
> +++ b/drivers/clk/clk-pwm.c
> @@ -0,0 +1,110 @@
> +/*
> + * 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;
> +};
> +
> +#define to_clk_pwm(_hw) container_of(_hw, struct clk_pwm, hw)
> +
> +static int clk_pwm_enable(struct clk_hw *hw)
> +{
> +       struct clk_pwm *clk_pwm = to_clk_pwm(hw);
> +
> +       return pwm_enable(clk_pwm->pwm);
> +}
> +
> +static void clk_pwm_disable(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);
> +       unsigned int period_ns = pwm_get_period(clk_pwm->pwm);
> +
> +       return period_ns ? (1000000000 / period_ns) : 0;
> +}
> +
> +const struct clk_ops clk_pwm_ops = {
> +       .enable = clk_pwm_enable,
> +       .disable = clk_pwm_disable,
> +       .recalc_rate = clk_pwm_recalc_rate,
> +};
> +
> +int clk_pwm_probe(struct platform_device *pdev)
> +{
> +       struct clk_init_data init;
> +       struct clk_pwm *clk_pwm;
> +       struct pwm_device *pwm;
> +       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);
> +
> +       ret = pwm_config(pwm, (pwm->period + 1) >> 1, pwm->period);
> +       if (ret < 0)
> +               return ret;
> +
> +       init.name = "pwm-clock";
> +       init.ops = &clk_pwm_ops;
> +       init.flags = 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(pdev->dev.of_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,
> +       .driver = {
> +               .name = "pwm-clock",
> +               .owner = THIS_MODULE,
> +               .of_match_table = of_match_ptr(clk_pwm_dt_ids),
> +       },
> +};
> +
> +module_platform_driver(clk_pwm_driver);
> -- 
> 2.1.1
> 

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2] clk: Add PWM clock driver
  2014-11-25  7:07 ` Mike Turquette
@ 2014-11-25  7:56   ` Philipp Zabel
  0 siblings, 0 replies; 7+ messages in thread
From: Philipp Zabel @ 2014-11-25  7:56 UTC (permalink / raw)
  To: Mike Turquette
  Cc: Thierry Reding, Janusz Użycki, Rob Herring, Pawel Moll,
	Mark Rutland, Ian Campbell, Kumar Gala, devicetree, linux-kernel,
	linux-pwm

Hi Mike,

Am Montag, den 24.11.2014, 23:07 -0800 schrieb Mike Turquette:
> Quoting Philipp Zabel (2014-11-03 01:31:18)
> > 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 <p.zabel@pengutronix.de>
> > ---
> > I'm resending this because last time the linux-pwm list was not in Cc:
> > So far I have not received any comments on the patch itself. I have used
> > it on a BoundaryDevices Nitrogen6X board to produce a master clock for
> > the OV5640 MIPI CSI-2 camera module.
> > 
> > Changes since v1:
> >  - none (rebased onto v3.18-rc3)
> 
> Hi Philipp,
> 
> Thanks for testing this. Is the Nitrogen6X board merged upstream? I'm OK
> with the patch but prefer to merge things when a consumer is ready to
> use it. I guess you are missing a patch to add the pwm-clock bits to the
> Nitrogen6X dts?

The Nitrogen6X board is merged upstream already, I used this dts patch
to register the clock:

diff --git a/arch/arm/boot/dts/imx6qdl-nitrogen6x.dtsi b/arch/arm/boot/dts/imx6qdl-nitrogen6x.dtsi
index 7628049..e2e7a22 100644
--- a/arch/arm/boot/dts/imx6qdl-nitrogen6x.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-nitrogen6x.dtsi
@@ -130,6 +130,18 @@
 		power-supply = <&reg_3p3v>;
 		status = "okay";
 	};
+
+	mipi_mclk: mipi-mclk {
+		compatible = "pwm-clock";
+		clock-output-names = "mipi-mclk";
+		/* Note: since the PWM reference clock is 66 MHz, the highest
+		 * possible frequency with 50% duty cycle smaller than 27 MHz
+		 * is 16.5 MHz: <&pwm3 0 61>;
+		 * But we can also do 22 MHz with 33% duty cycle:
+		 */
+		pwms = <&pwm3 0 46>; /* 22 MHz */
+		#clock-cells = <0>;
+	};
 };
 
 &audmux {
-- 
2.1.1

regards
Philipp


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2] clk: Add PWM clock driver
  2014-11-03  9:31 [PATCH v2] clk: Add PWM clock driver Philipp Zabel
  2014-11-25  7:07 ` Mike Turquette
@ 2014-12-04 18:00 ` Janusz Użycki
  2014-12-08 20:03   ` Janusz Użycki
  1 sibling, 1 reply; 7+ messages in thread
From: Janusz Użycki @ 2014-12-04 18:00 UTC (permalink / raw)
  To: Philipp Zabel, Mike Turquette, Thierry Reding
  Cc: Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
	devicetree, linux-kernel, linux-pwm

Hi,

W dniu 2014-11-03 o 10:31, 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 <p.zabel@pengutronix.de>
> ---
> I'm resending this because last time the linux-pwm list was not in Cc:
> So far I have not received any comments on the patch itself. I have used
> it on a BoundaryDevices Nitrogen6X board to produce a master clock for
> the OV5640 MIPI CSI-2 camera module.
>
> Changes since v1:
>   - none (rebased onto v3.18-rc3)
> ---
>   .../devicetree/bindings/clock/pwm-clock.txt        |  23 +++++
>   drivers/clk/Kconfig                                |   7 ++
>   drivers/clk/Makefile                               |   1 +
>   drivers/clk/clk-pwm.c                              | 110 +++++++++++++++++++++
>   4 files changed, 141 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..d127d17
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/pwm-clock.txt
> @@ -0,0 +1,23 @@
> +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.
> +
> +Example:
> +	clock {
> +		compatible = "pwm-clock";
> +		#clock-cells = <0>;
> +		clock-output-names = "mipi_mclk";
> +		pwms = <&pwm2 0 40>; /* 1 / 40 ns = 25 MHz */

Before I asked about [MHz/kHz] <-> [ns] conversion problem but I didn't 
noticed
you've just used [ns] in dt. As Thierry wrote:
"Also the PWM chips will most likely use the concept of period and 
duty-cycle
internally anyway, so it will convert back from Hz/percentage to nanoseconds
and fall victim to similar rounding effects."

Unfortunately I discovered that many pwm drivers are buggy around upper 
freqs limit.
The example is pwm-mxs.c. PWM has 24MHz clock. There is no problem to set
registers directly for 12MHz 50% output (period_cycles=2-1=1, 
inactive_c=1, active_c=0).

When I set pwm-mxs period to 83ns / duty to 42ns the output is low because
it sets inactive_c=0!
For 125ns/63ns I got about 6MHz instead of 8MHz, and +width=42ns.
When I set 42ns/42ns I got 12MHz 50%.
It means new patches for pwms...

I will test the patch backported to 3.14 using the mxs platform.
However as I wrote I  need to patch pwm-mxs first.

> [...]
>
> +static struct platform_driver clk_pwm_driver = {
> +	.probe = clk_pwm_probe,
> +	.driver = {
> +		.name = "pwm-clock",
> +		.owner = THIS_MODULE,

AFAIR new drivers don't need to set .owner.

thanks
Janusz

> +		.of_match_table = of_match_ptr(clk_pwm_dt_ids),
> +	},
> +};
> +
> +module_platform_driver(clk_pwm_driver);


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2] clk: Add PWM clock driver
  2014-12-04 18:00 ` Janusz Użycki
@ 2014-12-08 20:03   ` Janusz Użycki
  2014-12-09  9:15     ` Philipp Zabel
  0 siblings, 1 reply; 7+ messages in thread
From: Janusz Użycki @ 2014-12-08 20:03 UTC (permalink / raw)
  To: Philipp Zabel, Mike Turquette, Thierry Reding
  Cc: Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
	devicetree, linux-kernel, linux-pwm

Hi,

I've fixed my pwm driver and I can enable 12MHz 50% output using sysfs. 
Then I rebased the pwm-clock to 3.14.
I have connected mcp2515 and it works with fixed clock. When I switch 
the chip's clock to pwm clock in dt
I get "mcp251x: probe of spi1.2 failed with error -2". I've also added 
clock-frequency property
but it didn't help.
When I set the mcp2515 clock to fixed clock again but the clock is not 
applied to mcp2515 I get
"mcp251x spi1.2: MCP251x didn't enter in conf mode after reset".
So it looks this is indeed probe error caused likely by dt.

The fixed and pwm clock in DT:
         clocks {
                 #address-cells = <1>;
                 #size-cells = <1>;
                 ranges;
                 mcp251x_xtal_clk: mcp2515_xtal {
                         compatible = "fixed-clock";
                         #clock-cells = <0>;
                         clock-frequency = <12000000>;
                 };

                 mcp251x_pwm_clk: mcp2515_pwm {
                         compatible = "pwm-clock";
                         #clock-cells = <0>;
                         clock-frequency = <12000000>;
                         clock-output-names = "can_clk";
                         pwms = <&pwm 3 83>;     /* 12MHz = 1 / ~83ns */
                 };
         };

Also the mentioned frequency recalculation problem appears here.
In the case above recalc value is about 12.048MHz instead of 12.0MHz.
While PWM block is clocked 24MHz and the pwm generates exactly 12MHz
the pwm-clock driver returns drifted value. Using clock-frequency like
fixed-clock does could simply solve the binding problem.

thanks,
Janusz


W dniu 2014-12-04 o 19:00, Janusz Użycki pisze:
> Hi,
>
> W dniu 2014-11-03 o 10:31, 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 <p.zabel@pengutronix.de>
>> ---
>> I'm resending this because last time the linux-pwm list was not in Cc:
>> So far I have not received any comments on the patch itself. I have used
>> it on a BoundaryDevices Nitrogen6X board to produce a master clock for
>> the OV5640 MIPI CSI-2 camera module.
>>
>> Changes since v1:
>>   - none (rebased onto v3.18-rc3)
>> ---
>>   .../devicetree/bindings/clock/pwm-clock.txt        |  23 +++++
>>   drivers/clk/Kconfig                                |   7 ++
>>   drivers/clk/Makefile                               |   1 +
>>   drivers/clk/clk-pwm.c                              | 110 
>> +++++++++++++++++++++
>>   4 files changed, 141 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..d127d17
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/clock/pwm-clock.txt
>> @@ -0,0 +1,23 @@
>> +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.
>> +
>> +Example:
>> +    clock {
>> +        compatible = "pwm-clock";
>> +        #clock-cells = <0>;
>> +        clock-output-names = "mipi_mclk";
>> +        pwms = <&pwm2 0 40>; /* 1 / 40 ns = 25 MHz */
>
> Before I asked about [MHz/kHz] <-> [ns] conversion problem but I 
> didn't noticed
> you've just used [ns] in dt. As Thierry wrote:
> "Also the PWM chips will most likely use the concept of period and 
> duty-cycle
> internally anyway, so it will convert back from Hz/percentage to 
> nanoseconds
> and fall victim to similar rounding effects."
>
> Unfortunately I discovered that many pwm drivers are buggy around 
> upper freqs limit.
> The example is pwm-mxs.c. PWM has 24MHz clock. There is no problem to set
> registers directly for 12MHz 50% output (period_cycles=2-1=1, 
> inactive_c=1, active_c=0).
>
> When I set pwm-mxs period to 83ns / duty to 42ns the output is low 
> because
> it sets inactive_c=0!
> For 125ns/63ns I got about 6MHz instead of 8MHz, and +width=42ns.
> When I set 42ns/42ns I got 12MHz 50%.
> It means new patches for pwms...
>
> I will test the patch backported to 3.14 using the mxs platform.
> However as I wrote I  need to patch pwm-mxs first.
>
>> [...]
>>
>> +static struct platform_driver clk_pwm_driver = {
>> +    .probe = clk_pwm_probe,
>> +    .driver = {
>> +        .name = "pwm-clock",
>> +        .owner = THIS_MODULE,
>
> AFAIR new drivers don't need to set .owner.
>
> thanks
> Janusz
>
>> +        .of_match_table = of_match_ptr(clk_pwm_dt_ids),
>> +    },
>> +};
>> +
>> +module_platform_driver(clk_pwm_driver);
>


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2] clk: Add PWM clock driver
  2014-12-08 20:03   ` Janusz Użycki
@ 2014-12-09  9:15     ` Philipp Zabel
  2014-12-09 13:09       ` Janusz Użycki
  0 siblings, 1 reply; 7+ messages in thread
From: Philipp Zabel @ 2014-12-09  9:15 UTC (permalink / raw)
  To: Janusz Użycki
  Cc: Mike Turquette, Thierry Reding, Rob Herring, Pawel Moll,
	Mark Rutland, Ian Campbell, Kumar Gala, devicetree, linux-kernel,
	linux-pwm

Hi Janusz,

Am Montag, den 08.12.2014, 21:03 +0100 schrieb Janusz Użycki:
> Hi,
> 
> I've fixed my pwm driver and I can enable 12MHz 50% output using sysfs. 
> Then I rebased the pwm-clock to 3.14.
> I have connected mcp2515 and it works with fixed clock. When I switch 
> the chip's clock to pwm clock in dt
> I get "mcp251x: probe of spi1.2 failed with error -2". I've also added 
> clock-frequency property
> but it didn't help.
> When I set the mcp2515 clock to fixed clock again but the clock is not 
> applied to mcp2515 I get
> "mcp251x spi1.2: MCP251x didn't enter in conf mode after reset".
> So it looks this is indeed probe error caused likely by dt.

Did pwm-clock fail to probe already? Could you check with the patch
below?

> The fixed and pwm clock in DT:
>          clocks {
>                  #address-cells = <1>;
>                  #size-cells = <1>;
>                  ranges;
>                  mcp251x_xtal_clk: mcp2515_xtal {
>                          compatible = "fixed-clock";
>                          #clock-cells = <0>;
>                          clock-frequency = <12000000>;
>                  };
> 
>                  mcp251x_pwm_clk: mcp2515_pwm {
>                          compatible = "pwm-clock";
>                          #clock-cells = <0>;
>                          clock-frequency = <12000000>;
>                          clock-output-names = "can_clk";
>                          pwms = <&pwm 3 83>;     /* 12MHz = 1 / ~83ns */
>                  };
>          };
> 
> Also the mentioned frequency recalculation problem appears here.
> In the case above recalc value is about 12.048MHz instead of 12.0MHz.
> While PWM block is clocked 24MHz and the pwm generates exactly 12MHz
> the pwm-clock driver returns drifted value. Using clock-frequency like
> fixed-clock does could simply solve the binding problem.

Yes, for this case it is very unfortunate that there's only nanosecond
resolution for the duty cycle. Adding a clock-frequency property to
indicate the real frequency would solve this problem.

------8<------
diff --git a/drivers/clk/clk-pwm.c b/drivers/clk/clk-pwm.c
index 8f747b3..9c13856 100644
--- a/drivers/clk/clk-pwm.c
+++ b/drivers/clk/clk-pwm.c
@@ -63,12 +63,16 @@ int clk_pwm_probe(struct platform_device *pdev)
 		return -ENOMEM;
 
 	pwm = devm_pwm_get(&pdev->dev, NULL);
-	if (IS_ERR(pwm))
+	if (IS_ERR(pwm)) {
+		dev_err(&pdev->dev, "failed to get pwm: %ld\n", PTR_ERR(pwm));
 		return PTR_ERR(pwm);
+	}
 
 	ret = pwm_config(pwm, (pwm->period + 1) >> 1, pwm->period);
-	if (ret < 0)
+	if (ret < 0) {
+		dev_err(&pdev->dev, "failed to configure pwm: %d\n", ret);
 		return ret;
+	}
 
 	init.name = "pwm-clock";
 	init.ops = &clk_pwm_ops;
@@ -78,11 +82,18 @@ int clk_pwm_probe(struct platform_device *pdev)
 	clk_pwm->pwm = pwm;
 	clk_pwm->hw.init = &init;
 	clk = devm_clk_register(&pdev->dev, &clk_pwm->hw);
-	if (IS_ERR(clk))
+	if (IS_ERR(clk)) {
+		dev_err(&pdev->dev, "failed to register clock: %ld\n",
+			PTR_ERR(clk));
 		return PTR_ERR(clk);
+	}
 
-	return of_clk_add_provider(pdev->dev.of_node,
-				   of_clk_src_simple_get, clk);
+	ret = of_clk_add_provider(pdev->dev.of_node,
+				  of_clk_src_simple_get, clk);
+	if (ret)
+		dev_err(&pdev->dev, "failed to add clock provider: %d\n", ret);
+
+	return ret;
 }
 
 int clk_pwm_remove(struct platform_device *pdev)



^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2] clk: Add PWM clock driver
  2014-12-09  9:15     ` Philipp Zabel
@ 2014-12-09 13:09       ` Janusz Użycki
  0 siblings, 0 replies; 7+ messages in thread
From: Janusz Użycki @ 2014-12-09 13:09 UTC (permalink / raw)
  To: Philipp Zabel
  Cc: Mike Turquette, Thierry Reding, Rob Herring, Pawel Moll,
	Mark Rutland, Ian Campbell, Kumar Gala, devicetree, linux-kernel,
	linux-pwm

Hi Philipp,

W dniu 2014-12-09 o 10:15, Philipp Zabel pisze:
> Hi Janusz,
>
> Am Montag, den 08.12.2014, 21:03 +0100 schrieb Janusz Użycki:
>> Hi,
>>
>> I've fixed my pwm driver and I can enable 12MHz 50% output using sysfs.
>> Then I rebased the pwm-clock to 3.14.
>> I have connected mcp2515 and it works with fixed clock. When I switch
>> the chip's clock to pwm clock in dt
>> I get "mcp251x: probe of spi1.2 failed with error -2". I've also added
>> clock-frequency property
>> but it didn't help.
>> When I set the mcp2515 clock to fixed clock again but the clock is not
>> applied to mcp2515 I get
>> "mcp251x spi1.2: MCP251x didn't enter in conf mode after reset".
>> So it looks this is indeed probe error caused likely by dt.
> Did pwm-clock fail to probe already? Could you check with the patch
> below?

Added. clk_pwm_probe() is never called. The pwm-clk driver is built-in. 
mcp251x is a module.
mcp251x doesn't trigger pwm-clock dt. I found fixed-clock uses 
CLK_OF_DECLARE().
How does it work for you? What is missing?

The pwm-clock driver set CLK_IS_ROOT and no parents.
I think it fail the clock on suspend/resume race but I can be wrong.

>> The fixed and pwm clock in DT:
>>           clocks {
>>                   #address-cells = <1>;
>>                   #size-cells = <1>;
>>                   ranges;
>>                   mcp251x_xtal_clk: mcp2515_xtal {
>>                           compatible = "fixed-clock";
>>                           #clock-cells = <0>;
>>                           clock-frequency = <12000000>;
>>                   };
>>
>>                   mcp251x_pwm_clk: mcp2515_pwm {
>>                           compatible = "pwm-clock";
>>                           #clock-cells = <0>;
>>                           clock-frequency = <12000000>;
>>                           clock-output-names = "can_clk";
>>                           pwms = <&pwm 3 83>;     /* 12MHz = 1 / ~83ns */
>>                   };
>>           };
>>
>> Also the mentioned frequency recalculation problem appears here.
>> In the case above recalc value is about 12.048MHz instead of 12.0MHz.
>> While PWM block is clocked 24MHz and the pwm generates exactly 12MHz
>> the pwm-clock driver returns drifted value. Using clock-frequency like
>> fixed-clock does could simply solve the binding problem.
> Yes, for this case it is very unfortunate that there's only nanosecond
> resolution for the duty cycle. Adding a clock-frequency property to
> indicate the real frequency would solve this problem.

Yes but the property also must be supported by pwm-clock driver.

best regards
Janusz

>
> ------8<------
> diff --git a/drivers/clk/clk-pwm.c b/drivers/clk/clk-pwm.c
> index 8f747b3..9c13856 100644
> --- a/drivers/clk/clk-pwm.c
> +++ b/drivers/clk/clk-pwm.c
> @@ -63,12 +63,16 @@ int clk_pwm_probe(struct platform_device *pdev)
>   		return -ENOMEM;
>   
>   	pwm = devm_pwm_get(&pdev->dev, NULL);
> -	if (IS_ERR(pwm))
> +	if (IS_ERR(pwm)) {
> +		dev_err(&pdev->dev, "failed to get pwm: %ld\n", PTR_ERR(pwm));
>   		return PTR_ERR(pwm);
> +	}
>   
>   	ret = pwm_config(pwm, (pwm->period + 1) >> 1, pwm->period);
> -	if (ret < 0)
> +	if (ret < 0) {
> +		dev_err(&pdev->dev, "failed to configure pwm: %d\n", ret);
>   		return ret;
> +	}
>   
>   	init.name = "pwm-clock";
>   	init.ops = &clk_pwm_ops;
> @@ -78,11 +82,18 @@ int clk_pwm_probe(struct platform_device *pdev)
>   	clk_pwm->pwm = pwm;
>   	clk_pwm->hw.init = &init;
>   	clk = devm_clk_register(&pdev->dev, &clk_pwm->hw);
> -	if (IS_ERR(clk))
> +	if (IS_ERR(clk)) {
> +		dev_err(&pdev->dev, "failed to register clock: %ld\n",
> +			PTR_ERR(clk));
>   		return PTR_ERR(clk);
> +	}
>   
> -	return of_clk_add_provider(pdev->dev.of_node,
> -				   of_clk_src_simple_get, clk);
> +	ret = of_clk_add_provider(pdev->dev.of_node,
> +				  of_clk_src_simple_get, clk);
> +	if (ret)
> +		dev_err(&pdev->dev, "failed to add clock provider: %d\n", ret);
> +
> +	return ret;
>   }
>   
>   int clk_pwm_remove(struct platform_device *pdev)
>
>


^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2014-12-09 13:09 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-11-03  9:31 [PATCH v2] clk: Add PWM clock driver Philipp Zabel
2014-11-25  7:07 ` Mike Turquette
2014-11-25  7:56   ` Philipp Zabel
2014-12-04 18:00 ` Janusz Użycki
2014-12-08 20:03   ` Janusz Użycki
2014-12-09  9:15     ` Philipp Zabel
2014-12-09 13:09       ` Janusz Użycki

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).