Re: Hotplug issue with Marvell 6121

[Justin Maggard <jmaggard10@gmail.com>]

On Tue, Mar 9, 2010 at 4:51 PM, Justin Maggard <jmaggard10@gmail.com> wrote:
> I've been experiencing some issues with disk hotplugging using the
> AHCI driver with a Marvell 6121 controller.  It varies a bit when the
> driver gives up, but it's almost always within the first three
> remove/add attempts.  I was using 2.6.27.6 for quite some time and
> never had any issues with hotplug, even after many hotplugs.  The
> problems started when I updated to 2.6.29, and they still exist today
> on 2.6.33.  I'm running a regular i686 non-SMP kernel.  Anyone have
> any ideas?
>
Okay, so I did some more investigation and narrowed down the issue.
It seems the Marvell controller (at least the 6121) doesn't like when
you limit the link speed.  So when the libata error handler slows down
the link speed on the last hard reset after the disk is removed, there
is a good chance that a newly-added disk on that channel will not be
recognized when you plug it in.

I'm sure there's a much better way to fix it, as this patch feels
terribly wrong... but it fixes the issue for me.  Any chance of this
getting addressed upstream?

--- linux-2.6.33.orig/drivers/ata/libata-eh.c	2010-02-24
10:52:17.000000000 -0800
+++ linux-2.6.33/drivers/ata/libata-eh.c	2010-03-10 13:13:37.000000000 -0800
@@ -3300,9 +3305,11 @@ static int ata_eh_schedule_probe(struct
 	return 1;
 }

+#define AHCI_HFLAG_MV_PATA (1 << 4)
 static int ata_eh_handle_dev_fail(struct ata_device *dev, int err)
 {
 	struct ata_eh_context *ehc = &dev->link->eh_context;
+	unsigned int *hflags = dev->link->ap->host->private_data;

 	/* -EAGAIN from EH routine indicates retry without prejudice.
 	 * The requester is responsible for ensuring forward progress.
@@ -3322,7 +3329,12 @@ static int ata_eh_handle_dev_fail(struct
 			/* This is the last chance, better to slow
 			 * down than lose it.
 			 */
-			sata_down_spd_limit(ata_dev_phys_link(dev), 0);
+			if (*hflags & AHCI_HFLAG_MV_PATA) {
+				ata_dev_printk(dev, KERN_WARNING,
+					"Marvell controller detected,"
+					" not slowing down link\n");
+			} else
+				sata_down_spd_limit(ata_dev_phys_link(dev), 0);
 			if (dev->pio_mode > XFER_PIO_0)
 				ata_down_xfermask_limit(dev, ATA_DNXFER_PIO);
 		}