Re: [PATCH] Add port multiplier (PMP) support in sata_fsl driver
Kumar Gala wrote: From: Ashish Kalra [EMAIL PROTECTED] PMP support for sata_fsl driver. Signed-off-by: Ashish Kalra [EMAIL PROTECTED] --- Jeff, The following commit (4c9bf4e799ce06a7378f1196587084802a414c03): libata: replace tf_read with qc_fill_rtf for non-SFF drivers 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. On boot with debug enabled we get something like (w/o this patch): spurious interrupt!!, CC = 0x1 interrupt status 0x1 xx_scr_read, reg_in = 1 spurious interrupt!!, CC = 0x1 interrupt status 0x1 xx_scr_read, reg_in = 1 spurious interrupt!!, CC = 0x1 interrupt status 0x1 xx_scr_read, reg_in = 1 .. continues for ever. - k drivers/ata/sata_fsl.c | 224 +++- 1 files changed, 163 insertions(+), 61 deletions(-) applied ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
RE: [PATCH] Add port multiplier (PMP) support in sata_fsl driver
Hello Tejun,
Thanks for your review comments. Please find my answers below :
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?
Actually, the boot-time hang issue was caused due to my handling of
command completion interrupt and detecting
the queued commands being completed. I was not finding the correct
active link and hence causing command completion
interrupts not being ack'ed correctly and locking the machine as they
remain pending forever.
Now the fix I added works for both PMP and direct device attach cases,
and is actually part of the PMP patch because I
had changed command completion interrupt handling initially for PMP and
addition of links in the libata core,
and this initial change had introduced the bug which caused hangs in
direct device attach cases. Therefore, this fix was added
to my initial PMP patch. For the same reason, I would wish to keep this
as a single patch.
@@ -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()?
/*
- * 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.
Both of the above cases ( ATA_DEV_NONE NO_AUTOPSY ) are basically
hacks because as you have mentioned, currently sata_fsl_softreset() does
both
PHY-reset and softreset handling. This split of sata_fsl_softreset() has
been on my list of things to do for some time, and I will work on it
next
week and send you an updated patch for review.
Thanks,
Ashish
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH] Add port multiplier (PMP) support in sata_fsl driver
Kumar Gala wrote:
From: Ashish Kalra [EMAIL PROTECTED]
PMP support for sata_fsl driver.
Signed-off-by: Ashish Kalra [EMAIL PROTECTED]
---
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 0x)) {
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
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev
