Re: [PATCH] make ata_exec_internal_sg honor DMADIR

[Vincent Pelletier <plr.vincent@gmail.com>]

[-- Attachment #1: Type: Text/Plain, Size: 1434 bytes --]

Le mardi 14 mai 2013 21:06:16, Tejun Heo a écrit :
> Patch looks correct to me but can you please put more detail into the
> description preferably with a link to this thread?

Updated & attached. I write one (trivial) kernel commit message every 3 years, 
so please correct me if there is a more consistent way to present it.

> As for why atapi_dmadir isn't enabled by default, my memory is extremely
> fuzzy now but ISTR it to be deprecated and cause issues with some devices.

As atapi_id_dmadir() doesn't detect the bridge needs DMADIR, for now I need to 
enable it globally.
This means it doesn't work out of the box, and needs a reboot to work as 
atapi_dmadir is read-only in sysfs. Also, if it causes regression with other 
drives, maybe one would gain a drive by enabling DMADIR but loose another.

From my (very limited) understanding, the bridge just passes the drive's "id" 
(as in "atapi_id_dmadir(dev->id)") through. Is there another way to detect 
such bridge ? Other things atapi_id_dmadir() should look for in "id" ?

If not, would it be possible to have a rw sysfs pseudofile per-device (...per 
port ?) to enable DMADIR ?

If not, what about making atapi_dmadir sysfs pseudofile rw, to save a reboot ?

Would you have more ideas on how it could be solved ?

I'm willing to give a try to any of these options if they have a chance to get 
somewhere.

Regards,
-- 
Vincent Pelletier

[-- Attachment #2: 0001-libata-make-ata_exec_internal_sg-honor-DMADIR.patch --]
[-- Type: text/x-patch, Size: 1644 bytes --]

From beca064485e3c86e4abe08b9ce5c89b33ed8c780 Mon Sep 17 00:00:00 2001
Message-Id: <beca064485e3c86e4abe08b9ce5c89b33ed8c780.1368810901.git.plr.vincent@gmail.com>
From: Vincent Pelletier <plr.vincent@gmail.com>
Date: Fri, 17 May 2013 19:09:05 +0200
Subject: libata: make ata_exec_internal_sg honor DMADIR
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Fixes SATA-to-PATA bridge "Abit Serillel 2" when used on an ATAPI device,
which otherwise fails several tries with a timeout until it gets disabled:

  kernel: ata5.00: qc timeout (cmd 0xa0)
  kernel: ata5.00: failed to clear UNIT ATTENTION (err_mask=0x5)
  kernel: ata5.00: disabled

Based on a patch by Csaba Halász <csaba.halasz@gmail.com> on linux-ide:
http://marc.info/?l=linux-ide&m=136121147832295&w=2

Signed-off-by: Vincent Pelletier <plr.vincent@gmail.com>
---
 drivers/ata/libata-core.c |    7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index 63c743b..d121db7 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -1600,8 +1600,13 @@ unsigned ata_exec_internal_sg(struct ata_device *dev,
 
 	/* prepare & issue qc */
 	qc->tf = *tf;
-	if (cdb)
+	if (cdb) {
 		memcpy(qc->cdb, cdb, ATAPI_CDB_LEN);
+		if ((dev->flags & ATA_DFLAG_DMADIR) &&
+		    (dma_dir == DMA_FROM_DEVICE))
+			/* some SATA bridges need us to indicate data xfer direction */
+			qc->tf.feature |= ATAPI_DMADIR;
+	}
 	qc->flags |= ATA_QCFLAG_RESULT_TF;
 	qc->dma_dir = dma_dir;
 	if (dma_dir != DMA_NONE) {
-- 
1.7.10.4