Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Mon, 13 May 2019 15:42:26 +0300
From:   Serge Semin <fancer.lancer@gmail.com>
To:     Vicente Bergas <vicencb@gmail.com>
Cc:     Heiner Kallweit <hkallweit1@gmail.com>,
        Russell King <rmk+kernel@armlinux.org.uk>,
        Andrew Lunn <andrew@lunn.ch>,
        Florian Fainelli <f.fainelli@gmail.com>,
        netdev@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: net: phy: realtek: regression, kernel null pointer dereference
Message-ID: <20190513124225.odm3shcfo3tsq6xk@mobilestation>
References: <16f75ff4-e3e3-4d96-b084-e772e3ce1c2b@gmail.com>
 <742a2235-4571-aa7d-af90-14c708205c6f@gmail.com>
 <11446b0b-c8a4-4e5f-bfa0-0892b500f467@gmail.com>
 <61831f43-3b24-47d9-ec6f-15be6a4568c5@gmail.com>
 <0f16b2c5-ef2a-42a1-acdc-08fa9971b347@gmail.com>
 <20190513102941.4ocb3tz3wmh3pj4t@mobilestation>
 <20190513105104.af7d7n337lxqac63@mobilestation>
 <cf1e81d9-6f91-41fe-a390-b9688e5707f7@gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <cf1e81d9-6f91-41fe-a390-b9688e5707f7@gmail.com>
User-Agent: NeoMutt/20180716
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

Hello,

On Mon, May 13, 2019 at 02:19:17PM +0200, Vicente Bergas wrote:
> On Monday, May 13, 2019 12:51:05 PM CEST, Serge Semin wrote:
> > On Mon, May 13, 2019 at 01:29:42PM +0300, Serge Semin wrote:
> > > Hello Vincente,
> > > 
> > > On Sat, May 11, 2019 at 05:06:04PM +0200, Vicente Bergas wrote: ...
> > 
> > Hmm, just figured out, that in the datasheet RXDLY/TXDLY pins are
> > actually grounded, so
> > phy-mode="rgmii" should work for you. Are you sure that on your actual
> > hardware the
> > resistors aren't placed differently?
> 
> That is correct, the schematic has pull-down resistors and placeholders for
> pull-up resistors. On the board I can confirm the pull-ups are not
> populated and the pull-downs are.
> But the issue seems unrelated to this.
> 
> I have traced it down to a deadlock while trying to acquire a mutex.
> It hangs here:
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/net/phy/realtek.c?id=47782361aca2#n220
> while waiting for this mutex:
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/net/phy/mdio_bus.c?id=47782361aca2#n692
> 
> Regards,
>  Vicen?.
> 

Ahh, I see. Then using lock-less version of the access methods must fix the
problem. You could try something like this:
-------------------------------------------------------------------------------
diff --git a/drivers/net/phy/realtek.c b/drivers/net/phy/realtek.c
index 761ce3b1e7bd..14b61da1f32a 100644
--- a/drivers/net/phy/realtek.c
+++ b/drivers/net/phy/realtek.c
@@ -217,12 +217,12 @@ static int rtl8211e_config_init(struct phy_device *phydev)
 	if (oldpage < 0)
 		goto err_restore_page;
 
-	ret = phy_write(phydev, RTL821x_EXT_PAGE_SELECT, 0xa4);
+	ret = __phy_write(phydev, RTL821x_EXT_PAGE_SELECT, 0xa4);
 	if (ret)
 		goto err_restore_page;
 
-	ret = phy_modify(phydev, 0x1c, RTL8211E_TX_DELAY | RTL8211E_RX_DELAY,
-			 val);
+	ret = __phy_modify(phydev, 0x1c, RTL8211E_TX_DELAY | RTL8211E_RX_DELAY,
+			   val);
 
 err_restore_page:
 	return phy_restore_page(phydev, oldpage, ret);
-------------------------------------------------------------------------------

-Sergey

> > The current config register state can be read from the 0x1c extension
> > page. Something
> > like this:
> > --- a/drivers/net/phy/realtek.c
> > +++ b/drivers/net/phy/realtek.c
> > @@ -221,6 +221,9 @@ static int rtl8211e_config_init(struct phy_device
> > *phydev)
> >  	if (ret)
> >  		goto err_restore_page;
> > +	ret = phy_read(phydev, 0x1c);
> > +	dev_info(&phydev->mdio.dev, "PHY config register %08x\n", ret);
> > +
> >  	ret = phy_modify(phydev, 0x1c, RTL8211E_TX_DELAY | RTL8211E_RX_DELAY,
> >  			 val);
> > -Sergey
>