Re: Re: [PATCH] net: ethernet: mediatek: fix warning in phy_start_aneg

From: Sean Wang
Date: Tue Jan 08 2019 - 14:28:08 EST


On Tue, Jan 8, 2019 at 11:07 AM Frank Wunderlich
<frank-w@xxxxxxxxxxxxxxx> wrote:
>
>
> > In mtk_phy_connect() I would say this complete block can be removed.
> > All this is handled internally by phylib.
> >
> > dev->phydev->autoneg = AUTONEG_ENABLE;
> > dev->phydev->speed = 0;
> > dev->phydev->duplex = 0;
> >
> > phy_set_max_speed(dev->phydev, SPEED_1000);
> > phy_support_asym_pause(dev->phydev);
> > linkmode_copy(dev->phydev->advertising, dev->phydev->supported);
> > linkmode_set_bit(ETHTOOL_LINK_MODE_Autoneg_BIT,
> > dev->phydev->advertising);
> > phy_start_aneg(dev->phydev);
> >
> > Could you please test this?
>
> seems to work well, no warning in dmesg
>
> traffic 940Mbit sending, 900 receive
>

The change to phy initialized valve determined by phylib looks good to
me and simplify the mac driver a lot. You could have it in v2.

> > And a further proposal:
> > Instead of the open-coded dev_info() in mtk_phy_connect_node()
> > you could use phy_attached_info().
>
> have not tried this yet...because it's only an info-message and needs different parameters...have to look how i can convert it

I think it can be done in a separate patch

>
> > Calling phy_start() in the ndo_open callback is fine, you can leave
> > it there.
>
> have it re-added
>
> pushed to my repo: https://github.com/frank-w/BPI-R2-4.14/commits/5.0-rc
>
> regards Frank