From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752261AbdEDVvZ (ORCPT ); Thu, 4 May 2017 17:51:25 -0400 Received: from mail.kmu-office.ch ([178.209.48.109]:41378 "EHLO mail.kmu-office.ch" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751351AbdEDVvX (ORCPT ); Thu, 4 May 2017 17:51:23 -0400 MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Date: Thu, 04 May 2017 14:50:56 -0700 From: Stefan Agner To: Han Xu Cc: dwmw2@infradead.org, computersforpeace@gmail.com, boris.brezillon@free-electrons.com, marek.vasut@gmail.com, richard@nod.at, cyrille.pitchen@atmel.com, robh+dt@kernel.org, mark.rutland@arm.com, shawnguo@kernel.org, kernel@pengutronix.de, fabio.estevam@freescale.com, "LW@KARO-electronics.de" , linux-mtd@lists.infradead.org, devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH v2 4/5] ARM: dts: imx7: add GPMI NAND In-Reply-To: References: <20170422012338.4635-1-stefan@agner.ch> <20170422012338.4635-5-stefan@agner.ch> Message-ID: <48b5c543e713f9597b67a39a51ffd521@agner.ch> User-Agent: Roundcube Webmail/1.2.5 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 2017-05-04 12:13, Han Xu wrote: > On 04/21/2017 08:23 PM, Stefan Agner wrote: >> Add i.MX 7 GPMI NAND module. >> >> Signed-off-by: Stefan Agner >> --- >> arch/arm/boot/dts/imx7s.dtsi | 31 +++++++++++++++++++++++++++++++ >> 1 file changed, 31 insertions(+) >> >> diff --git a/arch/arm/boot/dts/imx7s.dtsi b/arch/arm/boot/dts/imx7s.dtsi >> index 843eb379e1ea..9645257638d4 100644 >> --- a/arch/arm/boot/dts/imx7s.dtsi >> +++ b/arch/arm/boot/dts/imx7s.dtsi >> @@ -995,5 +995,36 @@ >> status = "disabled"; >> }; >> }; >> + >> + dma_apbh: dma-apbh@33000000 { >> + compatible = "fsl,imx7d-dma-apbh", "fsl,imx28-dma-apbh"; >> + reg = <0x33000000 0x2000>; >> + interrupts = , >> + , >> + , >> + ; >> + interrupt-names = "gpmi0", "gpmi1", "gpmi2", "gpmi3"; >> + #dma-cells = <1>; >> + dma-channels = <4>; >> + clocks = <&clks IMX7D_NAND_USDHC_BUS_ROOT_CLK>, >> + <&clks IMX7D_NAND_ROOT_CLK>; >> + clock-names = "dma_apbh_bch", "dma_apbh_io"; >> + }; > Do you need some patches to enable all clks for APBH DMA? Refer to > https://patchwork.ozlabs.org/patch/551967/ > Oh I see, with the current code and this device tree we actually only enable the first clock... But it seems to work in practice... What is interesting, the chapter 5.2.5 System Clocks only describes APBHDMA, and no GPMI module. Chapter 9.4 about APBH-Bridge-DMA bridge only specifies "hclk" as clock sources. When looking at the table in chapter 5.2.5, hclk is NAND_USDHC_BUS_CLK_ROOT Module Syste Module Clock Clock Root APBHDMA apbhdma.hclk NAND_USDHC_BUS_CLK_ROOT So my guess is that the actual DMA controller should work fine with IMX7D_NAND_USDHC_BUS_ROOT_CLK only. It is only the GPMI module which needs the NAND_ROOT_CLK. My guess is since the APBHDMA controller is only used for GPMI these days, it has been combined in one module in the table in chapter 5.2.5. So we could actually only specify: clocks = <&clks IMX7D_NAND_USDHC_BUS_ROOT_CLK>; Tested with this and NAND seems to work fine. Also tested accessing APBHDMA register in U-Boot with IMX7D_NAND_ROOT_CLK disabled, it worked fine. What do you think? Should we go with this? This probably even saves power since GPMI's runtime clock management disables IMX7D_NAND_ROOT_CLK when NAND is idle (according to the nand_root_clk counter in clk_summary and the root clock register at 0x3038AA00). -- Stefan >> + >> + gpmi: gpmi-nand@33002000{ >> + compatible = "fsl,imx7d-gpmi-nand"; >> + #address-cells = <1>; >> + #size-cells = <1>; >> + reg = <0x33002000 0x2000>, <0x33004000 0x4000>; >> + reg-names = "gpmi-nand", "bch"; >> + interrupts = ; >> + interrupt-names = "bch"; >> + clocks = <&clks IMX7D_NAND_ROOT_CLK>, >> + <&clks IMX7D_NAND_USDHC_BUS_ROOT_CLK>; >> + clock-names = "gpmi_io", "gpmi_bch_apb"; >> + dmas = <&dma_apbh 0>; >> + dma-names = "rx-tx"; >> + status = "disabled"; >> + }; >> }; >> };