From mboxrd@z Thu Jan 1 00:00:00 1970 From: Zhangfei Gao Subject: Re: [PATCH 2/3] net: hisilicon: new hip04 MDIO driver Date: Thu, 20 Mar 2014 18:53:50 +0800 Message-ID: References: <1395132017-15928-1-git-send-email-zhangfei.gao@linaro.org> <1395132017-15928-3-git-send-email-zhangfei.gao@linaro.org> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1 Cc: Zhangfei Gao , netdev , "David S. Miller" , "linux-arm-kernel@lists.infradead.org" , "devicetree@vger.kernel.org" To: Florian Fainelli Return-path: Received: from mail-ee0-f46.google.com ([74.125.83.46]:33526 "EHLO mail-ee0-f46.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750743AbaCTKxw (ORCPT ); Thu, 20 Mar 2014 06:53:52 -0400 In-Reply-To: Sender: netdev-owner@vger.kernel.org List-ID: Dear Florian On Wed, Mar 19, 2014 at 1:28 AM, Florian Fainelli wrote: > 2014-03-18 1:40 GMT-07:00 Zhangfei Gao : >> +static int hip04_mdio_reset(struct mii_bus *bus) >> +{ >> + int temp, err, i; >> + >> + for (i = 0; i < 2; i++) { >> + hip04_mdio_write(bus, i, 22, 0); >> + temp = hip04_mdio_read(bus, i, MII_BMCR); >> + temp |= BMCR_RESET; >> + err = hip04_mdio_write(bus, i, MII_BMCR, temp); >> + if (err < 0) >> + return err; >> + } >> + >> + mdelay(500); > > This does not look correct, you should iterate over all possible PHYs: > PHY_MAX_ADDR instead of hardcoding the loop to 2. OK, got it. Use 2 is since only have 2 phy in the board, will use PHY_MAX_ADDR instead. > > I think we might want to remove the mdio bus reset callback in general > as the PHY library should already take care of software resetting the > PHY to put it in a sane state, as well as waiting for the appropriate > delay before using, unlike here, where you do not poll for BMCR_RESET > to be cleared by the PHY. > Do you mean will move BMCR_RESET to common code, that's would be great. The mdio_reset is added here to get phy_id, otherwise the phy_id can not be read as well as detection. Thanks From mboxrd@z Thu Jan 1 00:00:00 1970 From: zhangfei.gao@gmail.com (Zhangfei Gao) Date: Thu, 20 Mar 2014 18:53:50 +0800 Subject: [PATCH 2/3] net: hisilicon: new hip04 MDIO driver In-Reply-To: References: <1395132017-15928-1-git-send-email-zhangfei.gao@linaro.org> <1395132017-15928-3-git-send-email-zhangfei.gao@linaro.org> Message-ID: To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org Dear Florian On Wed, Mar 19, 2014 at 1:28 AM, Florian Fainelli wrote: > 2014-03-18 1:40 GMT-07:00 Zhangfei Gao : >> +static int hip04_mdio_reset(struct mii_bus *bus) >> +{ >> + int temp, err, i; >> + >> + for (i = 0; i < 2; i++) { >> + hip04_mdio_write(bus, i, 22, 0); >> + temp = hip04_mdio_read(bus, i, MII_BMCR); >> + temp |= BMCR_RESET; >> + err = hip04_mdio_write(bus, i, MII_BMCR, temp); >> + if (err < 0) >> + return err; >> + } >> + >> + mdelay(500); > > This does not look correct, you should iterate over all possible PHYs: > PHY_MAX_ADDR instead of hardcoding the loop to 2. OK, got it. Use 2 is since only have 2 phy in the board, will use PHY_MAX_ADDR instead. > > I think we might want to remove the mdio bus reset callback in general > as the PHY library should already take care of software resetting the > PHY to put it in a sane state, as well as waiting for the appropriate > delay before using, unlike here, where you do not poll for BMCR_RESET > to be cleared by the PHY. > Do you mean will move BMCR_RESET to common code, that's would be great. The mdio_reset is added here to get phy_id, otherwise the phy_id can not be read as well as detection. Thanks