[PATCH 01/02] sata_mv: enable async_notify for 60x1 Rev.C0 and higher

[PATCH 01/02] sata_mv: enable async_notify for 60x1 Rev.C0 and higher

[Mark Lord]

The early chipsets cannot safely handle Async Notification (AN),
but 6041/6081 chip revision "C0" (and newer) can handle it.

So allow AN for "C0" and higher.

This enables use of hotplug on PMP ports for the 6041/6081 PCI Rev.9 chips.

Signed-off-by: Mark Lord <mlord@pobox.com>

--- old/drivers/ata/sata_mv.c	2008-05-30 19:36:52.000000000 -0400
+++ linux/drivers/ata/sata_mv.c	2008-06-17 17:45:00.000000000 -0400
@@ -1322,6 +1322,9 @@
 		goto out_port_free_dma_mem;
 	memset(pp->crpb, 0, MV_CRPB_Q_SZ);
 
+	/* 6041/6081 Rev. "C0" (and newer) are okay with async notify */
+	if (hpriv->hp_flags & MV_HP_ERRATA_60X1C0)
+		ap->flags |= ATA_FLAG_AN;
 	/*
 	 * For GEN_I, there's no NCQ, so we only allocate a single sg_tbl.
 	 * For later hardware, we need one unique sg_tbl per NCQ tag.

[PATCH 02/02] sata_mv: warn on PIO with multiple DRQs

[Mark Lord]

Chip errata sometimes prevents reliable use of PIO commands which involve
more than a single DRQ (data request).  In normal operation, libata should
not generate such PIO commands (uses DMA instead), but they could be sent
in via SG_IO from userspace.

A full workaround might be to break up such commands into sequences
of single DRQ ones, but that's just way too complex for something
that doesn't normally happen in real life.

So, allow the attempt (it often works, despite the errata),
but log the event for reference when somebody screams.

Signed-off-by: Mark Lord <mlord@pobox.com>
---

Tejun/Alan/Jeff:

We may also want a patch to prevent libata-eh from downshifting
all the way to PIO on these chipsets, but I am not sure how to
accomplish that from inside sata_mv.  Any suggestions?


--- old/drivers/ata/sata_mv.c	2008-06-17 19:22:46.000000000 -0400
+++ linux/drivers/ata/sata_mv.c	2008-06-17 19:22:15.000000000 -0400
@@ -1595,6 +1595,24 @@
 
 	if ((qc->tf.protocol != ATA_PROT_DMA) &&
 	    (qc->tf.protocol != ATA_PROT_NCQ)) {
+		static int limit_warnings = 10;
+		/*
+		 * Errata SATA#16, SATA#24: warn if multiple DRQs expected.
+		 *
+		 * Someday, we might implement special polling workarounds
+		 * for these, but it all seems rather unnecessary since we
+		 * normally use only DMA for commands which transfer more
+		 * than a single block of data.
+		 *
+		 * Much of the time, this could just work regardless.
+		 * So for now, just log the incident, and allow the attempt.
+		 */
+		if (limit_warnings && (qc->nbytes / qc->sect_size) > 1) {
+			--limit_warnings;
+			ata_link_printk(qc->dev->link, KERN_WARNING, DRV_NAME
+					": attempting PIO w/multiple DRQ: "
+					"this may fail due to h/w errata\n");
+		}
 		/*
 		 * We're about to send a non-EDMA capable command to the
 		 * port.  Turn off EDMA so there won't be problems accessing

Re: [PATCH 02/02] sata_mv: warn on PIO with multiple DRQs

[Jeff Garzik]

Mark Lord wrote:
> Chip errata sometimes prevents reliable use of PIO commands which involve
> more than a single DRQ (data request).  In normal operation, libata should
> not generate such PIO commands (uses DMA instead), but they could be sent
> in via SG_IO from userspace.
> 
> A full workaround might be to break up such commands into sequences
> of single DRQ ones, but that's just way too complex for something
> that doesn't normally happen in real life.
> 
> So, allow the attempt (it often works, despite the errata),
> but log the event for reference when somebody screams.
> 
> Signed-off-by: Mark Lord <mlord@pobox.com>


applied patches 1-2



> Tejun/Alan/Jeff:
> 
> We may also want a patch to prevent libata-eh from downshifting
> all the way to PIO on these chipsets, but I am not sure how to
> accomplish that from inside sata_mv.  Any suggestions?

Do we really want that?  Seems to me we need that for older bridged PATA 
devices attached to a SATA bridge.

	Jeff

Re: [PATCH 02/02] sata_mv: warn on PIO with multiple DRQs

[Mark Lord]

Jeff Garzik wrote:
> Mark Lord wrote:
>> Chip errata sometimes prevents reliable use of PIO commands which involve
>> more than a single DRQ (data request).  In normal operation, libata 
>> should
>> not generate such PIO commands (uses DMA instead), but they could be sent
>> in via SG_IO from userspace.
>>
>> A full workaround might be to break up such commands into sequences
>> of single DRQ ones, but that's just way too complex for something
>> that doesn't normally happen in real life.
>>
>> So, allow the attempt (it often works, despite the errata),
>> but log the event for reference when somebody screams.
>>
>> Signed-off-by: Mark Lord <mlord@pobox.com>
> 
> 
> applied patches 1-2
> 
> 
> 
>> Tejun/Alan/Jeff:
>>
>> We may also want a patch to prevent libata-eh from downshifting
>> all the way to PIO on these chipsets, but I am not sure how to
>> accomplish that from inside sata_mv.  Any suggestions?
> 
> Do we really want that?  Seems to me we need that for older bridged PATA 
> devices attached to a SATA bridge.
..

Just about everything can do some form of DMA nowadays,
but regardless of that the sata_mv chipsets have errata
for PIO of anything more than a single sector.

In practice, I haven't managed to trigger the issue here,
but the errata descriptions indicate that it should be a major
issue for any volume of multi-sector PIO commands.

Cheers

Re: [PATCH 02/02] sata_mv: warn on PIO with multiple DRQs

[Tejun Heo]

Mark Lord wrote:
>>> We may also want a patch to prevent libata-eh from downshifting
>>> all the way to PIO on these chipsets, but I am not sure how to
>>> accomplish that from inside sata_mv.  Any suggestions?
>>
>> Do we really want that?  Seems to me we need that for older bridged
>> PATA devices attached to a SATA bridge.
> ..
> 
> Just about everything can do some form of DMA nowadays,
> but regardless of that the sata_mv chipsets have errata
> for PIO of anything more than a single sector.
> 
> In practice, I haven't managed to trigger the issue here,
> but the errata descriptions indicate that it should be a major
> issue for any volume of multi-sector PIO commands.

The following nack'd patch has something similar if it's really necessary.

http://article.gmane.org/gmane.linux.ide/32282

-- 
tejun

Re: [PATCH 02/02] sata_mv: warn on PIO with multiple DRQs

[Jeff Garzik]

Mark Lord wrote:
> Just about everything can do some form of DMA nowadays,
> but regardless of that the sata_mv chipsets have errata
> for PIO of anything more than a single sector.
> 
> In practice, I haven't managed to trigger the issue here,
> but the errata descriptions indicate that it should be a major
> issue for any volume of multi-sector PIO commands.


I need to review the state of PIO-multi, but IIRC it's not used by default.

We should be able to handle this errata without banning all PIO...

	Jeff

[PATCH] sata_mv: safer logic for limit_warnings

[Mark Lord]

There is a miniscule chance that two separate host controllers
might be in sata_mv at the same time and manage to decrement
the static limit_warnings variable below zero.

Fix the comparison to deal with it.

Signed-off-by: Mark Lord <mlord@pobox.com>
---

(blush) How did I miss that ???

--- old/drivers/ata/sata_mv.c	2008-06-17 19:22:15.000000000 -0400
+++ linux//drivers/ata/sata_mv.c	2008-06-18 21:52:19.000000000 -0400
@@ -1607,7 +1607,7 @@
 		 * Much of the time, this could just work regardless.
 		 * So for now, just log the incident, and allow the attempt.
 		 */
