lkml.org 
[lkml]   [2022]   [Oct]   [27]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRE: [PATCH net 1/1] net: phy: marvell: add link status check before enabling phy loopback
Date
> -----Original Message-----
> From: Andrew Lunn <andrew@lunn.ch>
> Sent: Tuesday, 20 September, 2022 7:46 AM
> To: Jamaluddin, Aminuddin <aminuddin.jamaluddin@intel.com>
> Cc: Heiner Kallweit <hkallweit1@gmail.com>; Russell King
> <linux@armlinux.org.uk>; David S . Miller <davem@davemloft.net>; Eric
> Dumazet <edumazet@google.com>; Jakub Kicinski <kuba@kernel.org>;
> Paolo Abeni <pabeni@redhat.com>; Ismail, Mohammad Athari
> <mohammad.athari.ismail@intel.com>; netdev@vger.kernel.org; linux-
> kernel@vger.kernel.org; stable@vger.kernel.org; Tan, Tee Min
> <tee.min.tan@intel.com>; Zulkifli, Muhammad Husaini
> <muhammad.husaini.zulkifli@intel.com>
> Subject: Re: [PATCH net 1/1] net: phy: marvell: add link status check before
> enabling phy loopback
>
> > > > Its required cabled plug in, back to back connection.
> > >
> > > Loopback should not require that. The whole point of loopback in the
> > > PHY is you can do it without needing a cable.
> > >
> > > > >
> > > > > Have you tested this with the cable unplugged?
> > > >
> > > > Yes we have and its expected to have the timeout. But the
> > > > self-test required the link to be up first before it can be run.
> > >
> > > So you get an ETIMEDOUT, and then skip the code which actually sets
> > > the LOOPBACK bit?
> >
> > If cable unplugged, test result will be displayed as 1. See comments below.
> >
> > >
> > > Please look at this again, and make it work without a cable.
> >
> > Related to this the flow without cable, what we see in the codes during
> debugging.
> > After the phy loopback bit was set.
> > The test will be run through this __stmmac_test_loopback()
> > https://elixir.bootlin.com/linux/v5.19.8/source/drivers/net/ethernet/s
> > tmicro/stmmac/stmmac_selftests.c#L320
> > Here, it will have another set of checking in dev_direct_xmit(),
> __dev_direct_xmit().
> > returning value 1(NET_XMIT_DROP)
> > https://elixir.bootlin.com/linux/v5.19.8/source/net/core/dev.c#L4288
> > Which means the interface is not available or the interface link status is not
> up.
> > For this case the interface link status is not up.
> > Thus failing the phy loopback test.
> > https://elixir.bootlin.com/linux/v5.19.8/source/net/core/dev.c#L4296
> > Since we don't own this __stmmac_test_loopback(), we conclude the
> behaviour was as expected.
> >
> > >
> > > Maybe you are addressing the wrong issue? Is the PHY actually
> > > performing loopback, but reporting the link is down? Maybe you need to
> fake a link up?
> > > Maybe you need the self test to not care about the link state, all
> > > it really needs is that packets get looped?
> >
> > When bit 14 was set, the link will be broken.
> > But before the self-test was triggered it requires link to be up as stated
> above comments.
>
> You have not said anything about my comment:
>
> > Maybe you need to fake a link up?
>
> My guess is, some PHYs are going to report link up when put into loopback.
> Others might not. For the Marvell PHY, it looks like you need to make
> marvell_read_status() return that the link is up if loopback is enabled.
>

We able to do the PHY loopback test, after fake link up without
cable plugged on as suggested above.
We will provide version 2 patch with minimum code changes
without having the status link check.
Only need to increase the msleep(200) to msleep(1000) inside
m88e1510_loopback() function.

Amin

\
 
 \ /
  Last update: 2022-10-27 06:36    [W:0.793 / U:0.508 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site