Re: net: phy: realtek: regression, kernel null pointer dereference

[Serge Semin]

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
>