[PATCH] libata: fix ata_dma_enabled

[PATCH] libata: fix ata_dma_enabled

[Aaron Lu]

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

Reported-by: Phillip Wood <phillip.wood@dunelm.org.uk>
Signed-off-by: Aaron Lu <aaron.lu@intel.com>
Tested-by: Dutra Julio <dutra.julio@gmail.com>
Tested-by: Szymon Janc <szymon@janc.net.pl>
Tested-by: Bernd Buschinski <b.buschinski@googlemail.com>
Cc: <stable@kernel.org>
---
 include/linux/libata.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/linux/libata.h b/include/linux/libata.h
index 77eeeda..2444695 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -1617,7 +1617,7 @@ static inline int ata_using_udma(struct ata_device *adev)
 
 static inline int ata_dma_enabled(struct ata_device *adev)
 {
-	return (adev->dma_mode == 0xFF ? 0 : 1);
+	return ata_using_mwdma(adev) || ata_using_udma(adev);
 }
 
 /**************************************************************************
-- 
1.8.0.1.2.gd1eded4

Re: [PATCH] libata: fix ata_dma_enabled

[Alan Cox]

On Mon, 03 Dec 2012 09:04:20 +0800
Aaron Lu <aaron.lu@intel.com> wrote:

> 
> 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

Alan

Re: [PATCH] libata: fix ata_dma_enabled

[Aaron Lu]

On Mon, Dec 03, 2012 at 01:22:25AM +0000, Alan Cox wrote:
> On Mon, 03 Dec 2012 09:04:20 +0800
> Aaron Lu <aaron.lu@intel.com> wrote:
> 
> > 
> > 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

Re: [PATCH] libata: fix ata_dma_enabled

[Alan Cox]

> 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?

Yes - definitely. That is the intended behaviour.

[PATCH] libata: set dma_mode to 0xff in reset

[Aaron Lu]

ata_device->dma_mode's initial value is zero, which is not a valid dma
mode, but ata_dma_enabled will return true for this value. This patch
sets dma_mode to 0xff in reset function, so that ata_dma_enabled will
not return true for this case, or it will cause problem for pata_acpi.

The corrsponding bugzilla page is at:
https://bugzilla.kernel.org/show_bug.cgi?id=49151

Reported-by: Phillip Wood <phillip.wood@dunelm.org.uk>
Signed-off-by: Aaron Lu <aaron.lu@intel.com>
Tested-by: Szymon Janc <szymon@janc.net.pl>
Tested-by: Dutra Julio <dutra.julio@gmail.com>
Cc: <stable@kernel.org>
---
 drivers/ata/libata-core.c | 1 +
 drivers/ata/libata-eh.c   | 1 +
 2 files changed, 2 insertions(+)

diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index f46fbd3..586362e 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -2560,6 +2560,7 @@ int ata_bus_probe(struct ata_port *ap)
 		 * 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
diff --git a/drivers/ata/libata-eh.c b/drivers/ata/libata-eh.c
index e60437c..bf039b0 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
-- 
1.7.12.4

Re: [PATCH] libata: set dma_mode to 0xff in reset

[Alan Cox]

On Mon, 03 Dec 2012 11:35:02 +0800
Aaron Lu <aaron.lu@intel.com> wrote:

> 
> ata_device->dma_mode's initial value is zero, which is not a valid dma
> mode, but ata_dma_enabled will return true for this value. This patch
> sets dma_mode to 0xff in reset function, so that ata_dma_enabled will
> not return true for this case, or it will cause problem for pata_acpi.
> 
> The corrsponding bugzilla page is at:
> https://bugzilla.kernel.org/show_bug.cgi?id=49151
> 
> Reported-by: Phillip Wood <phillip.wood@dunelm.org.uk>
> Signed-off-by: Aaron Lu <aaron.lu@intel.com>

Acked-by: Alan Cox <alan@linux.intel.com>

Re: [PATCH] libata: set dma_mode to 0xff in reset

[Jeff Garzik]

On 12/02/2012 10:35 PM, Aaron Lu wrote:
>
> ata_device->dma_mode's initial value is zero, which is not a valid dma
> mode, but ata_dma_enabled will return true for this value. This patch
> sets dma_mode to 0xff in reset function, so that ata_dma_enabled will
> not return true for this case, or it will cause problem for pata_acpi.
>
> The corrsponding bugzilla page is at:
> https://bugzilla.kernel.org/show_bug.cgi?id=49151
>
> Reported-by: Phillip Wood <phillip.wood@dunelm.org.uk>
> Signed-off-by: Aaron Lu <aaron.lu@intel.com>
> Tested-by: Szymon Janc <szymon@janc.net.pl>
> Tested-by: Dutra Julio <dutra.julio@gmail.com>
> Cc: <stable@kernel.org>
> ---
>   drivers/ata/libata-core.c | 1 +
>   drivers/ata/libata-eh.c   | 1 +
>   2 files changed, 2 insertions(+)

applied