Re: [Qemu-devel] [PATCH 5/5] cmd646: synchronise UDMA interrupt status with DMA interrupt status

[Stefan Hajnoczi]

On Mon, Aug 11, 2014 at 04:33:01PM +0100, Mark Cave-Ayland wrote:
> On 11/08/14 16:12, Stefan Hajnoczi wrote:
> 
> >On Fri, Aug 08, 2014 at 05:23:36PM +0100, Mark Cave-Ayland wrote:
> >>@@ -322,6 +342,10 @@ static int pci_cmd646_ide_initfn(PCIDevice *dev)
> >>      }
> >>
> >>      /* Set write-to-clear interrupt bits */
> >>+    dev->wmask[CFR] = 0x0;
> >>+    dev->w1cmask[CFR] = CFR_INTR_CH0;
> >>+    dev->wmask[ARTTIM23] = 0x0;
> >>+    dev->w1cmask[ARTTIM23] = ARTTIM23_INTR_CH1;
> >>      dev->wmask[MRDMODE] = 0x0;
> >>      dev->w1cmask[MRDMODE] = MRDMODE_INTR_CH0 | MRDMODE_INTR_CH1;
> >
> >It is not clear to me why the mask for MRDMODE has both Channel 0 and 1
> >but the ARTTIM23 and CFR masks only have one channel each.
> >
> >Please post a link to the datasheet.
> 
> Hi Stefan,
> 
> Thanks for the review. You can find a copy of the 646U2 datasheet at
> http://gkernel.sourceforge.net/specs/sii/PCI0646U2Specr030399.PDF.bz2.
> 
> My understanding from the datasheet is that CFR is the primary channel
> interrupt whilst ARTTIM23 is the secondary channel interrupt, and the note
> on page 28 explains that these bits are also accessible via the relevant
> bits of the MRDMODE register.
> 
> This fixes cmd646 under NetBSD since it programs UDMA via MRDMODE but clears
> the interrupts via CFR and ARTTIM23, so without this patchset the driver
> hangs because the interrupt isn't properly cleared across both registers.
> And I can also verify that Linux experiences no regressions with this
> patchset applied.

Great, thanks!

Stefan

pgpIpiLEeEVMN.pgp
Description: PGP signature