netdev - Re: [RESEND PATCH 3/4] arm64: dts: rockchip: support gmac for rk3399

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAD=FV=XQX__rFqGLY2U5mPT8WR4P4aW5tbCqFmA+3rZunbzUZQ@mail.gmail.com>
Date:   Wed, 31 Aug 2016 13:42:17 -0700
From:   Doug Anderson <dianders@...omium.org>
To:     Caesar Wang <wxt@...k-chips.com>
Cc:     Heiko Stuebner <heiko@...ech.de>, netdev@...r.kernel.org,
        "open list:ARM/Rockchip SoC..." <linux-rockchip@...ts.infradead.org>,
        Brian Norris <briannorris@...omium.org>,
        Derek Basehore <dbasehore@...omium.org>,
        Roger Chen <roger.chen@...k-chips.com>,
        Rob Herring <robh+dt@...nel.org>,
        Mark Rutland <mark.rutland@....com>,
        Catalin Marinas <catalin.marinas@....com>,
        Will Deacon <will.deacon@....com>,
        Xing Zheng <zhengxing@...k-chips.com>,
        Masahiro Yamada <yamada.masahiro@...ionext.com>,
        Jianqun Xu <jay.xu@...k-chips.com>,
        Elaine Zhang <zhangqing@...k-chips.com>,
        David Wu <david.wu@...k-chips.com>,
        Shunqian Zheng <zhengsq@...k-chips.com>,
        "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
        "linux-arm-kernel@...ts.infradead.org" 
        <linux-arm-kernel@...ts.infradead.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [RESEND PATCH 3/4] arm64: dts: rockchip: support gmac for rk3399

Caesar,

On Tue, Aug 30, 2016 at 11:13 PM, Caesar Wang <wxt@...k-chips.com> wrote:
> This patch adds needed gamc information for rk3399,
> also support the gmac pd.
>
> Signed-off-by: Roger Chen <roger.chen@...k-chips.com>
> Signed-off-by: Caesar Wang <wxt@...k-chips.com>
> ---
>
>  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@...00000 {

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 = <GIC_SPI 12 IRQ_TYPE_LEVEL_HIGH>;
> +               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@...10000 {
>                 compatible = "rockchip,rk3399-dw-mshc",
>                              "rockchip,rk3288-dw-mshc";
> @@ -611,6 +631,11 @@
>                 status = "disabled";
>         };
>
> +       qos_gmac: qos@...5c000 {
> +               compatible = "syscon";
> +               reg = <0x0 0xffa5c000 0x0 0x20>;
> +       };
> +
>         qos_hdcp: qos@...90000 {
>                 compatible = "syscon";
>                 reg = <0x0 0xffa90000 0x0 0x20>;
> @@ -704,6 +729,11 @@
>                         #size-cells = <0>;
>
>                         /* These power domains are grouped by VD_CENTER */
> +                       pd_gmac@...399_PD_GMAC {

RK3399_PD_GMAC is not in VD_CENTER but in VD_LOGIC, right?  ...so this
should move.

> +                               reg = <RK3399_PD_GMAC>;
> +                               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.