Re: [PATCH] Add port multiplier (PMP) support in sata_fsl driver

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Kumar Gala wrote:
From: Ashish Kalra <ashish.kalra@xxxxxxxxxxxxx>

PMP support for sata_fsl driver.

Signed-off-by: Ashish Kalra <ashish.kalra@xxxxxxxxxxxxx>
---
Jeff,

The following commit (4c9bf4e799ce06a7378f1196587084802a414c03):
libata: replace tf_read with qc_fill_rtf for non-SFF drivers

Heh.. I tried not to break anything and theoretically it shouldn't have. Oh well, there's theory and there's reality. Sorry about the trouble.

Broke the sata_fsl.c driver in 2.6.26.  I know the following patch fixes
the issue, it clearly also adds port multipler support.  I'm not sure if
you are willing to take that as part of 2.6.26 or not, but the current
2.6.26 driver is broken.

Would it be possible to break the patch into two pieces? One to fix the problem and the other to add PMP support?

@@ -771,7 +803,8 @@ try_offline_again:
 		ata_port_printk(ap, KERN_WARNING,
 				"No Device OR PHYRDY change,Hstatus = 0x%x\n",
 				ioread32(hcr_base + HSTATUS));
-		goto err;
+		*class = ATA_DEV_NONE;
+		goto out;
 	}

 	/*
@@ -783,7 +816,8 @@ try_offline_again:

 	if ((temp & 0xFF) != 0x18) {
 		ata_port_printk(ap, KERN_WARNING, "No Signature Update\n");
-		goto err;
+		*class = ATA_DEV_NONE;
+		goto out;

Is setting class to ATA_DEV_NONE necessary? What happens if you drop the above two chunks?

Also, it looks to me that currently sata_fsl_softreset() does both hard and soft resets. Is it possible to split it into sata_fsl_hardreset() and softreset()?

@@ -926,42 +975,28 @@ static void sata_fsl_error_intr(struct ata_port *ap)
 	sata_fsl_scr_read(ap, SCR_ERROR, &SError);
 	if (unlikely(SError & 0xFFFF0000)) {
 		sata_fsl_scr_write(ap, SCR_ERROR, SError);
-		err_mask |= AC_ERR_ATA_BUS;
 	}

 	DPRINTK("error_intr,hStat=0x%x,CE=0x%x,DE =0x%x,SErr=0x%x\n",
 		hstatus, cereg, ioread32(hcr_base + DE), SError);

-	/* handle single device errors */
-	if (cereg) {
-		/*
-		 * clear the command error, also clears queue to the device
-		 * in error, and we can (re)issue commands to this device.
-		 * When a device is in error all commands queued into the
-		 * host controller and at the device are considered aborted
-		 * and the queue for that device is stopped. Now, after
-		 * clearing the device error, we can issue commands to the
-		 * device to interrogate it to find the source of the error.
-		 */
-		dereg = ioread32(hcr_base + DE);
-		iowrite32(dereg, hcr_base + DE);
-		iowrite32(cereg, hcr_base + CE);
+	/* handle fatal errors */
+	if (hstatus & FATAL_ERROR_DECODE) {
+		ehi->err_mask |= AC_ERR_ATA_BUS;
+		ehi->action |= ATA_EH_SOFTRESET;

-		DPRINTK("single device error, CE=0x%x, DE=0x%x\n",
-			ioread32(hcr_base + CE), ioread32(hcr_base + DE));
 		/*
-		 * We should consider this as non fatal error, and TF must
-		 * be updated as done below.
+		 * Ignore serror in case of fatal errors as we always want
+		 * to do a soft-reset of the FSL SATA controller. Analyzing
+		 * serror may cause libata to schedule a hard-reset action,
+		 * and hard-reset currently does not do controller
+		 * offline/online, causing command timeouts and leads to an
+		 * un-recoverable state, hence make libATA ignore
+		 * autopsy in case of fatal errors.
 		 */

-		err_mask |= AC_ERR_DEV;
-	}
+		ehi->flags |= ATA_EHI_NO_AUTOPSY;

This is really fishy. NO_AUTOPSY isn't for stuff like this and libata EH as of the current #usptream and #upstream-fixes default to hardreset, so it will hardreset no matter what you do.

What do you mean by hardreset does'nt do controller offline/online? Is this PHY sequence in sata_fsl_softreset()? I think what should be done is...

* Separate out PHY diddling from sata_fsl_softreset() into sata_fsl_hardreset().

* If sata_fsl_hardreset() alone doesn't leave the controller in usable state. Return -EAGAIN from it to request follow-up SRST on success.

Thanks.

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

[Index of Archives]     [Linux Filesystems]     [Linux SCSI]     [Linux RAID]     [Git]     [Kernel Newbies]     [Linux Newbie]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Samba]     [Device Mapper]

  Powered by Linux