In my patch series for splitting out the serial code from ioc3-eth
by using a MFD device there was one big patch for ioc3-eth.c,
which wasn't really usefull for reviews. This series contains the
ioc3-eth changes splitted in smaller steps and few more cleanups.
Only the conversion to MFD will be done later in a different series.
Changes in v2:
- use net_err_ratelimited for printing various ioc3 errors
- added missing clearing of rx buf valid flags into ioc3_alloc_rings
- use __func__ for printing out of memory messages
Thomas Bogendoerfer (15):
MIPS: SGI-IP27: remove ioc3 ethernet init
MIPS: SGI-IP27: restructure ioc3 register access
net: sgi: ioc3-eth: remove checkpatch errors/warning
net: sgi: ioc3-eth: use defines for constants dealing with desc rings
net: sgi: ioc3-eth: allocate space for desc rings only once
net: sgi: ioc3-eth: get rid of ioc3_clean_rx_ring()
net: sgi: ioc3-eth: separate tx and rx ring handling
net: sgi: ioc3-eth: introduce chip start function
net: sgi: ioc3-eth: split ring cleaning/freeing and allocation
net: sgi: ioc3-eth: refactor rx buffer allocation
net: sgi: ioc3-eth: use dma-direct for dma allocations
net: sgi: ioc3-eth: use csum_fold
net: sgi: ioc3-eth: Fix IPG settings
net: sgi: ioc3-eth: protect emcr in all cases
net: sgi: ioc3-eth: no need to stop queue set_multicast_list
arch/mips/include/asm/sn/ioc3.h | 357 +++++-------
arch/mips/sgi-ip27/ip27-console.c | 5 +-
arch/mips/sgi-ip27/ip27-init.c | 13 -
drivers/net/ethernet/sgi/ioc3-eth.c | 1039 ++++++++++++++++++-----------------
4 files changed, 667 insertions(+), 747 deletions(-)
--
2.13.7
Removed not needed disabling of ethernet interrupts in IP27 platform code.
Acked-by: Paul Burton <[email protected]>
Signed-off-by: Thomas Bogendoerfer <[email protected]>
---
arch/mips/sgi-ip27/ip27-init.c | 13 -------------
1 file changed, 13 deletions(-)
diff --git a/arch/mips/sgi-ip27/ip27-init.c b/arch/mips/sgi-ip27/ip27-init.c
index 066b33f50bcc..59d5375c9021 100644
--- a/arch/mips/sgi-ip27/ip27-init.c
+++ b/arch/mips/sgi-ip27/ip27-init.c
@@ -130,17 +130,6 @@ cnodeid_t get_compact_nodeid(void)
return NASID_TO_COMPACT_NODEID(get_nasid());
}
-static inline void ioc3_eth_init(void)
-{
- struct ioc3 *ioc3;
- nasid_t nid;
-
- nid = get_nasid();
- ioc3 = (struct ioc3 *) KL_CONFIG_CH_CONS_INFO(nid)->memory_base;
-
- ioc3->eier = 0;
-}
-
extern void ip27_reboot_setup(void);
void __init plat_mem_setup(void)
@@ -182,8 +171,6 @@ void __init plat_mem_setup(void)
panic("Kernel compiled for N mode.");
#endif
- ioc3_eth_init();
-
ioport_resource.start = 0;
ioport_resource.end = ~0UL;
set_io_port_base(IO_BASE);
--
2.13.7
Move common code for rx buffer setup into ioc3_alloc_skb and deal
with allocation failures. Also clean up allocation size calculation.
Signed-off-by: Thomas Bogendoerfer <[email protected]>
---
drivers/net/ethernet/sgi/ioc3-eth.c | 95 ++++++++++++++++++-------------------
1 file changed, 45 insertions(+), 50 deletions(-)
diff --git a/drivers/net/ethernet/sgi/ioc3-eth.c b/drivers/net/ethernet/sgi/ioc3-eth.c
index 7d2372ef7872..9cb04ac283e4 100644
--- a/drivers/net/ethernet/sgi/ioc3-eth.c
+++ b/drivers/net/ethernet/sgi/ioc3-eth.c
@@ -11,11 +11,8 @@
*
* To do:
*
- * o Handle allocation failures in ioc3_alloc_skb() more gracefully.
- * o Handle allocation failures in ioc3_init_rings().
* o Use prefetching for large packets. What is a good lower limit for
* prefetching?
- * o We're probably allocating a bit too much memory.
* o Use hardware checksums.
* o Convert to using a IOC3 meta driver.
* o Which PHYs might possibly be attached to the IOC3 in real live,
@@ -72,6 +69,13 @@
#define TX_RING_ENTRIES 128
#define TX_RING_MASK (TX_RING_ENTRIES - 1)
+/* IOC3 does dma transfers in 128 byte blocks */
+#define IOC3_DMA_XFER_LEN 128UL
+
+/* Every RX buffer starts with 8 byte descriptor data */
+#define RX_OFFSET (sizeof(struct ioc3_erxbuf) + NET_IP_ALIGN)
+#define RX_BUF_SIZE (13 * IOC3_DMA_XFER_LEN)
+
#define ETCSR_FD ((17 << ETCSR_IPGR2_SHIFT) | (11 << ETCSR_IPGR1_SHIFT) | 21)
#define ETCSR_HD ((21 << ETCSR_IPGR2_SHIFT) | (21 << ETCSR_IPGR1_SHIFT) | 21)
@@ -108,36 +112,38 @@ static inline unsigned int ioc3_hash(const unsigned char *addr);
static void ioc3_start(struct ioc3_private *ip);
static inline void ioc3_stop(struct ioc3_private *ip);
static void ioc3_init(struct net_device *dev);
-static void ioc3_alloc_rx_bufs(struct net_device *dev);
+static int ioc3_alloc_rx_bufs(struct net_device *dev);
static void ioc3_free_rx_bufs(struct ioc3_private *ip);
static inline void ioc3_clean_tx_ring(struct ioc3_private *ip);
static const char ioc3_str[] = "IOC3 Ethernet";
static const struct ethtool_ops ioc3_ethtool_ops;
-/* We use this to acquire receive skb's that we can DMA directly into. */
-
-#define IOC3_CACHELINE 128UL
static inline unsigned long aligned_rx_skb_addr(unsigned long addr)
{
- return (~addr + 1) & (IOC3_CACHELINE - 1UL);
+ return (~addr + 1) & (IOC3_DMA_XFER_LEN - 1UL);
}
-static inline struct sk_buff *ioc3_alloc_skb(unsigned long length,
- unsigned int gfp_mask)
+static inline int ioc3_alloc_skb(struct sk_buff **skb, struct ioc3_erxbuf **rxb)
{
- struct sk_buff *skb;
+ struct sk_buff *new_skb;
+ int offset;
- skb = alloc_skb(length + IOC3_CACHELINE - 1, gfp_mask);
- if (likely(skb)) {
- int offset = aligned_rx_skb_addr((unsigned long)skb->data);
+ new_skb = alloc_skb(RX_BUF_SIZE + IOC3_DMA_XFER_LEN - 1, GFP_ATOMIC);
+ if (!new_skb)
+ return -ENOMEM;
- if (offset)
- skb_reserve(skb, offset);
- }
+ /* ensure buffer is aligned to IOC3_DMA_XFER_LEN */
+ offset = aligned_rx_skb_addr((unsigned long)new_skb->data);
+ if (offset)
+ skb_reserve(new_skb, offset);
+
+ *rxb = (struct ioc3_erxbuf *)new_skb->data;
+ skb_reserve(new_skb, RX_OFFSET);
+ *skb = new_skb;
- return skb;
+ return 0;
}
static inline unsigned long ioc3_map(void *ptr, unsigned long vdev)
@@ -151,13 +157,6 @@ static inline unsigned long ioc3_map(void *ptr, unsigned long vdev)
return virt_to_bus(ptr);
#endif
}
-
-/* BEWARE: The IOC3 documentation documents the size of rx buffers as
- * 1644 while it's actually 1664. This one was nasty to track down ...
- */
-#define RX_OFFSET 10
-#define RX_BUF_ALLOC_SIZE (1664 + RX_OFFSET + IOC3_CACHELINE)
-
#define IOC3_SIZE 0x100000
static inline u32 mcr_pack(u32 pulse, u32 sample)
@@ -538,11 +537,10 @@ static inline void ioc3_rx(struct net_device *dev)
err = be32_to_cpu(rxb->err); /* It's valid ... */
if (err & ERXBUF_GOODPKT) {
len = ((w0 >> ERXBUF_BYTECNT_SHIFT) & 0x7ff) - 4;
- skb_trim(skb, len);
+ skb_put(skb, len);
skb->protocol = eth_type_trans(skb, dev);
- new_skb = ioc3_alloc_skb(RX_BUF_ALLOC_SIZE, GFP_ATOMIC);
- if (!new_skb) {
+ if (ioc3_alloc_skb(&new_skb, &rxb)) {
/* Ouch, drop packet and just recycle packet
* to keep the ring filled.
*/
@@ -560,11 +558,6 @@ static inline void ioc3_rx(struct net_device *dev)
ip->rx_skbs[rx_entry] = NULL; /* Poison */
- /* Because we reserve afterwards. */
- skb_put(new_skb, (1664 + RX_OFFSET));
- rxb = (struct ioc3_erxbuf *)new_skb->data;
- skb_reserve(new_skb, RX_OFFSET);
-
dev->stats.rx_packets++; /* Statistics */
dev->stats.rx_bytes += len;
} else {
@@ -667,7 +660,11 @@ static void ioc3_error(struct net_device *dev, u32 eisr)
ioc3_clean_tx_ring(ip);
ioc3_init(dev);
- ioc3_alloc_rx_bufs(dev);
+ if (ioc3_alloc_rx_bufs(dev)) {
+ netdev_err(dev, "%s: rx buffer allocation failed\n", __func__);
+ spin_unlock(&ip->ioc3_lock);
+ return;
+ }
ioc3_start(ip);
ioc3_mii_init(ip);
@@ -804,7 +801,7 @@ static void ioc3_free_rx_bufs(struct ioc3_private *ip)
}
}
-static void ioc3_alloc_rx_bufs(struct net_device *dev)
+static int ioc3_alloc_rx_bufs(struct net_device *dev)
{
struct ioc3_private *ip = netdev_priv(dev);
struct ioc3_erxbuf *rxb;
@@ -815,25 +812,16 @@ static void ioc3_alloc_rx_bufs(struct net_device *dev)
* this for performance and memory later.
*/
for (i = 0; i < RX_BUFFS; i++) {
- struct sk_buff *skb;
-
- skb = ioc3_alloc_skb(RX_BUF_ALLOC_SIZE, GFP_ATOMIC);
- if (!skb) {
- show_free_areas(0, NULL);
- continue;
- }
-
- ip->rx_skbs[i] = skb;
+ if (ioc3_alloc_skb(&ip->rx_skbs[i], &rxb))
+ return -ENOMEM;
- /* Because we reserve afterwards. */
- skb_put(skb, (1664 + RX_OFFSET));
- rxb = (struct ioc3_erxbuf *)skb->data;
rxb->w0 = 0; /* Clear valid flag */
ip->rxr[i] = cpu_to_be64(ioc3_map(rxb, 1));
- skb_reserve(skb, RX_OFFSET);
}
ip->rx_ci = 0;
ip->rx_pi = RX_BUFFS;
+
+ return 0;
}
static inline void ioc3_ssram_disc(struct ioc3_private *ip)
@@ -945,7 +933,10 @@ static int ioc3_open(struct net_device *dev)
ip->ehar_l = 0;
ioc3_init(dev);
- ioc3_alloc_rx_bufs(dev);
+ if (ioc3_alloc_rx_bufs(dev)) {
+ netdev_err(dev, "%s: rx buffer allocation failed\n", __func__);
+ return -ENOMEM;
+ }
ioc3_start(ip);
ioc3_mii_start(ip);
@@ -1436,7 +1427,11 @@ static void ioc3_timeout(struct net_device *dev)
ioc3_clean_tx_ring(ip);
ioc3_init(dev);
- ioc3_alloc_rx_bufs(dev);
+ if (ioc3_alloc_rx_bufs(dev)) {
+ netdev_err(dev, "%s: rx buffer allocation failed\n", __func__);
+ spin_unlock_irq(&ip->ioc3_lock);
+ return;
+ }
ioc3_start(ip);
ioc3_mii_init(ip);
ioc3_mii_start(ip);
--
2.13.7
Do tx ring cleaning and freeing of rx buffers, when chip is shutdown and
allocate buffers before bringing chip up.
Signed-off-by: Thomas Bogendoerfer <[email protected]>
---
drivers/net/ethernet/sgi/ioc3-eth.c | 28 +++++++++++++---------------
1 file changed, 13 insertions(+), 15 deletions(-)
diff --git a/drivers/net/ethernet/sgi/ioc3-eth.c b/drivers/net/ethernet/sgi/ioc3-eth.c
index 05db0d1aeb04..7d2372ef7872 100644
--- a/drivers/net/ethernet/sgi/ioc3-eth.c
+++ b/drivers/net/ethernet/sgi/ioc3-eth.c
@@ -108,6 +108,9 @@ static inline unsigned int ioc3_hash(const unsigned char *addr);
static void ioc3_start(struct ioc3_private *ip);
static inline void ioc3_stop(struct ioc3_private *ip);
static void ioc3_init(struct net_device *dev);
+static void ioc3_alloc_rx_bufs(struct net_device *dev);
+static void ioc3_free_rx_bufs(struct ioc3_private *ip);
+static inline void ioc3_clean_tx_ring(struct ioc3_private *ip);
static const char ioc3_str[] = "IOC3 Ethernet";
static const struct ethtool_ops ioc3_ethtool_ops;
@@ -660,7 +663,11 @@ static void ioc3_error(struct net_device *dev, u32 eisr)
net_err_ratelimited("%s: TX PCI error.\n", dev->name);
ioc3_stop(ip);
+ ioc3_free_rx_bufs(ip);
+ ioc3_clean_tx_ring(ip);
+
ioc3_init(dev);
+ ioc3_alloc_rx_bufs(dev);
ioc3_start(ip);
ioc3_mii_init(ip);
@@ -829,16 +836,6 @@ static void ioc3_alloc_rx_bufs(struct net_device *dev)
ip->rx_pi = RX_BUFFS;
}
-static void ioc3_init_rings(struct net_device *dev)
-{
- struct ioc3_private *ip = netdev_priv(dev);
-
- ioc3_free_rx_bufs(ip);
- ioc3_alloc_rx_bufs(dev);
-
- ioc3_clean_tx_ring(ip);
-}
-
static inline void ioc3_ssram_disc(struct ioc3_private *ip)
{
struct ioc3_ethregs *regs = ip->regs;
@@ -891,8 +888,6 @@ static void ioc3_init(struct net_device *dev)
writel(ip->ehar_h, ®s->ehar_h);
writel(ip->ehar_l, ®s->ehar_l);
writel(42, ®s->ersr); /* XXX should be random */
-
- ioc3_init_rings(dev);
}
static void ioc3_start(struct ioc3_private *ip)
@@ -948,7 +943,9 @@ static int ioc3_open(struct net_device *dev)
ip->ehar_h = 0;
ip->ehar_l = 0;
+
ioc3_init(dev);
+ ioc3_alloc_rx_bufs(dev);
ioc3_start(ip);
ioc3_mii_start(ip);
@@ -1218,7 +1215,6 @@ static int ioc3_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
}
ioc3_init(dev);
- ioc3_start(ip);
ip->pdev = pdev;
@@ -1269,9 +1265,7 @@ static int ioc3_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
return 0;
out_stop:
- ioc3_stop(ip);
del_timer_sync(&ip->ioc3_timer);
- ioc3_free_rx_bufs(ip);
kfree(ip->rxr);
kfree(ip->txr);
out_res:
@@ -1438,7 +1432,11 @@ static void ioc3_timeout(struct net_device *dev)
spin_lock_irq(&ip->ioc3_lock);
ioc3_stop(ip);
+ ioc3_free_rx_bufs(ip);
+ ioc3_clean_tx_ring(ip);
+
ioc3_init(dev);
+ ioc3_alloc_rx_bufs(dev);
ioc3_start(ip);
ioc3_mii_init(ip);
ioc3_mii_start(ip);
--
2.13.7
Memory for descriptor rings are allocated/freed, when interface is
brought up/down. Since the size of the rings is not changeable by
hardware, we now allocate rings now during probe and free it, when
device is removed.
Signed-off-by: Thomas Bogendoerfer <[email protected]>
---
drivers/net/ethernet/sgi/ioc3-eth.c | 103 ++++++++++++++++++------------------
1 file changed, 51 insertions(+), 52 deletions(-)
diff --git a/drivers/net/ethernet/sgi/ioc3-eth.c b/drivers/net/ethernet/sgi/ioc3-eth.c
index ba18a53fbbe6..d9d94a55ac34 100644
--- a/drivers/net/ethernet/sgi/ioc3-eth.c
+++ b/drivers/net/ethernet/sgi/ioc3-eth.c
@@ -803,25 +803,17 @@ static void ioc3_free_rings(struct ioc3_private *ip)
struct sk_buff *skb;
int rx_entry, n_entry;
- if (ip->txr) {
- ioc3_clean_tx_ring(ip);
- free_pages((unsigned long)ip->txr, 2);
- ip->txr = NULL;
- }
+ ioc3_clean_tx_ring(ip);
- if (ip->rxr) {
- n_entry = ip->rx_ci;
- rx_entry = ip->rx_pi;
+ n_entry = ip->rx_ci;
+ rx_entry = ip->rx_pi;
- while (n_entry != rx_entry) {
- skb = ip->rx_skbs[n_entry];
- if (skb)
- dev_kfree_skb_any(skb);
+ while (n_entry != rx_entry) {
+ skb = ip->rx_skbs[n_entry];
+ if (skb)
+ dev_kfree_skb_any(skb);
- n_entry = (n_entry + 1) & RX_RING_MASK;
- }
- free_page((unsigned long)ip->rxr);
- ip->rxr = NULL;
+ n_entry = (n_entry + 1) & RX_RING_MASK;
}
}
@@ -829,49 +821,34 @@ static void ioc3_alloc_rings(struct net_device *dev)
{
struct ioc3_private *ip = netdev_priv(dev);
struct ioc3_erxbuf *rxb;
- unsigned long *rxr;
int i;
- if (!ip->rxr) {
- /* Allocate and initialize rx ring. 4kb = 512 entries */
- ip->rxr = (unsigned long *)get_zeroed_page(GFP_ATOMIC);
- rxr = ip->rxr;
- if (!rxr)
- pr_err("%s: get_zeroed_page() failed!\n", __func__);
-
- /* Now the rx buffers. The RX ring may be larger but
- * we only allocate 16 buffers for now. Need to tune
- * this for performance and memory later.
- */
- for (i = 0; i < RX_BUFFS; i++) {
- struct sk_buff *skb;
+ /* Now the rx buffers. The RX ring may be larger but
+ * we only allocate 16 buffers for now. Need to tune
+ * this for performance and memory later.
+ */
+ for (i = 0; i < RX_BUFFS; i++) {
+ struct sk_buff *skb;
- skb = ioc3_alloc_skb(RX_BUF_ALLOC_SIZE, GFP_ATOMIC);
- if (!skb) {
- show_free_areas(0, NULL);
- continue;
- }
+ skb = ioc3_alloc_skb(RX_BUF_ALLOC_SIZE, GFP_ATOMIC);
+ if (!skb) {
+ show_free_areas(0, NULL);
+ continue;
+ }
- ip->rx_skbs[i] = skb;
+ ip->rx_skbs[i] = skb;
- /* Because we reserve afterwards. */
- skb_put(skb, (1664 + RX_OFFSET));
- rxb = (struct ioc3_erxbuf *)skb->data;
- rxr[i] = cpu_to_be64(ioc3_map(rxb, 1));
- skb_reserve(skb, RX_OFFSET);
- }
- ip->rx_ci = 0;
- ip->rx_pi = RX_BUFFS;
+ /* Because we reserve afterwards. */
+ skb_put(skb, (1664 + RX_OFFSET));
+ rxb = (struct ioc3_erxbuf *)skb->data;
+ ip->rxr[i] = cpu_to_be64(ioc3_map(rxb, 1));
+ skb_reserve(skb, RX_OFFSET);
}
+ ip->rx_ci = 0;
+ ip->rx_pi = RX_BUFFS;
- if (!ip->txr) {
- /* Allocate and initialize tx rings. 16kb = 128 bufs. */
- ip->txr = (struct ioc3_etxd *)__get_free_pages(GFP_KERNEL, 2);
- if (!ip->txr)
- pr_err("%s: __get_free_pages() failed!\n", __func__);
- ip->tx_pi = 0;
- ip->tx_ci = 0;
- }
+ ip->tx_pi = 0;
+ ip->tx_ci = 0;
}
static void ioc3_init_rings(struct net_device *dev)
@@ -1239,6 +1216,23 @@ static int ioc3_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
timer_setup(&ip->ioc3_timer, ioc3_timer, 0);
ioc3_stop(ip);
+
+ /* Allocate and rx ring. 4kb = 512 entries */
+ ip->rxr = (unsigned long *)get_zeroed_page(GFP_ATOMIC);
+ if (!ip->rxr) {
+ pr_err("ioc3-eth: rx ring allocation failed\n");
+ err = -ENOMEM;
+ goto out_stop;
+ }
+
+ /* Allocate tx rings. 16kb = 128 bufs. */
+ ip->txr = (struct ioc3_etxd *)__get_free_pages(GFP_KERNEL, 2);
+ if (!ip->txr) {
+ pr_err("ioc3-eth: tx ring allocation failed\n");
+ err = -ENOMEM;
+ goto out_stop;
+ }
+
ioc3_init(dev);
ip->pdev = pdev;
@@ -1293,6 +1287,8 @@ static int ioc3_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
ioc3_stop(ip);
del_timer_sync(&ip->ioc3_timer);
ioc3_free_rings(ip);
+ kfree(ip->rxr);
+ kfree(ip->txr);
out_res:
pci_release_regions(pdev);
out_free:
@@ -1310,6 +1306,9 @@ static void ioc3_remove_one(struct pci_dev *pdev)
struct net_device *dev = pci_get_drvdata(pdev);
struct ioc3_private *ip = netdev_priv(dev);
+ kfree(ip->rxr);
+ kfree(ip->txr);
+
unregister_netdev(dev);
del_timer_sync(&ip->ioc3_timer);
--
2.13.7
ioc3_init did everything from reset to init rings to starting the chip.
This change move out chip start into a new function as preparation
for easier handling of receive buffer allocation failures.
Signed-off-by: Thomas Bogendoerfer <[email protected]>
---
drivers/net/ethernet/sgi/ioc3-eth.c | 49 ++++++++++++++++++++++---------------
1 file changed, 29 insertions(+), 20 deletions(-)
diff --git a/drivers/net/ethernet/sgi/ioc3-eth.c b/drivers/net/ethernet/sgi/ioc3-eth.c
index de20f644e07d..05db0d1aeb04 100644
--- a/drivers/net/ethernet/sgi/ioc3-eth.c
+++ b/drivers/net/ethernet/sgi/ioc3-eth.c
@@ -105,6 +105,7 @@ static void ioc3_set_multicast_list(struct net_device *dev);
static netdev_tx_t ioc3_start_xmit(struct sk_buff *skb, struct net_device *dev);
static void ioc3_timeout(struct net_device *dev);
static inline unsigned int ioc3_hash(const unsigned char *addr);
+static void ioc3_start(struct ioc3_private *ip);
static inline void ioc3_stop(struct ioc3_private *ip);
static void ioc3_init(struct net_device *dev);
@@ -660,6 +661,7 @@ static void ioc3_error(struct net_device *dev, u32 eisr)
ioc3_stop(ip);
ioc3_init(dev);
+ ioc3_start(ip);
ioc3_mii_init(ip);
netif_wake_queue(dev);
@@ -830,31 +832,11 @@ static void ioc3_alloc_rx_bufs(struct net_device *dev)
static void ioc3_init_rings(struct net_device *dev)
{
struct ioc3_private *ip = netdev_priv(dev);
- struct ioc3_ethregs *regs = ip->regs;
- unsigned long ring;
ioc3_free_rx_bufs(ip);
ioc3_alloc_rx_bufs(dev);
ioc3_clean_tx_ring(ip);
-
- /* Now the rx ring base, consume & produce registers. */
- ring = ioc3_map(ip->rxr, 0);
- writel(ring >> 32, ®s->erbr_h);
- writel(ring & 0xffffffff, ®s->erbr_l);
- writel(ip->rx_ci << 3, ®s->ercir);
- writel((ip->rx_pi << 3) | ERPIR_ARM, ®s->erpir);
-
- ring = ioc3_map(ip->txr, 0);
-
- ip->txqlen = 0; /* nothing queued */
-
- /* Now the tx ring base, consume & produce registers. */
- writel(ring >> 32, ®s->etbr_h);
- writel(ring & 0xffffffff, ®s->etbr_l);
- writel(ip->tx_pi << 7, ®s->etpir);
- writel(ip->tx_ci << 7, ®s->etcir);
- readl(®s->etcir); /* Flush */
}
static inline void ioc3_ssram_disc(struct ioc3_private *ip)
@@ -911,6 +893,30 @@ static void ioc3_init(struct net_device *dev)
writel(42, ®s->ersr); /* XXX should be random */
ioc3_init_rings(dev);
+}
+
+static void ioc3_start(struct ioc3_private *ip)
+{
+ struct ioc3_ethregs *regs = ip->regs;
+ unsigned long ring;
+
+ /* Now the rx ring base, consume & produce registers. */
+ ring = ioc3_map(ip->rxr, 0);
+ writel(ring >> 32, ®s->erbr_h);
+ writel(ring & 0xffffffff, ®s->erbr_l);
+ writel(ip->rx_ci << 3, ®s->ercir);
+ writel((ip->rx_pi << 3) | ERPIR_ARM, ®s->erpir);
+
+ ring = ioc3_map(ip->txr, 0);
+
+ ip->txqlen = 0; /* nothing queued */
+
+ /* Now the tx ring base, consume & produce registers. */
+ writel(ring >> 32, ®s->etbr_h);
+ writel(ring & 0xffffffff, ®s->etbr_l);
+ writel(ip->tx_pi << 7, ®s->etpir);
+ writel(ip->tx_ci << 7, ®s->etcir);
+ readl(®s->etcir); /* Flush */
ip->emcr |= ((RX_OFFSET / 2) << EMCR_RXOFF_SHIFT) | EMCR_TXDMAEN |
EMCR_TXEN | EMCR_RXDMAEN | EMCR_RXEN | EMCR_PADEN;
@@ -943,6 +949,7 @@ static int ioc3_open(struct net_device *dev)
ip->ehar_h = 0;
ip->ehar_l = 0;
ioc3_init(dev);
+ ioc3_start(ip);
ioc3_mii_start(ip);
netif_start_queue(dev);
@@ -1211,6 +1218,7 @@ static int ioc3_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
}
ioc3_init(dev);
+ ioc3_start(ip);
ip->pdev = pdev;
@@ -1431,6 +1439,7 @@ static void ioc3_timeout(struct net_device *dev)
ioc3_stop(ip);
ioc3_init(dev);
+ ioc3_start(ip);
ioc3_mii_init(ip);
ioc3_mii_start(ip);
--
2.13.7
emcr in private struct wasn't always protected by spinlock.
Signed-off-by: Thomas Bogendoerfer <[email protected]>
---
drivers/net/ethernet/sgi/ioc3-eth.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/drivers/net/ethernet/sgi/ioc3-eth.c b/drivers/net/ethernet/sgi/ioc3-eth.c
index 00942b37a1e4..d2b6659adba6 100644
--- a/drivers/net/ethernet/sgi/ioc3-eth.c
+++ b/drivers/net/ethernet/sgi/ioc3-eth.c
@@ -729,6 +729,8 @@ static inline void ioc3_setup_duplex(struct ioc3_private *ip)
{
struct ioc3_ethregs *regs = ip->regs;
+ spin_lock_irq(&ip->ioc3_lock);
+
if (ip->mii.full_duplex) {
writel(ETCSR_FD, ®s->etcsr);
ip->emcr |= EMCR_DUPLEX;
@@ -737,6 +739,8 @@ static inline void ioc3_setup_duplex(struct ioc3_private *ip)
ip->emcr &= ~EMCR_DUPLEX;
}
writel(ip->emcr, ®s->emcr);
+
+ spin_unlock_irq(&ip->ioc3_lock);
}
static void ioc3_timer(struct timer_list *t)
@@ -1628,6 +1632,8 @@ static void ioc3_set_multicast_list(struct net_device *dev)
netif_stop_queue(dev); /* Lock out others. */
+ spin_lock_irq(&ip->ioc3_lock);
+
if (dev->flags & IFF_PROMISC) { /* Set promiscuous. */
ip->emcr |= EMCR_PROMISC;
writel(ip->emcr, ®s->emcr);
@@ -1656,6 +1662,8 @@ static void ioc3_set_multicast_list(struct net_device *dev)
writel(ip->ehar_l, ®s->ehar_l);
}
+ spin_unlock_irq(&ip->ioc3_lock);
+
netif_wake_queue(dev); /* Let us get going again. */
}
--
2.13.7
The half/full duplex settings for inter packet gap counters/timer were
reversed.
Signed-off-by: Thomas Bogendoerfer <[email protected]>
---
drivers/net/ethernet/sgi/ioc3-eth.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/sgi/ioc3-eth.c b/drivers/net/ethernet/sgi/ioc3-eth.c
index 0c2713bba741..00942b37a1e4 100644
--- a/drivers/net/ethernet/sgi/ioc3-eth.c
+++ b/drivers/net/ethernet/sgi/ioc3-eth.c
@@ -79,8 +79,8 @@
#define RX_OFFSET (sizeof(struct ioc3_erxbuf) + NET_IP_ALIGN)
#define RX_BUF_SIZE (13 * IOC3_DMA_XFER_LEN)
-#define ETCSR_FD ((17 << ETCSR_IPGR2_SHIFT) | (11 << ETCSR_IPGR1_SHIFT) | 21)
-#define ETCSR_HD ((21 << ETCSR_IPGR2_SHIFT) | (21 << ETCSR_IPGR1_SHIFT) | 21)
+#define ETCSR_FD ((21 << ETCSR_IPGR2_SHIFT) | (21 << ETCSR_IPGR1_SHIFT) | 21)
+#define ETCSR_HD ((17 << ETCSR_IPGR2_SHIFT) | (11 << ETCSR_IPGR1_SHIFT) | 21)
/* Private per NIC data of the driver. */
struct ioc3_private {
--
2.13.7
After allocation of descriptor memory is now done once in probe
handling of tx ring is completely done by ioc3_clean_tx_ring. So
we remove the remaining tx ring actions out of ioc3_alloc_rings
and ioc3_free_rings and rename it to ioc3_[alloc|free]_rx_bufs
to better describe what they are doing.
Signed-off-by: Thomas Bogendoerfer <[email protected]>
---
drivers/net/ethernet/sgi/ioc3-eth.c | 19 ++++++++-----------
1 file changed, 8 insertions(+), 11 deletions(-)
diff --git a/drivers/net/ethernet/sgi/ioc3-eth.c b/drivers/net/ethernet/sgi/ioc3-eth.c
index e3867ea9abb7..de20f644e07d 100644
--- a/drivers/net/ethernet/sgi/ioc3-eth.c
+++ b/drivers/net/ethernet/sgi/ioc3-eth.c
@@ -778,13 +778,11 @@ static inline void ioc3_clean_tx_ring(struct ioc3_private *ip)
ip->tx_ci = 0;
}
-static void ioc3_free_rings(struct ioc3_private *ip)
+static void ioc3_free_rx_bufs(struct ioc3_private *ip)
{
struct sk_buff *skb;
int rx_entry, n_entry;
- ioc3_clean_tx_ring(ip);
-
n_entry = ip->rx_ci;
rx_entry = ip->rx_pi;
@@ -797,7 +795,7 @@ static void ioc3_free_rings(struct ioc3_private *ip)
}
}
-static void ioc3_alloc_rings(struct net_device *dev)
+static void ioc3_alloc_rx_bufs(struct net_device *dev)
{
struct ioc3_private *ip = netdev_priv(dev);
struct ioc3_erxbuf *rxb;
@@ -827,9 +825,6 @@ static void ioc3_alloc_rings(struct net_device *dev)
}
ip->rx_ci = 0;
ip->rx_pi = RX_BUFFS;
-
- ip->tx_pi = 0;
- ip->tx_ci = 0;
}
static void ioc3_init_rings(struct net_device *dev)
@@ -838,8 +833,8 @@ static void ioc3_init_rings(struct net_device *dev)
struct ioc3_ethregs *regs = ip->regs;
unsigned long ring;
- ioc3_free_rings(ip);
- ioc3_alloc_rings(dev);
+ ioc3_free_rx_bufs(ip);
+ ioc3_alloc_rx_bufs(dev);
ioc3_clean_tx_ring(ip);
@@ -965,7 +960,9 @@ static int ioc3_close(struct net_device *dev)
ioc3_stop(ip);
free_irq(dev->irq, dev);
- ioc3_free_rings(ip);
+ ioc3_free_rx_bufs(ip);
+ ioc3_clean_tx_ring(ip);
+
return 0;
}
@@ -1266,7 +1263,7 @@ static int ioc3_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
out_stop:
ioc3_stop(ip);
del_timer_sync(&ip->ioc3_timer);
- ioc3_free_rings(ip);
+ ioc3_free_rx_bufs(ip);
kfree(ip->rxr);
kfree(ip->txr);
out_res:
--
2.13.7
Descriptor ring sizes of the IOC3 are more or less fixed size. To
make clearer where there is a relation to ring sizes use defines.
Reviewed-by: Jakub Kicinski <[email protected]>
Signed-off-by: Thomas Bogendoerfer <[email protected]>
---
drivers/net/ethernet/sgi/ioc3-eth.c | 42 +++++++++++++++++++++----------------
1 file changed, 24 insertions(+), 18 deletions(-)
diff --git a/drivers/net/ethernet/sgi/ioc3-eth.c b/drivers/net/ethernet/sgi/ioc3-eth.c
index 51cc1389e204..ba18a53fbbe6 100644
--- a/drivers/net/ethernet/sgi/ioc3-eth.c
+++ b/drivers/net/ethernet/sgi/ioc3-eth.c
@@ -61,10 +61,16 @@
#include <asm/sn/ioc3.h>
#include <asm/pci/bridge.h>
-/* 64 RX buffers. This is tunable in the range of 16 <= x < 512. The
- * value must be a power of two.
+/* Number of RX buffers. This is tunable in the range of 16 <= x < 512.
+ * The value must be a power of two.
*/
-#define RX_BUFFS 64
+#define RX_BUFFS 64
+#define RX_RING_ENTRIES 512 /* fixed in hardware */
+#define RX_RING_MASK (RX_RING_ENTRIES - 1)
+
+/* 128 TX buffers (not tunable) */
+#define TX_RING_ENTRIES 128
+#define TX_RING_MASK (TX_RING_ENTRIES - 1)
#define ETCSR_FD ((17 << ETCSR_IPGR2_SHIFT) | (11 << ETCSR_IPGR1_SHIFT) | 21)
#define ETCSR_HD ((21 << ETCSR_IPGR2_SHIFT) | (21 << ETCSR_IPGR1_SHIFT) | 21)
@@ -76,8 +82,8 @@ struct ioc3_private {
u32 *ssram;
unsigned long *rxr; /* pointer to receiver ring */
struct ioc3_etxd *txr;
- struct sk_buff *rx_skbs[512];
- struct sk_buff *tx_skbs[128];
+ struct sk_buff *rx_skbs[RX_RING_ENTRIES];
+ struct sk_buff *tx_skbs[TX_RING_ENTRIES];
int rx_ci; /* RX consumer index */
int rx_pi; /* RX producer index */
int tx_ci; /* TX consumer index */
@@ -573,10 +579,10 @@ static inline void ioc3_rx(struct net_device *dev)
ip->rx_skbs[n_entry] = new_skb;
rxr[n_entry] = cpu_to_be64(ioc3_map(rxb, 1));
rxb->w0 = 0; /* Clear valid flag */
- n_entry = (n_entry + 1) & 511; /* Update erpir */
+ n_entry = (n_entry + 1) & RX_RING_MASK; /* Update erpir */
/* Now go on to the next ring entry. */
- rx_entry = (rx_entry + 1) & 511;
+ rx_entry = (rx_entry + 1) & RX_RING_MASK;
skb = ip->rx_skbs[rx_entry];
rxb = (struct ioc3_erxbuf *)(skb->data - RX_OFFSET);
w0 = be32_to_cpu(rxb->w0);
@@ -598,7 +604,7 @@ static inline void ioc3_tx(struct net_device *dev)
spin_lock(&ip->ioc3_lock);
etcir = readl(®s->etcir);
- tx_entry = (etcir >> 7) & 127;
+ tx_entry = (etcir >> 7) & TX_RING_MASK;
o_entry = ip->tx_ci;
packets = 0;
bytes = 0;
@@ -610,17 +616,17 @@ static inline void ioc3_tx(struct net_device *dev)
dev_consume_skb_irq(skb);
ip->tx_skbs[o_entry] = NULL;
- o_entry = (o_entry + 1) & 127; /* Next */
+ o_entry = (o_entry + 1) & TX_RING_MASK; /* Next */
etcir = readl(®s->etcir); /* More pkts sent? */
- tx_entry = (etcir >> 7) & 127;
+ tx_entry = (etcir >> 7) & TX_RING_MASK;
}
dev->stats.tx_packets += packets;
dev->stats.tx_bytes += bytes;
ip->txqlen -= packets;
- if (ip->txqlen < 128)
+ if (netif_queue_stopped(dev) && ip->txqlen < TX_RING_ENTRIES)
netif_wake_queue(dev);
ip->tx_ci = o_entry;
@@ -765,10 +771,10 @@ static inline void ioc3_clean_rx_ring(struct ioc3_private *ip)
ip->rx_skbs[ip->rx_pi] = ip->rx_skbs[ip->rx_ci];
ip->rxr[ip->rx_pi++] = ip->rxr[ip->rx_ci++];
}
- ip->rx_pi &= 511;
- ip->rx_ci &= 511;
+ ip->rx_pi &= RX_RING_MASK;
+ ip->rx_ci &= RX_RING_MASK;
- for (i = ip->rx_ci; i != ip->rx_pi; i = (i + 1) & 511) {
+ for (i = ip->rx_ci; i != ip->rx_pi; i = (i + 1) & RX_RING_MASK) {
skb = ip->rx_skbs[i];
rxb = (struct ioc3_erxbuf *)(skb->data - RX_OFFSET);
rxb->w0 = 0;
@@ -780,7 +786,7 @@ static inline void ioc3_clean_tx_ring(struct ioc3_private *ip)
struct sk_buff *skb;
int i;
- for (i = 0; i < 128; i++) {
+ for (i = 0; i < TX_RING_ENTRIES; i++) {
skb = ip->tx_skbs[i];
if (skb) {
ip->tx_skbs[i] = NULL;
@@ -812,7 +818,7 @@ static void ioc3_free_rings(struct ioc3_private *ip)
if (skb)
dev_kfree_skb_any(skb);
- n_entry = (n_entry + 1) & 511;
+ n_entry = (n_entry + 1) & RX_RING_MASK;
}
free_page((unsigned long)ip->rxr);
ip->rxr = NULL;
@@ -1425,13 +1431,13 @@ static netdev_tx_t ioc3_start_xmit(struct sk_buff *skb, struct net_device *dev)
mb(); /* make sure all descriptor changes are visible */
ip->tx_skbs[produce] = skb; /* Remember skb */
- produce = (produce + 1) & 127;
+ produce = (produce + 1) & TX_RING_MASK;
ip->tx_pi = produce;
writel(produce << 7, &ip->regs->etpir); /* Fire ... */
ip->txqlen++;
- if (ip->txqlen >= 127)
+ if (ip->txqlen >= (TX_RING_ENTRIES - 1))
netif_stop_queue(dev);
spin_unlock_irq(&ip->ioc3_lock);
--
2.13.7
Move clearing of the descriptor valid bit into ioc3_alloc_rings. This
makes ioc3_clean_rx_ring obsolete.
Signed-off-by: Thomas Bogendoerfer <[email protected]>
---
drivers/net/ethernet/sgi/ioc3-eth.c | 22 +---------------------
1 file changed, 1 insertion(+), 21 deletions(-)
diff --git a/drivers/net/ethernet/sgi/ioc3-eth.c b/drivers/net/ethernet/sgi/ioc3-eth.c
index d9d94a55ac34..e3867ea9abb7 100644
--- a/drivers/net/ethernet/sgi/ioc3-eth.c
+++ b/drivers/net/ethernet/sgi/ioc3-eth.c
@@ -761,26 +761,6 @@ static void ioc3_mii_start(struct ioc3_private *ip)
add_timer(&ip->ioc3_timer);
}
-static inline void ioc3_clean_rx_ring(struct ioc3_private *ip)
-{
- struct ioc3_erxbuf *rxb;
- struct sk_buff *skb;
- int i;
-
- for (i = ip->rx_ci; i & 15; i++) {
- ip->rx_skbs[ip->rx_pi] = ip->rx_skbs[ip->rx_ci];
- ip->rxr[ip->rx_pi++] = ip->rxr[ip->rx_ci++];
- }
- ip->rx_pi &= RX_RING_MASK;
- ip->rx_ci &= RX_RING_MASK;
-
- for (i = ip->rx_ci; i != ip->rx_pi; i = (i + 1) & RX_RING_MASK) {
- skb = ip->rx_skbs[i];
- rxb = (struct ioc3_erxbuf *)(skb->data - RX_OFFSET);
- rxb->w0 = 0;
- }
-}
-
static inline void ioc3_clean_tx_ring(struct ioc3_private *ip)
{
struct sk_buff *skb;
@@ -841,6 +821,7 @@ static void ioc3_alloc_rings(struct net_device *dev)
/* Because we reserve afterwards. */
skb_put(skb, (1664 + RX_OFFSET));
rxb = (struct ioc3_erxbuf *)skb->data;
+ rxb->w0 = 0; /* Clear valid flag */
ip->rxr[i] = cpu_to_be64(ioc3_map(rxb, 1));
skb_reserve(skb, RX_OFFSET);
}
@@ -860,7 +841,6 @@ static void ioc3_init_rings(struct net_device *dev)
ioc3_free_rings(ip);
ioc3_alloc_rings(dev);
- ioc3_clean_rx_ring(ip);
ioc3_clean_tx_ring(ip);
/* Now the rx ring base, consume & produce registers. */
--
2.13.7
On Thu, 29 Aug 2019 17:50:03 +0200, Thomas Bogendoerfer wrote:
> Memory for descriptor rings are allocated/freed, when interface is
> brought up/down. Since the size of the rings is not changeable by
> hardware, we now allocate rings now during probe and free it, when
> device is removed.
>
> Signed-off-by: Thomas Bogendoerfer <[email protected]>
> ---
> drivers/net/ethernet/sgi/ioc3-eth.c | 103 ++++++++++++++++++------------------
> 1 file changed, 51 insertions(+), 52 deletions(-)
>
> diff --git a/drivers/net/ethernet/sgi/ioc3-eth.c b/drivers/net/ethernet/sgi/ioc3-eth.c
> index ba18a53fbbe6..d9d94a55ac34 100644
> --- a/drivers/net/ethernet/sgi/ioc3-eth.c
> +++ b/drivers/net/ethernet/sgi/ioc3-eth.c
> @@ -803,25 +803,17 @@ static void ioc3_free_rings(struct ioc3_private *ip)
> struct sk_buff *skb;
> int rx_entry, n_entry;
>
> - if (ip->txr) {
> - ioc3_clean_tx_ring(ip);
> - free_pages((unsigned long)ip->txr, 2);
> - ip->txr = NULL;
> - }
> + ioc3_clean_tx_ring(ip);
>
> - if (ip->rxr) {
> - n_entry = ip->rx_ci;
> - rx_entry = ip->rx_pi;
> + n_entry = ip->rx_ci;
> + rx_entry = ip->rx_pi;
>
> - while (n_entry != rx_entry) {
> - skb = ip->rx_skbs[n_entry];
> - if (skb)
> - dev_kfree_skb_any(skb);
> + while (n_entry != rx_entry) {
> + skb = ip->rx_skbs[n_entry];
> + if (skb)
> + dev_kfree_skb_any(skb);
I think dev_kfree_skb_any() accepts NULL
>
> - n_entry = (n_entry + 1) & RX_RING_MASK;
> - }
> - free_page((unsigned long)ip->rxr);
> - ip->rxr = NULL;
> + n_entry = (n_entry + 1) & RX_RING_MASK;
> }
> }
>
> @@ -829,49 +821,34 @@ static void ioc3_alloc_rings(struct net_device *dev)
> {
> struct ioc3_private *ip = netdev_priv(dev);
> struct ioc3_erxbuf *rxb;
> - unsigned long *rxr;
> int i;
>
> - if (!ip->rxr) {
> - /* Allocate and initialize rx ring. 4kb = 512 entries */
> - ip->rxr = (unsigned long *)get_zeroed_page(GFP_ATOMIC);
> - rxr = ip->rxr;
> - if (!rxr)
> - pr_err("%s: get_zeroed_page() failed!\n", __func__);
> -
> - /* Now the rx buffers. The RX ring may be larger but
> - * we only allocate 16 buffers for now. Need to tune
> - * this for performance and memory later.
> - */
> - for (i = 0; i < RX_BUFFS; i++) {
> - struct sk_buff *skb;
> + /* Now the rx buffers. The RX ring may be larger but
> + * we only allocate 16 buffers for now. Need to tune
> + * this for performance and memory later.
> + */
> + for (i = 0; i < RX_BUFFS; i++) {
> + struct sk_buff *skb;
>
> - skb = ioc3_alloc_skb(RX_BUF_ALLOC_SIZE, GFP_ATOMIC);
> - if (!skb) {
> - show_free_areas(0, NULL);
> - continue;
> - }
> + skb = ioc3_alloc_skb(RX_BUF_ALLOC_SIZE, GFP_ATOMIC);
> + if (!skb) {
> + show_free_areas(0, NULL);
> + continue;
> + }
>
> - ip->rx_skbs[i] = skb;
> + ip->rx_skbs[i] = skb;
>
> - /* Because we reserve afterwards. */
> - skb_put(skb, (1664 + RX_OFFSET));
> - rxb = (struct ioc3_erxbuf *)skb->data;
> - rxr[i] = cpu_to_be64(ioc3_map(rxb, 1));
> - skb_reserve(skb, RX_OFFSET);
> - }
> - ip->rx_ci = 0;
> - ip->rx_pi = RX_BUFFS;
> + /* Because we reserve afterwards. */
> + skb_put(skb, (1664 + RX_OFFSET));
> + rxb = (struct ioc3_erxbuf *)skb->data;
> + ip->rxr[i] = cpu_to_be64(ioc3_map(rxb, 1));
> + skb_reserve(skb, RX_OFFSET);
> }
> + ip->rx_ci = 0;
> + ip->rx_pi = RX_BUFFS;
>
> - if (!ip->txr) {
> - /* Allocate and initialize tx rings. 16kb = 128 bufs. */
> - ip->txr = (struct ioc3_etxd *)__get_free_pages(GFP_KERNEL, 2);
> - if (!ip->txr)
> - pr_err("%s: __get_free_pages() failed!\n", __func__);
> - ip->tx_pi = 0;
> - ip->tx_ci = 0;
> - }
> + ip->tx_pi = 0;
> + ip->tx_ci = 0;
> }
>
> static void ioc3_init_rings(struct net_device *dev)
> @@ -1239,6 +1216,23 @@ static int ioc3_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
> timer_setup(&ip->ioc3_timer, ioc3_timer, 0);
>
> ioc3_stop(ip);
> +
> + /* Allocate and rx ring. 4kb = 512 entries */
> + ip->rxr = (unsigned long *)get_zeroed_page(GFP_ATOMIC);
> + if (!ip->rxr) {
> + pr_err("ioc3-eth: rx ring allocation failed\n");
> + err = -ENOMEM;
> + goto out_stop;
> + }
> +
> + /* Allocate tx rings. 16kb = 128 bufs. */
> + ip->txr = (struct ioc3_etxd *)__get_free_pages(GFP_KERNEL, 2);
> + if (!ip->txr) {
> + pr_err("ioc3-eth: tx ring allocation failed\n");
> + err = -ENOMEM;
> + goto out_stop;
> + }
Please just use kcalloc()/kmalloc_array() here, and make sure the flags
are set to GFP_KERNEL whenever possible. Here and in ioc3_alloc_rings()
it looks like GFP_ATOMIC is unnecessary.
On Thu, 29 Aug 2019 17:49:58 +0200, Thomas Bogendoerfer wrote:
> In my patch series for splitting out the serial code from ioc3-eth
> by using a MFD device there was one big patch for ioc3-eth.c,
> which wasn't really usefull for reviews. This series contains the
> ioc3-eth changes splitted in smaller steps and few more cleanups.
> Only the conversion to MFD will be done later in a different series.
>
> Changes in v2:
> - use net_err_ratelimited for printing various ioc3 errors
> - added missing clearing of rx buf valid flags into ioc3_alloc_rings
> - use __func__ for printing out of memory messages
Only a few more comments on patch 5, otherwise looks good!
On Thu, 29 Aug 2019 14:05:37 -0700
Jakub Kicinski <[email protected]> wrote:
> On Thu, 29 Aug 2019 17:50:03 +0200, Thomas Bogendoerfer wrote:
> > + if (skb)
> > + dev_kfree_skb_any(skb);
>
> I think dev_kfree_skb_any() accepts NULL
yes, I'll drop the if
> > +
> > + /* Allocate and rx ring. 4kb = 512 entries */
> > + ip->rxr = (unsigned long *)get_zeroed_page(GFP_ATOMIC);
> > + if (!ip->rxr) {
> > + pr_err("ioc3-eth: rx ring allocation failed\n");
> > + err = -ENOMEM;
> > + goto out_stop;
> > + }
> > +
> > + /* Allocate tx rings. 16kb = 128 bufs. */
> > + ip->txr = (struct ioc3_etxd *)__get_free_pages(GFP_KERNEL, 2);
> > + if (!ip->txr) {
> > + pr_err("ioc3-eth: tx ring allocation failed\n");
> > + err = -ENOMEM;
> > + goto out_stop;
> > + }
>
> Please just use kcalloc()/kmalloc_array() here,
both allocation will be replaced in patch 11 with dma_direct_alloc_pages.
So I hope I don't need to change it here.
Out of curiosity does kcalloc/kmalloc_array give me the same guarantees about
alignment ? rx ring needs to be 4KB aligned, tx ring 16KB aligned.
>, and make sure the flags
> are set to GFP_KERNEL whenever possible. Here and in ioc3_alloc_rings()
> it looks like GFP_ATOMIC is unnecessary.
yes, I'll change it
Thomas.
--
SUSE Software Solutions Germany GmbH
HRB 247165 (AG M?nchen)
Gesch?ftsf?hrer: Felix Imend?rffer
On Fri, 30 Aug 2019 00:00:58 +0200, Thomas Bogendoerfer wrote:
> On Thu, 29 Aug 2019 14:05:37 -0700
> Jakub Kicinski <[email protected]> wrote:
>
> > On Thu, 29 Aug 2019 17:50:03 +0200, Thomas Bogendoerfer wrote:
> > > + if (skb)
> > > + dev_kfree_skb_any(skb);
> >
> > I think dev_kfree_skb_any() accepts NULL
>
> yes, I'll drop the if
>
> > > +
> > > + /* Allocate and rx ring. 4kb = 512 entries */
> > > + ip->rxr = (unsigned long *)get_zeroed_page(GFP_ATOMIC);
> > > + if (!ip->rxr) {
> > > + pr_err("ioc3-eth: rx ring allocation failed\n");
> > > + err = -ENOMEM;
> > > + goto out_stop;
> > > + }
> > > +
> > > + /* Allocate tx rings. 16kb = 128 bufs. */
> > > + ip->txr = (struct ioc3_etxd *)__get_free_pages(GFP_KERNEL, 2);
> > > + if (!ip->txr) {
> > > + pr_err("ioc3-eth: tx ring allocation failed\n");
> > > + err = -ENOMEM;
> > > + goto out_stop;
> > > + }
> >
> > Please just use kcalloc()/kmalloc_array() here,
>
> both allocation will be replaced in patch 11 with dma_direct_alloc_pages.
> So I hope I don't need to change it here.
Ah, missed that!
> Out of curiosity does kcalloc/kmalloc_array give me the same guarantees about
> alignment ? rx ring needs to be 4KB aligned, tx ring 16KB aligned.
I don't think so, actually, I was mostly worried you are passing
address from get_page() into kfree() here ;) But patch 11 cures that,
so that's good, too.
> >, and make sure the flags
> > are set to GFP_KERNEL whenever possible. Here and in ioc3_alloc_rings()
> > it looks like GFP_ATOMIC is unnecessary.
>
> yes, I'll change it
On Thu, 29 Aug 2019 15:05:04 -0700
Jakub Kicinski <[email protected]> wrote:
> On Fri, 30 Aug 2019 00:00:58 +0200, Thomas Bogendoerfer wrote:
> > Out of curiosity does kcalloc/kmalloc_array give me the same guarantees about
> > alignment ? rx ring needs to be 4KB aligned, tx ring 16KB aligned.
>
> I don't think so, actually, I was mostly worried you are passing
> address from get_page() into kfree() here ;) But patch 11 cures that,
> so that's good, too.
I realized that after sending my last mail. I'll fix that in v3 even
it's just a transient bug.
Thomas.
--
SUSE Software Solutions Germany GmbH
HRB 247165 (AG M?nchen)
Gesch?ftsf?hrer: Felix Imend?rffer