Re: [PATCH] libata: fix ata_dma_enabled

On Mon, Dec 03, 2012 at 01:22:25AM +0000, Alan Cox wrote:

On Mon, 03 Dec 2012 09:04:20 +0800
Aaron Lu [off-list ref] wrote:

quoted

ata_dma_enabled should check if device is either using multi word DMA
or ultra DMA instead of checking 0xff, as dma_mode 0 is not a valid dma
mode either.

This patch fixes the following bug:
https://bugzilla.kernel.org/show_bug.cgi?id=49151

NAK

dma_mode should *NEVER* be zero. If it's getting set to zero you have
another bug and that needs fixing instead

The ata_dev->dma_mode is initialized as zero after we allocate the
ata_device structure, and we did not set this field until the set mode
function.

If you think it should _never_ be zero, probably my previous patch did
the job: it sets ata_dev->dma_mode to 0xff in the reset operation:

diff --git a/drivers/ata/libata-eh.c b/drivers/ata/libata-eh.c
index 9426423..d772d66 100644
--- a/drivers/ata/libata-eh.c
+++ b/drivers/ata/libata-eh.c

@@ -2657,6 +2657,7 @@ int ata_eh_reset(struct ata_link *link, int classify,
 		 * bus as we may be talking too fast.
 		 */
 		dev->pio_mode = XFER_PIO_0;
+		dev->dma_mode = 0xff;
 
 		/* If the controller has a pio mode setup function
 		 * then use it to set the chipset to rights. Don't

So do you prefer this?

Thanks,
Aaron

Alan