The FPE support for xgmac is incomplete, drop it temporarily.
Once FPE implementation is refactored, xgmac support will be added.
Signed-off-by: Furong Xu <0x1207@gmail.com>
---
.../net/ethernet/stmicro/stmmac/dwxgmac2.h | 2 --
.../ethernet/stmicro/stmmac/dwxgmac2_core.c | 27 -------------------
2 files changed, 29 deletions(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h
index 6a2c7d22df1e..917796293c26 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h
+++ b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h
@@ -193,8 +193,6 @@
#define XGMAC_MDIO_ADDR 0x00000200
#define XGMAC_MDIO_DATA 0x00000204
#define XGMAC_MDIO_C22P 0x00000220
-#define XGMAC_FPE_CTRL_STS 0x00000280
-#define XGMAC_EFPE BIT(0)
#define XGMAC_ADDRx_HIGH(x) (0x00000300 + (x) * 0x8)
#define XGMAC_ADDR_MAX 32
#define XGMAC_AE BIT(31)
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c
index 6a987cf598e4..e5bc3957041e 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c
@@ -1505,31 +1505,6 @@ static void dwxgmac2_set_arp_offload(struct mac_device_info *hw, bool en,
writel(value, ioaddr + XGMAC_RX_CONFIG);
}
-static void dwxgmac3_fpe_configure(void __iomem *ioaddr, struct stmmac_fpe_cfg *cfg,
- u32 num_txq,
- u32 num_rxq, bool enable)
-{
- u32 value;
-
- if (!enable) {
- value = readl(ioaddr + XGMAC_FPE_CTRL_STS);
-
- value &= ~XGMAC_EFPE;
-
- writel(value, ioaddr + XGMAC_FPE_CTRL_STS);
- return;
- }
-
- value = readl(ioaddr + XGMAC_RXQ_CTRL1);
- value &= ~XGMAC_RQ;
- value |= (num_rxq - 1) << XGMAC_RQ_SHIFT;
- writel(value, ioaddr + XGMAC_RXQ_CTRL1);
-
- value = readl(ioaddr + XGMAC_FPE_CTRL_STS);
- value |= XGMAC_EFPE;
- writel(value, ioaddr + XGMAC_FPE_CTRL_STS);
-}
-
const struct stmmac_ops dwxgmac210_ops = {
.core_init = dwxgmac2_core_init,
.set_mac = dwxgmac2_set_mac,
@@ -1570,7 +1545,6 @@ const struct stmmac_ops dwxgmac210_ops = {
.config_l3_filter = dwxgmac2_config_l3_filter,
.config_l4_filter = dwxgmac2_config_l4_filter,
.set_arp_offload = dwxgmac2_set_arp_offload,
- .fpe_configure = dwxgmac3_fpe_configure,
};
static void dwxlgmac2_rx_queue_enable(struct mac_device_info *hw, u8 mode,
@@ -1627,7 +1601,6 @@ const struct stmmac_ops dwxlgmac2_ops = {
.config_l3_filter = dwxgmac2_config_l3_filter,
.config_l4_filter = dwxgmac2_config_l4_filter,
.set_arp_offload = dwxgmac2_set_arp_offload,
- .fpe_configure = dwxgmac3_fpe_configure,
};
int dwxgmac2_setup(struct stmmac_priv *priv)
--
2.34.1On Tue, Jul 09, 2024 at 04:21:19PM +0800, Furong Xu wrote: > The FPE support for xgmac is incomplete, drop it temporarily. > Once FPE implementation is refactored, xgmac support will be added. This is a pretty unusual thing to do. What does the current implementation do? Is there enough for it to actually work? If i was doing a git bisect and landed on this patch, could i find my networking is broken? More normal is to build a new implementation by the side, and then swap to it. Andrew
Hi Andrew, Furong, On Tue, Jul 09, 2024 at 03:16:35PM +0200, Andrew Lunn wrote: > On Tue, Jul 09, 2024 at 04:21:19PM +0800, Furong Xu wrote: > > The FPE support for xgmac is incomplete, drop it temporarily. > > Once FPE implementation is refactored, xgmac support will be added. > > This is a pretty unusual thing to do. What does the current > implementation do? Is there enough for it to actually work? If i was > doing a git bisect and landed on this patch, could i find my > networking is broken? > > More normal is to build a new implementation by the side, and then > swap to it. > > Andrew > There were 2 earlier attempts from Jianheng Zhang @ Synopsys to add FPE support to new hardware. I told him that the #1 priority should be to move the stmmac driver over to the new standard API which uses ethtool + tc. https://lore.kernel.org/netdev/CY5PR12MB63726FED738099761A9B81E7BF8FA@CY5PR12MB6372.namprd12.prod.outlook.com/ https://lore.kernel.org/netdev/CY5PR12MB63727C24923AE855CFF0D425BF8EA@CY5PR12MB6372.namprd12.prod.outlook.com/ I'm not sure what happened in the meantime. Jianheng must have faced some issue, because he never came back. I did comment this at the time: | Even this very patch is slightly strange - it is not brand new hardware | support, but it fills in some more FPE ops in dwxlgmac2_ops - when | dwxgmac3_fpe_configure() was already there. So this suggests the | existing support was incomplete. How complete is it now? No way to tell. | There is a selftest to tell, but we can't run it because the driver | doesn't integrate with those kernel APIs. So it is relatively known that the support is incomplete. But I still think we should push for more reviewer insight into this driver by having access to a selftest to get a clearer picture of how it behaves. For that, we need the compliance to the common API. Otherwise, I completely agree to the idea that any development should be done incrementally on top of whatever already exists, instead of putting a curtain on and then taking it back off.
Hi Vladimir On Tue, 9 Jul 2024 20:10:18 +0300, Vladimir Oltean <olteanv@gmail.com> wrote: > Hi Andrew, Furong, > > On Tue, Jul 09, 2024 at 03:16:35PM +0200, Andrew Lunn wrote: > > On Tue, Jul 09, 2024 at 04:21:19PM +0800, Furong Xu wrote: > > > The FPE support for xgmac is incomplete, drop it temporarily. > > > Once FPE implementation is refactored, xgmac support will be added. > > > > This is a pretty unusual thing to do. What does the current > > implementation do? Is there enough for it to actually work? If i was > > doing a git bisect and landed on this patch, could i find my > > networking is broken? > > > > More normal is to build a new implementation by the side, and then > > swap to it. > > > > Andrew > > > > There were 2 earlier attempts from Jianheng Zhang @ Synopsys to add FPE > support to new hardware. > > I told him that the #1 priority should be to move the stmmac driver over > to the new standard API which uses ethtool + tc. > https://lore.kernel.org/netdev/CY5PR12MB63726FED738099761A9B81E7BF8FA@CY5PR12MB6372.namprd12.prod.outlook.com/ > https://lore.kernel.org/netdev/CY5PR12MB63727C24923AE855CFF0D425BF8EA@CY5PR12MB6372.namprd12.prod.outlook.com/ > > I'm not sure what happened in the meantime. Jianheng must have faced > some issue, because he never came back. > > I did comment this at the time: > > | Even this very patch is slightly strange - it is not brand new hardware > | support, but it fills in some more FPE ops in dwxlgmac2_ops - when > | dwxgmac3_fpe_configure() was already there. So this suggests the > | existing support was incomplete. How complete is it now? No way to tell. > | There is a selftest to tell, but we can't run it because the driver > | doesn't integrate with those kernel APIs. > > So it is relatively known that the support is incomplete. But I still > think we should push for more reviewer insight into this driver by > having access to a selftest to get a clearer picture of how it behaves. > For that, we need the compliance to the common API. > After some searching and learning about your commits for FPE using the generic framework, I think it is clear enough to me to implement the new standard driver interface which uses ethtool + tc, and then the refactor of low level FPE function can follow. Thanks.
© 2016 - 2025 Red Hat, Inc.