From: Stefan Agner <stefan@agner.ch>
To: Han Xu <han.xu@nxp.com>
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" <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
Date: Thu, 25 May 2017 01:22:27 -0700 [thread overview]
Message-ID: <0149534ab05adac384070fe432d83c48@agner.ch> (raw)
In-Reply-To: <8f039bb6-9bc3-349a-cfe8-727e4caf41de@nxp.com>
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 <stefan@agner.ch>
>>>> ---
>>>> 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 = <GIC_SPI 12 IRQ_TYPE_LEVEL_HIGH>,
>>>> + <GIC_SPI 12 IRQ_TYPE_LEVEL_HIGH>,
>>>> + <GIC_SPI 12 IRQ_TYPE_LEVEL_HIGH>,
>>>> + <GIC_SPI 12 IRQ_TYPE_LEVEL_HIGH>;
>>>> + 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 = <GIC_SPI 14 IRQ_TYPE_LEVEL_HIGH>;
>>>> + 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";
>>>> + };
>>>> };
>>>> };
next prev parent reply other threads:[~2017-05-25 8:24 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-04-22 1:23 [PATCH v2 0/5] mtd: nand: gpmi: add i.MX 7 support Stefan Agner
2017-04-22 1:23 ` [PATCH v2 1/5] mtd: nand: gpmi: unify clock handling Stefan Agner
2017-04-22 1:23 ` [PATCH v2 2/5] mtd: nand: gpmi: add i.MX 7 SoC support Stefan Agner
2017-04-22 1:23 ` [PATCH v2 3/5] mtd: gpmi: document current clock requirements Stefan Agner
2017-04-28 17:13 ` Rob Herring
2017-04-22 1:23 ` [PATCH v2 4/5] ARM: dts: imx7: add GPMI NAND Stefan Agner
2017-05-04 19:13 ` Han Xu
2017-05-04 21:50 ` Stefan Agner
2017-05-04 22:59 ` Han Xu
2017-05-25 8:22 ` Stefan Agner [this message]
2017-04-22 1:23 ` [PATCH v2 5/5] ARM: dts: imx7-colibri: add NAND support Stefan Agner
2017-04-22 11:43 ` [PATCH v2 0/5] mtd: nand: gpmi: add i.MX 7 support Marek Vasut
2017-05-03 13:25 ` Shawn Guo
2017-05-15 19:38 ` Boris Brezillon
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=0149534ab05adac384070fe432d83c48@agner.ch \
--to=stefan@agner.ch \
--cc=LW@karo-electronics.de \
--cc=boris.brezillon@free-electrons.com \
--cc=computersforpeace@gmail.com \
--cc=cyrille.pitchen@atmel.com \
--cc=devicetree@vger.kernel.org \
--cc=dwmw2@infradead.org \
--cc=fabio.estevam@freescale.com \
--cc=han.xu@nxp.com \
--cc=kernel@pengutronix.de \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mtd@lists.infradead.org \
--cc=marek.vasut@gmail.com \
--cc=mark.rutland@arm.com \
--cc=richard@nod.at \
--cc=robh+dt@kernel.org \
--cc=shawnguo@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).