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

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".
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
};

	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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel