From mboxrd@z Thu Jan 1 00:00:00 1970 From: Heiko =?ISO-8859-1?Q?St=FCbner?= Subject: Re: [RESEND PATCH 3/4] arm64: dts: rockchip: support gmac for rk3399 Date: Wed, 31 Aug 2016 23:29:37 +0200 Message-ID: <2498365.kqzATN1cjf@diego> References: <1472624028-7082-1-git-send-email-wxt@rock-chips.com> <1472624028-7082-4-git-send-email-wxt@rock-chips.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7Bit Return-path: In-Reply-To: Sender: devicetree-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org To: Doug Anderson Cc: Caesar Wang , netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, "open list:ARM/Rockchip SoC..." , Brian Norris , Derek Basehore , Roger Chen , Rob Herring , Mark Rutland , Catalin Marinas , Will Deacon , Xing Zheng , Masahiro Yamada , Jianqun Xu , Elaine Zhang , David Wu , Shunqian Zheng , "devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org" , "linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org" List-Id: linux-rockchip.vger.kernel.org Am Mittwoch, 31. August 2016, 13:42:17 schrieb Doug Anderson: > Caesar, > > On Tue, Aug 30, 2016 at 11:13 PM, Caesar Wang wrote: > > This patch adds needed gamc information for rk3399, > > also support the gmac pd. > > > > Signed-off-by: Roger Chen > > Signed-off-by: Caesar Wang > > --- > > > > arch/arm64/boot/dts/rockchip/rk3399.dtsi | 90 > > ++++++++++++++++++++++++++++++++ 1 file changed, 90 insertions(+) > > I noticed that your subject for this patch contains "RESEND" and not > "v2" event though there are changes between this version and the last > one. That's really confusing. This should have been "v2" and the > next version should be "v3". > > > diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi > > b/arch/arm64/boot/dts/rockchip/rk3399.dtsi index 32aebc8..abf27a4 100644 > > --- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi > > +++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi > > @@ -200,6 +200,26 @@ > > > > }; > > > > }; > > > > + gmac: eth@fe300000 { > > nit: on rk3288 the node was "ethernet@" instead of "eth@". Presumably > "ethernet" is more correct? > > > + compatible = "rockchip,rk3399-gmac"; > > + reg = <0x0 0xfe300000 0x0 0x10000>; > > + interrupts = ; > > + interrupt-names = "macirq"; > > + clocks = <&cru SCLK_MAC>, <&cru SCLK_MAC_RX>, > > + <&cru SCLK_MAC_TX>, <&cru SCLK_MACREF>, > > + <&cru SCLK_MACREF_OUT>, <&cru ACLK_GMAC>, > > + <&cru PCLK_GMAC>; > > + clock-names = "stmmaceth", "mac_clk_rx", > > + "mac_clk_tx", "clk_mac_ref", > > + "clk_mac_refout", "aclk_mac", > > + "pclk_mac"; > > + power-domains = <&power RK3399_PD_GMAC>; > > + resets = <&cru SRST_A_GMAC>; > > + reset-names = "stmmaceth"; > > + rockchip,grf = <&grf>; > > + status = "disabled"; > > + }; > > + > > > > sdio0: dwmmc@fe310000 { > > > > compatible = "rockchip,rk3399-dw-mshc", > > > > "rockchip,rk3288-dw-mshc"; > > > > @@ -611,6 +631,11 @@ > > > > status = "disabled"; > > > > }; > > > > + qos_gmac: qos@ffa5c000 { > > + compatible = "syscon"; > > + reg = <0x0 0xffa5c000 0x0 0x20>; > > + }; > > + > > > > qos_hdcp: qos@ffa90000 { > > > > compatible = "syscon"; > > reg = <0x0 0xffa90000 0x0 0x20>; > > > > @@ -704,6 +729,11 @@ > > > > #size-cells = <0>; > > > > /* These power domains are grouped by VD_CENTER */ > > > > + pd_gmac@RK3399_PD_GMAC { > > RK3399_PD_GMAC is not in VD_CENTER but in VD_LOGIC, right? ...so this > should move. > > > + reg = ; > > + clocks = <&cru ACLK_GMAC>; > > + pm_qos = <&qos_gmac>; > > + }; > > IMHO it would be nice if this were broken into two patches. > > 1. First patch would be the power domain patch and that could land any > time. You wouldn't actually be able to use the gmac but at least > you'd be able to turn off its power. This would be a handy patch to > be able to backport if you happened to not need Ethernet support but > wanted to save power. > > 2. Second patch would actually add the gmac. according to my talk with Caesar in the real v1, the gmac even with power- domains should work just nicely even without the dts patches, as the driver core takes care of powering up the pd before probe. But I may miss some peculiarity of the dwmac? Heiko -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html