On Wed, Apr 28, 2021 at 08:06:05AM +0000, Joakim Zhang wrote:

Hi Florian,

quoted

-----Original Message-----
From: Florian Fainelli <f.fainelli@gmail.com>
Sent: 2021年4月28日 0:41
To: Joakim Zhang <redacted>; Oleksij Rempel
[off-list ref]
Cc: Shawn Guo <shawnguo@kernel.org>; Sascha Hauer
[off-list ref]; Andrew Lunn [off-list ref]; Heiner Kallweit
[off-list ref]; Fugang Duan [off-list ref];
kernel@pengutronix.de; netdev@vger.kernel.org;
linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org;
dl-linux-imx [off-list ref]; Fabio Estevam [off-list ref];
David Jander [off-list ref]; Russell King [off-list ref];
Philippe Schenker [off-list ref]
Subject: Re: [PATCH net-next v3 0/6] provide generic net selftest support



On 4/26/2021 9:48 PM, Joakim Zhang wrote:

quoted

quoted

-----Original Message-----
From: Oleksij Rempel <o.rempel@pengutronix.de>
Sent: 2021年4月23日 12:37
To: Joakim Zhang <redacted>
Cc: Shawn Guo <shawnguo@kernel.org>; Sascha Hauer
[off-list ref]; Andrew Lunn [off-list ref]; Florian
Fainelli [off-list ref]; Heiner Kallweit
[off-list ref]; Fugang Duan [off-list ref];
kernel@pengutronix.de; netdev@vger.kernel.org;
linux-arm-kernel@lists.infradead.org;
linux-kernel@vger.kernel.org; dl-linux-imx [off-list ref]; Fabio
Estevam [off-list ref]; David Jander [off-list ref];
Russell King [off-list ref]; Philippe Schenker
[off-list ref]
Subject: Re: [PATCH net-next v3 0/6] provide generic net selftest
support

Hi Joakim,

On Fri, Apr 23, 2021 at 03:18:32AM +0000, Joakim Zhang wrote:

quoted

Hi Oleksij,

I look both stmmac selftest code and this patch set. For stmmac, if
PHY

doesn't support loopback, it will fallthrough to MAC loopback.

quoted

You provide this generic net selftest support based on PHY loopback,
I have a

question, is it possible to extend it also support MAC loopback later?

Yes. If you have interest and time to implement it, please do.
It should be some kind of generic callback as phy_loopback() and if
PHY and MAC loopbacks are supported we need to tests both variants.

Hi Oleksij,

Yes, I can try to implement it when I am free, but I still have some questions:
1. Where we place the generic function? Such as mac_loopback().
2. MAC is different from PHY, need program different registers to enable

loopback on different SoCs, that means we need get MAC private data from
"struct net_device".

quoted

So we need a callback for MAC drivers, where we extend this callback? Could

be "struct net_device_ops"? Such as ndo_set_loopback?

Even for PHY devices, if we implemented external PHY loopback in the future,
the programming would be different from one vendor to another. I am starting
to wonder if the existing ethtool self-tests are the best API to expose the ability
for an user to perform PHY and MAC loopback testing.

From an Ethernet MAC and PHY driver perspective, what I would imagine we
could have for a driver API is:

enum ethtool_loopback_mode {
	ETHTOOL_LOOPBACK_OFF,
	ETHTOOL_LOOPBACK_PHY_INTERNAL,
	ETHTOOL_LOOPBACK_PHY_EXTERNAL,
	ETHTOOL_LOOPBACK_MAC_INTERNAL,
	ETHTOOL_LOOPBACK_MAC_EXTERNAL,
	ETHTOOL_LOOPBACK_FIXTURE,
	__ETHTOOL_LOOPBACK_MAX
};

What's the difference between internal and external loopback for both PHY and MAC? I am not familiar with these concepts. Thanks.

For example KSZ9031 PHY. It supports two loopback modes. See page 23:
https://ww1.microchip.com/downloads/en/DeviceDoc/00002096E.pdf

TI DP83TC811R-Q1 PHY supports 4 modes. See page 27:
https://www.ti.com/lit/ds/symlink/dp83tc811r-q1.pdf

Best Regards,
Joakim Zhang

quoted

	int (*ndo_set_loopback_mode)(struct net_device *dev, enum
ethtool_loopback_mode mode);

and within the Ethernet MAC driver you would do something like this:

	switch (mode) {
	case ETHTOOL_LOOPBACK_PHY_INTERNAL:
	case ETHTOOL_LOOPBACK_PHY_EXTERNAL:
	case ETHTOOL_LOOPBACK_OFF:
		ret = phy_loopback(ndev->phydev, mode);
		break;
	/* Other case statements implemented in driver */

we would need to change the signature of phy_loopback() to accept being
passed ethtool_loopback_mode so we can support different modes.

Whether we want to continue using the self-tests API, or if we implement a
new ethtool command in order to request a loopback operation is up for
discussion.
--
Florian

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |