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

Am Mittwoch, 12. Juli 2017, 16:38:09 CEST schrieb David.Wu:
> Hi Doug,
>
> 在 2017/7/12 1:03, Doug Anderson 写道:
> > 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.
>
> So we can keep code: the pc->clk = devm_clk_get(&pdev->dev, NULL);
> If the name is NULL, we can get the first clk defined at DTB.

I don't think it will work that way.

clk_get with NULL argument will likely grab the first clock of the list
independent of clock-names being present.

It might be better to do something like [pseudo-code]:

pwm->pclk = clk_get( ..., "pclk");
if (IS_ERR(pwm->pclk))
pwm->pclk = NULL;

pwm->pwm_clk = clk_get(..., "pwm");
if (IS_ERR(pwm->pwm_clk))
pwm->pwm_clk = clk_get(..., NULL);
if (IS_ERR(pwm->pwm_clk))
return PTR_ERR(pwm->pwm_clk);

That way, you make your way backwards through the pwm-binding-history.
Ending at the single-clock without clock-names property as the last fallback.


Heiko

\
 
 \ /
  Last update: 2017-07-12 21:09    [W:0.035 / U:2.040 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site