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

[Vicente Bergas]

On Saturday, May 11, 2019 4:56:56 PM CEST, Heiner Kallweit wrote:
> On 11.05.2019 16:46, Vicente Bergas wrote:
> > On Friday, May 10, 2019 10:28:06 PM CEST, Heiner Kallweit wrote:
> > > On 10.05.2019 17:05, Vicente Bergas wrote: ...
> >
> > Hello Heiner,
> > just tried your patch and indeed the NPE is gone. But still no network...
> > The MAC <-> PHY link was working before, so, maybe the rgmii 
> > delays are not
> > correctly configured.
>
> That's a question to the author of the original patch. My patch was just
> meant to fix the NPE. In which configuration are you using the RTL8211E?
> As a standalone PHY (with which MAC/driver?) or is it the integrated PHY
> in a member of the RTL8168 family?

It is the one on the Sapphire board, so is connected to the MAC on the
RK3399 SoC. It is on page 8 of the schematics:
http://dl.vamrs.com/products/sapphire_excavator/RK_SAPPHIRE_SOCBOARD_RK3399_LPDDR3D178P232SD8_V12_20161109HXS.pdf

> Serge: The issue with the NPE gave a hint already that you didn't test your
> patch. Was your patch based on an actual issue on some board and did you
> test it? We may have to consider reverting the patch.
>
> > With this change it is back to working:
> > --- a/drivers/net/phy/realtek.c
> > +++ b/drivers/net/phy/realtek.c
> > @@ -300,7 +300,6 @@
> >     }, {
> >         PHY_ID_MATCH_EXACT(0x001cc915),
> >         .name        = "RTL8211E Gigabit Ethernet",
> > -        .config_init    = &rtl8211e_config_init,
> >         .ack_interrupt    = &rtl821x_ack_interrupt,
> >         .config_intr    = &rtl8211e_config_intr,
> >         .suspend    = genphy_suspend,
> > That is basically reverting the patch.
> >
> > Regards,
> >  VicenÃ.
> >
> > > Nevertheless your proposed patch looks good to me, just one small change
> > > would be needed and it should be splitted.
> > >
> > > The change to phy-core I would consider a fix and it should be fine to
> > > submit it to net (net-next is closed currently).
> > >
> > > Adding the warning to the Realtek driver is fine, but this would be ...