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
pgpxeRMYhlhaX.pgp
Description: PGP signature