lkml.org 
[lkml]   [2017]   [Jul]   [11]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
Date
SubjectRe: [PATCH v2 1/7] pwm: rockchip: Add APB and function both clocks support
Hi,

On Fri, Jul 7, 2017 at 9:03 PM, David Wu <david.wu@rock-chips.com> wrote:

> @@ -6,7 +6,13 @@ Required properties:
> "rockchip,rk3288-pwm": found on RK3288 SoC
> "rockchip,vop-pwm": found integrated in VOP on RK3288 SoC
> - reg: physical base address and length of the controller's registers
> - - clocks: phandle and clock specifier of the PWM reference clock
> + - clocks: See ../clock/clock-bindings.txt
> + - For older hardware (rk2928, rk3066, rk3188, rk3228, rk3288, rk3399):
> + - There is one clock that's used both to derive the functional clock
> + for the device and as the bus clock.
> + - For newer hardware (rk3328 and future socs): specified by name
> + - "pwm": This is used to derive the functional clock.
> + - "pclk": This is the APB bus clock.

I'm pretty sure that that the above description doesn't quite match the code.

* The above description says that for old hardware there is one clock
and 'clock-names' was not necessary (though as I understand it it's OK
if it's there).

* The old code matched the old description. AKA: if there was no
"clock-names" then everything was OK.

* The new code will not work if there was no "clock-names".

Many of the old devices had a clock-names present (and it was "pwm"),
but not all. Specifically it looks like
"arch/arm/boot/dts/rk3xxx.dtsi" doesn't specify a clock-names.

> @@ -343,13 +344,38 @@ static int rockchip_pwm_probe(struct platform_device *pdev)
> if (IS_ERR(pc->base))
> return PTR_ERR(pc->base);
>
> - pc->clk = devm_clk_get(&pdev->dev, NULL);
> - if (IS_ERR(pc->clk))
> - return PTR_ERR(pc->clk);
> + pc->clk = devm_clk_get(&pdev->dev, "pwm");
> + count = of_property_count_strings(pdev->dev.of_node, "clock-names");
> + if (count == 2)
> + pc->pclk = devm_clk_get(&pdev->dev, "pclk");
> + else
> + pc->pclk = pc->clk;
> +
> + if (IS_ERR(pc->clk)) {
> + ret = PTR_ERR(pc->clk);
> + if (ret != -EPROBE_DEFER)
> + dev_err(&pdev->dev, "Can't get bus clk: %d\n", ret);
> + return ret;
> + }
> +
> + if (IS_ERR(pc->pclk)) {
> + ret = PTR_ERR(pc->pclk);
> + if (ret != -EPROBE_DEFER)
> + dev_err(&pdev->dev, "Can't get APB clk: %d\n", ret);
> + return ret;
> + }

In the above code you need to check the count _before_ trying to get
the clock by name.


-Doug

\
 
 \ /
  Last update: 2017-07-11 19:04    [W:0.169 / U:1.600 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site