Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753276AbdGLTJq convert rfc822-to-8bit (ORCPT ); Wed, 12 Jul 2017 15:09:46 -0400 Received: from gloria.sntech.de ([95.129.55.99]:53648 "EHLO gloria.sntech.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752434AbdGLTJo (ORCPT ); Wed, 12 Jul 2017 15:09:44 -0400 From: Heiko =?ISO-8859-1?Q?St=FCbner?= To: "David.Wu" Cc: Doug Anderson , Thierry Reding , Boris Brezillon , Rob Herring , Catalin Marinas , Brian Norris , Mark Rutland , =?utf-8?B?6buE5rab?= , linux-pwm , "linux-arm-kernel@lists.infradead.org" , "open list:ARM/Rockchip SoC..." , "devicetree@vger.kernel.org" , "linux-kernel@vger.kernel.org" Subject: Re: [PATCH v2 1/7] pwm: rockchip: Add APB and function both clocks support Date: Wed, 12 Jul 2017 21:09:30 +0200 Message-ID: <2474407.Xar3q3nCx6@diego> User-Agent: KMail/5.2.3 (Linux/4.8.0-2-amd64; KDE/5.27.0; x86_64; ; ) In-Reply-To: References: <1499486629-9659-1-git-send-email-david.wu@rock-chips.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8BIT Content-Type: text/plain; charset="UTF-8" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2384 Lines: 67 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 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