| Message ID | 130fa9cb-7e94-4698-ac3f-f987a03c0604@mary.at.omicron.at (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Friday 07 March 2014 07:37 PM, Christian Riesch wrote: > Signed-off-by: Christian Riesch <christian.riesch@omicron.at> > Cc: Prabhakar Lad <prabhakar.csengg@gmail.com> > Cc: Mugunthan V N <mugunthanvnm@ti.com> > Cc: Florian Fainelli <f.fainelli@gmail.com> > --- > > Hi again, > > How about this solution for fixing the rollback of emac_dev_open()? > Especially the change in davinci_cpdma.c, would this break anything? > > The patch applies on top of > [PATCH] net: davinci_emac: Replace devm_request_irq with request_irq > > Regards, > Christian > > drivers/net/ethernet/ti/davinci_cpdma.c | 4 +-- > drivers/net/ethernet/ti/davinci_emac.c | 44 ++++++++++++++++++++----------- > 2 files changed, 31 insertions(+), 17 deletions(-) > > diff --git a/drivers/net/ethernet/ti/davinci_cpdma.c b/drivers/net/ethernet/ti/davinci_cpdma.c > index 364d0c7..88ef270 100644 > --- a/drivers/net/ethernet/ti/davinci_cpdma.c > +++ b/drivers/net/ethernet/ti/davinci_cpdma.c > @@ -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; > } > @@ -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; > } Even when in idle mode chan stop should return error. Regards Mugunthan V N
--On March 07, 2014 20:15 +0530 Mugunthan V N <mugunthanvnm@ti.com> wrote: > On Friday 07 March 2014 07:37 PM, Christian Riesch wrote: >> Signed-off-by: Christian Riesch <christian.riesch@omicron.at> >> Cc: Prabhakar Lad <prabhakar.csengg@gmail.com> >> Cc: Mugunthan V N <mugunthanvnm@ti.com> >> Cc: Florian Fainelli <f.fainelli@gmail.com> >> --- >> >> Hi again, >> >> How about this solution for fixing the rollback of emac_dev_open()? >> Especially the change in davinci_cpdma.c, would this break anything? >> >> The patch applies on top of >> [PATCH] net: davinci_emac: Replace devm_request_irq with request_irq >> >> Regards, >> Christian >> >> drivers/net/ethernet/ti/davinci_cpdma.c | 4 +-- >> drivers/net/ethernet/ti/davinci_emac.c | 44 >> ++++++++++++++++++++----------- 2 files changed, 31 insertions(+), 17 >> deletions(-) >> >> diff --git a/drivers/net/ethernet/ti/davinci_cpdma.c >> b/drivers/net/ethernet/ti/davinci_cpdma.c index 364d0c7..88ef270 100644 >> --- a/drivers/net/ethernet/ti/davinci_cpdma.c >> +++ b/drivers/net/ethernet/ti/davinci_cpdma.c >> @@ -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; >> } >> @@ -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; >> } > > Even when in idle mode chan stop should return error. Can you please explain? I do not see why. I must be able to call cpdma_ctlr_stop in idle mode to free the rx descriptors in case ndo_open in davinci_emac.c fails. Any other ideas how to solve the problem addressed in the rest of the patch? Thanks, Christian
Hi Mugunthan, On Mon, Mar 10, 2014 at 12:46 PM, Christian Riesch <christian.riesch@omicron.at> wrote: > > > --On March 07, 2014 20:15 +0530 Mugunthan V N <mugunthanvnm@ti.com> wrote: > >> On Friday 07 March 2014 07:37 PM, Christian Riesch wrote: >>> >>> Signed-off-by: Christian Riesch <christian.riesch@omicron.at> >>> Cc: Prabhakar Lad <prabhakar.csengg@gmail.com> >>> Cc: Mugunthan V N <mugunthanvnm@ti.com> >>> Cc: Florian Fainelli <f.fainelli@gmail.com> >>> --- >>> >>> Hi again, >>> >>> How about this solution for fixing the rollback of emac_dev_open()? >>> Especially the change in davinci_cpdma.c, would this break anything? >>> >>> The patch applies on top of >>> [PATCH] net: davinci_emac: Replace devm_request_irq with request_irq >>> >>> Regards, >>> Christian >>> >>> drivers/net/ethernet/ti/davinci_cpdma.c | 4 +-- >>> drivers/net/ethernet/ti/davinci_emac.c | 44 >>> ++++++++++++++++++++----------- 2 files changed, 31 insertions(+), 17 >>> deletions(-) >>> >>> diff --git a/drivers/net/ethernet/ti/davinci_cpdma.c >>> b/drivers/net/ethernet/ti/davinci_cpdma.c index 364d0c7..88ef270 100644 >>> --- a/drivers/net/ethernet/ti/davinci_cpdma.c >>> +++ b/drivers/net/ethernet/ti/davinci_cpdma.c >>> @@ -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; >>> } >>> @@ -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; >>> } >> >> >> Even when in idle mode chan stop should return error. > > > Can you please explain? I do not see why. > > I must be able to call cpdma_ctlr_stop in idle mode to free the rx > descriptors in case ndo_open in davinci_emac.c fails. Any other ideas how to > solve the problem addressed in the rest of the patch? > Do you have any comments on this patch ? I have tested it on OMAP-L138 evm and works fine. If you have none comments may be Christian can consolidate this patch with earlier and repost it. Thanks, --Prabhakar Lad
diff --git a/drivers/net/ethernet/ti/davinci_cpdma.c b/drivers/net/ethernet/ti/davinci_cpdma.c index 364d0c7..88ef270 100644 --- a/drivers/net/ethernet/ti/davinci_cpdma.c +++ b/drivers/net/ethernet/ti/davinci_cpdma.c @@ -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; } @@ -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; } diff --git a/drivers/net/ethernet/ti/davinci_emac.c b/drivers/net/ethernet/ti/davinci_emac.c index 2514304..8f0e69c 100644 --- a/drivers/net/ethernet/ti/davinci_emac.c +++ b/drivers/net/ethernet/ti/davinci_emac.c @@ -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); @@ -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); @@ -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; }
Signed-off-by: Christian Riesch <christian.riesch@omicron.at> Cc: Prabhakar Lad <prabhakar.csengg@gmail.com> Cc: Mugunthan V N <mugunthanvnm@ti.com> Cc: Florian Fainelli <f.fainelli@gmail.com> --- Hi again, How about this solution for fixing the rollback of emac_dev_open()? Especially the change in davinci_cpdma.c, would this break anything? The patch applies on top of [PATCH] net: davinci_emac: Replace devm_request_irq with request_irq Regards, Christian drivers/net/ethernet/ti/davinci_cpdma.c | 4 +-- drivers/net/ethernet/ti/davinci_emac.c | 44 ++++++++++++++++++++----------- 2 files changed, 31 insertions(+), 17 deletions(-)