-		if (limit_warnings && (qc->nbytes / qc->sect_size) > 1) {
+		if (limit_warnings > 0 && (qc->nbytes / qc->sect_size) > 1) {
 			--limit_warnings;
 			ata_link_printk(qc->dev->link, KERN_WARNING, DRV_NAME
 					": attempting PIO w/multiple DRQ: "

Re: [PATCH 02/02] sata_mv: warn on PIO with multiple DRQs

[Mark Lord]

Jeff Garzik wrote:
> Mark Lord wrote:
>> Just about everything can do some form of DMA nowadays,
>> but regardless of that the sata_mv chipsets have errata
>> for PIO of anything more than a single sector.
>>
>> In practice, I haven't managed to trigger the issue here,
>> but the errata descriptions indicate that it should be a major
>> issue for any volume of multi-sector PIO commands.
> 
> 
> I need to review the state of PIO-multi, but IIRC it's not used by default.
> 
> We should be able to handle this errata without banning all PIO...
..

I'm not really all that worried about it.
But it affects *non multi* PIO as well -- anything that can generate
more than a single DRQ per command-issue.

So a READ_SECTORS or WRITE_SECTORS (pio) for anything more than
a single sector could do it, in theory.

If we *really* care, then I'll dig into it deeper in the next kernel.
It can be worked around in the IRQ handler, by taking great care in
the exact sequence of IRQ-arrival and DRQ-status checks.

Heck.. we might even have it right (by accident) today.

Cheers

Re: [PATCH 02/02] sata_mv: warn on PIO with multiple DRQs

[Jeff Garzik]

Mark Lord wrote:
> In practice, I haven't managed to trigger the issue here,
> but the errata descriptions indicate that it should be a major
> issue for any volume of multi-sector PIO commands.


To be specific:  multi-sector or multi-DRQ?

	Jeff

Re: [PATCH 02/02] sata_mv: warn on PIO with multiple DRQs

[Mark Lord]

Jeff Garzik wrote:
> Mark Lord wrote:
>> In practice, I haven't managed to trigger the issue here,
>> but the errata descriptions indicate that it should be a major
>> issue for any volume of multi-sector PIO commands.
> 
> 
> To be specific:  multi-sector or multi-DRQ?
..

Multi-DRQ.  There's also an 8-sector blocksize limit on READ/WRITE_MULTI.

Cheers

Re: [PATCH] sata_mv: safer logic for limit_warnings

[Jeff Garzik]

Mark Lord wrote:
> There is a miniscule chance that two separate host controllers
> might be in sata_mv at the same time and manage to decrement
> the static limit_warnings variable below zero.
> 
> Fix the comparison to deal with it.
> 
> Signed-off-by: Mark Lord <mlord@pobox.com>
> ---
> 
> (blush) How did I miss that ???
> 
> --- old/drivers/ata/sata_mv.c    2008-06-17 19:22:15.000000000 -0400
> +++ linux//drivers/ata/sata_mv.c    2008-06-18 21:52:19.000000000 -0400
> @@ -1607,7 +1607,7 @@
>          * Much of the time, this could just work regardless.
>          * So for now, just log the incident, and allow the attempt.
>          */
> -        if (limit_warnings && (qc->nbytes / qc->sect_size) > 1) {
> +        if (limit_warnings > 0 && (qc->nbytes / qc->sect_size) > 1) {
>             --limit_warnings;
>             ata_link_printk(qc->dev->link, KERN_WARNING, DRV_NAME

applied.  sorry, this got buried, should have gone in sooner.