From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jacky Bai Subject: RE: [PATCH v2 1/3] arm64: dts: imx: Add i.mx8mm dtsi support Date: Fri, 22 Mar 2019 01:55:45 +0000 Message-ID: References: <1552369779-7614-1-git-send-email-ping.bai@nxp.com> <20190321082253.GD12513@dragon> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <20190321082253.GD12513@dragon> Content-Language: en-US List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=m.gmane.org@lists.infradead.org To: Shawn Guo Cc: "mark.rutland@arm.com" , Aisheng Dong , "devicetree@vger.kernel.org" , "s.hauer@pengutronix.de" , "robh+dt@kernel.org" , dl-linux-imx , "kernel@pengutronix.de" , "festevam@gmail.com" , "linux-arm-kernel@lists.infradead.org" , "l.stach@pengutronix.de" List-Id: devicetree@vger.kernel.org > Subject: Re: [PATCH v2 1/3] arm64: dts: imx: Add i.mx8mm dtsi support > > On Tue, Mar 12, 2019 at 05:44:53AM +0000, Jacky Bai wrote: > > The i.MX8M Mini is new SOC of the i.MX8M family. it is focused on > > delivering the latest and greatest video and audio experience > > combining state-of-the-art media-specific features with > > high-performance processing while optimized for lowest power > > consumption. The i.MX 8M Mini Media Applications Processor is 14nm > > FinFET product of the growing i.MX8M family targeting the consumer & > > industrial market. It is built in 14LPP to achieve both high > > performance and low power consumption and relies on a powerful fully > > coherent core complex based on a quad Cortex-A53 cluster with video > > and graphics accelerators > > > > This patch adds the basic dtsi support for i.MX8MM. > > > > Signed-off-by: Jacky Bai > > --- > > change v1->v2: > > - removed unnecessary 'okay' status > > - change AIPS's address-cells and size-cells to '1' > > - remove sdma's undocumented property > > --- > > arch/arm64/boot/dts/freescale/imx8mm.dtsi | 717 > > ++++++++++++++++++++++++++++++ > > 1 file changed, 717 insertions(+) > > create mode 100644 arch/arm64/boot/dts/freescale/imx8mm.dtsi > > > > diff --git a/arch/arm64/boot/dts/freescale/imx8mm.dtsi > > b/arch/arm64/boot/dts/freescale/imx8mm.dtsi ... > > + }; > > + > > + soc@0 { > > The unit-address 0 makes no sense here, right? Yes, I will remove this in V3. > > + gpio5: gpio@30240000 { > > + compatible = "fsl,imx8mm-gpio", "fsl,imx35-gpio"; > > + reg = <0x30240000 0x10000>; > > + interrupts = , > > + ; > > + gpio-controller; > > + #gpio-cells = <2>; > > + interrupt-controller; > > + #interrupt-cells = <2>; > > + }; > > + > > + wdog1: wdog@30280000 { > > watchdog for node name. ok, will fix it. > > > + compatible = "fsl,imx21-wdt"; > > Don't you need a "fsl,imx8mm-wdt"? > ok, I will add it. > > + > > + sdma2: dma-controller@302c0000 { > > + compatible = "fsl,imx8mq-sdma", "fsl,imx7d-sdma"; > > Should be fsl,imx8mm-sdma? OK, will fix it. > > > + reg = <0x302c0000 0x10000>; > > + interrupts = ; > > + clocks = <&clk IMX8MM_CLK_SDMA2_ROOT>, > > + <&clk IMX8MM_CLK_SDMA2_ROOT>; > > + clock-names = "ipg", "ahb"; > > + #dma-cells = <3>; > > + fsl,sdma-ram-script-name = "imx/sdma/sdma-imx7d.bin"; > > i.mx8mm reuses the exactly same SDMA RAM script as imx7d? > Yes, imx8mm reuse the same firmware at present. > > + > > + ocotp: ocotp-ctrl@30350000 { > > + compatible = "fsl,imx8mq-ocotp", "fsl,imx7d-ocotp", > "syscon"; > > Should be fsl,imx8mm-ocotp? Ok, will fix it. > > > + reg = <0x30350000 0x10000>; > > + clocks = <&clk IMX8MM_CLK_OCOTP_ROOT>; > > + /* For nvmem subnodes */ > > + #address-cells = <1>; > > + #size-cells = <1>; > > + }; > > + > > + anatop: anatop@30360000 { > > + compatible = "fsl,imx8mm-anatop", "syscon", > "simple-bus"; > > + reg = <0x30360000 0x10000>; > > + }; > > + > > + snvs: snvs@30370000 { > > + compatible = "fsl,sec-v4.0-mon","syscon", "simple-mfd"; > > + reg = <0x30370000 0x10000>; > > + > > + snvs_rtc: snvs-rtc-lp{ > > Miss one space before {. Ok, will fix it. > > > + compatible = "fsl,sec-v4.0-mon-rtc-lp"; > > + regmap =<&snvs>; > > Miss one space after =. > Thanks, will fix it > > + offset = <0x34>; > > + interrupts = , > > + ; > > + }; > > + > > + snvs_pwrkey: snvs-powerkey { > > + compatible = "fsl,sec-v4.0-pwrkey"; > > + regmap = <&snvs>; > > + interrupts = ; > > + linux,keycode = ; > > + wakeup-source; > > + }; > > + }; > > + > > + clk: clock-controller@30380000 { > > + compatible = "fsl,imx8mm-ccm"; > > + reg = <0x30380000 0x10000>; > > + #clock-cells = <1>; > > + clocks = <&osc_32k>, <&osc_24m>, <&clk_ext1>, > <&clk_ext2>, > > + <&clk_ext3>, <&clk_ext4>; > > + clock-names = "osc_32k", "osc_24m", "clk_ext1", > "clk_ext2", > > + "clk_ext3", "clk_ext4"; > > + }; > > + > > + src: src@30390000 { > > reset-controller for node name? > Ok, will fix it. > > + compatible = "fsl,imx8mm-src", "syscon"; > > + reg = <0x30390000 0x10000>; > > + interrupts = ; > > + #reset-cells = <1>; > > + }; > > + > > + gpc: gpc@303a0000 { > > interrupt-controller for node name? > For the i.MX8MM and future i.MX8M serious, it is not very necessary to use GPC as a interrupt controller in linux kernel side. For system suspend, ATF can config the GPC IMRs based on IRQ enable status in GICv3. For cpuidle support, GICv3 has per-CPU core wakeup signals connected to the GPC logic, so GICv3 has the ability to wake up the corresponding CPU core if the IRQ is routed to that CPU core. Additionally, I am thinking to config the GPC as a secure resource in the future. Maybe, we can remove the GPC node for now. If necessary, we can add it back again? BTW, i.MX8MQ is a special case due to some HW issue, GPC still need to be used as a interrupt controller in kernel. > > + compatible = "fsl,imx8mm-gpc", "syscon"; > > + reg = <0x303a0000 0x10000>; > > + interrupt-controller; > > + interrupts = ; > > + #interrupt-cells = <3>; > > + }; > > + }; ... > > + aips3: bus@30800000 { > > + compatible = "fsl,aips-bus", "simple-bus"; > > + #address-cells = <1>; > > + #size-cells = <1>; > > + ranges; > > + > > + ecspi1: ecspi@30820000 { > > spi for node name? > OK, will fix it. > > + #address-cells = <1>; > > + #size-cells = <0>; > > We usually start properties with compatible. Can these be moved > somewhere after compatible? > Ok, will fix it > > + compatible = "fsl,imx8mm-ecspi", "fsl,imx51-ecspi"; > > + reg = <0x30820000 0x10000>; > > + interrupts = ; > > + clocks = <&clk IMX8MM_CLK_ECSPI1_ROOT>, > > + <&clk IMX8MM_CLK_ECSPI1_ROOT>; > > + clock-names = "ipg", "per"; > > + dmas = <&sdma1 0 7 1>, <&sdma1 1 7 2>; > > + dma-names = "rx", "tx"; > > + status = "disabled"; > > + }; .... > > + > > + uart1: serial@30860000 { > > + compatible = "fsl,imx8mm-uart", > > + "fsl,imx6q-uart", "fsl,imx21-uart"; > > I think "fsl,imx21-uart" can be dropped in this case? > Ok, I will remove this. > > + reg = <0x30860000 0x10000>; > > + interrupts = ; > > + clocks = <&clk IMX8MM_CLK_UART1_ROOT>, > > + <&clk IMX8MM_CLK_UART1_ROOT>; > > + clock-names = "ipg", "per"; > > + dmas = <&sdma1 22 4 0>, <&sdma1 23 4 0>; > > + dma-names = "rx", "tx"; > > + status = "disabled"; > > + }; > > + ... > > + > > + usdhc1: mmc@30b40000 { > > + compatible = "fsl,imx8mq-usdhc", "fsl,imx7d-usdhc"; > > Should be fsl,imx8mm-usdhc? > Ok, will fix it. > > + reg = <0x30b40000 0x10000>; > > + interrupts = ; > > + clocks = <&clk IMX8MM_CLK_DUMMY>, > > + <&clk IMX8MM_CLK_NAND_USDHC_BUS>, > > + <&clk IMX8MM_CLK_USDHC1_ROOT>; > > + clock-names = "ipg", "ahb", "per"; > > + assigned-clocks = <&clk IMX8MM_CLK_USDHC1>; > > + assigned-clock-rates = <400000000>; > > + fsl,tuning-start-tap = <20>; > > + fsl,tuning-step= <2>; > > + bus-width = <4>; > > + status = "disabled"; > > + }; > > + ... > > + > > + sdma1: dma-controller@30bd0000 { > > + compatible = "fsl,imx8mq-sdma", "fsl,imx7d-sdma"; > > Should be fsl,imx8mm-sdma? > Ok, will update it. > > + reg = <0x30bd0000 0x10000>; > > + interrupts = ; > > + clocks = <&clk IMX8MM_CLK_SDMA1_ROOT>, > > + <&clk IMX8MM_CLK_SDMA1_ROOT>; > > + clock-names = "ipg", "ahb"; > > + #dma-cells = <3>; > > + fsl,sdma-ram-script-name = "imx/sdma/sdma-imx7d.bin"; > > + }; > > + > > + fec1: ethernet@30be0000 { > > + compatible = "fsl,imx8mq-fec", "fsl,imx6sx-fec"; > > Should be fsl,imx8mm-fec? > Ok, will update it > > + reg = <0x30be0000 0x10000>; > > + interrupts = , > > + , > > + ; > > + clocks = <&clk IMX8MM_CLK_ENET1_ROOT>, > > + <&clk IMX8MM_CLK_ENET1_ROOT>, > > + <&clk IMX8MM_CLK_ENET_TIMER>, > > + <&clk IMX8MM_CLK_ENET_REF>, > > + <&clk IMX8MM_CLK_ENET_PHY_REF>; > > + clock-names = "ipg", "ahb", "ptp", > > + "enet_clk_ref", "enet_out"; > > + assigned-clocks = <&clk IMX8MM_CLK_ENET_AXI>, > > + <&clk IMX8MM_CLK_ENET_TIMER>, > > + <&clk IMX8MM_CLK_ENET_REF>, > > + <&clk IMX8MM_CLK_ENET_TIMER>; > > + assigned-clock-parents = <&clk > IMX8MM_SYS_PLL1_266M>, > > + <&clk IMX8MM_SYS_PLL2_100M>, > > + <&clk IMX8MM_SYS_PLL2_125M>; > > + assigned-clock-rates = <0>, <0>, <125000000>, > <100000000>; > > + stop-mode = <&gpr 0x10 3>; > > vendor property? > Yes, will remove it. > > + fsl,num-tx-queues=<3>; > > + fsl,num-rx-queues=<3>; > > Miss space before and after =. > Thanks, will fix it. > > + fsl,wakeup_irq = <2>; > > vendor property? > Yes, will remove it > > + status = "disabled"; > > + }; > > + > > + }; > > + > > + aips4: bus@32c00000 { > > + compatible = "fsl,aips-bus", "simple-bus"; > > + #address-cells = <1>; > > + #size-cells = <1>; > > + ranges; > > + > > + usbotg1: usb@32e40000 { > > + compatible = "fsl,imx8mm-usb", "fsl,imx7d-usb", > "fsl,imx27-usb"; > > fsl,imx27-usb can be saved? ok, will remove it. > > > + reg = <0x32e40000 0x200>; > > + interrupts = ; > > + clocks = <&clk IMX8MM_CLK_USB1_CTRL_ROOT>; > > + clock-names = "usb1_ctrl_root_clk"; > > + assigned-clocks = <&clk IMX8MM_CLK_USB_BUS>, > > + <&clk IMX8MM_CLK_USB_CORE_REF>; > > + assigned-clock-parents = <&clk > IMX8MM_SYS_PLL2_500M>, > > + <&clk IMX8MM_SYS_PLL1_100M>; > > + fsl,usbphy = <&usbphynop1>; > > + fsl,usbmisc = <&usbmisc1 0>; > > + status = "disabled"; > > + }; > > + > > + usbphynop1: usbphynop1 { > > + compatible = "usb-nop-xceiv"; > > + clocks = <&clk IMX8MM_CLK_USB_PHY_REF>; > > + assigned-clocks = <&clk IMX8MM_CLK_USB_PHY_REF>; > > + assigned-clock-parents = <&clk > IMX8MM_SYS_PLL1_100M>; > > + clock-names = "main_clk"; > > + }; > > + > > + usbmisc1: usbmisc@32e40200 { > > + #index-cells = <1>; > > Move it afterwards? > OK, thanks. > > + compatible = "fsl,imx7d-usbmisc", "fsl,imx6q-usbmisc"; > > Should be compatible = "fsl,imx8mm-usbmisc", "fsl,imx7d-usbmisc"? > Ok, will fix it. > > + reg = <0x32e40200 0x200>; > > + }; > > + > > + usbotg2: usb@32e50000 { > > + compatible = "fsl,imx8mm-usb", "fsl,imx7d-usb", > "fsl,imx27-usb"; > > + reg = <0x32e50000 0x200>; > > + interrupts = ; > > + clocks = <&clk IMX8MM_CLK_USB1_CTRL_ROOT>; > > + clock-names = "usb1_ctrl_root_clk"; > > + assigned-clocks = <&clk IMX8MM_CLK_USB_BUS>, > > + <&clk IMX8MM_CLK_USB_CORE_REF>; > > + assigned-clock-parents = <&clk > IMX8MM_SYS_PLL2_500M>, > > + <&clk IMX8MM_SYS_PLL1_100M>; > > + fsl,usbphy = <&usbphynop2>; > > + fsl,usbmisc = <&usbmisc2 0>; > > + status = "disabled"; > > + }; > > + > > + usbphynop2: usbphynop2 { > > + compatible = "usb-nop-xceiv"; > > + clocks = <&clk IMX8MM_CLK_USB_PHY_REF>; > > + assigned-clocks = <&clk IMX8MM_CLK_USB_PHY_REF>; > > + assigned-clock-parents = <&clk > IMX8MM_SYS_PLL1_100M>; > > + clock-names = "main_clk"; > > + }; > > + > > + usbmisc2: usbmisc@32e50200 { > > + #index-cells = <1>; > > + compatible = "fsl,imx7d-usbmisc", "fsl,imx6q-usbmisc"; > > + reg = <0x32e50200 0x200>; > > + }; > > + > > + }; > > + > > + dma_apbh: dma-apbh@33000000 { > > dma-controller for node name. > Ok. Thanks > > + 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 = <&clk > IMX8MM_CLK_NAND_USDHC_BUS_RAWNAND_CLK>; > > + }; > > + > > + gpmi: gpmi-nand@33002000{ > > nand-controller for node name. > Ok, will fix it. > > + compatible = "fsl,imx7d-gpmi-nand"; > > fsl,imx8mm-gpmi-nand before it? > Ok, thanks. > > + #address-cells = <1>; > > + #size-cells = <1>; > > + reg = <0x33002000 0x2000>, <0x33004000 0x4000>; > > + reg-names = "gpmi-nand", "bch"; > > + interrupts = ; > > + interrupt-names = "bch"; > > + clocks = <&clk IMX8MM_CLK_NAND_ROOT>, > > + <&clk IMX8MM_CLK_NAND_USDHC_BUS_RAWNAND_CLK>; > > Miss indentation. > Thanks for review, will fix this. BR > Shawn > > > + clock-names = "gpmi_io", "gpmi_bch_apb"; > > + dmas = <&dma_apbh 0>; > > + dma-names = "rx-tx"; > > + status = "disabled"; > > + }; > > + }; > > +}; > > -- > > 1.9.1 > > From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.8 required=3.0 tests=DKIM_INVALID,DKIM_SIGNED, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY, SPF_PASS autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 733DEC43381 for ; Fri, 22 Mar 2019 01:56:03 +0000 (UTC) Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 3EBFB21900 for ; Fri, 22 Mar 2019 01:56:03 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=fail reason="signature verification failed" (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="bu/0RMAt"; dkim=fail reason="signature verification failed" (1024-bit key) header.d=nxp.com header.i=@nxp.com header.b="uQ8EddoW" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 3EBFB21900 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=nxp.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+infradead-linux-arm-kernel=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20170209; h=Sender: Content-Transfer-Encoding:Content-Type:Cc:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:MIME-Version:In-Reply-To:References: Message-ID:Date:Subject:To:From:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=ZoWJmlhJSvGWrRf00D5ed+cLcimBKxPUrMOdCxXavyU=; b=bu/0RMAtxhiO3L tAoADosX9qDMcortGzITCfSO0DyDA0ivDb8pB9ovNllh+MaiSHq4OdJ9WRFs44T85qDzKRjonNsvh gm0oK2YZmJQpi7gH2tA/51JXE/pxlQVU3JvsQrViB4vgoPgblqvhRdZhMveeU33rP08+oGkZckwSe +nHtj715Vjr9KJkaFOU9BYZreOhTRqU5KMHapoFCdxDttv4Pgk08HOuuujUkGMWo9ogIte7QbOH91 DgHnIhJJzknYqo7A6VGlAbi9Vxvsb6a+WbOQp67BAEmMdIvlichcrjByJ9tvOGtwjL0I9phcW7B6y 7Hd8hynTBUeQOzqpHayw==; Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.90_1 #2 (Red Hat Linux)) id 1h79Pa-00042S-E1; Fri, 22 Mar 2019 01:55:58 +0000 Received: from mail-eopbgr130059.outbound.protection.outlook.com ([40.107.13.59] helo=EUR01-HE1-obe.outbound.protection.outlook.com) by bombadil.infradead.org with esmtps (Exim 4.90_1 #2 (Red Hat Linux)) id 1h79PT-00041p-Fz for linux-arm-kernel@lists.infradead.org; Fri, 22 Mar 2019 01:55:53 +0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=nxp.com; s=selector1; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=EeqQJYhW8gnX3bwK++nG9II8OtUVs3w/JvJfpMNbQ30=; b=uQ8EddoWagN3zooxkCZ1t9qDz7H2cXcsATvXO+Kpcbm5ypyuf/1ADXYbXq8WVzLNyOyk3LrKNL6hOrH7ASJDHo+1zRq7tY9eyrR346lzQotCOolfUE94wvieDHonaTy3cXSOxEEty8Y1mjvT8I1u8SQyJZI9a4yWlVbfo7pqJMc= Received: from VI1PR0402MB3519.eurprd04.prod.outlook.com (52.134.4.24) by VI1PR0402MB2814.eurprd04.prod.outlook.com (10.172.255.20) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.1730.15; Fri, 22 Mar 2019 01:55:45 +0000 Received: from VI1PR0402MB3519.eurprd04.prod.outlook.com ([fe80::78bc:c1e8:3caf:656b]) by VI1PR0402MB3519.eurprd04.prod.outlook.com ([fe80::78bc:c1e8:3caf:656b%6]) with mapi id 15.20.1730.013; Fri, 22 Mar 2019 01:55:45 +0000 From: Jacky Bai To: Shawn Guo Subject: RE: [PATCH v2 1/3] arm64: dts: imx: Add i.mx8mm dtsi support Thread-Topic: [PATCH v2 1/3] arm64: dts: imx: Add i.mx8mm dtsi support Thread-Index: AQHU2Ja3mJEs7YgPYUao7JvFlPTTxaYVzWQAgAEXugA= Date: Fri, 22 Mar 2019 01:55:45 +0000 Message-ID: References: <1552369779-7614-1-git-send-email-ping.bai@nxp.com> <20190321082253.GD12513@dragon> In-Reply-To: <20190321082253.GD12513@dragon> Accept-Language: zh-CN, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: authentication-results: spf=none (sender IP is ) smtp.mailfrom=ping.bai@nxp.com; x-originating-ip: [119.31.174.71] x-ms-publictraffictype: Email x-ms-office365-filtering-correlation-id: f217b014-4d3f-4f29-3f08-08d6ae697f8f x-ms-office365-filtering-ht: Tenant x-microsoft-antispam: BCL:0; PCL:0; RULEID:(2390118)(7020095)(4652040)(8989299)(4534185)(4627221)(201703031133081)(201702281549075)(8990200)(5600127)(711020)(4605104)(4618075)(2017052603328)(7153060)(7193020); SRVR:VI1PR0402MB2814; x-ms-traffictypediagnostic: VI1PR0402MB2814: x-microsoft-antispam-prvs: x-forefront-prvs: 09840A4839 x-forefront-antispam-report: SFV:NSPM; SFS:(10009020)(346002)(396003)(366004)(136003)(376002)(39860400002)(199004)(189003)(256004)(81156014)(3846002)(6116002)(6916009)(86362001)(33656002)(25786009)(97736004)(4326008)(71200400001)(71190400001)(6436002)(14444005)(55016002)(9686003)(6246003)(68736007)(8936002)(81166006)(14454004)(8676002)(74316002)(305945005)(7736002)(105586002)(478600001)(229853002)(52536014)(6506007)(66066001)(76176011)(102836004)(30864003)(106356001)(26005)(5660300002)(446003)(7696005)(54906003)(186003)(99286004)(476003)(53936002)(316002)(486006)(2906002)(11346002); DIR:OUT; SFP:1101; SCL:1; SRVR:VI1PR0402MB2814; H:VI1PR0402MB3519.eurprd04.prod.outlook.com; FPR:; SPF:None; LANG:en; PTR:InfoNoRecords; A:1; MX:1; received-spf: None (protection.outlook.com: nxp.com does not designate permitted sender hosts) x-ms-exchange-senderadcheck: 1 x-microsoft-antispam-message-info: Rpj6yYpYBRAX398tyx/Zrk3rbvA5HQMzZeYi5AeoED3q9xcXfAqUorBP9ij3gKYBSnPWM3MjtVV1Qga1Tdpg+Mwl+uyLpXDHNblnOnVsVRg9ViXwX+zLA2SMfZwEnLe1vj7AjRYowknnOmkKRdAHPin0ykXLgWd9G2UB4MTqq6Quwgr/AwBEOlMyb6clRcT8INArTaZKX1xkPqJn9Hb0xZ939Omd4JSmsSzwoCSNdYCD3GBkgTab2MqdvJYt2tcjvUlqsbVwTzKMD2JqhYf4f+VfX1hXjUaWLXdVXNOcPFKdOIv4rj1/Kr96DfY6Lyb1h/j3MizQrqJJBogNjMgOLJ9JaziLOIQPJCqQoXTgHEGqQ3grRIxPEdq/SFkYvGfOHv1jkBZlHrygISqX2DtKbSSO1npkqelYMDqmZUmLP0U= MIME-Version: 1.0 X-OriginatorOrg: nxp.com X-MS-Exchange-CrossTenant-Network-Message-Id: f217b014-4d3f-4f29-3f08-08d6ae697f8f X-MS-Exchange-CrossTenant-originalarrivaltime: 22 Mar 2019 01:55:45.2906 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: 686ea1d3-bc2b-4c6f-a92c-d99c5c301635 X-MS-Exchange-CrossTenant-mailboxtype: HOSTED X-MS-Exchange-Transport-CrossTenantHeadersStamped: VI1PR0402MB2814 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20190321_185551_898621_3D4CD61E X-CRM114-Status: GOOD ( 24.70 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: "mark.rutland@arm.com" , Aisheng Dong , "devicetree@vger.kernel.org" , "s.hauer@pengutronix.de" , "robh+dt@kernel.org" , dl-linux-imx , "kernel@pengutronix.de" , "festevam@gmail.com" , "linux-arm-kernel@lists.infradead.org" , "l.stach@pengutronix.de" Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+infradead-linux-arm-kernel=archiver.kernel.org@lists.infradead.org > Subject: Re: [PATCH v2 1/3] arm64: dts: imx: Add i.mx8mm dtsi support > > On Tue, Mar 12, 2019 at 05:44:53AM +0000, Jacky Bai wrote: > > The i.MX8M Mini is new SOC of the i.MX8M family. it is focused on > > delivering the latest and greatest video and audio experience > > combining state-of-the-art media-specific features with > > high-performance processing while optimized for lowest power > > consumption. The i.MX 8M Mini Media Applications Processor is 14nm > > FinFET product of the growing i.MX8M family targeting the consumer & > > industrial market. It is built in 14LPP to achieve both high > > performance and low power consumption and relies on a powerful fully > > coherent core complex based on a quad Cortex-A53 cluster with video > > and graphics accelerators > > > > This patch adds the basic dtsi support for i.MX8MM. > > > > Signed-off-by: Jacky Bai > > --- > > change v1->v2: > > - removed unnecessary 'okay' status > > - change AIPS's address-cells and size-cells to '1' > > - remove sdma's undocumented property > > --- > > arch/arm64/boot/dts/freescale/imx8mm.dtsi | 717 > > ++++++++++++++++++++++++++++++ > > 1 file changed, 717 insertions(+) > > create mode 100644 arch/arm64/boot/dts/freescale/imx8mm.dtsi > > > > diff --git a/arch/arm64/boot/dts/freescale/imx8mm.dtsi > > b/arch/arm64/boot/dts/freescale/imx8mm.dtsi ... > > + }; > > + > > + soc@0 { > > The unit-address 0 makes no sense here, right? Yes, I will remove this in V3. > > + gpio5: gpio@30240000 { > > + compatible = "fsl,imx8mm-gpio", "fsl,imx35-gpio"; > > + reg = <0x30240000 0x10000>; > > + interrupts = , > > + ; > > + gpio-controller; > > + #gpio-cells = <2>; > > + interrupt-controller; > > + #interrupt-cells = <2>; > > + }; > > + > > + wdog1: wdog@30280000 { > > watchdog for node name. ok, will fix it. > > > + compatible = "fsl,imx21-wdt"; > > Don't you need a "fsl,imx8mm-wdt"? > ok, I will add it. > > + > > + sdma2: dma-controller@302c0000 { > > + compatible = "fsl,imx8mq-sdma", "fsl,imx7d-sdma"; > > Should be fsl,imx8mm-sdma? OK, will fix it. > > > + reg = <0x302c0000 0x10000>; > > + interrupts = ; > > + clocks = <&clk IMX8MM_CLK_SDMA2_ROOT>, > > + <&clk IMX8MM_CLK_SDMA2_ROOT>; > > + clock-names = "ipg", "ahb"; > > + #dma-cells = <3>; > > + fsl,sdma-ram-script-name = "imx/sdma/sdma-imx7d.bin"; > > i.mx8mm reuses the exactly same SDMA RAM script as imx7d? > Yes, imx8mm reuse the same firmware at present. > > + > > + ocotp: ocotp-ctrl@30350000 { > > + compatible = "fsl,imx8mq-ocotp", "fsl,imx7d-ocotp", > "syscon"; > > Should be fsl,imx8mm-ocotp? Ok, will fix it. > > > + reg = <0x30350000 0x10000>; > > + clocks = <&clk IMX8MM_CLK_OCOTP_ROOT>; > > + /* For nvmem subnodes */ > > + #address-cells = <1>; > > + #size-cells = <1>; > > + }; > > + > > + anatop: anatop@30360000 { > > + compatible = "fsl,imx8mm-anatop", "syscon", > "simple-bus"; > > + reg = <0x30360000 0x10000>; > > + }; > > + > > + snvs: snvs@30370000 { > > + compatible = "fsl,sec-v4.0-mon","syscon", "simple-mfd"; > > + reg = <0x30370000 0x10000>; > > + > > + snvs_rtc: snvs-rtc-lp{ > > Miss one space before {. Ok, will fix it. > > > + compatible = "fsl,sec-v4.0-mon-rtc-lp"; > > + regmap =<&snvs>; > > Miss one space after =. > Thanks, will fix it > > + offset = <0x34>; > > + interrupts = , > > + ; > > + }; > > + > > + snvs_pwrkey: snvs-powerkey { > > + compatible = "fsl,sec-v4.0-pwrkey"; > > + regmap = <&snvs>; > > + interrupts = ; > > + linux,keycode = ; > > + wakeup-source; > > + }; > > + }; > > + > > + clk: clock-controller@30380000 { > > + compatible = "fsl,imx8mm-ccm"; > > + reg = <0x30380000 0x10000>; > > + #clock-cells = <1>; > > + clocks = <&osc_32k>, <&osc_24m>, <&clk_ext1>, > <&clk_ext2>, > > + <&clk_ext3>, <&clk_ext4>; > > + clock-names = "osc_32k", "osc_24m", "clk_ext1", > "clk_ext2", > > + "clk_ext3", "clk_ext4"; > > + }; > > + > > + src: src@30390000 { > > reset-controller for node name? > Ok, will fix it. > > + compatible = "fsl,imx8mm-src", "syscon"; > > + reg = <0x30390000 0x10000>; > > + interrupts = ; > > + #reset-cells = <1>; > > + }; > > + > > + gpc: gpc@303a0000 { > > interrupt-controller for node name? > For the i.MX8MM and future i.MX8M serious, it is not very necessary to use GPC as a interrupt controller in linux kernel side. For system suspend, ATF can config the GPC IMRs based on IRQ enable status in GICv3. For cpuidle support, GICv3 has per-CPU core wakeup signals connected to the GPC logic, so GICv3 has the ability to wake up the corresponding CPU core if the IRQ is routed to that CPU core. Additionally, I am thinking to config the GPC as a secure resource in the future. Maybe, we can remove the GPC node for now. If necessary, we can add it back again? BTW, i.MX8MQ is a special case due to some HW issue, GPC still need to be used as a interrupt controller in kernel. > > + compatible = "fsl,imx8mm-gpc", "syscon"; > > + reg = <0x303a0000 0x10000>; > > + interrupt-controller; > > + interrupts = ; > > + #interrupt-cells = <3>; > > + }; > > + }; ... > > + aips3: bus@30800000 { > > + compatible = "fsl,aips-bus", "simple-bus"; > > + #address-cells = <1>; > > + #size-cells = <1>; > > + ranges; > > + > > + ecspi1: ecspi@30820000 { > > spi for node name? > OK, will fix it. > > + #address-cells = <1>; > > + #size-cells = <0>; > > We usually start properties with compatible. Can these be moved > somewhere after compatible? > Ok, will fix it > > + compatible = "fsl,imx8mm-ecspi", "fsl,imx51-ecspi"; > > + reg = <0x30820000 0x10000>; > > + interrupts = ; > > + clocks = <&clk IMX8MM_CLK_ECSPI1_ROOT>, > > + <&clk IMX8MM_CLK_ECSPI1_ROOT>; > > + clock-names = "ipg", "per"; > > + dmas = <&sdma1 0 7 1>, <&sdma1 1 7 2>; > > + dma-names = "rx", "tx"; > > + status = "disabled"; > > + }; .... > > + > > + uart1: serial@30860000 { > > + compatible = "fsl,imx8mm-uart", > > + "fsl,imx6q-uart", "fsl,imx21-uart"; > > I think "fsl,imx21-uart" can be dropped in this case? > Ok, I will remove this. > > + reg = <0x30860000 0x10000>; > > + interrupts = ; > > + clocks = <&clk IMX8MM_CLK_UART1_ROOT>, > > + <&clk IMX8MM_CLK_UART1_ROOT>; > > + clock-names = "ipg", "per"; > > + dmas = <&sdma1 22 4 0>, <&sdma1 23 4 0>; > > + dma-names = "rx", "tx"; > > + status = "disabled"; > > + }; > > + ... > > + > > + usdhc1: mmc@30b40000 { > > + compatible = "fsl,imx8mq-usdhc", "fsl,imx7d-usdhc"; > > Should be fsl,imx8mm-usdhc? > Ok, will fix it. > > + reg = <0x30b40000 0x10000>; > > + interrupts = ; > > + clocks = <&clk IMX8MM_CLK_DUMMY>, > > + <&clk IMX8MM_CLK_NAND_USDHC_BUS>, > > + <&clk IMX8MM_CLK_USDHC1_ROOT>; > > + clock-names = "ipg", "ahb", "per"; > > + assigned-clocks = <&clk IMX8MM_CLK_USDHC1>; > > + assigned-clock-rates = <400000000>; > > + fsl,tuning-start-tap = <20>; > > + fsl,tuning-step= <2>; > > + bus-width = <4>; > > + status = "disabled"; > > + }; > > + ... > > + > > + sdma1: dma-controller@30bd0000 { > > + compatible = "fsl,imx8mq-sdma", "fsl,imx7d-sdma"; > > Should be fsl,imx8mm-sdma? > Ok, will update it. > > + reg = <0x30bd0000 0x10000>; > > + interrupts = ; > > + clocks = <&clk IMX8MM_CLK_SDMA1_ROOT>, > > + <&clk IMX8MM_CLK_SDMA1_ROOT>; > > + clock-names = "ipg", "ahb"; > > + #dma-cells = <3>; > > + fsl,sdma-ram-script-name = "imx/sdma/sdma-imx7d.bin"; > > + }; > > + > > + fec1: ethernet@30be0000 { > > + compatible = "fsl,imx8mq-fec", "fsl,imx6sx-fec"; > > Should be fsl,imx8mm-fec? > Ok, will update it > > + reg = <0x30be0000 0x10000>; > > + interrupts = , > > + , > > + ; > > + clocks = <&clk IMX8MM_CLK_ENET1_ROOT>, > > + <&clk IMX8MM_CLK_ENET1_ROOT>, > > + <&clk IMX8MM_CLK_ENET_TIMER>, > > + <&clk IMX8MM_CLK_ENET_REF>, > > + <&clk IMX8MM_CLK_ENET_PHY_REF>; > > + clock-names = "ipg", "ahb", "ptp", > > + "enet_clk_ref", "enet_out"; > > + assigned-clocks = <&clk IMX8MM_CLK_ENET_AXI>, > > + <&clk IMX8MM_CLK_ENET_TIMER>, > > + <&clk IMX8MM_CLK_ENET_REF>, > > + <&clk IMX8MM_CLK_ENET_TIMER>; > > + assigned-clock-parents = <&clk > IMX8MM_SYS_PLL1_266M>, > > + <&clk IMX8MM_SYS_PLL2_100M>, > > + <&clk IMX8MM_SYS_PLL2_125M>; > > + assigned-clock-rates = <0>, <0>, <125000000>, > <100000000>; > > + stop-mode = <&gpr 0x10 3>; > > vendor property? > Yes, will remove it. > > + fsl,num-tx-queues=<3>; > > + fsl,num-rx-queues=<3>; > > Miss space before and after =. > Thanks, will fix it. > > + fsl,wakeup_irq = <2>; > > vendor property? > Yes, will remove it > > + status = "disabled"; > > + }; > > + > > + }; > > + > > + aips4: bus@32c00000 { > > + compatible = "fsl,aips-bus", "simple-bus"; > > + #address-cells = <1>; > > + #size-cells = <1>; > > + ranges; > > + > > + usbotg1: usb@32e40000 { > > + compatible = "fsl,imx8mm-usb", "fsl,imx7d-usb", > "fsl,imx27-usb"; > > fsl,imx27-usb can be saved? ok, will remove it. > > > + reg = <0x32e40000 0x200>; > > + interrupts = ; > > + clocks = <&clk IMX8MM_CLK_USB1_CTRL_ROOT>; > > + clock-names = "usb1_ctrl_root_clk"; > > + assigned-clocks = <&clk IMX8MM_CLK_USB_BUS>, > > + <&clk IMX8MM_CLK_USB_CORE_REF>; > > + assigned-clock-parents = <&clk > IMX8MM_SYS_PLL2_500M>, > > + <&clk IMX8MM_SYS_PLL1_100M>; > > + fsl,usbphy = <&usbphynop1>; > > + fsl,usbmisc = <&usbmisc1 0>; > > + status = "disabled"; > > + }; > > + > > + usbphynop1: usbphynop1 { > > + compatible = "usb-nop-xceiv"; > > + clocks = <&clk IMX8MM_CLK_USB_PHY_REF>; > > + assigned-clocks = <&clk IMX8MM_CLK_USB_PHY_REF>; > > + assigned-clock-parents = <&clk > IMX8MM_SYS_PLL1_100M>; > > + clock-names = "main_clk"; > > + }; > > + > > + usbmisc1: usbmisc@32e40200 { > > + #index-cells = <1>; > > Move it afterwards? > OK, thanks. > > + compatible = "fsl,imx7d-usbmisc", "fsl,imx6q-usbmisc"; > > Should be compatible = "fsl,imx8mm-usbmisc", "fsl,imx7d-usbmisc"? > Ok, will fix it. > > + reg = <0x32e40200 0x200>; > > + }; > > + > > + usbotg2: usb@32e50000 { > > + compatible = "fsl,imx8mm-usb", "fsl,imx7d-usb", > "fsl,imx27-usb"; > > + reg = <0x32e50000 0x200>; > > + interrupts = ; > > + clocks = <&clk IMX8MM_CLK_USB1_CTRL_ROOT>; > > + clock-names = "usb1_ctrl_root_clk"; > > + assigned-clocks = <&clk IMX8MM_CLK_USB_BUS>, > > + <&clk IMX8MM_CLK_USB_CORE_REF>; > > + assigned-clock-parents = <&clk > IMX8MM_SYS_PLL2_500M>, > > + <&clk IMX8MM_SYS_PLL1_100M>; > > + fsl,usbphy = <&usbphynop2>; > > + fsl,usbmisc = <&usbmisc2 0>; > > + status = "disabled"; > > + }; > > + > > + usbphynop2: usbphynop2 { > > + compatible = "usb-nop-xceiv"; > > + clocks = <&clk IMX8MM_CLK_USB_PHY_REF>; > > + assigned-clocks = <&clk IMX8MM_CLK_USB_PHY_REF>; > > + assigned-clock-parents = <&clk > IMX8MM_SYS_PLL1_100M>; > > + clock-names = "main_clk"; > > + }; > > + > > + usbmisc2: usbmisc@32e50200 { > > + #index-cells = <1>; > > + compatible = "fsl,imx7d-usbmisc", "fsl,imx6q-usbmisc"; > > + reg = <0x32e50200 0x200>; > > + }; > > + > > + }; > > + > > + dma_apbh: dma-apbh@33000000 { > > dma-controller for node name. > Ok. Thanks > > + 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 = <&clk > IMX8MM_CLK_NAND_USDHC_BUS_RAWNAND_CLK>; > > + }; > > + > > + gpmi: gpmi-nand@33002000{ > > nand-controller for node name. > Ok, will fix it. > > + compatible = "fsl,imx7d-gpmi-nand"; > > fsl,imx8mm-gpmi-nand before it? > Ok, thanks. > > + #address-cells = <1>; > > + #size-cells = <1>; > > + reg = <0x33002000 0x2000>, <0x33004000 0x4000>; > > + reg-names = "gpmi-nand", "bch"; > > + interrupts = ; > > + interrupt-names = "bch"; > > + clocks = <&clk IMX8MM_CLK_NAND_ROOT>, > > + <&clk IMX8MM_CLK_NAND_USDHC_BUS_RAWNAND_CLK>; > > Miss indentation. > Thanks for review, will fix this. BR > Shawn > > > + clock-names = "gpmi_io", "gpmi_bch_apb"; > > + dmas = <&dma_apbh 0>; > > + dma-names = "rx-tx"; > > + status = "disabled"; > > + }; > > + }; > > +}; > > -- > > 1.9.1 > > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel