>-----Original Message----- >From: [email protected] >[mailto:[email protected]] On >Behalf Of Cai, Cliff >Sent: Friday, January 22, 2010 9:23 AM >To: Mike Frysinger >Cc: [email protected] >Subject: Re: [Linux-kernel-commits] [8206] >trunk/drivers/char/bfin_sport.c:bug[#5832] quickly jump out of >error interrupt handler,when DMA tx interrupt handler is >waiting for sport to completetransfer > > > > >>-----Original Message----- >>From: Mike Frysinger [mailto:[email protected]] >>Sent: Friday, January 22, 2010 3:07 AM >>To: [email protected] >>Cc: [email protected] >>Subject: Re: [Linux-kernel-commits] [8206] >>trunk/drivers/char/bfin_sport.c: bug[#5832] quickly jump out of error >>interrupt handler, when DMA tx interrupt handler is waiting for sport >>to complete transfer >> >>On Thu, Jan 21, 2010 at 02:14, <[email protected]> wrote: >>> Revision 8206 Author cliff Date 2010-01-21 02:14:30 -0500 >>(Thu, 21 Jan >>> 2010) >>> >>> Log Message >>> >>> 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.
Cliff _______________________________________________ >Linux-kernel-commits mailing list >[email protected] >https://blackfin.uclinux.org/mailman/listinfo/linux-kernel-commits > _______________________________________________ Linux-kernel-commits mailing list [email protected] https://blackfin.uclinux.org/mailman/listinfo/linux-kernel-commits
