If memory allocation fails in alloc_list(), free the already allocated
memory and return -ENODEV. In rio_open(), call alloc_list() first and
abort if it fails. Move HW access (set RFDListPtr) out ot alloc_list().
Signed-off-by: Ondrej Zary <[email protected]>
---
drivers/net/ethernet/dlink/dl2k.c | 182 ++++++++++++++++++++-----------------
1 file changed, 97 insertions(+), 85 deletions(-)
diff --git a/drivers/net/ethernet/dlink/dl2k.c b/drivers/net/ethernet/dlink/dl2k.c
index ccca479..ec0eb7f 100644
--- a/drivers/net/ethernet/dlink/dl2k.c
+++ b/drivers/net/ethernet/dlink/dl2k.c
@@ -70,7 +70,6 @@ static const int multicast_filter_limit = 0x40;
static int rio_open (struct net_device *dev);
static void rio_timer (unsigned long data);
static void rio_tx_timeout (struct net_device *dev);
-static void alloc_list (struct net_device *dev);
static netdev_tx_t start_xmit (struct sk_buff *skb, struct net_device *dev);
static irqreturn_t rio_interrupt (int irq, void *dev_instance);
static void rio_free_tx (struct net_device *dev, int irq);
@@ -446,6 +445,92 @@ static void rio_set_led_mode(struct net_device *dev)
dw32(ASICCtrl, mode);
}
+static inline dma_addr_t desc_to_dma(struct netdev_desc *desc)
+{
+ return le64_to_cpu(desc->fraginfo) & DMA_BIT_MASK(48);
+}
+
+static void free_list(struct net_device *dev)
+{
+ struct netdev_private *np = netdev_priv(dev);
+ struct sk_buff *skb;
+ int i;
+
+ /* Free all the skbuffs in the queue. */
+ for (i = 0; i < RX_RING_SIZE; i++) {
+ skb = np->rx_skbuff[i];
+ if (skb) {
+ pci_unmap_single(np->pdev, desc_to_dma(&np->rx_ring[i]),
+ skb->len, PCI_DMA_FROMDEVICE);
+ dev_kfree_skb(skb);
+ np->rx_skbuff[i] = NULL;
+ }
+ np->rx_ring[i].status = 0;
+ np->rx_ring[i].fraginfo = 0;
+ }
+ for (i = 0; i < TX_RING_SIZE; i++) {
+ skb = np->tx_skbuff[i];
+ if (skb) {
+ pci_unmap_single(np->pdev, desc_to_dma(&np->tx_ring[i]),
+ skb->len, PCI_DMA_TODEVICE);
+ dev_kfree_skb(skb);
+ np->tx_skbuff[i] = NULL;
+ }
+ }
+}
+
+ /* allocate and initialize Tx and Rx descriptors */
+static int alloc_list(struct net_device *dev)
+{
+ struct netdev_private *np = netdev_priv(dev);
+ int i;
+
+ np->cur_rx = np->cur_tx = 0;
+ np->old_rx = np->old_tx = 0;
+ np->rx_buf_sz = (dev->mtu <= 1500 ? PACKET_SIZE : dev->mtu + 32);
+
+ /* Initialize Tx descriptors, TFDListPtr leaves in start_xmit(). */
+ for (i = 0; i < TX_RING_SIZE; i++) {
+ np->tx_skbuff[i] = NULL;
+ np->tx_ring[i].status = cpu_to_le64(TFDDone);
+ np->tx_ring[i].next_desc = cpu_to_le64(np->tx_ring_dma +
+ ((i + 1) % TX_RING_SIZE) *
+ sizeof(struct netdev_desc));
+ }
+
+ /* Initialize Rx descriptors */
+ for (i = 0; i < RX_RING_SIZE; i++) {
+ np->rx_ring[i].next_desc = cpu_to_le64(np->rx_ring_dma +
+ ((i + 1) % RX_RING_SIZE) *
+ sizeof(struct netdev_desc));
+ np->rx_ring[i].status = 0;
+ np->rx_ring[i].fraginfo = 0;
+ np->rx_skbuff[i] = NULL;
+ }
+
+ /* Allocate the rx buffers */
+ for (i = 0; i < RX_RING_SIZE; i++) {
+ /* Allocated fixed size of skbuff */
+ struct sk_buff *skb;
+
+ skb = netdev_alloc_skb_ip_align(dev, np->rx_buf_sz);
+ np->rx_skbuff[i] = skb;
+ if (!skb) {
+ free_list(dev);
+ return -ENOMEM;
+ }
+
+ /* Rubicon now supports 40 bits of addressing space. */
+ np->rx_ring[i].fraginfo =
+ cpu_to_le64(pci_map_single(
+ np->pdev, skb->data, np->rx_buf_sz,
+ PCI_DMA_FROMDEVICE));
+ np->rx_ring[i].fraginfo |= cpu_to_le64((u64)np->rx_buf_sz << 48);
+ }
+
+ return 0;
+}
+
static int
rio_open (struct net_device *dev)
{
@@ -455,10 +540,16 @@ rio_open (struct net_device *dev)
int i;
u16 macctrl;
- i = request_irq(irq, rio_interrupt, IRQF_SHARED, dev->name, dev);
+ i = alloc_list(dev);
if (i)
return i;
+ i = request_irq(irq, rio_interrupt, IRQF_SHARED, dev->name, dev);
+ if (i) {
+ free_list(dev);
+ return i;
+ }
+
/* Reset all logic functions */
dw16(ASICCtrl + 2,
GlobalReset | DMAReset | FIFOReset | NetworkReset | HostReset);
@@ -473,7 +564,9 @@ rio_open (struct net_device *dev)
if (np->jumbo != 0)
dw16(MaxFrameSize, MAX_JUMBO+14);
- alloc_list (dev);
+ /* Set RFDListPtr */
+ dw32(RFDListPtr0, np->rx_ring_dma);
+ dw32(RFDListPtr1, 0);
/* Set station address */
/* 16 or 32-bit access is required by TC9020 datasheet but 8-bit works
@@ -586,60 +679,6 @@ rio_tx_timeout (struct net_device *dev)
dev->trans_start = jiffies; /* prevent tx timeout */
}
- /* allocate and initialize Tx and Rx descriptors */
-static void
-alloc_list (struct net_device *dev)
-{
- struct netdev_private *np = netdev_priv(dev);
- void __iomem *ioaddr = np->ioaddr;
- int i;
-
- np->cur_rx = np->cur_tx = 0;
- np->old_rx = np->old_tx = 0;
- np->rx_buf_sz = (dev->mtu <= 1500 ? PACKET_SIZE : dev->mtu + 32);
-
- /* Initialize Tx descriptors, TFDListPtr leaves in start_xmit(). */
- for (i = 0; i < TX_RING_SIZE; i++) {
- np->tx_skbuff[i] = NULL;
- np->tx_ring[i].status = cpu_to_le64 (TFDDone);
- np->tx_ring[i].next_desc = cpu_to_le64 (np->tx_ring_dma +
- ((i+1)%TX_RING_SIZE) *
- sizeof (struct netdev_desc));
- }
-
- /* Initialize Rx descriptors */
- for (i = 0; i < RX_RING_SIZE; i++) {
- np->rx_ring[i].next_desc = cpu_to_le64 (np->rx_ring_dma +
- ((i + 1) % RX_RING_SIZE) *
- sizeof (struct netdev_desc));
- np->rx_ring[i].status = 0;
- np->rx_ring[i].fraginfo = 0;
- np->rx_skbuff[i] = NULL;
- }
-
- /* Allocate the rx buffers */
- for (i = 0; i < RX_RING_SIZE; i++) {
- /* Allocated fixed size of skbuff */
- struct sk_buff *skb;
-
- skb = netdev_alloc_skb_ip_align(dev, np->rx_buf_sz);
- np->rx_skbuff[i] = skb;
- if (skb == NULL)
- break;
-
- /* Rubicon now supports 40 bits of addressing space. */
- np->rx_ring[i].fraginfo =
- cpu_to_le64 ( pci_map_single (
- np->pdev, skb->data, np->rx_buf_sz,
- PCI_DMA_FROMDEVICE));
- np->rx_ring[i].fraginfo |= cpu_to_le64((u64)np->rx_buf_sz << 48);
- }
-
- /* Set RFDListPtr */
- dw32(RFDListPtr0, np->rx_ring_dma);
- dw32(RFDListPtr1, 0);
-}
-
static netdev_tx_t
start_xmit (struct sk_buff *skb, struct net_device *dev)
{
@@ -748,11 +787,6 @@ rio_interrupt (int irq, void *dev_instance)
return IRQ_RETVAL(handled);
}
-static inline dma_addr_t desc_to_dma(struct netdev_desc *desc)
-{
- return le64_to_cpu(desc->fraginfo) & DMA_BIT_MASK(48);
-}
-
static void
rio_free_tx (struct net_device *dev, int irq)
{
@@ -1733,8 +1767,6 @@ rio_close (struct net_device *dev)
void __iomem *ioaddr = np->ioaddr;
struct pci_dev *pdev = np->pdev;
- struct sk_buff *skb;
- int i;
netif_stop_queue (dev);
@@ -1747,27 +1779,7 @@ rio_close (struct net_device *dev)
free_irq(pdev->irq, dev);
del_timer_sync (&np->timer);
- /* Free all the skbuffs in the queue. */
- for (i = 0; i < RX_RING_SIZE; i++) {
- skb = np->rx_skbuff[i];
- if (skb) {
- pci_unmap_single(pdev, desc_to_dma(&np->rx_ring[i]),
- skb->len, PCI_DMA_FROMDEVICE);
- dev_kfree_skb (skb);
- np->rx_skbuff[i] = NULL;
- }
- np->rx_ring[i].status = 0;
- np->rx_ring[i].fraginfo = 0;
- }
- for (i = 0; i < TX_RING_SIZE; i++) {
- skb = np->tx_skbuff[i];
- if (skb) {
- pci_unmap_single(pdev, desc_to_dma(&np->tx_ring[i]),
- skb->len, PCI_DMA_TODEVICE);
- dev_kfree_skb (skb);
- np->tx_skbuff[i] = NULL;
- }
- }
+ free_list(dev);
return 0;
}
--
Ondrej Zary
Move HW init and stop into separate functions.
Request IRQ only after the HW has been reset (so interrupts are
disabled and no stale interrupts are pending).
Signed-off-by: Ondrej Zary <[email protected]>
---
drivers/net/ethernet/dlink/dl2k.c | 95 ++++++++++++++++++++++---------------
1 file changed, 56 insertions(+), 39 deletions(-)
diff --git a/drivers/net/ethernet/dlink/dl2k.c b/drivers/net/ethernet/dlink/dl2k.c
index ec0eb7f..9e9baa0 100644
--- a/drivers/net/ethernet/dlink/dl2k.c
+++ b/drivers/net/ethernet/dlink/dl2k.c
@@ -252,19 +252,6 @@ rio_probe1 (struct pci_dev *pdev, const struct pci_device_id *ent)
if (err)
goto err_out_unmap_rx;
- if (np->chip_id == CHIP_IP1000A &&
- (np->pdev->revision == 0x40 || np->pdev->revision == 0x41)) {
- /* PHY magic taken from ipg driver, undocumented registers */
- mii_write(dev, np->phy_addr, 31, 0x0001);
- mii_write(dev, np->phy_addr, 27, 0x01e0);
- mii_write(dev, np->phy_addr, 31, 0x0002);
- mii_write(dev, np->phy_addr, 27, 0xeb8e);
- mii_write(dev, np->phy_addr, 31, 0x0000);
- mii_write(dev, np->phy_addr, 30, 0x005e);
- /* advertise 1000BASE-T half & full duplex, prefer MASTER */
- mii_write(dev, np->phy_addr, MII_CTRL1000, 0x0700);
- }
-
/* Fiber device? */
np->phy_media = (dr16(ASICCtrl) & PhyMedia) ? 1 : 0;
np->link_status = 0;
@@ -274,13 +261,11 @@ rio_probe1 (struct pci_dev *pdev, const struct pci_device_id *ent)
if (np->an_enable == 2) {
np->an_enable = 1;
}
- mii_set_media_pcs (dev);
} else {
/* Auto-Negotiation is mandatory for 1000BASE-T,
IEEE 802.3ab Annex 28D page 14 */
if (np->speed == 1000)
np->an_enable = 1;
- mii_set_media (dev);
}
err = register_netdev (dev);
@@ -531,25 +516,13 @@ static int alloc_list(struct net_device *dev)
return 0;
}
-static int
-rio_open (struct net_device *dev)
+static void rio_hw_init(struct net_device *dev)
{
struct netdev_private *np = netdev_priv(dev);
void __iomem *ioaddr = np->ioaddr;
- const int irq = np->pdev->irq;
int i;
u16 macctrl;
- i = alloc_list(dev);
- if (i)
- return i;
-
- i = request_irq(irq, rio_interrupt, IRQF_SHARED, dev->name, dev);
- if (i) {
- free_list(dev);
- return i;
- }
-
/* Reset all logic functions */
dw16(ASICCtrl + 2,
GlobalReset | DMAReset | FIFOReset | NetworkReset | HostReset);
@@ -560,6 +533,24 @@ rio_open (struct net_device *dev)
/* DebugCtrl bit 4, 5, 9 must set */
dw32(DebugCtrl, dr32(DebugCtrl) | 0x0230);
+ if (np->chip_id == CHIP_IP1000A &&
+ (np->pdev->revision == 0x40 || np->pdev->revision == 0x41)) {
+ /* PHY magic taken from ipg driver, undocumented registers */
+ mii_write(dev, np->phy_addr, 31, 0x0001);
+ mii_write(dev, np->phy_addr, 27, 0x01e0);
+ mii_write(dev, np->phy_addr, 31, 0x0002);
+ mii_write(dev, np->phy_addr, 27, 0xeb8e);
+ mii_write(dev, np->phy_addr, 31, 0x0000);
+ mii_write(dev, np->phy_addr, 30, 0x005e);
+ /* advertise 1000BASE-T half & full duplex, prefer MASTER */
+ mii_write(dev, np->phy_addr, MII_CTRL1000, 0x0700);
+ }
+
+ if (np->phy_media)
+ mii_set_media_pcs(dev);
+ else
+ mii_set_media(dev);
+
/* Jumbo frame */
if (np->jumbo != 0)
dw16(MaxFrameSize, MAX_JUMBO+14);
@@ -602,10 +593,6 @@ rio_open (struct net_device *dev)
dw32(MACCtrl, dr32(MACCtrl) | AutoVLANuntagging);
}
- setup_timer(&np->timer, rio_timer, (unsigned long)dev);
- np->timer.expires = jiffies + 1*HZ;
- add_timer (&np->timer);
-
/* Start Tx/Rx */
dw32(MACCtrl, dr32(MACCtrl) | StatsEnable | RxEnable | TxEnable);
@@ -615,6 +602,42 @@ rio_open (struct net_device *dev)
macctrl |= (np->tx_flow) ? TxFlowControlEnable : 0;
macctrl |= (np->rx_flow) ? RxFlowControlEnable : 0;
dw16(MACCtrl, macctrl);
+}
+
+static void rio_hw_stop(struct net_device *dev)
+{
+ struct netdev_private *np = netdev_priv(dev);
+ void __iomem *ioaddr = np->ioaddr;
+
+ /* Disable interrupts */
+ dw16(IntEnable, 0);
+
+ /* Stop Tx and Rx logics */
+ dw32(MACCtrl, TxDisable | RxDisable | StatsDisable);
+}
+
+static int rio_open(struct net_device *dev)
+{
+ struct netdev_private *np = netdev_priv(dev);
+ const int irq = np->pdev->irq;
+ int i;
+
+ i = alloc_list(dev);
+ if (i)
+ return i;
+
+ rio_hw_init(dev);
+
+ i = request_irq(irq, rio_interrupt, IRQF_SHARED, dev->name, dev);
+ if (i) {
+ rio_hw_stop(dev);
+ free_list(dev);
+ return i;
+ }
+
+ setup_timer(&np->timer, rio_timer, (unsigned long)dev);
+ np->timer.expires = jiffies + 1 * HZ;
+ add_timer(&np->timer);
netif_start_queue (dev);
@@ -1764,17 +1787,11 @@ static int
rio_close (struct net_device *dev)
{
struct netdev_private *np = netdev_priv(dev);
- void __iomem *ioaddr = np->ioaddr;
-
struct pci_dev *pdev = np->pdev;
netif_stop_queue (dev);
- /* Disable interrupts */
- dw16(IntEnable, 0);
-
- /* Stop Tx and Rx logics */
- dw32(MACCtrl, TxDisable | RxDisable | StatsDisable);
+ rio_hw_stop(dev);
free_irq(pdev->irq, dev);
del_timer_sync (&np->timer);
--
Ondrej Zary
Add suspend/resume support to dl2k driver.
Tested on Asus NX1101 (IP1000A) and D-Link DGE-550T (DL-2000).
Signed-off-by: Ondrej Zary <[email protected]>
---
drivers/net/ethernet/dlink/dl2k.c | 46 +++++++++++++++++++++++++++++++++++++
1 file changed, 46 insertions(+)
diff --git a/drivers/net/ethernet/dlink/dl2k.c b/drivers/net/ethernet/dlink/dl2k.c
index 9e9baa0..b53dfa7 100644
--- a/drivers/net/ethernet/dlink/dl2k.c
+++ b/drivers/net/ethernet/dlink/dl2k.c
@@ -1824,11 +1824,57 @@ rio_remove1 (struct pci_dev *pdev)
}
}
+#ifdef CONFIG_PM
+static int rio_suspend(struct pci_dev *pdev, pm_message_t state)
+{
+ struct net_device *dev = pci_get_drvdata(pdev);
+ struct netdev_private *np = netdev_priv(dev);
+
+ pci_save_state(pdev);
+
+ if (netif_running(dev)) {
+ netif_device_detach(dev);
+ del_timer_sync(&np->timer);
+ rio_hw_stop(dev);
+ free_list(dev);
+ }
+
+ pci_set_power_state(pdev, PCI_D3hot);
+
+ return 0;
+}
+
+static int rio_resume(struct pci_dev *pdev)
+{
+ struct net_device *dev = pci_get_drvdata(pdev);
+ struct netdev_private *np = netdev_priv(dev);
+
+ pci_set_power_state(pdev, PCI_D0);
+ pci_restore_state(pdev);
+
+ if (netif_running(dev)) {
+ alloc_list(dev);
+ rio_hw_init(dev);
+ np->timer.expires = jiffies + 1 * HZ;
+ add_timer(&np->timer);
+ netif_device_attach(dev);
+ dl2k_enable_int(np);
+ }
+
+ return 0;
+}
+
+#endif /* CONFIG_PM */
+
static struct pci_driver rio_driver = {
.name = "dl2k",
.id_table = rio_pci_tbl,
.probe = rio_probe1,
.remove = rio_remove1,
+#ifdef CONFIG_PM
+ .suspend = rio_suspend,
+ .resume = rio_resume,
+#endif /* CONFIG_PM */
};
module_pci_driver(rio_driver);
--
Ondrej Zary
Ondrej Zary <[email protected]> :
> If memory allocation fails in alloc_list(), free the already allocated
> memory and return -ENODEV. In rio_open(), call alloc_list() first and
> abort if it fails. Move HW access (set RFDListPtr) out ot alloc_list().
>
> Signed-off-by: Ondrej Zary <[email protected]>
ENODEV vs ENOMEM aside, it's ok with me.
The driver may avoid depleting the receive ring when netdev_alloc_skb_ip_align
fails in receive_packet (drop and increase stats->rx_dropped).
Then you may replace the pci_* dma helpers with the plain dma_* ones (they
can fail).
And perform a plain napi conversion.
--
Ueimor
Ondrej Zary <[email protected]> :
[...]
> diff --git a/drivers/net/ethernet/dlink/dl2k.c b/drivers/net/ethernet/dlink/dl2k.c
> index 9e9baa0..b53dfa7 100644
> --- a/drivers/net/ethernet/dlink/dl2k.c
> +++ b/drivers/net/ethernet/dlink/dl2k.c
> @@ -1824,11 +1824,57 @@ rio_remove1 (struct pci_dev *pdev)
> }
> }
>
> +#ifdef CONFIG_PM
> +static int rio_suspend(struct pci_dev *pdev, pm_message_t state)
> +{
> + struct net_device *dev = pci_get_drvdata(pdev);
> + struct netdev_private *np = netdev_priv(dev);
> +
> + pci_save_state(pdev);
Cargo-cultism ?
> +
> + if (netif_running(dev)) {
> + netif_device_detach(dev);
> + del_timer_sync(&np->timer);
> + rio_hw_stop(dev);
> + free_list(dev);
If free_list is used here, so must alloc_list be in resume, whence
an extra failure opportunity.
You may not need to free both Tx and Rx here.
[...]
> static struct pci_driver rio_driver = {
> .name = "dl2k",
> .id_table = rio_pci_tbl,
> .probe = rio_probe1,
> .remove = rio_remove1,
> +#ifdef CONFIG_PM
> + .suspend = rio_suspend,
> + .resume = rio_resume,
> +#endif /* CONFIG_PM */
It looks a bit old school.
See Documentation/power/pci.txt and drivers/net/ethernet/via/via-rhine.c
for an instance of SIMPLE_DEV_PM_OPS.
At some point you'll probably support runtime power management though.
--
Ueimor
On Tuesday 17 November 2015, Francois Romieu wrote:
> Ondrej Zary <[email protected]> :
> > If memory allocation fails in alloc_list(), free the already allocated
> > memory and return -ENODEV. In rio_open(), call alloc_list() first and
> > abort if it fails. Move HW access (set RFDListPtr) out ot alloc_list().
> >
> > Signed-off-by: Ondrej Zary <[email protected]>
>
> ENODEV vs ENOMEM aside, it's ok with me.
Will fix the description in v2.
> The driver may avoid depleting the receive ring when
> netdev_alloc_skb_ip_align fails in receive_packet (drop and increase
> stats->rx_dropped).
>
> Then you may replace the pci_* dma helpers with the plain dma_* ones (they
> can fail).
>
> And perform a plain napi conversion.
NAPI is on the todo list. Thanks for suggestions.
--
Ondrej Zary
On Tuesday 17 November 2015, Francois Romieu wrote:
> Ondrej Zary <[email protected]> :
> [...]
>
> > diff --git a/drivers/net/ethernet/dlink/dl2k.c
> > b/drivers/net/ethernet/dlink/dl2k.c index 9e9baa0..b53dfa7 100644
> > --- a/drivers/net/ethernet/dlink/dl2k.c
> > +++ b/drivers/net/ethernet/dlink/dl2k.c
> > @@ -1824,11 +1824,57 @@ rio_remove1 (struct pci_dev *pdev)
> > }
> > }
> >
> > +#ifdef CONFIG_PM
> > +static int rio_suspend(struct pci_dev *pdev, pm_message_t state)
> > +{
> > + struct net_device *dev = pci_get_drvdata(pdev);
> > + struct netdev_private *np = netdev_priv(dev);
> > +
> > + pci_save_state(pdev);
>
> Cargo-cultism ?
>
> > +
> > + if (netif_running(dev)) {
> > + netif_device_detach(dev);
> > + del_timer_sync(&np->timer);
> > + rio_hw_stop(dev);
> > + free_list(dev);
>
> If free_list is used here, so must alloc_list be in resume, whence
> an extra failure opportunity.
>
> You may not need to free both Tx and Rx here.
I'll better drop the free_list/alloc_list from suspend/resume (as you
suggested before).
> [...]
>
> > static struct pci_driver rio_driver = {
> > .name = "dl2k",
> > .id_table = rio_pci_tbl,
> > .probe = rio_probe1,
> > .remove = rio_remove1,
> > +#ifdef CONFIG_PM
> > + .suspend = rio_suspend,
> > + .resume = rio_resume,
> > +#endif /* CONFIG_PM */
>
> It looks a bit old school.
>
> See Documentation/power/pci.txt and drivers/net/ethernet/via/via-rhine.c
> for an instance of SIMPLE_DEV_PM_OPS.
>
> At some point you'll probably support runtime power management though.
Thanks for suggestion, I've been looking at the wrong drivers as none of them
used dev_pm_ops.
--
Ondrej Zary