net: stmmac: Fix Rx COE re-enabling after interface re-uping

If NETIF_F_RXCSUM feature is user-disabled and the network interface is
cycled down/up, then the feature will be completely off and couldn't be
re-enabled until the driver or system is reloaded. This happens due to the
way the runtime-check has been implemented in the stmmac_hw_setup()
method.

See if the stmmac_rx_ipc() method couldn't enable the Checksum Offload
engine (which happens for sure if the feature was disabled by the user),
then driver assumes that no Rx COE detected and clears out all the flags
(mac_device_info::rx_csum and plat_stmmacenet_data::rx_coe) indicating the
engine availability. As such the user won't be able to enable feature
anymore since the ndo_set_feature() callback will constantly call disable
Rx COE (call stmmac_rx_ipc() with mac_device_info::rx_csum cleared)
despite of the NETIF_F_RXCSUM flag state.

Let's fix that by re-developing the RXCSUM feature support a bit. First
let's runtime check the RxCOE availability only once during the net-device
registration. It can be done because the run-time check on each
device-open has turned to be pointless since the blamed commit if the
feature isn't declared to be available. Second let's drop the
mac_device_info::rx_csum and always use the NETIF_F_RXCSUM flag state to
determined the Rx COE feature state since the driver won't assume anymore
that the feature might be somehow failed to be enabled.

Fixes: d2afb5bdffde ("stmmac: fix the rx csum feature")
Signed-off-by: Serge Semin <fancer.lancer@gmail.com>

---

Note the problem could be fixed in a less invasive way but the suggested
solution makes the RXCSUM feature implementation looking cleaner and more
maintainable.

8 files changed, 26 insertions, 42 deletions

