On Thu, Jan 21, 2010 at 22:25, Cai, Cliff wrote: >From: [email protected] >>From: Mike Frysinger [mailto:[email protected]] >>>On Thu, Jan 21, 2010 at 02:14, <[email protected]> wrote: >>>> bug[#5832]quickly jump out of error interrupt handler,when DMA tx >>>> interrupt handler is waiting for sport to complete transfer >>>> >>>> Modified: trunk/drivers/char/bfin_sport.c (8205 => 8206) >>>> >>>> @@ -54,6 +54,7 @@ >>>> >>>> volatile struct sport_register *regs; >>>> struct sport_config config; >>>> + int dma_tx_run; >>>> }; >>>> >>>> /* XXX: this should get pushed to platform device */ @@ >>>-285,7 +286,7 >>>> @@ >>>> dev->regs->tcr1 &= ~TSPEN; >>>> SSYNC(); >>>> disable_dma(dev->dma_tx_chan); >>>> - >>>> + dev->dma_tx_run = 0; >>>> complete(&dev->c); >>>> >>>> /* Clear the interrupt status */ >>>> @@ -399,6 +400,10 @@ >>>> >>>> if (status & (TOVF | TUVF | ROVF | RUVF)) { >>>> dev->regs->stat = (status & (TOVF | TUVF | ROVF >>>| RUVF)); >>>> + if (dev->dma_tx_run && dev->config.dma_enabled) { >>>> + dev->regs->tcr1 &= ~TSPEN; >>>> + goto out; >>>> + } >>>> if (dev->config.dma_enabled) { >>>> disable_dma(dev->dma_rx_chan); >>>> disable_dma(dev->dma_tx_chan); >>>> @@ -406,21 +411,16 @@ >>>> dev->regs->tcr1 &= ~TSPEN; >>>> dev->regs->rcr1 &= ~RSPEN; >>>> SSYNC(); >>>> - >>>> - if (!dev->config.dma_enabled && !dev->config.int_clk) { >>>> - if (status & TUVF) >>>> - complete(&dev->c); >>>> - } else >>>> - pr_warning("sport %p status error:%s%s%s%s\n", >>>> - dev->regs, >>>> - status & TOVF ? " TOVF" : "", >>>> - status & TUVF ? " TUVF" : "", >>>> - status & ROVF ? " ROVF" : "", >>>> - status & RUVF ? " RUVF" : ""); >>>> + pr_warning("sport %p status error:%s%s%s%s\n", >>>> + dev->regs, >>>> + status & TOVF ? " TOVF" : "", >>>> + status & TUVF ? " TUVF" : "", >>>> + status & ROVF ? " ROVF" : "", >>>> + status & RUVF ? " RUVF" : ""); >>>> } >>>> >>>> /* XXX: should we always complete here and have >>>read/write error ? >>>> */ >>>> - >>>> +out: >>>> return IRQ_HANDLED; >>>> } >>>> >>>> @@ -640,6 +640,7 @@ >>>> set_dma_config(dev->dma_tx_chan, dma_config); >>>> >>>> enable_dma(dev->dma_tx_chan); >>>> + dev->dma_tx_run = 1; >>>> } else { >>>> /* Configure parameters to start PIO transfer */ >>>> dev->tx_buf = buf; >>> >>>you've changed it so that when a DMA TX is done, all status >>>errors are >>>simply ignored. they only get processed _if_ the DMA TX finishes and >>>by that point, there really arent going to be any errors >>>flying up. i dont think this is the correct fix. >>> >>>if the issue is nested interrupts, why not have the DMA TX handler >>>disable interrupts at the start and restore when it's done ? that >>>should give the same behavior as IRQF_DISABLED ... > > I don't think this way is good enough, > If restore the err interrupt again,the pending interrupt will be > triggered immediately.
if that fails, why would IRQF_DISABLED make any difference ? it's pretty much doing the same thing. -mike _______________________________________________ Linux-kernel-commits mailing list [email protected] https://blackfin.uclinux.org/mailman/listinfo/linux-kernel-commits
