| Message ID | 20210419130106.6707-1-o.rempel@pengutronix.de |
|---|---|
| Headers | show |
| Series | provide generic net selftest support | expand |
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. 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? Best Regards, Joakim Zhang > -----Original Message----- > From: Oleksij Rempel <o.rempel@pengutronix.de> > Sent: 2021年4月19日 21:01 > To: Shawn Guo <shawnguo@kernel.org>; Sascha Hauer > <s.hauer@pengutronix.de>; Andrew Lunn <andrew@lunn.ch>; Florian Fainelli > <f.fainelli@gmail.com>; Heiner Kallweit <hkallweit1@gmail.com>; Fugang > Duan <fugang.duan@nxp.com> > Cc: Oleksij Rempel <o.rempel@pengutronix.de>; kernel@pengutronix.de; > netdev@vger.kernel.org; linux-arm-kernel@lists.infradead.org; > linux-kernel@vger.kernel.org; dl-linux-imx <linux-imx@nxp.com>; Fabio > Estevam <festevam@gmail.com>; David Jander <david@protonic.nl>; Russell > King <linux@armlinux.org.uk>; Philippe Schenker > <philippe.schenker@toradex.com> > Subject: [PATCH net-next v3 0/6] provide generic net selftest support > > changes v3: > - make more granular tests > - enable loopback for all PHYs by default > - fix allmodconfig build errors > - poll for link status update after switching to the loopback mode > > changes v2: > - make generic selftests available for all networking devices. > - make use of net_selftest* on FEC, ag71xx and all DSA switches. > - add loopback support on more PHYs. > > This patch set provides diagnostic capabilities for some iMX, ag71xx or any DSA > based devices. For proper functionality, PHY loopback support is needed. > So far there is only initial infrastructure with basic tests. > > Oleksij Rempel (6): > net: phy: execute genphy_loopback() per default on all PHYs > net: phy: genphy_loopback: add link speed configuration > net: add generic selftest support > net: fec: make use of generic NET_SELFTESTS library > net: ag71xx: make use of generic NET_SELFTESTS library > net: dsa: enable selftest support for all switches by default > > drivers/net/ethernet/atheros/Kconfig | 1 + > drivers/net/ethernet/atheros/ag71xx.c | 20 +- > drivers/net/ethernet/freescale/Kconfig | 1 + > drivers/net/ethernet/freescale/fec_main.c | 7 + > drivers/net/phy/phy.c | 3 +- > drivers/net/phy/phy_device.c | 35 +- > include/linux/phy.h | 1 + > include/net/dsa.h | 2 + > include/net/selftests.h | 12 + > net/Kconfig | 4 + > net/core/Makefile | 1 + > net/core/selftests.c | 400 > ++++++++++++++++++++++ > net/dsa/Kconfig | 1 + > net/dsa/slave.c | 21 ++ > 14 files changed, 500 insertions(+), 9 deletions(-) create mode 100644 > include/net/selftests.h create mode 100644 net/core/selftests.c > > -- > 2.29.2
Hi Joakim, On Fri, Apr 23, 2021 at 03:18:32AM +0000, Joakim Zhang wrote: > > 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. > 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. Best regards, Oleksij > > -----Original Message----- > > From: Oleksij Rempel <o.rempel@pengutronix.de> > > Sent: 2021年4月19日 21:01 > > To: Shawn Guo <shawnguo@kernel.org>; Sascha Hauer > > <s.hauer@pengutronix.de>; Andrew Lunn <andrew@lunn.ch>; Florian Fainelli > > <f.fainelli@gmail.com>; Heiner Kallweit <hkallweit1@gmail.com>; Fugang > > Duan <fugang.duan@nxp.com> > > Cc: Oleksij Rempel <o.rempel@pengutronix.de>; kernel@pengutronix.de; > > netdev@vger.kernel.org; linux-arm-kernel@lists.infradead.org; > > linux-kernel@vger.kernel.org; dl-linux-imx <linux-imx@nxp.com>; Fabio > > Estevam <festevam@gmail.com>; David Jander <david@protonic.nl>; Russell > > King <linux@armlinux.org.uk>; Philippe Schenker > > <philippe.schenker@toradex.com> > > Subject: [PATCH net-next v3 0/6] provide generic net selftest support > > > > changes v3: > > - make more granular tests > > - enable loopback for all PHYs by default > > - fix allmodconfig build errors > > - poll for link status update after switching to the loopback mode > > > > changes v2: > > - make generic selftests available for all networking devices. > > - make use of net_selftest* on FEC, ag71xx and all DSA switches. > > - add loopback support on more PHYs. > > > > This patch set provides diagnostic capabilities for some iMX, ag71xx or any DSA > > based devices. For proper functionality, PHY loopback support is needed. > > So far there is only initial infrastructure with basic tests. > > > > Oleksij Rempel (6): > > net: phy: execute genphy_loopback() per default on all PHYs > > net: phy: genphy_loopback: add link speed configuration > > net: add generic selftest support > > net: fec: make use of generic NET_SELFTESTS library > > net: ag71xx: make use of generic NET_SELFTESTS library > > net: dsa: enable selftest support for all switches by default > > > > drivers/net/ethernet/atheros/Kconfig | 1 + > > drivers/net/ethernet/atheros/ag71xx.c | 20 +- > > drivers/net/ethernet/freescale/Kconfig | 1 + > > drivers/net/ethernet/freescale/fec_main.c | 7 + > > drivers/net/phy/phy.c | 3 +- > > drivers/net/phy/phy_device.c | 35 +- > > include/linux/phy.h | 1 + > > include/net/dsa.h | 2 + > > include/net/selftests.h | 12 + > > net/Kconfig | 4 + > > net/core/Makefile | 1 + > > net/core/selftests.c | 400 > > ++++++++++++++++++++++ > > net/dsa/Kconfig | 1 + > > net/dsa/slave.c | 21 ++ > > 14 files changed, 500 insertions(+), 9 deletions(-) create mode 100644 > > include/net/selftests.h create mode 100644 net/core/selftests.c > > > > -- > > 2.29.2 > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel -- 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 |
> -----Original Message----- > From: Oleksij Rempel <o.rempel@pengutronix.de> > Sent: 2021年4月23日 12:37 > To: Joakim Zhang <qiangqing.zhang@nxp.com> > Cc: Shawn Guo <shawnguo@kernel.org>; Sascha Hauer > <s.hauer@pengutronix.de>; Andrew Lunn <andrew@lunn.ch>; Florian Fainelli > <f.fainelli@gmail.com>; Heiner Kallweit <hkallweit1@gmail.com>; Fugang > Duan <fugang.duan@nxp.com>; kernel@pengutronix.de; > netdev@vger.kernel.org; linux-arm-kernel@lists.infradead.org; > linux-kernel@vger.kernel.org; dl-linux-imx <linux-imx@nxp.com>; Fabio > Estevam <festevam@gmail.com>; David Jander <david@protonic.nl>; Russell > King <linux@armlinux.org.uk>; Philippe Schenker > <philippe.schenker@toradex.com> > 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: > > > > 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. > > 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? Best Regards, Joakim Zhang > Best regards, > Oleksij > > > > -----Original Message----- > > > From: Oleksij Rempel <o.rempel@pengutronix.de> > > > Sent: 2021年4月19日 21:01 > > > To: Shawn Guo <shawnguo@kernel.org>; Sascha Hauer > > > <s.hauer@pengutronix.de>; Andrew Lunn <andrew@lunn.ch>; Florian > > > Fainelli <f.fainelli@gmail.com>; Heiner Kallweit > > > <hkallweit1@gmail.com>; Fugang Duan <fugang.duan@nxp.com> > > > Cc: Oleksij Rempel <o.rempel@pengutronix.de>; kernel@pengutronix.de; > > > netdev@vger.kernel.org; linux-arm-kernel@lists.infradead.org; > > > linux-kernel@vger.kernel.org; dl-linux-imx <linux-imx@nxp.com>; > > > Fabio Estevam <festevam@gmail.com>; David Jander > > > <david@protonic.nl>; Russell King <linux@armlinux.org.uk>; Philippe > > > Schenker <philippe.schenker@toradex.com> > > > Subject: [PATCH net-next v3 0/6] provide generic net selftest > > > support > > > > > > changes v3: > > > - make more granular tests > > > - enable loopback for all PHYs by default > > > - fix allmodconfig build errors > > > - poll for link status update after switching to the loopback mode > > > > > > changes v2: > > > - make generic selftests available for all networking devices. > > > - make use of net_selftest* on FEC, ag71xx and all DSA switches. > > > - add loopback support on more PHYs. > > > > > > This patch set provides diagnostic capabilities for some iMX, ag71xx > > > or any DSA based devices. For proper functionality, PHY loopback support is > needed. > > > So far there is only initial infrastructure with basic tests. > > > > > > Oleksij Rempel (6): > > > net: phy: execute genphy_loopback() per default on all PHYs > > > net: phy: genphy_loopback: add link speed configuration > > > net: add generic selftest support > > > net: fec: make use of generic NET_SELFTESTS library > > > net: ag71xx: make use of generic NET_SELFTESTS library > > > net: dsa: enable selftest support for all switches by default > > > > > > drivers/net/ethernet/atheros/Kconfig | 1 + > > > drivers/net/ethernet/atheros/ag71xx.c | 20 +- > > > drivers/net/ethernet/freescale/Kconfig | 1 + > > > drivers/net/ethernet/freescale/fec_main.c | 7 + > > > drivers/net/phy/phy.c | 3 +- > > > drivers/net/phy/phy_device.c | 35 +- > > > include/linux/phy.h | 1 + > > > include/net/dsa.h | 2 + > > > include/net/selftests.h | 12 + > > > net/Kconfig | 4 + > > > net/core/Makefile | 1 + > > > net/core/selftests.c | 400 > > > ++++++++++++++++++++++ > > > net/dsa/Kconfig | 1 + > > > net/dsa/slave.c | 21 ++ > > > 14 files changed, 500 insertions(+), 9 deletions(-) create mode > > > 100644 include/net/selftests.h create mode 100644 > > > net/core/selftests.c > > > > > > -- > > > 2.29.2 > > > > _______________________________________________ > > linux-arm-kernel mailing list > > linux-arm-kernel@lists.infradead.org > > https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Flists > > .infradead.org%2Fmailman%2Flistinfo%2Flinux-arm-kernel&data=04%7 > C0 > > > 1%7Cqiangqing.zhang%40nxp.com%7C8796bf53e46b4b1be92b08d9061186f9 > %7C686 > > > ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637547494614753358%7CU > nknown%7 > > > CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiL > CJXV > > > CI6Mn0%3D%7C1000&sdata=x%2BUFB%2B1Xp0zbR1mG5HDGvqBUvKhX > VJn337T%2BB > > D7cO6g%3D&reserved=0 > > -- > Pengutronix e.K. | > | > Steuerwalder Str. 21 | > https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww.pe > ngutronix.de%2F&data=04%7C01%7Cqiangqing.zhang%40nxp.com%7C87 > 96bf53e46b4b1be92b08d9061186f9%7C686ea1d3bc2b4c6fa92cd99c5c301635 > %7C0%7C0%7C637547494614753358%7CUnknown%7CTWFpbGZsb3d8eyJWIj > oiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C10 > 00&sdata=K2dsGVxEXv%2FtC7p0l4TFlLlaqzzTa6ktrbSdcCJ10J0%3D& > reserved=0 | > 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 > | > Amtsgericht Hildesheim, HRA 2686 | Fax: > +49-5121-206917-5555 |
Hi Joakim, On Tue, Apr 27, 2021 at 04:48:42AM +0000, Joakim Zhang wrote: > > Hi Joakim, > > > > On Fri, Apr 23, 2021 at 03:18:32AM +0000, Joakim Zhang wrote: > > > > > > 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. > > > 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". ACK > So we need a callback for MAC drivers, where we extend this callback? Could be "struct net_device_ops"? Such as ndo_set_loopback? yes. Sounds good for me. ndo_set_loopback could be implemented for ethernet controllers, DSA and even CAN. Regards, Oleksij > > > > -----Original Message----- > > > > From: Oleksij Rempel <o.rempel@pengutronix.de> > > > > Sent: 2021年4月19日 21:01 > > > > To: Shawn Guo <shawnguo@kernel.org>; Sascha Hauer > > > > <s.hauer@pengutronix.de>; Andrew Lunn <andrew@lunn.ch>; Florian > > > > Fainelli <f.fainelli@gmail.com>; Heiner Kallweit > > > > <hkallweit1@gmail.com>; Fugang Duan <fugang.duan@nxp.com> > > > > Cc: Oleksij Rempel <o.rempel@pengutronix.de>; kernel@pengutronix.de; > > > > netdev@vger.kernel.org; linux-arm-kernel@lists.infradead.org; > > > > linux-kernel@vger.kernel.org; dl-linux-imx <linux-imx@nxp.com>; > > > > Fabio Estevam <festevam@gmail.com>; David Jander > > > > <david@protonic.nl>; Russell King <linux@armlinux.org.uk>; Philippe > > > > Schenker <philippe.schenker@toradex.com> > > > > Subject: [PATCH net-next v3 0/6] provide generic net selftest > > > > support > > > > > > > > changes v3: > > > > - make more granular tests > > > > - enable loopback for all PHYs by default > > > > - fix allmodconfig build errors > > > > - poll for link status update after switching to the loopback mode > > > > > > > > changes v2: > > > > - make generic selftests available for all networking devices. > > > > - make use of net_selftest* on FEC, ag71xx and all DSA switches. > > > > - add loopback support on more PHYs. > > > > > > > > This patch set provides diagnostic capabilities for some iMX, ag71xx > > > > or any DSA based devices. For proper functionality, PHY loopback support is > > needed. > > > > So far there is only initial infrastructure with basic tests. > > > > > > > > Oleksij Rempel (6): > > > > net: phy: execute genphy_loopback() per default on all PHYs > > > > net: phy: genphy_loopback: add link speed configuration > > > > net: add generic selftest support > > > > net: fec: make use of generic NET_SELFTESTS library > > > > net: ag71xx: make use of generic NET_SELFTESTS library > > > > net: dsa: enable selftest support for all switches by default > > > > > > > > drivers/net/ethernet/atheros/Kconfig | 1 + > > > > drivers/net/ethernet/atheros/ag71xx.c | 20 +- > > > > drivers/net/ethernet/freescale/Kconfig | 1 + > > > > drivers/net/ethernet/freescale/fec_main.c | 7 + > > > > drivers/net/phy/phy.c | 3 +- > > > > drivers/net/phy/phy_device.c | 35 +- > > > > include/linux/phy.h | 1 + > > > > include/net/dsa.h | 2 + > > > > include/net/selftests.h | 12 + > > > > net/Kconfig | 4 + > > > > net/core/Makefile | 1 + > > > > net/core/selftests.c | 400 > > > > ++++++++++++++++++++++ > > > > net/dsa/Kconfig | 1 + > > > > net/dsa/slave.c | 21 ++ > > > > 14 files changed, 500 insertions(+), 9 deletions(-) create mode > > > > 100644 include/net/selftests.h create mode 100644 > > > > net/core/selftests.c > > > > > > > > -- > > > > 2.29.2 > > > > > > _______________________________________________ > > > linux-arm-kernel mailing list > > > linux-arm-kernel@lists.infradead.org > > > https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Flists > > > .infradead.org%2Fmailman%2Flistinfo%2Flinux-arm-kernel&data=04%7 > > C0 > > > > > 1%7Cqiangqing.zhang%40nxp.com%7C8796bf53e46b4b1be92b08d9061186f9 > > %7C686 > > > > > ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637547494614753358%7CU > > nknown%7 > > > > > CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiL > > CJXV > > > > > CI6Mn0%3D%7C1000&sdata=x%2BUFB%2B1Xp0zbR1mG5HDGvqBUvKhX > > VJn337T%2BB > > > D7cO6g%3D&reserved=0 > > > > -- > > Pengutronix e.K. | > > | > > Steuerwalder Str. 21 | > > https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww.pe > > ngutronix.de%2F&data=04%7C01%7Cqiangqing.zhang%40nxp.com%7C87 > > 96bf53e46b4b1be92b08d9061186f9%7C686ea1d3bc2b4c6fa92cd99c5c301635 > > %7C0%7C0%7C637547494614753358%7CUnknown%7CTWFpbGZsb3d8eyJWIj > > oiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C10 > > 00&sdata=K2dsGVxEXv%2FtC7p0l4TFlLlaqzzTa6ktrbSdcCJ10J0%3D& > > reserved=0 | > > 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 > > | > > Amtsgericht Hildesheim, HRA 2686 | Fax: > > +49-5121-206917-5555 | -- 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 |
On 4/26/2021 9:48 PM, Joakim Zhang wrote: > >> -----Original Message----- >> From: Oleksij Rempel <o.rempel@pengutronix.de> >> Sent: 2021年4月23日 12:37 >> To: Joakim Zhang <qiangqing.zhang@nxp.com> >> Cc: Shawn Guo <shawnguo@kernel.org>; Sascha Hauer >> <s.hauer@pengutronix.de>; Andrew Lunn <andrew@lunn.ch>; Florian Fainelli >> <f.fainelli@gmail.com>; Heiner Kallweit <hkallweit1@gmail.com>; Fugang >> Duan <fugang.duan@nxp.com>; kernel@pengutronix.de; >> netdev@vger.kernel.org; linux-arm-kernel@lists.infradead.org; >> linux-kernel@vger.kernel.org; dl-linux-imx <linux-imx@nxp.com>; Fabio >> Estevam <festevam@gmail.com>; David Jander <david@protonic.nl>; Russell >> King <linux@armlinux.org.uk>; Philippe Schenker >> <philippe.schenker@toradex.com> >> 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: >>> >>> 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. >>> 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
Hi Florian, > -----Original Message----- > From: Florian Fainelli <f.fainelli@gmail.com> > Sent: 2021年4月28日 0:41 > To: Joakim Zhang <qiangqing.zhang@nxp.com>; Oleksij Rempel > <o.rempel@pengutronix.de> > Cc: Shawn Guo <shawnguo@kernel.org>; Sascha Hauer > <s.hauer@pengutronix.de>; Andrew Lunn <andrew@lunn.ch>; Heiner Kallweit > <hkallweit1@gmail.com>; Fugang Duan <fugang.duan@nxp.com>; > kernel@pengutronix.de; netdev@vger.kernel.org; > linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org; > dl-linux-imx <linux-imx@nxp.com>; Fabio Estevam <festevam@gmail.com>; > David Jander <david@protonic.nl>; Russell King <linux@armlinux.org.uk>; > Philippe Schenker <philippe.schenker@toradex.com> > Subject: Re: [PATCH net-next v3 0/6] provide generic net selftest support > > > > On 4/26/2021 9:48 PM, Joakim Zhang wrote: > > > >> -----Original Message----- > >> From: Oleksij Rempel <o.rempel@pengutronix.de> > >> Sent: 2021年4月23日 12:37 > >> To: Joakim Zhang <qiangqing.zhang@nxp.com> > >> Cc: Shawn Guo <shawnguo@kernel.org>; Sascha Hauer > >> <s.hauer@pengutronix.de>; Andrew Lunn <andrew@lunn.ch>; Florian > >> Fainelli <f.fainelli@gmail.com>; Heiner Kallweit > >> <hkallweit1@gmail.com>; Fugang Duan <fugang.duan@nxp.com>; > >> kernel@pengutronix.de; netdev@vger.kernel.org; > >> linux-arm-kernel@lists.infradead.org; > >> linux-kernel@vger.kernel.org; dl-linux-imx <linux-imx@nxp.com>; Fabio > >> Estevam <festevam@gmail.com>; David Jander <david@protonic.nl>; > >> Russell King <linux@armlinux.org.uk>; Philippe Schenker > >> <philippe.schenker@toradex.com> > >> 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: > >>> > >>> 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. > >>> 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 > }; What's the difference between internal and external loopback for both PHY and MAC? I am not familiar with these concepts. Thanks. Best Regards, Joakim Zhang > 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
On Wed, Apr 28, 2021 at 08:06:05AM +0000, Joakim Zhang wrote: > > Hi Florian, > > > -----Original Message----- > > From: Florian Fainelli <f.fainelli@gmail.com> > > Sent: 2021年4月28日 0:41 > > To: Joakim Zhang <qiangqing.zhang@nxp.com>; Oleksij Rempel > > <o.rempel@pengutronix.de> > > Cc: Shawn Guo <shawnguo@kernel.org>; Sascha Hauer > > <s.hauer@pengutronix.de>; Andrew Lunn <andrew@lunn.ch>; Heiner Kallweit > > <hkallweit1@gmail.com>; Fugang Duan <fugang.duan@nxp.com>; > > kernel@pengutronix.de; netdev@vger.kernel.org; > > linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org; > > dl-linux-imx <linux-imx@nxp.com>; Fabio Estevam <festevam@gmail.com>; > > David Jander <david@protonic.nl>; Russell King <linux@armlinux.org.uk>; > > Philippe Schenker <philippe.schenker@toradex.com> > > Subject: Re: [PATCH net-next v3 0/6] provide generic net selftest support > > > > > > > > On 4/26/2021 9:48 PM, Joakim Zhang wrote: > > > > > >> -----Original Message----- > > >> From: Oleksij Rempel <o.rempel@pengutronix.de> > > >> Sent: 2021年4月23日 12:37 > > >> To: Joakim Zhang <qiangqing.zhang@nxp.com> > > >> Cc: Shawn Guo <shawnguo@kernel.org>; Sascha Hauer > > >> <s.hauer@pengutronix.de>; Andrew Lunn <andrew@lunn.ch>; Florian > > >> Fainelli <f.fainelli@gmail.com>; Heiner Kallweit > > >> <hkallweit1@gmail.com>; Fugang Duan <fugang.duan@nxp.com>; > > >> kernel@pengutronix.de; netdev@vger.kernel.org; > > >> linux-arm-kernel@lists.infradead.org; > > >> linux-kernel@vger.kernel.org; dl-linux-imx <linux-imx@nxp.com>; Fabio > > >> Estevam <festevam@gmail.com>; David Jander <david@protonic.nl>; > > >> Russell King <linux@armlinux.org.uk>; Philippe Schenker > > >> <philippe.schenker@toradex.com> > > >> 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: > > >>> > > >>> 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. > > >>> 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 > > }; > > 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 > > 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 |