* [PATCH] SB600 SATA controller PMP support
@ 2008-06-04 2:51 Shane Huang
2008-06-09 5:10 ` Tejun Heo
0 siblings, 1 reply; 4+ messages in thread
From: Shane Huang @ 2008-06-04 2:51 UTC (permalink / raw)
To: jgarzik; +Cc: linux-ide
There is one bug in SB600 SATA PMP which leads to softreset failure,
this patch can fix the bug.
Signed-off-by: Shane Huang <shane.huang@amd.com>
diff -ruN a/drivers/ata/ahci.c b/drivers/ata/ahci.c
--- a/drivers/ata/ahci.c 2008-06-03 15:23:59.000000000 +0800
+++ b/drivers/ata/ahci.c 2008-06-03 15:20:44.000000000 +0800
@@ -253,6 +253,8 @@
static void ahci_pmp_detach(struct ata_port *ap);
static int ahci_softreset(struct ata_link *link, unsigned int *class,
unsigned long deadline);
+static int ahci_sb600_softreset(struct ata_link *link, unsigned int *class,
+ unsigned long deadline);
static int ahci_hardreset(struct ata_link *link, unsigned int *class,
unsigned long deadline);
static int ahci_vt8251_hardreset(struct ata_link *link, unsigned int *class,
@@ -329,6 +331,12 @@
.hardreset = ahci_p5wdh_hardreset,
};
+static struct ata_port_operations ahci_sb600_ops = {
+ .inherits = &ahci_ops,
+ .softreset = ahci_sb600_softreset,
+ .pmp_softreset = ahci_sb600_softreset,
+};
+
#define AHCI_HFLAGS(flags) .private_data = (void *)(flags)
static const struct ata_port_info ahci_port_info[] = {
@@ -359,11 +367,11 @@
{
AHCI_HFLAGS (AHCI_HFLAG_IGN_SERR_INTERNAL |
AHCI_HFLAG_32BIT_ONLY | AHCI_HFLAG_NO_MSI |
- AHCI_HFLAG_SECT255 | AHCI_HFLAG_NO_PMP),
+ AHCI_HFLAG_SECT255),
.flags = AHCI_FLAG_COMMON,
.pio_mask = 0x1f, /* pio0-4 */
.udma_mask = ATA_UDMA6,
- .port_ops = &ahci_ops,
+ .port_ops = &ahci_sb600_ops,
},
/* board_ahci_mv */
{
@@ -1270,11 +1278,24 @@
return ata_check_ready(status);
}
-static int ahci_softreset(struct ata_link *link, unsigned int *class,
- unsigned long deadline)
+static int ahci_sb600_check_ready(struct ata_link *link)
+{
+ void __iomem *port_mmio = ahci_port_base(link->ap);
+
+ u8 status = readl(port_mmio + PORT_TFDATA) & 0xFF;
+ u32 irq_status = readl(port_mmio + PORT_IRQ_STAT);
+
+ if (irq_status & PORT_IRQ_BAD_PMP)
+ return -EIO;
+
+ return ata_check_ready(status);
+}
+
+static int ahci_do_softreset(struct ata_link *link, unsigned int *class,
+ int pmp, unsigned long deadline,
+ int (*check_ready)(struct ata_link *link))
{
struct ata_port *ap = link->ap;
- int pmp = sata_srst_pmp(link);
const char *reason = NULL;
unsigned long now, msecs;
struct ata_taskfile tf;
@@ -1312,15 +1333,12 @@
ahci_exec_polled_cmd(ap, pmp, &tf, 0, 0, 0);
/* wait for link to become ready */
- rc = ata_wait_after_reset(link, deadline, ahci_check_ready);
+ rc = ata_wait_after_reset(link, deadline, check_ready);
/* link occupied, -ENODEV too is an error */
if (rc) {
reason = "device not ready";
goto fail;
}
- *class = ahci_dev_classify(ap);
-
- DPRINTK("EXIT, class=%u\n", *class);
return 0;
fail:
@@ -1328,6 +1346,57 @@
return rc;
}
+static int ahci_softreset(struct ata_link *link, unsigned int *class,
+ unsigned long deadline)
+{
+ struct ata_port *ap = link->ap;
+ int pmp = sata_srst_pmp(link);
+ int rc;
+
+ DPRINTK("ENTER\n");
+
+ rc = ahci_do_softreset(link, class, pmp, deadline, ahci_check_ready);
+ if (!rc) {
+ *class = ahci_dev_classify(ap);
+ DPRINTK("EXIT, class=%u\n", *class);
+ }
+ return rc;
+}
+
+static int ahci_sb600_softreset(struct ata_link *link, unsigned int *class,
+ unsigned long deadline)
+{
+ struct ata_port *ap = link->ap;
+ void __iomem *port_mmio = ahci_port_base(ap);
+ int pmp = sata_srst_pmp(link);
+ int rc;
+ u32 irq_sts;
+
+ DPRINTK("ENTER\n");
+
+ rc = ahci_do_softreset(link, class, pmp, deadline,
+ ahci_sb600_check_ready);
+
+ /* Soft reset fails on some ATI chips with IPMS set when PMP
+ is enabled but SATA HDD/ODD is connected to SATA port,
+ do soft reset again to port 0. */
+ if (rc == -EIO) {
+ irq_sts = readl(port_mmio + PORT_IRQ_STAT);
+ if (irq_sts & PORT_IRQ_BAD_PMP) {
+ ata_link_printk(link, KERN_WARNING,
+ "softreset failed, try again\n");
+ rc = ahci_do_softreset(link, class, 0, deadline,
+ ahci_check_ready);
+ }
+ }
+
+ if (!rc) {
+ *class = ahci_dev_classify(ap);
+ DPRINTK("EXIT, class=%u\n", *class);
+ }
+ return rc;
+}
+
static int ahci_hardreset(struct ata_link *link, unsigned int *class,
unsigned long deadline)
{
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] SB600 SATA controller PMP support
2008-06-04 2:51 Shane Huang
@ 2008-06-09 5:10 ` Tejun Heo
0 siblings, 0 replies; 4+ messages in thread
From: Tejun Heo @ 2008-06-09 5:10 UTC (permalink / raw)
To: Shane Huang; +Cc: jgarzik, linux-ide
Hello, Shane.
I think it's in the right direction. Just a few nits.
Shane Huang wrote:
> +static int ahci_sb600_check_ready(struct ata_link *link)
> +{
> + void __iomem *port_mmio = ahci_port_base(link->ap);
> +
Please kill this empty line inbetween variable declarations.
> + u8 status = readl(port_mmio + PORT_TFDATA) & 0xFF;
> + u32 irq_status = readl(port_mmio + PORT_IRQ_STAT);
> +
> + if (irq_status & PORT_IRQ_BAD_PMP)
> + return -EIO;
> +
> + return ata_check_ready(status);
> +}
> +
> +static int ahci_do_softreset(struct ata_link *link, unsigned int *class,
> + int pmp, unsigned long deadline,
> + int (*check_ready)(struct ata_link *link))
Why take @class if not used?
> {
> struct ata_port *ap = link->ap;
> - int pmp = sata_srst_pmp(link);
> const char *reason = NULL;
> unsigned long now, msecs;
> struct ata_taskfile tf;
> @@ -1312,15 +1333,12 @@
> ahci_exec_polled_cmd(ap, pmp, &tf, 0, 0, 0);
>
> /* wait for link to become ready */
> - rc = ata_wait_after_reset(link, deadline, ahci_check_ready);
> + rc = ata_wait_after_reset(link, deadline, check_ready);
> /* link occupied, -ENODEV too is an error */
> if (rc) {
> reason = "device not ready";
> goto fail;
> }
> - *class = ahci_dev_classify(ap);
> -
> - DPRINTK("EXIT, class=%u\n", *class);
Or maybe you can leave this part in ahci_do_softreset()?
> +static int ahci_softreset(struct ata_link *link, unsigned int *class,
> + unsigned long deadline)
> +{
> + struct ata_port *ap = link->ap;
> + int pmp = sata_srst_pmp(link);
> + int rc;
> +
> + DPRINTK("ENTER\n");
> +
> + rc = ahci_do_softreset(link, class, pmp, deadline, ahci_check_ready);
> + if (!rc) {
> + *class = ahci_dev_classify(ap);
> + DPRINTK("EXIT, class=%u\n", *class);
> + }
> + return rc;
> +}
> +
> +static int ahci_sb600_softreset(struct ata_link *link, unsigned int *class,
> + unsigned long deadline)
> +{
> + struct ata_port *ap = link->ap;
> + void __iomem *port_mmio = ahci_port_base(ap);
> + int pmp = sata_srst_pmp(link);
> + int rc;
> + u32 irq_sts;
> +
> + DPRINTK("ENTER\n");
> +
> + rc = ahci_do_softreset(link, class, pmp, deadline,
> + ahci_sb600_check_ready);
> +
> + /* Soft reset fails on some ATI chips with IPMS set when PMP
> + is enabled but SATA HDD/ODD is connected to SATA port,
> + do soft reset again to port 0. */
Please use
/* Blah... blah
* blah
*/
Or
/*
* Blah blah
* blah
*/
> + if (rc == -EIO) {
> + irq_sts = readl(port_mmio + PORT_IRQ_STAT);
> + if (irq_sts & PORT_IRQ_BAD_PMP) {
> + ata_link_printk(link, KERN_WARNING,
> + "softreset failed, try again\n");
Please use a bit more detailed message.
> + rc = ahci_do_softreset(link, class, 0, deadline,
> + ahci_check_ready);
> + }
> + }
> +
> + if (!rc) {
> + *class = ahci_dev_classify(ap);
> + DPRINTK("EXIT, class=%u\n", *class);
> + }
> + return rc;
> +}
I think it's better to organize functions in the following order.
ahci_do_softreset()
ahci_ready()
ahci_softreset()
ahci_sb600_ready()
ahci_sb600_softreset()
W/ explanation on how sb600_ready and sb600_softreset are different.
Please at least add why BAD_PMP quick exit path is necessary in
ahci_sb600_ready().
Thanks.
--
tejun
^ permalink raw reply [flat|nested] 4+ messages in thread
* RE: [PATCH] SB600 SATA controller PMP support
@ 2008-06-10 7:52 Shane Huang
2008-06-13 6:42 ` Jeff Garzik
0 siblings, 1 reply; 4+ messages in thread
From: Shane Huang @ 2008-06-10 7:52 UTC (permalink / raw)
To: htejun, jgarzik; +Cc: linux-ide, shane.huang
Hi Tejun and Jeff,
Tejun wrote:
> I think it's in the right direction. Just a few nits...
I modified the SATA PMP patch with your nice suggestions,
please check the updated one below.
Thanks
Shane
====== CUT HERE =====
There is one bug in ATI SATA PMP of SB600 and SB700 old revision, which leads
to soft reset failure. This patch can fix the bug.
Signed-off-by: Shane Huang <shane.huang@amd.com>
Acked-by: Tejun Heo <htejun@gmail.com>
diff -ruN a/drivers/ata/ahci.c b/drivers/ata/ahci.c
--- a/drivers/ata/ahci.c 2008-06-10 03:27:08.000000000 +0800
+++ b/drivers/ata/ahci.c 2008-06-10 06:15:48.000000000 +0800
@@ -253,6 +253,8 @@
static void ahci_pmp_detach(struct ata_port *ap);
static int ahci_softreset(struct ata_link *link, unsigned int *class,
unsigned long deadline);
+static int ahci_sb600_softreset(struct ata_link *link, unsigned int *class,
+ unsigned long deadline);
static int ahci_hardreset(struct ata_link *link, unsigned int *class,
unsigned long deadline);
static int ahci_vt8251_hardreset(struct ata_link *link, unsigned int *class,
@@ -329,6 +331,12 @@
.hardreset = ahci_p5wdh_hardreset,
};
+static struct ata_port_operations ahci_sb600_ops = {
+ .inherits = &ahci_ops,
+ .softreset = ahci_sb600_softreset,
+ .pmp_softreset = ahci_sb600_softreset,
+};
+
#define AHCI_HFLAGS(flags) .private_data = (void *)(flags)
static const struct ata_port_info ahci_port_info[] = {
@@ -359,11 +367,11 @@
{
AHCI_HFLAGS (AHCI_HFLAG_IGN_SERR_INTERNAL |
AHCI_HFLAG_32BIT_ONLY | AHCI_HFLAG_NO_MSI |
- AHCI_HFLAG_SECT255 | AHCI_HFLAG_NO_PMP),
+ AHCI_HFLAG_SECT255),
.flags = AHCI_FLAG_COMMON,
.pio_mask = 0x1f, /* pio0-4 */
.udma_mask = ATA_UDMA6,
- .port_ops = &ahci_ops,
+ .port_ops = &ahci_sb600_ops,
},
/* board_ahci_mv */
{
@@ -377,12 +385,11 @@
},
/* board_ahci_sb700 */
{
- AHCI_HFLAGS (AHCI_HFLAG_IGN_SERR_INTERNAL |
- AHCI_HFLAG_NO_PMP),
+ AHCI_HFLAGS (AHCI_HFLAG_IGN_SERR_INTERNAL),
.flags = AHCI_FLAG_COMMON,
.pio_mask = 0x1f, /* pio0-4 */
.udma_mask = ATA_UDMA6,
- .port_ops = &ahci_ops,
+ .port_ops = &ahci_sb600_ops,
},
};
@@ -1262,19 +1269,11 @@
return 0;
}
-static int ahci_check_ready(struct ata_link *link)
-{
- void __iomem *port_mmio = ahci_port_base(link->ap);
- u8 status = readl(port_mmio + PORT_TFDATA) & 0xFF;
-
- return ata_check_ready(status);
-}
-
-static int ahci_softreset(struct ata_link *link, unsigned int *class,
- unsigned long deadline)
+static int ahci_do_softreset(struct ata_link *link, unsigned int *class,
+ int pmp, unsigned long deadline,
+ int (*check_ready)(struct ata_link *link))
{
struct ata_port *ap = link->ap;
- int pmp = sata_srst_pmp(link);
const char *reason = NULL;
unsigned long now, msecs;
struct ata_taskfile tf;
@@ -1312,7 +1311,7 @@
ahci_exec_polled_cmd(ap, pmp, &tf, 0, 0, 0);
/* wait for link to become ready */
- rc = ata_wait_after_reset(link, deadline, ahci_check_ready);
+ rc = ata_wait_after_reset(link, deadline, check_ready);
/* link occupied, -ENODEV too is an error */
if (rc) {
reason = "device not ready";
@@ -1328,6 +1327,72 @@
return rc;
}
+static int ahci_check_ready(struct ata_link *link)
+{
+ void __iomem *port_mmio = ahci_port_base(link->ap);
+ u8 status = readl(port_mmio + PORT_TFDATA) & 0xFF;
+
+ return ata_check_ready(status);
+}
+
+static int ahci_softreset(struct ata_link *link, unsigned int *class,
+ unsigned long deadline)
+{
+ int pmp = sata_srst_pmp(link);
+
+ DPRINTK("ENTER\n");
+
+ return ahci_do_softreset(link, class, pmp, deadline, ahci_check_ready);
+}
+
+static int ahci_sb600_check_ready(struct ata_link *link)
+{
+ void __iomem *port_mmio = ahci_port_base(link->ap);
+ u8 status = readl(port_mmio + PORT_TFDATA) & 0xFF;
+ u32 irq_status = readl(port_mmio + PORT_IRQ_STAT);
+
+ /*
+ * There is no need to check TFDATA if BAD PMP is found due to HW bug,
+ * which can save timeout delay.
+ */
+ if (irq_status & PORT_IRQ_BAD_PMP)
+ return -EIO;
+
+ return ata_check_ready(status);
+}
+
+static int ahci_sb600_softreset(struct ata_link *link, unsigned int *class,
+ unsigned long deadline)
+{
+ struct ata_port *ap = link->ap;
+ void __iomem *port_mmio = ahci_port_base(ap);
+ int pmp = sata_srst_pmp(link);
+ int rc;
+ u32 irq_sts;
+
+ DPRINTK("ENTER\n");
+
+ rc = ahci_do_softreset(link, class, pmp, deadline,
+ ahci_sb600_check_ready);
+
+ /*
+ * Soft reset fails on some ATI chips with IPMS set when PMP
+ * is enabled but SATA HDD/ODD is connected to SATA port,
+ * do soft reset again to port 0.
+ */
+ if (rc == -EIO) {
+ irq_sts = readl(port_mmio + PORT_IRQ_STAT);
+ if (irq_sts & PORT_IRQ_BAD_PMP) {
+ ata_link_printk(link, KERN_WARNING,
+ "failed due to HW bug, retry pmp=0\n");
+ rc = ahci_do_softreset(link, class, 0, deadline,
+ ahci_check_ready);
+ }
+ }
+
+ return rc;
+}
+
static int ahci_hardreset(struct ata_link *link, unsigned int *class,
unsigned long deadline)
{
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] SB600 SATA controller PMP support
2008-06-10 7:52 [PATCH] SB600 SATA controller PMP support Shane Huang
@ 2008-06-13 6:42 ` Jeff Garzik
0 siblings, 0 replies; 4+ messages in thread
From: Jeff Garzik @ 2008-06-13 6:42 UTC (permalink / raw)
To: shane.huang; +Cc: htejun, linux-ide
Shane Huang wrote:
> Hi Tejun and Jeff,
>
> Tejun wrote:
>> I think it's in the right direction. Just a few nits...
>
> I modified the SATA PMP patch with your nice suggestions,
> please check the updated one below.
>
> Thanks
> Shane
>
> ====== CUT HERE =====
> There is one bug in ATI SATA PMP of SB600 and SB700 old revision, which leads
> to soft reset failure. This patch can fix the bug.
>
> Signed-off-by: Shane Huang <shane.huang@amd.com>
> Acked-by: Tejun Heo <htejun@gmail.com>
applied
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2008-06-13 6:42 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-06-10 7:52 [PATCH] SB600 SATA controller PMP support Shane Huang
2008-06-13 6:42 ` Jeff Garzik
-- strict thread matches above, loose matches on Subject: below --
2008-06-04 2:51 Shane Huang
2008-06-09 5:10 ` Tejun Heo
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).