Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752842AbdFQKlT (ORCPT ); Sat, 17 Jun 2017 06:41:19 -0400 Received: from szxga02-in.huawei.com ([45.249.212.188]:8321 "EHLO szxga02-in.huawei.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752699AbdFQKlR (ORCPT ); Sat, 17 Jun 2017 06:41:17 -0400 From: Salil Mehta To: Florian Fainelli , "davem@davemloft.net" CC: "Zhuangyuzeng (Yisen)" , huangdaode , "lipeng (Y)" , "mehta.salil.lnk@gmail.com" , "netdev@vger.kernel.org" , "linux-kernel@vger.kernel.org" , Linuxarm Subject: RE: [PATCH V2 net-next 6/8] net: hns3: Add MDIO support to HNS3 Ethernet driver for hip08 SoC Thread-Topic: [PATCH V2 net-next 6/8] net: hns3: Add MDIO support to HNS3 Ethernet driver for hip08 SoC Thread-Index: AQHS5JptS1KqKLo8T0aC7jAwXGapTKIjU8IAgAWNQPA= Date: Sat, 17 Jun 2017 10:40:58 +0000 Message-ID: References: <20170613231035.494020-1-salil.mehta@huawei.com> <20170613231035.494020-7-salil.mehta@huawei.com> In-Reply-To: Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.203.181.160] Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 X-CFilter-Loop: Reflected X-Mirapoint-Virus-RAPID-Raw: score=unknown(0), refid=str=0001.0A020206.59450747.00A6,ss=1,re=0.000,recu=0.000,reip=0.000,cl=1,cld=1,fgs=0, ip=169.254.2.25, so=2014-11-16 11:51:01, dmn=2013-03-21 17:37:32 X-Mirapoint-Loop-Id: c6e8d9794e350993ed9ae57fc5613f03 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Transfer-Encoding: 8bit X-MIME-Autoconverted: from base64 to 8bit by mail.home.local id v5HAfOR3009422 Content-Length: 4237 Lines: 158 Hi Florian > -----Original Message----- > From: Florian Fainelli [mailto:f.fainelli@gmail.com] > Sent: Wednesday, June 14, 2017 12:46 AM > To: Salil Mehta; davem@davemloft.net > Cc: Zhuangyuzeng (Yisen); huangdaode; lipeng (Y); > mehta.salil.lnk@gmail.com; netdev@vger.kernel.org; linux- > kernel@vger.kernel.org; Linuxarm > Subject: Re: [PATCH V2 net-next 6/8] net: hns3: Add MDIO support to > HNS3 Ethernet driver for hip08 SoC > > On 06/13/2017 04:10 PM, Salil Mehta wrote: > > This patch adds the support of MDIO bus interface for HNS3 driver. > > Code provides various interfaces to start and stop the PHY layer > > and to read and write the MDIO bus or PHY. > > > > Signed-off-by: Daode Huang > > Signed-off-by: lipeng > > Signed-off-by: Salil Mehta > > Signed-off-by: Yisen Zhuang > > --- > > > + phy = get_phy_device(mdio_bus, mac->phy_addr, mac->is_c45); > > + if (!phy || IS_ERR(phy)) { > > + dev_err(mdio_bus->parent, "Failed to get phy device\n"); > > + ret = -EIO; > > + goto err_mdio_register; > > + } > > + > > + phy->irq = mdio_bus->irq[mac->phy_addr]; > > + > > + /* All data is now stored in the phy struct; > > + * register it > > + */ > > + ret = phy_device_register(phy); > > + if (ret) { > > + ret = -ENODEV; > > + goto err_phy_register; > > + } > > Until this gets fixed in the core PHY library, it's okay to do that, > but > please fix the For now we are planning to drop the support of C45 till we fix this issue in the core. I Will try to address this as a separate patch. Thanks Salil > > > + > > + mac->phy_dev = phy; > > + > > + return 0; > > + > > +err_phy_register: > > + phy_device_free(phy); > > + > > +err_mdio_register: > > + mdiobus_unregister(mdio_bus); > > + mdiobus_free(mdio_bus); > > +err_miibus_alloc: > > + return ret; > > +} > > + > > +static void hclge_mac_adjust_link(struct net_device *net_dev) > > +{ > > + int duplex; > > + int speed; > > + struct hclge_mac *hw_mac; > > + struct hclge_hw *hw; > > + struct hclge_dev *hdev; > > + > > + if (!net_dev) > > + return; > > + > > + hw_mac = container_of(net_dev, struct hclge_mac, ndev); > > + hw = container_of(hw_mac, struct hclge_hw, mac); > > + hdev = hw->back; > > + > > + speed = hw_mac->phy_dev->speed; > > + duplex = hw_mac->phy_dev->duplex; > > + > > + /* update antoneg. */ > > + hw_mac->autoneg = hw_mac->phy_dev->autoneg; > > + > > + if ((hw_mac->speed != speed) || (hw_mac->duplex != duplex)) > > + (void)hclge_cfg_mac_speed_dup(hdev, speed, !!duplex); > > +} > > + > > +int hclge_mac_start_phy(struct hclge_dev *hdev) > > +{ > > + struct hclge_mac *mac = &hdev->hw.mac; > > + struct phy_device *phy_dev = mac->phy_dev; > > + int ret; > > + > > + if (!phy_dev) > > + return 0; > > + > > + if (mac->phy_if != PHY_INTERFACE_MODE_XGMII) { > > + phy_dev->dev_flags = 0; > > + > > + ret = phy_connect_direct(&mac->ndev, phy_dev, > > + hclge_mac_adjust_link, > > + mac->phy_if); > > + phy_dev->supported = SUPPORTED_10baseT_Half | > > + SUPPORTED_10baseT_Full | > > + SUPPORTED_100baseT_Half | > > + SUPPORTED_100baseT_Full | > > + SUPPORTED_Autoneg | > > + SUPPORTED_1000baseT_Full; > > + > > + phy_dev->autoneg = false; > > + } else { > > + ret = phy_attach_direct(&mac->ndev, phy_dev, 0, mac- > >phy_if); > > + phy_dev->supported = SUPPORTED_10000baseR_FEC | > > + SUPPORTED_10000baseKR_Full; > > + } > > I really don't see why you don't take the exact same path whether it's > XGMII or not, just like you are not masking with that the PHY driver > already supports, this needs fixing. Sure will fix it in next V3 patch. Thanks Salil > > > + if (unlikely(ret)) > > + return -ENODEV; > > + > > + phy_start(phy_dev); > > + > > + return 0; > > +} > > + > > +void hclge_mac_stop_phy(struct hclge_dev *hdev) > > +{ > > + struct hclge_mac *mac = &hdev->hw.mac; > > + struct phy_device *phy_dev = mac->phy_dev; > > + > > + if (!phy_dev) > > + return; > > + > > + phy_stop(phy_dev); > > + > > + if (mac->phy_if != PHY_INTERFACE_MODE_XGMII) > > + phy_disconnect(phy_dev); > > + else > > + phy_detach(phy_dev); > > Same here. Will fix it in next V3 patch. Thanks Salil > -- > Florian