From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752498AbcEPAlK (ORCPT ); Sun, 15 May 2016 20:41:10 -0400 Received: from szxga01-in.huawei.com ([58.251.152.64]:4245 "EHLO szxga01-in.huawei.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751908AbcEPAlI (ORCPT ); Sun, 15 May 2016 20:41:08 -0400 Subject: Re: [patch net-next 05/11] net: hns: add uniform interface for phy connection To: Andy Shevchenko , Yisen Zhuang , , , References: <1463127557-90824-1-git-send-email-Yisen.Zhuang@huawei.com> <1463127557-90824-6-git-send-email-Yisen.Zhuang@huawei.com> <1463144845.17131.342.camel@linux.intel.com> CC: , , , , , , , , , From: "Yankejian (Hackim Yim)" Message-ID: <573916F8.50005@huawei.com> Date: Mon, 16 May 2016 08:40:24 +0800 User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:38.0) Gecko/20100101 Thunderbird/38.5.1 MIME-Version: 1.0 In-Reply-To: <1463144845.17131.342.camel@linux.intel.com> Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit X-Originating-IP: [10.57.126.191] X-CFilter-Loop: Reflected X-Mirapoint-Virus-RAPID-Raw: score=unknown(0), refid=str=0001.0A020203.57391705.00A1,ss=1,re=0.000,recu=0.000,reip=0.000,cl=1,cld=1,fgs=0, ip=0.0.0.0, so=2013-06-18 04:22:30, dmn=2013-03-21 17:37:32 X-Mirapoint-Loop-Id: 127f05cd2b4b4b8905cd020bec374b20 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 2016/5/13 21:07, Andy Shevchenko wrote: > On Fri, 2016-05-13 at 16:19 +0800, Yisen Zhuang wrote: >> From: Kejian Yan >> >> As device_node is only used by OF case, HNS needs to treat the others >> cases including ACPI. It needs to use uniform ways to handle both of >> OF and ACPI. This patch chooses phy_device, and of_phy_connect and >> of_phy_attach are only used by OF case. It needs to add uniform >> interface >> to handle that sequence by both OF and ACPI. > --- a/drivers/net/ethernet/hisilicon/hns/hns_enet.c >> +++ b/drivers/net/ethernet/hisilicon/hns/hns_enet.c >> @@ -987,6 +987,41 @@ static void hns_nic_adjust_link(struct net_device >> *ndev) >> h->dev->ops->adjust_link(h, ndev->phydev->speed, ndev- >>> phydev->duplex); >> } >> >> +static >> +struct phy_device *hns_nic_phy_attach(struct net_device *dev, >> + struct phy_device *phy, >> + u32 flags, >> + phy_interface_t iface) >> +{ >> + int ret; >> + >> + if (!phy) >> + return NULL; > No need to use defensive programming here. > >> + >> + ret = phy_attach_direct(dev, phy, flags, iface); >> + >> + return ret ? NULL : phy; > Shouldn't it return an error? > > >> +} >> + >> +static >> +struct phy_device *hns_nic_phy_connect(struct net_device *dev, >> + struct phy_device *phy, >> + void (*hndlr)(struct >> net_device *), >> + u32 flags, >> + phy_interface_t iface) >> +{ >> + int ret; >> + >> + if (!phy) >> + return NULL; >> + >> + phy->dev_flags = flags; >> + >> + ret = phy_connect_direct(dev, phy, hndlr, iface); >> + >> + return ret ? NULL : phy; >> +} >> + > For now looks that above functions are redundant and you may call them > directly in below code. Hi Andy, Thanks for you suggestions, it will be fixed in next submit MBR, Kejian >> /** >> *hns_nic_init_phy - init phy >> *@ndev: net device >> @@ -996,16 +1031,17 @@ static void hns_nic_adjust_link(struct >> net_device *ndev) >> int hns_nic_init_phy(struct net_device *ndev, struct hnae_handle *h) >> { >> struct hns_nic_priv *priv = netdev_priv(ndev); >> - struct phy_device *phy_dev = NULL; >> + struct phy_device *phy_dev = h->phy_dev; >> >> - if (!h->phy_node) >> + if (!h->phy_dev) >> return 0; >> >> if (h->phy_if != PHY_INTERFACE_MODE_XGMII) >> - phy_dev = of_phy_connect(ndev, h->phy_node, >> - hns_nic_adjust_link, 0, h- >>> phy_if); >> + phy_dev = hns_nic_phy_connect(ndev, phy_dev, >> + hns_nic_adjust_link, >> + 0, h->phy_if); >> else >> - phy_dev = of_phy_attach(ndev, h->phy_node, 0, h- >>> phy_if); >> + phy_dev = hns_nic_phy_attach(ndev, phy_dev, 0, h- >>> phy_if); >