From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1423841AbdEYIYR (ORCPT ); Thu, 25 May 2017 04:24:17 -0400 Received: from mail.kmu-office.ch ([178.209.48.109]:41576 "EHLO mail.kmu-office.ch" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1423676AbdEYIXT (ORCPT ); Thu, 25 May 2017 04:23:19 -0400 MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Date: Thu, 25 May 2017 01:22:27 -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: <8f039bb6-9bc3-349a-cfe8-727e4caf41de@nxp.com> References: <20170422012338.4635-1-stefan@agner.ch> <20170422012338.4635-5-stefan@agner.ch> <48b5c543e713f9597b67a39a51ffd521@agner.ch> <8f039bb6-9bc3-349a-cfe8-727e4caf41de@nxp.com> Message-ID: <0149534ab05adac384070fe432d83c48@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 15:59, Han Xu wrote: > On 05/04/2017 04:50 PM, Stefan Agner wrote: >> 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://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatchwork.ozlabs.org%2Fpatch%2F551967%2F&data=01%7C01%7Chan.xu%40nxp.com%7C98493f7e475341dbf67008d49337b441%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0&sdata=FH9k%2FC4VcVCeyidkRgUJ5Ahu4%2B%2FlDAjXVwO0DY7XMhA%3D&reserved=0 >>> >> 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). > > It also surprised me how it can work on your side, I remember we made > the code change for some purpose, please give me some time I can check > if the doc is wrong. > Hi Han, Do you have an update to this? Otherwise, I will send out a version which only uses IMX7D_NAND_USDHC_BUS_ROOT_CLK for now. This binding is already defined and needs no driver change. We can still extend the binding (and ddriver) when it turns out it is actually required. -- Stefan >> -- >> 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"; >>>> + }; >>>> }; >>>> };