> -----Original Message----- > From: Andrew Lunn <andrew@xxxxxxx> > Sent: Tuesday, 20 September, 2022 7:46 AM > To: Jamaluddin, Aminuddin <aminuddin.jamaluddin@xxxxxxxxx> > Cc: Heiner Kallweit <hkallweit1@xxxxxxxxx>; Russell King > <linux@xxxxxxxxxxxxxxx>; David S . Miller <davem@xxxxxxxxxxxxx>; Eric > Dumazet <edumazet@xxxxxxxxxx>; Jakub Kicinski <kuba@xxxxxxxxxx>; > Paolo Abeni <pabeni@xxxxxxxxxx>; Ismail, Mohammad Athari > <mohammad.athari.ismail@xxxxxxxxx>; netdev@xxxxxxxxxxxxxxx; linux- > kernel@xxxxxxxxxxxxxxx; stable@xxxxxxxxxxxxxxx; Tan, Tee Min > <tee.min.tan@xxxxxxxxx>; Zulkifli, Muhammad Husaini > <muhammad.husaini.zulkifli@xxxxxxxxx> > 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
