Re: [PATCH #upstream-fixes] libata: implement and use HORKAGE_NOSETXFER

[Tejun Heo <tj@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 2326 bytes --]

Reviving an old thread and quoting whole body for new reporter.

Tejun Heo wrote:
> Alan Cox wrote:
>>>> Sorry you can't just blindly do this because some of the controllers snoop
>>>> the SETXFER command to set their timings and whether they expect DMA. Also
>>>> we've no idea if this is a bug in a specific firmware revision, a quirky
>>>> pata/sata bridge or a timing problem of some sort.
>>> I think it'll generally be okay for SATA unless it's bridged over to
>>> PATA controller.  Any other ideas?
>> We seem to have one report, from one user, with one configuration, on one
>> controller, using one firmware set.
> 
> There are two more reports linked from the thread.
> 
>   http://ubuntuforums.org/showthread.php?t=986871
>   https://bugs.launchpad.net/ubuntu/+source/linux/+bug/299865
> 
> And all of them are showing the same problem.  This being a slim SATA
> drive for a laptop, I don't really expect to see the drive in varied
> configurations.  The problem might as well be limited to certain OEM
> drive(s).
> 
>> That to me isn't meaningful evidence of anything that needs a workaround,
>> beyond maybe doing what old IDE does and if a setxfer times out
>> continuing in hope.
> 
> So, as far as the validity of the report goes, I think it's at
> reasonable level.  Add to that my general trigger happiness toward
> quirks and the machine is a VAIO and to me quirking it doesn't seem
> too careless.
> 
>> (We should of course then re-read the identify pages and check the mode
>> in use)
> 
> Of course, whether the workaround is proper is a completely separate
> issue but unless other devices with the same problem creep up and the
> device is happy with the quirk, I don't really think we should modify
> the default configuration sequence for devices like this.
> 
> That said, Moritz, can you please post the output of "hdparm -I" with
> and without the quirk applied?  And can you please use a preoper mail
> address?

Martin Novak (cc'd) is reporting the same problem.  I really think
doing nothing is the worst choice here.  Given the rarity of the
issue, I don't think we need worry too much about long term impact of
the quirk.

Anyways, Martin, can you please post what I asked Moritz - the output
of "hdparm -I" with and without the quirk applied?  Quirk patch is
attached.

Thanks.

-- 
tejun

[-- Attachment #2: horkage-notsetxfer.patch --]
[-- Type: text/x-patch, Size: 2101 bytes --]

libata: implement and use HORKAGE_NOSETXFER

PIONEER DVD-RW DVRTD08 times out SETXFER if no media is present.  The
device is SATA and simply skipping SETXFER works around the problem.
Implement ATA_HORKAGE_NOSETXFER and apply it to the device.

Reported by Moritz Rigler in the following thread.

  http://thread.gmane.org/gmane.linux.ide/36790

Signed-off-by: Tejun Heo <tj@kernel.org>
---
 drivers/ata/libata-core.c |   10 ++++++++--
 include/linux/libata.h    |    1 +
 2 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index c924230..5dcdcb4 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -3390,14 +3390,17 @@ static int ata_dev_set_mode(struct ata_device *dev)
 	struct ata_eh_context *ehc = &dev->link->eh_context;
 	const char *dev_err_whine = "";
 	int ign_dev_err = 0;
-	unsigned int err_mask;
+	unsigned int err_mask = 0;
 	int rc;
 
 	dev->flags &= ~ATA_DFLAG_PIO;
 	if (dev->xfer_shift == ATA_SHIFT_PIO)
 		dev->flags |= ATA_DFLAG_PIO;
 
-	err_mask = ata_dev_set_xfermode(dev);
+	if (!(dev->horkage & ATA_HORKAGE_NOSETXFER))
+		err_mask = ata_dev_set_xfermode(dev);
+	else
+		dev_err_whine = " (SET_XFERMODE skipped)";
 
 	if (err_mask & ~AC_ERR_DEV)
 		goto fail;
@@ -4292,6 +4295,9 @@ static const struct ata_blacklist_entry ata_device_blacklist [] = {
 	/* Devices which aren't very happy with higher link speeds */
 	{ "WD My Book",			NULL,	ATA_HORKAGE_1_5_GBPS, },
 
+	/* Devices which choke on SETXFER.  Presumably SATA only. */
+	{ "PIONEER DVD-RW  DVRTD08",	"1.00",	ATA_HORKAGE_NOSETXFER },
+
 	/* End Marker */
 	{ }
 };
diff --git a/include/linux/libata.h b/include/linux/libata.h
index 3d501db..2b641af 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -385,6 +385,7 @@ enum {
 						    not multiple of 16 bytes */
 	ATA_HORKAGE_FIRMWARE_WARN = (1 << 12),	/* firmware update warning */
 	ATA_HORKAGE_1_5_GBPS	= (1 << 13),	/* force 1.5 Gbps */
+	ATA_HORKAGE_NOSETXFER	= (1 << 14),
 
 	 /* DMA mask for user DMA control: User visible values; DO NOT
 	    renumber */