Re: [PATCH] libata: automatically use DMADIR if drive/bridge requires it

2008-02-23 Thread Jeff Garzik

Tejun Heo wrote:

Back in 2.6.17-rc2, a libata module parameter was added for atapi_dmadir.

That's nice, but most SATA devices which need it will tell us about it
in their IDENTIFY PACKET response, as bit-15 of word-62 of the
returned data (as per ATA7, ATA8 specifications).

So for those which specify it, we should automatically use the DMADIR bit.
Otherwise, disc writing will fail by default on many SATA-ATAPI drives.

This patch adds ATA_DFLAG_DMADIR and make ata_dev_configure() set it
if atapi_dmadir is set or identify data indicates DMADIR is necessary.
atapi_xlat() is converted to check ATA_DFLAG_DMADIR before setting
DMADIR.

Original patch is from Mark Lord.

Signed-off-by: Tejun Heo <[EMAIL PROTECTED]>
Cc: Mark Lord <[EMAIL PROTECTED]>
---
I don't have a bridge which sets DMADIR but so only checked atapi_dmadir
parameter.  Thanks.

 drivers/ata/libata-core.c |   11 +--
 drivers/ata/libata-scsi.c |3 ++-
 include/linux/ata.h   |5 +
 include/linux/libata.h|1 +
 4 files changed, 17 insertions(+), 3 deletions(-)


applied

-
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] libata: automatically use DMADIR if drive/bridge requires it

2008-02-22 Thread Jeff Garzik

Mark Lord wrote:

Albert Lee wrote:

Tejun Heo wrote:
Back in 2.6.17-rc2, a libata module parameter was added for 
atapi_dmadir.


That's nice, but most SATA devices which need it will tell us about it
in their IDENTIFY PACKET response, as bit-15 of word-62 of the
returned data (as per ATA7, ATA8 specifications).

So for those which specify it, we should automatically use the DMADIR 
bit.

Otherwise, disc writing will fail by default on many SATA-ATAPI drives.

This patch adds ATA_DFLAG_DMADIR and make ata_dev_configure() set it
if atapi_dmadir is set or identify data indicates DMADIR is necessary.
atapi_xlat() is converted to check ATA_DFLAG_DMADIR before setting
DMADIR.

Original patch is from Mark Lord.

Signed-off-by: Tejun Heo <[EMAIL PROTECTED]>
Cc: Mark Lord <[EMAIL PROTECTED]>
---
I don't have a bridge which sets DMADIR but so only checked atapi_dmadir
parameter.  Thanks.



The patch looks good. However, it seems there is no realworld IDE-to-SATA
bridge that requires DMADIR and also mangles IDENTIFY PACKET bit-15 of
word-62 to indicate its presence.


From the previous test of the IDE-to-SATA bridges, only the Sil 3611

requires the host software to hint on ATAPI DMADIR. But Sil 3611 doesn't
mangle IDENTIFY PACKET word-62:
http://www.spinics.net/lists/linux-ide/msg01514.html

..

Ahh, great!  We have "history" here!

But there are more SATA bridges out there than the ones Alber has there
(SiI 3611/3811, Marvell 88i8030/88SA8040, Acard ARC770, JMicron JM20330).

There's the Marvell 88SA8050, and I have a funky looking thing here
that says "SataLink SPiF223A" on it ().  I wonder what it does?
And how many other ones are there?

The Word62 patch (from Tejun) is simple enough that the "complexity"
point from before really doesn't hold water.


I'm going to apply Tejun's patch.  It's fine (as I noted I conditionally 
ACK'd your original patch).


My opinion, though, it that this is merely coding to the spec, rather 
than making a bunch of real-world hardware work.  All the SATA<->PATA 
bridges I have, that support ATAPI, do not flag DMADIR.


Jeff




-
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] libata: automatically use DMADIR if drive/bridge requires it

2008-02-22 Thread Mark Lord

Mark Lord wrote:

I have a funky looking thing here
that says "SataLink SPiF223A" on it ().  I wonder what it does?

..

Well, after some experimentation, this one appears to be ATA-only,
with no ATAPI support at all.

Cheers
-
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] libata: automatically use DMADIR if drive/bridge requires it

2008-02-22 Thread Mark Lord

Albert Lee wrote:

