From patchwork Tue Jun 25 15:05:59 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Michael Grzeschik X-Patchwork-Id: 2777441 Return-Path: X-Original-To: patchwork-linux-arm@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.19.201]) by patchwork2.web.kernel.org (Postfix) with ESMTP id 4566EC0AB1 for ; Tue, 25 Jun 2013 15:06:46 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id DE958202EB for ; Tue, 25 Jun 2013 15:06:44 +0000 (UTC) Received: from casper.infradead.org (casper.infradead.org [85.118.1.10]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 1F39B202EA for ; Tue, 25 Jun 2013 15:06:43 +0000 (UTC) Received: from merlin.infradead.org ([2001:4978:20e::2]) by casper.infradead.org with esmtps (Exim 4.80.1 #2 (Red Hat Linux)) id 1UrUpK-0001gu-KT; Tue, 25 Jun 2013 15:06:38 +0000 Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.80.1 #2 (Red Hat Linux)) id 1UrUpI-0002rC-1s; Tue, 25 Jun 2013 15:06:36 +0000 Received: from metis.ext.pengutronix.de ([2001:6f8:1178:4:290:27ff:fe1d:cc33]) by merlin.infradead.org with esmtps (Exim 4.80.1 #2 (Red Hat Linux)) id 1UrUpD-0002qN-6D for linux-arm-kernel@lists.infradead.org; Tue, 25 Jun 2013 15:06:32 +0000 Received: from ptx.hi.pengutronix.de ([2001:6f8:1178:2:5054:ff:fec0:8e10] ident=Debian-exim) by metis.ext.pengutronix.de with esmtp (Exim 4.72) (envelope-from ) id 1UrUoi-0000Wr-Ny; Tue, 25 Jun 2013 17:06:00 +0200 Received: from mgr by ptx.hi.pengutronix.de with local (Exim 4.72) (envelope-from ) id 1UrUoi-0004Jq-0F; Tue, 25 Jun 2013 17:06:00 +0200 Date: Tue, 25 Jun 2013 17:05:59 +0200 From: Michael Grzeschik To: Alexander Shiyan Subject: Re: [PATCH] ARM: dts: imx: fix clocks for cspi Message-ID: <20130625150559.GA3150@pengutronix.de> References: <1369309085-31146-1-git-send-email-jonas@microbit.se> <1369319290.379285076@f398.i.mail.ru> <20130523173306.GT32299@pengutronix.de> <1369369002.837105175@f322.mail.ru> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <1369369002.837105175@f322.mail.ru> X-Sent-From: Pengutronix Hildesheim X-URL: http://www.pengutronix.de/ X-IRC: #ptxdist @freenode X-Accept-Language: de,en X-Accept-Content-Type: text/plain X-Uptime: 17:00:47 up 12 days, 3:11, 17 users, load average: 0.01, 0.02, 0.05 User-Agent: Mutt/1.5.20 (2009-06-14) X-SA-Exim-Connect-IP: 2001:6f8:1178:2:5054:ff:fec0:8e10 X-SA-Exim-Mail-From: mgr@pengutronix.de X-SA-Exim-Scanned: No (on metis.ext.pengutronix.de); SAEximRunCond expanded to false X-PTX-Original-Recipient: linux-arm-kernel@lists.infradead.org X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20130625_110631_750168_42A183F7 X-CRM114-Status: GOOD ( 34.85 ) X-Spam-Score: -3.2 (---) Cc: Jonas Andersson , Sascha Hauer , linux-arm-kernel@lists.infradead.org X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.org X-Spam-Status: No, score=-5.5 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_MED, RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP Hi, On Fri, May 24, 2013 at 08:16:42AM +0400, Alexander Shiyan wrote: > > > > On Thu, May 23, 2013 at 04:10:59PM +0400, Alexander Shiyan wrote: > > > > > > > The CSPI controller has only one clock, but the driver spi-imx.c needs clock "per" to calculate bitrate divisor. > > > > > > > > > > > > > > Signed-off-by: Jonas Andersson > > > > > > > --- > > > > > > > arch/arm/boot/dts/imx25.dtsi | 12 ++++++------ > > > > > > > arch/arm/boot/dts/imx27.dtsi | 6 +++--- > > > > > > > arch/arm/boot/dts/imx51.dtsi | 2 +- > > > > > > > arch/arm/boot/dts/imx53.dtsi | 2 +- > > > > > > > 4 files changed, 11 insertions(+), 11 deletions(-) > > > > > > > > > > > > > > diff --git a/arch/arm/boot/dts/imx25.dtsi b/arch/arm/boot/dts/imx25.dtsi > > > > > > > index d2550e0..7011539 100644 > > > > > > > --- a/arch/arm/boot/dts/imx25.dtsi > > > > > > > +++ b/arch/arm/boot/dts/imx25.dtsi > > > > > > > @@ -141,8 +141,8 @@ > > > > > > > #size-cells = <0>; > > > > > > > compatible = "fsl,imx25-cspi", "fsl,imx35-cspi"; > > > > > > > reg = <0x43fa4000 0x4000>; > > > > > > > - clocks = <&clks 62>; > > > > > > > - clock-names = "ipg"; > > > > > > > + clocks = <&clks 62>, <&clks 62>; > > > > > > > + clock-names = "ipg", "per"; > > > > > > > interrupts = <14>; > > > > > > > status = "disabled"; > > > > > > > }; > > > > > > > > > > Sorry, typo. > > > > > > > > > > > Why you do not use "dummy" clock for "per" here? > > > > > > > > > > *for "ipg" > > > > > > > > Because the same clock is used for the register clock and the baudrate > > > > clock. You have to enable the ipg clock to access registers. > > > > > > I am not see any "ipg" clock usage in the spi driver. > > > > It's the register clock. The driver enables it in order to access the > > registers. > > > > > If "ipg" clock is > > > a dependency for "per" clock, it should be registered in ccm as parent. > > > > Look, the eCSPI unit has two clock inputs, one for accessing the > > registers (ipg) and one for generating the SPI bit clock (per). Now the > > CSPI unit only has a single clock input, so the 100% correct way would > > be to specify only a single clock for this unit. However, since we > > handle both units with the same driver the simplest way to cope with > > it is to provide the very same clock twice, once for the register access > > and once for generating the SPI bit clock. > > And yes, we have to specify the real clock twice and can't replace one > > with a dummy clock, because if we specify a dummy clock for the ipg > > clock, then the driver couldn't access the registers even if it enabled > > the clock. If we would replace the SPI bit clock with a dummy clock then > > the device couldn't send data even if the driver enabled the bit clock. > > (That of course only becomes relevant when the driver actually > > en/disables the clocks during runtime and not only once during probe). > > Well, I completely forgot about the clock for registers. > Everything looks right then. > However, it would be nice to add a few words about the clocks in the > Documentation/devicetree/bindings/spi/fsl-imx~spi.txt > Thanks for the clarification. the spi device i got, only keeps working if having the per2_gate enabled. It also seems that we lost that dependency, as the file arch/arm/mach-imx/clk-imx27.c contains this: clk_register_clkdev(clk[per2_gate], "per", "imx27-cspi.0"); clk_register_clkdev(clk[cspi1_ipg_gate], "ipg", "imx27-cspi.0"); clk_register_clkdev(clk[per2_gate], "per", "imx27-cspi.1"); clk_register_clkdev(clk[cspi2_ipg_gate], "ipg", "imx27-cspi.1"); clk_register_clkdev(clk[per2_gate], "per", "imx27-cspi.2"); clk_register_clkdev(clk[cspi3_ipg_gate], "ipg", "imx27-cspi.2"); So my suggestion is as follows: From: Michael Grzeschik Subject: [PATCH] ARM: dts: imx: fix clocks for cspi The CSPI controller needs the per2_gate clock to work. Signed-off-by: Michael Grzeschik --- arch/arm/boot/dts/imx27.dtsi | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/arch/arm/boot/dts/imx27.dtsi b/arch/arm/boot/dts/imx27.dtsi index 7f7d4a5..14683c3 100644 --- a/arch/arm/boot/dts/imx27.dtsi +++ b/arch/arm/boot/dts/imx27.dtsi @@ -169,7 +169,7 @@ compatible = "fsl,imx27-cspi"; reg = <0x1000e000 0x1000>; interrupts = <16>; - clocks = <&clks 53>, <&clks 53>; + clocks = <&clks 53>, <&clks 60>; clock-names = "ipg", "per"; status = "disabled"; }; @@ -180,7 +180,7 @@ compatible = "fsl,imx27-cspi"; reg = <0x1000f000 0x1000>; interrupts = <15>; - clocks = <&clks 52>, <&clks 52>; + clocks = <&clks 52>, <&clks 60>; clock-names = "ipg", "per"; status = "disabled"; }; @@ -345,7 +345,7 @@ compatible = "fsl,imx27-cspi"; reg = <0x10017000 0x1000>; interrupts = <6>; - clocks = <&clks 51>, <&clks 51>; + clocks = <&clks 51>, <&clks 60>; clock-names = "ipg", "per"; status = "disabled"; };