From 969431d2b0df7e926169b211164f534be5098f8e Mon Sep 17 00:00:00 2001 From: Rosen Penev Date: Thu, 5 Sep 2024 12:49:32 -0700 Subject: [PATCH 1/7] net: ag71xx: add COMPILE_TEST to test compilation While this driver is meant for MIPS only, it can be compiled on x86 just fine. Remove pointless parentheses while at it. Enables CI building of this driver. Signed-off-by: Rosen Penev Reviewed-by: Simon Horman Link: https://patch.msgid.link/20240905194938.8453-2-rosenp@gmail.com Signed-off-by: Jakub Kicinski --- drivers/net/ethernet/atheros/Kconfig | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/net/ethernet/atheros/Kconfig b/drivers/net/ethernet/atheros/Kconfig index 482c58c4c584..bec5cdf8d1da 100644 --- a/drivers/net/ethernet/atheros/Kconfig +++ b/drivers/net/ethernet/atheros/Kconfig @@ -6,7 +6,7 @@ config NET_VENDOR_ATHEROS bool "Atheros devices" default y - depends on (PCI || ATH79) + depends on PCI || ATH79 || COMPILE_TEST help If you have a network (Ethernet) card belonging to this class, say Y. @@ -19,7 +19,7 @@ if NET_VENDOR_ATHEROS config AG71XX tristate "Atheros AR7XXX/AR9XXX built-in ethernet mac support" - depends on ATH79 + depends on ATH79 || COMPILE_TEST select PHYLINK imply NET_SELFTESTS help From 7c3736a12938112a4d1c007156272b7036ce4981 Mon Sep 17 00:00:00 2001 From: Rosen Penev Date: Thu, 5 Sep 2024 12:49:33 -0700 Subject: [PATCH 2/7] net: ag71xx: add MODULE_DESCRIPTION Now that COMPILE_TEST is enabled, it gets flagged when building with allmodconfig W=1 builds. Text taken from the beginning of the file. Signed-off-by: Rosen Penev Reviewed-by: Simon Horman Link: https://patch.msgid.link/20240905194938.8453-3-rosenp@gmail.com Signed-off-by: Jakub Kicinski --- drivers/net/ethernet/atheros/ag71xx.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/net/ethernet/atheros/ag71xx.c b/drivers/net/ethernet/atheros/ag71xx.c index db2a8ade6205..e7da70f14f06 100644 --- a/drivers/net/ethernet/atheros/ag71xx.c +++ b/drivers/net/ethernet/atheros/ag71xx.c @@ -2033,4 +2033,5 @@ static struct platform_driver ag71xx_driver = { }; module_platform_driver(ag71xx_driver); +MODULE_DESCRIPTION("Atheros AR71xx built-in ethernet mac driver"); MODULE_LICENSE("GPL v2"); From 28540850577b269a881f5f7b579ba93ea670abb5 Mon Sep 17 00:00:00 2001 From: Rosen Penev Date: Thu, 5 Sep 2024 12:49:34 -0700 Subject: [PATCH 3/7] net: ag71xx: update FIFO bits and descriptions Taken from QCA SDK. No functional difference as same bits get applied. Signed-off-by: Rosen Penev Reviewed-by: Oleksij Rempel Link: https://patch.msgid.link/20240905194938.8453-4-rosenp@gmail.com Signed-off-by: Jakub Kicinski --- drivers/net/ethernet/atheros/ag71xx.c | 48 +++++++++++++-------------- 1 file changed, 24 insertions(+), 24 deletions(-) diff --git a/drivers/net/ethernet/atheros/ag71xx.c b/drivers/net/ethernet/atheros/ag71xx.c index e7da70f14f06..74e01ce6161f 100644 --- a/drivers/net/ethernet/atheros/ag71xx.c +++ b/drivers/net/ethernet/atheros/ag71xx.c @@ -149,11 +149,11 @@ #define FIFO_CFG4_MC BIT(8) /* Multicast Packet */ #define FIFO_CFG4_BC BIT(9) /* Broadcast Packet */ #define FIFO_CFG4_DR BIT(10) /* Dribble */ -#define FIFO_CFG4_LE BIT(11) /* Long Event */ -#define FIFO_CFG4_CF BIT(12) /* Control Frame */ -#define FIFO_CFG4_PF BIT(13) /* Pause Frame */ -#define FIFO_CFG4_UO BIT(14) /* Unsupported Opcode */ -#define FIFO_CFG4_VT BIT(15) /* VLAN tag detected */ +#define FIFO_CFG4_CF BIT(11) /* Control Frame */ +#define FIFO_CFG4_PF BIT(12) /* Pause Frame */ +#define FIFO_CFG4_UO BIT(13) /* Unsupported Opcode */ +#define FIFO_CFG4_VT BIT(14) /* VLAN tag detected */ +#define FIFO_CFG4_LE BIT(15) /* Long Event */ #define FIFO_CFG4_FT BIT(16) /* Frame Truncated */ #define FIFO_CFG4_UC BIT(17) /* Unicast Packet */ #define FIFO_CFG4_INIT (FIFO_CFG4_DE | FIFO_CFG4_DV | FIFO_CFG4_FC | \ @@ -168,28 +168,28 @@ #define FIFO_CFG5_DV BIT(1) /* RX_DV Event */ #define FIFO_CFG5_FC BIT(2) /* False Carrier */ #define FIFO_CFG5_CE BIT(3) /* Code Error */ -#define FIFO_CFG5_LM BIT(4) /* Length Mismatch */ -#define FIFO_CFG5_LO BIT(5) /* Length Out of Range */ -#define FIFO_CFG5_OK BIT(6) /* Packet is OK */ -#define FIFO_CFG5_MC BIT(7) /* Multicast Packet */ -#define FIFO_CFG5_BC BIT(8) /* Broadcast Packet */ -#define FIFO_CFG5_DR BIT(9) /* Dribble */ -#define FIFO_CFG5_CF BIT(10) /* Control Frame */ -#define FIFO_CFG5_PF BIT(11) /* Pause Frame */ -#define FIFO_CFG5_UO BIT(12) /* Unsupported Opcode */ -#define FIFO_CFG5_VT BIT(13) /* VLAN tag detected */ -#define FIFO_CFG5_LE BIT(14) /* Long Event */ -#define FIFO_CFG5_FT BIT(15) /* Frame Truncated */ -#define FIFO_CFG5_16 BIT(16) /* unknown */ -#define FIFO_CFG5_17 BIT(17) /* unknown */ +#define FIFO_CFG5_CR BIT(4) /* CRC error */ +#define FIFO_CFG5_LM BIT(5) /* Length Mismatch */ +#define FIFO_CFG5_LO BIT(6) /* Length Out of Range */ +#define FIFO_CFG5_OK BIT(7) /* Packet is OK */ +#define FIFO_CFG5_MC BIT(8) /* Multicast Packet */ +#define FIFO_CFG5_BC BIT(9) /* Broadcast Packet */ +#define FIFO_CFG5_DR BIT(10) /* Dribble */ +#define FIFO_CFG5_CF BIT(11) /* Control Frame */ +#define FIFO_CFG5_PF BIT(12) /* Pause Frame */ +#define FIFO_CFG5_UO BIT(13) /* Unsupported Opcode */ +#define FIFO_CFG5_VT BIT(14) /* VLAN tag detected */ +#define FIFO_CFG5_LE BIT(15) /* Long Event */ +#define FIFO_CFG5_FT BIT(16) /* Frame Truncated */ +#define FIFO_CFG5_UC BIT(17) /* Unicast Packet */ #define FIFO_CFG5_SF BIT(18) /* Short Frame */ #define FIFO_CFG5_BM BIT(19) /* Byte Mode */ #define FIFO_CFG5_INIT (FIFO_CFG5_DE | FIFO_CFG5_DV | FIFO_CFG5_FC | \ - FIFO_CFG5_CE | FIFO_CFG5_LO | FIFO_CFG5_OK | \ - FIFO_CFG5_MC | FIFO_CFG5_BC | FIFO_CFG5_DR | \ - FIFO_CFG5_CF | FIFO_CFG5_PF | FIFO_CFG5_VT | \ - FIFO_CFG5_LE | FIFO_CFG5_FT | FIFO_CFG5_16 | \ - FIFO_CFG5_17 | FIFO_CFG5_SF) + FIFO_CFG5_CE | FIFO_CFG5_LM | FIFO_CFG5_LO | \ + FIFO_CFG5_OK | FIFO_CFG5_MC | FIFO_CFG5_BC | \ + FIFO_CFG5_DR | FIFO_CFG5_CF | FIFO_CFG5_UO | \ + FIFO_CFG5_VT | FIFO_CFG5_LE | FIFO_CFG5_FT | \ + FIFO_CFG5_UC | FIFO_CFG5_SF) #define AG71XX_REG_TX_CTRL 0x0180 #define TX_CTRL_TXE BIT(0) /* Tx Enable */ From 441a2798623cc3eefd936b33a9cf8d1973b6688b Mon Sep 17 00:00:00 2001 From: Rosen Penev Date: Thu, 5 Sep 2024 12:49:35 -0700 Subject: [PATCH 4/7] net: ag71xx: use ethtool_puts Allows simplifying get_strings and avoids manual pointer manipulation. Signed-off-by: Rosen Penev Reviewed-by: Oleksij Rempel Link: https://patch.msgid.link/20240905194938.8453-5-rosenp@gmail.com Signed-off-by: Jakub Kicinski --- drivers/net/ethernet/atheros/ag71xx.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/net/ethernet/atheros/ag71xx.c b/drivers/net/ethernet/atheros/ag71xx.c index 74e01ce6161f..35db6912e845 100644 --- a/drivers/net/ethernet/atheros/ag71xx.c +++ b/drivers/net/ethernet/atheros/ag71xx.c @@ -509,8 +509,7 @@ static void ag71xx_ethtool_get_strings(struct net_device *netdev, u32 sset, switch (sset) { case ETH_SS_STATS: for (i = 0; i < ARRAY_SIZE(ag71xx_statistics); i++) - memcpy(data + i * ETH_GSTRING_LEN, - ag71xx_statistics[i].name, ETH_GSTRING_LEN); + ethtool_puts(&data, ag71xx_statistics[i].name); break; case ETH_SS_TEST: net_selftest_get_strings(data); From bfff5d8e211189396c8f9841d0b027f13452b162 Mon Sep 17 00:00:00 2001 From: Rosen Penev Date: Thu, 5 Sep 2024 12:49:36 -0700 Subject: [PATCH 5/7] net: ag71xx: get reset control using devm api Currently, the of variant is missing reset_control_put in error paths. The devm variant does not require it. Allows removing mdio_reset from the struct as it is not used outside the function. Signed-off-by: Rosen Penev Reviewed-by: Oleksij Rempel Link: https://patch.msgid.link/20240905194938.8453-6-rosenp@gmail.com Signed-off-by: Jakub Kicinski --- drivers/net/ethernet/atheros/ag71xx.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/drivers/net/ethernet/atheros/ag71xx.c b/drivers/net/ethernet/atheros/ag71xx.c index 35db6912e845..a32a72fa4179 100644 --- a/drivers/net/ethernet/atheros/ag71xx.c +++ b/drivers/net/ethernet/atheros/ag71xx.c @@ -379,7 +379,6 @@ struct ag71xx { u32 fifodata[3]; int mac_idx; - struct reset_control *mdio_reset; struct clk *clk_mdio; }; @@ -689,6 +688,7 @@ static int ag71xx_mdio_probe(struct ag71xx *ag) { struct device *dev = &ag->pdev->dev; struct net_device *ndev = ag->ndev; + struct reset_control *mdio_reset; static struct mii_bus *mii_bus; struct device_node *np, *mnp; int err; @@ -705,10 +705,10 @@ static int ag71xx_mdio_probe(struct ag71xx *ag) if (!mii_bus) return -ENOMEM; - ag->mdio_reset = of_reset_control_get_exclusive(np, "mdio"); - if (IS_ERR(ag->mdio_reset)) { + mdio_reset = devm_reset_control_get_exclusive(dev, "mdio"); + if (IS_ERR(mdio_reset)) { netif_err(ag, probe, ndev, "Failed to get reset mdio.\n"); - return PTR_ERR(ag->mdio_reset); + return PTR_ERR(mdio_reset); } mii_bus->name = "ag71xx_mdio"; @@ -719,10 +719,10 @@ static int ag71xx_mdio_probe(struct ag71xx *ag) mii_bus->parent = dev; snprintf(mii_bus->id, MII_BUS_ID_SIZE, "%s.%d", np->name, ag->mac_idx); - if (!IS_ERR(ag->mdio_reset)) { - reset_control_assert(ag->mdio_reset); + if (!IS_ERR(mdio_reset)) { + reset_control_assert(mdio_reset); msleep(100); - reset_control_deassert(ag->mdio_reset); + reset_control_deassert(mdio_reset); msleep(200); } From 40f111cc6e1b35562e815170d8f7efb49bf0fb21 Mon Sep 17 00:00:00 2001 From: Rosen Penev Date: Thu, 5 Sep 2024 12:49:37 -0700 Subject: [PATCH 6/7] net: ag71xx: remove always true branch The opposite of this condition is checked above and if true, function returns. Which means this can never be false. Signed-off-by: Rosen Penev Reviewed-by: Oleksij Rempel Link: https://patch.msgid.link/20240905194938.8453-7-rosenp@gmail.com Signed-off-by: Jakub Kicinski --- drivers/net/ethernet/atheros/ag71xx.c | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/drivers/net/ethernet/atheros/ag71xx.c b/drivers/net/ethernet/atheros/ag71xx.c index a32a72fa4179..e28a4b018b11 100644 --- a/drivers/net/ethernet/atheros/ag71xx.c +++ b/drivers/net/ethernet/atheros/ag71xx.c @@ -719,12 +719,10 @@ static int ag71xx_mdio_probe(struct ag71xx *ag) mii_bus->parent = dev; snprintf(mii_bus->id, MII_BUS_ID_SIZE, "%s.%d", np->name, ag->mac_idx); - if (!IS_ERR(mdio_reset)) { - reset_control_assert(mdio_reset); - msleep(100); - reset_control_deassert(mdio_reset); - msleep(200); - } + reset_control_assert(mdio_reset); + msleep(100); + reset_control_deassert(mdio_reset); + msleep(200); mnp = of_get_child_by_name(np, "mdio"); err = devm_of_mdiobus_register(dev, mii_bus, mnp); From 8410adf2e38ad32f20edf6d90e049a8203f51d9e Mon Sep 17 00:00:00 2001 From: Sven Eckelmann Date: Thu, 5 Sep 2024 12:49:38 -0700 Subject: [PATCH 7/7] net: ag71xx: disable napi interrupts during probe ag71xx_probe is registering ag71xx_interrupt as handler for gmac0/gmac1 interrupts. The handler is trying to use napi_schedule to handle the processing of packets. But the netif_napi_add for this device is called a lot later in ag71xx_probe. It can therefore happen that a still running gmac0/gmac1 is triggering the interrupt handler with a bit from AG71XX_INT_POLL set in AG71XX_REG_INT_STATUS. The handler will then call napi_schedule and the napi code will crash the system because the ag->napi is not yet initialized. The gmcc0/gmac1 must be brought in a state in which it doesn't signal a AG71XX_INT_POLL related status bits as interrupt before registering the interrupt handler. ag71xx_hw_start will take care of re-initializing the AG71XX_REG_INT_ENABLE. This will become relevant when dual GMAC devices get added here. Signed-off-by: Sven Eckelmann Signed-off-by: Rosen Penev Link: https://patch.msgid.link/20240905194938.8453-8-rosenp@gmail.com Signed-off-by: Jakub Kicinski --- drivers/net/ethernet/atheros/ag71xx.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/drivers/net/ethernet/atheros/ag71xx.c b/drivers/net/ethernet/atheros/ag71xx.c index e28a4b018b11..96a6189cc31e 100644 --- a/drivers/net/ethernet/atheros/ag71xx.c +++ b/drivers/net/ethernet/atheros/ag71xx.c @@ -1850,6 +1850,12 @@ static int ag71xx_probe(struct platform_device *pdev) if (!ag->mac_base) return -ENOMEM; + /* ensure that HW is in manual polling mode before interrupts are + * activated. Otherwise ag71xx_interrupt might call napi_schedule + * before it is initialized by netif_napi_add. + */ + ag71xx_int_disable(ag, AG71XX_INT_POLL); + ndev->irq = platform_get_irq(pdev, 0); err = devm_request_irq(&pdev->dev, ndev->irq, ag71xx_interrupt, 0x0, dev_name(&pdev->dev), ndev);