Tejun Heo wrote:

Back in 2.6.17-rc2, a libata module parameter was added for atapi_dmadir.

That's nice, but most SATA devices which need it will tell us about it
in their IDENTIFY PACKET response, as bit-15 of word-62 of the
returned data (as per ATA7, ATA8 specifications).

So for those which specify it, we should automatically use the DMADIR bit.
Otherwise, disc writing will fail by default on many SATA-ATAPI drives.

This patch adds ATA_DFLAG_DMADIR and make ata_dev_configure() set it
if atapi_dmadir is set or identify data indicates DMADIR is necessary.
atapi_xlat() is converted to check ATA_DFLAG_DMADIR before setting
DMADIR.

Original patch is from Mark Lord.

Signed-off-by: Tejun Heo <[EMAIL PROTECTED]>
Cc: Mark Lord <[EMAIL PROTECTED]>
---
I don't have a bridge which sets DMADIR but so only checked atapi_dmadir
parameter.  Thanks.



The patch looks good. However, it seems there is no realworld IDE-to-SATA
bridge that requires DMADIR and also mangles IDENTIFY PACKET bit-15 of
word-62 to indicate its presence.


From the previous test of the IDE-to-SATA bridges, only the Sil 3611

requires the host software to hint on ATAPI DMADIR. But Sil 3611 doesn't
mangle IDENTIFY PACKET word-62:
http://www.spinics.net/lists/linux-ide/msg01514.html

..

Ahh, great!  We have "history" here!

But there are more SATA bridges out there than the ones Alber has there
(SiI 3611/3811, Marvell 88i8030/88SA8040, Acard ARC770, JMicron JM20330).

There's the Marvell 88SA8050, and I have a funky looking thing here
that says "SataLink SPiF223A" on it ().  I wonder what it does?
And how many other ones are there?

The Word62 patch (from Tejun) is simple enough that the "complexity"
point from before really doesn't hold water.

To be ATA compliant, we really should add it in.
But yes, it would also be good to see it in action.

That's more likely to happen in the general user community,
than in our limited circle of kernel folk with limited hardware.

Cheers
-
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] libata: automatically use DMADIR if drive/bridge requires it

2008-02-22 Thread Albert Lee
Tejun Heo wrote:
> Back in 2.6.17-rc2, a libata module parameter was added for atapi_dmadir.
> 
> That's nice, but most SATA devices which need it will tell us about it
> in their IDENTIFY PACKET response, as bit-15 of word-62 of the
> returned data (as per ATA7, ATA8 specifications).
> 
> So for those which specify it, we should automatically use the DMADIR bit.
> Otherwise, disc writing will fail by default on many SATA-ATAPI drives.
> 
> This patch adds ATA_DFLAG_DMADIR and make ata_dev_configure() set it
> if atapi_dmadir is set or identify data indicates DMADIR is necessary.
> atapi_xlat() is converted to check ATA_DFLAG_DMADIR before setting
> DMADIR.
> 
> Original patch is from Mark Lord.
> 
> Signed-off-by: Tejun Heo <[EMAIL PROTECTED]>
> Cc: Mark Lord <[EMAIL PROTECTED]>
> ---
> I don't have a bridge which sets DMADIR but so only checked atapi_dmadir
> parameter.  Thanks.
> 

The patch looks good. However, it seems there is no realworld IDE-to-SATA
bridge that requires DMADIR and also mangles IDENTIFY PACKET bit-15 of
word-62 to indicate its presence.

>From the previous test of the IDE-to-SATA bridges, only the Sil 3611
requires the host software to hint on ATAPI DMADIR. But Sil 3611 doesn't
mangle IDENTIFY PACKET word-62:
http://www.spinics.net/lists/linux-ide/msg01514.html

Maybe we could use ata_dev_knobble() instead? i.e. If it's an ATAPI
device && ata_dev_knobble() then we turn on DMADIR (regardless the
bridge chip used since setting DMADIR won't hurt those don't need it.)
--
albert

-
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] libata: automatically use DMADIR if drive/bridge requires it

2008-02-20 Thread Tejun Heo
Back in 2.6.17-rc2, a libata module parameter was added for atapi_dmadir.

That's nice, but most SATA devices which need it will tell us about it
in their IDENTIFY PACKET response, as bit-15 of word-62 of the
returned data (as per ATA7, ATA8 specifications).

