From mboxrd@z Thu Jan 1 00:00:00 1970 From: Dan Murphy Subject: Re: [PATCH] phy dp83867: depend on CONFIG_OF_MDIO Date: Mon, 16 May 2016 08:38:20 -0500 Message-ID: <5739CD4C.7020208@ti.com> References: <1463398095-123269-1-git-send-email-agraf@suse.de> Mime-Version: 1.0 Content-Type: text/plain; charset="windows-1252" Content-Transfer-Encoding: 7bit Cc: , To: Alexander Graf , Return-path: Received: from bear.ext.ti.com ([198.47.19.11]:47697 "EHLO bear.ext.ti.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751815AbcEPNiZ (ORCPT ); Mon, 16 May 2016 09:38:25 -0400 In-Reply-To: <1463398095-123269-1-git-send-email-agraf@suse.de> Sender: netdev-owner@vger.kernel.org List-ID: Alexander On 05/16/2016 06:28 AM, Alexander Graf wrote: > The DP83867 phy driver doesn't actually work when CONFIG_OF_MDIO isn't enabled. > It simply passes the device tree test, but leaves all internal configuration > initialized at 0. Then it configures the phy with those values and renders a > previously working configuration useless. > > This patch makes sure that we only build the DP83867 phy code when > CONFIG_OF_MDIO is set, to not run into that problem. > > Signed-off-by: Alexander Graf > --- > drivers/net/phy/Kconfig | 1 + > drivers/net/phy/dp83867.c | 7 ------- > 2 files changed, 1 insertion(+), 7 deletions(-) > > diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig > index 6dad9a9..4265ad5 100644 > --- a/drivers/net/phy/Kconfig > +++ b/drivers/net/phy/Kconfig > @@ -148,6 +148,7 @@ config DP83848_PHY > > config DP83867_PHY > tristate "Drivers for Texas Instruments DP83867 Gigabit PHY" > + depends on OF_MDIO > ---help--- > Currently supports the DP83867 PHY. > > diff --git a/drivers/net/phy/dp83867.c b/drivers/net/phy/dp83867.c > index 2afa61b..ff867ba 100644 > --- a/drivers/net/phy/dp83867.c > +++ b/drivers/net/phy/dp83867.c > @@ -99,7 +99,6 @@ static int dp83867_config_intr(struct phy_device *phydev) > return phy_write(phydev, MII_DP83867_MICR, micr_status); > } > > -#ifdef CONFIG_OF_MDIO > static int dp83867_of_init(struct phy_device *phydev) > { > struct dp83867_private *dp83867 = phydev->priv; > @@ -123,12 +122,6 @@ static int dp83867_of_init(struct phy_device *phydev) > return of_property_read_u32(of_node, "ti,fifo-depth", > &dp83867->fifo_depth); > } > -#else > -static int dp83867_of_init(struct phy_device *phydev) > -{ > - return 0; > -} > -#endif /* CONFIG_OF_MDIO */ > > static int dp83867_config_init(struct phy_device *phydev) > { I don't think we want this to depend solely on OF_MDIO. The #else case should probably be coded to look at platform data, if it exists. I don't have any boards that still used platform data to test this out so I did not feel comfortable adding code I could not test. The platform data should contain the RGMII tx/rx delay and FIFO control. Dan -- ------------------ Dan Murphy