libata git tree, mbox queue status and contents

libata git tree, mbox queue status and contents

[Jeff Garzik]

This is a quick guide to current libata work that is queued or in 
progress in some way, shape or form.

First, a guide to the branches currently in use in 
git://git.kernel.org/pub/scm/linux/kernel/git/jgarzik/libata-dev.git

   ALL

Everything that is exported for testing (and akpm's -mm tree). 
Currently consists of upstream-fixes + upstream + sil680-mmio branches.


   master

Vanilla upstream linux-2.6.git tree.


   mv-ahci-pata

Marvell 6121/6141 PATA support.  Needs fixing in the 'PATA controller 
command' area before it is usable, and can go upstream.


   mv-ncq

sata_mv NCQ support.  Code is complete and /should/ be working, but it 
does nothing but timeout and fallback to non-NCQ here.  Needs debugging 
before can go upstream.


   new-eh

Convert sata_qstor and sata_sx4 to new EH.  Each needs 
testing/verification before it can go upstream.


   pmask

Add proto_mask to LLDDs, alongside pio_mask, udma_mask, etc.


   sii-lbt

Enable Large Block Transfer mode on sata_sil.  Verified to work locally, 
but some bug reports on the 'net need to be tracked down before this can 
go upstream.


   sil680-mmio

BenH's MMIO patch for pata_sil680.  Some MMIO/flushing type issues were 
raised on the list, a solution was reached.  That solution must be coded 
before this can go upstream.


   upstream

Everything queued for 2.6.24.  Includes everything in upstream-fixes.


   upstream-fixes

Everything queued for 2.6.23-rc.  This tends to go upstream rapidly.




mbox queue, prefixed by author:

* Kristen: ALPM patches.  We definitely want them, as they save a ton of 
power.

* Tony Vroon: LED trigger....  hmmm

* Alan: IORDY handling -- upstream whenever Alan is happy

* Tejun: improved probe info printout.  Want to test and review in 
depth, but probably OK

* Alan: ACPI checks for 80wire cable -- upstream whenever Alan is happy

* Tejun:  Port Multiplier Support -- I still need to review in depth, 
but would like go ahead and push ata_link in

* NVIDIA: sata_nv SW NCQ: need to review WRT FIS state machine, though I 
saw some flaws in there.  OK if that is OK.

* Albert: irq_on/off.  Really need to give this some thought.  Not sure 
I like where this model is going.  Polling and twiddling irq on/off 
should be kept to a minimum, because it's sorta an admission that the 
host state machine has broken down, and we need to bandaid.  Its a 
bandaid not a root-cause solution.

* Albert: minor PIO fixes.  Need to review in depth.

* Kristen: AN: it seems that things got stuck once Al Viro voiced an 
objection?

* Ben Collins: cleanup HPA support.  Need to review and see what's 
needed today, from this patch.

Re: libata git tree, mbox queue status and contents

[Alan Cox]

> * Alan: IORDY handling -- upstream whenever Alan is happy

I'm happy with it from testing. Just a little worried about it going
upstream mid -rc as it could have a weird side effect somewhere. I've
verified an original pre ATA IDE drive with it too now 8)

> * Alan: ACPI checks for 80wire cable -- upstream whenever Alan is happy

Happy

> * Albert: irq_on/off.  Really need to give this some thought.  Not sure 
> I like where this model is going.  Polling and twiddling irq on/off 
> should be kept to a minimum, because it's sorta an admission that the 
> host state machine has broken down, and we need to bandaid.  Its a 
> bandaid not a root-cause solution.

I think of it more as an admission that the IDE design is lacking in a
few areas. No suprise as its an emulation of a 15 year old interface that
was normally used polled.

> * Ben Collins: cleanup HPA support.  Need to review and see what's 
> needed today, from this patch.

We need to track the HPA case Bartlomiej is working on with old IDE - one
drive out there is reporting its geometry wrong.

Re: libata git tree, mbox queue status and contents

[Tejun Heo]

Alan Cox wrote:
>> * Albert: irq_on/off.  Really need to give this some thought.  Not sure 
>> I like where this model is going.  Polling and twiddling irq on/off 
>> should be kept to a minimum, because it's sorta an admission that the 
>> host state machine has broken down, and we need to bandaid.  Its a 
>> bandaid not a root-cause solution.
> 
> I think of it more as an admission that the IDE design is lacking in a
> few areas. No suprise as its an emulation of a 15 year old interface that
> was normally used polled.

I second Alan's opinion.  The root cause of the problem is that the
controller lacks reliable IRQ masking and pending register.  If you add
a slightly malfunctioning controller and/or device into the mix, it's
guaranteed to break.

And there's another addition to the mess from SATA land which I
discovered a few days ago.  Some SATA controllers with SFF interface
don't send non-command H2D Reg FIS when ATA_NIEN is toggled, so the
updated ATA_NIEN is sent together with the next command && some SATA
devices don't honor ATA_NIEN on command H2D Reg FIS.  Fun!

I think we should go combined IRQ + polling mode Jeff once mentioned so
that we automatically fallback to polling if IRQ doesn't work for some
reason.  To do that, we need reliable IRQ masking && the only way to
achieve proper IRQ masking on SFF controllers seems plugging it from the
host side.

Thanks.

-- 
tejun

Re: libata git tree, mbox queue status and contents

[Sergei Shtylyov]

Alan Cox wrote:

>>* Alan: IORDY handling -- upstream whenever Alan is happy

> I'm happy with it from testing. Just a little worried about it going
> upstream mid -rc as it could have a weird side effect somewhere. I've
> verified an original pre ATA IDE drive with it too now 8)

