Re: [PATCH NET] net/hns:bugfix of ethtool -t phy self_test

[Andrew Lunn]

On Mon, Jun 19, 2017 at 02:00:43PM -0700, Florian Fainelli wrote:
> On 06/16/2017 02:24 AM, Lin Yun Sheng wrote:
> > This patch fixes the phy loopback self_test failed issue. when
> > Marvell Phy Module is loaded, it will powerdown fiber when doing
> > phy loopback self test, which cause phy loopback self_test fail.
> > 
> > Signed-off-by: Lin Yun Sheng <linyunsheng@xxxxxxxxxx>
> > ---
> >  drivers/net/ethernet/hisilicon/hns/hns_ethtool.c | 16 ++++++++++++++--
> >  1 file changed, 14 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/net/ethernet/hisilicon/hns/hns_ethtool.c b/drivers/net/ethernet/hisilicon/hns/hns_ethtool.c
> > index b8fab14..e95795b 100644
> > --- a/drivers/net/ethernet/hisilicon/hns/hns_ethtool.c
> > +++ b/drivers/net/ethernet/hisilicon/hns/hns_ethtool.c
> > @@ -288,9 +288,15 @@ static int hns_nic_config_phy_loopback(struct phy_device *phy_dev, u8 en)
> 
> The question really is, why is not this properly integrated into the PHY
> driver and PHYLIB such that the only thing the Ethernet MAC driver has
> to call is a function of the PHY driver putting it in self-test?

This whole driver pokes various PHY registers, rather than use
phylib. And it does so without taking the PHY lock. It also assumes it
is a Marvell PHY and i don't see anywhere it actually verifies this.

This is all broken.

     Andrew