Re: Acer M5229 IDE bugs (esp. on sparc64)

[Manuel Bouyer <bouyer%antioche.eu.org@localhost>]

On Thu, Feb 14, 2008 at 02:17:47PM -0500, Rafal Boni wrote:
> Some more testing says this patch, nor any additional rearranging of the 
> aceride register writes (per Linux/FreeBSD drivers) makes any difference.
> 
> What is interesting is that if I add the following bit of debug to 
> ata/ata_wdc.c:
> 
> Index: ata/ata_wdc.c
> ===================================================================
> RCS file: /cvsroot/src/sys/dev/ata/ata_wdc.c,v
> retrieving revision 1.87
> diff -u -p -b -u -p -r1.87 ata_wdc.c
> --- ata/ata_wdc.c       19 Oct 2007 11:59:36 -0000      1.87
> +++ ata/ata_wdc.c       14 Feb 2008 18:57:03 -0000
> @@ -687,6 +687,9 @@ wdc_ata_bio_intr(struct ata_channel *chp
>                 }
>                 if (wdc->dma_status != 0) {
>                         if (drv_err != WDC_ATA_ERR) {
> +                               printf("%s:%d:%d: DMA error (st=0x%x, 
> er=0x%x)\n
> ",
> +                                   atac->atac_dev.dv_xname, 
> chp->ch_channel,
> +                                   xfer->c_drive, wdc->dma_status, 
> ata_bio->r_error);
>                                 ata_bio->error = ERR_DMA;
>                                 drv_err = WDC_ATA_ERR;
>                         }
> 
> I see that wdc->dma_status is always 0x04 (WDC_DMAST_UNDER), which is a 
> synthetic error generated only by pciide_dma_finish().  I'm guessing 
> that the suspect pciiide_dma_finish() is the one called from wdcintr(). 
>  Because the rev of the M1559 IDE controller I have doesn't have a 
> chan-id register to determine which channel caused an interrupt, for 
> this chip we end up *always* checking both channels, and the code in 
> wdcintr() / pciide_dma_finish() looks very suspicious... stop DMA first, 
> ask questions later.

AFAIK it's not: before stopping the DMA channel, we check if the controller
did interrupt (status & IDEDMA_CTL_INTR).
We have to stop the DMA first, because in some case the DMA engine will still
be active at end of transfer (if the device has less data to return than
requested for example - as the comment says, it's a valid condition for
ATAPI devices).

> 
> FreeBSD looks like it does the checks a bit differnetly (their whole 
> handling of the Bus-Master DMA status register -- IDEDMA_CTL -- is a bit 
> different, but they key is they look like they check the active status 
> (IDEDMA_CTL_ACT) before attempting to kill the dma vs. our kill-1st, 
> check if active later.

If this chip asserts IDEDMA_CTL_INTR supriously, and we need to check
IDEDMA_CTL_ACT instead, then it's broken. So it needs a private intr
routine, and it needs to disable DMA for ATAPI devices.

-- 
Manuel Bouyer <bouyer%antioche.eu.org@localhost>
     NetBSD: 26 ans d'experience feront toujours la difference
--