Re: [PATCH v2] clk: Add PWM clock driver

From: Janusz UÅycki
Date: Tue Dec 09 2014 - 08:09:56 EST

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:

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

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>;
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

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;
+ } = "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)

To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at
Please read the FAQ at