So for those which specify it, we should automatically use the DMADIR bit.
Otherwise, disc writing will fail by default on many SATA-ATAPI drives.

This patch adds ATA_DFLAG_DMADIR and make ata_dev_configure() set it
if atapi_dmadir is set or identify data indicates DMADIR is necessary.
atapi_xlat() is converted to check ATA_DFLAG_DMADIR before setting
DMADIR.

Original patch is from Mark Lord.

Signed-off-by: Tejun Heo <[EMAIL PROTECTED]>
Cc: Mark Lord <[EMAIL PROTECTED]>
---
I don't have a bridge which sets DMADIR but so only checked atapi_dmadir
parameter.  Thanks.

 drivers/ata/libata-core.c |   11 +--
 drivers/ata/libata-scsi.c |3 ++-
 include/linux/ata.h   |5 +
 include/linux/libata.h|1 +
 4 files changed, 17 insertions(+), 3 deletions(-)

diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index 3011919..d38f113 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -2200,6 +2200,7 @@ int ata_dev_configure(struct ata_device *dev)
else if (dev->class == ATA_DEV_ATAPI) {
const char *cdb_intr_string = "";
const char *atapi_an_string = "";
+   const char *dma_dir_string = "";
u32 sntf;
 
rc = atapi_cdb_len(id);
@@ -2240,13 +2241,19 @@ int ata_dev_configure(struct ata_device *dev)
cdb_intr_string = ", CDB intr";
}
 
+   if (atapi_dmadir || atapi_id_dmadir(dev->id)) {
+   dev->flags |= ATA_DFLAG_DMADIR;
+   dma_dir_string = ", DMADIR";
+   }
+
/* print device info to dmesg */
if (ata_msg_drv(ap) && print_info)
ata_dev_printk(dev, KERN_INFO,
-  "ATAPI: %s, %s, max %s%s%s\n",
+  "ATAPI: %s, %s, max %s%s%s%s\n",
   modelbuf, fwrevbuf,
   ata_mode_string(xfer_mask),
-  cdb_intr_string, atapi_an_string);
+  cdb_intr_string, atapi_an_string,
+  dma_dir_string);
}
 
/* determine max_sectors */
diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index c02c490..f50b96b 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -2543,7 +2543,8 @@ static unsigned int atapi_xlat(struct ata_queued_cmd *qc)
qc->tf.protocol = ATAPI_PROT_DMA;
qc->tf.feature |= ATAPI_PKT_DMA;
 
-   if (atapi_dmadir && (scmd->sc_data_direction != DMA_TO_DEVICE))
+   if ((dev->flags & ATA_DFLAG_DMADIR) &&
+   (scmd->sc_data_direction != DMA_TO_DEVICE))
/* some SATA bridges need us to indicate data xfer 
direction */
qc->tf.feature |= ATAPI_DMADIR;
}
diff --git a/include/linux/ata.h b/include/linux/ata.h
index 78bbaca..1c622e2 100644
--- a/include/linux/ata.h
+++ b/include/linux/ata.h
@@ -659,6 +659,11 @@ static inline int atapi_command_packet_set(const u16 
*dev_id)
return (dev_id[0] >> 8) & 0x1f;
 }
 
+static inline int atapi_id_dmadir(const u16 *dev_id)
+{
+   return ata_id_major_version(dev_id) >= 7 && (dev_id[62] & 0x8000);
+}
+
 static inline int is_multi_taskfile(struct ata_taskfile *tf)
 {
return (tf->command == ATA_CMD_READ_MULTI) ||
diff --git a/include/linux/libata.h b/include/linux/libata.h
index bc5a8d0..b942e90 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -138,6 +138,7 @@ enum {
ATA_DFLAG_AN= (1 << 7), /* AN configured */
ATA_DFLAG_HIPM  = (1 << 8), /* device supports HIPM */
ATA_DFLAG_DIPM  = (1 << 9), /* device supports DIPM */
+   ATA_DFLAG_DMADIR= (1 << 10), /* device requires DMADIR */
ATA_DFLAG_CFG_MASK  = (1 << 12) - 1,
 
ATA_DFLAG_PIO   = (1 << 12), /* device limited to PIO mode */
-
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html