Re: [PATCH] ftgmac100: use bus name in mdio error messages

[Zev Weiss <zev@bewilderbeest.net>]

On Mon, May 16, 2022 at 09:54:35PM PDT, Joel Stanley wrote:
> On Tue, 17 May 2022 at 04:51, Zev Weiss <zev@bewilderbeest.net> wrote:
> >
> > On Mon, May 16, 2022 at 09:38:25PM PDT, Zev Weiss wrote:
> > > Previously we'd been using a device name retrieved via
> > > ftgmac100_data->phydev, but the mdio read/write functions may be
> > > called before that member is initialized in ftgmac100_phy_init(),
> > > leading to a NULL pointer dereference while printing the error message
> > > issued if the mdio access fails.  We can instead use bus->name, which
> > > is already available at that point.
> > >
> > > Signed-off-by: Zev Weiss <zev@bewilderbeest.net>
> > > Fixes: 538e75d3fc54 ("net: ftgmac100: add MDIO bus and phylib support")
> > > ---
> > >  drivers/net/ftgmac100.c | 4 ++--
> > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > >
> >
> > Sorry, forgot the subject-prefix tag on this one -- this is for u-boot
> > v2019.04-aspeed-openbmc in case it wasn't clear.
> 
> Thanks, I figured that out :)
> 
> How do you reproduce this one?
> 

I'm in the process of trying to transition e3c246d4i off of the 2016 
branch, and with the 2019 branch I'm finding u-boot spewing a bunch of 
output like

  : mdio read failed (phy:0 reg:2)
  : mdio read failed (phy:1 reg:2)
  : mdio read failed (phy:2 reg:2)
  ...

(sometimes with some amount of garbage before the colon at the start of 
the line, and with "eth1" instead after this patch.)

I'm currently experimenting with various Kconfig settings and dts tweaks 
(on two different variants of the hardware) to try to narrow down the 
underlying cause and hopefully eliminate the error spew entirely, but in 
the meantime I figured we might as well get that error-reporting path 
smoothed out a bit.

> I didn't realise we had downstream changes for this driver, we should
> ask aspeed to submit their downstream changes to u-boot mainline.

That sounds appropriate -- though as Cédric pointed out, since the 
relevant parts for this particular patch are already in mainline u-boot 
I should probably send this upstream as well.