From rmk Mon Aug 15 18:57:31 2016 References: Message-ID: From: Russell King To: Linux Kernel Mailing list Date: Mon, 15 Aug 2016 18:57:31 +0100 Content-type: text/plain; charset="utf-8" Content-Disposition: inline Content-Transfer-Encoding: 8bit Mime-Version: 1.0 Subject: Re: [PATCH] net: fec: fix receive error handling From: Russell King The FEC receive error handling suffers from several problems: 1. When a FIFO overrun error occurs, the descriptor is closed and reception stops. The remainder of the packet is discarded by the MAC. The documentation states that other status bits are invalid, and they will be zero. However, practical experience on iMX6 shows this is not the case - the CR (crc error) bit will also be set. This leads to each FIFO overrun event incrementing both the fifo error count and the crc error count, which makes the error statistics less useful. Fix this by ignoring all other status bits of the FIFO overrun is set, and add a comment to that effect. 2. A late collision invalidates all but the overrun condition; the remaining error conditions must be ignored. 3. Despite accounting for errors, it continues to receive the errored packets and pass them into the network stack as if they were correctly received. Signed-off-by: Russell King --- drivers/net/ethernet/freescale/fec.h | 1 + drivers/net/ethernet/freescale/fec_main.c | 47 ++++++++++++++++++------------- 2 files changed, 28 insertions(+), 20 deletions(-) diff --git a/drivers/net/ethernet/freescale/fec.h b/drivers/net/ethernet/freescale/fec.h index ee41d98b44b6..ec4c76264eb9 100644 --- a/drivers/net/ethernet/freescale/fec.h +++ b/drivers/net/ethernet/freescale/fec.h @@ -202,6 +202,7 @@ struct bufdesc_ex { #define BD_ENET_RX_OV ((ushort)0x0002) #define BD_ENET_RX_CL ((ushort)0x0001) #define BD_ENET_RX_STATS ((ushort)0x013f) /* All status bits */ +#define BD_ENET_RX_ERROR ((ushort)0x003f) /* Enhanced buffer descriptor control/status used by Ethernet receive */ #define BD_ENET_RX_VLAN 0x00000004 diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c index 89355a719625..fd49bd70d15d 100644 --- a/drivers/net/ethernet/freescale/fec_main.c +++ b/drivers/net/ethernet/freescale/fec_main.c @@ -1218,28 +1218,35 @@ fec_enet_rx(struct net_device *ndev, int budget) writel(FEC_ENET_RXF, fep->hwp + FEC_IEVENT); /* Check for errors. */ - if (status & (BD_ENET_RX_LG | BD_ENET_RX_SH | BD_ENET_RX_NO | - BD_ENET_RX_CR | BD_ENET_RX_OV)) { + if (status & BD_ENET_RX_ERROR) { ndev->stats.rx_errors++; - if (status & (BD_ENET_RX_LG | BD_ENET_RX_SH)) { - /* Frame too long or too short. */ - ndev->stats.rx_length_errors++; - } - if (status & BD_ENET_RX_NO) /* Frame alignment */ - ndev->stats.rx_frame_errors++; - if (status & BD_ENET_RX_CR) /* CRC Error */ - ndev->stats.rx_crc_errors++; - if (status & BD_ENET_RX_OV) /* FIFO overrun */ - ndev->stats.rx_fifo_errors++; - } - /* Report late collisions as a frame error. - * On this error, the BD is closed, but we don't know what we - * have in the buffer. So, just drop this frame on the floor. - */ - if (status & BD_ENET_RX_CL) { - ndev->stats.rx_errors++; - ndev->stats.rx_frame_errors++; + if (status & BD_ENET_RX_OV) { + /* + * FIFO overrun - stored frame and the other + * status (M, LG, NO, CR, CL) are invalid. + * Although docs say these status bits will + * be zero, it has been observed on iMX6 FEC + * that OV is always accompanied by CR set. + */ + ndev->stats.rx_fifo_errors++; + } else if (status & BD_ENET_RX_CL) { + /* + * Report late collisions as a frame error. + * On this error, the BD is closed, but we + * don't know what we have in the buffer. + * So, just drop this frame on the floor. + */ + ndev->stats.rx_frame_errors++; + } else { + if (status & (BD_ENET_RX_LG | BD_ENET_RX_SH)) + /* Frame too long or too short. */ + ndev->stats.rx_length_errors++; + if (status & BD_ENET_RX_NO) /* Frame alignment */ + ndev->stats.rx_frame_errors++; + if (status & BD_ENET_RX_CR) /* CRC Error */ + ndev->stats.rx_crc_errors++; + } goto rx_processing_done; } From rmk Mon Aug 15 18:57:31 2016 References: Message-ID: From: Russell King To: Linux Kernel Mailing list Date: Mon, 15 Aug 2016 18:57:31 +0100 Content-type: text/plain; charset="utf-8" Content-Disposition: inline Content-Transfer-Encoding: 8bit Mime-Version: 1.0 Subject: Re: [PATCH] net: fec: ensure that we dma unmap the transmit descriptors From: Russell King On transmit timeout or close, dirty transmit descriptors were not being correctly cleaned: the only time that DMA mappings are cleaned is when scanning the TX ring after a transmit interrupt. Fix this. Signed-off-by: Russell King --- drivers/net/ethernet/freescale/fec_main.c | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c index fd49bd70d15d..87120522ba66 100644 --- a/drivers/net/ethernet/freescale/fec_main.c +++ b/drivers/net/ethernet/freescale/fec_main.c @@ -364,6 +364,15 @@ fec_enet_clear_csum(struct sk_buff *skb, struct net_device *ndev) return 0; } +static void +fec_enet_tx_unmap(struct bufdesc *bdp, struct fec_enet_private *fep) +{ + dma_addr_t addr = bdp->cbd_bufaddr; + size_t length = bdp->cbd_datlen; + + dma_unmap_single(&fep->pdev->dev, addr, length, DMA_TO_DEVICE); +} + static int fec_enet_txq_submit_frag_skb(struct sk_buff *skb, struct net_device *ndev) { @@ -806,11 +815,13 @@ static void fec_enet_bd_init(struct net_device *dev) /* Initialize the BD for every fragment in the page. */ bdp->cbd_sc = 0; + if (bdp->cbd_bufaddr && !IS_TSO_HEADER(fep, bdp->cbd_bufaddr)) + fec_enet_tx_unmap(bdp, fep); + bdp->cbd_bufaddr = 0; if (fep->tx_skbuff[i]) { dev_kfree_skb_any(fep->tx_skbuff[i]); fep->tx_skbuff[i] = NULL; } - bdp->cbd_bufaddr = 0; bdp = fec_enet_get_nextdesc(bdp, fep); } @@ -1107,8 +1118,7 @@ fec_enet_tx(struct net_device *ndev) skb = fep->tx_skbuff[index]; fep->tx_skbuff[index] = NULL; if (!IS_TSO_HEADER(fep, bdp->cbd_bufaddr)) - dma_unmap_single(&fep->pdev->dev, bdp->cbd_bufaddr, - bdp->cbd_datlen, DMA_TO_DEVICE); + fec_enet_tx_unmap(bdp, fep); bdp->cbd_bufaddr = 0; if (!skb) { bdp = fec_enet_get_nextdesc(bdp, fep); @@ -2127,6 +2137,9 @@ static void fec_enet_free_buffers(struct net_device *ndev) bdp = fep->tx_bd_base; for (i = 0; i < fep->tx_ring_size; i++) { + if (bdp->cbd_bufaddr && !IS_TSO_HEADER(fep, bdp->cbd_bufaddr)) + fec_enet_tx_unmap(bdp, fep); + bdp->cbd_bufaddr = 0; kfree(fep->tx_bounce[i]); fep->tx_bounce[i] = NULL; skb = fep->tx_skbuff[i]; From rmk Mon Aug 15 18:57:31 2016 References: Message-ID: From: Russell King To: Linux Kernel Mailing list Date: Mon, 15 Aug 2016 18:57:31 +0100 Content-type: text/plain; charset="utf-8" Content-Disposition: inline Content-Transfer-Encoding: 8bit Mime-Version: 1.0 Subject: Re: [PATCH] net: fec: avoid hitting transmitter if it is still running From: Russell King Signed-off-by: Russell King --- drivers/net/ethernet/freescale/fec_main.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c index 87120522ba66..4e164e6ab945 100644 --- a/drivers/net/ethernet/freescale/fec_main.c +++ b/drivers/net/ethernet/freescale/fec_main.c @@ -568,7 +568,8 @@ static int fec_enet_txq_submit_skb(struct sk_buff *skb, struct net_device *ndev) fep->cur_tx = bdp; /* Trigger transmission start */ - writel(0, fep->hwp + FEC_X_DES_ACTIVE); + if (readl(fep->hwp + FEC_X_DES_ACTIVE) == 0) + writel(0, fep->hwp + FEC_X_DES_ACTIVE); return 0; } @@ -752,7 +753,8 @@ static int fec_enet_txq_submit_tso(struct sk_buff *skb, struct net_device *ndev) fep->cur_tx = bdp; /* Trigger transmission start */ - writel(0, fep->hwp + FEC_X_DES_ACTIVE); + if (readl(fep->hwp + FEC_X_DES_ACTIVE) == 0) + writel(0, fep->hwp + FEC_X_DES_ACTIVE); return 0; From rmk Mon Aug 15 18:57:31 2016 References: Message-ID: From: Russell King To: Linux Kernel Mailing list Date: Mon, 15 Aug 2016 18:57:31 +0100 Content-type: text/plain; charset="utf-8" Content-Disposition: inline Content-Transfer-Encoding: 8bit Mime-Version: 1.0 Subject: Re: [PATCH] net: fec: verify checksum offset From: Russell King Verify that the checksum offset is inside the packet header before we zero the entry. This ensures that we don't perform an out of bounds write. Signed-off-by: Russell King --- drivers/net/ethernet/freescale/fec_main.c | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c index 4e164e6ab945..1ab6388abc20 100644 --- a/drivers/net/ethernet/freescale/fec_main.c +++ b/drivers/net/ethernet/freescale/fec_main.c @@ -350,10 +350,19 @@ static inline bool is_ipv4_pkt(struct sk_buff *skb) static int fec_enet_clear_csum(struct sk_buff *skb, struct net_device *ndev) { + int csum_start; + /* Only run for packets requiring a checksum. */ if (skb->ip_summed != CHECKSUM_PARTIAL) return 0; + csum_start = skb_checksum_start_offset(skb); + if (csum_start + skb->csum_offset > skb_headlen(skb)) { + netdev_err(ndev, "checksum outside skb head: headlen %u start %u offset %u\n", + skb_headlen(skb), csum_start, skb->csum_offset); + return -1; + } + if (unlikely(skb_cow_head(skb, 0))) return -1; From rmk Mon Aug 15 18:57:31 2016 References: Message-ID: From: Russell King To: Linux Kernel Mailing list Date: Mon, 15 Aug 2016 18:57:31 +0100 Content-type: text/plain; charset="utf-8" Content-Disposition: inline Content-Transfer-Encoding: 8bit Mime-Version: 1.0 Subject: Re: [PATCH] net: fec: improve flow control support From: Russell King The FEC hardware in iMX6 is capable of separate control of each flow control path: the receiver can be programmed via the receive control register to detect flow control frames, and the transmitter can be programmed via the receive FIFO thresholds to enable generation of pause frames. This means we can implement the full range of flow control: both symmetric and asymmetric flow control. We support ethtool configuring all options: forced manual mode, where each path can be controlled individually, and autonegotiation mode. In autonegotiation mode, the tx/rx enable bits can be used to influence the outcome, though they don't precisely define which paths will be enabled. One combination we don't support is "symmetric only" since we can always configure each path independently, a case which Linux seems to often get wrong. Signed-off-by: Russell King --- drivers/net/ethernet/freescale/fec.h | 3 +- drivers/net/ethernet/freescale/fec_main.c | 196 ++++++++++++++++++++++-------- 2 files changed, 146 insertions(+), 53 deletions(-) diff --git a/drivers/net/ethernet/freescale/fec.h b/drivers/net/ethernet/freescale/fec.h index ec4c76264eb9..6f652ec2a037 100644 --- a/drivers/net/ethernet/freescale/fec.h +++ b/drivers/net/ethernet/freescale/fec.h @@ -321,7 +321,8 @@ struct fec_enet_private { struct completion mdio_done; int irq[FEC_IRQ_NUM]; int bufdesc_ex; - int pause_flag; + unsigned short pause_flag; + unsigned short pause_mode; struct napi_struct napi; int csum_flags; diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c index 1ab6388abc20..b79d78fd6e7a 100644 --- a/drivers/net/ethernet/freescale/fec_main.c +++ b/drivers/net/ethernet/freescale/fec_main.c @@ -227,8 +227,9 @@ MODULE_PARM_DESC(macaddr, "FEC Ethernet MAC address"); /* Transmitter timeout */ #define TX_TIMEOUT (2 * HZ) -#define FEC_PAUSE_FLAG_AUTONEG 0x1 -#define FEC_PAUSE_FLAG_ENABLE 0x2 +#define FEC_PAUSE_FLAG_AUTONEG BIT(0) +#define FEC_PAUSE_FLAG_RX BIT(1) +#define FEC_PAUSE_FLAG_TX BIT(2) #define TSO_HEADER_SIZE 128 /* Max number of allowed TCP segments for software TSO */ @@ -927,7 +928,7 @@ fec_restart(struct net_device *ndev) */ if (id_entry->driver_data & FEC_QUIRK_ENET_MAC) { /* Enable flow control and length check */ - rcntl |= 0x40000000 | 0x00000020; + rcntl |= 0x40000000; /* RGMII, RMII or MII */ if (fep->phy_interface == PHY_INTERFACE_MODE_RGMII) @@ -973,22 +974,24 @@ fec_restart(struct net_device *ndev) } #if !defined(CONFIG_M5272) - /* enable pause frame*/ - if ((fep->pause_flag & FEC_PAUSE_FLAG_ENABLE) || - ((fep->pause_flag & FEC_PAUSE_FLAG_AUTONEG) && - fep->phy_dev && fep->phy_dev->pause)) { - rcntl |= FEC_ENET_FCE; - - /* set FIFO threshold parameter to reduce overrun */ - writel(FEC_ENET_RSEM_V, fep->hwp + FEC_R_FIFO_RSEM); - writel(FEC_ENET_RSFL_V, fep->hwp + FEC_R_FIFO_RSFL); - writel(FEC_ENET_RAEM_V, fep->hwp + FEC_R_FIFO_RAEM); - writel(FEC_ENET_RAFL_V, fep->hwp + FEC_R_FIFO_RAFL); - - /* OPD */ - writel(FEC_ENET_OPD_V, fep->hwp + FEC_OPD); - } else { - rcntl &= ~FEC_ENET_FCE; + if (fep->full_duplex == DUPLEX_FULL) { + /* + * Configure pause modes according to the current status. + * Must only be enabled for full duplex links. + */ + if (fep->pause_mode & FEC_PAUSE_FLAG_RX) + rcntl |= FEC_ENET_FCE; + + if (fep->pause_mode & FEC_PAUSE_FLAG_TX) { + /* set FIFO threshold parameter to reduce overrun */ + writel(FEC_ENET_RSEM_V, fep->hwp + FEC_R_FIFO_RSEM); + writel(FEC_ENET_RSFL_V, fep->hwp + FEC_R_FIFO_RSFL); + writel(FEC_ENET_RAEM_V, fep->hwp + FEC_R_FIFO_RAEM); + writel(FEC_ENET_RAFL_V, fep->hwp + FEC_R_FIFO_RAFL); + + /* OPD */ + writel(FEC_ENET_OPD_V, fep->hwp + FEC_OPD); + } } #endif /* !defined(CONFIG_M5272) */ @@ -1543,6 +1546,35 @@ static void fec_enet_adjust_link(struct net_device *ndev) status_change = 1; } + if (fep->pause_flag & FEC_PAUSE_FLAG_AUTONEG) { + u32 lcl_adv = phy_dev->advertising; + u32 rmt_adv = phy_dev->lp_advertising; + unsigned mode = 0; + + if (lcl_adv & rmt_adv & ADVERTISED_Pause) { + /* + * Local Device Link Partner + * Pause AsymDir Pause AsymDir Result + * 1 X 1 X TX+RX + */ + mode = FEC_PAUSE_FLAG_TX | FEC_PAUSE_FLAG_RX; + } else if (lcl_adv & rmt_adv & ADVERTISED_Asym_Pause) { + /* + * 0 1 1 1 RX + * 1 1 0 1 TX + */ + if (rmt_adv & ADVERTISED_Pause) + mode = FEC_PAUSE_FLAG_RX; + else + mode = FEC_PAUSE_FLAG_TX; + } + + if (mode != fep->pause_mode) { + fep->pause_mode = mode; + status_change = 1; + } + } + /* if any of the above changed restart the FEC */ if (status_change) { napi_disable(&fep->napi); @@ -1674,6 +1706,34 @@ static int fec_enet_clk_enable(struct net_device *ndev, bool enable) return ret; } +static void fec_enet_phy_config(struct net_device *ndev) +{ +#ifndef CONFIG_M5272 + struct fec_enet_private *fep = netdev_priv(ndev); + struct phy_device *phy = fep->phy_dev; + unsigned pause = 0; + + /* + * Pause advertisment logic is weird. We don't advertise the raw + * "can tx" and "can rx" modes, but instead it is whether we support + * symmetric flow or asymmetric flow. + * + * Symmetric flow means we can only support both transmit and receive + * flow control frames together. Asymmetric flow means we can + * independently control each. Note that there is no bit encoding + * for "I can only receive flow control frames." + */ + if (fep->pause_flag & FEC_PAUSE_FLAG_RX) + pause |= ADVERTISED_Asym_Pause | ADVERTISED_Pause; + if (fep->pause_flag & FEC_PAUSE_FLAG_TX) + pause |= ADVERTISED_Asym_Pause; + + pause &= phy->supported; + pause |= phy->advertising & ~(ADVERTISED_Pause | ADVERTISED_Asym_Pause); + phy->advertising = pause; +#endif +} + static int fec_enet_mii_probe(struct net_device *ndev) { struct fec_enet_private *fep = netdev_priv(ndev); @@ -1728,7 +1788,7 @@ static int fec_enet_mii_probe(struct net_device *ndev) phy_dev->supported &= PHY_GBIT_FEATURES; phy_dev->supported &= ~SUPPORTED_1000baseT_Half; #if !defined(CONFIG_M5272) - phy_dev->supported |= SUPPORTED_Pause; + phy_dev->supported |= SUPPORTED_Pause | SUPPORTED_Asym_Pause; #endif } else @@ -1740,6 +1800,8 @@ static int fec_enet_mii_probe(struct net_device *ndev) fep->link = 0; fep->full_duplex = 0; + fec_enet_phy_config(ndev); + netdev_info(ndev, "Freescale FEC PHY driver [%s] (mii_bus:phy_addr=%s, irq=%d)\n", fep->phy_dev->drv->name, dev_name(&fep->phy_dev->dev), fep->phy_dev->irq); @@ -1930,50 +1992,78 @@ static void fec_enet_get_pauseparam(struct net_device *ndev, struct fec_enet_private *fep = netdev_priv(ndev); pause->autoneg = (fep->pause_flag & FEC_PAUSE_FLAG_AUTONEG) != 0; - pause->tx_pause = (fep->pause_flag & FEC_PAUSE_FLAG_ENABLE) != 0; - pause->rx_pause = pause->tx_pause; + pause->rx_pause = (fep->pause_flag & FEC_PAUSE_FLAG_RX) != 0; + pause->tx_pause = (fep->pause_flag & FEC_PAUSE_FLAG_TX) != 0; } static int fec_enet_set_pauseparam(struct net_device *ndev, struct ethtool_pauseparam *pause) { struct fec_enet_private *fep = netdev_priv(ndev); + unsigned pause_flag, changed; + struct phy_device *phy = fep->phy_dev; - if (!fep->phy_dev) + if (!phy) return -ENODEV; - - if (pause->tx_pause != pause->rx_pause) { - netdev_info(ndev, - "hardware only support enable/disable both tx and rx"); + if (!(phy->supported & SUPPORTED_Pause)) + return -EINVAL; + if (!(phy->supported & SUPPORTED_Asym_Pause) && + pause->rx_pause != pause->tx_pause) return -EINVAL; - } - fep->pause_flag = 0; + pause_flag = 0; + if (pause->autoneg) + pause_flag |= FEC_PAUSE_FLAG_AUTONEG; + if (pause->rx_pause) + pause_flag |= FEC_PAUSE_FLAG_RX; + if (pause->tx_pause) + pause_flag |= FEC_PAUSE_FLAG_TX; - /* tx pause must be same as rx pause */ - fep->pause_flag |= pause->rx_pause ? FEC_PAUSE_FLAG_ENABLE : 0; - fep->pause_flag |= pause->autoneg ? FEC_PAUSE_FLAG_AUTONEG : 0; + changed = fep->pause_flag ^ pause_flag; + fep->pause_flag = pause_flag; - if (pause->rx_pause || pause->autoneg) { - fep->phy_dev->supported |= ADVERTISED_Pause; - fep->phy_dev->advertising |= ADVERTISED_Pause; - } else { - fep->phy_dev->supported &= ~ADVERTISED_Pause; - fep->phy_dev->advertising &= ~ADVERTISED_Pause; - } + /* configure the phy advertisment according to our new options */ + fec_enet_phy_config(ndev); - if (pause->autoneg) { - if (netif_running(ndev)) - fec_stop(ndev); - phy_start_aneg(fep->phy_dev); - } - if (netif_running(ndev)) { - napi_disable(&fep->napi); - netif_tx_lock_bh(ndev); - fec_restart(ndev); - netif_wake_queue(ndev); - netif_tx_unlock_bh(ndev); - napi_enable(&fep->napi); + if (changed) { + if (pause_flag & FEC_PAUSE_FLAG_AUTONEG) { + if (netif_running(ndev)) + phy_start_aneg(fep->phy_dev); + } else { + int adv, old_adv; + + /* + * Even if we are not in autonegotiate mode, we + * still update the phy with our capabilities so + * our link parter can make the appropriate + * decision. PHYLIB provides no way to do this. + */ + adv = phy_read(phy, MII_ADVERTISE); + if (adv >= 0) { + old_adv = adv; + adv &= ~(ADVERTISE_PAUSE_CAP | ADVERTISE_PAUSE_ASYM); + if (phy->advertising & ADVERTISED_Pause) + adv |= ADVERTISE_PAUSE_CAP; + if (phy->advertising & ADVERTISED_Asym_Pause) + adv |= ADVERTISE_PAUSE_ASYM; + + if (old_adv != adv) + phy_write(phy, MII_ADVERTISE, adv); + } + + /* Forced pause mode */ + fep->pause_mode = fep->pause_flag; + + if (netif_running(ndev)) { + napi_disable(&fep->napi); + netif_tx_lock_bh(ndev); + fec_stop(ndev); + fec_restart(ndev); + netif_wake_queue(ndev); + netif_tx_unlock_bh(ndev); + napi_enable(&fep->napi); + } + } } return 0; @@ -2606,7 +2696,9 @@ fec_probe(struct platform_device *pdev) /* default enable pause frame auto negotiation */ if (pdev->id_entry && (pdev->id_entry->driver_data & FEC_QUIRK_HAS_GBIT)) - fep->pause_flag |= FEC_PAUSE_FLAG_AUTONEG; + fep->pause_flag |= FEC_PAUSE_FLAG_AUTONEG | + FEC_PAUSE_FLAG_TX | + FEC_PAUSE_FLAG_RX; #endif /* Select default pin state */ From rmk Mon Aug 15 18:57:31 2016 References: Message-ID: From: Russell King To: Linux Kernel Mailing list Date: Mon, 15 Aug 2016 18:57:31 +0100 Content-type: text/plain; charset="utf-8" Content-Disposition: inline Content-Transfer-Encoding: 8bit Mime-Version: 1.0 Subject: Re: [PATCH] net: fec: improve UDP performance From: Russell King UDP performance is extremely poor. iperf reports UDP receive speeds in the range of 50Mbps with a high packet loss. The interface typically reports a few thousand overrun errors per iperf run. This is far from satisfactory. Adjust the receive FIFO thresholds to reduce the number of errors. Firstly, we decrease RAFL (receive almost full threshold) to the minimum value of 4, which gives us the maximum available space in the FIFO prior to indicating a FIFO overrun condition. Secondly, we adjust the RSEM value to send a XOFF pause frame early enough that an in-progress transmission can complete without overflowing the FIFO. Document these registers and the changes being made in the driver. Signed-off-by: Russell King --- drivers/net/ethernet/freescale/fec_main.c | 28 +++++++++++++++++++++++++--- 1 file changed, 25 insertions(+), 3 deletions(-) diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c index b79d78fd6e7a..c30270168bec 100644 --- a/drivers/net/ethernet/freescale/fec_main.c +++ b/drivers/net/ethernet/freescale/fec_main.c @@ -72,12 +72,34 @@ static void set_multicast_list(struct net_device *ndev); #define DRIVER_NAME "fec" -/* Pause frame feild and FIFO threshold */ +/* Pause frame field and FIFO threshold */ #define FEC_ENET_FCE (1 << 5) -#define FEC_ENET_RSEM_V 0x84 +/* + * RSEM: the FIFO threshold in 64-bit words (from zero bytes) at which + * XOFF will be sent. Minimum 0, maximum possible 255. + * RAFL: the number of remaining 64-bit words in the FIFO at which the + * frame is marked with overrun. Minimum 4, maximum possible 255. + * + * When XOFF is sent, the link partner should continue sending any + * packet currently in progress, and then pause transmission for the + * specified duration. It should not terminate transmission of the + * packet. + * + * The FIFO can store the full frame and the FCS value, which on standard + * Ethernet is 1518 bytes, or 190 FIFO entries. However, we do not know + * the size of the receive FIFO, so we are unable to properly calculate + * the thresholds. + * + * Practical tests with 1514 byte ethernet packets show that with + * RSEM = 132, RAFL = 8, we see a large quantity of overflowed UDP packets. + * Reducing RAFL to 4 has almost no effect. Reducing RSEM to 78 gives us + * a small number of overflow errors, and at 77 it gives only a single + * error per UDP iperf run. + */ +#define FEC_ENET_RSEM_V 77 #define FEC_ENET_RSFL_V 16 #define FEC_ENET_RAEM_V 0x8 -#define FEC_ENET_RAFL_V 0x8 +#define FEC_ENET_RAFL_V 4 #define FEC_ENET_OPD_V 0xFFF0 /* Controller is ENET-MAC */ From rmk Mon Aug 15 18:57:31 2016 References: Message-ID: From: Russell King To: Linux Kernel Mailing list Date: Mon, 15 Aug 2016 18:57:31 +0100 Content-type: text/plain; charset="utf-8" Content-Disposition: inline Content-Transfer-Encoding: 8bit Mime-Version: 1.0 Subject: Re: [PATCH] net: fec: add byte queue limits support From: Russell King Add support for byte queue limits, which allows the network schedulers to control packet latency and packet queues more accurately. Further information on this feature can be found at https://lwn.net/Articles/469652/ Signed-off-by: Russell King --- drivers/net/ethernet/freescale/fec_main.c | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c index c30270168bec..6d4142d4ef39 100644 --- a/drivers/net/ethernet/freescale/fec_main.c +++ b/drivers/net/ethernet/freescale/fec_main.c @@ -596,6 +596,7 @@ static int fec_enet_txq_submit_skb(struct sk_buff *skb, struct net_device *ndev) bdp = fec_enet_get_nextdesc(last_bdp, fep); skb_tx_timestamp(skb); + netdev_sent_queue(ndev, skb->len); fep->cur_tx = bdp; @@ -782,6 +783,7 @@ static int fec_enet_txq_submit_tso(struct sk_buff *skb, struct net_device *ndev) fep->tx_skbuff[index] = skb; skb_tx_timestamp(skb); + netdev_sent_queue(ndev, skb->len); fep->cur_tx = bdp; /* Trigger transmission start */ @@ -903,6 +905,7 @@ fec_restart(struct net_device *ndev) writel(PKT_MAXBLR_SIZE, fep->hwp + FEC_R_BUFF_SIZE); fec_enet_bd_init(ndev); + netdev_reset_queue(ndev); /* Set receive and transmit descriptor base. */ writel(fep->bd_dma, fep->hwp + FEC_R_DES_START); @@ -1136,6 +1139,7 @@ fec_enet_tx(struct net_device *ndev) struct sk_buff *skb; int index = 0; int entries_free; + unsigned int pkts_compl, bytes_compl; fep = netdev_priv(ndev); bdp = fep->dirty_tx; @@ -1143,6 +1147,7 @@ fec_enet_tx(struct net_device *ndev) /* get next bdp of dirty_tx */ bdp = fec_enet_get_nextdesc(bdp, fep); + pkts_compl = bytes_compl = 0; while (((status = bdp->cbd_sc) & BD_ENET_TX_READY) == 0) { /* current queue is empty */ @@ -1196,6 +1201,9 @@ fec_enet_tx(struct net_device *ndev) if (status & BD_ENET_TX_DEF) ndev->stats.collisions++; + pkts_compl++; + bytes_compl += skb->len; + /* Free the sk buffer associated with this last transmit */ dev_kfree_skb_any(skb); @@ -1213,6 +1221,8 @@ fec_enet_tx(struct net_device *ndev) } } + netdev_completed_queue(ndev, pkts_compl, bytes_compl); + /* ERR006538: Keep the transmitter going */ if (bdp != fep->cur_tx && readl(fep->hwp + FEC_X_DES_ACTIVE) == 0) writel(0, fep->hwp + FEC_X_DES_ACTIVE); From rmk Mon Aug 15 18:57:31 2016 References: Message-ID: From: Russell King To: Linux Kernel Mailing list Date: Mon, 15 Aug 2016 18:57:31 +0100 Content-type: text/plain; charset="utf-8" Content-Disposition: inline Content-Transfer-Encoding: 8bit Mime-Version: 1.0 Subject: Re: [PATCH] net: fec: increase receive ring size From: Russell King Increasing the receive ring size allows more packets to be received before we run out of ring entries to store the packets. With 16 ring entries, this corresponds with a maximum of 16 * 1534 bytes, or just 24KiB. At gigabit speeds, this would take around 200us to fill. Increasing this to 512 entries gives us more scope when we have busy workloads, as it increases the ring to 767K, and around 6.3ms. Signed-off-by: Russell King --- drivers/net/ethernet/freescale/fec.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/ethernet/freescale/fec.h b/drivers/net/ethernet/freescale/fec.h index 6f652ec2a037..702fdae3d4f0 100644 --- a/drivers/net/ethernet/freescale/fec.h +++ b/drivers/net/ethernet/freescale/fec.h @@ -241,7 +241,7 @@ struct bufdesc_ex { * the skbuffer directly. */ -#define FEC_ENET_RX_PAGES 8 +#define FEC_ENET_RX_PAGES 256 #define FEC_ENET_RX_FRSIZE 2048 #define FEC_ENET_RX_FRPPG (PAGE_SIZE / FEC_ENET_RX_FRSIZE) #define RX_RING_SIZE (FEC_ENET_RX_FRPPG * FEC_ENET_RX_PAGES) From rmk Mon Aug 15 18:57:31 2016 References: Message-ID: From: Russell King To: Linux Kernel Mailing list Date: Mon, 15 Aug 2016 18:57:31 +0100 Content-type: text/plain; charset="utf-8" Content-Disposition: inline Content-Transfer-Encoding: 8bit Mime-Version: 1.0 Subject: Re: [PATCH] net: fec: move transmit dma ring address calculation to fec_enet_init() From: Russell King Move the calculation of the transmit DMA ring address to fec_enet_init() so the CPU and DMA ring address calculations are localised. Signed-off-by: Russell King --- drivers/net/ethernet/freescale/fec.h | 3 ++- drivers/net/ethernet/freescale/fec_main.c | 26 +++++++++++++------------- 2 files changed, 15 insertions(+), 14 deletions(-) diff --git a/drivers/net/ethernet/freescale/fec.h b/drivers/net/ethernet/freescale/fec.h index 702fdae3d4f0..a1caee8fd648 100644 --- a/drivers/net/ethernet/freescale/fec.h +++ b/drivers/net/ethernet/freescale/fec.h @@ -285,7 +285,8 @@ struct fec_enet_private { struct sk_buff *rx_skbuff[RX_RING_SIZE]; /* CPM dual port RAM relative addresses */ - dma_addr_t bd_dma; + dma_addr_t rx_bd_dma; + dma_addr_t tx_bd_dma; /* Address of Rx and Tx buffers */ struct bufdesc *rx_bd_base; struct bufdesc *tx_bd_base; diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c index 6d4142d4ef39..30a48ff1f798 100644 --- a/drivers/net/ethernet/freescale/fec_main.c +++ b/drivers/net/ethernet/freescale/fec_main.c @@ -908,14 +908,8 @@ fec_restart(struct net_device *ndev) netdev_reset_queue(ndev); /* Set receive and transmit descriptor base. */ - writel(fep->bd_dma, fep->hwp + FEC_R_DES_START); - if (fep->bufdesc_ex) - writel((unsigned long)fep->bd_dma + sizeof(struct bufdesc_ex) - * fep->rx_ring_size, fep->hwp + FEC_X_DES_START); - else - writel((unsigned long)fep->bd_dma + sizeof(struct bufdesc) - * fep->rx_ring_size, fep->hwp + FEC_X_DES_START); - + writel(fep->rx_bd_dma, fep->hwp + FEC_R_DES_START); + writel(fep->tx_bd_dma, fep->hwp + FEC_X_DES_START); for (i = 0; i <= TX_RING_MOD_MASK; i++) { if (fep->tx_skbuff[i]) { @@ -2586,6 +2580,7 @@ static int fec_enet_init(struct net_device *ndev) const struct platform_device_id *id_entry = platform_get_device_id(fep->pdev); struct bufdesc *cbd_base; + dma_addr_t cbd_dma; int bd_size; /* init the tx & rx ring size */ @@ -2603,15 +2598,14 @@ static int fec_enet_init(struct net_device *ndev) fep->bufdesc_size; /* Allocate memory for buffer descriptors. */ - cbd_base = dma_alloc_coherent(NULL, bd_size, &fep->bd_dma, - GFP_KERNEL); + cbd_base = dma_alloc_coherent(NULL, bd_size, &cbd_dma, GFP_KERNEL); if (!cbd_base) return -ENOMEM; fep->tso_hdrs = dma_alloc_coherent(NULL, fep->tx_ring_size * TSO_HEADER_SIZE, &fep->tso_hdrs_dma, GFP_KERNEL); if (!fep->tso_hdrs) { - dma_free_coherent(NULL, bd_size, cbd_base, fep->bd_dma); + dma_free_coherent(NULL, bd_size, cbd_base, cbd_dma); return -ENOMEM; } @@ -2626,11 +2620,17 @@ static int fec_enet_init(struct net_device *ndev) /* Set receive and transmit descriptor base. */ fep->rx_bd_base = cbd_base; - if (fep->bufdesc_ex) + fep->rx_bd_dma = cbd_dma; + if (fep->bufdesc_ex) { fep->tx_bd_base = (struct bufdesc *) (((struct bufdesc_ex *)cbd_base) + fep->rx_ring_size); - else + fep->tx_bd_dma = cbd_dma + sizeof(struct bufdesc_ex) * + fep->rx_ring_size; + } else { fep->tx_bd_base = cbd_base + fep->rx_ring_size; + fep->tx_bd_dma = cbd_dma + sizeof(struct bufdesc) * + fep->rx_ring_size; + } /* The FEC Ethernet specific entries in the device structure */ ndev->watchdog_timeo = TX_TIMEOUT; From rmk Mon Aug 15 18:57:31 2016 References: Message-ID: From: Russell King To: Linux Kernel Mailing list Date: Mon, 15 Aug 2016 18:57:31 +0100 Content-type: text/plain; charset="utf-8" Content-Disposition: inline Content-Transfer-Encoding: 8bit Mime-Version: 1.0 Subject: Re: [PATCH] net: fec: clean up fec_enet_get_free_txdesc_num() callsites From: Russell King Clean up the various callsites to this function; most callsites are using a temporary variable and immediately following it with a test. There is no need of a temporary variable for this. Signed-off-by: Russell King --- drivers/net/ethernet/freescale/fec_main.c | 17 +++++------------ 1 file changed, 5 insertions(+), 12 deletions(-) diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c index 30a48ff1f798..3fedb5f6cb7a 100644 --- a/drivers/net/ethernet/freescale/fec_main.c +++ b/drivers/net/ethernet/freescale/fec_main.c @@ -504,11 +504,9 @@ static int fec_enet_txq_submit_skb(struct sk_buff *skb, struct net_device *ndev) unsigned short buflen; unsigned int estatus = 0; unsigned int index; - int entries_free; int ret; - entries_free = fec_enet_get_free_txdesc_num(fep); - if (entries_free < MAX_SKB_FRAGS + 1) { + if (fec_enet_get_free_txdesc_num(fep) < MAX_SKB_FRAGS + 1) { dev_kfree_skb_any(skb); if (net_ratelimit()) netdev_err(ndev, "NOT enough BD for SG!\n"); @@ -801,7 +799,6 @@ static netdev_tx_t fec_enet_start_xmit(struct sk_buff *skb, struct net_device *ndev) { struct fec_enet_private *fep = netdev_priv(ndev); - int entries_free; int ret; if (skb_is_gso(skb)) @@ -811,8 +808,7 @@ fec_enet_start_xmit(struct sk_buff *skb, struct net_device *ndev) if (ret) return ret; - entries_free = fec_enet_get_free_txdesc_num(fep); - if (entries_free <= fep->tx_stop_threshold) + if (fec_enet_get_free_txdesc_num(fep) <= fep->tx_stop_threshold) netif_stop_queue(ndev); return NETDEV_TX_OK; @@ -1132,7 +1128,6 @@ fec_enet_tx(struct net_device *ndev) unsigned short status; struct sk_buff *skb; int index = 0; - int entries_free; unsigned int pkts_compl, bytes_compl; fep = netdev_priv(ndev); @@ -1208,11 +1203,9 @@ fec_enet_tx(struct net_device *ndev) /* Since we have freed up a buffer, the ring is no longer full */ - if (netif_queue_stopped(ndev)) { - entries_free = fec_enet_get_free_txdesc_num(fep); - if (entries_free >= fep->tx_wake_threshold) - netif_wake_queue(ndev); - } + if (netif_queue_stopped(ndev) && + fec_enet_get_free_txdesc_num(fep) >= fep->tx_wake_threshold) + netif_wake_queue(ndev); } netdev_completed_queue(ndev, pkts_compl, bytes_compl); From rmk Mon Aug 15 18:57:31 2016 References: Message-ID: From: Russell King To: Linux Kernel Mailing list Date: Mon, 15 Aug 2016 18:57:31 +0100 Content-type: text/plain; charset="utf-8" Content-Disposition: inline Content-Transfer-Encoding: 8bit Mime-Version: 1.0 Subject: Re: [PATCH] net: fec: move netif_wake_queue() out of transmit loop From: Russell King It is wasteful to keep checking whether we're stopped, and the number of free packets to see whether we should restart the queue for each entry in the ring which we process. Move this to the end of the processing so we check once per ring clean. Signed-off-by: Russell King --- drivers/net/ethernet/freescale/fec_main.c | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c index 3fedb5f6cb7a..d48e16da198a 100644 --- a/drivers/net/ethernet/freescale/fec_main.c +++ b/drivers/net/ethernet/freescale/fec_main.c @@ -1200,12 +1200,6 @@ fec_enet_tx(struct net_device *ndev) /* Update pointer to next buffer descriptor to be transmitted */ bdp = fec_enet_get_nextdesc(bdp, fep); - - /* Since we have freed up a buffer, the ring is no longer full - */ - if (netif_queue_stopped(ndev) && - fec_enet_get_free_txdesc_num(fep) >= fep->tx_wake_threshold) - netif_wake_queue(ndev); } netdev_completed_queue(ndev, pkts_compl, bytes_compl); @@ -1213,6 +1207,10 @@ fec_enet_tx(struct net_device *ndev) /* ERR006538: Keep the transmitter going */ if (bdp != fep->cur_tx && readl(fep->hwp + FEC_X_DES_ACTIVE) == 0) writel(0, fep->hwp + FEC_X_DES_ACTIVE); + + if (netif_queue_stopped(ndev) && + fec_enet_get_free_txdesc_num(fep) >= fep->tx_wake_threshold) + netif_wake_queue(ndev); } /* During a receive, the cur_rx points to the current incoming buffer. From rmk Mon Aug 15 18:57:31 2016 References: Message-ID: From: Russell King To: Linux Kernel Mailing list Date: Mon, 15 Aug 2016 18:57:31 +0100 Content-type: text/plain; charset="utf-8" Content-Disposition: inline Content-Transfer-Encoding: 8bit Mime-Version: 1.0 Subject: Re: [PATCH] net: fec: fix calculation of free packets From: Russell King The FEC maintains two pointers into the transmit ring: next - this is the insertion point in the ring, and points at a free entry which can be written to. dirty - this is the last dirty entry which we successfully cleaned, and is always incremented prior to cleaning an entry. The calculation used for the number of free packets is slightly buggy, which is: entries = dirty - next - 1 if (entries < 0) entries += size; Let's take some examples: At ring initialisation, dirty is set to size-1, and next is set to 0. This gives: entries = size-1 - 0 - 1 = size - 2 => size - 2 But hang on, we have no packets in the empty ring, so why "size - 2" ? Let's also check if we back the pointers up by one position - so dirty=size-2, next=size-1. entries = size-2 - size-1 - 1 = -2 - -1 - 1 = -2 => size - 2 Okay, so that's the same. Now, what about the "ring full" criteria. We can never completely fill the transmit ring, because a completely full ring is indistinguishable from a completely empty ring. We reserve one entry to permit us to keep a distinction. Hence, our "full" case is when both pointers are equal, so dirty=size-1, next=size-1. entries = size-1 - size-1 - 1 = -1 => size - 1 This is where things break down - in this case, the function is not returning the number of free entries in the ring, because it should be zero! Fix this by changing the calculation to something which reflects the actual ring behaviour: entries = dirty - next; if (entries < 0) entries += size; Plugging the above three cases into this gives: entries = size-1 - 0 = size - 1 => size - 1 entries = size-2 - size-1 = -1 => size - 1 entries = size-1 - size-1 = 0 => 0 Here, we have more correct behaviour (remembering that we have reserved one entry as described above). The perverse thing is that every test at this function's called site almost took account of the off-by-one error. Let's fix this to have saner semantics - returning the number of permissible free entries in the ring which can then be compared using expected tests against our required numbers of packets. Signed-off-by: Russell King --- drivers/net/ethernet/freescale/fec_main.c | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c index d48e16da198a..85841fb1dfda 100644 --- a/drivers/net/ethernet/freescale/fec_main.c +++ b/drivers/net/ethernet/freescale/fec_main.c @@ -323,14 +323,11 @@ static int fec_enet_get_bd_index(struct bufdesc *base, struct bufdesc *bdp, return ((const char *)bdp - (const char *)base) / fep->bufdesc_size; } -static int fec_enet_get_free_txdesc_num(struct fec_enet_private *fep) +static unsigned int fec_enet_get_free_txdesc_num(struct fec_enet_private *fep) { - int entries; - - entries = ((const char *)fep->dirty_tx - - (const char *)fep->cur_tx) / fep->bufdesc_size - 1; - - return entries > 0 ? entries : entries + fep->tx_ring_size; + int num = ((const char *)fep->dirty_tx - + (const char *)fep->cur_tx) / fep->bufdesc_size; + return num < 0 ? num + fep->tx_ring_size : num; } static void *swap_buffer(void *bufaddr, int len) @@ -808,7 +805,7 @@ fec_enet_start_xmit(struct sk_buff *skb, struct net_device *ndev) if (ret) return ret; - if (fec_enet_get_free_txdesc_num(fep) <= fep->tx_stop_threshold) + if (fec_enet_get_free_txdesc_num(fep) < fep->tx_stop_threshold) netif_stop_queue(ndev); return NETDEV_TX_OK; From rmk Mon Aug 15 18:57:31 2016 References: Message-ID: From: Russell King To: Linux Kernel Mailing list Date: Mon, 15 Aug 2016 18:57:31 +0100 Content-type: text/plain; charset="utf-8" Content-Disposition: inline Content-Transfer-Encoding: 8bit Mime-Version: 1.0 Subject: Re: [PATCH] net: fec: add barrier to transmit path to ensure proper ordering From: Russell King Ensure that the writes to the descriptor data is visible to the hardware before the descriptor is handed over to the hardware. Having discussed this with Will Deacon, we need a wmb() between writing the descriptor data and handing the descriptor over to the hardware. The corresponding rmb() is in the ethernet hardware. Signed-off-by: Russell King --- drivers/net/ethernet/freescale/fec_main.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c index 85841fb1dfda..5c75217169f8 100644 --- a/drivers/net/ethernet/freescale/fec_main.c +++ b/drivers/net/ethernet/freescale/fec_main.c @@ -581,6 +581,13 @@ static int fec_enet_txq_submit_skb(struct sk_buff *skb, struct net_device *ndev) bdp->cbd_datlen = buflen; bdp->cbd_bufaddr = addr; + /* + * We need the preceding stores to the descriptor to complete + * before updating the status field, which hands it over to the + * hardware. The corresponding rmb() is "in the hardware". + */ + wmb(); + /* Send it on its way. Tell FEC it's ready, interrupt when done, * it's the last BD of the frame, and to put the CRC on the end. */ From rmk Mon Aug 15 18:57:31 2016 References: Message-ID: From: Russell King To: Linux Kernel Mailing list Date: Mon, 15 Aug 2016 18:57:32 +0100 Content-type: text/plain; charset="utf-8" Content-Disposition: inline Content-Transfer-Encoding: 8bit Mime-Version: 1.0 Subject: Re: [PATCH] net: fec: add barrier to receive path to ensure proper ordering From: Russell King Just as we need a barrier in the transmit path, we also need a barrier in the receive path to ensure that we don't modify a handed over descriptor. Signed-off-by: Russell King --- drivers/net/ethernet/freescale/fec_main.c | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c index 5c75217169f8..27ef27f6994f 100644 --- a/drivers/net/ethernet/freescale/fec_main.c +++ b/drivers/net/ethernet/freescale/fec_main.c @@ -1377,13 +1377,6 @@ fec_enet_rx(struct net_device *ndev, int budget) dma_sync_single_for_device(&fep->pdev->dev, bdp->cbd_bufaddr, FEC_ENET_RX_FRSIZE, DMA_FROM_DEVICE); rx_processing_done: - /* Clear the status flags for this buffer */ - status &= ~BD_ENET_RX_STATS; - - /* Mark the buffer empty */ - status |= BD_ENET_RX_EMPTY; - bdp->cbd_sc = status; - if (fep->bufdesc_ex) { struct bufdesc_ex *ebdp = (struct bufdesc_ex *)bdp; @@ -1392,6 +1385,19 @@ fec_enet_rx(struct net_device *ndev, int budget) ebdp->cbd_bdu = 0; } + /* + * Ensure that the previous writes have completed before + * the status update becomes visible. + */ + wmb(); + + /* Clear the status flags for this buffer */ + status &= ~BD_ENET_RX_STATS; + + /* Mark the buffer empty */ + status |= BD_ENET_RX_EMPTY; + bdp->cbd_sc = status; + /* Update BD pointer to next entry */ bdp = fec_enet_get_nextdesc(bdp, fep); From rmk Mon Aug 15 18:57:32 2016 References: Message-ID: From: Russell King To: Linux Kernel Mailing list Date: Mon, 15 Aug 2016 18:57:32 +0100 Content-type: text/plain; charset="utf-8" Content-Disposition: inline Content-Transfer-Encoding: 8bit Mime-Version: 1.0 Subject: Re: [PATCH] net: fec: use a union for the buffer descriptors From: Russell King Using a union gives clearer C code than the existing solution, and allows the removal of some odd code from the receive path whose purpose was to merely store the enhanced buffer descriptor. Signed-off-by: Russell King --- drivers/net/ethernet/freescale/fec.h | 14 +- drivers/net/ethernet/freescale/fec_main.c | 256 ++++++++++++++---------------- 2 files changed, 130 insertions(+), 140 deletions(-) diff --git a/drivers/net/ethernet/freescale/fec.h b/drivers/net/ethernet/freescale/fec.h index a1caee8fd648..e850e7e5118f 100644 --- a/drivers/net/ethernet/freescale/fec.h +++ b/drivers/net/ethernet/freescale/fec.h @@ -170,6 +170,11 @@ struct bufdesc_ex { unsigned short res0[4]; }; +union bufdesc_u { + struct bufdesc bd; + struct bufdesc_ex ebd; +}; + /* * The following definitions courtesy of commproc.h, which where * Copyright (c) 1997 Dan Malek (dmalek@jlc.net). @@ -288,14 +293,13 @@ struct fec_enet_private { dma_addr_t rx_bd_dma; dma_addr_t tx_bd_dma; /* Address of Rx and Tx buffers */ - struct bufdesc *rx_bd_base; - struct bufdesc *tx_bd_base; + union bufdesc_u *rx_bd_base; + union bufdesc_u *tx_bd_base; /* The next free ring entry */ - struct bufdesc *cur_rx, *cur_tx; + union bufdesc_u *cur_rx, *cur_tx; /* The ring entries to be free()ed */ - struct bufdesc *dirty_tx; + union bufdesc_u *dirty_tx; - unsigned short bufdesc_size; unsigned short tx_ring_size; unsigned short rx_ring_size; unsigned short tx_stop_threshold; diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c index 27ef27f6994f..c49f2e22e6ee 100644 --- a/drivers/net/ethernet/freescale/fec_main.c +++ b/drivers/net/ethernet/freescale/fec_main.c @@ -265,68 +265,73 @@ MODULE_PARM_DESC(macaddr, "FEC Ethernet MAC address"); static int mii_cnt; static inline -struct bufdesc *fec_enet_get_nextdesc(struct bufdesc *bdp, struct fec_enet_private *fep) +union bufdesc_u *fec_enet_get_nextdesc(union bufdesc_u *bdp, struct fec_enet_private *fep) { - struct bufdesc *new_bd = bdp + 1; - struct bufdesc_ex *ex_new_bd = (struct bufdesc_ex *)bdp + 1; - struct bufdesc_ex *ex_base; - struct bufdesc *base; + union bufdesc_u *base; int ring_size; if (bdp >= fep->tx_bd_base) { base = fep->tx_bd_base; ring_size = fep->tx_ring_size; - ex_base = (struct bufdesc_ex *)fep->tx_bd_base; } else { base = fep->rx_bd_base; ring_size = fep->rx_ring_size; - ex_base = (struct bufdesc_ex *)fep->rx_bd_base; } - if (fep->bufdesc_ex) - return (struct bufdesc *)((ex_new_bd >= (ex_base + ring_size)) ? - ex_base : ex_new_bd); - else - return (new_bd >= (base + ring_size)) ? - base : new_bd; + if (fep->bufdesc_ex) { + struct bufdesc_ex *ebd = &bdp->ebd + 1; + return ebd >= (&base->ebd + ring_size) ? + base : (union bufdesc_u *)ebd; + } else { + struct bufdesc *bd = &bdp->bd + 1; + return bd >= (&base->bd + ring_size) ? + base : (union bufdesc_u *)bd; + } } static inline -struct bufdesc *fec_enet_get_prevdesc(struct bufdesc *bdp, struct fec_enet_private *fep) +union bufdesc_u *fec_enet_get_prevdesc(union bufdesc_u *bdp, struct fec_enet_private *fep) { - struct bufdesc *new_bd = bdp - 1; - struct bufdesc_ex *ex_new_bd = (struct bufdesc_ex *)bdp - 1; - struct bufdesc_ex *ex_base; - struct bufdesc *base; + union bufdesc_u *base; int ring_size; if (bdp >= fep->tx_bd_base) { base = fep->tx_bd_base; ring_size = fep->tx_ring_size; - ex_base = (struct bufdesc_ex *)fep->tx_bd_base; } else { base = fep->rx_bd_base; ring_size = fep->rx_ring_size; - ex_base = (struct bufdesc_ex *)fep->rx_bd_base; } - if (fep->bufdesc_ex) - return (struct bufdesc *)((ex_new_bd < ex_base) ? - (ex_new_bd + ring_size) : ex_new_bd); - else - return (new_bd < base) ? (new_bd + ring_size) : new_bd; + if (fep->bufdesc_ex) { + struct bufdesc_ex *ebd = &bdp->ebd - 1; + return (union bufdesc_u *)(ebd < &base->ebd ? + ebd + ring_size : ebd); + } else { + struct bufdesc *bd = &bdp->bd - 1; + return (union bufdesc_u *)(bd < &base->bd ? + bd + ring_size : bd); + } } -static int fec_enet_get_bd_index(struct bufdesc *base, struct bufdesc *bdp, +static int fec_enet_get_bd_index(union bufdesc_u *base, union bufdesc_u *bdp, struct fec_enet_private *fep) { - return ((const char *)bdp - (const char *)base) / fep->bufdesc_size; + if (fep->bufdesc_ex) + return &bdp->ebd - &base->ebd; + else + return &bdp->bd - &base->bd; } static unsigned int fec_enet_get_free_txdesc_num(struct fec_enet_private *fep) { - int num = ((const char *)fep->dirty_tx - - (const char *)fep->cur_tx) / fep->bufdesc_size; + int num; + + if (fep->bufdesc_ex) + num = &fep->dirty_tx->ebd - &fep->cur_tx->ebd; + else + num = &fep->dirty_tx->bd - &fep->cur_tx->bd; + return num < 0 ? num + fep->tx_ring_size : num; } @@ -344,7 +349,7 @@ static void *swap_buffer(void *bufaddr, int len) static void fec_dump(struct net_device *ndev) { struct fec_enet_private *fep = netdev_priv(ndev); - struct bufdesc *bdp = fep->tx_bd_base; + union bufdesc_u *bdp = fep->tx_bd_base; unsigned int index = 0; netdev_info(ndev, "TX ring dump\n"); @@ -355,7 +360,8 @@ static void fec_dump(struct net_device *ndev) index, bdp == fep->cur_tx ? 'S' : ' ', bdp == fep->dirty_tx ? 'H' : ' ', - bdp->cbd_sc, bdp->cbd_bufaddr, bdp->cbd_datlen, + bdp->bd.cbd_sc, bdp->bd.cbd_bufaddr, + bdp->bd.cbd_datlen, fep->tx_skbuff[index]); bdp = fec_enet_get_nextdesc(bdp, fep); index++; @@ -394,10 +400,10 @@ fec_enet_clear_csum(struct sk_buff *skb, struct net_device *ndev) } static void -fec_enet_tx_unmap(struct bufdesc *bdp, struct fec_enet_private *fep) +fec_enet_tx_unmap(union bufdesc_u *bdp, struct fec_enet_private *fep) { - dma_addr_t addr = bdp->cbd_bufaddr; - size_t length = bdp->cbd_datlen; + dma_addr_t addr = bdp->bd.cbd_bufaddr; + size_t length = bdp->bd.cbd_datlen; dma_unmap_single(&fep->pdev->dev, addr, length, DMA_TO_DEVICE); } @@ -408,8 +414,7 @@ fec_enet_txq_submit_frag_skb(struct sk_buff *skb, struct net_device *ndev) struct fec_enet_private *fep = netdev_priv(ndev); const struct platform_device_id *id_entry = platform_get_device_id(fep->pdev); - struct bufdesc *bdp = fep->cur_tx; - struct bufdesc_ex *ebdp; + union bufdesc_u *bdp = fep->cur_tx; int nr_frags = skb_shinfo(skb)->nr_frags; int frag, frag_len; unsigned short status; @@ -423,9 +428,8 @@ fec_enet_txq_submit_frag_skb(struct sk_buff *skb, struct net_device *ndev) for (frag = 0; frag < nr_frags; frag++) { this_frag = &skb_shinfo(skb)->frags[frag]; bdp = fec_enet_get_nextdesc(bdp, fep); - ebdp = (struct bufdesc_ex *)bdp; - status = bdp->cbd_sc; + status = bdp->bd.cbd_sc; status &= ~BD_ENET_TX_STATS; status |= (BD_ENET_TX_TC | BD_ENET_TX_READY); frag_len = skb_shinfo(skb)->frags[frag].size; @@ -444,8 +448,8 @@ fec_enet_txq_submit_frag_skb(struct sk_buff *skb, struct net_device *ndev) if (fep->bufdesc_ex) { if (skb->ip_summed == CHECKSUM_PARTIAL) estatus |= BD_ENET_TX_PINS | BD_ENET_TX_IINS; - ebdp->cbd_bdu = 0; - ebdp->cbd_esc = estatus; + bdp->ebd.cbd_bdu = 0; + bdp->ebd.cbd_esc = estatus; } bufaddr = page_address(this_frag->page.p) + this_frag->page_offset; @@ -469,9 +473,9 @@ fec_enet_txq_submit_frag_skb(struct sk_buff *skb, struct net_device *ndev) goto dma_mapping_error; } - bdp->cbd_bufaddr = addr; - bdp->cbd_datlen = frag_len; - bdp->cbd_sc = status; + bdp->bd.cbd_bufaddr = addr; + bdp->bd.cbd_datlen = frag_len; + bdp->bd.cbd_sc = status; } fep->cur_tx = bdp; @@ -482,8 +486,8 @@ fec_enet_txq_submit_frag_skb(struct sk_buff *skb, struct net_device *ndev) bdp = fep->cur_tx; for (i = 0; i < frag; i++) { bdp = fec_enet_get_nextdesc(bdp, fep); - dma_unmap_single(&fep->pdev->dev, bdp->cbd_bufaddr, - bdp->cbd_datlen, DMA_TO_DEVICE); + dma_unmap_single(&fep->pdev->dev, bdp->bd.cbd_bufaddr, + bdp->bd.cbd_datlen, DMA_TO_DEVICE); } return NETDEV_TX_OK; } @@ -494,7 +498,7 @@ static int fec_enet_txq_submit_skb(struct sk_buff *skb, struct net_device *ndev) const struct platform_device_id *id_entry = platform_get_device_id(fep->pdev); int nr_frags = skb_shinfo(skb)->nr_frags; - struct bufdesc *bdp, *last_bdp; + union bufdesc_u *bdp, *last_bdp; void *bufaddr; dma_addr_t addr; unsigned short status; @@ -518,7 +522,7 @@ static int fec_enet_txq_submit_skb(struct sk_buff *skb, struct net_device *ndev) /* Fill in a Tx ring entry */ bdp = fep->cur_tx; - status = bdp->cbd_sc; + status = bdp->bd.cbd_sc; status &= ~BD_ENET_TX_STATS; /* Set buffer length and buffer pointer */ @@ -559,9 +563,6 @@ static int fec_enet_txq_submit_skb(struct sk_buff *skb, struct net_device *ndev) } if (fep->bufdesc_ex) { - - struct bufdesc_ex *ebdp = (struct bufdesc_ex *)bdp; - if (unlikely(skb_shinfo(skb)->tx_flags & SKBTX_HW_TSTAMP && fep->hwts_tx_en)) skb_shinfo(skb)->tx_flags |= SKBTX_IN_PROGRESS; @@ -569,8 +570,8 @@ static int fec_enet_txq_submit_skb(struct sk_buff *skb, struct net_device *ndev) if (skb->ip_summed == CHECKSUM_PARTIAL) estatus |= BD_ENET_TX_PINS | BD_ENET_TX_IINS; - ebdp->cbd_bdu = 0; - ebdp->cbd_esc = estatus; + bdp->ebd.cbd_bdu = 0; + bdp->ebd.cbd_esc = estatus; } last_bdp = fep->cur_tx; @@ -578,8 +579,8 @@ static int fec_enet_txq_submit_skb(struct sk_buff *skb, struct net_device *ndev) /* Save skb pointer */ fep->tx_skbuff[index] = skb; - bdp->cbd_datlen = buflen; - bdp->cbd_bufaddr = addr; + bdp->bd.cbd_datlen = buflen; + bdp->bd.cbd_bufaddr = addr; /* * We need the preceding stores to the descriptor to complete @@ -592,7 +593,7 @@ static int fec_enet_txq_submit_skb(struct sk_buff *skb, struct net_device *ndev) * it's the last BD of the frame, and to put the CRC on the end. */ status |= (BD_ENET_TX_READY | BD_ENET_TX_TC); - bdp->cbd_sc = status; + bdp->bd.cbd_sc = status; /* If this was the last BD in the ring, start at the beginning again. */ bdp = fec_enet_get_nextdesc(last_bdp, fep); @@ -611,18 +612,17 @@ static int fec_enet_txq_submit_skb(struct sk_buff *skb, struct net_device *ndev) static int fec_enet_txq_put_data_tso(struct sk_buff *skb, struct net_device *ndev, - struct bufdesc *bdp, int index, char *data, + union bufdesc_u *bdp, int index, char *data, int size, bool last_tcp, bool is_last) { struct fec_enet_private *fep = netdev_priv(ndev); const struct platform_device_id *id_entry = platform_get_device_id(fep->pdev); - struct bufdesc_ex *ebdp = (struct bufdesc_ex *)bdp; unsigned short status; unsigned int estatus = 0; dma_addr_t addr; - status = bdp->cbd_sc; + status = bdp->bd.cbd_sc; status &= ~BD_ENET_TX_STATS; status |= (BD_ENET_TX_TC | BD_ENET_TX_READY); @@ -644,14 +644,14 @@ fec_enet_txq_put_data_tso(struct sk_buff *skb, struct net_device *ndev, return NETDEV_TX_BUSY; } - bdp->cbd_datlen = size; - bdp->cbd_bufaddr = addr; + bdp->bd.cbd_datlen = size; + bdp->bd.cbd_bufaddr = addr; if (fep->bufdesc_ex) { if (skb->ip_summed == CHECKSUM_PARTIAL) estatus |= BD_ENET_TX_PINS | BD_ENET_TX_IINS; - ebdp->cbd_bdu = 0; - ebdp->cbd_esc = estatus; + bdp->ebd.cbd_bdu = 0; + bdp->ebd.cbd_esc = estatus; } /* Handle the last BD specially */ @@ -660,29 +660,28 @@ fec_enet_txq_put_data_tso(struct sk_buff *skb, struct net_device *ndev, if (is_last) { status |= BD_ENET_TX_INTR; if (fep->bufdesc_ex) - ebdp->cbd_esc |= BD_ENET_TX_INT; + bdp->ebd.cbd_esc |= BD_ENET_TX_INT; } - bdp->cbd_sc = status; + bdp->bd.cbd_sc = status; return 0; } static int fec_enet_txq_put_hdr_tso(struct sk_buff *skb, struct net_device *ndev, - struct bufdesc *bdp, int index) + union bufdesc_u *bdp, int index) { struct fec_enet_private *fep = netdev_priv(ndev); const struct platform_device_id *id_entry = platform_get_device_id(fep->pdev); int hdr_len = skb_transport_offset(skb) + tcp_hdrlen(skb); - struct bufdesc_ex *ebdp = (struct bufdesc_ex *)bdp; void *bufaddr; unsigned long dmabuf; unsigned short status; unsigned int estatus = 0; - status = bdp->cbd_sc; + status = bdp->bd.cbd_sc; status &= ~BD_ENET_TX_STATS; status |= (BD_ENET_TX_TC | BD_ENET_TX_READY); @@ -706,17 +705,17 @@ fec_enet_txq_put_hdr_tso(struct sk_buff *skb, struct net_device *ndev, } } - bdp->cbd_bufaddr = dmabuf; - bdp->cbd_datlen = hdr_len; + bdp->bd.cbd_bufaddr = dmabuf; + bdp->bd.cbd_datlen = hdr_len; if (fep->bufdesc_ex) { if (skb->ip_summed == CHECKSUM_PARTIAL) estatus |= BD_ENET_TX_PINS | BD_ENET_TX_IINS; - ebdp->cbd_bdu = 0; - ebdp->cbd_esc = estatus; + bdp->ebd.cbd_bdu = 0; + bdp->ebd.cbd_esc = estatus; } - bdp->cbd_sc = status; + bdp->bd.cbd_sc = status; return 0; } @@ -726,7 +725,7 @@ static int fec_enet_txq_submit_tso(struct sk_buff *skb, struct net_device *ndev) struct fec_enet_private *fep = netdev_priv(ndev); int hdr_len = skb_transport_offset(skb) + tcp_hdrlen(skb); int total_len, data_left; - struct bufdesc *bdp = fep->cur_tx; + union bufdesc_u *bdp = fep->cur_tx; struct tso_t tso; unsigned int index = 0; int ret; @@ -823,7 +822,7 @@ fec_enet_start_xmit(struct sk_buff *skb, struct net_device *ndev) static void fec_enet_bd_init(struct net_device *dev) { struct fec_enet_private *fep = netdev_priv(dev); - struct bufdesc *bdp; + union bufdesc_u *bdp; unsigned int i; /* Initialize the receive buffer descriptors. */ @@ -831,16 +830,16 @@ static void fec_enet_bd_init(struct net_device *dev) for (i = 0; i < fep->rx_ring_size; i++) { /* Initialize the BD for every fragment in the page. */ - if (bdp->cbd_bufaddr) - bdp->cbd_sc = BD_ENET_RX_EMPTY; + if (bdp->bd.cbd_bufaddr) + bdp->bd.cbd_sc = BD_ENET_RX_EMPTY; else - bdp->cbd_sc = 0; + bdp->bd.cbd_sc = 0; bdp = fec_enet_get_nextdesc(bdp, fep); } /* Set the last buffer to wrap */ bdp = fec_enet_get_prevdesc(bdp, fep); - bdp->cbd_sc |= BD_SC_WRAP; + bdp->bd.cbd_sc |= BD_SC_WRAP; fep->cur_rx = fep->rx_bd_base; @@ -850,10 +849,10 @@ static void fec_enet_bd_init(struct net_device *dev) for (i = 0; i < fep->tx_ring_size; i++) { /* Initialize the BD for every fragment in the page. */ - bdp->cbd_sc = 0; - if (bdp->cbd_bufaddr && !IS_TSO_HEADER(fep, bdp->cbd_bufaddr)) + bdp->bd.cbd_sc = 0; + if (bdp->bd.cbd_bufaddr) fec_enet_tx_unmap(bdp, fep); - bdp->cbd_bufaddr = 0; + bdp->bd.cbd_bufaddr = 0; if (fep->tx_skbuff[i]) { dev_kfree_skb_any(fep->tx_skbuff[i]); fep->tx_skbuff[i] = NULL; @@ -863,7 +862,7 @@ static void fec_enet_bd_init(struct net_device *dev) /* Set the last buffer to wrap */ bdp = fec_enet_get_prevdesc(bdp, fep); - bdp->cbd_sc |= BD_SC_WRAP; + bdp->bd.cbd_sc |= BD_SC_WRAP; fep->dirty_tx = bdp; } @@ -1128,7 +1127,7 @@ static void fec_enet_tx(struct net_device *ndev) { struct fec_enet_private *fep; - struct bufdesc *bdp; + union bufdesc_u *bdp; unsigned short status; struct sk_buff *skb; int index = 0; @@ -1141,7 +1140,7 @@ fec_enet_tx(struct net_device *ndev) bdp = fec_enet_get_nextdesc(bdp, fep); pkts_compl = bytes_compl = 0; - while (((status = bdp->cbd_sc) & BD_ENET_TX_READY) == 0) { + while (((status = bdp->bd.cbd_sc) & BD_ENET_TX_READY) == 0) { /* current queue is empty */ if (bdp == fep->cur_tx) @@ -1151,9 +1150,9 @@ fec_enet_tx(struct net_device *ndev) skb = fep->tx_skbuff[index]; fep->tx_skbuff[index] = NULL; - if (!IS_TSO_HEADER(fep, bdp->cbd_bufaddr)) + if (!IS_TSO_HEADER(fep, bdp->bd.cbd_bufaddr)) fec_enet_tx_unmap(bdp, fep); - bdp->cbd_bufaddr = 0; + bdp->bd.cbd_bufaddr = 0; if (!skb) { bdp = fec_enet_get_nextdesc(bdp, fep); continue; @@ -1182,9 +1181,8 @@ fec_enet_tx(struct net_device *ndev) if (unlikely(skb_shinfo(skb)->tx_flags & SKBTX_IN_PROGRESS) && fep->bufdesc_ex) { struct skb_shared_hwtstamps shhwtstamps; - struct bufdesc_ex *ebdp = (struct bufdesc_ex *)bdp; - fec_enet_hwtstamp(fep, ebdp->ts, &shhwtstamps); + fec_enet_hwtstamp(fep, bdp->ebd.ts, &shhwtstamps); skb_tstamp_tx(skb, &shhwtstamps); } @@ -1228,13 +1226,12 @@ fec_enet_rx(struct net_device *ndev, int budget) struct fec_enet_private *fep = netdev_priv(ndev); const struct platform_device_id *id_entry = platform_get_device_id(fep->pdev); - struct bufdesc *bdp; + union bufdesc_u *bdp; unsigned short status; struct sk_buff *skb; ushort pkt_len; __u8 *data; int pkt_received = 0; - struct bufdesc_ex *ebdp = NULL; bool vlan_packet_rcvd = false; u16 vlan_tag; int index = 0; @@ -1248,7 +1245,7 @@ fec_enet_rx(struct net_device *ndev, int budget) */ bdp = fep->cur_rx; - while (!((status = bdp->cbd_sc) & BD_ENET_RX_EMPTY)) { + while (!((status = bdp->bd.cbd_sc) & BD_ENET_RX_EMPTY)) { if (pkt_received >= budget) break; @@ -1297,26 +1294,21 @@ fec_enet_rx(struct net_device *ndev, int budget) /* Process the incoming frame. */ ndev->stats.rx_packets++; - pkt_len = bdp->cbd_datlen; + pkt_len = bdp->bd.cbd_datlen; ndev->stats.rx_bytes += pkt_len; index = fec_enet_get_bd_index(fep->rx_bd_base, bdp, fep); data = fep->rx_skbuff[index]->data; - dma_sync_single_for_cpu(&fep->pdev->dev, bdp->cbd_bufaddr, + dma_sync_single_for_cpu(&fep->pdev->dev, bdp->bd.cbd_bufaddr, FEC_ENET_RX_FRSIZE, DMA_FROM_DEVICE); if (id_entry->driver_data & FEC_QUIRK_SWAP_FRAME) swap_buffer(data, pkt_len); - /* Extract the enhanced buffer descriptor */ - ebdp = NULL; - if (fep->bufdesc_ex) - ebdp = (struct bufdesc_ex *)bdp; - /* If this is a VLAN packet remove the VLAN Tag */ vlan_packet_rcvd = false; if ((ndev->features & NETIF_F_HW_VLAN_CTAG_RX) && - fep->bufdesc_ex && (ebdp->cbd_esc & BD_ENET_RX_VLAN)) { + fep->bufdesc_ex && (bdp->ebd.cbd_esc & BD_ENET_RX_VLAN)) { /* Push and remove the vlan tag */ struct vlan_hdr *vlan_header = (struct vlan_hdr *) (data + ETH_HLEN); @@ -1352,12 +1344,12 @@ fec_enet_rx(struct net_device *ndev, int budget) /* Get receive timestamp from the skb */ if (fep->hwts_rx_en && fep->bufdesc_ex) - fec_enet_hwtstamp(fep, ebdp->ts, + fec_enet_hwtstamp(fep, bdp->ebd.ts, skb_hwtstamps(skb)); if (fep->bufdesc_ex && (fep->csum_flags & FLAG_RX_CSUM_ENABLED)) { - if (!(ebdp->cbd_esc & FLAG_RX_CSUM_ERROR)) { + if (!(bdp->ebd.cbd_esc & FLAG_RX_CSUM_ERROR)) { /* don't check it */ skb->ip_summed = CHECKSUM_UNNECESSARY; } else { @@ -1374,15 +1366,13 @@ fec_enet_rx(struct net_device *ndev, int budget) napi_gro_receive(&fep->napi, skb); } - dma_sync_single_for_device(&fep->pdev->dev, bdp->cbd_bufaddr, + dma_sync_single_for_device(&fep->pdev->dev, bdp->bd.cbd_bufaddr, FEC_ENET_RX_FRSIZE, DMA_FROM_DEVICE); rx_processing_done: if (fep->bufdesc_ex) { - struct bufdesc_ex *ebdp = (struct bufdesc_ex *)bdp; - - ebdp->cbd_esc = BD_ENET_RX_INT; - ebdp->cbd_prot = 0; - ebdp->cbd_bdu = 0; + bdp->ebd.cbd_esc = BD_ENET_RX_INT; + bdp->ebd.cbd_prot = 0; + bdp->ebd.cbd_bdu = 0; } /* @@ -1396,7 +1386,7 @@ fec_enet_rx(struct net_device *ndev, int budget) /* Mark the buffer empty */ status |= BD_ENET_RX_EMPTY; - bdp->cbd_sc = status; + bdp->bd.cbd_sc = status; /* Update BD pointer to next entry */ bdp = fec_enet_get_nextdesc(bdp, fep); @@ -2249,14 +2239,14 @@ static void fec_enet_free_buffers(struct net_device *ndev) struct fec_enet_private *fep = netdev_priv(ndev); unsigned int i; struct sk_buff *skb; - struct bufdesc *bdp; + union bufdesc_u *bdp; bdp = fep->rx_bd_base; for (i = 0; i < fep->rx_ring_size; i++) { skb = fep->rx_skbuff[i]; fep->rx_skbuff[i] = NULL; if (skb) { - dma_unmap_single(&fep->pdev->dev, bdp->cbd_bufaddr, + dma_unmap_single(&fep->pdev->dev, bdp->bd.cbd_bufaddr, FEC_ENET_RX_FRSIZE, DMA_FROM_DEVICE); dev_kfree_skb(skb); } @@ -2265,9 +2255,9 @@ static void fec_enet_free_buffers(struct net_device *ndev) bdp = fep->tx_bd_base; for (i = 0; i < fep->tx_ring_size; i++) { - if (bdp->cbd_bufaddr && !IS_TSO_HEADER(fep, bdp->cbd_bufaddr)) + if (bdp->bd.cbd_bufaddr) fec_enet_tx_unmap(bdp, fep); - bdp->cbd_bufaddr = 0; + bdp->bd.cbd_bufaddr = 0; kfree(fep->tx_bounce[i]); fep->tx_bounce[i] = NULL; skb = fep->tx_skbuff[i]; @@ -2281,7 +2271,7 @@ static int fec_enet_alloc_buffers(struct net_device *ndev) struct fec_enet_private *fep = netdev_priv(ndev); unsigned int i; struct sk_buff *skb; - struct bufdesc *bdp; + union bufdesc_u *bdp; bdp = fep->rx_bd_base; for (i = 0; i < fep->rx_ring_size; i++) { @@ -2301,20 +2291,18 @@ static int fec_enet_alloc_buffers(struct net_device *ndev) } fep->rx_skbuff[i] = skb; - bdp->cbd_bufaddr = addr; - bdp->cbd_sc = BD_ENET_RX_EMPTY; + bdp->bd.cbd_bufaddr = addr; + bdp->bd.cbd_sc = BD_ENET_RX_EMPTY; - if (fep->bufdesc_ex) { - struct bufdesc_ex *ebdp = (struct bufdesc_ex *)bdp; - ebdp->cbd_esc = BD_ENET_RX_INT; - } + if (fep->bufdesc_ex) + bdp->ebd.cbd_esc = BD_ENET_RX_INT; bdp = fec_enet_get_nextdesc(bdp, fep); } /* Set the last buffer to wrap. */ bdp = fec_enet_get_prevdesc(bdp, fep); - bdp->cbd_sc |= BD_SC_WRAP; + bdp->bd.cbd_sc |= BD_SC_WRAP; bdp = fep->tx_bd_base; for (i = 0; i < fep->tx_ring_size; i++) { @@ -2322,20 +2310,18 @@ static int fec_enet_alloc_buffers(struct net_device *ndev) if (!fep->tx_bounce[i]) goto err_alloc; - bdp->cbd_sc = 0; - bdp->cbd_bufaddr = 0; + bdp->bd.cbd_sc = 0; + bdp->bd.cbd_bufaddr = 0; - if (fep->bufdesc_ex) { - struct bufdesc_ex *ebdp = (struct bufdesc_ex *)bdp; - ebdp->cbd_esc = BD_ENET_TX_INT; - } + if (fep->bufdesc_ex) + bdp->ebd.cbd_esc = BD_ENET_TX_INT; bdp = fec_enet_get_nextdesc(bdp, fep); } /* Set the last buffer to wrap. */ bdp = fec_enet_get_prevdesc(bdp, fep); - bdp->cbd_sc |= BD_SC_WRAP; + bdp->bd.cbd_sc |= BD_SC_WRAP; return 0; @@ -2580,7 +2566,7 @@ static int fec_enet_init(struct net_device *ndev) struct fec_enet_private *fep = netdev_priv(ndev); const struct platform_device_id *id_entry = platform_get_device_id(fep->pdev); - struct bufdesc *cbd_base; + union bufdesc_u *cbd_base; dma_addr_t cbd_dma; int bd_size; @@ -2591,12 +2577,11 @@ static int fec_enet_init(struct net_device *ndev) fep->tx_stop_threshold = FEC_MAX_SKB_DESCS; fep->tx_wake_threshold = (fep->tx_ring_size - fep->tx_stop_threshold) / 2; + bd_size = fep->tx_ring_size + fep->rx_ring_size; if (fep->bufdesc_ex) - fep->bufdesc_size = sizeof(struct bufdesc_ex); + bd_size *= sizeof(struct bufdesc_ex); else - fep->bufdesc_size = sizeof(struct bufdesc); - bd_size = (fep->tx_ring_size + fep->rx_ring_size) * - fep->bufdesc_size; + bd_size *= sizeof(struct bufdesc); /* Allocate memory for buffer descriptors. */ cbd_base = dma_alloc_coherent(NULL, bd_size, &cbd_dma, GFP_KERNEL); @@ -2623,12 +2608,13 @@ static int fec_enet_init(struct net_device *ndev) fep->rx_bd_base = cbd_base; fep->rx_bd_dma = cbd_dma; if (fep->bufdesc_ex) { - fep->tx_bd_base = (struct bufdesc *) - (((struct bufdesc_ex *)cbd_base) + fep->rx_ring_size); + fep->tx_bd_base = (union bufdesc_u *) + (&cbd_base->ebd + fep->rx_ring_size); fep->tx_bd_dma = cbd_dma + sizeof(struct bufdesc_ex) * fep->rx_ring_size; } else { - fep->tx_bd_base = cbd_base + fep->rx_ring_size; + fep->tx_bd_base = (union bufdesc_u *) + (&cbd_base->bd + fep->rx_ring_size); fep->tx_bd_dma = cbd_dma + sizeof(struct bufdesc) * fep->rx_ring_size; } From rmk Mon Aug 15 18:57:32 2016 References: Message-ID: From: Russell King To: Linux Kernel Mailing list Date: Mon, 15 Aug 2016 18:57:32 +0100 Content-type: text/plain; charset="utf-8" Content-Disposition: inline Content-Transfer-Encoding: 8bit Mime-Version: 1.0 Subject: Re: [PATCH] net: fec: better indexing for transmit descriptor ring From: Russell King Maintaining the transmit ring position via pointers is inefficient, especially when it involves complex pointer manipulation, and overlap with the receive descriptor logic. Re-implement this using indexes, and a single function which returns the descriptor (appropriate to the descriptor type). As an additional benefit, using a union allows cleaner access to the descriptor. Signed-off-by: Russell King --- drivers/net/ethernet/freescale/fec.h | 6 +- drivers/net/ethernet/freescale/fec_main.c | 188 +++++++++++++++--------------- 2 files changed, 98 insertions(+), 96 deletions(-) diff --git a/drivers/net/ethernet/freescale/fec.h b/drivers/net/ethernet/freescale/fec.h index e850e7e5118f..76fe5a9bb85e 100644 --- a/drivers/net/ethernet/freescale/fec.h +++ b/drivers/net/ethernet/freescale/fec.h @@ -296,9 +296,9 @@ struct fec_enet_private { union bufdesc_u *rx_bd_base; union bufdesc_u *tx_bd_base; /* The next free ring entry */ - union bufdesc_u *cur_rx, *cur_tx; - /* The ring entries to be free()ed */ - union bufdesc_u *dirty_tx; + unsigned short tx_next; + unsigned short tx_dirty; + union bufdesc_u *cur_rx; unsigned short tx_ring_size; unsigned short rx_ring_size; diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c index c49f2e22e6ee..fdd190f4bf0e 100644 --- a/drivers/net/ethernet/freescale/fec_main.c +++ b/drivers/net/ethernet/freescale/fec_main.c @@ -264,19 +264,28 @@ MODULE_PARM_DESC(macaddr, "FEC Ethernet MAC address"); static int mii_cnt; +static union bufdesc_u * +fec_enet_tx_get(unsigned int index, struct fec_enet_private *fep) +{ + union bufdesc_u *base = fep->tx_bd_base; + union bufdesc_u *bdp; + + if (fep->bufdesc_ex) + bdp = (union bufdesc_u *)(&base->ebd + index); + else + bdp = (union bufdesc_u *)(&base->bd + index); + + return bdp; +} + static inline union bufdesc_u *fec_enet_get_nextdesc(union bufdesc_u *bdp, struct fec_enet_private *fep) { union bufdesc_u *base; int ring_size; - if (bdp >= fep->tx_bd_base) { - base = fep->tx_bd_base; - ring_size = fep->tx_ring_size; - } else { - base = fep->rx_bd_base; - ring_size = fep->rx_ring_size; - } + base = fep->rx_bd_base; + ring_size = fep->rx_ring_size; if (fep->bufdesc_ex) { struct bufdesc_ex *ebd = &bdp->ebd + 1; @@ -295,13 +304,8 @@ union bufdesc_u *fec_enet_get_prevdesc(union bufdesc_u *bdp, struct fec_enet_pri union bufdesc_u *base; int ring_size; - if (bdp >= fep->tx_bd_base) { - base = fep->tx_bd_base; - ring_size = fep->tx_ring_size; - } else { - base = fep->rx_bd_base; - ring_size = fep->rx_ring_size; - } + base = fep->rx_bd_base; + ring_size = fep->rx_ring_size; if (fep->bufdesc_ex) { struct bufdesc_ex *ebd = &bdp->ebd - 1; @@ -325,13 +329,7 @@ static int fec_enet_get_bd_index(union bufdesc_u *base, union bufdesc_u *bdp, static unsigned int fec_enet_get_free_txdesc_num(struct fec_enet_private *fep) { - int num; - - if (fep->bufdesc_ex) - num = &fep->dirty_tx->ebd - &fep->cur_tx->ebd; - else - num = &fep->dirty_tx->bd - &fep->cur_tx->bd; - + int num = fep->tx_dirty - fep->tx_next; return num < 0 ? num + fep->tx_ring_size : num; } @@ -349,23 +347,23 @@ static void *swap_buffer(void *bufaddr, int len) static void fec_dump(struct net_device *ndev) { struct fec_enet_private *fep = netdev_priv(ndev); - union bufdesc_u *bdp = fep->tx_bd_base; + union bufdesc_u *bdp; unsigned int index = 0; netdev_info(ndev, "TX ring dump\n"); pr_info("Nr SC addr len SKB\n"); - do { + for (index = 0; index < fep->tx_ring_size; index++) { + bdp = fec_enet_tx_get(index, fep); + pr_info("%3u %c%c 0x%04x 0x%08lx %4u %p\n", index, - bdp == fep->cur_tx ? 'S' : ' ', - bdp == fep->dirty_tx ? 'H' : ' ', + index == fep->tx_next ? 'S' : ' ', + index == fep->tx_dirty ? 'H' : ' ', bdp->bd.cbd_sc, bdp->bd.cbd_bufaddr, bdp->bd.cbd_datlen, fep->tx_skbuff[index]); - bdp = fec_enet_get_nextdesc(bdp, fep); - index++; - } while (bdp != fep->tx_bd_base); + } } static inline bool is_ipv4_pkt(struct sk_buff *skb) @@ -414,21 +412,24 @@ fec_enet_txq_submit_frag_skb(struct sk_buff *skb, struct net_device *ndev) struct fec_enet_private *fep = netdev_priv(ndev); const struct platform_device_id *id_entry = platform_get_device_id(fep->pdev); - union bufdesc_u *bdp = fep->cur_tx; + union bufdesc_u *bdp; int nr_frags = skb_shinfo(skb)->nr_frags; int frag, frag_len; unsigned short status; unsigned int estatus = 0; skb_frag_t *this_frag; - unsigned int index; + unsigned int index = fep->tx_next; void *bufaddr; dma_addr_t addr; int i; for (frag = 0; frag < nr_frags; frag++) { this_frag = &skb_shinfo(skb)->frags[frag]; - bdp = fec_enet_get_nextdesc(bdp, fep); + if (++index >= fep->tx_ring_size) + index = 0; + + bdp = fec_enet_tx_get(index, fep); status = bdp->bd.cbd_sc; status &= ~BD_ENET_TX_STATS; status |= (BD_ENET_TX_TC | BD_ENET_TX_READY); @@ -454,7 +455,6 @@ fec_enet_txq_submit_frag_skb(struct sk_buff *skb, struct net_device *ndev) bufaddr = page_address(this_frag->page.p) + this_frag->page_offset; - index = fec_enet_get_bd_index(fep->tx_bd_base, bdp, fep); if (((unsigned long) bufaddr) & FEC_ALIGNMENT || id_entry->driver_data & FEC_QUIRK_SWAP_FRAME) { memcpy(fep->tx_bounce[index], bufaddr, frag_len); @@ -478,14 +478,16 @@ fec_enet_txq_submit_frag_skb(struct sk_buff *skb, struct net_device *ndev) bdp->bd.cbd_sc = status; } - fep->cur_tx = bdp; + fep->tx_next = index; return 0; dma_mapping_error: - bdp = fep->cur_tx; + index = fep->tx_next; for (i = 0; i < frag; i++) { - bdp = fec_enet_get_nextdesc(bdp, fep); + if (++index >= fep->tx_ring_size) + index = 0; + bdp = fec_enet_tx_get(index, fep); dma_unmap_single(&fep->pdev->dev, bdp->bd.cbd_bufaddr, bdp->bd.cbd_datlen, DMA_TO_DEVICE); } @@ -498,7 +500,7 @@ static int fec_enet_txq_submit_skb(struct sk_buff *skb, struct net_device *ndev) const struct platform_device_id *id_entry = platform_get_device_id(fep->pdev); int nr_frags = skb_shinfo(skb)->nr_frags; - union bufdesc_u *bdp, *last_bdp; + union bufdesc_u *bdp; void *bufaddr; dma_addr_t addr; unsigned short status; @@ -521,15 +523,17 @@ static int fec_enet_txq_submit_skb(struct sk_buff *skb, struct net_device *ndev) } /* Fill in a Tx ring entry */ - bdp = fep->cur_tx; - status = bdp->bd.cbd_sc; - status &= ~BD_ENET_TX_STATS; + index = fep->tx_next; /* Set buffer length and buffer pointer */ bufaddr = skb->data; buflen = skb_headlen(skb); - index = fec_enet_get_bd_index(fep->tx_bd_base, bdp, fep); + /* + * On some FEC implementations data must be aligned on + * 4-byte boundaries. Use bounce buffers to copy data + * and get it aligned. Ugh. + */ if (((unsigned long) bufaddr) & FEC_ALIGNMENT || id_entry->driver_data & FEC_QUIRK_SWAP_FRAME) { memcpy(fep->tx_bounce[index], skb->data, buflen); @@ -548,6 +552,9 @@ static int fec_enet_txq_submit_skb(struct sk_buff *skb, struct net_device *ndev) return NETDEV_TX_OK; } + bdp = fec_enet_tx_get(index, fep); + status = bdp->bd.cbd_sc & ~BD_ENET_TX_STATS; + if (nr_frags) { ret = fec_enet_txq_submit_frag_skb(skb, ndev); if (ret) @@ -574,8 +581,7 @@ static int fec_enet_txq_submit_skb(struct sk_buff *skb, struct net_device *ndev) bdp->ebd.cbd_esc = estatus; } - last_bdp = fep->cur_tx; - index = fec_enet_get_bd_index(fep->tx_bd_base, last_bdp, fep); + index = fep->tx_next; /* Save skb pointer */ fep->tx_skbuff[index] = skb; @@ -592,16 +598,15 @@ static int fec_enet_txq_submit_skb(struct sk_buff *skb, struct net_device *ndev) /* Send it on its way. Tell FEC it's ready, interrupt when done, * it's the last BD of the frame, and to put the CRC on the end. */ - status |= (BD_ENET_TX_READY | BD_ENET_TX_TC); - bdp->bd.cbd_sc = status; - - /* If this was the last BD in the ring, start at the beginning again. */ - bdp = fec_enet_get_nextdesc(last_bdp, fep); + bdp->bd.cbd_sc = status | BD_ENET_TX_READY | BD_ENET_TX_TC; skb_tx_timestamp(skb); netdev_sent_queue(ndev, skb->len); - fep->cur_tx = bdp; + if (++index >= fep->tx_ring_size) + index = 0; + + fep->tx_next = index; /* Trigger transmission start */ if (readl(fep->hwp + FEC_X_DES_ACTIVE) == 0) @@ -725,9 +730,9 @@ static int fec_enet_txq_submit_tso(struct sk_buff *skb, struct net_device *ndev) struct fec_enet_private *fep = netdev_priv(ndev); int hdr_len = skb_transport_offset(skb) + tcp_hdrlen(skb); int total_len, data_left; - union bufdesc_u *bdp = fep->cur_tx; struct tso_t tso; - unsigned int index = 0; + unsigned int index = fep->tx_next; + unsigned int last_index = index; int ret; if (tso_count_descs(skb) >= fec_enet_get_free_txdesc_num(fep)) { @@ -748,9 +753,10 @@ static int fec_enet_txq_submit_tso(struct sk_buff *skb, struct net_device *ndev) total_len = skb->len - hdr_len; while (total_len > 0) { + union bufdesc_u *bdp; char *hdr; - index = fec_enet_get_bd_index(fep->tx_bd_base, bdp, fep); + bdp = fec_enet_tx_get(index, fep); data_left = min_t(int, skb_shinfo(skb)->gso_size, total_len); total_len -= data_left; @@ -764,9 +770,11 @@ static int fec_enet_txq_submit_tso(struct sk_buff *skb, struct net_device *ndev) while (data_left > 0) { int size; + if (++index >= fep->tx_ring_size) + index = 0; + size = min_t(int, tso.size, data_left); - bdp = fec_enet_get_nextdesc(bdp, fep); - index = fec_enet_get_bd_index(fep->tx_bd_base, bdp, fep); + bdp = fec_enet_tx_get(index, fep); ret = fec_enet_txq_put_data_tso(skb, ndev, bdp, index, tso.data, size, size == data_left, total_len == 0); @@ -777,15 +785,17 @@ static int fec_enet_txq_submit_tso(struct sk_buff *skb, struct net_device *ndev) tso_build_data(skb, &tso, size); } - bdp = fec_enet_get_nextdesc(bdp, fep); + last_index = index; + if (++index >= fep->tx_ring_size) + index = 0; } /* Save skb pointer */ - fep->tx_skbuff[index] = skb; + fep->tx_skbuff[last_index] = skb; skb_tx_timestamp(skb); netdev_sent_queue(ndev, skb->len); - fep->cur_tx = bdp; + fep->tx_next = index; /* Trigger transmission start */ if (readl(fep->hwp + FEC_X_DES_ACTIVE) == 0) @@ -844,12 +854,14 @@ static void fec_enet_bd_init(struct net_device *dev) fep->cur_rx = fep->rx_bd_base; /* ...and the same for transmit */ - bdp = fep->tx_bd_base; - fep->cur_tx = bdp; for (i = 0; i < fep->tx_ring_size; i++) { + bdp = fec_enet_tx_get(i, fep); /* Initialize the BD for every fragment in the page. */ - bdp->bd.cbd_sc = 0; + if (i == fep->tx_ring_size - 1) + bdp->bd.cbd_sc = BD_SC_WRAP; + else + bdp->bd.cbd_sc = 0; if (bdp->bd.cbd_bufaddr) fec_enet_tx_unmap(bdp, fep); bdp->bd.cbd_bufaddr = 0; @@ -857,13 +869,10 @@ static void fec_enet_bd_init(struct net_device *dev) dev_kfree_skb_any(fep->tx_skbuff[i]); fep->tx_skbuff[i] = NULL; } - bdp = fec_enet_get_nextdesc(bdp, fep); } - /* Set the last buffer to wrap */ - bdp = fec_enet_get_prevdesc(bdp, fep); - bdp->bd.cbd_sc |= BD_SC_WRAP; - fep->dirty_tx = bdp; + fep->tx_next = 0; + fep->tx_dirty = fep->tx_ring_size - 1; } /* @@ -1126,37 +1135,35 @@ fec_enet_hwtstamp(struct fec_enet_private *fep, unsigned ts, static void fec_enet_tx(struct net_device *ndev) { - struct fec_enet_private *fep; + struct fec_enet_private *fep = netdev_priv(ndev); union bufdesc_u *bdp; unsigned short status; struct sk_buff *skb; - int index = 0; + unsigned int index = fep->tx_dirty; unsigned int pkts_compl, bytes_compl; - fep = netdev_priv(ndev); - bdp = fep->dirty_tx; - - /* get next bdp of dirty_tx */ - bdp = fec_enet_get_nextdesc(bdp, fep); - pkts_compl = bytes_compl = 0; - while (((status = bdp->bd.cbd_sc) & BD_ENET_TX_READY) == 0) { + do { + if (++index >= fep->tx_ring_size) + index = 0; /* current queue is empty */ - if (bdp == fep->cur_tx) + if (index == fep->tx_next) break; - index = fec_enet_get_bd_index(fep->tx_bd_base, bdp, fep); + bdp = fec_enet_tx_get(index, fep); + + status = bdp->bd.cbd_sc; + if (status & BD_ENET_TX_READY) + break; skb = fep->tx_skbuff[index]; fep->tx_skbuff[index] = NULL; if (!IS_TSO_HEADER(fep, bdp->bd.cbd_bufaddr)) fec_enet_tx_unmap(bdp, fep); bdp->bd.cbd_bufaddr = 0; - if (!skb) { - bdp = fec_enet_get_nextdesc(bdp, fep); + if (!skb) continue; - } /* Check for errors. */ if (status & (BD_ENET_TX_HB | BD_ENET_TX_LC | @@ -1198,16 +1205,13 @@ fec_enet_tx(struct net_device *ndev) /* Free the sk buffer associated with this last transmit */ dev_kfree_skb_any(skb); - fep->dirty_tx = bdp; - - /* Update pointer to next buffer descriptor to be transmitted */ - bdp = fec_enet_get_nextdesc(bdp, fep); - } + fep->tx_dirty = index; + } while (1); netdev_completed_queue(ndev, pkts_compl, bytes_compl); /* ERR006538: Keep the transmitter going */ - if (bdp != fep->cur_tx && readl(fep->hwp + FEC_X_DES_ACTIVE) == 0) + if (index != fep->tx_next && readl(fep->hwp + FEC_X_DES_ACTIVE) == 0) writel(0, fep->hwp + FEC_X_DES_ACTIVE); if (netif_queue_stopped(ndev) && @@ -2253,8 +2257,8 @@ static void fec_enet_free_buffers(struct net_device *ndev) bdp = fec_enet_get_nextdesc(bdp, fep); } - bdp = fep->tx_bd_base; for (i = 0; i < fep->tx_ring_size; i++) { + bdp = fec_enet_tx_get(i, fep); if (bdp->bd.cbd_bufaddr) fec_enet_tx_unmap(bdp, fep); bdp->bd.cbd_bufaddr = 0; @@ -2304,25 +2308,23 @@ static int fec_enet_alloc_buffers(struct net_device *ndev) bdp = fec_enet_get_prevdesc(bdp, fep); bdp->bd.cbd_sc |= BD_SC_WRAP; - bdp = fep->tx_bd_base; for (i = 0; i < fep->tx_ring_size; i++) { + bdp = fec_enet_tx_get(i, fep); fep->tx_bounce[i] = kmalloc(FEC_ENET_TX_FRSIZE, GFP_KERNEL); if (!fep->tx_bounce[i]) goto err_alloc; - bdp->bd.cbd_sc = 0; + /* Set the last buffer to wrap. */ + if (i == fep->tx_ring_size - 1) + bdp->bd.cbd_sc = BD_SC_WRAP; + else + bdp->bd.cbd_sc = 0; bdp->bd.cbd_bufaddr = 0; if (fep->bufdesc_ex) bdp->ebd.cbd_esc = BD_ENET_TX_INT; - - bdp = fec_enet_get_nextdesc(bdp, fep); } - /* Set the last buffer to wrap. */ - bdp = fec_enet_get_prevdesc(bdp, fep); - bdp->bd.cbd_sc |= BD_SC_WRAP; - return 0; err_alloc: From rmk Mon Aug 15 18:57:32 2016 References: Message-ID: From: Russell King To: Linux Kernel Mailing list Date: Mon, 15 Aug 2016 18:57:32 +0100 Content-type: text/plain; charset="utf-8" Content-Disposition: inline Content-Transfer-Encoding: 8bit Mime-Version: 1.0 Subject: Re: [PATCH] net: fec: better indexing for receive descriptor ring From: Russell King Extend the previous commit to the receive descriptor ring as well. This gets rid of the two nextdesc/prevdesc functions, since we now just need to get the descriptor for an index instead. Signed-off-by: Russell King --- drivers/net/ethernet/freescale/fec.h | 2 +- drivers/net/ethernet/freescale/fec_main.c | 102 ++++++++++-------------------- 2 files changed, 34 insertions(+), 70 deletions(-) diff --git a/drivers/net/ethernet/freescale/fec.h b/drivers/net/ethernet/freescale/fec.h index 76fe5a9bb85e..17ab4849802d 100644 --- a/drivers/net/ethernet/freescale/fec.h +++ b/drivers/net/ethernet/freescale/fec.h @@ -298,7 +298,7 @@ struct fec_enet_private { /* The next free ring entry */ unsigned short tx_next; unsigned short tx_dirty; - union bufdesc_u *cur_rx; + unsigned short rx_next; unsigned short tx_ring_size; unsigned short rx_ring_size; diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c index fdd190f4bf0e..b0f218797630 100644 --- a/drivers/net/ethernet/freescale/fec_main.c +++ b/drivers/net/ethernet/freescale/fec_main.c @@ -278,53 +278,20 @@ fec_enet_tx_get(unsigned int index, struct fec_enet_private *fep) return bdp; } -static inline -union bufdesc_u *fec_enet_get_nextdesc(union bufdesc_u *bdp, struct fec_enet_private *fep) -{ - union bufdesc_u *base; - int ring_size; - - base = fep->rx_bd_base; - ring_size = fep->rx_ring_size; - - if (fep->bufdesc_ex) { - struct bufdesc_ex *ebd = &bdp->ebd + 1; - return ebd >= (&base->ebd + ring_size) ? - base : (union bufdesc_u *)ebd; - } else { - struct bufdesc *bd = &bdp->bd + 1; - return bd >= (&base->bd + ring_size) ? - base : (union bufdesc_u *)bd; - } -} - -static inline -union bufdesc_u *fec_enet_get_prevdesc(union bufdesc_u *bdp, struct fec_enet_private *fep) +static union bufdesc_u * +fec_enet_rx_get(unsigned int index, struct fec_enet_private *fep) { - union bufdesc_u *base; - int ring_size; + union bufdesc_u *base = fep->rx_bd_base; + union bufdesc_u *bdp; - base = fep->rx_bd_base; - ring_size = fep->rx_ring_size; + index &= fep->rx_ring_size - 1; - if (fep->bufdesc_ex) { - struct bufdesc_ex *ebd = &bdp->ebd - 1; - return (union bufdesc_u *)(ebd < &base->ebd ? - ebd + ring_size : ebd); - } else { - struct bufdesc *bd = &bdp->bd - 1; - return (union bufdesc_u *)(bd < &base->bd ? - bd + ring_size : bd); - } -} - -static int fec_enet_get_bd_index(union bufdesc_u *base, union bufdesc_u *bdp, - struct fec_enet_private *fep) -{ if (fep->bufdesc_ex) - return &bdp->ebd - &base->ebd; + bdp = (union bufdesc_u *)(&base->ebd + index); else - return &bdp->bd - &base->bd; + bdp = (union bufdesc_u *)(&base->bd + index); + + return bdp; } static unsigned int fec_enet_get_free_txdesc_num(struct fec_enet_private *fep) @@ -836,22 +803,20 @@ static void fec_enet_bd_init(struct net_device *dev) unsigned int i; /* Initialize the receive buffer descriptors. */ - bdp = fep->rx_bd_base; for (i = 0; i < fep->rx_ring_size; i++) { + bdp = fec_enet_rx_get(i, fep); /* Initialize the BD for every fragment in the page. */ if (bdp->bd.cbd_bufaddr) bdp->bd.cbd_sc = BD_ENET_RX_EMPTY; else bdp->bd.cbd_sc = 0; - bdp = fec_enet_get_nextdesc(bdp, fep); - } - /* Set the last buffer to wrap */ - bdp = fec_enet_get_prevdesc(bdp, fep); - bdp->bd.cbd_sc |= BD_SC_WRAP; + if (i == fep->rx_ring_size - 1) + bdp->bd.cbd_sc |= BD_SC_WRAP; + } - fep->cur_rx = fep->rx_bd_base; + fep->rx_next = 0; /* ...and the same for transmit */ for (i = 0; i < fep->tx_ring_size; i++) { @@ -1219,7 +1184,7 @@ fec_enet_tx(struct net_device *ndev) netif_wake_queue(ndev); } -/* During a receive, the cur_rx points to the current incoming buffer. +/* During a receive, the rx_next points to the current incoming buffer. * When we update through the ring, if the next incoming buffer has * not been given to the system, we just set the empty indicator, * effectively tossing the packet. @@ -1230,7 +1195,6 @@ fec_enet_rx(struct net_device *ndev, int budget) struct fec_enet_private *fep = netdev_priv(ndev); const struct platform_device_id *id_entry = platform_get_device_id(fep->pdev); - union bufdesc_u *bdp; unsigned short status; struct sk_buff *skb; ushort pkt_len; @@ -1238,7 +1202,7 @@ fec_enet_rx(struct net_device *ndev, int budget) int pkt_received = 0; bool vlan_packet_rcvd = false; u16 vlan_tag; - int index = 0; + unsigned int index = fep->rx_next; #ifdef CONFIG_M532x flush_cache_all(); @@ -1247,12 +1211,16 @@ fec_enet_rx(struct net_device *ndev, int budget) /* First, grab all of the stats for the incoming packet. * These get messed up if we get called due to a busy condition. */ - bdp = fep->cur_rx; + do { + union bufdesc_u *bdp = fec_enet_rx_get(index, fep); - while (!((status = bdp->bd.cbd_sc) & BD_ENET_RX_EMPTY)) { + status = bdp->bd.cbd_sc; + if (status & BD_ENET_RX_EMPTY) + break; if (pkt_received >= budget) break; + pkt_received++; /* Since we have allocated space to hold a complete frame, @@ -1301,7 +1269,6 @@ fec_enet_rx(struct net_device *ndev, int budget) pkt_len = bdp->bd.cbd_datlen; ndev->stats.rx_bytes += pkt_len; - index = fec_enet_get_bd_index(fep->rx_bd_base, bdp, fep); data = fep->rx_skbuff[index]->data; dma_sync_single_for_cpu(&fep->pdev->dev, bdp->bd.cbd_bufaddr, FEC_ENET_RX_FRSIZE, DMA_FROM_DEVICE); @@ -1392,16 +1359,16 @@ fec_enet_rx(struct net_device *ndev, int budget) status |= BD_ENET_RX_EMPTY; bdp->bd.cbd_sc = status; - /* Update BD pointer to next entry */ - bdp = fec_enet_get_nextdesc(bdp, fep); - /* Doing this here will keep the FEC running while we process * incoming frames. On a heavily loaded network, we should be * able to keep up at the expense of system resources. */ writel(0, fep->hwp + FEC_R_DES_ACTIVE); - } - fep->cur_rx = bdp; + + if (++index >= fep->rx_ring_size) + index = 0; + } while (1); + fep->rx_next = index; return pkt_received; } @@ -2245,8 +2212,9 @@ static void fec_enet_free_buffers(struct net_device *ndev) struct sk_buff *skb; union bufdesc_u *bdp; - bdp = fep->rx_bd_base; for (i = 0; i < fep->rx_ring_size; i++) { + bdp = fec_enet_rx_get(i, fep); + skb = fep->rx_skbuff[i]; fep->rx_skbuff[i] = NULL; if (skb) { @@ -2254,7 +2222,6 @@ static void fec_enet_free_buffers(struct net_device *ndev) FEC_ENET_RX_FRSIZE, DMA_FROM_DEVICE); dev_kfree_skb(skb); } - bdp = fec_enet_get_nextdesc(bdp, fep); } for (i = 0; i < fep->tx_ring_size; i++) { @@ -2277,7 +2244,6 @@ static int fec_enet_alloc_buffers(struct net_device *ndev) struct sk_buff *skb; union bufdesc_u *bdp; - bdp = fep->rx_bd_base; for (i = 0; i < fep->rx_ring_size; i++) { dma_addr_t addr; @@ -2295,19 +2261,17 @@ static int fec_enet_alloc_buffers(struct net_device *ndev) } fep->rx_skbuff[i] = skb; + bdp = fec_enet_rx_get(i, fep); bdp->bd.cbd_bufaddr = addr; bdp->bd.cbd_sc = BD_ENET_RX_EMPTY; + /* Set the last buffer to wrap. */ + if (i == fep->rx_ring_size - 1) + bdp->bd.cbd_sc |= BD_SC_WRAP; if (fep->bufdesc_ex) bdp->ebd.cbd_esc = BD_ENET_RX_INT; - - bdp = fec_enet_get_nextdesc(bdp, fep); } - /* Set the last buffer to wrap. */ - bdp = fec_enet_get_prevdesc(bdp, fep); - bdp->bd.cbd_sc |= BD_SC_WRAP; - for (i = 0; i < fep->tx_ring_size; i++) { bdp = fec_enet_tx_get(i, fep); fep->tx_bounce[i] = kmalloc(FEC_ENET_TX_FRSIZE, GFP_KERNEL); From rmk Mon Aug 15 18:57:32 2016 References: Message-ID: From: Russell King To: Linux Kernel Mailing list Date: Mon, 15 Aug 2016 18:57:32 +0100 Content-type: text/plain; charset="utf-8" Content-Disposition: inline Content-Transfer-Encoding: 8bit Mime-Version: 1.0 Subject: Re: [PATCH] net: fec: adjust ring threshold according to SG setting From: Russell King When SG is enabled, we must ensure that there are up to 1+MAX_SKB_FRAGS free entries in the ring. When SG is disabled, this can be reduced to one, and we can allow smaller rings. Adjust this setting according to the state of SG. Signed-off-by: Russell King --- drivers/net/ethernet/freescale/fec.h | 1 + drivers/net/ethernet/freescale/fec_main.c | 44 +++++++++++++++++++++++++++++-- 2 files changed, 43 insertions(+), 2 deletions(-) diff --git a/drivers/net/ethernet/freescale/fec.h b/drivers/net/ethernet/freescale/fec.h index 17ab4849802d..19210acabcaa 100644 --- a/drivers/net/ethernet/freescale/fec.h +++ b/drivers/net/ethernet/freescale/fec.h @@ -298,6 +298,7 @@ struct fec_enet_private { /* The next free ring entry */ unsigned short tx_next; unsigned short tx_dirty; + unsigned short tx_min; unsigned short rx_next; unsigned short tx_ring_size; diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c index b0f218797630..59e1d15c4d02 100644 --- a/drivers/net/ethernet/freescale/fec_main.c +++ b/drivers/net/ethernet/freescale/fec_main.c @@ -197,6 +197,9 @@ MODULE_PARM_DESC(macaddr, "FEC Ethernet MAC address"); #endif #endif /* CONFIG_M5272 */ +/* Minimum TX ring size when using NETIF_F_SG */ +#define TX_RING_SIZE_MIN_SG (2 * (MAX_SKB_FRAGS + 1)) + /* Interrupt events/masks. */ #define FEC_ENET_HBERR ((uint)0x80000000) /* Heartbeat error */ #define FEC_ENET_BABR ((uint)0x40000000) /* Babbling receiver */ @@ -476,7 +479,7 @@ static int fec_enet_txq_submit_skb(struct sk_buff *skb, struct net_device *ndev) unsigned int index; int ret; - if (fec_enet_get_free_txdesc_num(fep) < MAX_SKB_FRAGS + 1) { + if (fec_enet_get_free_txdesc_num(fep) < fep->tx_min) { dev_kfree_skb_any(skb); if (net_ratelimit()) netdev_err(ndev, "NOT enough BD for SG!\n"); @@ -2471,7 +2474,22 @@ static void fec_poll_controller(struct net_device *dev) } #endif -#define FEATURES_NEED_QUIESCE NETIF_F_RXCSUM +static netdev_features_t fec_fix_features(struct net_device *ndev, + netdev_features_t features) +{ + struct fec_enet_private *fep = netdev_priv(ndev); + + /* + * NETIF_F_SG requires a minimum transmit ring size. If we + * have less than this size, we can't support this feature. + */ + if (fep->tx_ring_size < TX_RING_SIZE_MIN_SG) + features &= ~NETIF_F_SG; + + return features; +} + +#define FEATURES_NEED_QUIESCE (NETIF_F_RXCSUM | NETIF_F_SG) static int fec_set_features(struct net_device *netdev, netdev_features_t features) @@ -2496,6 +2514,17 @@ static int fec_set_features(struct net_device *netdev, fep->csum_flags &= ~FLAG_RX_CSUM_ENABLED; } + /* Set the appropriate minimum transmit ring free threshold */ + if (features & NETIF_F_SG) { + fep->tx_min = MAX_SKB_FRAGS + 1; + fep->tx_stop_threshold = FEC_MAX_SKB_DESCS; + fep->tx_wake_threshold = (fep->tx_ring_size - fep->tx_stop_threshold) / 2; + } else { + fep->tx_min = 1; + fep->tx_stop_threshold = 1; + fep->tx_wake_threshold = 1; + } + /* Resume the device after updates */ if (netif_running(netdev) && changed & FEATURES_NEED_QUIESCE) { fec_restart(netdev); @@ -2520,6 +2549,7 @@ static const struct net_device_ops fec_netdev_ops = { #ifdef CONFIG_NET_POLL_CONTROLLER .ndo_poll_controller = fec_poll_controller, #endif + .ndo_fix_features = fec_fix_features, .ndo_set_features = fec_set_features, }; @@ -2606,6 +2636,16 @@ static int fec_enet_init(struct net_device *ndev) fep->csum_flags |= FLAG_RX_CSUM_ENABLED; } + if (ndev->features & NETIF_F_SG) { + fep->tx_min = MAX_SKB_FRAGS + 1; + fep->tx_stop_threshold = FEC_MAX_SKB_DESCS; + fep->tx_wake_threshold = (fep->tx_ring_size - fep->tx_stop_threshold) / 2; + } else { + fep->tx_min = 1; + fep->tx_stop_threshold = 1; + fep->tx_wake_threshold = 1; + } + ndev->hw_features = ndev->features; fec_restart(ndev); From rmk Mon Aug 15 18:57:32 2016 References: Message-ID: From: Russell King To: Linux Kernel Mailing list Date: Mon, 15 Aug 2016 18:57:32 +0100 Content-type: text/plain; charset="utf-8" Content-Disposition: inline Content-Transfer-Encoding: 8bit Mime-Version: 1.0 Subject: Re: [PATCH] net: fec: consolidate common parts of mdio read/write From: Russell King There is a commonality to fec_enet_mdio_read() and fec_enet_mdio_write() which can be factored out. Factor that commonality out, since we need to add some locking to prevent resets interfering with MDIO accesses. Signed-off-by: Russell King --- drivers/net/ethernet/freescale/fec_main.c | 41 +++++++++++++------------------ 1 file changed, 17 insertions(+), 24 deletions(-) diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c index 59e1d15c4d02..f786485acf63 100644 --- a/drivers/net/ethernet/freescale/fec_main.c +++ b/drivers/net/ethernet/freescale/fec_main.c @@ -1591,23 +1591,27 @@ static void fec_enet_adjust_link(struct net_device *ndev) phy_print_status(phy_dev); } -static int fec_enet_mdio_read(struct mii_bus *bus, int mii_id, int regnum) +static unsigned long fec_enet_mdio_op(struct fec_enet_private *fep, + unsigned data) { - struct fec_enet_private *fep = bus->priv; - unsigned long time_left; - fep->mii_timeout = 0; init_completion(&fep->mdio_done); - /* start a read op */ - writel(FEC_MMFR_ST | FEC_MMFR_OP_READ | - FEC_MMFR_PA(mii_id) | FEC_MMFR_RA(regnum) | - FEC_MMFR_TA, fep->hwp + FEC_MII_DATA); + /* start operation */ + writel(data, fep->hwp + FEC_MII_DATA); /* wait for end of transfer */ - time_left = wait_for_completion_timeout(&fep->mdio_done, + return wait_for_completion_timeout(&fep->mdio_done, usecs_to_jiffies(FEC_MII_TIMEOUT)); - if (time_left == 0) { +} + +static int fec_enet_mdio_read(struct mii_bus *bus, int mii_id, int regnum) +{ + struct fec_enet_private *fep = bus->priv; + + if (fec_enet_mdio_op(fep, FEC_MMFR_ST | FEC_MMFR_OP_READ | + FEC_MMFR_PA(mii_id) | FEC_MMFR_RA(regnum) | + FEC_MMFR_TA) == 0) { fep->mii_timeout = 1; netdev_err(fep->netdev, "MDIO read timeout\n"); return -ETIMEDOUT; @@ -1621,21 +1625,10 @@ static int fec_enet_mdio_write(struct mii_bus *bus, int mii_id, int regnum, u16 value) { struct fec_enet_private *fep = bus->priv; - unsigned long time_left; - - fep->mii_timeout = 0; - init_completion(&fep->mdio_done); - /* start a write op */ - writel(FEC_MMFR_ST | FEC_MMFR_OP_WRITE | - FEC_MMFR_PA(mii_id) | FEC_MMFR_RA(regnum) | - FEC_MMFR_TA | FEC_MMFR_DATA(value), - fep->hwp + FEC_MII_DATA); - - /* wait for end of transfer */ - time_left = wait_for_completion_timeout(&fep->mdio_done, - usecs_to_jiffies(FEC_MII_TIMEOUT)); - if (time_left == 0) { + if (fec_enet_mdio_op(fep, FEC_MMFR_ST | FEC_MMFR_OP_WRITE | + FEC_MMFR_PA(mii_id) | FEC_MMFR_RA(regnum) | + FEC_MMFR_TA | FEC_MMFR_DATA(value)) == 0) { fep->mii_timeout = 1; netdev_err(fep->netdev, "MDIO write timeout\n"); return -ETIMEDOUT; From rmk Mon Aug 15 18:57:32 2016 References: Message-ID: From: Russell King To: Linux Kernel Mailing list Date: Mon, 15 Aug 2016 18:57:32 +0100 Content-type: text/plain; charset="utf-8" Content-Disposition: inline Content-Transfer-Encoding: 8bit Mime-Version: 1.0 Subject: Re: [PATCH] net: fec: add locking for MDIO bus access From: Russell King Both fec_restart() and fec_stop() both perform a reset of the FEC. This reset can result in an in-process MDIO transfer being terminated, which can lead to the MDIO read/write functions timing out. Add some locking to prevent this occuring. We can't use a spinlock for this as the MDIO accessor functions use wait_for_completion_timeout(), which sleeps. Signed-off-by: Russell King --- drivers/net/ethernet/freescale/fec.h | 3 +++ drivers/net/ethernet/freescale/fec_main.c | 30 +++++++++++++++++++++++++++++- 2 files changed, 32 insertions(+), 1 deletion(-) diff --git a/drivers/net/ethernet/freescale/fec.h b/drivers/net/ethernet/freescale/fec.h index 19210acabcaa..1757aa7eb473 100644 --- a/drivers/net/ethernet/freescale/fec.h +++ b/drivers/net/ethernet/freescale/fec.h @@ -14,6 +14,7 @@ /****************************************************************************/ #include +#include #include #include @@ -310,6 +311,8 @@ struct fec_enet_private { char *tso_hdrs; dma_addr_t tso_hdrs_dma; + struct mutex mutex; + struct platform_device *pdev; int dev_id; diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c index f786485acf63..f67e83967a15 100644 --- a/drivers/net/ethernet/freescale/fec_main.c +++ b/drivers/net/ethernet/freescale/fec_main.c @@ -1075,12 +1075,14 @@ static void fec_enet_timeout_work(struct work_struct *work) rtnl_lock(); if (netif_device_present(ndev) || netif_running(ndev)) { + mutex_lock(&fep->mutex); napi_disable(&fep->napi); netif_tx_lock_bh(ndev); fec_restart(ndev); netif_wake_queue(ndev); netif_tx_unlock_bh(ndev); napi_enable(&fep->napi); + mutex_unlock(&fep->mutex); } rtnl_unlock(); } @@ -1568,20 +1570,24 @@ static void fec_enet_adjust_link(struct net_device *ndev) /* if any of the above changed restart the FEC */ if (status_change) { + mutex_lock(&fep->mutex); napi_disable(&fep->napi); netif_tx_lock_bh(ndev); fec_restart(ndev); netif_wake_queue(ndev); netif_tx_unlock_bh(ndev); napi_enable(&fep->napi); + mutex_unlock(&fep->mutex); } } else { if (fep->link) { + mutex_lock(&fep->mutex); napi_disable(&fep->napi); netif_tx_lock_bh(ndev); fec_stop(ndev); netif_tx_unlock_bh(ndev); napi_enable(&fep->napi); + mutex_unlock(&fep->mutex); fep->link = phy_dev->link; status_change = 1; } @@ -1594,15 +1600,23 @@ static void fec_enet_adjust_link(struct net_device *ndev) static unsigned long fec_enet_mdio_op(struct fec_enet_private *fep, unsigned data) { + unsigned long time_left; + fep->mii_timeout = 0; init_completion(&fep->mdio_done); + mutex_lock(&fep->mutex); + /* start operation */ writel(data, fep->hwp + FEC_MII_DATA); /* wait for end of transfer */ - return wait_for_completion_timeout(&fep->mdio_done, + time_left = wait_for_completion_timeout(&fep->mdio_done, usecs_to_jiffies(FEC_MII_TIMEOUT)); + + mutex_unlock(&fep->mutex); + + return time_left; } static int fec_enet_mdio_read(struct mii_bus *bus, int mii_id, int regnum) @@ -2039,6 +2053,7 @@ static int fec_enet_set_pauseparam(struct net_device *ndev, fep->pause_mode = fep->pause_flag; if (netif_running(ndev)) { + mutex_lock(&fep->mutex); napi_disable(&fep->napi); netif_tx_lock_bh(ndev); fec_stop(ndev); @@ -2046,6 +2061,7 @@ static int fec_enet_set_pauseparam(struct net_device *ndev, netif_wake_queue(ndev); netif_tx_unlock_bh(ndev); napi_enable(&fep->napi); + mutex_unlock(&fep->mutex); } } } @@ -2318,7 +2334,9 @@ fec_enet_open(struct net_device *ndev) return ret; } + mutex_lock(&fep->mutex); fec_restart(ndev); + mutex_unlock(&fep->mutex); napi_enable(&fep->napi); phy_start(fep->phy_dev); netif_start_queue(ndev); @@ -2335,7 +2353,9 @@ fec_enet_close(struct net_device *ndev) if (netif_device_present(ndev)) { napi_disable(&fep->napi); netif_tx_disable(ndev); + mutex_lock(&fep->mutex); fec_stop(ndev); + mutex_unlock(&fep->mutex); } phy_disconnect(fep->phy_dev); @@ -2492,6 +2512,7 @@ static int fec_set_features(struct net_device *netdev, /* Quiesce the device if necessary */ if (netif_running(netdev) && changed & FEATURES_NEED_QUIESCE) { + mutex_lock(&fep->mutex); napi_disable(&fep->napi); netif_tx_lock_bh(netdev); fec_stop(netdev); @@ -2524,6 +2545,7 @@ static int fec_set_features(struct net_device *netdev, netif_wake_queue(netdev); netif_tx_unlock_bh(netdev); napi_enable(&fep->napi); + mutex_unlock(&fep->mutex); } return 0; @@ -2710,6 +2732,8 @@ fec_probe(struct platform_device *pdev) /* setup board info structure */ fep = netdev_priv(ndev); + mutex_init(&fep->mutex); + #if !defined(CONFIG_M5272) /* default enable pause frame auto negotiation */ if (pdev->id_entry && @@ -2895,7 +2919,9 @@ static int __maybe_unused fec_suspend(struct device *dev) netif_tx_lock_bh(ndev); netif_device_detach(ndev); netif_tx_unlock_bh(ndev); + mutex_lock(&fep->mutex); fec_stop(ndev); + mutex_unlock(&fep->mutex); } rtnl_unlock(); @@ -2927,7 +2953,9 @@ static int __maybe_unused fec_resume(struct device *dev) rtnl_lock(); if (netif_running(ndev)) { + mutex_lock(&fep->mutex); fec_restart(ndev); + mutex_unlock(&fep->mutex); netif_tx_lock_bh(ndev); netif_device_attach(ndev); netif_tx_unlock_bh(ndev); From rmk Mon Aug 15 18:57:32 2016 References: Message-ID: From: Russell King To: Linux Kernel Mailing list Date: Mon, 15 Aug 2016 18:57:32 +0100 Content-type: text/plain; charset="utf-8" Content-Disposition: inline Content-Transfer-Encoding: 8bit Mime-Version: 1.0 Subject: Re: [PATCH] net: fec: move bufdesc_ex flag into flags From: Russell King Add a new flags field to contain the mostly static driver configuration, the first of which is the bufdesc_ex flag. Signed-off-by: Russell King --- drivers/net/ethernet/freescale/fec.h | 3 +- drivers/net/ethernet/freescale/fec_main.c | 65 +++++++++++++++++-------------- 2 files changed, 37 insertions(+), 31 deletions(-) diff --git a/drivers/net/ethernet/freescale/fec.h b/drivers/net/ethernet/freescale/fec.h index 1757aa7eb473..34302a2bb5a1 100644 --- a/drivers/net/ethernet/freescale/fec.h +++ b/drivers/net/ethernet/freescale/fec.h @@ -311,6 +311,8 @@ struct fec_enet_private { char *tso_hdrs; dma_addr_t tso_hdrs_dma; + unsigned char flags; + struct mutex mutex; struct platform_device *pdev; @@ -329,7 +331,6 @@ struct fec_enet_private { int speed; struct completion mdio_done; int irq[FEC_IRQ_NUM]; - int bufdesc_ex; unsigned short pause_flag; unsigned short pause_mode; diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c index f67e83967a15..5ed94160f9ed 100644 --- a/drivers/net/ethernet/freescale/fec_main.c +++ b/drivers/net/ethernet/freescale/fec_main.c @@ -252,10 +252,14 @@ MODULE_PARM_DESC(macaddr, "FEC Ethernet MAC address"); /* Transmitter timeout */ #define TX_TIMEOUT (2 * HZ) +/* pause mode/flag */ #define FEC_PAUSE_FLAG_AUTONEG BIT(0) #define FEC_PAUSE_FLAG_RX BIT(1) #define FEC_PAUSE_FLAG_TX BIT(2) +/* flags */ +#define FEC_FLAG_BUFDESC_EX BIT(0) + #define TSO_HEADER_SIZE 128 /* Max number of allowed TCP segments for software TSO */ #define FEC_MAX_TSO_SEGS 100 @@ -273,7 +277,7 @@ fec_enet_tx_get(unsigned int index, struct fec_enet_private *fep) union bufdesc_u *base = fep->tx_bd_base; union bufdesc_u *bdp; - if (fep->bufdesc_ex) + if (fep->flags & FEC_FLAG_BUFDESC_EX) bdp = (union bufdesc_u *)(&base->ebd + index); else bdp = (union bufdesc_u *)(&base->bd + index); @@ -289,7 +293,7 @@ fec_enet_rx_get(unsigned int index, struct fec_enet_private *fep) index &= fep->rx_ring_size - 1; - if (fep->bufdesc_ex) + if (fep->flags & FEC_FLAG_BUFDESC_EX) bdp = (union bufdesc_u *)(&base->ebd + index); else bdp = (union bufdesc_u *)(&base->bd + index); @@ -408,7 +412,7 @@ fec_enet_txq_submit_frag_skb(struct sk_buff *skb, struct net_device *ndev) /* Handle the last BD specially */ if (frag == nr_frags - 1) { status |= (BD_ENET_TX_INTR | BD_ENET_TX_LAST); - if (fep->bufdesc_ex) { + if (fep->flags & FEC_FLAG_BUFDESC_EX) { estatus |= BD_ENET_TX_INT; if (unlikely(skb_shinfo(skb)->tx_flags & SKBTX_HW_TSTAMP && fep->hwts_tx_en)) @@ -416,7 +420,7 @@ fec_enet_txq_submit_frag_skb(struct sk_buff *skb, struct net_device *ndev) } } - if (fep->bufdesc_ex) { + if (fep->flags & FEC_FLAG_BUFDESC_EX) { if (skb->ip_summed == CHECKSUM_PARTIAL) estatus |= BD_ENET_TX_PINS | BD_ENET_TX_IINS; bdp->ebd.cbd_bdu = 0; @@ -531,7 +535,7 @@ static int fec_enet_txq_submit_skb(struct sk_buff *skb, struct net_device *ndev) return ret; } else { status |= (BD_ENET_TX_INTR | BD_ENET_TX_LAST); - if (fep->bufdesc_ex) { + if (fep->flags & FEC_FLAG_BUFDESC_EX) { estatus = BD_ENET_TX_INT; if (unlikely(skb_shinfo(skb)->tx_flags & SKBTX_HW_TSTAMP && fep->hwts_tx_en)) @@ -539,7 +543,7 @@ static int fec_enet_txq_submit_skb(struct sk_buff *skb, struct net_device *ndev) } } - if (fep->bufdesc_ex) { + if (fep->flags & FEC_FLAG_BUFDESC_EX) { if (unlikely(skb_shinfo(skb)->tx_flags & SKBTX_HW_TSTAMP && fep->hwts_tx_en)) skb_shinfo(skb)->tx_flags |= SKBTX_IN_PROGRESS; @@ -622,7 +626,7 @@ fec_enet_txq_put_data_tso(struct sk_buff *skb, struct net_device *ndev, bdp->bd.cbd_datlen = size; bdp->bd.cbd_bufaddr = addr; - if (fep->bufdesc_ex) { + if (fep->flags & FEC_FLAG_BUFDESC_EX) { if (skb->ip_summed == CHECKSUM_PARTIAL) estatus |= BD_ENET_TX_PINS | BD_ENET_TX_IINS; bdp->ebd.cbd_bdu = 0; @@ -634,7 +638,7 @@ fec_enet_txq_put_data_tso(struct sk_buff *skb, struct net_device *ndev, status |= (BD_ENET_TX_LAST | BD_ENET_TX_TC); if (is_last) { status |= BD_ENET_TX_INTR; - if (fep->bufdesc_ex) + if (fep->flags & FEC_FLAG_BUFDESC_EX) bdp->ebd.cbd_esc |= BD_ENET_TX_INT; } @@ -683,7 +687,7 @@ fec_enet_txq_put_hdr_tso(struct sk_buff *skb, struct net_device *ndev, bdp->bd.cbd_bufaddr = dmabuf; bdp->bd.cbd_datlen = hdr_len; - if (fep->bufdesc_ex) { + if (fep->flags & FEC_FLAG_BUFDESC_EX) { if (skb->ip_summed == CHECKSUM_PARTIAL) estatus |= BD_ENET_TX_PINS | BD_ENET_TX_IINS; bdp->ebd.cbd_bdu = 0; @@ -1006,7 +1010,7 @@ fec_restart(struct net_device *ndev) writel(1 << 8, fep->hwp + FEC_X_WMRK); } - if (fep->bufdesc_ex) + if (fep->flags & FEC_FLAG_BUFDESC_EX) ecntl |= (1 << 4); #ifndef CONFIG_M5272 @@ -1018,7 +1022,7 @@ fec_restart(struct net_device *ndev) writel(ecntl, fep->hwp + FEC_ECNTRL); writel(0, fep->hwp + FEC_R_DES_ACTIVE); - if (fep->bufdesc_ex) + if (fep->flags & FEC_FLAG_BUFDESC_EX) fec_ptp_start_cyclecounter(ndev); /* Enable interrupts we wish to service */ @@ -1155,8 +1159,8 @@ fec_enet_tx(struct net_device *ndev) ndev->stats.tx_bytes += skb->len; } - if (unlikely(skb_shinfo(skb)->tx_flags & SKBTX_IN_PROGRESS) && - fep->bufdesc_ex) { + if (fep->flags & FEC_FLAG_BUFDESC_EX && + unlikely(skb_shinfo(skb)->tx_flags & SKBTX_IN_PROGRESS)) { struct skb_shared_hwtstamps shhwtstamps; fec_enet_hwtstamp(fep, bdp->ebd.ts, &shhwtstamps); @@ -1283,8 +1287,9 @@ fec_enet_rx(struct net_device *ndev, int budget) /* If this is a VLAN packet remove the VLAN Tag */ vlan_packet_rcvd = false; - if ((ndev->features & NETIF_F_HW_VLAN_CTAG_RX) && - fep->bufdesc_ex && (bdp->ebd.cbd_esc & BD_ENET_RX_VLAN)) { + if (ndev->features & NETIF_F_HW_VLAN_CTAG_RX && + fep->flags & FEC_FLAG_BUFDESC_EX && + bdp->ebd.cbd_esc & BD_ENET_RX_VLAN) { /* Push and remove the vlan tag */ struct vlan_hdr *vlan_header = (struct vlan_hdr *) (data + ETH_HLEN); @@ -1319,11 +1324,11 @@ fec_enet_rx(struct net_device *ndev, int budget) skb->protocol = eth_type_trans(skb, ndev); /* Get receive timestamp from the skb */ - if (fep->hwts_rx_en && fep->bufdesc_ex) + if (fep->hwts_rx_en && fep->flags & FEC_FLAG_BUFDESC_EX) fec_enet_hwtstamp(fep, bdp->ebd.ts, skb_hwtstamps(skb)); - if (fep->bufdesc_ex && + if (fep->flags & FEC_FLAG_BUFDESC_EX && (fep->csum_flags & FLAG_RX_CSUM_ENABLED)) { if (!(bdp->ebd.cbd_esc & FLAG_RX_CSUM_ERROR)) { /* don't check it */ @@ -1345,7 +1350,7 @@ fec_enet_rx(struct net_device *ndev, int budget) dma_sync_single_for_device(&fep->pdev->dev, bdp->bd.cbd_bufaddr, FEC_ENET_RX_FRSIZE, DMA_FROM_DEVICE); rx_processing_done: - if (fep->bufdesc_ex) { + if (fep->flags & FEC_FLAG_BUFDESC_EX) { bdp->ebd.cbd_esc = BD_ENET_RX_INT; bdp->ebd.cbd_prot = 0; bdp->ebd.cbd_bdu = 0; @@ -1958,7 +1963,7 @@ static int fec_enet_get_ts_info(struct net_device *ndev, { struct fec_enet_private *fep = netdev_priv(ndev); - if (fep->bufdesc_ex) { + if (fep->flags & FEC_FLAG_BUFDESC_EX) { info->so_timestamping = SOF_TIMESTAMPING_TX_SOFTWARE | SOF_TIMESTAMPING_RX_SOFTWARE | @@ -2207,7 +2212,7 @@ static int fec_enet_ioctl(struct net_device *ndev, struct ifreq *rq, int cmd) if (!phydev) return -ENODEV; - if (fep->bufdesc_ex) { + if (fep->flags & FEC_FLAG_BUFDESC_EX) { if (cmd == SIOCSHWTSTAMP) return fec_ptp_set(ndev, rq); if (cmd == SIOCGHWTSTAMP) @@ -2280,7 +2285,7 @@ static int fec_enet_alloc_buffers(struct net_device *ndev) if (i == fep->rx_ring_size - 1) bdp->bd.cbd_sc |= BD_SC_WRAP; - if (fep->bufdesc_ex) + if (fep->flags & FEC_FLAG_BUFDESC_EX) bdp->ebd.cbd_esc = BD_ENET_RX_INT; } @@ -2297,7 +2302,7 @@ static int fec_enet_alloc_buffers(struct net_device *ndev) bdp->bd.cbd_sc = 0; bdp->bd.cbd_bufaddr = 0; - if (fep->bufdesc_ex) + if (fep->flags & FEC_FLAG_BUFDESC_EX) bdp->ebd.cbd_esc = BD_ENET_TX_INT; } @@ -2589,7 +2594,7 @@ static int fec_enet_init(struct net_device *ndev) fep->tx_wake_threshold = (fep->tx_ring_size - fep->tx_stop_threshold) / 2; bd_size = fep->tx_ring_size + fep->rx_ring_size; - if (fep->bufdesc_ex) + if (fep->flags & FEC_FLAG_BUFDESC_EX) bd_size *= sizeof(struct bufdesc_ex); else bd_size *= sizeof(struct bufdesc); @@ -2618,7 +2623,7 @@ static int fec_enet_init(struct net_device *ndev) /* Set receive and transmit descriptor base. */ fep->rx_bd_base = cbd_base; fep->rx_bd_dma = cbd_dma; - if (fep->bufdesc_ex) { + if (fep->flags & FEC_FLAG_BUFDESC_EX) { fep->tx_bd_base = (union bufdesc_u *) (&cbd_base->ebd + fep->rx_ring_size); fep->tx_bd_dma = cbd_dma + sizeof(struct bufdesc_ex) * @@ -2756,7 +2761,9 @@ fec_probe(struct platform_device *pdev) fep->pdev = pdev; fep->dev_id = dev_id++; - fep->bufdesc_ex = 0; + fep->flags = 0; + if (pdev->id_entry->driver_data & FEC_QUIRK_HAS_BUFDESC_EX) + fep->flags |= FEC_FLAG_BUFDESC_EX; platform_set_drvdata(pdev, ndev); @@ -2803,11 +2810,9 @@ fec_probe(struct platform_device *pdev) fep->ptp_clk_on = false; mutex_init(&fep->ptp_clk_mutex); fep->clk_ptp = devm_clk_get(&pdev->dev, "ptp"); - fep->bufdesc_ex = - pdev->id_entry->driver_data & FEC_QUIRK_HAS_BUFDESC_EX; if (IS_ERR(fep->clk_ptp)) { fep->clk_ptp = NULL; - fep->bufdesc_ex = 0; + fep->flags &= ~FEC_FLAG_BUFDESC_EX; } ret = fec_enet_clk_enable(ndev, true); @@ -2828,7 +2833,7 @@ fec_probe(struct platform_device *pdev) fec_reset_phy(pdev); - if (fep->bufdesc_ex) + if (fep->flags & FEC_FLAG_BUFDESC_EX) fec_ptp_init(pdev); ret = fec_enet_init(ndev); @@ -2862,7 +2867,7 @@ fec_probe(struct platform_device *pdev) if (ret) goto failed_register; - if (fep->bufdesc_ex && fep->ptp_clock) + if (fep->flags & FEC_FLAG_BUFDESC_EX && fep->ptp_clock) netdev_info(ndev, "registered PHC device %d\n", fep->dev_id); INIT_WORK(&fep->tx_timeout_work, fec_enet_timeout_work); From rmk Mon Aug 15 18:57:32 2016 References: Message-ID: From: Russell King To: Linux Kernel Mailing list Date: Mon, 15 Aug 2016 18:57:32 +0100 Content-type: text/plain; charset="utf-8" Content-Disposition: inline Content-Transfer-Encoding: 8bit Mime-Version: 1.0 Subject: Re: [PATCH] net: fec: move receive checksum flags into new flags member From: Russell King The receive checksum flag is only changed upon ethtool configuration, so move it into the flags member. In any case, this is checked alongside the BUFDESC_EX flag. Signed-off-by: Russell King --- drivers/net/ethernet/freescale/fec.h | 2 -- drivers/net/ethernet/freescale/fec_main.c | 11 ++++++----- 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/drivers/net/ethernet/freescale/fec.h b/drivers/net/ethernet/freescale/fec.h index 34302a2bb5a1..6f576a3cc204 100644 --- a/drivers/net/ethernet/freescale/fec.h +++ b/drivers/net/ethernet/freescale/fec.h @@ -260,7 +260,6 @@ union bufdesc_u { #define BD_ENET_RX_PTP ((ushort)0x0400) #define BD_ENET_RX_ICE 0x00000020 #define BD_ENET_RX_PCR 0x00000010 -#define FLAG_RX_CSUM_ENABLED (BD_ENET_RX_ICE | BD_ENET_RX_PCR) #define FLAG_RX_CSUM_ERROR (BD_ENET_RX_ICE | BD_ENET_RX_PCR) /* The FEC buffer descriptors track the ring buffers. The rx_bd_base and @@ -335,7 +334,6 @@ struct fec_enet_private { unsigned short pause_mode; struct napi_struct napi; - int csum_flags; struct work_struct tx_timeout_work; diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c index 5ed94160f9ed..7f99cc6bd868 100644 --- a/drivers/net/ethernet/freescale/fec_main.c +++ b/drivers/net/ethernet/freescale/fec_main.c @@ -259,6 +259,7 @@ MODULE_PARM_DESC(macaddr, "FEC Ethernet MAC address"); /* flags */ #define FEC_FLAG_BUFDESC_EX BIT(0) +#define FEC_FLAG_RX_CSUM BIT(1) #define TSO_HEADER_SIZE 128 /* Max number of allowed TCP segments for software TSO */ @@ -914,7 +915,7 @@ fec_restart(struct net_device *ndev) #if !defined(CONFIG_M5272) /* set RX checksum */ val = readl(fep->hwp + FEC_RACC); - if (fep->csum_flags & FLAG_RX_CSUM_ENABLED) + if (fep->flags & FEC_FLAG_RX_CSUM) val |= FEC_RACC_OPTIONS; else val &= ~FEC_RACC_OPTIONS; @@ -1329,7 +1330,7 @@ fec_enet_rx(struct net_device *ndev, int budget) skb_hwtstamps(skb)); if (fep->flags & FEC_FLAG_BUFDESC_EX && - (fep->csum_flags & FLAG_RX_CSUM_ENABLED)) { + fep->flags & FEC_FLAG_RX_CSUM) { if (!(bdp->ebd.cbd_esc & FLAG_RX_CSUM_ERROR)) { /* don't check it */ skb->ip_summed = CHECKSUM_UNNECESSARY; @@ -2528,9 +2529,9 @@ static int fec_set_features(struct net_device *netdev, /* Receive checksum has been changed */ if (changed & NETIF_F_RXCSUM) { if (features & NETIF_F_RXCSUM) - fep->csum_flags |= FLAG_RX_CSUM_ENABLED; + fep->flags |= FEC_FLAG_RX_CSUM; else - fep->csum_flags &= ~FLAG_RX_CSUM_ENABLED; + fep->flags &= ~FEC_FLAG_RX_CSUM; } /* Set the appropriate minimum transmit ring free threshold */ @@ -2653,7 +2654,7 @@ static int fec_enet_init(struct net_device *ndev) /* enable hw accelerator */ ndev->features |= (NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM | NETIF_F_RXCSUM | NETIF_F_SG | NETIF_F_TSO); - fep->csum_flags |= FLAG_RX_CSUM_ENABLED; + fep->flags |= FEC_FLAG_RX_CSUM; } if (ndev->features & NETIF_F_SG) { From rmk Mon Aug 15 18:57:32 2016 References: Message-ID: From: Russell King To: Linux Kernel Mailing list Date: Mon, 15 Aug 2016 18:57:32 +0100 Content-type: text/plain; charset="utf-8" Content-Disposition: inline Content-Transfer-Encoding: 8bit Mime-Version: 1.0 Subject: Re: [PATCH] net: fec: move vlan receive flag into our private flags From: Russell King Signed-off-by: Russell King --- drivers/net/ethernet/freescale/fec_main.c | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c index 7f99cc6bd868..cc8ad8134bc6 100644 --- a/drivers/net/ethernet/freescale/fec_main.c +++ b/drivers/net/ethernet/freescale/fec_main.c @@ -260,6 +260,7 @@ MODULE_PARM_DESC(macaddr, "FEC Ethernet MAC address"); /* flags */ #define FEC_FLAG_BUFDESC_EX BIT(0) #define FEC_FLAG_RX_CSUM BIT(1) +#define FEC_FLAG_RX_VLAN BIT(2) #define TSO_HEADER_SIZE 128 /* Max number of allowed TCP segments for software TSO */ @@ -1288,7 +1289,7 @@ fec_enet_rx(struct net_device *ndev, int budget) /* If this is a VLAN packet remove the VLAN Tag */ vlan_packet_rcvd = false; - if (ndev->features & NETIF_F_HW_VLAN_CTAG_RX && + if (fep->flags & FEC_FLAG_RX_VLAN && fep->flags & FEC_FLAG_BUFDESC_EX && bdp->ebd.cbd_esc & BD_ENET_RX_VLAN) { /* Push and remove the vlan tag */ @@ -2534,6 +2535,13 @@ static int fec_set_features(struct net_device *netdev, fep->flags &= ~FEC_FLAG_RX_CSUM; } + if (changed & NETIF_F_HW_VLAN_CTAG_RX) { + if (features & NETIF_F_HW_VLAN_CTAG_RX) + fep->flags |= FEC_FLAG_RX_VLAN; + else + fep->flags &= ~FEC_FLAG_RX_VLAN; + } + /* Set the appropriate minimum transmit ring free threshold */ if (features & NETIF_F_SG) { fep->tx_min = MAX_SKB_FRAGS + 1; @@ -2644,9 +2652,11 @@ static int fec_enet_init(struct net_device *ndev) writel(FEC_RX_DISABLED_IMASK, fep->hwp + FEC_IMASK); netif_napi_add(ndev, &fep->napi, fec_enet_rx_napi, NAPI_POLL_WEIGHT); - if (id_entry->driver_data & FEC_QUIRK_HAS_VLAN) + if (id_entry->driver_data & FEC_QUIRK_HAS_VLAN) { /* enable hw VLAN support */ ndev->features |= NETIF_F_HW_VLAN_CTAG_RX; + fep->flags |= FEC_FLAG_RX_VLAN; + } if (id_entry->driver_data & FEC_QUIRK_HAS_CSUM) { ndev->gso_max_segs = FEC_MAX_TSO_SEGS; From rmk Mon Aug 15 18:57:32 2016 References: Message-ID: From: Russell King To: Linux Kernel Mailing list Date: Mon, 15 Aug 2016 18:57:32 +0100 Content-type: text/plain; charset="utf-8" Content-Disposition: inline Content-Transfer-Encoding: 8bit Mime-Version: 1.0 Subject: Re: [PATCH] net: fec: only enable CTAG_RX and IP checksumming if we have enhanced buffer descriptors From: Russell King Only set the vlan tag and ip checksumming options if we have enhanced buffer descriptors - enhanced descriptors are required for these features. Signed-off-by: Russell King --- drivers/net/ethernet/freescale/fec_main.c | 25 ++++++++++++++----------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c index cc8ad8134bc6..2c8187950491 100644 --- a/drivers/net/ethernet/freescale/fec_main.c +++ b/drivers/net/ethernet/freescale/fec_main.c @@ -2652,19 +2652,22 @@ static int fec_enet_init(struct net_device *ndev) writel(FEC_RX_DISABLED_IMASK, fep->hwp + FEC_IMASK); netif_napi_add(ndev, &fep->napi, fec_enet_rx_napi, NAPI_POLL_WEIGHT); - if (id_entry->driver_data & FEC_QUIRK_HAS_VLAN) { - /* enable hw VLAN support */ - ndev->features |= NETIF_F_HW_VLAN_CTAG_RX; - fep->flags |= FEC_FLAG_RX_VLAN; - } + if (fep->flags & FEC_FLAG_BUFDESC_EX) { + if (id_entry->driver_data & FEC_QUIRK_HAS_VLAN) { + /* enable hw VLAN support */ + ndev->features |= NETIF_F_HW_VLAN_CTAG_RX; + fep->flags |= FEC_FLAG_RX_VLAN; + } - if (id_entry->driver_data & FEC_QUIRK_HAS_CSUM) { - ndev->gso_max_segs = FEC_MAX_TSO_SEGS; + if (id_entry->driver_data & FEC_QUIRK_HAS_CSUM) { + ndev->gso_max_segs = FEC_MAX_TSO_SEGS; - /* enable hw accelerator */ - ndev->features |= (NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM - | NETIF_F_RXCSUM | NETIF_F_SG | NETIF_F_TSO); - fep->flags |= FEC_FLAG_RX_CSUM; + /* enable hw accelerator */ + ndev->features |= (NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM + | NETIF_F_RXCSUM | NETIF_F_SG | + NETIF_F_TSO); + fep->flags |= FEC_FLAG_RX_CSUM; + } } if (ndev->features & NETIF_F_SG) { From rmk Mon Aug 15 18:57:32 2016 References: Message-ID: From: Russell King To: Linux Kernel Mailing list Date: Mon, 15 Aug 2016 18:57:32 +0100 Content-type: text/plain; charset="utf-8" Content-Disposition: inline Content-Transfer-Encoding: 8bit Mime-Version: 1.0 Subject: Re: [PATCH] net: fec: avoid checking more flags than necessary in rx path From: Russell King Avoid checking for the enhanced buffer descriptor flag and the receive checksumming/vlan flags - we now only set these feature flags if we already know that we have enhanced buffer descriptors. Signed-off-by: Russell King --- drivers/net/ethernet/freescale/fec_main.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c index 2c8187950491..781b2c5a5abe 100644 --- a/drivers/net/ethernet/freescale/fec_main.c +++ b/drivers/net/ethernet/freescale/fec_main.c @@ -1290,7 +1290,6 @@ fec_enet_rx(struct net_device *ndev, int budget) /* If this is a VLAN packet remove the VLAN Tag */ vlan_packet_rcvd = false; if (fep->flags & FEC_FLAG_RX_VLAN && - fep->flags & FEC_FLAG_BUFDESC_EX && bdp->ebd.cbd_esc & BD_ENET_RX_VLAN) { /* Push and remove the vlan tag */ struct vlan_hdr *vlan_header = @@ -1330,8 +1329,7 @@ fec_enet_rx(struct net_device *ndev, int budget) fec_enet_hwtstamp(fep, bdp->ebd.ts, skb_hwtstamps(skb)); - if (fep->flags & FEC_FLAG_BUFDESC_EX && - fep->flags & FEC_FLAG_RX_CSUM) { + if (fep->flags & FEC_FLAG_RX_CSUM) { if (!(bdp->ebd.cbd_esc & FLAG_RX_CSUM_ERROR)) { /* don't check it */ skb->ip_summed = CHECKSUM_UNNECESSARY; From rmk Mon Aug 15 18:57:32 2016 References: Message-ID: From: Russell King To: Linux Kernel Mailing list Date: Mon, 15 Aug 2016 18:57:32 +0100 Content-type: text/plain; charset="utf-8" Content-Disposition: inline Content-Transfer-Encoding: 8bit Mime-Version: 1.0 Subject: Re: [PATCH] net: fec: move removal of FCS from packet length to single location From: Russell King Rather than keep subtracting four off the packet length throughout the receive path, do it at the point we read the packet length from the descriptor. Signed-off-by: Russell King --- drivers/net/ethernet/freescale/fec_main.c | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c index 781b2c5a5abe..e8a241754a46 100644 --- a/drivers/net/ethernet/freescale/fec_main.c +++ b/drivers/net/ethernet/freescale/fec_main.c @@ -1277,7 +1277,13 @@ fec_enet_rx(struct net_device *ndev, int budget) /* Process the incoming frame. */ ndev->stats.rx_packets++; - pkt_len = bdp->bd.cbd_datlen; + + /* + * The packet length includes FCS, but we don't want + * to include that when passing upstream as it messes + * up bridging applications. + */ + pkt_len = bdp->bd.cbd_datlen - 4; ndev->stats.rx_bytes += pkt_len; data = fep->rx_skbuff[index]->data; @@ -1301,18 +1307,15 @@ fec_enet_rx(struct net_device *ndev, int budget) } /* This does 16 byte alignment, exactly what we need. - * The packet length includes FCS, but we don't want to - * include that when passing upstream as it messes up - * bridging applications. */ - skb = netdev_alloc_skb(ndev, pkt_len - 4 + NET_IP_ALIGN); + skb = netdev_alloc_skb(ndev, pkt_len + NET_IP_ALIGN); if (unlikely(!skb)) { ndev->stats.rx_dropped++; } else { int payload_offset = (2 * ETH_ALEN); skb_reserve(skb, NET_IP_ALIGN); - skb_put(skb, pkt_len - 4); /* Make room */ + skb_put(skb, pkt_len); /* Make room */ /* Extract the frame data without the VLAN header. */ skb_copy_to_linear_data(skb, data, (2 * ETH_ALEN)); @@ -1320,7 +1323,7 @@ fec_enet_rx(struct net_device *ndev, int budget) payload_offset = (2 * ETH_ALEN) + VLAN_HLEN; skb_copy_to_linear_data_offset(skb, (2 * ETH_ALEN), data + payload_offset, - pkt_len - 4 - (2 * ETH_ALEN)); + pkt_len - (2 * ETH_ALEN)); skb->protocol = eth_type_trans(skb, ndev); From rmk Mon Aug 15 18:57:32 2016 References: Message-ID: From: Russell King To: Linux Kernel Mailing list Date: Mon, 15 Aug 2016 18:57:32 +0100 Content-type: text/plain; charset="utf-8" Content-Disposition: inline Content-Transfer-Encoding: 8bit Mime-Version: 1.0 Subject: Re: [PATCH] net:fec: add ethtool support for tx/rx ring sizes From: Russell King Add ethtool support to retrieve and set the transmit and receive ring sizes. This allows runtime tuning of these parameters, which can result in better throughput with gigabit parts. Signed-off-by: Russell King --- drivers/net/ethernet/freescale/fec_main.c | 47 +++++++++++++++++++++++++++++++ 1 file changed, 47 insertions(+) diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c index e8a241754a46..7f6e5a5654fd 100644 --- a/drivers/net/ethernet/freescale/fec_main.c +++ b/drivers/net/ethernet/freescale/fec_main.c @@ -1990,6 +1990,51 @@ static int fec_enet_get_ts_info(struct net_device *ndev, } } +static void fec_enet_get_ringparam(struct net_device *ndev, + struct ethtool_ringparam *ring) +{ + struct fec_enet_private *fep = netdev_priv(ndev); + + ring->rx_max_pending = RX_RING_SIZE; + ring->tx_max_pending = TX_RING_SIZE; + ring->rx_pending = fep->rx_ring_size; + ring->tx_pending = fep->tx_ring_size; +} + +static int fec_enet_set_ringparam(struct net_device *ndev, + struct ethtool_ringparam *ring) +{ + struct fec_enet_private *fep = netdev_priv(ndev); + unsigned rx, tx, tx_min; + + tx_min = ndev->features & NETIF_F_SG ? TX_RING_SIZE_MIN_SG : 16; + + rx = clamp_t(u32, ring->rx_pending, 16, RX_RING_SIZE); + tx = clamp_t(u32, ring->tx_pending, tx_min, TX_RING_SIZE); + + if (tx == fep->tx_ring_size && rx == fep->rx_ring_size) + return 0; + + /* Setting the ring size while the interface is down is easy */ + if (!netif_running(ndev)) { + fep->tx_ring_size = tx; + fep->rx_ring_size = rx; + } else { + return -EINVAL; + + napi_disable(&fep->napi); + netif_tx_lock_bh(ndev); + fec_stop(ndev); + /* reallocate ring */ + fec_restart(ndev); + netif_wake_queue(ndev); + netif_tx_unlock_bh(ndev); + napi_enable(&fep->napi); + } + + return 0; +} + #if !defined(CONFIG_M5272) static void fec_enet_get_pauseparam(struct net_device *ndev, @@ -2194,6 +2239,8 @@ static const struct ethtool_ops fec_enet_ethtool_ops = { .get_drvinfo = fec_enet_get_drvinfo, .nway_reset = fec_enet_nway_reset, .get_link = ethtool_op_get_link, + .get_ringparam = fec_enet_get_ringparam, + .set_ringparam = fec_enet_set_ringparam, #ifndef CONFIG_M5272 .get_pauseparam = fec_enet_get_pauseparam, .set_pauseparam = fec_enet_set_pauseparam, From rmk Mon Aug 15 18:57:32 2016 References: Message-ID: From: Russell King To: Linux Kernel Mailing list Date: Mon, 15 Aug 2016 18:57:32 +0100 Content-type: text/plain; charset="utf-8" Content-Disposition: inline Content-Transfer-Encoding: 8bit Mime-Version: 1.0 Subject: Re: [PATCH] Revert FEC commits From: Russell King This reverts commit d5e42c86e07935f3cab4dcfca787a733146d0c70 ("net: fec: Don't clear IPV6 header checksum field when IP accelerator enable"). This reverts commit 79f339125ea316e910220e5f5b4ad30370f4de85 ("net: fec: Add software TSO support"), as 96c50caa5148 ("net: fec: Enable IP header hardware checksum") causes a regression with IPv6 by overwriting the location where the IPv4 header checksum would be: - 0x0000: 6000 0000 0028 0640 fd8f 7570 feb6 0001 `....(.@..up.... + 0x0000: 6000 0000 0028 0640 fd8f 0000 feb6 0001 `....(.@........ This reverts commit 96c50caa5148e0e0a077672574785700885c6764 ("net: fec: Enable IP header hardware checksum"), as it causes a regression with IPv6 by overwriting the location where the IPv4 header checksum would be: - 0x0000: 6000 0000 0028 0640 fd8f 7570 feb6 0001 `....(.@..up.... + 0x0000: 6000 0000 0028 0640 fd8f 0000 feb6 0001 `....(.@........ --- drivers/net/ethernet/freescale/fec.h | 6 - drivers/net/ethernet/freescale/fec_main.c | 288 +++--------------------------- 2 files changed, 24 insertions(+), 270 deletions(-) diff --git a/drivers/net/ethernet/freescale/fec.h b/drivers/net/ethernet/freescale/fec.h index 6f576a3cc204..4777242aab79 100644 --- a/drivers/net/ethernet/freescale/fec.h +++ b/drivers/net/ethernet/freescale/fec.h @@ -303,12 +303,6 @@ struct fec_enet_private { unsigned short tx_ring_size; unsigned short rx_ring_size; - unsigned short tx_stop_threshold; - unsigned short tx_wake_threshold; - - /* Software TSO */ - char *tso_hdrs; - dma_addr_t tso_hdrs_dma; unsigned char flags; diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c index 7f6e5a5654fd..1ec5385e7863 100644 --- a/drivers/net/ethernet/freescale/fec_main.c +++ b/drivers/net/ethernet/freescale/fec_main.c @@ -33,13 +33,6 @@ #include #include #include -#include -#include -#include -#include -#include -#include -#include #include #include #include @@ -262,15 +255,6 @@ MODULE_PARM_DESC(macaddr, "FEC Ethernet MAC address"); #define FEC_FLAG_RX_CSUM BIT(1) #define FEC_FLAG_RX_VLAN BIT(2) -#define TSO_HEADER_SIZE 128 -/* Max number of allowed TCP segments for software TSO */ -#define FEC_MAX_TSO_SEGS 100 -#define FEC_MAX_SKB_DESCS (FEC_MAX_TSO_SEGS * 2 + MAX_SKB_FRAGS) - -#define IS_TSO_HEADER(txq, addr) \ - ((addr >= txq->tso_hdrs_dma) && \ - (addr < txq->tso_hdrs_dma + txq->tx_ring_size * TSO_HEADER_SIZE)) - static int mii_cnt; static union bufdesc_u * @@ -303,10 +287,10 @@ fec_enet_rx_get(unsigned int index, struct fec_enet_private *fep) return bdp; } -static unsigned int fec_enet_get_free_txdesc_num(struct fec_enet_private *fep) +static unsigned ring_free(unsigned ins, unsigned rem, unsigned size) { - int num = fep->tx_dirty - fep->tx_next; - return num < 0 ? num + fep->tx_ring_size : num; + int num = rem - ins; + return num < 0 ? num + size : num; } static void *swap_buffer(void *bufaddr, int len) @@ -342,11 +326,6 @@ static void fec_dump(struct net_device *ndev) } } -static inline bool is_ipv4_pkt(struct sk_buff *skb) -{ - return skb->protocol == htons(ETH_P_IP) && ip_hdr(skb)->version == 4; -} - static int fec_enet_clear_csum(struct sk_buff *skb, struct net_device *ndev) { @@ -366,8 +345,6 @@ fec_enet_clear_csum(struct sk_buff *skb, struct net_device *ndev) if (unlikely(skb_cow_head(skb, 0))) return -1; - if (is_ipv4_pkt(skb)) - ip_hdr(skb)->check = 0; *(__sum16 *)(skb->head + skb->csum_start + skb->csum_offset) = 0; return 0; @@ -424,7 +401,7 @@ fec_enet_txq_submit_frag_skb(struct sk_buff *skb, struct net_device *ndev) if (fep->flags & FEC_FLAG_BUFDESC_EX) { if (skb->ip_summed == CHECKSUM_PARTIAL) - estatus |= BD_ENET_TX_PINS | BD_ENET_TX_IINS; + estatus |= BD_ENET_TX_PINS; bdp->ebd.cbd_bdu = 0; bdp->ebd.cbd_esc = estatus; } @@ -485,13 +462,6 @@ static int fec_enet_txq_submit_skb(struct sk_buff *skb, struct net_device *ndev) unsigned int index; int ret; - if (fec_enet_get_free_txdesc_num(fep) < fep->tx_min) { - dev_kfree_skb_any(skb); - if (net_ratelimit()) - netdev_err(ndev, "NOT enough BD for SG!\n"); - return NETDEV_TX_OK; - } - /* Protocol checksum off-load for TCP and UDP. */ if (fec_enet_clear_csum(skb, ndev)) { dev_kfree_skb_any(skb); @@ -551,7 +521,7 @@ static int fec_enet_txq_submit_skb(struct sk_buff *skb, struct net_device *ndev) skb_shinfo(skb)->tx_flags |= SKBTX_IN_PROGRESS; if (skb->ip_summed == CHECKSUM_PARTIAL) - estatus |= BD_ENET_TX_PINS | BD_ENET_TX_IINS; + estatus |= BD_ENET_TX_PINS; bdp->ebd.cbd_bdu = 0; bdp->ebd.cbd_esc = estatus; @@ -591,213 +561,27 @@ static int fec_enet_txq_submit_skb(struct sk_buff *skb, struct net_device *ndev) return 0; } -static int -fec_enet_txq_put_data_tso(struct sk_buff *skb, struct net_device *ndev, - union bufdesc_u *bdp, int index, char *data, - int size, bool last_tcp, bool is_last) -{ - struct fec_enet_private *fep = netdev_priv(ndev); - const struct platform_device_id *id_entry = - platform_get_device_id(fep->pdev); - unsigned short status; - unsigned int estatus = 0; - dma_addr_t addr; - - status = bdp->bd.cbd_sc; - status &= ~BD_ENET_TX_STATS; - - status |= (BD_ENET_TX_TC | BD_ENET_TX_READY); - - if (((unsigned long) data) & FEC_ALIGNMENT || - id_entry->driver_data & FEC_QUIRK_SWAP_FRAME) { - memcpy(fep->tx_bounce[index], data, size); - data = fep->tx_bounce[index]; - - if (id_entry->driver_data & FEC_QUIRK_SWAP_FRAME) - swap_buffer(data, size); - } - - addr = dma_map_single(&fep->pdev->dev, data, size, DMA_TO_DEVICE); - if (dma_mapping_error(&fep->pdev->dev, addr)) { - dev_kfree_skb_any(skb); - if (net_ratelimit()) - netdev_err(ndev, "Tx DMA memory map failed\n"); - return NETDEV_TX_BUSY; - } - - bdp->bd.cbd_datlen = size; - bdp->bd.cbd_bufaddr = addr; - - if (fep->flags & FEC_FLAG_BUFDESC_EX) { - if (skb->ip_summed == CHECKSUM_PARTIAL) - estatus |= BD_ENET_TX_PINS | BD_ENET_TX_IINS; - bdp->ebd.cbd_bdu = 0; - bdp->ebd.cbd_esc = estatus; - } - - /* Handle the last BD specially */ - if (last_tcp) - status |= (BD_ENET_TX_LAST | BD_ENET_TX_TC); - if (is_last) { - status |= BD_ENET_TX_INTR; - if (fep->flags & FEC_FLAG_BUFDESC_EX) - bdp->ebd.cbd_esc |= BD_ENET_TX_INT; - } - - bdp->bd.cbd_sc = status; - - return 0; -} - -static int -fec_enet_txq_put_hdr_tso(struct sk_buff *skb, struct net_device *ndev, - union bufdesc_u *bdp, int index) -{ - struct fec_enet_private *fep = netdev_priv(ndev); - const struct platform_device_id *id_entry = - platform_get_device_id(fep->pdev); - int hdr_len = skb_transport_offset(skb) + tcp_hdrlen(skb); - void *bufaddr; - unsigned long dmabuf; - unsigned short status; - unsigned int estatus = 0; - - status = bdp->bd.cbd_sc; - status &= ~BD_ENET_TX_STATS; - status |= (BD_ENET_TX_TC | BD_ENET_TX_READY); - - bufaddr = fep->tso_hdrs + index * TSO_HEADER_SIZE; - dmabuf = fep->tso_hdrs_dma + index * TSO_HEADER_SIZE; - if (((unsigned long) bufaddr) & FEC_ALIGNMENT || - id_entry->driver_data & FEC_QUIRK_SWAP_FRAME) { - memcpy(fep->tx_bounce[index], skb->data, hdr_len); - bufaddr = fep->tx_bounce[index]; - - if (id_entry->driver_data & FEC_QUIRK_SWAP_FRAME) - swap_buffer(bufaddr, hdr_len); - - dmabuf = dma_map_single(&fep->pdev->dev, bufaddr, - hdr_len, DMA_TO_DEVICE); - if (dma_mapping_error(&fep->pdev->dev, dmabuf)) { - dev_kfree_skb_any(skb); - if (net_ratelimit()) - netdev_err(ndev, "Tx DMA memory map failed\n"); - return NETDEV_TX_BUSY; - } - } - - bdp->bd.cbd_bufaddr = dmabuf; - bdp->bd.cbd_datlen = hdr_len; - - if (fep->flags & FEC_FLAG_BUFDESC_EX) { - if (skb->ip_summed == CHECKSUM_PARTIAL) - estatus |= BD_ENET_TX_PINS | BD_ENET_TX_IINS; - bdp->ebd.cbd_bdu = 0; - bdp->ebd.cbd_esc = estatus; - } - - bdp->bd.cbd_sc = status; - - return 0; -} - -static int fec_enet_txq_submit_tso(struct sk_buff *skb, struct net_device *ndev) +static netdev_tx_t +fec_enet_start_xmit(struct sk_buff *skb, struct net_device *ndev) { struct fec_enet_private *fep = netdev_priv(ndev); - int hdr_len = skb_transport_offset(skb) + tcp_hdrlen(skb); - int total_len, data_left; - struct tso_t tso; - unsigned int index = fep->tx_next; - unsigned int last_index = index; + int nr_frags = skb_shinfo(skb)->nr_frags; int ret; - if (tso_count_descs(skb) >= fec_enet_get_free_txdesc_num(fep)) { - dev_kfree_skb_any(skb); + if (ring_free(fep->tx_next, fep->tx_dirty, fep->tx_ring_size) < 1 + nr_frags) { + /* Ooops. All transmit buffers are full. Bail out. + * This should not happen, since ndev->tbusy should be set. + */ if (net_ratelimit()) - netdev_err(ndev, "NOT enough BD for TSO!\n"); - return NETDEV_TX_OK; - } - - /* Protocol checksum off-load for TCP and UDP. */ - if (fec_enet_clear_csum(skb, ndev)) { - dev_kfree_skb_any(skb); - return NETDEV_TX_OK; - } - - /* Initialize the TSO handler, and prepare the first payload */ - tso_start(skb, &tso); - - total_len = skb->len - hdr_len; - while (total_len > 0) { - union bufdesc_u *bdp; - char *hdr; - - bdp = fec_enet_tx_get(index, fep); - data_left = min_t(int, skb_shinfo(skb)->gso_size, total_len); - total_len -= data_left; - - /* prepare packet headers: MAC + IP + TCP */ - hdr = fep->tso_hdrs + index * TSO_HEADER_SIZE; - tso_build_hdr(skb, hdr, &tso, data_left, total_len == 0); - ret = fec_enet_txq_put_hdr_tso(skb, ndev, bdp, index); - if (ret) - goto err_release; - - while (data_left > 0) { - int size; - - if (++index >= fep->tx_ring_size) - index = 0; - - size = min_t(int, tso.size, data_left); - bdp = fec_enet_tx_get(index, fep); - ret = fec_enet_txq_put_data_tso(skb, ndev, bdp, index, tso.data, - size, size == data_left, - total_len == 0); - if (ret) - goto err_release; - - data_left -= size; - tso_build_data(skb, &tso, size); - } - - last_index = index; - if (++index >= fep->tx_ring_size) - index = 0; + netdev_err(ndev, "tx queue full!\n"); + return NETDEV_TX_BUSY; } - /* Save skb pointer */ - fep->tx_skbuff[last_index] = skb; - - skb_tx_timestamp(skb); - netdev_sent_queue(ndev, skb->len); - fep->tx_next = index; - - /* Trigger transmission start */ - if (readl(fep->hwp + FEC_X_DES_ACTIVE) == 0) - writel(0, fep->hwp + FEC_X_DES_ACTIVE); - - return 0; - -err_release: - /* TODO: Release all used data descriptors for TSO */ - return ret; -} - -static netdev_tx_t -fec_enet_start_xmit(struct sk_buff *skb, struct net_device *ndev) -{ - struct fec_enet_private *fep = netdev_priv(ndev); - int ret; - - if (skb_is_gso(skb)) - ret = fec_enet_txq_submit_tso(skb, ndev); - else - ret = fec_enet_txq_submit_skb(skb, ndev); + ret = fec_enet_txq_submit_skb(skb, ndev); if (ret) return ret; - if (fec_enet_get_free_txdesc_num(fep) < fep->tx_stop_threshold) + if (ring_free(fep->tx_next, fep->tx_dirty, fep->tx_ring_size) < fep->tx_min) netif_stop_queue(ndev); return NETDEV_TX_OK; @@ -1135,9 +919,7 @@ fec_enet_tx(struct net_device *ndev) skb = fep->tx_skbuff[index]; fep->tx_skbuff[index] = NULL; - if (!IS_TSO_HEADER(fep, bdp->bd.cbd_bufaddr)) - fec_enet_tx_unmap(bdp, fep); - bdp->bd.cbd_bufaddr = 0; + fec_enet_tx_unmap(bdp, fep); if (!skb) continue; @@ -1191,7 +973,8 @@ fec_enet_tx(struct net_device *ndev) writel(0, fep->hwp + FEC_X_DES_ACTIVE); if (netif_queue_stopped(ndev) && - fec_enet_get_free_txdesc_num(fep) >= fep->tx_wake_threshold) + ring_free(fep->tx_next, fep->tx_dirty, fep->tx_ring_size) >= + fep->tx_min) netif_wake_queue(ndev); } @@ -2591,15 +2374,10 @@ static int fec_set_features(struct net_device *netdev, } /* Set the appropriate minimum transmit ring free threshold */ - if (features & NETIF_F_SG) { + if (features & NETIF_F_SG) fep->tx_min = MAX_SKB_FRAGS + 1; - fep->tx_stop_threshold = FEC_MAX_SKB_DESCS; - fep->tx_wake_threshold = (fep->tx_ring_size - fep->tx_stop_threshold) / 2; - } else { + else fep->tx_min = 1; - fep->tx_stop_threshold = 1; - fep->tx_wake_threshold = 1; - } /* Resume the device after updates */ if (netif_running(netdev) && changed & FEATURES_NEED_QUIESCE) { @@ -2647,9 +2425,6 @@ static int fec_enet_init(struct net_device *ndev) fep->tx_ring_size = TX_RING_SIZE; fep->rx_ring_size = RX_RING_SIZE; - fep->tx_stop_threshold = FEC_MAX_SKB_DESCS; - fep->tx_wake_threshold = (fep->tx_ring_size - fep->tx_stop_threshold) / 2; - bd_size = fep->tx_ring_size + fep->rx_ring_size; if (fep->flags & FEC_FLAG_BUFDESC_EX) bd_size *= sizeof(struct bufdesc_ex); @@ -2661,13 +2436,6 @@ static int fec_enet_init(struct net_device *ndev) if (!cbd_base) return -ENOMEM; - fep->tso_hdrs = dma_alloc_coherent(NULL, fep->tx_ring_size * TSO_HEADER_SIZE, - &fep->tso_hdrs_dma, GFP_KERNEL); - if (!fep->tso_hdrs) { - dma_free_coherent(NULL, bd_size, cbd_base, cbd_dma); - return -ENOMEM; - } - memset(cbd_base, 0, PAGE_SIZE); fep->netdev = ndev; @@ -2708,25 +2476,17 @@ static int fec_enet_init(struct net_device *ndev) } if (id_entry->driver_data & FEC_QUIRK_HAS_CSUM) { - ndev->gso_max_segs = FEC_MAX_TSO_SEGS; - /* enable hw accelerator */ ndev->features |= (NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM - | NETIF_F_RXCSUM | NETIF_F_SG | - NETIF_F_TSO); + | NETIF_F_RXCSUM | NETIF_F_SG); fep->flags |= FEC_FLAG_RX_CSUM; } } - if (ndev->features & NETIF_F_SG) { + if (ndev->features & NETIF_F_SG) fep->tx_min = MAX_SKB_FRAGS + 1; - fep->tx_stop_threshold = FEC_MAX_SKB_DESCS; - fep->tx_wake_threshold = (fep->tx_ring_size - fep->tx_stop_threshold) / 2; - } else { + else fep->tx_min = 1; - fep->tx_stop_threshold = 1; - fep->tx_wake_threshold = 1; - } ndev->hw_features = ndev->features; From rmk Mon Aug 15 18:57:32 2016 References: Message-ID: From: Russell King To: Linux Kernel Mailing list Date: Mon, 15 Aug 2016 18:57:32 +0100 Content-type: text/plain; charset="utf-8" Content-Disposition: inline Content-Transfer-Encoding: 8bit Mime-Version: 1.0 Subject: Re: [PATCH] net:fec: convert scatter-gather support From: Russell King Add scatter-gather support for SKB transmission. This allows the driver to make use of GSO, which when enabled allows the iMX6Q to increase TCP transmission throughput from about 320 to 420Mbps, measured with iperf 2.0.5 We adjust the minimum transmit ring space according to whether SG support is enabled or not. This allows non-SG configurations to avoid the tx ring reservation necessary for SG, thereby making full use of their available ring (since non-SG requires just one tx ring entry per packet.) Signed-off-by: Russell King --- drivers/net/ethernet/freescale/fec.h | 1 + drivers/net/ethernet/freescale/fec_main.c | 183 ++++++++++++++---------------- 2 files changed, 87 insertions(+), 97 deletions(-) diff --git a/drivers/net/ethernet/freescale/fec.h b/drivers/net/ethernet/freescale/fec.h index 4777242aab79..d9eb328559d8 100644 --- a/drivers/net/ethernet/freescale/fec.h +++ b/drivers/net/ethernet/freescale/fec.h @@ -284,6 +284,7 @@ struct fec_enet_private { bool ptp_clk_on; struct mutex ptp_clk_mutex; + unsigned char tx_page_map[TX_RING_SIZE]; /* The saved address of a sent-in-place packet/buffer, for skfree(). */ unsigned char *tx_bounce[TX_RING_SIZE]; struct sk_buff *tx_skbuff[TX_RING_SIZE]; diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c index 1ec5385e7863..93eeb70c644b 100644 --- a/drivers/net/ethernet/freescale/fec_main.c +++ b/drivers/net/ethernet/freescale/fec_main.c @@ -351,30 +351,31 @@ fec_enet_clear_csum(struct sk_buff *skb, struct net_device *ndev) } static void -fec_enet_tx_unmap(union bufdesc_u *bdp, struct fec_enet_private *fep) +fec_enet_tx_unmap(unsigned index, union bufdesc_u *bdp, struct fec_enet_private *fep) { dma_addr_t addr = bdp->bd.cbd_bufaddr; size_t length = bdp->bd.cbd_datlen; - dma_unmap_single(&fep->pdev->dev, addr, length, DMA_TO_DEVICE); + if (fep->tx_page_map[index]) + dma_unmap_page(&fep->pdev->dev, addr, length, DMA_TO_DEVICE); + else + dma_unmap_single(&fep->pdev->dev, addr, length, DMA_TO_DEVICE); } -static int -fec_enet_txq_submit_frag_skb(struct sk_buff *skb, struct net_device *ndev) +static bool +fec_enet_txq_submit_frag_skb(struct sk_buff *skb, struct net_device *ndev, + unsigned int estatus) { struct fec_enet_private *fep = netdev_priv(ndev); const struct platform_device_id *id_entry = platform_get_device_id(fep->pdev); union bufdesc_u *bdp; int nr_frags = skb_shinfo(skb)->nr_frags; - int frag, frag_len; unsigned short status; - unsigned int estatus = 0; - skb_frag_t *this_frag; - unsigned int index = fep->tx_next; - void *bufaddr; + const skb_frag_t *this_frag; + unsigned int frag_len, index = fep->tx_next; dma_addr_t addr; - int i; + int frag, i; for (frag = 0; frag < nr_frags; frag++) { this_frag = &skb_shinfo(skb)->frags[frag]; @@ -382,58 +383,49 @@ fec_enet_txq_submit_frag_skb(struct sk_buff *skb, struct net_device *ndev) if (++index >= fep->tx_ring_size) index = 0; - bdp = fec_enet_tx_get(index, fep); - status = bdp->bd.cbd_sc; - status &= ~BD_ENET_TX_STATS; - status |= (BD_ENET_TX_TC | BD_ENET_TX_READY); - frag_len = skb_shinfo(skb)->frags[frag].size; + frag_len = skb_frag_size(this_frag); - /* Handle the last BD specially */ - if (frag == nr_frags - 1) { - status |= (BD_ENET_TX_INTR | BD_ENET_TX_LAST); - if (fep->flags & FEC_FLAG_BUFDESC_EX) { - estatus |= BD_ENET_TX_INT; - if (unlikely(skb_shinfo(skb)->tx_flags & - SKBTX_HW_TSTAMP && fep->hwts_tx_en)) - estatus |= BD_ENET_TX_TS; - } - } + /* If the alignment is unsuitable, we need to bounce. */ + if (this_frag->page_offset & FEC_ALIGNMENT || + id_entry->driver_data & FEC_QUIRK_SWAP_FRAME) { + unsigned char *bounce = fep->tx_bounce[index]; - if (fep->flags & FEC_FLAG_BUFDESC_EX) { - if (skb->ip_summed == CHECKSUM_PARTIAL) - estatus |= BD_ENET_TX_PINS; - bdp->ebd.cbd_bdu = 0; - bdp->ebd.cbd_esc = estatus; + memcpy(bounce, skb_frag_address(this_frag), frag_len); + if (id_entry->driver_data & FEC_QUIRK_SWAP_FRAME) + swap_buffer(bounce, frag_len); + + addr = dma_map_single(&fep->pdev->dev, bounce, + frag_len, DMA_TO_DEVICE); + fep->tx_page_map[index] = 0; + } else { + addr = skb_frag_dma_map(&fep->pdev->dev, this_frag, 0, + frag_len, DMA_TO_DEVICE); + fep->tx_page_map[index] = 1; } - bufaddr = page_address(this_frag->page.p) + this_frag->page_offset; + if (dma_mapping_error(&fep->pdev->dev, addr)) + goto dma_mapping_error; - if (((unsigned long) bufaddr) & FEC_ALIGNMENT || - id_entry->driver_data & FEC_QUIRK_SWAP_FRAME) { - memcpy(fep->tx_bounce[index], bufaddr, frag_len); - bufaddr = fep->tx_bounce[index]; + status = BD_ENET_TX_TC | BD_ENET_TX_READY; - if (id_entry->driver_data & FEC_QUIRK_SWAP_FRAME) - swap_buffer(bufaddr, frag_len); - } + /* Handle the last BD specially */ + if (frag == nr_frags - 1) + status |= BD_ENET_TX_INTR | BD_ENET_TX_LAST; - addr = dma_map_single(&fep->pdev->dev, bufaddr, frag_len, - DMA_TO_DEVICE); - if (dma_mapping_error(&fep->pdev->dev, addr)) { - dev_kfree_skb_any(skb); - if (net_ratelimit()) - netdev_err(ndev, "Tx DMA memory map failed\n"); - goto dma_mapping_error; + bdp = fec_enet_tx_get(index, fep); + if (fep->flags & FEC_FLAG_BUFDESC_EX) { + bdp->ebd.cbd_bdu = 0; + bdp->ebd.cbd_esc = estatus; } bdp->bd.cbd_bufaddr = addr; bdp->bd.cbd_datlen = frag_len; - bdp->bd.cbd_sc = status; + bdp->bd.cbd_sc = (bdp->bd.cbd_sc & ~BD_ENET_TX_STATS) | status; } fep->tx_next = index; - return 0; + return true; dma_mapping_error: index = fep->tx_next; @@ -441,40 +433,32 @@ fec_enet_txq_submit_frag_skb(struct sk_buff *skb, struct net_device *ndev) if (++index >= fep->tx_ring_size) index = 0; bdp = fec_enet_tx_get(index, fep); - dma_unmap_single(&fep->pdev->dev, bdp->bd.cbd_bufaddr, - bdp->bd.cbd_datlen, DMA_TO_DEVICE); + fec_enet_tx_unmap(index, bdp, fep); } - return NETDEV_TX_OK; + return false; } -static int fec_enet_txq_submit_skb(struct sk_buff *skb, struct net_device *ndev) +static bool fec_enet_txq_submit_skb(struct sk_buff *skb, struct net_device *ndev) { struct fec_enet_private *fep = netdev_priv(ndev); const struct platform_device_id *id_entry = platform_get_device_id(fep->pdev); - int nr_frags = skb_shinfo(skb)->nr_frags; union bufdesc_u *bdp; void *bufaddr; dma_addr_t addr; unsigned short status; - unsigned short buflen; - unsigned int estatus = 0; - unsigned int index; - int ret; + unsigned int index, buflen, estatus; /* Protocol checksum off-load for TCP and UDP. */ - if (fec_enet_clear_csum(skb, ndev)) { - dev_kfree_skb_any(skb); - return NETDEV_TX_OK; - } - - /* Fill in a Tx ring entry */ - index = fep->tx_next; + if (fec_enet_clear_csum(skb, ndev)) + return false; /* Set buffer length and buffer pointer */ bufaddr = skb->data; buflen = skb_headlen(skb); + index = fep->tx_next; + /* * On some FEC implementations data must be aligned on * 4-byte boundaries. Use bounce buffers to copy data @@ -491,34 +475,24 @@ static int fec_enet_txq_submit_skb(struct sk_buff *skb, struct net_device *ndev) /* Push the data cache so the CPM does not get stale memory data. */ addr = dma_map_single(&fep->pdev->dev, bufaddr, buflen, DMA_TO_DEVICE); - if (dma_mapping_error(&fep->pdev->dev, addr)) { - dev_kfree_skb_any(skb); - if (net_ratelimit()) - netdev_err(ndev, "Tx DMA memory map failed\n"); - return NETDEV_TX_OK; - } + if (dma_mapping_error(&fep->pdev->dev, addr)) + goto release; bdp = fec_enet_tx_get(index, fep); - status = bdp->bd.cbd_sc & ~BD_ENET_TX_STATS; - if (nr_frags) { - ret = fec_enet_txq_submit_frag_skb(skb, ndev); - if (ret) - return ret; - } else { - status |= (BD_ENET_TX_INTR | BD_ENET_TX_LAST); - if (fep->flags & FEC_FLAG_BUFDESC_EX) { - estatus = BD_ENET_TX_INT; - if (unlikely(skb_shinfo(skb)->tx_flags & - SKBTX_HW_TSTAMP && fep->hwts_tx_en)) - estatus |= BD_ENET_TX_TS; - } - } + /* Fill in a Tx ring entry */ + bdp->bd.cbd_datlen = buflen; + bdp->bd.cbd_bufaddr = addr; + + fep->tx_page_map[index] = 0; + estatus = BD_ENET_TX_INT; if (fep->flags & FEC_FLAG_BUFDESC_EX) { if (unlikely(skb_shinfo(skb)->tx_flags & SKBTX_HW_TSTAMP && - fep->hwts_tx_en)) + fep->hwts_tx_en)) { + estatus |= BD_ENET_TX_TS; skb_shinfo(skb)->tx_flags |= SKBTX_IN_PROGRESS; + } if (skb->ip_summed == CHECKSUM_PARTIAL) estatus |= BD_ENET_TX_PINS; @@ -527,13 +501,19 @@ static int fec_enet_txq_submit_skb(struct sk_buff *skb, struct net_device *ndev) bdp->ebd.cbd_esc = estatus; } + status = BD_ENET_TX_READY | BD_ENET_TX_TC; + if (skb_shinfo(skb)->nr_frags) { + if (!fec_enet_txq_submit_frag_skb(skb, ndev, estatus)) + goto unmap; + } else { + status |= BD_ENET_TX_INTR | BD_ENET_TX_LAST; + } + index = fep->tx_next; + /* Save skb pointer */ fep->tx_skbuff[index] = skb; - bdp->bd.cbd_datlen = buflen; - bdp->bd.cbd_bufaddr = addr; - /* * We need the preceding stores to the descriptor to complete * before updating the status field, which hands it over to the @@ -544,7 +524,7 @@ static int fec_enet_txq_submit_skb(struct sk_buff *skb, struct net_device *ndev) /* Send it on its way. Tell FEC it's ready, interrupt when done, * it's the last BD of the frame, and to put the CRC on the end. */ - bdp->bd.cbd_sc = status | BD_ENET_TX_READY | BD_ENET_TX_TC; + bdp->bd.cbd_sc = status | (bdp->bd.cbd_sc & BD_ENET_TX_WRAP); skb_tx_timestamp(skb); netdev_sent_queue(ndev, skb->len); @@ -558,7 +538,14 @@ static int fec_enet_txq_submit_skb(struct sk_buff *skb, struct net_device *ndev) if (readl(fep->hwp + FEC_X_DES_ACTIVE) == 0) writel(0, fep->hwp + FEC_X_DES_ACTIVE); - return 0; + return true; + + unmap: + fec_enet_tx_unmap(index, bdp, fep); + release: + if (net_ratelimit()) + netdev_err(ndev, "Tx DMA memory map failed\n"); + return false; } static netdev_tx_t @@ -566,7 +553,6 @@ fec_enet_start_xmit(struct sk_buff *skb, struct net_device *ndev) { struct fec_enet_private *fep = netdev_priv(ndev); int nr_frags = skb_shinfo(skb)->nr_frags; - int ret; if (ring_free(fep->tx_next, fep->tx_dirty, fep->tx_ring_size) < 1 + nr_frags) { /* Ooops. All transmit buffers are full. Bail out. @@ -577,9 +563,10 @@ fec_enet_start_xmit(struct sk_buff *skb, struct net_device *ndev) return NETDEV_TX_BUSY; } - ret = fec_enet_txq_submit_skb(skb, ndev); - if (ret) - return ret; + if (!fec_enet_txq_submit_skb(skb, ndev)) { + dev_kfree_skb_any(skb); + return NETDEV_TX_OK; + } if (ring_free(fep->tx_next, fep->tx_dirty, fep->tx_ring_size) < fep->tx_min) netif_stop_queue(ndev); @@ -621,7 +608,7 @@ static void fec_enet_bd_init(struct net_device *dev) else bdp->bd.cbd_sc = 0; if (bdp->bd.cbd_bufaddr) - fec_enet_tx_unmap(bdp, fep); + fec_enet_tx_unmap(i, bdp, fep); bdp->bd.cbd_bufaddr = 0; if (fep->tx_skbuff[i]) { dev_kfree_skb_any(fep->tx_skbuff[i]); @@ -919,7 +906,7 @@ fec_enet_tx(struct net_device *ndev) skb = fep->tx_skbuff[index]; fep->tx_skbuff[index] = NULL; - fec_enet_tx_unmap(bdp, fep); + fec_enet_tx_unmap(index, bdp, fep); if (!skb) continue; @@ -2077,7 +2064,7 @@ static void fec_enet_free_buffers(struct net_device *ndev) for (i = 0; i < fep->tx_ring_size; i++) { bdp = fec_enet_tx_get(i, fep); if (bdp->bd.cbd_bufaddr) - fec_enet_tx_unmap(bdp, fep); + fec_enet_tx_unmap(i, bdp, fep); bdp->bd.cbd_bufaddr = 0; kfree(fep->tx_bounce[i]); fep->tx_bounce[i] = NULL; @@ -2478,11 +2465,13 @@ static int fec_enet_init(struct net_device *ndev) if (id_entry->driver_data & FEC_QUIRK_HAS_CSUM) { /* enable hw accelerator */ ndev->features |= (NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM - | NETIF_F_RXCSUM | NETIF_F_SG); + | NETIF_F_RXCSUM); fep->flags |= FEC_FLAG_RX_CSUM; } } + ndev->features |= NETIF_F_SG; + if (ndev->features & NETIF_F_SG) fep->tx_min = MAX_SKB_FRAGS + 1; else From rmk Mon Aug 15 18:57:32 2016 References: Message-ID: From: Russell King To: Linux Kernel Mailing list Date: Mon, 15 Aug 2016 18:57:32 +0100 Content-type: text/plain; charset="utf-8" Content-Disposition: inline Content-Transfer-Encoding: 8bit Mime-Version: 1.0 Subject: Re: [PATCH] net:fec: allocate descriptor rings at device open, and free on device close From: Russell King The driver has had a memory leak for quite some time: when the device is probed, we allocate a page for the descriptor rings, but we never free this. Rather than trying to free it on the various probe failure paths, move its allocation to device open time - we have to restart the FEC for this to take effect. This also means we can free the descriptor rings when we clean up in the device close function, which gives this a nice symmetry. Signed-off-by: Russell King --- drivers/net/ethernet/freescale/fec_main.c | 81 +++++++++++++++---------------- 1 file changed, 39 insertions(+), 42 deletions(-) diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c index 93eeb70c644b..a29f392e229d 100644 --- a/drivers/net/ethernet/freescale/fec_main.c +++ b/drivers/net/ethernet/freescale/fec_main.c @@ -657,7 +657,8 @@ fec_restart(struct net_device *ndev) /* Set maximum receive buffer size. */ writel(PKT_MAXBLR_SIZE, fep->hwp + FEC_R_BUFF_SIZE); - fec_enet_bd_init(ndev); + if (fep->rx_bd_base) + fec_enet_bd_init(ndev); netdev_reset_queue(ndev); /* Set receive and transmit descriptor base. */ @@ -2072,6 +2073,8 @@ static void fec_enet_free_buffers(struct net_device *ndev) fep->tx_skbuff[i] = NULL; dev_kfree_skb(skb); } + + dma_free_coherent(NULL, PAGE_SIZE, fep->rx_bd_base, fep->rx_bd_dma); } static int fec_enet_alloc_buffers(struct net_device *ndev) @@ -2080,6 +2083,37 @@ static int fec_enet_alloc_buffers(struct net_device *ndev) unsigned int i; struct sk_buff *skb; union bufdesc_u *bdp; + union bufdesc_u *cbd_base; + dma_addr_t cbd_dma; + int bd_size; + + bd_size = fep->tx_ring_size + fep->rx_ring_size; + if (fep->flags & FEC_FLAG_BUFDESC_EX) + bd_size *= sizeof(struct bufdesc_ex); + else + bd_size *= sizeof(struct bufdesc); + + /* Allocate memory for buffer descriptors. */ + cbd_base = dma_alloc_coherent(NULL, bd_size, &cbd_dma, GFP_KERNEL); + if (!cbd_base) + return -ENOMEM; + + memset(cbd_base, 0, PAGE_SIZE); + + /* Set receive and transmit descriptor base. */ + fep->rx_bd_base = cbd_base; + fep->rx_bd_dma = cbd_dma; + if (fep->flags & FEC_FLAG_BUFDESC_EX) { + fep->tx_bd_base = (union bufdesc_u *) + (&cbd_base->ebd + fep->rx_ring_size); + fep->tx_bd_dma = cbd_dma + sizeof(struct bufdesc_ex) * + fep->rx_ring_size; + } else { + fep->tx_bd_base = (union bufdesc_u *) + (&cbd_base->bd + fep->rx_ring_size); + fep->tx_bd_dma = cbd_dma + sizeof(struct bufdesc) * + fep->rx_ring_size; + } for (i = 0; i < fep->rx_ring_size; i++) { dma_addr_t addr; @@ -2395,36 +2429,16 @@ static const struct net_device_ops fec_netdev_ops = { .ndo_set_features = fec_set_features, }; - /* - * XXX: We need to clean up on failure exits here. - * - */ -static int fec_enet_init(struct net_device *ndev) +static void fec_enet_init(struct net_device *ndev) { struct fec_enet_private *fep = netdev_priv(ndev); const struct platform_device_id *id_entry = platform_get_device_id(fep->pdev); - union bufdesc_u *cbd_base; - dma_addr_t cbd_dma; - int bd_size; /* init the tx & rx ring size */ fep->tx_ring_size = TX_RING_SIZE; fep->rx_ring_size = RX_RING_SIZE; - bd_size = fep->tx_ring_size + fep->rx_ring_size; - if (fep->flags & FEC_FLAG_BUFDESC_EX) - bd_size *= sizeof(struct bufdesc_ex); - else - bd_size *= sizeof(struct bufdesc); - - /* Allocate memory for buffer descriptors. */ - cbd_base = dma_alloc_coherent(NULL, bd_size, &cbd_dma, GFP_KERNEL); - if (!cbd_base) - return -ENOMEM; - - memset(cbd_base, 0, PAGE_SIZE); - fep->netdev = ndev; /* Get the Ethernet address */ @@ -2432,20 +2446,8 @@ static int fec_enet_init(struct net_device *ndev) /* make sure MAC we just acquired is programmed into the hw */ fec_set_mac_address(ndev, NULL); - /* Set receive and transmit descriptor base. */ - fep->rx_bd_base = cbd_base; - fep->rx_bd_dma = cbd_dma; - if (fep->flags & FEC_FLAG_BUFDESC_EX) { - fep->tx_bd_base = (union bufdesc_u *) - (&cbd_base->ebd + fep->rx_ring_size); - fep->tx_bd_dma = cbd_dma + sizeof(struct bufdesc_ex) * - fep->rx_ring_size; - } else { - fep->tx_bd_base = (union bufdesc_u *) - (&cbd_base->bd + fep->rx_ring_size); - fep->tx_bd_dma = cbd_dma + sizeof(struct bufdesc) * - fep->rx_ring_size; - } + fep->rx_bd_base = fep->tx_bd_base = NULL; + fep->rx_bd_dma = fep->tx_bd_dma = 0; /* The FEC Ethernet specific entries in the device structure */ ndev->watchdog_timeo = TX_TIMEOUT; @@ -2480,8 +2482,6 @@ static int fec_enet_init(struct net_device *ndev) ndev->hw_features = ndev->features; fec_restart(ndev); - - return 0; } #ifdef CONFIG_OF @@ -2647,9 +2647,7 @@ fec_probe(struct platform_device *pdev) if (fep->flags & FEC_FLAG_BUFDESC_EX) fec_ptp_init(pdev); - ret = fec_enet_init(ndev); - if (ret) - goto failed_init; + fec_enet_init(ndev); for (i = 0; i < FEC_IRQ_NUM; i++) { irq = platform_get_irq(pdev, i); @@ -2688,7 +2686,6 @@ fec_probe(struct platform_device *pdev) fec_enet_mii_remove(fep); failed_mii_init: failed_irq: -failed_init: if (fep->reg_phy) regulator_disable(fep->reg_phy); failed_regulator: From rmk Mon Aug 15 18:57:32 2016 References: Message-ID: From: Russell King To: Linux Kernel Mailing list Date: Mon, 15 Aug 2016 18:57:32 +0100 Content-type: text/plain; charset="utf-8" Content-Disposition: inline Content-Transfer-Encoding: 8bit Mime-Version: 1.0 Subject: Re: [PATCH] net:fec: remove unnecessary code From: Russell King fec_enet_bd_init() already frees the transmit skbuffs, so there's no need for fec_restart() to do this again. Signed-off-by: Russell King --- drivers/net/ethernet/freescale/fec.h | 1 - drivers/net/ethernet/freescale/fec_main.c | 8 -------- 2 files changed, 9 deletions(-) diff --git a/drivers/net/ethernet/freescale/fec.h b/drivers/net/ethernet/freescale/fec.h index d9eb328559d8..442fa513e000 100644 --- a/drivers/net/ethernet/freescale/fec.h +++ b/drivers/net/ethernet/freescale/fec.h @@ -254,7 +254,6 @@ union bufdesc_u { #define FEC_ENET_TX_FRSIZE 2048 #define FEC_ENET_TX_FRPPG (PAGE_SIZE / FEC_ENET_TX_FRSIZE) #define TX_RING_SIZE 512 /* Must be power of two */ -#define TX_RING_MOD_MASK 511 /* for this to work */ #define BD_ENET_RX_INT 0x00800000 #define BD_ENET_RX_PTP ((ushort)0x0400) diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c index a29f392e229d..95a9fbd70f4e 100644 --- a/drivers/net/ethernet/freescale/fec_main.c +++ b/drivers/net/ethernet/freescale/fec_main.c @@ -631,7 +631,6 @@ fec_restart(struct net_device *ndev) struct fec_enet_private *fep = netdev_priv(ndev); const struct platform_device_id *id_entry = platform_get_device_id(fep->pdev); - int i; u32 val; u32 temp_mac[2]; u32 rcntl = OPT_FRAME_SIZE | 0x04; @@ -665,13 +664,6 @@ fec_restart(struct net_device *ndev) writel(fep->rx_bd_dma, fep->hwp + FEC_R_DES_START); writel(fep->tx_bd_dma, fep->hwp + FEC_X_DES_START); - for (i = 0; i <= TX_RING_MOD_MASK; i++) { - if (fep->tx_skbuff[i]) { - dev_kfree_skb_any(fep->tx_skbuff[i]); - fep->tx_skbuff[i] = NULL; - } - } - /* Enable MII mode */ if (fep->full_duplex == DUPLEX_FULL) { /* FD enable */ From rmk Mon Aug 15 18:57:32 2016 References: Message-ID: From: Russell King To: Linux Kernel Mailing list Date: Mon, 15 Aug 2016 18:57:32 +0100 Content-type: text/plain; charset="utf-8" Content-Disposition: inline Content-Transfer-Encoding: 8bit Mime-Version: 1.0 Subject: Re: [PATCH] net:fec: even larger rings From: Russell King Signed-off-by: Russell King --- drivers/net/ethernet/freescale/fec_main.c | 68 +++++++++++++++++++------------ 1 file changed, 42 insertions(+), 26 deletions(-) diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c index 95a9fbd70f4e..95fad9862ef5 100644 --- a/drivers/net/ethernet/freescale/fec_main.c +++ b/drivers/net/ethernet/freescale/fec_main.c @@ -872,7 +872,7 @@ fec_enet_hwtstamp(struct fec_enet_private *fep, unsigned ts, hwtstamps->hwtstamp = ns_to_ktime(ns); } -static void +static void noinline fec_enet_tx(struct net_device *ndev) { struct fec_enet_private *fep = netdev_priv(ndev); @@ -2041,6 +2041,7 @@ static void fec_enet_free_buffers(struct net_device *ndev) unsigned int i; struct sk_buff *skb; union bufdesc_u *bdp; + int rx_cbd_size, tx_cbd_size; for (i = 0; i < fep->rx_ring_size; i++) { bdp = fec_enet_rx_get(i, fep); @@ -2066,7 +2067,18 @@ static void fec_enet_free_buffers(struct net_device *ndev) dev_kfree_skb(skb); } - dma_free_coherent(NULL, PAGE_SIZE, fep->rx_bd_base, fep->rx_bd_dma); + rx_cbd_size = fep->rx_ring_size; + tx_cbd_size = fep->tx_ring_size; + if (fep->flags & FEC_FLAG_BUFDESC_EX) { + rx_cbd_size *= sizeof(struct bufdesc_ex); + tx_cbd_size *= sizeof(struct bufdesc_ex); + } else { + rx_cbd_size *= sizeof(struct bufdesc); + tx_cbd_size *= sizeof(struct bufdesc); + } + + dma_free_coherent(NULL, rx_cbd_size, fep->rx_bd_base, fep->rx_bd_dma); + dma_free_coherent(NULL, tx_cbd_size, fep->tx_bd_base, fep->tx_bd_dma); } static int fec_enet_alloc_buffers(struct net_device *ndev) @@ -2074,38 +2086,42 @@ static int fec_enet_alloc_buffers(struct net_device *ndev) struct fec_enet_private *fep = netdev_priv(ndev); unsigned int i; struct sk_buff *skb; + union bufdesc_u *rx_cbd_cpu, *tx_cbd_cpu; + dma_addr_t rx_cbd_dma, tx_cbd_dma; union bufdesc_u *bdp; - union bufdesc_u *cbd_base; - dma_addr_t cbd_dma; - int bd_size; + int rx_cbd_size, tx_cbd_size; - bd_size = fep->tx_ring_size + fep->rx_ring_size; - if (fep->flags & FEC_FLAG_BUFDESC_EX) - bd_size *= sizeof(struct bufdesc_ex); - else - bd_size *= sizeof(struct bufdesc); + rx_cbd_size = fep->rx_ring_size; + tx_cbd_size = fep->tx_ring_size; + if (fep->flags & FEC_FLAG_BUFDESC_EX) { + rx_cbd_size *= sizeof(struct bufdesc_ex); + tx_cbd_size *= sizeof(struct bufdesc_ex); + } else { + rx_cbd_size *= sizeof(struct bufdesc); + tx_cbd_size *= sizeof(struct bufdesc); + } /* Allocate memory for buffer descriptors. */ - cbd_base = dma_alloc_coherent(NULL, bd_size, &cbd_dma, GFP_KERNEL); - if (!cbd_base) + rx_cbd_cpu = dma_alloc_coherent(NULL, rx_cbd_size, &rx_cbd_dma, + GFP_KERNEL); + tx_cbd_cpu = dma_alloc_coherent(NULL, tx_cbd_size, &tx_cbd_dma, + GFP_KERNEL); + if (!rx_cbd_cpu || !tx_cbd_cpu) { + if (rx_cbd_cpu) + dma_free_coherent(NULL, rx_cbd_size, rx_cbd_cpu, rx_cbd_dma); + if (tx_cbd_cpu) + dma_free_coherent(NULL, tx_cbd_size, tx_cbd_cpu, tx_cbd_dma); return -ENOMEM; + } - memset(cbd_base, 0, PAGE_SIZE); + memset(rx_cbd_cpu, 0, PAGE_SIZE); + memset(tx_cbd_cpu, 0, PAGE_SIZE); /* Set receive and transmit descriptor base. */ - fep->rx_bd_base = cbd_base; - fep->rx_bd_dma = cbd_dma; - if (fep->flags & FEC_FLAG_BUFDESC_EX) { - fep->tx_bd_base = (union bufdesc_u *) - (&cbd_base->ebd + fep->rx_ring_size); - fep->tx_bd_dma = cbd_dma + sizeof(struct bufdesc_ex) * - fep->rx_ring_size; - } else { - fep->tx_bd_base = (union bufdesc_u *) - (&cbd_base->bd + fep->rx_ring_size); - fep->tx_bd_dma = cbd_dma + sizeof(struct bufdesc) * - fep->rx_ring_size; - } + fep->rx_bd_base = rx_cbd_cpu; + fep->rx_bd_dma = rx_cbd_dma; + fep->tx_bd_base = tx_cbd_cpu; + fep->tx_bd_dma = tx_cbd_dma; for (i = 0; i < fep->rx_ring_size; i++) { dma_addr_t addr; From rmk Mon Aug 15 18:57:32 2016 References: Message-ID: From: Russell King To: Linux Kernel Mailing list Date: Mon, 15 Aug 2016 18:57:32 +0100 Content-type: text/plain; charset="utf-8" Content-Disposition: inline Content-Transfer-Encoding: 8bit Mime-Version: 1.0 Subject: Re: [PATCH] net:fec: unsorted hacks From: Russell King Signed-off-by: Russell King --- drivers/net/ethernet/freescale/fec.h | 15 ++++-- drivers/net/ethernet/freescale/fec_main.c | 85 +++++++++++++++++++------------ 2 files changed, 63 insertions(+), 37 deletions(-) diff --git a/drivers/net/ethernet/freescale/fec.h b/drivers/net/ethernet/freescale/fec.h index 442fa513e000..377477f18d01 100644 --- a/drivers/net/ethernet/freescale/fec.h +++ b/drivers/net/ethernet/freescale/fec.h @@ -231,10 +231,17 @@ union bufdesc_u { #define BD_ENET_TX_STATS ((ushort)0x0fff) /* All status bits */ /*enhanced buffer descriptor control/status used by Ethernet transmit*/ -#define BD_ENET_TX_INT 0x40000000 -#define BD_ENET_TX_TS 0x20000000 -#define BD_ENET_TX_PINS 0x10000000 -#define BD_ENET_TX_IINS 0x08000000 +#define BD_ENET_TX_INT BIT(30) +#define BD_ENET_TX_TS BIT(29) +#define BD_ENET_TX_PINS BIT(28) +#define BD_ENET_TX_IINS BIT(27) +#define BD_ENET_TX_TXE BIT(15) +#define BD_ENET_TX_UE BIT(13) +#define BD_ENET_TX_EE BIT(12) +#define BD_ENET_TX_FE BIT(11) +#define BD_ENET_TX_LCE BIT(10) +#define BD_ENET_TX_OE BIT(9) +#define BD_ENET_TX_TSE BIT(8) /* This device has up to three irqs on some platforms */ diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c index 95fad9862ef5..001db716cbf7 100644 --- a/drivers/net/ethernet/freescale/fec_main.c +++ b/drivers/net/ethernet/freescale/fec_main.c @@ -109,16 +109,6 @@ static void set_multicast_list(struct net_device *ndev); #define FEC_QUIRK_HAS_CSUM (1 << 5) /* Controller has hardware vlan support */ #define FEC_QUIRK_HAS_VLAN (1 << 6) -/* ENET IP errata ERR006358 - * - * If the ready bit in the transmit buffer descriptor (TxBD[R]) is previously - * detected as not set during a prior frame transmission, then the - * ENET_TDAR[TDAR] bit is cleared at a later time, even if additional TxBDs - * were added to the ring and the ENET_TDAR[TDAR] bit is set. This results in - * frames not being transmitted until there is a 0-to-1 transition on - * ENET_TDAR[TDAR]. - */ -#define FEC_QUIRK_ERR006358 (1 << 7) static struct platform_device_id fec_devtype[] = { { @@ -138,7 +128,7 @@ static struct platform_device_id fec_devtype[] = { .name = "imx6q-fec", .driver_data = FEC_QUIRK_ENET_MAC | FEC_QUIRK_HAS_GBIT | FEC_QUIRK_HAS_BUFDESC_EX | FEC_QUIRK_HAS_CSUM | - FEC_QUIRK_HAS_VLAN | FEC_QUIRK_ERR006358, + FEC_QUIRK_HAS_VLAN, }, { .name = "mvf600-fec", .driver_data = FEC_QUIRK_ENET_MAC, @@ -316,13 +306,16 @@ static void fec_dump(struct net_device *ndev) for (index = 0; index < fep->tx_ring_size; index++) { bdp = fec_enet_tx_get(index, fep); - pr_info("%3u %c%c 0x%04x 0x%08lx %4u %p\n", + pr_info("%3u %c%c 0x%04x 0x%08lx %4u %p", index, index == fep->tx_next ? 'S' : ' ', index == fep->tx_dirty ? 'H' : ' ', bdp->bd.cbd_sc, bdp->bd.cbd_bufaddr, bdp->bd.cbd_datlen, fep->tx_skbuff[index]); + if (fep->flags & FEC_FLAG_BUFDESC_EX) + pr_cont(" %08lx", bdp->ebd.cbd_esc); + pr_cont("\n"); } } @@ -877,13 +870,14 @@ fec_enet_tx(struct net_device *ndev) { struct fec_enet_private *fep = netdev_priv(ndev); union bufdesc_u *bdp; - unsigned short status; struct sk_buff *skb; unsigned int index = fep->tx_dirty; unsigned int pkts_compl, bytes_compl; pkts_compl = bytes_compl = 0; do { + unsigned status, cbd_esc; + if (++index >= fep->tx_ring_size) index = 0; @@ -904,25 +898,42 @@ fec_enet_tx(struct net_device *ndev) continue; /* Check for errors. */ - if (status & (BD_ENET_TX_HB | BD_ENET_TX_LC | - BD_ENET_TX_RL | BD_ENET_TX_UN | - BD_ENET_TX_CSL)) { - ndev->stats.tx_errors++; - if (status & BD_ENET_TX_HB) /* No heartbeat */ - ndev->stats.tx_heartbeat_errors++; - if (status & BD_ENET_TX_LC) /* Late collision */ - ndev->stats.tx_window_errors++; - if (status & BD_ENET_TX_RL) /* Retrans limit */ - ndev->stats.tx_aborted_errors++; - if (status & BD_ENET_TX_UN) /* Underrun */ - ndev->stats.tx_fifo_errors++; - if (status & BD_ENET_TX_CSL) /* Carrier lost */ - ndev->stats.tx_carrier_errors++; + if (fep->flags & FEC_FLAG_BUFDESC_EX) { + cbd_esc = bdp->ebd.cbd_esc; + if (cbd_esc & BD_ENET_TX_TXE) { + ndev->stats.tx_errors++; + if (cbd_esc & BD_ENET_TX_EE) { /* excess collision */ + ndev->stats.collisions += 16; + ndev->stats.tx_aborted_errors++; + } + if (cbd_esc & BD_ENET_TX_LCE) /* late collision error */ + ndev->stats.tx_window_errors++; + if (cbd_esc & (BD_ENET_TX_UE | BD_ENET_TX_FE | BD_ENET_TX_OE)) + ndev->stats.tx_fifo_errors++; + goto next; + } } else { - ndev->stats.tx_packets++; - ndev->stats.tx_bytes += skb->len; + if (status & (BD_ENET_TX_HB | BD_ENET_TX_LC | + BD_ENET_TX_RL | BD_ENET_TX_UN | + BD_ENET_TX_CSL)) { + ndev->stats.tx_errors++; + if (status & BD_ENET_TX_HB) /* No heartbeat */ + ndev->stats.tx_heartbeat_errors++; + if (status & BD_ENET_TX_LC) /* Late collision */ + ndev->stats.tx_window_errors++; + if (status & BD_ENET_TX_RL) /* Retrans limit */ + ndev->stats.tx_aborted_errors++; + if (status & BD_ENET_TX_UN) /* Underrun */ + ndev->stats.tx_fifo_errors++; + if (status & BD_ENET_TX_CSL) /* Carrier lost */ + ndev->stats.tx_carrier_errors++; + goto next; + } } + ndev->stats.tx_packets++; + ndev->stats.tx_bytes += skb->len; + if (fep->flags & FEC_FLAG_BUFDESC_EX && unlikely(skb_shinfo(skb)->tx_flags & SKBTX_IN_PROGRESS)) { struct skb_shared_hwtstamps shhwtstamps; @@ -937,6 +948,7 @@ fec_enet_tx(struct net_device *ndev) if (status & BD_ENET_TX_DEF) ndev->stats.collisions++; + next: pkts_compl++; bytes_compl += skb->len; @@ -969,7 +981,6 @@ fec_enet_rx(struct net_device *ndev, int budget) struct fec_enet_private *fep = netdev_priv(ndev); const struct platform_device_id *id_entry = platform_get_device_id(fep->pdev); - unsigned short status; struct sk_buff *skb; ushort pkt_len; __u8 *data; @@ -987,6 +998,7 @@ fec_enet_rx(struct net_device *ndev, int budget) */ do { union bufdesc_u *bdp = fec_enet_rx_get(index, fep); + unsigned status, cbd_esc; status = bdp->bd.cbd_sc; if (status & BD_ENET_RX_EMPTY) @@ -1056,10 +1068,17 @@ fec_enet_rx(struct net_device *ndev, int budget) if (id_entry->driver_data & FEC_QUIRK_SWAP_FRAME) swap_buffer(data, pkt_len); + if (fep->flags & FEC_FLAG_BUFDESC_EX) { + cbd_esc = bdp->ebd.cbd_esc; + if (!(fep->flags & FEC_FLAG_RX_VLAN)) + cbd_esc &= ~BD_ENET_RX_VLAN; + } else { + cbd_esc = 0; + } + /* If this is a VLAN packet remove the VLAN Tag */ vlan_packet_rcvd = false; - if (fep->flags & FEC_FLAG_RX_VLAN && - bdp->ebd.cbd_esc & BD_ENET_RX_VLAN) { + if (cbd_esc & BD_ENET_RX_VLAN) { /* Push and remove the vlan tag */ struct vlan_hdr *vlan_header = (struct vlan_hdr *) (data + ETH_HLEN); @@ -1096,7 +1115,7 @@ fec_enet_rx(struct net_device *ndev, int budget) skb_hwtstamps(skb)); if (fep->flags & FEC_FLAG_RX_CSUM) { - if (!(bdp->ebd.cbd_esc & FLAG_RX_CSUM_ERROR)) { + if (!(cbd_esc & FLAG_RX_CSUM_ERROR)) { /* don't check it */ skb->ip_summed = CHECKSUM_UNNECESSARY; } else { From rmk Mon Aug 15 18:57:33 2016 References: Message-ID: From: Russell King To: Linux Kernel Mailing list Date: Mon, 15 Aug 2016 18:57:33 +0100 Content-type: text/plain; charset="utf-8" Content-Disposition: inline Content-Transfer-Encoding: 8bit Mime-Version: 1.0 Subject: Re: [PATCH] net:fec: move quirks into fec_enet_private data structure From: Russell King Move the quirks bitmap into the fec_enet_private data structure so we don't need to keep reading it via a chain of pointers. Signed-off-by: Russell King --- drivers/net/ethernet/freescale/fec.h | 1 + drivers/net/ethernet/freescale/fec_main.c | 48 ++++++++++++------------------- 2 files changed, 19 insertions(+), 30 deletions(-) diff --git a/drivers/net/ethernet/freescale/fec.h b/drivers/net/ethernet/freescale/fec.h index 377477f18d01..99a5f695a11e 100644 --- a/drivers/net/ethernet/freescale/fec.h +++ b/drivers/net/ethernet/freescale/fec.h @@ -351,6 +351,7 @@ struct fec_enet_private { int hwts_tx_en; struct delayed_work time_keep; struct regulator *reg_phy; + unsigned long quirks; }; void fec_ptp_init(struct platform_device *pdev); diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c index 001db716cbf7..c0f166538738 100644 --- a/drivers/net/ethernet/freescale/fec_main.c +++ b/drivers/net/ethernet/freescale/fec_main.c @@ -434,8 +434,6 @@ fec_enet_txq_submit_frag_skb(struct sk_buff *skb, struct net_device *ndev, static bool fec_enet_txq_submit_skb(struct sk_buff *skb, struct net_device *ndev) { struct fec_enet_private *fep = netdev_priv(ndev); - const struct platform_device_id *id_entry = - platform_get_device_id(fep->pdev); union bufdesc_u *bdp; void *bufaddr; dma_addr_t addr; @@ -458,11 +456,11 @@ static bool fec_enet_txq_submit_skb(struct sk_buff *skb, struct net_device *ndev * and get it aligned. Ugh. */ if (((unsigned long) bufaddr) & FEC_ALIGNMENT || - id_entry->driver_data & FEC_QUIRK_SWAP_FRAME) { + fep->quirks & FEC_QUIRK_SWAP_FRAME) { memcpy(fep->tx_bounce[index], skb->data, buflen); bufaddr = fep->tx_bounce[index]; - if (id_entry->driver_data & FEC_QUIRK_SWAP_FRAME) + if (fep->quirks & FEC_QUIRK_SWAP_FRAME) swap_buffer(bufaddr, buflen); } @@ -622,8 +620,6 @@ static void fec_restart(struct net_device *ndev) { struct fec_enet_private *fep = netdev_priv(ndev); - const struct platform_device_id *id_entry = - platform_get_device_id(fep->pdev); u32 val; u32 temp_mac[2]; u32 rcntl = OPT_FRAME_SIZE | 0x04; @@ -637,7 +633,7 @@ fec_restart(struct net_device *ndev) * enet-mac reset will reset mac address registers too, * so need to reconfigure it. */ - if (id_entry->driver_data & FEC_QUIRK_ENET_MAC) { + if (fep->quirks & FEC_QUIRK_ENET_MAC) { memcpy(&temp_mac, ndev->dev_addr, ETH_ALEN); writel(cpu_to_be32(temp_mac[0]), fep->hwp + FEC_ADDR_LOW); writel(cpu_to_be32(temp_mac[1]), fep->hwp + FEC_ADDR_HIGH); @@ -684,7 +680,7 @@ fec_restart(struct net_device *ndev) * The phy interface and speed need to get configured * differently on enet-mac. */ - if (id_entry->driver_data & FEC_QUIRK_ENET_MAC) { + if (fep->quirks & FEC_QUIRK_ENET_MAC) { /* Enable flow control and length check */ rcntl |= 0x40000000; @@ -707,7 +703,7 @@ fec_restart(struct net_device *ndev) } } else { #ifdef FEC_MIIGSK_ENR - if (id_entry->driver_data & FEC_QUIRK_USE_GASKET) { + if (fep->quirks & FEC_QUIRK_USE_GASKET) { u32 cfgr; /* disable the gasket and wait */ writel(0, fep->hwp + FEC_MIIGSK_ENR); @@ -762,7 +758,7 @@ fec_restart(struct net_device *ndev) writel(0, fep->hwp + FEC_HASH_TABLE_LOW); #endif - if (id_entry->driver_data & FEC_QUIRK_ENET_MAC) { + if (fep->quirks & FEC_QUIRK_ENET_MAC) { /* enable ENET endian swap */ ecntl |= (1 << 8); /* enable ENET store and forward mode */ @@ -792,8 +788,6 @@ static void fec_stop(struct net_device *ndev) { struct fec_enet_private *fep = netdev_priv(ndev); - const struct platform_device_id *id_entry = - platform_get_device_id(fep->pdev); u32 rmii_mode = readl(fep->hwp + FEC_R_CNTRL) & (1 << 8); /* We cannot expect a graceful transmit stop without link !!! */ @@ -811,7 +805,7 @@ fec_stop(struct net_device *ndev) writel(FEC_DEFAULT_IMASK, fep->hwp + FEC_IMASK); /* We have to keep ENET enabled to have MII interrupt stay working */ - if (id_entry->driver_data & FEC_QUIRK_ENET_MAC) { + if (fep->quirks & FEC_QUIRK_ENET_MAC) { writel(2, fep->hwp + FEC_ECNTRL); writel(rmii_mode, fep->hwp + FEC_R_CNTRL); } @@ -979,8 +973,6 @@ static int fec_enet_rx(struct net_device *ndev, int budget) { struct fec_enet_private *fep = netdev_priv(ndev); - const struct platform_device_id *id_entry = - platform_get_device_id(fep->pdev); struct sk_buff *skb; ushort pkt_len; __u8 *data; @@ -1065,7 +1057,7 @@ fec_enet_rx(struct net_device *ndev, int budget) dma_sync_single_for_cpu(&fep->pdev->dev, bdp->bd.cbd_bufaddr, FEC_ENET_RX_FRSIZE, DMA_FROM_DEVICE); - if (id_entry->driver_data & FEC_QUIRK_SWAP_FRAME) + if (fep->quirks & FEC_QUIRK_SWAP_FRAME) swap_buffer(data, pkt_len); if (fep->flags & FEC_FLAG_BUFDESC_EX) { @@ -1525,8 +1517,6 @@ static void fec_enet_phy_config(struct net_device *ndev) static int fec_enet_mii_probe(struct net_device *ndev) { struct fec_enet_private *fep = netdev_priv(ndev); - const struct platform_device_id *id_entry = - platform_get_device_id(fep->pdev); struct phy_device *phy_dev = NULL; char mdio_bus_id[MII_BUS_ID_SIZE]; char phy_name[MII_BUS_ID_SIZE + 3]; @@ -1572,7 +1562,7 @@ static int fec_enet_mii_probe(struct net_device *ndev) } /* mask with MAC supported features */ - if (id_entry->driver_data & FEC_QUIRK_HAS_GBIT) { + if (fep->quirks & FEC_QUIRK_HAS_GBIT) { phy_dev->supported &= PHY_GBIT_FEATURES; phy_dev->supported &= ~SUPPORTED_1000baseT_Half; #if !defined(CONFIG_M5272) @@ -1602,8 +1592,6 @@ static int fec_enet_mii_init(struct platform_device *pdev) static struct mii_bus *fec0_mii_bus; struct net_device *ndev = platform_get_drvdata(pdev); struct fec_enet_private *fep = netdev_priv(ndev); - const struct platform_device_id *id_entry = - platform_get_device_id(fep->pdev); struct device_node *node; int err = -ENXIO, i; @@ -1623,7 +1611,7 @@ static int fec_enet_mii_init(struct platform_device *pdev) * mdio interface in board design, and need to be configured by * fec0 mii_bus. */ - if ((id_entry->driver_data & FEC_QUIRK_ENET_MAC) && fep->dev_id > 0) { + if ((fep->quirks & FEC_QUIRK_ENET_MAC) && fep->dev_id > 0) { /* fec1 uses fec0 mii_bus */ if (mii_cnt && fec0_mii_bus) { fep->mii_bus = fec0_mii_bus; @@ -1644,7 +1632,7 @@ static int fec_enet_mii_init(struct platform_device *pdev) * document. */ fep->phy_speed = DIV_ROUND_UP(clk_get_rate(fep->clk_ipg), 5000000); - if (id_entry->driver_data & FEC_QUIRK_ENET_MAC) + if (fep->quirks & FEC_QUIRK_ENET_MAC) fep->phy_speed--; fep->phy_speed <<= 1; writel(fep->phy_speed, fep->hwp + FEC_MII_SPEED); @@ -1686,7 +1674,7 @@ static int fec_enet_mii_init(struct platform_device *pdev) mii_cnt++; /* save fec0 mii_bus */ - if (id_entry->driver_data & FEC_QUIRK_ENET_MAC) + if (fep->quirks & FEC_QUIRK_ENET_MAC) fec0_mii_bus = fep->mii_bus; return 0; @@ -2459,8 +2447,6 @@ static const struct net_device_ops fec_netdev_ops = { static void fec_enet_init(struct net_device *ndev) { struct fec_enet_private *fep = netdev_priv(ndev); - const struct platform_device_id *id_entry = - platform_get_device_id(fep->pdev); /* init the tx & rx ring size */ fep->tx_ring_size = TX_RING_SIZE; @@ -2485,13 +2471,14 @@ static void fec_enet_init(struct net_device *ndev) netif_napi_add(ndev, &fep->napi, fec_enet_rx_napi, NAPI_POLL_WEIGHT); if (fep->flags & FEC_FLAG_BUFDESC_EX) { - if (id_entry->driver_data & FEC_QUIRK_HAS_VLAN) { + /* Features which require the enhanced buffer descriptors */ + if (fep->quirks & FEC_QUIRK_HAS_VLAN) { /* enable hw VLAN support */ ndev->features |= NETIF_F_HW_VLAN_CTAG_RX; fep->flags |= FEC_FLAG_RX_VLAN; } - if (id_entry->driver_data & FEC_QUIRK_HAS_CSUM) { + if (fep->quirks & FEC_QUIRK_HAS_CSUM) { /* enable hw accelerator */ ndev->features |= (NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM | NETIF_F_RXCSUM); @@ -2577,10 +2564,11 @@ fec_probe(struct platform_device *pdev) mutex_init(&fep->mutex); + if (pdev->id_entry) + fep->quirks = pdev->id_entry->driver_data; #if !defined(CONFIG_M5272) /* default enable pause frame auto negotiation */ - if (pdev->id_entry && - (pdev->id_entry->driver_data & FEC_QUIRK_HAS_GBIT)) + if (fep->quirks & FEC_QUIRK_HAS_GBIT) fep->pause_flag |= FEC_PAUSE_FLAG_AUTONEG | FEC_PAUSE_FLAG_TX | FEC_PAUSE_FLAG_RX; From rmk Mon Aug 15 18:57:33 2016 References: Message-ID: From: Russell King To: Linux Kernel Mailing list Date: Mon, 15 Aug 2016 18:57:33 +0100 Content-type: text/plain; charset="utf-8" Content-Disposition: inline Content-Transfer-Encoding: 8bit Mime-Version: 1.0 Subject: Re: [PATCH] net:fec: move skb allocation earlier From: Russell King Move the skb allocation before we sync the buffer for CPU access. In order to do this, we need to detect whether the packet contains a VLAN header so we can adjust the packet size appropriately. This allows us to tidy the code a little in the following patches. Signed-off-by: Russell King --- drivers/net/ethernet/freescale/fec_main.c | 45 ++++++++++++++++--------------- 1 file changed, 24 insertions(+), 21 deletions(-) diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c index c0f166538738..791d6f519256 100644 --- a/drivers/net/ethernet/freescale/fec_main.c +++ b/drivers/net/ethernet/freescale/fec_main.c @@ -977,7 +977,6 @@ fec_enet_rx(struct net_device *ndev, int budget) ushort pkt_len; __u8 *data; int pkt_received = 0; - bool vlan_packet_rcvd = false; u16 vlan_tag; unsigned int index = fep->rx_next; @@ -1053,13 +1052,6 @@ fec_enet_rx(struct net_device *ndev, int budget) pkt_len = bdp->bd.cbd_datlen - 4; ndev->stats.rx_bytes += pkt_len; - data = fep->rx_skbuff[index]->data; - dma_sync_single_for_cpu(&fep->pdev->dev, bdp->bd.cbd_bufaddr, - FEC_ENET_RX_FRSIZE, DMA_FROM_DEVICE); - - if (fep->quirks & FEC_QUIRK_SWAP_FRAME) - swap_buffer(data, pkt_len); - if (fep->flags & FEC_FLAG_BUFDESC_EX) { cbd_esc = bdp->ebd.cbd_esc; if (!(fep->flags & FEC_FLAG_RX_VLAN)) @@ -1068,32 +1060,43 @@ fec_enet_rx(struct net_device *ndev, int budget) cbd_esc = 0; } + /* + * Detect the presence of the VLANG tag, and just + * the packet length appropriately. + */ + if (cbd_esc & BD_ENET_RX_VLAN) + pkt_len -= VLAN_HLEN; + + /* This does 16 byte alignment, exactly what we need. */ + skb = netdev_alloc_skb(ndev, pkt_len + NET_IP_ALIGN); + if (unlikely(!skb)) { + ndev->stats.rx_dropped++; + goto rx_processing_done; + } + + data = fep->rx_skbuff[index]->data; + dma_sync_single_for_cpu(&fep->pdev->dev, bdp->bd.cbd_bufaddr, + FEC_ENET_RX_FRSIZE, DMA_FROM_DEVICE); + + if (fep->quirks & FEC_QUIRK_SWAP_FRAME) + swap_buffer(data, pkt_len); + /* If this is a VLAN packet remove the VLAN Tag */ - vlan_packet_rcvd = false; if (cbd_esc & BD_ENET_RX_VLAN) { /* Push and remove the vlan tag */ struct vlan_hdr *vlan_header = (struct vlan_hdr *) (data + ETH_HLEN); vlan_tag = ntohs(vlan_header->h_vlan_TCI); - pkt_len -= VLAN_HLEN; - - vlan_packet_rcvd = true; } - /* This does 16 byte alignment, exactly what we need. - */ - skb = netdev_alloc_skb(ndev, pkt_len + NET_IP_ALIGN); - - if (unlikely(!skb)) { - ndev->stats.rx_dropped++; - } else { + { int payload_offset = (2 * ETH_ALEN); skb_reserve(skb, NET_IP_ALIGN); skb_put(skb, pkt_len); /* Make room */ /* Extract the frame data without the VLAN header. */ skb_copy_to_linear_data(skb, data, (2 * ETH_ALEN)); - if (vlan_packet_rcvd) + if (cbd_esc & BD_ENET_RX_VLAN) payload_offset = (2 * ETH_ALEN) + VLAN_HLEN; skb_copy_to_linear_data_offset(skb, (2 * ETH_ALEN), data + payload_offset, @@ -1116,7 +1119,7 @@ fec_enet_rx(struct net_device *ndev, int budget) } /* Handle received VLAN packets */ - if (vlan_packet_rcvd) + if (cbd_esc & BD_ENET_RX_VLAN) __vlan_hwaccel_put_tag(skb, htons(ETH_P_8021Q), vlan_tag); From rmk Mon Aug 15 18:57:33 2016 References: Message-ID: From: Russell King To: Linux Kernel Mailing list Date: Mon, 15 Aug 2016 18:57:33 +0100 Content-type: text/plain; charset="utf-8" Content-Disposition: inline Content-Transfer-Encoding: 8bit Mime-Version: 1.0 Subject: Re: [PATCH] net:fec: extract and record vlan tag receive processing in one place From: Russell King Rather than having the vlan tag processing scattered in multiple different places, we can now localize the reading of the tag with storing the tag in the skb. Group this code together. Signed-off-by: Russell King --- drivers/net/ethernet/freescale/fec_main.c | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c index 791d6f519256..ab10480718a8 100644 --- a/drivers/net/ethernet/freescale/fec_main.c +++ b/drivers/net/ethernet/freescale/fec_main.c @@ -977,7 +977,6 @@ fec_enet_rx(struct net_device *ndev, int budget) ushort pkt_len; __u8 *data; int pkt_received = 0; - u16 vlan_tag; unsigned int index = fep->rx_next; #ifdef CONFIG_M532x @@ -1084,9 +1083,12 @@ fec_enet_rx(struct net_device *ndev, int budget) /* If this is a VLAN packet remove the VLAN Tag */ if (cbd_esc & BD_ENET_RX_VLAN) { /* Push and remove the vlan tag */ - struct vlan_hdr *vlan_header = + struct vlan_hdr *vlan = (struct vlan_hdr *) (data + ETH_HLEN); - vlan_tag = ntohs(vlan_header->h_vlan_TCI); + + __vlan_hwaccel_put_tag(skb, + htons(ETH_P_8021Q), + ntohs(vlan->h_vlan_TCI)); } { @@ -1118,12 +1120,6 @@ fec_enet_rx(struct net_device *ndev, int budget) } } - /* Handle received VLAN packets */ - if (cbd_esc & BD_ENET_RX_VLAN) - __vlan_hwaccel_put_tag(skb, - htons(ETH_P_8021Q), - vlan_tag); - napi_gro_receive(&fep->napi, skb); } From rmk Mon Aug 15 18:57:33 2016 References: Message-ID: From: Russell King To: Linux Kernel Mailing list Date: Mon, 15 Aug 2016 18:57:33 +0100 Content-type: text/plain; charset="utf-8" Content-Disposition: inline Content-Transfer-Encoding: 8bit Mime-Version: 1.0 Subject: Re: [PATCH] net:fec: simplify packet data copying and vlan tag removal From: Russell King Implement this as two distinct code paths: this allows non-tagged traffic to be copied in one go, whereas tagged traffic needs to be copied in two separate chunks. Signed-off-by: Russell King --- drivers/net/ethernet/freescale/fec_main.c | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c index ab10480718a8..d26c29c914ff 100644 --- a/drivers/net/ethernet/freescale/fec_main.c +++ b/drivers/net/ethernet/freescale/fec_main.c @@ -1080,6 +1080,9 @@ fec_enet_rx(struct net_device *ndev, int budget) if (fep->quirks & FEC_QUIRK_SWAP_FRAME) swap_buffer(data, pkt_len); + skb_reserve(skb, NET_IP_ALIGN); + skb_put(skb, pkt_len); /* Make room */ + /* If this is a VLAN packet remove the VLAN Tag */ if (cbd_esc & BD_ENET_RX_VLAN) { /* Push and remove the vlan tag */ @@ -1089,21 +1092,18 @@ fec_enet_rx(struct net_device *ndev, int budget) __vlan_hwaccel_put_tag(skb, htons(ETH_P_8021Q), ntohs(vlan->h_vlan_TCI)); - } - - { - int payload_offset = (2 * ETH_ALEN); - skb_reserve(skb, NET_IP_ALIGN); - skb_put(skb, pkt_len); /* Make room */ /* Extract the frame data without the VLAN header. */ - skb_copy_to_linear_data(skb, data, (2 * ETH_ALEN)); - if (cbd_esc & BD_ENET_RX_VLAN) - payload_offset = (2 * ETH_ALEN) + VLAN_HLEN; - skb_copy_to_linear_data_offset(skb, (2 * ETH_ALEN), - data + payload_offset, - pkt_len - (2 * ETH_ALEN)); + skb_copy_to_linear_data(skb, data, 2 * ETH_ALEN); + skb_copy_to_linear_data_offset(skb, 2 * ETH_ALEN, + data + 2 * ETH_ALEN + + VLAN_HLEN, + pkt_len - 2 * ETH_ALEN); + } else { + skb_copy_to_linear_data(skb, data, pkt_len); + } + { skb->protocol = eth_type_trans(skb, ndev); /* Get receive timestamp from the skb */ From rmk Mon Aug 15 18:57:33 2016 References: Message-ID: From: Russell King To: Linux Kernel Mailing list Date: Mon, 15 Aug 2016 18:57:33 +0100 Content-type: text/plain; charset="utf-8" Content-Disposition: inline Content-Transfer-Encoding: 8bit Mime-Version: 1.0 Subject: Re: [PATCH] net:fec: move final receive to separate function From: Russell King Move the final receive code sequence to a separate function. Performance tests show that having lots of code integrated into one big function can make the driver performance very dependent on compiler behaviour, and more reliable performance can be obtained by separating this out. Signed-off-by: Russell King --- drivers/net/ethernet/freescale/fec_main.c | 43 +++++++++++++++++-------------- 1 file changed, 24 insertions(+), 19 deletions(-) diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c index d26c29c914ff..d90fd50f8881 100644 --- a/drivers/net/ethernet/freescale/fec_main.c +++ b/drivers/net/ethernet/freescale/fec_main.c @@ -964,6 +964,29 @@ fec_enet_tx(struct net_device *ndev) netif_wake_queue(ndev); } +static void +fec_enet_receive(struct sk_buff *skb, union bufdesc_u *bdp, struct net_device *ndev) +{ + struct fec_enet_private *fep = netdev_priv(ndev); + + skb->protocol = eth_type_trans(skb, ndev); + + /* Get receive timestamp from the skb */ + if (fep->hwts_rx_en && fep->flags & FEC_FLAG_BUFDESC_EX) + fec_enet_hwtstamp(fep, bdp->ebd.ts, skb_hwtstamps(skb)); + + if (fep->flags & FEC_FLAG_RX_CSUM) { + if (!(bdp->ebd.cbd_esc & FLAG_RX_CSUM_ERROR)) { + /* don't check it */ + skb->ip_summed = CHECKSUM_UNNECESSARY; + } else { + skb_checksum_none_assert(skb); + } + } + + napi_gro_receive(&fep->napi, skb); +} + /* During a receive, the rx_next points to the current incoming buffer. * When we update through the ring, if the next incoming buffer has * not been given to the system, we just set the empty indicator, @@ -1103,25 +1126,7 @@ fec_enet_rx(struct net_device *ndev, int budget) skb_copy_to_linear_data(skb, data, pkt_len); } - { - skb->protocol = eth_type_trans(skb, ndev); - - /* Get receive timestamp from the skb */ - if (fep->hwts_rx_en && fep->flags & FEC_FLAG_BUFDESC_EX) - fec_enet_hwtstamp(fep, bdp->ebd.ts, - skb_hwtstamps(skb)); - - if (fep->flags & FEC_FLAG_RX_CSUM) { - if (!(cbd_esc & FLAG_RX_CSUM_ERROR)) { - /* don't check it */ - skb->ip_summed = CHECKSUM_UNNECESSARY; - } else { - skb_checksum_none_assert(skb); - } - } - - napi_gro_receive(&fep->napi, skb); - } + fec_enet_receive(skb, bdp, ndev); dma_sync_single_for_device(&fep->pdev->dev, bdp->bd.cbd_bufaddr, FEC_ENET_RX_FRSIZE, DMA_FROM_DEVICE); From rmk Mon Aug 15 18:57:33 2016 References: Message-ID: From: Russell King To: Linux Kernel Mailing list Date: Mon, 15 Aug 2016 18:57:33 +0100 Content-type: text/plain; charset="utf-8" Content-Disposition: inline Content-Transfer-Encoding: 8bit Mime-Version: 1.0 Subject: Re: [PATCH] net:fec: move syncing of receive buffer back to device directly after copy From: Russell King Move the syncing for the DMA buffer back to the device immediately after we've done with copying data from it. This means the logic in the receive path handing the copying of data is now located together. Signed-off-by: Russell King --- drivers/net/ethernet/freescale/fec_main.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c index d90fd50f8881..496e60228521 100644 --- a/drivers/net/ethernet/freescale/fec_main.c +++ b/drivers/net/ethernet/freescale/fec_main.c @@ -1126,10 +1126,11 @@ fec_enet_rx(struct net_device *ndev, int budget) skb_copy_to_linear_data(skb, data, pkt_len); } + dma_sync_single_for_device(&fep->pdev->dev, bdp->bd.cbd_bufaddr, + FEC_ENET_RX_FRSIZE, DMA_FROM_DEVICE); + fec_enet_receive(skb, bdp, ndev); - dma_sync_single_for_device(&fep->pdev->dev, bdp->bd.cbd_bufaddr, - FEC_ENET_RX_FRSIZE, DMA_FROM_DEVICE); rx_processing_done: if (fep->flags & FEC_FLAG_BUFDESC_EX) { bdp->ebd.cbd_esc = BD_ENET_RX_INT; From rmk Mon Aug 15 18:57:33 2016 References: Message-ID: From: Russell King To: Linux Kernel Mailing list Date: Mon, 15 Aug 2016 18:57:33 +0100 Content-type: text/plain; charset="utf-8" Content-Disposition: inline Content-Transfer-Encoding: 8bit Mime-Version: 1.0 Subject: Re: [PATCH] net:fec: implement almost zero-copy receive path From: Russell King iMX6 SoCs have an IP accelerator which can pad and strip two bytes in the FIFO at the beginning of each packet. Since the FEC has alignment restrictions on the buffer addresses, this feature is particularly interesting for receive as it allows us to receive an appropriately aligned packet for IP. This allows us to reduce the CPU overhead by avoiding copying large packets - and in the spirit of all network drivers which use this trick, we provide a copybreak tunable which allows the packet size to be copied to be adjusted. Signed-off-by: Russell King --- drivers/net/ethernet/freescale/fec_main.c | 195 +++++++++++++++++++++++------- 1 file changed, 149 insertions(+), 46 deletions(-) diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c index 496e60228521..45ceac59ae61 100644 --- a/drivers/net/ethernet/freescale/fec_main.c +++ b/drivers/net/ethernet/freescale/fec_main.c @@ -109,6 +109,8 @@ static void set_multicast_list(struct net_device *ndev); #define FEC_QUIRK_HAS_CSUM (1 << 5) /* Controller has hardware vlan support */ #define FEC_QUIRK_HAS_VLAN (1 << 6) +/* Controller has ability to offset rx packets */ +#define FEC_QUIRK_RX_SHIFT16 (1 << 8) static struct platform_device_id fec_devtype[] = { { @@ -128,7 +130,7 @@ static struct platform_device_id fec_devtype[] = { .name = "imx6q-fec", .driver_data = FEC_QUIRK_ENET_MAC | FEC_QUIRK_HAS_GBIT | FEC_QUIRK_HAS_BUFDESC_EX | FEC_QUIRK_HAS_CSUM | - FEC_QUIRK_HAS_VLAN, + FEC_QUIRK_HAS_VLAN | FEC_QUIRK_RX_SHIFT16, }, { .name = "mvf600-fec", .driver_data = FEC_QUIRK_ENET_MAC, @@ -207,6 +209,7 @@ MODULE_PARM_DESC(macaddr, "FEC Ethernet MAC address"); /* FEC receive acceleration */ #define FEC_RACC_IPDIS (1 << 1) #define FEC_RACC_PRODIS (1 << 2) +#define FEC_RACC_SHIFT16 BIT(7) #define FEC_RACC_OPTIONS (FEC_RACC_IPDIS | FEC_RACC_PRODIS) /* @@ -247,6 +250,20 @@ MODULE_PARM_DESC(macaddr, "FEC Ethernet MAC address"); static int mii_cnt; +static unsigned copybreak = 200; +module_param(copybreak, uint, 0644); +MODULE_PARM_DESC(copybreak, + "Maximum size of packet that is copied to a new buffer on receive"); + +static bool fec_enet_rx_zerocopy(struct fec_enet_private *fep, unsigned pktlen) +{ +#ifndef CONFIG_M5272 + if (fep->quirks & FEC_QUIRK_RX_SHIFT16 && pktlen >= copybreak) + return true; +#endif + return false; +} + static union bufdesc_u * fec_enet_tx_get(unsigned int index, struct fec_enet_private *fep) { @@ -669,6 +686,8 @@ fec_restart(struct net_device *ndev) #if !defined(CONFIG_M5272) /* set RX checksum */ val = readl(fep->hwp + FEC_RACC); + if (fep->quirks & FEC_QUIRK_RX_SHIFT16) + val |= FEC_RACC_SHIFT16; if (fep->flags & FEC_FLAG_RX_CSUM) val |= FEC_RACC_OPTIONS; else @@ -987,6 +1006,68 @@ fec_enet_receive(struct sk_buff *skb, union bufdesc_u *bdp, struct net_device *n napi_gro_receive(&fep->napi, skb); } +static void +fec_enet_receive_nocopy(unsigned pkt_len, unsigned index, union bufdesc_u *bdp, + struct net_device *ndev) +{ + struct fec_enet_private *fep = netdev_priv(ndev); + struct sk_buff *skb, *skb_new; + unsigned char *data; + dma_addr_t addr; + + skb_new = netdev_alloc_skb(ndev, FEC_ENET_RX_FRSIZE); + if (!skb_new) { + ndev->stats.rx_dropped++; + return; + } + + addr = dma_map_single(&fep->pdev->dev, skb_new->data, + FEC_ENET_RX_FRSIZE, DMA_FROM_DEVICE); + if (dma_mapping_error(&fep->pdev->dev, addr)) { + dev_kfree_skb(skb_new); + ndev->stats.rx_dropped++; + return; + } + + /* We have the new skb, so proceed to deal with the received data. */ + dma_unmap_single(&fep->pdev->dev, bdp->bd.cbd_bufaddr, + FEC_ENET_RX_FRSIZE, DMA_FROM_DEVICE); + + skb = fep->rx_skbuff[index]; + + /* Now subsitute in the new skb */ + fep->rx_skbuff[index] = skb_new; + bdp->bd.cbd_bufaddr = addr; + + /* + * Update the skb length according to the raw packet length. + * Then remove the two bytes of additional padding. + */ + skb_put(skb, pkt_len); + data = skb_pull_inline(skb, 2); + + if (fep->quirks & FEC_QUIRK_SWAP_FRAME) + swap_buffer(data, skb->len); + + /* + * Now juggle things for the VLAN tag - if the hardware + * flags this as present, we need to read the tag, and + * then shuffle the ethernet addresses up. + */ + if (ndev->features & NETIF_F_HW_VLAN_CTAG_RX && + bdp->ebd.cbd_esc & BD_ENET_RX_VLAN) { + struct vlan_hdr *vlan = (struct vlan_hdr *)(data + ETH_HLEN); + + __vlan_hwaccel_put_tag(skb, htons(ETH_P_8021Q), + ntohs(vlan->h_vlan_TCI)); + + memmove(data + VLAN_HLEN, data, 2 * ETH_ALEN); + skb_pull_inline(skb, VLAN_HLEN); + } + + fec_enet_receive(skb, bdp, ndev); +} + /* During a receive, the rx_next points to the current incoming buffer. * When we update through the ring, if the next incoming buffer has * not been given to the system, we just set the empty indicator, @@ -996,7 +1077,6 @@ static int fec_enet_rx(struct net_device *ndev, int budget) { struct fec_enet_private *fep = netdev_priv(ndev); - struct sk_buff *skb; ushort pkt_len; __u8 *data; int pkt_received = 0; @@ -1012,6 +1092,7 @@ fec_enet_rx(struct net_device *ndev, int budget) do { union bufdesc_u *bdp = fec_enet_rx_get(index, fep); unsigned status, cbd_esc; + struct sk_buff *skb; status = bdp->bd.cbd_sc; if (status & BD_ENET_RX_EMPTY) @@ -1074,62 +1155,84 @@ fec_enet_rx(struct net_device *ndev, int budget) pkt_len = bdp->bd.cbd_datlen - 4; ndev->stats.rx_bytes += pkt_len; - if (fep->flags & FEC_FLAG_BUFDESC_EX) { - cbd_esc = bdp->ebd.cbd_esc; - if (!(fep->flags & FEC_FLAG_RX_VLAN)) - cbd_esc &= ~BD_ENET_RX_VLAN; + if (fec_enet_rx_zerocopy(fep, pkt_len)) { + fec_enet_receive_nocopy(pkt_len, index, bdp, ndev); } else { - cbd_esc = 0; - } + if (fep->flags & FEC_FLAG_BUFDESC_EX) { + cbd_esc = bdp->ebd.cbd_esc; + if (!(fep->flags & FEC_FLAG_RX_VLAN)) + cbd_esc &= ~BD_ENET_RX_VLAN; + } else { + cbd_esc = 0; + } - /* - * Detect the presence of the VLANG tag, and just - * the packet length appropriately. - */ - if (cbd_esc & BD_ENET_RX_VLAN) - pkt_len -= VLAN_HLEN; + /* + * Detect the presence of the VLANG tag, and just + * the packet length appropriately. + */ + if (cbd_esc & BD_ENET_RX_VLAN) + pkt_len -= VLAN_HLEN; - /* This does 16 byte alignment, exactly what we need. */ - skb = netdev_alloc_skb(ndev, pkt_len + NET_IP_ALIGN); - if (unlikely(!skb)) { - ndev->stats.rx_dropped++; - goto rx_processing_done; - } + /* + * This does 16 byte alignment, exactly what we need. + */ + skb = netdev_alloc_skb(ndev, pkt_len + NET_IP_ALIGN); + if (unlikely(!skb)) { + ndev->stats.rx_dropped++; + goto rx_processing_done; + } - data = fep->rx_skbuff[index]->data; - dma_sync_single_for_cpu(&fep->pdev->dev, bdp->bd.cbd_bufaddr, - FEC_ENET_RX_FRSIZE, DMA_FROM_DEVICE); + dma_sync_single_for_cpu(&fep->pdev->dev, + bdp->bd.cbd_bufaddr, + FEC_ENET_RX_FRSIZE, + DMA_FROM_DEVICE); - if (fep->quirks & FEC_QUIRK_SWAP_FRAME) - swap_buffer(data, pkt_len); + data = fep->rx_skbuff[index]->data; - skb_reserve(skb, NET_IP_ALIGN); - skb_put(skb, pkt_len); /* Make room */ +#ifndef CONFIG_M5272 + /* + * If we have enabled this feature, we need to discard + * the two bytes at the beginning of the packet before + * copying it. + */ + if (fep->quirks & FEC_QUIRK_RX_SHIFT16) { + pkt_len -= 2; + data += 2; + } +#endif - /* If this is a VLAN packet remove the VLAN Tag */ - if (cbd_esc & BD_ENET_RX_VLAN) { - /* Push and remove the vlan tag */ - struct vlan_hdr *vlan = + if (fep->quirks & FEC_QUIRK_SWAP_FRAME) + swap_buffer(data, pkt_len); + + skb_reserve(skb, NET_IP_ALIGN); + skb_put(skb, pkt_len); /* Make room */ + + /* If this is a VLAN packet remove the VLAN Tag */ + if (cbd_esc & BD_ENET_RX_VLAN) { + /* Push and remove the vlan tag */ + struct vlan_hdr *vlan = (struct vlan_hdr *) (data + ETH_HLEN); - __vlan_hwaccel_put_tag(skb, - htons(ETH_P_8021Q), - ntohs(vlan->h_vlan_TCI)); + __vlan_hwaccel_put_tag(skb, + htons(ETH_P_8021Q), + ntohs(vlan->h_vlan_TCI)); - /* Extract the frame data without the VLAN header. */ - skb_copy_to_linear_data(skb, data, 2 * ETH_ALEN); - skb_copy_to_linear_data_offset(skb, 2 * ETH_ALEN, - data + 2 * ETH_ALEN + - VLAN_HLEN, - pkt_len - 2 * ETH_ALEN); - } else { - skb_copy_to_linear_data(skb, data, pkt_len); - } + /* Extract the frame data without the VLAN header. */ + skb_copy_to_linear_data(skb, data, 2 * ETH_ALEN); + skb_copy_to_linear_data_offset(skb, 2 * ETH_ALEN, + data + 2 * ETH_ALEN + VLAN_HLEN, + pkt_len - 2 * ETH_ALEN); + } else { + skb_copy_to_linear_data(skb, data, pkt_len); + } - dma_sync_single_for_device(&fep->pdev->dev, bdp->bd.cbd_bufaddr, - FEC_ENET_RX_FRSIZE, DMA_FROM_DEVICE); + dma_sync_single_for_device(&fep->pdev->dev, + bdp->bd.cbd_bufaddr, + FEC_ENET_RX_FRSIZE, + DMA_FROM_DEVICE); - fec_enet_receive(skb, bdp, ndev); + fec_enet_receive(skb, bdp, ndev); + } rx_processing_done: if (fep->flags & FEC_FLAG_BUFDESC_EX) { From rmk Mon Aug 15 18:57:33 2016 References: Message-ID: From: Russell King To: Linux Kernel Mailing list Date: Mon, 15 Aug 2016 18:57:33 +0100 Content-type: text/plain; charset="utf-8" Content-Disposition: inline Content-Transfer-Encoding: 8bit Mime-Version: 1.0 Subject: Re: [PATCH] net:fec: move copying receive function out of line From: Russell King Signed-off-by: Russell King --- drivers/net/ethernet/freescale/fec_main.c | 149 +++++++++++++++--------------- 1 file changed, 72 insertions(+), 77 deletions(-) diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c index 45ceac59ae61..578f0bdac309 100644 --- a/drivers/net/ethernet/freescale/fec_main.c +++ b/drivers/net/ethernet/freescale/fec_main.c @@ -1007,6 +1007,76 @@ fec_enet_receive(struct sk_buff *skb, union bufdesc_u *bdp, struct net_device *n } static void +fec_enet_receive_copy(unsigned pkt_len, unsigned index, union bufdesc_u *bdp, struct net_device *ndev) +{ + struct fec_enet_private *fep = netdev_priv(ndev); + struct sk_buff *skb; + unsigned char *data; + bool vlan_packet_rcvd = false; + + /* + * Detect the presence of the VLAN tag, and adjust + * the packet length appropriately. + */ + if (fep->flags & FEC_FLAG_RX_VLAN && + bdp->ebd.cbd_esc & BD_ENET_RX_VLAN) { + pkt_len -= VLAN_HLEN; + vlan_packet_rcvd = true; + } + + /* This does 16 byte alignment, exactly what we need. */ + skb = netdev_alloc_skb(ndev, pkt_len + NET_IP_ALIGN); + if (unlikely(!skb)) { + ndev->stats.rx_dropped++; + return; + } + + dma_sync_single_for_cpu(&fep->pdev->dev, bdp->bd.cbd_bufaddr, + FEC_ENET_RX_FRSIZE, DMA_FROM_DEVICE); + + data = fep->rx_skbuff[index]->data; + +#ifndef CONFIG_M5272 + /* + * If we have enabled this feature, we need to discard + * the two bytes at the beginning of the packet before + * copying it. + */ + if (fep->quirks & FEC_QUIRK_RX_SHIFT16) { + pkt_len -= 2; + data += 2; + } +#endif + + if (fep->quirks & FEC_QUIRK_SWAP_FRAME) + swap_buffer(data, pkt_len); + + skb_reserve(skb, NET_IP_ALIGN); + skb_put(skb, pkt_len); /* Make room */ + + /* If this is a VLAN packet remove the VLAN Tag */ + if (vlan_packet_rcvd) { + struct vlan_hdr *vlan = (struct vlan_hdr *)(data + ETH_HLEN); + + __vlan_hwaccel_put_tag(skb, htons(ETH_P_8021Q), + ntohs(vlan->h_vlan_TCI)); + + /* Extract the frame data without the VLAN header. */ + skb_copy_to_linear_data(skb, data, 2 * ETH_ALEN); + skb_copy_to_linear_data_offset(skb, 2 * ETH_ALEN, + data + 2 * ETH_ALEN + VLAN_HLEN, + pkt_len - 2 * ETH_ALEN); + } else { + skb_copy_to_linear_data(skb, data, pkt_len); + } + + dma_sync_single_for_device(&fep->pdev->dev, bdp->bd.cbd_bufaddr, + FEC_ENET_RX_FRSIZE, DMA_FROM_DEVICE); + + fec_enet_receive(skb, bdp, ndev); +} + +static void fec_enet_receive_nocopy(unsigned pkt_len, unsigned index, union bufdesc_u *bdp, struct net_device *ndev) { @@ -1078,7 +1148,6 @@ fec_enet_rx(struct net_device *ndev, int budget) { struct fec_enet_private *fep = netdev_priv(ndev); ushort pkt_len; - __u8 *data; int pkt_received = 0; unsigned int index = fep->rx_next; @@ -1091,8 +1160,7 @@ fec_enet_rx(struct net_device *ndev, int budget) */ do { union bufdesc_u *bdp = fec_enet_rx_get(index, fep); - unsigned status, cbd_esc; - struct sk_buff *skb; + unsigned status; status = bdp->bd.cbd_sc; if (status & BD_ENET_RX_EMPTY) @@ -1158,80 +1226,7 @@ fec_enet_rx(struct net_device *ndev, int budget) if (fec_enet_rx_zerocopy(fep, pkt_len)) { fec_enet_receive_nocopy(pkt_len, index, bdp, ndev); } else { - if (fep->flags & FEC_FLAG_BUFDESC_EX) { - cbd_esc = bdp->ebd.cbd_esc; - if (!(fep->flags & FEC_FLAG_RX_VLAN)) - cbd_esc &= ~BD_ENET_RX_VLAN; - } else { - cbd_esc = 0; - } - - /* - * Detect the presence of the VLANG tag, and just - * the packet length appropriately. - */ - if (cbd_esc & BD_ENET_RX_VLAN) - pkt_len -= VLAN_HLEN; - - /* - * This does 16 byte alignment, exactly what we need. - */ - skb = netdev_alloc_skb(ndev, pkt_len + NET_IP_ALIGN); - if (unlikely(!skb)) { - ndev->stats.rx_dropped++; - goto rx_processing_done; - } - - dma_sync_single_for_cpu(&fep->pdev->dev, - bdp->bd.cbd_bufaddr, - FEC_ENET_RX_FRSIZE, - DMA_FROM_DEVICE); - - data = fep->rx_skbuff[index]->data; - -#ifndef CONFIG_M5272 - /* - * If we have enabled this feature, we need to discard - * the two bytes at the beginning of the packet before - * copying it. - */ - if (fep->quirks & FEC_QUIRK_RX_SHIFT16) { - pkt_len -= 2; - data += 2; - } -#endif - - if (fep->quirks & FEC_QUIRK_SWAP_FRAME) - swap_buffer(data, pkt_len); - - skb_reserve(skb, NET_IP_ALIGN); - skb_put(skb, pkt_len); /* Make room */ - - /* If this is a VLAN packet remove the VLAN Tag */ - if (cbd_esc & BD_ENET_RX_VLAN) { - /* Push and remove the vlan tag */ - struct vlan_hdr *vlan = - (struct vlan_hdr *) (data + ETH_HLEN); - - __vlan_hwaccel_put_tag(skb, - htons(ETH_P_8021Q), - ntohs(vlan->h_vlan_TCI)); - - /* Extract the frame data without the VLAN header. */ - skb_copy_to_linear_data(skb, data, 2 * ETH_ALEN); - skb_copy_to_linear_data_offset(skb, 2 * ETH_ALEN, - data + 2 * ETH_ALEN + VLAN_HLEN, - pkt_len - 2 * ETH_ALEN); - } else { - skb_copy_to_linear_data(skb, data, pkt_len); - } - - dma_sync_single_for_device(&fep->pdev->dev, - bdp->bd.cbd_bufaddr, - FEC_ENET_RX_FRSIZE, - DMA_FROM_DEVICE); - - fec_enet_receive(skb, bdp, ndev); + fec_enet_receive_copy(pkt_len, index, bdp, ndev); } rx_processing_done: From rmk Mon Aug 15 18:57:33 2016 References: Message-ID: From: Russell King To: Linux Kernel Mailing list Date: Mon, 15 Aug 2016 18:57:33 +0100 Content-type: text/plain; charset="utf-8" Content-Disposition: inline Content-Transfer-Encoding: 8bit Mime-Version: 1.0 Subject: Re: [PATCH] Rest From: Russell King --- arch/arm/mm/fault.c | 1 + drivers/net/ethernet/freescale/fec.h | 2 ++ drivers/net/ethernet/freescale/fec_main.c | 35 ++++++++++++++++++++++++------- kernel/softirq.c | 11 +++++++--- 4 files changed, 39 insertions(+), 10 deletions(-) diff --git a/arch/arm/mm/fault.c b/arch/arm/mm/fault.c index eb8830a4c5ed..f3961a71d1f4 100644 --- a/arch/arm/mm/fault.c +++ b/arch/arm/mm/fault.c @@ -553,6 +553,7 @@ do_DataAbort(unsigned long addr, unsigned int fsr, struct pt_regs *regs) printk(KERN_ALERT "Unhandled fault: %s (0x%03x) at 0x%08lx\n", inf->name, fsr, addr); + WARN_ON(1); info.si_signo = inf->sig; info.si_errno = 0; diff --git a/drivers/net/ethernet/freescale/fec.h b/drivers/net/ethernet/freescale/fec.h index 99a5f695a11e..c4a245071d77 100644 --- a/drivers/net/ethernet/freescale/fec.h +++ b/drivers/net/ethernet/freescale/fec.h @@ -352,6 +352,8 @@ struct fec_enet_private { struct delayed_work time_keep; struct regulator *reg_phy; unsigned long quirks; + u64 stop_ns; + u64 start_ns; }; void fec_ptp_init(struct platform_device *pdev); diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c index 578f0bdac309..299b1f494065 100644 --- a/drivers/net/ethernet/freescale/fec_main.c +++ b/drivers/net/ethernet/freescale/fec_main.c @@ -254,6 +254,10 @@ static unsigned copybreak = 200; module_param(copybreak, uint, 0644); MODULE_PARM_DESC(copybreak, "Maximum size of packet that is copied to a new buffer on receive"); +static unsigned long total_stopped; +module_param(total_stopped, ulong, 0644); +static unsigned long start_latency; +module_param(start_latency, ulong, 0644); static bool fec_enet_rx_zerocopy(struct fec_enet_private *fep, unsigned pktlen) { @@ -457,6 +461,15 @@ static bool fec_enet_txq_submit_skb(struct sk_buff *skb, struct net_device *ndev unsigned short status; unsigned int index, buflen, estatus; + if (fep->start_ns) { + unsigned delta = sched_clock() - fep->start_ns; + if (start_latency == 0 && delta < 1000000) + WARN_ON(1); + if (delta < 1000000) + start_latency += delta; + fep->start_ns = 0; + } + /* Protocol checksum off-load for TCP and UDP. */ if (fec_enet_clear_csum(skb, ndev)) return false; @@ -576,8 +589,10 @@ fec_enet_start_xmit(struct sk_buff *skb, struct net_device *ndev) return NETDEV_TX_OK; } - if (ring_free(fep->tx_next, fep->tx_dirty, fep->tx_ring_size) < fep->tx_min) + if (ring_free(fep->tx_next, fep->tx_dirty, fep->tx_ring_size) < fep->tx_min) { + fep->stop_ns = sched_clock(); netif_stop_queue(ndev); + } return NETDEV_TX_OK; } @@ -979,8 +994,15 @@ fec_enet_tx(struct net_device *ndev) if (netif_queue_stopped(ndev) && ring_free(fep->tx_next, fep->tx_dirty, fep->tx_ring_size) >= - fep->tx_min) + fep->tx_min) { + total_stopped += sched_clock() - fep->stop_ns; + if (start_latency == 0) { +// WARN_ON(1); +extern int __trace2; __trace2 = 1; + } netif_wake_queue(ndev); + fep->start_ns = sched_clock(); + } } static void @@ -1006,7 +1028,7 @@ fec_enet_receive(struct sk_buff *skb, union bufdesc_u *bdp, struct net_device *n napi_gro_receive(&fep->napi, skb); } -static void +static void noinline fec_enet_receive_copy(unsigned pkt_len, unsigned index, union bufdesc_u *bdp, struct net_device *ndev) { struct fec_enet_private *fep = netdev_priv(ndev); @@ -1076,7 +1098,7 @@ fec_enet_receive_copy(unsigned pkt_len, unsigned index, union bufdesc_u *bdp, st fec_enet_receive(skb, bdp, ndev); } -static void +static void noinline fec_enet_receive_nocopy(unsigned pkt_len, unsigned index, union bufdesc_u *bdp, struct net_device *ndev) { @@ -1143,11 +1165,10 @@ fec_enet_receive_nocopy(unsigned pkt_len, unsigned index, union bufdesc_u *bdp, * not been given to the system, we just set the empty indicator, * effectively tossing the packet. */ -static int +static int noinline fec_enet_rx(struct net_device *ndev, int budget) { struct fec_enet_private *fep = netdev_priv(ndev); - ushort pkt_len; int pkt_received = 0; unsigned int index = fep->rx_next; @@ -1160,7 +1181,7 @@ fec_enet_rx(struct net_device *ndev, int budget) */ do { union bufdesc_u *bdp = fec_enet_rx_get(index, fep); - unsigned status; + unsigned status, pkt_len; status = bdp->bd.cbd_sc; if (status & BD_ENET_RX_EMPTY) diff --git a/kernel/softirq.c b/kernel/softirq.c index 5918d227730f..65a6d6ff15dc 100644 --- a/kernel/softirq.c +++ b/kernel/softirq.c @@ -222,7 +222,7 @@ static inline void lockdep_softirq_end(bool in_hardirq) static inline bool lockdep_softirq_start(void) { return false; } static inline void lockdep_softirq_end(bool in_hardirq) { } #endif - +int __trace2; asmlinkage __visible void __do_softirq(void) { unsigned long end = jiffies + MAX_SOFTIRQ_TIME; @@ -259,7 +259,7 @@ asmlinkage __visible void __do_softirq(void) int prev_count; h += softirq_bit - 1; - +if (__trace2) printk("%s: running %pf\n", __func__, h->action); vec_nr = h - softirq_vec; prev_count = preempt_count(); @@ -282,6 +282,8 @@ asmlinkage __visible void __do_softirq(void) local_irq_disable(); pending = local_softirq_pending(); +if (__trace2) + printk("%s: pending=0x%08x\n", __func__, pending); if (pending) { if (time_before(jiffies, end) && !need_resched() && --max_restart) @@ -289,7 +291,10 @@ asmlinkage __visible void __do_softirq(void) wakeup_softirqd(); } - +if (__trace2) + printk("%s: pending=0x%08x jiffies=%lu end=%lu nrs=%u restart=%d\n", + __func__, pending, jiffies, end, need_resched(), max_restart); +__trace2 = 0; lockdep_softirq_end(in_hardirq); account_irq_exit_time(current); __local_bh_enable(SOFTIRQ_OFFSET); From rmk Mon Aug 15 18:57:33 2016 References: Message-ID: From: Russell King To: Linux Kernel Mailing list Date: Mon, 15 Aug 2016 18:57:33 +0100 Content-type: text/plain; charset="utf-8" Content-Disposition: inline Content-Transfer-Encoding: 8bit Mime-Version: 1.0 Subject: Re: [PATCH] net: fec: try better napi packet processing algorithm From: Russell King Signed-off-by: Russell King --- drivers/net/ethernet/freescale/fec_main.c | 33 +++++++++++++++++++------------ 1 file changed, 20 insertions(+), 13 deletions(-) diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c index 299b1f494065..cff4a73e9574 100644 --- a/drivers/net/ethernet/freescale/fec_main.c +++ b/drivers/net/ethernet/freescale/fec_main.c @@ -893,7 +893,7 @@ fec_enet_hwtstamp(struct fec_enet_private *fep, unsigned ts, hwtstamps->hwtstamp = ns_to_ktime(ns); } -static void noinline +static int noinline fec_enet_tx(struct net_device *ndev) { struct fec_enet_private *fep = netdev_priv(ndev); @@ -1003,6 +1003,8 @@ extern int __trace2; __trace2 = 1; netif_wake_queue(ndev); fep->start_ns = sched_clock(); } + + return pkts_compl; } static void @@ -1198,8 +1200,6 @@ fec_enet_rx(struct net_device *ndev, int budget) if ((status & BD_ENET_RX_LAST) == 0) netdev_err(ndev, "rcv is not +last\n"); - writel(FEC_ENET_RXF, fep->hwp + FEC_IEVENT); - /* Check for errors. */ if (status & BD_ENET_RX_ERROR) { ndev->stats.rx_errors++; @@ -1316,23 +1316,30 @@ static int fec_enet_rx_napi(struct napi_struct *napi, int budget) { struct net_device *ndev = napi->dev; struct fec_enet_private *fep = netdev_priv(ndev); - int pkts; + int rx_work, rx_pkts, tx_work, tx_pkts; - /* - * Clear any pending transmit or receive interrupts before - * processing the rings to avoid racing with the hardware. - */ - writel(FEC_ENET_RXF | FEC_ENET_TXF, fep->hwp + FEC_IEVENT); + for (rx_work = tx_work = 0; rx_work < budget; ) { + /* + * Clear any pending transmit or receive interrupts before + * processing the rings to avoid racing with the hardware. + */ + writel(FEC_ENET_RXF | FEC_ENET_TXF, fep->hwp + FEC_IEVENT); - pkts = fec_enet_rx(ndev, budget); + rx_pkts = fec_enet_rx(ndev, budget - total_work); + tx_pkts = fec_enet_tx(ndev); - fec_enet_tx(ndev); + if (!rx_pkts && !tx_pkts) + break; + + rx_work += rx_pkts; + tx_work += tx_pkts; + } - if (pkts < budget) { + if (rx_work < budget) { napi_complete(napi); writel(FEC_DEFAULT_IMASK, fep->hwp + FEC_IMASK); } - return pkts; + return rx_work; } /* ------------------------------------------------------------------------- */