On 4/15/2021 6:07 AM, Oleksij Rempel wrote:
> Port some parts of the stmmac selftest and reuse it as basic generic selftest
> library. This patch was tested with following combinations:
> - iMX6DL FEC -> AT8035
> - iMX6DL FEC -> SJA1105Q switch -> KSZ8081
> - iMX6DL FEC -> SJA1105Q switch -> KSZ9031
> - AR9331 ag71xx -> AR9331 PHY
> - AR9331 ag71xx -> AR9331 switch -> AR9331 PHY
>
> Signed-off-by: Oleksij Rempel <o.rem...@pengutronix.de>
> ---
[snip]
> +
> +struct net_packet_attrs {
> + unsigned char *src;
> + unsigned char *dst;
> + u32 ip_src;
> + u32 ip_dst;
> + int tcp;
This can be an u8 and named proto maybe?
> + int sport;
> + int dport;
These two can be u16
> + int timeout;
> + int size;
> + int max_size;
> + u8 id;
> + u16 queue_mapping;
> +};
[snip]
> +static const struct net_test {
> + char name[ETH_GSTRING_LEN];
> + int (*fn)(struct net_device *ndev);
> +} net_selftests[] = {
> + {
> + .name = "PHY Loopback, UDP ",
This should be "PHY internal loopback, UDP"
> + .fn = net_test_phy_loopback_udp,
> + }, {
> + .name = "PHY Loopback, TCP ",
> + .fn = net_test_phy_loopback_tcp,
and "PHY internal loopback, TCP"
to make it clear that the loopback is internal, as opposed to external.
Or if you prefer to use the line-side or MAC-side that works too.
> + },
> +};
> +
> +void net_selftest(struct net_device *ndev, struct ethtool_test *etest, u64
> *buf)
> +{
> + int count = net_selftest_get_count();
> + int i;
> +
> + memset(buf, 0, sizeof(*buf) * count);
> + net_test_next_id = 0;
> +
> + if (etest->flags != ETH_TEST_FL_OFFLINE) {
> + netdev_err(ndev, "Only offline tests are supported\n");
> + etest->flags |= ETH_TEST_FL_FAILED;
> + return;
> + } else if (!netif_carrier_ok(ndev)) {
> + netdev_err(ndev, "You need valid Link to execute tests\n");
> + etest->flags |= ETH_TEST_FL_FAILED;
> + return;
> + }
> +
> + if (!ndev->phydev)
> + return;
Can you move that as the first test and return -EOPNOTSUPP instead?
> +
> + /* PHY loopback tests should be combined to avoid delays on each PHY
> + * reconfiguration
> + */
> + phy_loopback(ndev->phydev, true);
> +
> + /* give PHYs some time to establish the loopback link */
> + msleep(100);
Cannot you poll for LSTATUS instead?
> +
> + for (i = 0; i < count; i++) {
> + buf[i] = net_selftests[i].fn(ndev);
> + if (buf[i] && (buf[i] != -EOPNOTSUPP))
> + etest->flags |= ETH_TEST_FL_FAILED;
> + }
> +
> + phy_loopback(ndev->phydev, false);
Can you propagate the return value here?
As spotted by the test robot please export all of these symbols as
EXPORT_SYMBOL_GPL().
--
Florian
