Skip to content

Commit

Permalink
net: davinci_emac: Fix rollback of emac_dev_open()
Browse files Browse the repository at this point in the history
If an error occurs during the initialization in emac_dev_open() (the
driver's ndo_open function), interrupts, DMA descriptors etc. must be freed.
The current rollback code is buggy in several ways.

  1) Freeing the interrupts. The current code will not free all interrupts
     that were requested by the driver. Furthermore,  the code tries to do a
     platform_get_resource(priv->pdev, IORESOURCE_IRQ, -1) in its last
     iteration.

     This patch fixes these bugs.

  2) Wrong order of err: and rollback: labels. If the setup of the PHY in
     the code fails, the interrupts that have been requested before are
     not freed:

        request irq
                if requesting irqs fails, goto rollback
        setup phy
                if phy setup fails, goto err
        return 0

     rollback:
        free irqs
     err:

     This patch brings the code into the correct order.

  3) The code calls napi_enable() and emac_int_enable(), but does not
     undo both in case of an error.

     This patch adds calls of emac_int_disable() and napi_disable() to the
     rollback code.

  4) RX DMA descriptors are not freed in case of an error: Right before
     requesting the irqs, the function creates DMA descriptors for the
     RX channel. These RX descriptors are never freed when we jump to either
     rollback or err.

     This patch adds code for freeing the DMA descriptors in the case of
     an initialization error. This required a modification of
     cpdma_ctrl_stop() in davinci_cpdma.c: We must be able to call this
     function to free the DMA descriptors while the DMA channels are
     in IDLE state (before cpdma_ctlr_start() was called).

Tested on a custom board with the Texas Instruments AM1808.

Signed-off-by: Christian Riesch <christian.riesch@omicron.at>
Signed-off-by: David S. Miller <davem@davemloft.net>
  • Loading branch information
Christian Riesch authored and David S. Miller committed Mar 24, 2014
1 parent 33b7107 commit cd11cf5
Show file tree
Hide file tree
Showing 2 changed files with 31 additions and 17 deletions.
4 changes: 2 additions & 2 deletions drivers/net/ethernet/ti/davinci_cpdma.c
Original file line number Diff line number Diff line change
Expand Up @@ -355,7 +355,7 @@ int cpdma_ctlr_stop(struct cpdma_ctlr *ctlr)
int i;

spin_lock_irqsave(&ctlr->lock, flags);
if (ctlr->state != CPDMA_STATE_ACTIVE) {
if (ctlr->state == CPDMA_STATE_TEARDOWN) {
spin_unlock_irqrestore(&ctlr->lock, flags);
return -EINVAL;
}
Expand Down Expand Up @@ -891,7 +891,7 @@ int cpdma_chan_stop(struct cpdma_chan *chan)
unsigned timeout;

spin_lock_irqsave(&chan->lock, flags);
if (chan->state != CPDMA_STATE_ACTIVE) {
if (chan->state == CPDMA_STATE_TEARDOWN) {
spin_unlock_irqrestore(&chan->lock, flags);
return -EINVAL;
}
Expand Down
44 changes: 29 additions & 15 deletions drivers/net/ethernet/ti/davinci_emac.c
Original file line number Diff line number Diff line change
Expand Up @@ -1533,8 +1533,8 @@ static int emac_dev_open(struct net_device *ndev)
u32 cnt;
struct resource *res;
int q, m, ret;
int res_num = 0, irq_num = 0;
int i = 0;
int k = 0;
struct emac_priv *priv = netdev_priv(ndev);

pm_runtime_get(&priv->pdev->dev);
Expand Down Expand Up @@ -1564,14 +1564,24 @@ static int emac_dev_open(struct net_device *ndev)
}

/* Request IRQ */
while ((res = platform_get_resource(priv->pdev, IORESOURCE_IRQ,
res_num))) {
for (irq_num = res->start; irq_num <= res->end; irq_num++) {
dev_err(emac_dev, "Request IRQ %d\n", irq_num);
if (request_irq(irq_num, emac_irq, 0, ndev->name,
ndev)) {
dev_err(emac_dev,
"DaVinci EMAC: request_irq() failed\n");
ret = -EBUSY;

while ((res = platform_get_resource(priv->pdev, IORESOURCE_IRQ, k))) {
for (i = res->start; i <= res->end; i++) {
if (request_irq(i, emac_irq, 0, ndev->name, ndev))
goto rollback;
}
}
k++;
res_num++;
}
/* prepare counters for rollback in case of an error */
res_num--;
irq_num--;

/* Start/Enable EMAC hardware */
emac_hw_enable(priv);
Expand Down Expand Up @@ -1638,19 +1648,23 @@ static int emac_dev_open(struct net_device *ndev)

return 0;

rollback:

dev_err(emac_dev, "DaVinci EMAC: request_irq() failed");
err:
emac_int_disable(priv);
napi_disable(&priv->napi);

for (q = k; k >= 0; k--) {
for (m = i; m >= res->start; m--)
rollback:
for (q = res_num; q >= 0; q--) {
res = platform_get_resource(priv->pdev, IORESOURCE_IRQ, q);
/* at the first iteration, irq_num is already set to the
* right value
*/
if (q != res_num)
irq_num = res->end;

for (m = irq_num; m >= res->start; m--)
free_irq(m, ndev);
res = platform_get_resource(priv->pdev, IORESOURCE_IRQ, k-1);
m = res->end;
}

ret = -EBUSY;
err:
cpdma_ctlr_stop(priv->dma);
pm_runtime_put(&priv->pdev->dev);
return ret;
}
Expand Down

0 comments on commit cd11cf5

Please sign in to comment.