diff --git a/drivers/net/ethernet/stmicro/stmmac/common.h b/drivers/net/ethernet/stmicro/stmmac/common.h
index 6a87b8c6218b..02df8523111a 100644
--- a/drivers/net/ethernet/stmicro/stmmac/common.h
+++ b/drivers/net/ethernet/stmicro/stmmac/common.h
@@ -646,7 +646,6 @@ struct mac_device_info {
 	unsigned int multicast_filter_bins;
 	unsigned int unicast_filter_entries;
 	unsigned int mcast_bits_log2;
-	unsigned int rx_csum;
 	unsigned int pcs;
 	unsigned int xlgmac;
 	unsigned int num_vlan;
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
index e2d467582698..4e8295492098 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
@@ -663,14 +663,14 @@ static void sun8i_dwmac_get_umac_addr(struct mac_device_info *hw,
 }
 
 /* caution this function must return non 0 to work */
-static int sun8i_dwmac_rx_ipc_enable(struct mac_device_info *hw)
+static int sun8i_dwmac_rx_ipc_enable(struct mac_device_info *hw, bool enable)
 {
 	void __iomem *ioaddr = hw->pcsr;
 	u32 v;
 
 	v = readl(ioaddr + EMAC_RX_CTL0);
 
-	if (hw->rx_csum)
+	if (enable)
 		v |= EMAC_RX_DO_CRC;
 	else
 		v &= ~EMAC_RX_DO_CRC;
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c b/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c
index ba24be32220e..83ec2c00b49a 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c
@@ -72,12 +72,12 @@ static void dwmac1000_irq_modify(struct mac_device_info *hw, u32 disable,
 	spin_unlock_irqrestore(&hw->irq_ctrl_lock, flags);
 }
 
-static int dwmac1000_rx_ipc_enable(struct mac_device_info *hw)
+static int dwmac1000_rx_ipc_enable(struct mac_device_info *hw, bool enable)
 {
 	void __iomem *ioaddr = hw->pcsr;
 	u32 value = readl(ioaddr + GMAC_CONTROL);
 
-	if (hw->rx_csum)
+	if (enable)
 		value |= GMAC_CONTROL_IPC;
 	else
 		value &= ~GMAC_CONTROL_IPC;
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac100_core.c b/drivers/net/ethernet/stmicro/stmmac/dwmac100_core.c
index db4fbe64a38a..9c3e1ab992dc 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac100_core.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac100_core.c
@@ -48,11 +48,6 @@ static void dwmac100_dump_mac_regs(struct mac_device_info *hw, u32 *reg_space)
 	reg_space[MAC_VLAN2 / 4] = readl(ioaddr + MAC_VLAN2);
 }
 
-static int dwmac100_rx_ipc_enable(struct mac_device_info *hw)
-{
-	return 0;
-}
-
 static int dwmac100_irq_status(struct stmmac_priv *priv,
 			       struct stmmac_extra_stats *x)
 {
@@ -157,7 +152,6 @@ static void dwmac100_set_mac_loopback(void __iomem *ioaddr, bool enable)
 const struct stmmac_ops dwmac100_ops = {
 	.core_init = dwmac100_core_init,
 	.set_mac = stmmac_set_mac,
-	.rx_ipc = dwmac100_rx_ipc_enable,
 	.dump_regs = dwmac100_dump_mac_regs,
 	.host_irq_status = dwmac100_irq_status,
 	.set_filter = dwmac100_set_filter,
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c b/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c
index 429a344a7896..5337376452a7 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c
@@ -318,12 +318,12 @@ static void dwmac4_dump_regs(struct mac_device_info *hw, u32 *reg_space)
 		reg_space[i] = readl(ioaddr + i * 4);
 }
 
-static int dwmac4_rx_ipc_enable(struct mac_device_info *hw)
+static int dwmac4_rx_ipc_enable(struct mac_device_info *hw, bool enable)
 {
 	void __iomem *ioaddr = hw->pcsr;
 	u32 value = readl(ioaddr + GMAC_CONFIG);
 
-	if (hw->rx_csum)
+	if (enable)
 		value |= GMAC_CONFIG_IPC;
 	else
 		value &= ~GMAC_CONFIG_IPC;
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c
index 57b0f35e0b78..8be75536b284 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c
@@ -65,13 +65,13 @@ static void dwxgmac2_set_mac(void __iomem *ioaddr, bool enable)
 	writel(rx, ioaddr + XGMAC_RX_CONFIG);
 }
 
-static int dwxgmac2_rx_ipc(struct mac_device_info *hw)
+static int dwxgmac2_rx_ipc(struct mac_device_info *hw, bool enable)
 {
 	void __iomem *ioaddr = hw->pcsr;
 	u32 value;
 
 	value = readl(ioaddr + XGMAC_RX_CONFIG);
-	if (hw->rx_csum)
+	if (enable)
 		value |= XGMAC_CONFIG_IPC;
 	else
 		value &= ~XGMAC_CONFIG_IPC;
diff --git a/drivers/net/ethernet/stmicro/stmmac/hwif.h b/drivers/net/ethernet/stmicro/stmmac/hwif.h
index 8ee2f3fbef23..c15fa1cafdea 100644
--- a/drivers/net/ethernet/stmicro/stmmac/hwif.h
+++ b/drivers/net/ethernet/stmicro/stmmac/hwif.h
@@ -326,7 +326,7 @@ struct stmmac_ops {
 	/* Enable the MAC RX/TX */
 	void (*set_mac)(void __iomem *ioaddr, bool enable);
 	/* Enable and verify that the IPC module is supported */
-	int (*rx_ipc)(struct mac_device_info *hw);
+	int (*rx_ipc)(struct mac_device_info *hw, bool enable);
 	/* Enable RX Queues */
 	void (*rx_queue_enable)(struct mac_device_info *hw, u8 mode, u32 queue);
 	/* RX Queues Priority */
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 91aa8e9187ce..8235401a8e3b 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -3716,12 +3716,8 @@ static int stmmac_hw_setup(struct net_device *dev)
 	/* Initialize Safety Features */
 	stmmac_safety_feat_configuration(priv);
 
-	ret = stmmac_rx_ipc(priv, priv->hw);
-	if (!ret) {
-		netdev_warn(priv->dev, "RX IPC Checksum Offload disabled\n");
-		priv->plat->rx_coe = 0;
-		priv->hw->rx_csum = 0;
-	}
+	/* Initialize Rx Checksum Offload Engine */
+	stmmac_rx_ipc(priv, priv->hw, dev->features & NETIF_F_RXCSUM);
 
 	/* Enable the MAC Rx/Tx */
 	stmmac_mac_set(priv, priv->ioaddr, true);
@@ -5365,9 +5361,9 @@ static void stmmac_dispatch_skb_zc(struct stmmac_priv *priv, u32 queue,
 {
 	struct stmmac_rxq_stats *rxq_stats = &priv->xstats.rxq_stats[queue];
 	struct stmmac_channel *ch = &priv->channel[queue];
+	int coe = priv->dev->features & NETIF_F_RXCSUM;
 	unsigned int len = xdp->data_end - xdp->data;
 	enum pkt_hash_types hash_type;
-	int coe = priv->hw->rx_csum;
 	struct sk_buff *skb;
 	u32 hash;
 
@@ -5666,14 +5662,14 @@ static int stmmac_rx(struct stmmac_priv *priv, int limit, u32 queue)
 	struct stmmac_rxq_stats *rxq_stats = &priv->xstats.rxq_stats[queue];
 	struct stmmac_rx_queue *rx_q = &priv->dma_conf.rx_queue[queue];
 	struct stmmac_channel *ch = &priv->channel[queue];
+	int coe = priv->dev->features & NETIF_F_RXCSUM;
 	unsigned int count = 0, error = 0, len = 0;
-	int status = 0, coe = priv->hw->rx_csum;
 	unsigned int next_entry = rx_q->cur_rx;
 	enum dma_data_direction dma_dir;
+	int status = 0, xdp_status = 0;
 	unsigned int desc_size;
 	struct sk_buff *skb = NULL;
 	struct stmmac_xdp_buff ctx;
-	int xdp_status = 0;
 
 	dma_dir = page_pool_get_dma_dir(rx_q->page_pool);
 	limit = min(priv->dma_conf.dma_rx_size - 1, (unsigned int)limit);
@@ -6160,9 +6156,6 @@ static netdev_features_t stmmac_fix_features(struct net_device *dev,
 {
 	struct stmmac_priv *priv = netdev_priv(dev);
 
-	if (!priv->plat->rx_coe)
-		features &= ~NETIF_F_RXCSUM;
-
 	if (!priv->plat->tx_coe)
 		features &= ~NETIF_F_CSUM_MASK;
 
@@ -6191,15 +6184,7 @@ static int stmmac_set_features(struct net_device *netdev,
 	struct stmmac_priv *priv = netdev_priv(netdev);
 	u32 chan;
 
-	/* Keep the COE Type in case of csum is supporting */
-	if (features & NETIF_F_RXCSUM)
-		priv->hw->rx_csum = priv->plat->rx_coe;
-	else
-		priv->hw->rx_csum = 0;
-	/* No check needed because rx_coe has been set before and it will be
-	 * fixed in case of issue.
-	 */
-	stmmac_rx_ipc(priv, priv->hw);
+	stmmac_rx_ipc(priv, priv->hw, features & NETIF_F_RXCSUM);
 
 	for (chan = 0; chan < priv->plat->tx_queues_to_use; chan++) {
 		struct stmmac_tx_queue *tx_q = &priv->dma_conf.tx_queue[chan];
@@ -7785,17 +7770,20 @@ static int stmmac_hw_init(struct stmmac_priv *priv)
 		else
 			priv->plat->tx_coe = priv->dma_cap.tx_coe;
 
-		priv->plat->rx_coe = priv->dma_cap.rx_coe || priv->dma_cap.rx_coe_type2;
+		/* RXCOE runtime check (for backward compatibility) */
+		if (priv->dma_cap.rx_coe || priv->dma_cap.rx_coe_type2) {
+			priv->plat->rx_coe =
+				(stmmac_rx_ipc(priv, priv->hw, true) == 1);
+		}
 
 		stmmac_print_actphyif(priv);
 	} else {
 		dev_info(priv->device, "No HW DMA feature register supported\n");
 	}
 
-	if (priv->plat->rx_coe) {
-		priv->hw->rx_csum = priv->plat->rx_coe;
+	if (priv->plat->rx_coe)
 		dev_info(priv->device, "RX Checksum Offload Engine supported\n");
-	}
+
 	if (priv->plat->tx_coe)
 		dev_info(priv->device, "TX Checksum insertion supported\n");
 
@@ -8248,10 +8236,13 @@ static int __stmmac_dvr_probe(struct device *device,
 	ndev->xdp_metadata_ops = &stmmac_xdp_metadata_ops;
 	ndev->xsk_tx_metadata_ops = &stmmac_xsk_tx_metadata_ops;
 
-	ndev->hw_features = NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM | NETIF_F_RXCSUM;
+	ndev->hw_features = NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM;
 	ndev->xdp_features = NETDEV_XDP_ACT_BASIC | NETDEV_XDP_ACT_REDIRECT |
 			     NETDEV_XDP_ACT_XSK_ZEROCOPY;
 
+	if (priv->plat->rx_coe)
+		ndev->hw_features |= NETIF_F_RXCSUM;
+
 	if (priv->dma_cap.frpsel || priv->dma_cap.av || priv->dma_cap.l3l4fnum ||
 	    priv->dma_cap.tbssel || priv->dma_cap.estsel) {
 		ret = stmmac_tc_init(priv, priv);