[{"id":1773844,"web_url":"http://patchwork.ozlabs.org/comment/1773844/","msgid":"<2262f266-6fb9-cce4-f83a-933dd41b9062@gmail.com>","list_archive_url":null,"date":"2017-09-22T20:00:32","subject":"Re: [PATCH net-next v2 1/3] net: dsa: use slave device phydev","submitter":{"id":2800,"url":"http://patchwork.ozlabs.org/api/people/2800/","name":"Florian Fainelli","email":"f.fainelli@gmail.com"},"content":"On 09/22/2017 12:40 PM, Vivien Didelot wrote:\n> There is no need to store a phy_device in dsa_slave_priv since\n> net_device already provides one. Simply s/p->phy/dev->phydev/.\n\nYou can therefore remove the phy_device from dsa_slave_priv, see below\nfor more comments. I will have to regress test the heck out of this,\nthis should take a few hours.\n\n> \n> While at it, return -ENODEV when it is NULL instead of -EOPNOTSUPP.\n> \n> Signed-off-by: Vivien Didelot \n> ---\n\n> static int dsa_slave_port_attr_set(struct net_device *dev,\n> @@ -435,12 +433,10 @@ static int\n> dsa_slave_get_link_ksettings(struct net_device *dev,\n> \t\t\t struct ethtool_link_ksettings *cmd)\n> {\n> -\tstruct dsa_slave_priv *p = netdev_priv(dev);\n> +\tif (!dev->phydev)\n> +\t\treturn -ENODEV;\n> \n> -\tif (!p->phy)\n> -\t\treturn -EOPNOTSUPP;\n> -\n> -\tphy_ethtool_ksettings_get(p->phy, cmd);\n> +\tphy_ethtool_ksettings_get(dev->phydev, cmd);\n\nThis can be replaced by phy_ethtool_get_link_ksettings()\n\n> \n> \treturn 0;\n> }\n> @@ -449,12 +445,10 @@ static int\n> dsa_slave_set_link_ksettings(struct net_device *dev,\n> \t\t\t const struct ethtool_link_ksettings *cmd)\n> {\n> -\tstruct dsa_slave_priv *p = netdev_priv(dev);\n> +\tif (!dev->phydev)\n> +\t\treturn -ENODEV;\n> \n> -\tif (p->phy != NULL)\n> -\t\treturn phy_ethtool_ksettings_set(p->phy, cmd);\n> -\n> -\treturn -EOPNOTSUPP;\n> +\treturn phy_ethtool_ksettings_set(dev->phydev, cmd);\n> }\n\nThis can disappear and you can assign this ethtool operation to\nphy_ethtool_set_link_ksettings()\n\n> \n> static void dsa_slave_get_drvinfo(struct net_device *dev,\n> @@ -488,24 +482,20 @@ dsa_slave_get_regs(struct net_device *dev, struct ethtool_regs *regs, void *_p)\n> \n> static int dsa_slave_nway_reset(struct net_device *dev)\n> {\n> -\tstruct dsa_slave_priv *p = netdev_priv(dev);\n> +\tif (!dev->phydev)\n> +\t\treturn -ENODEV;\n> \n> -\tif (p->phy != NULL)\n> -\t\treturn genphy_restart_aneg(p->phy);\n> -\n> -\treturn -EOPNOTSUPP;\n> +\treturn genphy_restart_aneg(dev->phydev);\n> }\n\nThis can now disappear and you can use phy_ethtool_nway_reset() directly\nin ethtool_ops\n\n> \n> static u32 dsa_slave_get_link(struct net_device *dev)\n> {\n> -\tstruct dsa_slave_priv *p = netdev_priv(dev);\n> +\tif (!dev->phydev)\n> +\t\treturn -ENODEV;\n> \n> -\tif (p->phy != NULL) {\n> -\t\tgenphy_update_link(p->phy);\n> -\t\treturn p->phy->link;\n> -\t}\n> +\tgenphy_update_link(dev->phydev);\n> \n> -\treturn -EOPNOTSUPP;\n> +\treturn dev->phydev->link;\n> }\n\nThis should certainly be just ethtool_op_get_link(), not sure why we\nkept that around here...","headers":{"Return-Path":"","X-Original-To":"patchwork-incoming@ozlabs.org","Delivered-To":"patchwork-incoming@ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=netdev-owner@vger.kernel.org;\n\treceiver=)","ozlabs.org; dkim=pass (2048-bit key;\n\tunprotected) header.d=gmail.com header.i=@gmail.com\n\theader.b=\"N1b6pbdC\"; dkim-atps=neutral"],"Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3xzPVQ4YWsz9t38\n\tfor ;\n\tSat, 23 Sep 2017 06:00:46 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1752540AbdIVUAl (ORCPT );\n\tFri, 22 Sep 2017 16:00:41 -0400","from mail-qk0-f193.google.com ([209.85.220.193]:35804 \"EHLO\n\tmail-qk0-f193.google.com\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n\twith ESMTP id S1751915AbdIVUAh (ORCPT\n\t); Fri, 22 Sep 2017 16:00:37 -0400","by mail-qk0-f193.google.com with SMTP id o77so1324812qke.2;\n\tFri, 22 Sep 2017 13:00:36 -0700 (PDT)","from [10.112.156.244] ([192.19.255.250])\n\tby smtp.googlemail.com with ESMTPSA id\n\to17sm505753qkl.14.2017.09.22.13.00.33\n\t(version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128);\n\tFri, 22 Sep 2017 13:00:34 -0700 (PDT)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=gmail.com; s=20161025;\n\th=subject:to:cc:references:from:message-id:date:user-agent\n\t:mime-version:in-reply-to:content-language:content-transfer-encoding; \n\tbh=nXebkU5YoWNUNNvEWF/cpdpb2bX6h5uhbAO/lX+lfdM=;\n\tb=N1b6pbdCyuIOh2qWePAIvvU3Pt6NQEeLzI3hIPcu9rvWej96f50heNkOlDdBYX3tMf\n\tu1V9W9LM2MHRi+dEXHBKrNMO6ufjPaaMOpMhBeejTbSnNQXh59PK/9eXeVn2ulNXyNLp\n\tpl6cy4PoKkrapKlOFzdSi5AxUKB60m0Eu+9kB57mOp7XngCxuZ8hqYOdx7xFO/0PL+yr\n\t2K0fuLJb/uJ/eXodRf0DPtMlxNUmN5qp0umgrQjGIIEkPMon/eyoL0tPQe3ZI0GDQidB\n\t/Rn+VRxo8TiYOcQ1LGyXmqsxmTwvAO5PUSygqj3Rf7iZej4hfAiMeskgu7gOvGYUU0a/\n\thnvA==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:subject:to:cc:references:from:message-id:date\n\t:user-agent:mime-version:in-reply-to:content-language\n\t:content-transfer-encoding;\n\tbh=nXebkU5YoWNUNNvEWF/cpdpb2bX6h5uhbAO/lX+lfdM=;\n\tb=DFLYWq2mH828fXMIwEer645tyZVxpeoTlaQ/qPVAzaPgezzTX9wsgjsNz2A1O2S/v0\n\tKKIVixGLxfuLeZhKIwHEhzL/86J4YUDq5Ij+EonihAgQDBvY87EWGI1OtOkw9d9OsjgJ\n\tHd8xCr9fNC9+b2GaFelCIIXo9HCZkhhPDzOZ0YPLph2rID2Vv8NnJ400QMrcJoe5bl45\n\tBfsbbbl+8XH+gzcfiIjtnTHRyOUx500bjBooti9ItVij60iEsK3w+ENz6sVNpVZmLdeX\n\t3vqCye+eSLvxIxem+bAaznCdfpdv2qkyOawV09KgCEhl+8jQ6qdwhvNvc1Ek9I6zV/nX\n\t3hkQ==","X-Gm-Message-State":"AHPjjUgmkwHINh5nkPBmFzyWD62HPsaMEqj9u/SsMEZm3FGVqp1k3lF/\n\t5siIdAwtfti/39l5oZ8Ri4o3eUew","X-Google-Smtp-Source":"AOwi7QAWnR0jcvQdIbjEd8LpEoOCRsIMgy+A1GAkuIk0HYCKJNKXbUmf2S52lBn35PBLbu1R+jlH9A==","X-Received":"by 10.55.134.135 with SMTP id i129mr435391qkd.214.1506110435805; \n\tFri, 22 Sep 2017 13:00:35 -0700 (PDT)","Subject":"Re: [PATCH net-next v2 1/3] net: dsa: use slave device phydev","To":"Vivien Didelot ,\n\tnetdev@vger.kernel.org","Cc":"linux-kernel@vger.kernel.org, kernel@savoirfairelinux.com,\n\t\"David S. Miller\" , Andrew Lunn ","References":"<20170922194045.18814-1-vivien.didelot@savoirfairelinux.com>\n\t<20170922194045.18814-2-vivien.didelot@savoirfairelinux.com>","From":"Florian Fainelli ","Message-ID":"<2262f266-6fb9-cce4-f83a-933dd41b9062@gmail.com>","Date":"Fri, 22 Sep 2017 13:00:32 -0700","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101\n\tThunderbird/52.2.1","MIME-Version":"1.0","In-Reply-To":"<20170922194045.18814-2-vivien.didelot@savoirfairelinux.com>","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-US","Content-Transfer-Encoding":"8bit","Sender":"netdev-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"netdev@vger.kernel.org"}},{"id":1773845,"web_url":"http://patchwork.ozlabs.org/comment/1773845/","msgid":"<20170922200304.GI3470@lunn.ch>","list_archive_url":null,"date":"2017-09-22T20:03:04","subject":"Re: [PATCH net-next v2 1/3] net: dsa: use slave device phydev","submitter":{"id":13608,"url":"http://patchwork.ozlabs.org/api/people/13608/","name":"Andrew Lunn","email":"andrew@lunn.ch"},"content":"On Fri, Sep 22, 2017 at 03:40:43PM -0400, Vivien Didelot wrote:\n> There is no need to store a phy_device in dsa_slave_priv since\n> net_device already provides one. Simply s/p->phy/dev->phydev/.\n> \n> While at it, return -ENODEV when it is NULL instead of -EOPNOTSUPP.\n\nI just did a quick poll for calling phy_mii_ioctl(). ENODEV seems the\nmost popular, second to EINVAL. Marvell drivers all use EOPNOTSUPP.\n\n> static int dsa_slave_nway_reset(struct net_device *dev)\n> {\n> -\tstruct dsa_slave_priv *p = netdev_priv(dev);\n> +\tif (!dev->phydev)\n> +\t\treturn -ENODEV;\n> \n> -\tif (p->phy != NULL)\n> -\t\treturn genphy_restart_aneg(p->phy);\n> -\n> -\treturn -EOPNOTSUPP;\n> +\treturn genphy_restart_aneg(dev->phydev);\n> }\n\nIt looks like this can now be replaced with phy_ethtool_nway_reset().\n\nIt could be there are other phy_ethtool_ helpers which can be used,\nnow that we have phydev in ndev.\n\n Andrew","headers":{"Return-Path":"","X-Original-To":"patchwork-incoming@ozlabs.org","Delivered-To":"patchwork-incoming@ozlabs.org","Authentication-Results":"ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=netdev-owner@vger.kernel.org;\n\treceiver=)","Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3xzPfc1qYQz9t32\n\tfor ;\n\tSat, 23 Sep 2017 06:07:52 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1752709AbdIVUDK (ORCPT );\n\tFri, 22 Sep 2017 16:03:10 -0400","from vps0.lunn.ch ([185.16.172.187]:52870 \"EHLO vps0.lunn.ch\"\n\trhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP\n\tid S1751937AbdIVUDI (ORCPT );\n\tFri, 22 Sep 2017 16:03:08 -0400","from andrew by vps0.lunn.ch with local (Exim 4.84_2)\n\t(envelope-from )\n\tid 1dvUAC-0002SG-0m; Fri, 22 Sep 2017 22:03:04 +0200"],"Date":"Fri, 22 Sep 2017 22:03:04 +0200","From":"Andrew Lunn ","To":"Vivien Didelot ","Cc":"netdev@vger.kernel.org, linux-kernel@vger.kernel.org,\n\tkernel@savoirfairelinux.com, \"David S. Miller\" ,\n\tFlorian Fainelli ","Subject":"Re: [PATCH net-next v2 1/3] net: dsa: use slave device phydev","Message-ID":"<20170922200304.GI3470@lunn.ch>","References":"<20170922194045.18814-1-vivien.didelot@savoirfairelinux.com>\n\t<20170922194045.18814-2-vivien.didelot@savoirfairelinux.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<20170922194045.18814-2-vivien.didelot@savoirfairelinux.com>","User-Agent":"Mutt/1.5.23 (2014-03-12)","Sender":"netdev-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"netdev@vger.kernel.org"}},{"id":1773856,"web_url":"http://patchwork.ozlabs.org/comment/1773856/","msgid":"<87r2uymr25.fsf@weeman.i-did-not-set--mail-host-address--so-tickle-me>","list_archive_url":null,"date":"2017-09-22T20:11:46","subject":"Re: [PATCH net-next v2 1/3] net: dsa: use slave device phydev","submitter":{"id":15889,"url":"http://patchwork.ozlabs.org/api/people/15889/","name":"Vivien Didelot","email":"vivien.didelot@savoirfairelinux.com"},"content":"Hi Florian,\n\nFlorian Fainelli writes:\n\n> On 09/22/2017 12:40 PM, Vivien Didelot wrote:\n>> There is no need to store a phy_device in dsa_slave_priv since\n>> net_device already provides one. Simply s/p->phy/dev->phydev/.\n>\n> You can therefore remove the phy_device from dsa_slave_priv, see below\n> for more comments. I will have to regress test the heck out of this,\n> this should take a few hours.\n\nOK, since this is a sensible topic, I will respin a v3 without this\npatch, so that a future patchset can address your comments below and\nalso gives you time to test this one patch alone.\n\n>> static int dsa_slave_port_attr_set(struct net_device *dev,\n>> @@ -435,12 +433,10 @@ static int\n>> dsa_slave_get_link_ksettings(struct net_device *dev,\n>> \t\t\t struct ethtool_link_ksettings *cmd)\n>> {\n>> -\tstruct dsa_slave_priv *p = netdev_priv(dev);\n>> +\tif (!dev->phydev)\n>> +\t\treturn -ENODEV;\n>> \n>> -\tif (!p->phy)\n>> -\t\treturn -EOPNOTSUPP;\n>> -\n>> -\tphy_ethtool_ksettings_get(p->phy, cmd);\n>> +\tphy_ethtool_ksettings_get(dev->phydev, cmd);\n>\n> This can be replaced by phy_ethtool_get_link_ksettings()\n>\n>> \n>> \treturn 0;\n>> }\n>> @@ -449,12 +445,10 @@ static int\n>> dsa_slave_set_link_ksettings(struct net_device *dev,\n>> \t\t\t const struct ethtool_link_ksettings *cmd)\n>> {\n>> -\tstruct dsa_slave_priv *p = netdev_priv(dev);\n>> +\tif (!dev->phydev)\n>> +\t\treturn -ENODEV;\n>> \n>> -\tif (p->phy != NULL)\n>> -\t\treturn phy_ethtool_ksettings_set(p->phy, cmd);\n>> -\n>> -\treturn -EOPNOTSUPP;\n>> +\treturn phy_ethtool_ksettings_set(dev->phydev, cmd);\n>> }\n>\n> This can disappear and you can assign this ethtool operation to\n> phy_ethtool_set_link_ksettings()\n>\n>> \n>> static void dsa_slave_get_drvinfo(struct net_device *dev,\n>> @@ -488,24 +482,20 @@ dsa_slave_get_regs(struct net_device *dev, struct ethtool_regs *regs, void *_p)\n>> \n>> static int dsa_slave_nway_reset(struct net_device *dev)\n>> {\n>> -\tstruct dsa_slave_priv *p = netdev_priv(dev);\n>> +\tif (!dev->phydev)\n>> +\t\treturn -ENODEV;\n>> \n>> -\tif (p->phy != NULL)\n>> -\t\treturn genphy_restart_aneg(p->phy);\n>> -\n>> -\treturn -EOPNOTSUPP;\n>> +\treturn genphy_restart_aneg(dev->phydev);\n>> }\n>\n> This can now disappear and you can use phy_ethtool_nway_reset() directly\n> in ethtool_ops\n>\n>> \n>> static u32 dsa_slave_get_link(struct net_device *dev)\n>> {\n>> -\tstruct dsa_slave_priv *p = netdev_priv(dev);\n>> +\tif (!dev->phydev)\n>> +\t\treturn -ENODEV;\n>> \n>> -\tif (p->phy != NULL) {\n>> -\t\tgenphy_update_link(p->phy);\n>> -\t\treturn p->phy->link;\n>> -\t}\n>> +\tgenphy_update_link(dev->phydev);\n>> \n>> -\treturn -EOPNOTSUPP;\n>> +\treturn dev->phydev->link;\n>> }\n>\n> This should certainly be just ethtool_op_get_link(), not sure why we\n> kept that around here...\n\nHaaa, good to read that! I wasn't sure about this, but with this patch\nthe slave phy ethtool functions seemed indeed quite generic...\n\n\nThanks,\n\n Vivien","headers":{"Return-Path":"","X-Original-To":"patchwork-incoming@ozlabs.org","Delivered-To":"patchwork-incoming@ozlabs.org","Authentication-Results":"ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=netdev-owner@vger.kernel.org;\n\treceiver=)","Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3xzPqV72N8z9s82\n\tfor ;\n\tSat, 23 Sep 2017 06:15:34 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1752158AbdIVUPX (ORCPT );\n\tFri, 22 Sep 2017 16:15:23 -0400","from mail.savoirfairelinux.com ([208.88.110.44]:56092 \"EHLO\n\tmail.savoirfairelinux.com\" rhost-flags-OK-OK-OK-OK) by\n\tvger.kernel.org with ESMTP id S1751878AbdIVUPV (ORCPT\n\t); Fri, 22 Sep 2017 16:15:21 -0400","from localhost (localhost [127.0.0.1])\n\tby mail.savoirfairelinux.com (Postfix) with ESMTP id 1219B9C2D3E;\n\tFri, 22 Sep 2017 16:15:21 -0400 (EDT)","from mail.savoirfairelinux.com ([127.0.0.1])\n\tby localhost (mail.savoirfairelinux.com [127.0.0.1]) (amavisd-new,\n\tport 10032)\n\twith ESMTP id bKhchSoQWPDu; Fri, 22 Sep 2017 16:15:20 -0400 (EDT)","from localhost (localhost [127.0.0.1])\n\tby mail.savoirfairelinux.com (Postfix) with ESMTP id 8E72E9C2D3D;\n\tFri, 22 Sep 2017 16:15:20 -0400 (EDT)","from mail.savoirfairelinux.com ([127.0.0.1])\n\tby localhost (mail.savoirfairelinux.com [127.0.0.1]) (amavisd-new,\n\tport 10026)\n\twith ESMTP id Uifn2Hs7nBKS; Fri, 22 Sep 2017 16:15:20 -0400 (EDT)","from localhost (unknown [192.168.49.104])\n\tby mail.savoirfairelinux.com (Postfix) with ESMTPSA id 64E369C2D41;\n\tFri, 22 Sep 2017 16:15:20 -0400 (EDT)"],"X-Virus-Scanned":"amavisd-new at mail.savoirfairelinux.com","From":"Vivien Didelot ","To":"Florian Fainelli , netdev@vger.kernel.org","Cc":"linux-kernel@vger.kernel.org, kernel@savoirfairelinux.com,\n\t\"David S. Miller\" , Andrew Lunn ","Subject":"Re: [PATCH net-next v2 1/3] net: dsa: use slave device phydev","In-Reply-To":"<2262f266-6fb9-cce4-f83a-933dd41b9062@gmail.com>","References":"<20170922194045.18814-1-vivien.didelot@savoirfairelinux.com>\n\t<20170922194045.18814-2-vivien.didelot@savoirfairelinux.com>\n\t<2262f266-6fb9-cce4-f83a-933dd41b9062@gmail.com>","Date":"Fri, 22 Sep 2017 16:11:46 -0400","Message-ID":"<87r2uymr25.fsf@weeman.i-did-not-set--mail-host-address--so-tickle-me>","MIME-Version":"1.0","Content-Type":"text/plain","Sender":"netdev-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"netdev@vger.kernel.org"}}]