[{"id":1776217,"web_url":"http://patchwork.ozlabs.org/comment/1776217/","msgid":"","list_archive_url":null,"date":"2017-09-27T09:55:33","subject":"Re: [U-Boot] [U-Boot,\n\t5/6] net: gmac_rockchip: Use the proerty of \"clock_in_out\" to set\n\tmac clock","submitter":{"id":53488,"url":"http://patchwork.ozlabs.org/api/people/53488/","name":"Philipp Tomsich","email":"philipp.tomsich@theobroma-systems.com"},"content":"> If the mac clock if from the external IO, set clock rate with 0;\n> If the mac clock if from the internal divider pll, set 50M for\n> rmii mode and set 125M for rgmii.\n> \n> Signed-off-by: David Wu \n> ---\n> \n> drivers/net/gmac_rockchip.c | 23 +++++++++++++++++++----\n> 1 file changed, 19 insertions(+), 4 deletions(-)\n> \n\nAcked-by: Philipp Tomsich ","headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":"ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=lists.denx.de\n\t(client-ip=81.169.180.215; helo=lists.denx.de;\n\tenvelope-from=u-boot-bounces@lists.denx.de;\n\treceiver=)","Received":["from lists.denx.de (dione.denx.de [81.169.180.215])\n\tby ozlabs.org (Postfix) with ESMTP id 3y2DVz49gJz9tXQ\n\tfor ;\n\tWed, 27 Sep 2017 20:26:03 +1000 (AEST)","by lists.denx.de (Postfix, from userid 105)\n\tid 5815DC21E5B; Wed, 27 Sep 2017 10:25:50 +0000 (UTC)","from lists.denx.de (localhost [IPv6:::1])\n\tby lists.denx.de (Postfix) with ESMTP id 42F99C21E1D;\n\tWed, 27 Sep 2017 09:57:01 +0000 (UTC)","by lists.denx.de (Postfix, from userid 105)\n\tid DDCABC21DF4; Wed, 27 Sep 2017 09:56:59 +0000 (UTC)","from mail.theobroma-systems.com (vegas.theobroma-systems.com\n\t[144.76.126.164])\n\tby lists.denx.de (Postfix) with ESMTPS id 82873C21E35\n\tfor ; Wed, 27 Sep 2017 09:55:35 +0000 (UTC)","from 89-104-28-141.customer.bnet.at ([89.104.28.141]:60984\n\thelo=vpn-10-11-0-14.lan) by mail.theobroma-systems.com with esmtpsa\n\t(TLS1.2:DHE_RSA_AES_256_CBC_SHA256:256) (Exim 4.80)\n\t(envelope-from )\n\tid 1dx941-0001PT-Fo; Wed, 27 Sep 2017 11:55:33 +0200"],"X-Spam-Checker-Version":"SpamAssassin 3.4.0 (2014-02-07) on lists.denx.de","X-Spam-Level":"","X-Spam-Status":"No, score=0.0 required=5.0 tests=none autolearn=unavailable\n\tautolearn_force=no version=3.4.0","MIME-Version":"1.0","From":"Philipp Tomsich ","To":"David Wu ","In-Reply-To":"<1506003760-34739-1-git-send-email-david.wu@rock-chips.com>","References":"<1506003760-34739-1-git-send-email-david.wu@rock-chips.com>","Message-Id":"","Date":"Wed, 27 Sep 2017 11:55:33 +0200","Cc":"huangtao@rock-chips.com, u-boot@lists.denx.de,\n\tDavid Wu , andy.yan@rock-chips.com,\n\tchenjh@rock-chips.com","Subject":"Re: [U-Boot] [U-Boot,\n\t5/6] net: gmac_rockchip: Use the proerty of \"clock_in_out\" to set\n\tmac clock","X-BeenThere":"u-boot@lists.denx.de","X-Mailman-Version":"2.1.18","Precedence":"list","List-Id":"U-Boot discussion ","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Content-Type":"text/plain; charset=\"utf-8\"","Content-Transfer-Encoding":"base64","Errors-To":"u-boot-bounces@lists.denx.de","Sender":"\"U-Boot\" "}},{"id":1781387,"web_url":"http://patchwork.ozlabs.org/comment/1781387/","msgid":"","list_archive_url":null,"date":"2017-10-06T09:45:29","subject":"Re: [U-Boot] [U-Boot,\n\t5/6] net: gmac_rockchip: Use the proerty of \"clock_in_out\" to set\n\tmac clock","submitter":{"id":53488,"url":"http://patchwork.ozlabs.org/api/people/53488/","name":"Philipp Tomsich","email":"philipp.tomsich@theobroma-systems.com"},"content":"> If the mac clock if from the external IO, set clock rate with 0;\n> If the mac clock if from the internal divider pll, set 50M for\n> rmii mode and set 125M for rgmii.\n> \n> Signed-off-by: David Wu \n> Acked-by: Philipp Tomsich \n> ---\n> \n> drivers/net/gmac_rockchip.c | 23 +++++++++++++++++++----\n> 1 file changed, 19 insertions(+), 4 deletions(-)\n> \n\nReviewed-by: Philipp Tomsich ","headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":"ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=lists.denx.de\n\t(client-ip=81.169.180.215; helo=lists.denx.de;\n\tenvelope-from=u-boot-bounces@lists.denx.de;\n\treceiver=)","Received":["from lists.denx.de (dione.denx.de [81.169.180.215])\n\tby ozlabs.org (Postfix) with ESMTP id 3y7lBr3QcWz9t3m\n\tfor ;\n\tFri, 6 Oct 2017 20:46:12 +1100 (AEDT)","by lists.denx.de (Postfix, from userid 105)\n\tid 49A41C21D92; Fri, 6 Oct 2017 09:46:02 +0000 (UTC)","from lists.denx.de (localhost [IPv6:::1])\n\tby lists.denx.de (Postfix) with ESMTP id B3B96C21D90;\n\tFri, 6 Oct 2017 09:45:36 +0000 (UTC)","by lists.denx.de (Postfix, from userid 105)\n\tid A5F42C21C3F; Fri, 6 Oct 2017 09:45:34 +0000 (UTC)","from mail.theobroma-systems.com (vegas.theobroma-systems.com\n\t[144.76.126.164])\n\tby lists.denx.de (Postfix) with ESMTPS id 4A78EC21C40\n\tfor ; Fri, 6 Oct 2017 09:45:33 +0000 (UTC)","from 89-104-28-141.customer.bnet.at ([89.104.28.141]:62886\n\thelo=vpn-10-11-0-14.lan) by mail.theobroma-systems.com with esmtpsa\n\t(TLS1.2:DHE_RSA_AES_256_CBC_SHA256:256) (Exim 4.80)\n\t(envelope-from )\n\tid 1e0PCD-0003Tn-5d; Fri, 06 Oct 2017 11:45:29 +0200"],"X-Spam-Checker-Version":"SpamAssassin 3.4.0 (2014-02-07) on lists.denx.de","X-Spam-Level":"","X-Spam-Status":"No, score=0.0 required=5.0 tests=none autolearn=unavailable\n\tautolearn_force=no version=3.4.0","MIME-Version":"1.0","From":"Philipp Tomsich ","To":"David Wu ","In-Reply-To":"<1506003760-34739-1-git-send-email-david.wu@rock-chips.com>","References":"<1506003760-34739-1-git-send-email-david.wu@rock-chips.com>","Message-Id":"","Date":"Fri, 06 Oct 2017 11:45:29 +0200","Cc":"huangtao@rock-chips.com, u-boot@lists.denx.de,\n\tDavid Wu , andy.yan@rock-chips.com,\n\tchenjh@rock-chips.com","Subject":"Re: [U-Boot] [U-Boot,\n\t5/6] net: gmac_rockchip: Use the proerty of \"clock_in_out\" to set\n\tmac clock","X-BeenThere":"u-boot@lists.denx.de","X-Mailman-Version":"2.1.18","Precedence":"list","List-Id":"U-Boot discussion ","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Content-Type":"text/plain; charset=\"utf-8\"","Content-Transfer-Encoding":"base64","Errors-To":"u-boot-bounces@lists.denx.de","Sender":"\"U-Boot\" "}},{"id":1781401,"web_url":"http://patchwork.ozlabs.org/comment/1781401/","msgid":"","list_archive_url":null,"date":"2017-10-06T10:17:33","subject":"Re: [U-Boot] [U-Boot,\n\t5/6] net: gmac_rockchip: Use the proerty of \"clock_in_out\" to set\n\tmac clock","submitter":{"id":53488,"url":"http://patchwork.ozlabs.org/api/people/53488/","name":"Philipp Tomsich","email":"philipp.tomsich@theobroma-systems.com"},"content":"On Thu, 21 Sep 2017, David Wu wrote:\n\n> If the mac clock if from the external IO, set clock rate with 0;\n> If the mac clock if from the internal divider pll, set 50M for\n> rmii mode and set 125M for rgmii.\n\nIf we do this, we really shouldn't treat the external clock any different.\nSo we should request the speed we expect from the external clk and not \njust pass 0...\n\nMore below.\n\n>\n> Signed-off-by: David Wu \n> Acked-by: Philipp Tomsich \n> ---\n>\n> drivers/net/gmac_rockchip.c | 23 +++++++++++++++++++----\n> 1 file changed, 19 insertions(+), 4 deletions(-)\n>\n> diff --git a/drivers/net/gmac_rockchip.c b/drivers/net/gmac_rockchip.c\n> index 5f8f0cd..26f7a96 100644\n> --- a/drivers/net/gmac_rockchip.c\n> +++ b/drivers/net/gmac_rockchip.c\n> @@ -29,6 +29,7 @@ DECLARE_GLOBAL_DATA_PTR;\n> struct gmac_rockchip_platdata {\n> \tstruct dw_eth_pdata dw_eth_pdata;\n> \tvoid *grf;\n> +\tint clk_in;\n\nPlease use an enum here to make it clear which value in INPUT and which \none is OUTPUT.\n\n> \tint tx_delay;\n> \tint rx_delay;\n> };\n> @@ -64,6 +65,8 @@ static int gmac_rockchip_ofdata_to_platdata(struct udevice *dev)\n> {\n> \tstruct gmac_rockchip_platdata *pdata = dev_get_platdata(dev);\n>\n> +\tpdata->clk_in = dev_read_u32_default(dev, \"clock_in_out\", 1);\n> +\n\nThe 'u32' seems to be wrong, relative to common usage:\narch/arm/dts/rk3229-evb.dts: clock_in_out = \"input\";\narch/arm/dts/rk3288-evb.dtsi: clock_in_out = \"input\";\narch/arm/dts/rk3288-fennec.dtsi: clock_in_out = \"input\";\narch/arm/dts/rk3288-firefly.dtsi: clock_in_out = \"input\";\narch/arm/dts/rk3288-miqi.dtsi: clock_in_out = \"input\";\narch/arm/dts/rk3288-phycore-som.dtsi: clock_in_out = \"input\";\narch/arm/dts/rk3288-popmetal.dtsi: clock_in_out = \"input\";\narch/arm/dts/rk3288-rock2-som.dtsi: clock_in_out = \"input\";\narch/arm/dts/rk3288-tinker.dtsi: clock_in_out = \"input\";\narch/arm/dts/rk3368-geekbox.dts: clock_in_out = \"input\";\narch/arm/dts/rk3368-lion.dts: clock_in_out = \"input\";\narch/arm/dts/rk3368-px5-evb.dts: clock_in_out = \"input\";\narch/arm/dts/rk3368-sheep.dts: clock_in_out = \"input\";\narch/arm/dts/rk3399-evb.dts: clock_in_out = \"input\";\narch/arm/dts/rk3399-firefly.dts: clock_in_out = \"input\";\narch/arm/dts/rk3399-puma.dtsi: clock_in_out = \"input\";\n\nPlease remain consistent and test for a string here and for the string \nvalue. Also, we are missing documentation on the DTS binding in /doc: \nplease add documentation there.\n\n> \t/* Check the new naming-style first... */\n> \tpdata->tx_delay = dev_read_u32_default(dev, \"tx_delay\", -ENOENT);\n> \tpdata->rx_delay = dev_read_u32_default(dev, \"rx_delay\", -ENOENT);\n> @@ -294,6 +297,8 @@ static int gmac_rockchip_probe(struct udevice *dev)\n> \tstruct gmac_rockchip_platdata *pdata = dev_get_platdata(dev);\n> \tstruct gmac_rockchip_driver_data *data =\n> \t\t(struct gmac_rockchip_driver_data *)dev_get_driver_data(dev);\n> +\tstruct dw_eth_pdata *dw_pdata = dev_get_platdata(dev);\n> +\tstruct eth_pdata *eth_pdata = &dw_pdata->eth_pdata;\n> \tconst struct rk_gmac_ops *ops = data->ops;\n> \tstruct clk clk;\n> \tint ret;\n> @@ -302,10 +307,20 @@ static int gmac_rockchip_probe(struct udevice *dev)\n> \tif (ret)\n> \t\treturn ret;\n>\n> -\t/* Since mac_clk is fed by an external clock we can use 0 here */\n> -\tret = clk_set_rate(&clk, 0);\n> -\tif (ret)\n> -\t\treturn ret;\n> +\tif (pdata->clk_in) {\n> +\t\t/*\n> +\t\t * Since mac_clk is fed by an external clock\n> +\t\t * we can use 0 here.\n> +\t\t */\n> +\t\tret = clk_set_rate(&clk, 0);\n\nIn hindsight using '0' was a bad decision.\nWe should always request the expected external clock rate here (so the \nclock driver could do some sanity checking).\n\n> +\t\tif (ret)\n> +\t\t\treturn ret;\n> +\t} else {\n> +\t\tif (eth_pdata->phy_interface == PHY_INTERFACE_MODE_RGMII)\n> +\t\t\tclk_set_rate(&clk, 125000000);\n> +\t\telse\n> +\t\t\tclk_set_rate(&clk, 50000000);\n> +\t}\n>\n> \tpdata->grf = syscon_get_first_range(ROCKCHIP_SYSCON_GRF) +\n> \t\t data->grf_offset;\n>","headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":"ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=lists.denx.de\n\t(client-ip=81.169.180.215; helo=lists.denx.de;\n\tenvelope-from=u-boot-bounces@lists.denx.de;\n\treceiver=)","Received":["from lists.denx.de (dione.denx.de [81.169.180.215])\n\tby ozlabs.org (Postfix) with ESMTP id 3y7lvP04zyz9t48\n\tfor ;\n\tFri, 6 Oct 2017 21:17:51 +1100 (AEDT)","by lists.denx.de (Postfix, from userid 105)\n\tid 6E927C21E32; Fri, 6 Oct 2017 10:17:47 +0000 (UTC)","from lists.denx.de (localhost [IPv6:::1])\n\tby lists.denx.de (Postfix) with ESMTP id 5C44BC21C40;\n\tFri, 6 Oct 2017 10:17:44 +0000 (UTC)","by lists.denx.de (Postfix, from userid 105)\n\tid C31A8C21C40; Fri, 6 Oct 2017 10:17:43 +0000 (UTC)","from mail.theobroma-systems.com (vegas.theobroma-systems.com\n\t[144.76.126.164])\n\tby lists.denx.de (Postfix) with ESMTPS id 3011CC21C3F\n\tfor ; Fri, 6 Oct 2017 10:17:41 +0000 (UTC)","from [86.59.122.178] (port=51521 helo=android.lan)\n\tby mail.theobroma-systems.com with esmtps\n\t(TLS1.2:RSA_AES_128_CBC_SHA1:128)\n\t(Exim 4.80) (envelope-from )\n\tid 1e0PhL-0004Pn-0Y; Fri, 06 Oct 2017 12:17:39 +0200","from [10.11.0.14] (helo=vpn-10-11-0-14.lan)\n\tby android.lan with esmtp (Exim 4.84_2)\n\t(envelope-from )\n\tid 1e0PhI-000FRf-J3; Fri, 06 Oct 2017 12:17:38 +0200"],"X-Spam-Checker-Version":"SpamAssassin 3.4.0 (2014-02-07) on lists.denx.de","X-Spam-Level":"","X-Spam-Status":"No, score=0.0 required=5.0 tests=none autolearn=unavailable\n\tautolearn_force=no version=3.4.0","Date":"Fri, 6 Oct 2017 12:17:33 +0200 (CEST)","From":"Philipp Tomsich ","X-X-Sender":"ptomsich@vpn-10-11-0-14.lan","To":"David Wu ","In-Reply-To":"<1506003760-34739-1-git-send-email-david.wu@rock-chips.com>","Message-ID":"","References":"<1506003760-34739-1-git-send-email-david.wu@rock-chips.com>","User-Agent":"Alpine 2.21 (OSX 202 2017-01-01)","MIME-Version":"1.0","Cc":"huangtao@rock-chips.com, u-boot@lists.denx.de, andy.yan@rock-chips.com, \n\tchenjh@rock-chips.com","Subject":"Re: [U-Boot] [U-Boot,\n\t5/6] net: gmac_rockchip: Use the proerty of \"clock_in_out\" to set\n\tmac clock","X-BeenThere":"u-boot@lists.denx.de","X-Mailman-Version":"2.1.18","Precedence":"list","List-Id":"U-Boot discussion ","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Content-Transfer-Encoding":"base64","Content-Type":"text/plain; charset=\"utf-8\"; Format=\"flowed\"","Errors-To":"u-boot-bounces@lists.denx.de","Sender":"\"U-Boot\" "}}]