>>* Alan: ACPI checks for 80wire cable -- upstream whenever Alan is happy

> Happy

>>* Albert: irq_on/off.  Really need to give this some thought.  Not sure 
>>I like where this model is going.  Polling and twiddling irq on/off 
>>should be kept to a minimum, because it's sorta an admission that the 
>>host state machine has broken down, and we need to bandaid.  Its a 
>>bandaid not a root-cause solution.

> I think of it more as an admission that the IDE design is lacking in a
> few areas. No suprise as its an emulation of a 15 year old interface that
> was normally used polled.

    Hehe, note that even host polling has always been racy the way ATA spec. 
described it: there was noting said about the period whithin which the device 
should assert INTRQ after clearing BSY (and the interrupt-pending state wasn't 
clearly specified also), so there's a possibility for the fast host to *not* 
clear interrupt pending by reading the status reg. with BSY=0 but before the 
devie enters interrupt-pending state, and thus possibly stalling the further 
transfer.

MBR, Sergei

Re: libata git tree, mbox queue status and contents

[Mark Lord]

Jeff,

This patch (below) is needed by WD drives that powerup-in-standby.

Cheers

Mark

-------- Original Message --------
Subject: + libata-adjust-libata-to-ignore-errors-after.patch added to -mm tree
Date: Wed, 18 Jul 2007 17:03:29 -0700
From: akpm@linux-foundation.org
To: mm-commits@vger.kernel.org
CC: rpower@sysreset.com, htejun@gmail.com, jeff@garzik.org,	liml@rtr.ca


The patch titled
     libata: adjust libata to ignore errors after spinup
has been added to the -mm tree.  Its filename is
     libata-adjust-libata-to-ignore-errors-after.patch

*** Remember to use Documentation/SubmitChecklist when testing your code ***

See http://www.zip.com.au/~akpm/linux/patches/stuff/added-to-mm.txt to find
out what to do about this

------------------------------------------------------
Subject: libata: adjust libata to ignore errors after spinup
From: Ryan Power <rpower@sysreset.com>

Adjust libata to ignore errors after spinup

This patch is to ignore errors from the spinup attempt if the drive is
in the "standby id" state.

Signed-off-by: Ryan Power <rpower@sysreset.com>
Acked-by: Mark Lord <liml@rtr.ca>
Cc: Jeff Garzik <jeff@garzik.org>
Cc: Tejun Heo <htejun@gmail.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---

 drivers/ata/libata-core.c |    2 +-
 1 files changed, 1 insertion(+), 1 deletion(-)

diff -puN /dev/null /dev/null
diff -puN drivers/ata/libata-core.c~libata-adjust-libata-to-ignore-errors-after drivers/ata/libata-core.c
--- a/drivers/ata/libata-core.c~libata-adjust-libata-to-ignore-errors-after
+++ a/drivers/ata/libata-core.c
@@ -1723,7 +1723,7 @@ int ata_dev_read_id(struct ata_device *d
 		tf.protocol = ATA_PROT_NODATA;
 		tf.flags |= ATA_TFLAG_ISADDR | ATA_TFLAG_DEVICE;
 		err_mask = ata_exec_internal(dev, &tf, NULL, DMA_NONE, NULL, 0);
-		if (err_mask) {
+		if (err_mask && id[2] != 0x738c) {
 			rc = -EIO;
 			reason = "SPINUP failed";
 			goto err_out;
_

Patches currently in -mm which might be from rpower@sysreset.com are

libata-adjust-libata-to-ignore-errors-after.patch

Re: libata git tree, mbox queue status and contents

[Jeff Garzik]

Mark Lord wrote:
> Jeff,
> 
> This patch (below) is needed by WD drives that powerup-in-standby.

> drivers/ata/libata-core.c~libata-adjust-libata-to-ignore-errors-after 
> drivers/ata/libata-core.c
> --- a/drivers/ata/libata-core.c~libata-adjust-libata-to-ignore-errors-after
> +++ a/drivers/ata/libata-core.c
> @@ -1723,7 +1723,7 @@ int ata_dev_read_id(struct ata_device *d
>         tf.protocol = ATA_PROT_NODATA;
>         tf.flags |= ATA_TFLAG_ISADDR | ATA_TFLAG_DEVICE;
>         err_mask = ata_exec_internal(dev, &tf, NULL, DMA_NONE, NULL, 0);
> -        if (err_mask) {
> +        if (err_mask && id[2] != 0x738c) {
>             rc = -EIO;
>             reason = "SPINUP failed";
>             goto err_out;


Thanks for the poke, it wasn't in my queue.

	Jeff

Re: libata git tree, mbox queue status and contents

[Jens Axboe]

On Fri, Aug 03 2007, Jeff Garzik wrote:
> * Kristen: ALPM patches.  We definitely want them, as they save a ton of 
> power.

The problem with ALPM, as I see it, is that it is way too aggressive. It
really needs to be combined with a timer to be useful, it's really a
huge shame that it doesn't come equipped with a timeout setting in
hardware. Lacking that, we could punt to using a second aligned timer
that just checks for activity in the last second, and if none was seen
then enable ALPM. That should have absolutely minimal impact on CPU
consumption. Likewise for when we see IO, when the rate/sec goes beyond
a low threshold then disable ALPM again.

In my testing on this notebook (x60), throughput was reduced to about
30% when using ALPM. So while it does save a good amount of power, it
also makes the disk a slow dog if you are actually using it.

-- 
Jens Axboe