Re: Fwd: [LIBATA] drives not detected

2007-02-23 Thread Patrick Ale

On 2/23/07, Tejun Heo [EMAIL PROTECTED] wrote:


libata and ide use different methods to discover devices.  libata
depends on reset to work.  After reset, devices are supposed to send
diagnostic and the classfication code via TF registers.  OTOH, ide just
issues IDENTIFY and then IDENTIFY PACKET to each device slot without
resetting the channel.  Maybe we're doing something wrong in the pdc
driver or the controller just doesn't like libata's probing method.


Extra info for you:

Tejun, you have way more libata experience as me, all of you do
probably and I don't question your theory but let me try to give you
my theory about why I think it's a drive or cable.

As you remember I have this onboard controller,also Promise, with
master and slave on one controller. And this works just fine, which
makes it kind of hard for me to believe that it is something in the
driver. Why would one bus work great and the other doesnt?

A very short summary for the gurus:

- The secundaiy bus on the onboard promise board wasnt in use yet, so
I connected the drives to this bus, and you can guess it, the drives
got detected by the BIOS but not by the drivers. So we can rule out at
least that my PCI-Add on Promise controller is broken and that we have
to look for the problem some where else.

- Legacy IDE drivers DO work, but, as you say yourself, they dont
depend on a drive reset,

- libata detects the drives, but only with one drive attached to the cable.

Now, taking the single drive operation working with libata and the old
ide drives not needing a reset to detect disks into account, plus this
information above in account: my theory.

What if, the Promise BIOS doesn't depend on this reset either?
What if the BIOS does something along the line of screaming HELLO?!
and the drives, both independent from each other yell back: Ola, I am
a Seagate ST8whatever and I can do UDMA100, and the BIOS just takes
this for granted. With other words, what if the BIOS only cares for
the disk controller to be able to give its identification information
from the firmware, even if the platters are all to the moon? It
wouldn't  be the first time I see disks being detected by an ATA BIOS
and the disks being totally useless, not to mention, disks lying about
their capabilities,

Please note in past dmesg-es I sent that the legacy IDE drivers claim
how the native capacity of both drives is something like 102 petabyte.
Do disks these size exist even?
Based on what you tell me, the legacy IDE drivers tend to take things
for granted too, either it just reads what the ATA BIOS tells the
drivers or it does it own check, just by screaming HELLO and getting
the same Ola back from the drives.

Now, libata seems to be way more suffisticated and handles drives and
detection in a way more fancy matter. So, what if the machine boots
fine, less suffisticated BIOS detects the drives and its sizes and
now we boot the kernel. Libata does its stuff, does a reset of the bus
and *bang* a collision of some sort comes forth?

I checked the jumpers, changed them, changed the ATA bus even, nothing helps.
And even tho I changed the ATA cable allready, who guarantees me that
this cable is correct? With my luck, it isnt. Which is why I want to
just yank the cables from the other _working_ master and slave combo
and see how this goes.

Right now I am moving data from the problem disks to a safe
location, just to guarantee I dont fry my data in any sort.

Please let me know if my theory makes any sense or if I should cut on
the slibovica and palinka :)


Patrick
-
To unsubscribe from this list: send the line unsubscribe linux-ide in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Fwd: [LIBATA] drives not detected

2007-02-23 Thread Jeff Garzik

Is this detection problem limited to Promise controllers?

It's possible we might be programming the Promise controllers in dual 
master mode, and then accidentally missing the second channel somehow.


Jeff



-
To unsubscribe from this list: send the line unsubscribe linux-ide in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Fwd: [LIBATA] drives not detected

2007-02-23 Thread Patrick Ale

On 2/23/07, Jeff Garzik [EMAIL PROTECTED] wrote:

Is this detection problem limited to Promise controllers?

It's possible we might be programming the Promise controllers in dual
master mode, and then accidentally missing the second channel somehow.


Good point and good news, I have a free bus on my VIA controller
aswell, not really free, but I can disconnect the DVD writer to test
this.

Will do so when I am home.


Patrick
-
To unsubscribe from this list: send the line unsubscribe linux-ide in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] libata-eh: Add a simple mechanism for silencing error reporting

2007-02-23 Thread Jeff Garzik

Alan wrote:

We want to be able to issue commands that fail silently some of the time
(set_features/xfer rate to CF 1.4 devices, perhaps some others such as
user SG_IO commands ought to be silent too as the error is for the app)

This is a minimal implementation, we can extend it so the QUIET flag
isn't quiet about errors that are not command errors but indicate
infrastructre problems (CRC errors, HSM violation, DeviceFault) if need
be.

Signed-off-by: Alan Cox [EMAIL PROTECTED]

diff -u --new-file --recursive --exclude-from /usr/src/exclude 
linux.vanilla-2.6.20-rc6-mm3/drivers/ata/libata-eh.c 
linux-2.6.20-rc6-mm3/drivers/ata/libata-eh.c
--- linux.vanilla-2.6.20-rc6-mm3/drivers/ata/libata-eh.c2007-01-31 
14:20:39.0 +
+++ linux-2.6.20-rc6-mm3/drivers/ata/libata-eh.c2007-01-31 
14:27:25.0 +
@@ -1407,6 +1407,8 @@
continue;
if (qc-flags  ATA_QCFLAG_SENSE_VALID  !qc-err_mask)
continue;
+   if (qc-tf.flags  ATA_TFLAG_QUIET)
+   continue;
 
 		nr_failed++;

}


I would rather pass an ok-to-fail-without-recovery type flag to EH. 
This TFLAG_QUIET approach is poorly defined, probably temporary, and not 
really interesting for upstream


Jeff



-
To unsubscribe from this list: send the line unsubscribe linux-ide in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] libata: IORDY handling, quiet handling

2007-02-23 Thread Jeff Garzik

Alan wrote:

For further review

Turn on the IORDY handling logic.

If a controller doesnt support IORDY don't try and use it
If a device doesn't support IORDY don't try and use it
Otherwise use it whenever possible.
Always use IORDY for PIO3 and higher (it's mandatory)

Send the correct set_features command if operating without IORDY

Signed-off-by: Alan Cox [EMAIL PROTECTED]


modulo TFLAG_QUIET mentioned in previous email, this seems OK to me.


-
To unsubscribe from this list: send the line unsubscribe linux-ide in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 5/5] ahci: consider SDB FIS containing spurious NCQ completions HSM violation (regenerated)

2007-02-23 Thread Jeff Garzik

Tejun Heo wrote:

SDB FIS containing spurious NCQ completions is a clear protocol
violation.  Currently, only some Maxtors with early firmware revisions
are showing this problem.  Those firmwares have other NCQ related
problems including buggy NCQ error reporting and occasional lock up
after NCQ errors.

Consider spurious NCQ completions HSM violation and freeze the port
after it.  EH will turn off NCQ after this happens several times.
Eventually drives which show this behavior should be blacklisted for
NCQ.

Signed-off-by: Tejun Heo [EMAIL PROTECTED]
---
Regenerated against the current #upstream
(b216f3f887bb3e73808250841035a6fc4f6fc9e0).


applied


-
To unsubscribe from this list: send the line unsubscribe linux-ide in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] libata: fix remaining ap-id

2007-02-23 Thread Jeff Garzik

Tejun Heo wrote:

Merge order left libata-acpi and pata_scc with remainling usage of
ap-id.  Kill superflous id printing and substitute the remaining ones
with ap-print_id.

Signed-off-by: Tejun Heo [EMAIL PROTECTED]


applied


-
To unsubscribe from this list: send the line unsubscribe linux-ide in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] remove duplicate ids from ata_piix

2007-02-23 Thread Jeff Garzik

Greg KH wrote:

From: Greg Kroah-Hartman [EMAIL PROTECTED]

It seems that the ata_piix driver has two duplicate ids, one of them
with a different 'private' field in it, which was never being used due
to the match for the device happening on an earlier entry.

This patch removes the duplicates, if this is the correct thing to do in
this case for the ICH5 device or not, I'll leave to you :)

This duplication was pointed out to me by Kay Sievers [EMAIL PROTECTED]

Signed-off-by: Greg Kroah-Hartman [EMAIL PROTECTED]


manually applied the proper fix, this for spotting this


-
To unsubscribe from this list: send the line unsubscribe linux-ide in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] libata-eh: Add a simple mechanism for silencing error reporting

2007-02-23 Thread Alan
 I would rather pass an ok-to-fail-without-recovery type flag to EH. 
 This TFLAG_QUIET approach is poorly defined, probably temporary, and not 
 really interesting for upstream

Fail without recovery flags make sense, but then we also need to ensure
it is a fail without recovery messages. We could add qc-ignore_mask to
mirror qc-err_mask ?

Alan

-
To unsubscribe from this list: send the line unsubscribe linux-ide in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: hdb: irq timeout: status=0xd0 { Busy }

2007-02-23 Thread Alan
 Feb 20 23:15:17 localhost kernel: hdb: irq timeout: status=0xd0 { Busy }
 Feb 20 23:15:27 localhost kernel: ide: failed opcode was: unknown
 Feb 20 23:15:27 localhost kernel: hdb: status timeout: status=0xd0 { Busy }
 Feb 20 23:15:27 localhost kernel: ide: failed opcode was: unknown

Your CD-ROM drive stopped responding

 Feb 20 23:17:30 localhost kernel: ide: failed opcode was: unknown
 Feb 20 23:17:30 localhost kernel: hdb: drive not ready for command
 Feb 20 23:17:36 localhost kernel: hdb: status timeout: status=0xd0 { Busy }
 repeats infinitely.

and never came back.

It would be very interesting to know if an older kernel doesn't trigger
this.
-
To unsubscribe from this list: send the line unsubscribe linux-ide in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: hdb: irq timeout: status=0xd0 { Busy }

2007-02-23 Thread Thue Janus Kristensen

On 2/23/07, Alan [EMAIL PROTECTED] wrote:

 Feb 20 23:15:17 localhost kernel: hdb: irq timeout: status=0xd0 { Busy }
 Feb 20 23:15:27 localhost kernel: ide: failed opcode was: unknown
 Feb 20 23:15:27 localhost kernel: hdb: status timeout: status=0xd0 { Busy }
 Feb 20 23:15:27 localhost kernel: ide: failed opcode was: unknown

Your CD-ROM drive stopped responding

 Feb 20 23:17:30 localhost kernel: ide: failed opcode was: unknown
 Feb 20 23:17:30 localhost kernel: hdb: drive not ready for command
 Feb 20 23:17:36 localhost kernel: hdb: status timeout: status=0xd0 { Busy }
 repeats infinitely.

and never came back.

It would be very interesting to know if an older kernel doesn't trigger
this.


I can keep running 2.6.16.41 for a few weeks. If the problem does not
occur by then it is reasonably probably that it is newly introduced.
As I said, I am not able to trigger the problem intentionally :(.

Would it be possible for the kernel to realize that the drive stopped
responding? And make sure that it doesn't make the whole computer
unresponsive...

Regards, Thue
-
To unsubscribe from this list: send the line unsubscribe linux-ide in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: hdb: irq timeout: status=0xd0 { Busy }

2007-02-23 Thread Alan
 Would it be possible for the kernel to realize that the drive stopped
 responding? And make sure that it doesn't make the whole computer
 unresponsive...

libata should eventually get rid of the drive. The old ide isn't quite so
smart.

-
To unsubscribe from this list: send the line unsubscribe linux-ide in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] (pata-2.6 fix queue) sl82c105: DMA support code cleanup (take 2)

2007-02-23 Thread Sergei Shtylyov
Fold the now equivalent code in the ide_dma_check() method into a mere call to
ide_use_dma().  Make config_for_dma() return non-zero if DMA mode has been set
and call it from the ide_dma_check() method instead of ide_dma_on().
Also, defer writing the DMA timings to the chip registers until DMA is actually
turned on (and do not enable IORDY for DMA).  Remove unnecessary code from the
init_hwif() method, improve its looks overall, and also rename the driver's
ide_dma_check() and dma_start() methods.

---
This version takes into account that the ide-use-fast-pio.patch had been merged
to mainline...

Signed-off-by: Sergei Shtylyov [EMAIL PROTECTED]
Signed-off-by: Bartlomiej Zolnierkiewicz [EMAIL PROTECTED]

 drivers/ide/pci/sl82c105.c |  112 -
 1 files changed, 41 insertions(+), 71 deletions(-)

Index: linux-2.6/drivers/ide/pci/sl82c105.c
===
--- linux-2.6.orig/drivers/ide/pci/sl82c105.c
+++ linux-2.6/drivers/ide/pci/sl82c105.c
@@ -11,6 +11,8 @@
  * Merge in Russell's HW workarounds, fix various problems
  * with the timing registers setup.
  *  -- Benjamin Herrenschmidt (01/11/03) [EMAIL PROTECTED]
+ *
+ * Copyright (C) 2006-2007 MontaVista Software, Inc. [EMAIL PROTECTED]
  */
 
 #include linux/types.h
@@ -115,59 +117,28 @@ static void sl82c105_tune_drive(ide_driv
 }
 
 /*
- * Configure the drive and the chipset for DMA
++  * Configure the drive for DMA.
++  * We'll program the chipset only when DMA is actually turned on.
  */
 static int config_for_dma (ide_drive_t *drive)
 {
-   ide_hwif_t *hwif = HWIF(drive);
-   struct pci_dev *dev = hwif-pci_dev;
-   unsigned int reg;
-
DBG((config_for_dma(drive:%s)\n, drive-name));
 
-   reg = (hwif-channel ? 0x4c : 0x44) + (drive-select.b.unit ? 4 : 0);
-
if (ide_config_drive_speed(drive, XFER_MW_DMA_2) != 0)
-   return 1;
-
-   pci_write_config_word(dev, reg, 0x0240);
+   return 0;
 
-   return 0;
+   return ide_dma_enable(drive);
 }
 
 /*
- * Check to see if the drive and
- * chipset is capable of DMA mode
+ * Check to see if the drive and chipset are capable of DMA mode.
  */
-
-static int sl82c105_check_drive (ide_drive_t *drive)
+static int sl82c105_ide_dma_check (ide_drive_t *drive)
 {
-   ide_hwif_t *hwif= HWIF(drive);
-
-   DBG((sl82c105_check_drive(drive:%s)\n, drive-name));
-
-   do {
-   struct hd_driveid *id = drive-id;
-
-   if (!drive-autodma)
-   break;
+   DBG((sl82c105_ide_dma_check(drive:%s)\n, drive-name));
 
-   if (!id || !(id-capability  1))
-   break;
-
-   /* Consult the list of known bad drives */
-   if (__ide_dma_bad_drive(drive))
-   break;
-
-   if (id-field_valid  2) {
-   if ((id-dma_mword  hwif-mwdma_mask) ||
-   (id-dma_1word  hwif-swdma_mask))
-   return 0;
-   }
-
-   if (__ide_dma_good_drive(drive)  id-eide_dma_time  150)
-   return 0;
-   } while (0);
+   if (ide_use_dma(drive)  config_for_dma(drive))
+   return 0;
 
return -1;
 }
@@ -236,7 +207,7 @@ static int sl82c105_ide_dma_lost_irq(ide
  * The generic IDE core will have disabled the BMEN bit before this
  * function is called.
  */
-static void sl82c105_ide_dma_start(ide_drive_t *drive)
+static void sl82c105_dma_start(ide_drive_t *drive)
 {
ide_hwif_t *hwif = HWIF(drive);
struct pci_dev *dev = hwif-pci_dev;
@@ -258,12 +229,18 @@ static int sl82c105_ide_dma_timeout(ide_
 
 static int sl82c105_ide_dma_on (ide_drive_t *drive)
 {
+   struct pci_dev *dev = HWIF(drive)-pci_dev;
+   int rc, reg = 0x44 + drive-dn * 4;
+
DBG((sl82c105_ide_dma_on(drive:%s)\n, drive-name));
 
-   if (config_for_dma(drive))
-   return 1;
-   printk(KERN_INFO %s: DMA enabled\n, drive-name);
-   return __ide_dma_on(drive);
+   rc = __ide_dma_on(drive);
+   if (rc == 0) {
+   pci_write_config_word(dev, reg, 0x0200);
+
+   printk(KERN_INFO %s: DMA enabled\n, drive-name);
+   }
+   return rc;
 }
 
 static void sl82c105_dma_off_quietly(ide_drive_t *drive)
@@ -376,7 +353,7 @@ static unsigned int __devinit init_chips
 }
 
 /*
- * Initialise the chip
+ * Initialise IDE channel
  */
 static void __devinit init_hwif_sl82c105(ide_hwif_t *hwif)
 {
@@ -396,11 +373,6 @@ static void __devinit init_hwif_sl82c105
hwif-drives[0].drive_data = hwif-drives[1].drive_data = 0x0909;
hwif-drives[0].autotune   = hwif-drives[1].autotune   = 1;
 
-   hwif-atapi_dma = 0;
-   hwif-mwdma_mask = 0;
-   hwif-swdma_mask = 0;
-   hwif-autodma = 0;
-
if (!hwif-dma_base)
return;
 
@@ -410,27 +382,27 @@ static 

[PATCH] (pata-2.6 fix queue) sl82c105: DMA support code cleanup (take 3)

2007-02-23 Thread Sergei Shtylyov
Fold the now equivalent code in the ide_dma_check() method into a mere call to
ide_use_dma().  Make config_for_dma() return non-zero if DMA mode has been set
and call it from the ide_dma_check() method instead of ide_dma_on().
Also, defer writing the DMA timings to the chip registers until DMA is actually
turned on (and do not enable IORDY for DMA).  Remove unnecessary code from the
init_hwif() method, improve its looks overall, and also rename the driver's
ide_dma_check() and dma_start() methods.

---
This version takes into account that the ide-use-fast-pio.patch had been merged
to mainline...

Argh! Where those extra pluses camen from in the second hunk?!

Signed-off-by: Sergei Shtylyov [EMAIL PROTECTED]
Signed-off-by: Bartlomiej Zolnierkiewicz [EMAIL PROTECTED]

 drivers/ide/pci/sl82c105.c |  112 -
 1 files changed, 41 insertions(+), 71 deletions(-)

Index: linux-2.6/drivers/ide/pci/sl82c105.c
===
--- linux-2.6.orig/drivers/ide/pci/sl82c105.c
+++ linux-2.6/drivers/ide/pci/sl82c105.c
@@ -11,6 +11,8 @@
  * Merge in Russell's HW workarounds, fix various problems
  * with the timing registers setup.
  *  -- Benjamin Herrenschmidt (01/11/03) [EMAIL PROTECTED]
+ *
+ * Copyright (C) 2006-2007 MontaVista Software, Inc. [EMAIL PROTECTED]
  */
 
 #include linux/types.h
@@ -115,59 +117,28 @@ static void sl82c105_tune_drive(ide_driv
 }
 
 /*
- * Configure the drive and the chipset for DMA
+ * Configure the drive for DMA.
+ * We'll program the chipset only when DMA is actually turned on.
  */
 static int config_for_dma (ide_drive_t *drive)
 {
-   ide_hwif_t *hwif = HWIF(drive);
-   struct pci_dev *dev = hwif-pci_dev;
-   unsigned int reg;
-
DBG((config_for_dma(drive:%s)\n, drive-name));
 
-   reg = (hwif-channel ? 0x4c : 0x44) + (drive-select.b.unit ? 4 : 0);
-
if (ide_config_drive_speed(drive, XFER_MW_DMA_2) != 0)
-   return 1;
-
-   pci_write_config_word(dev, reg, 0x0240);
+   return 0;
 
-   return 0;
+   return ide_dma_enable(drive);
 }
 
 /*
- * Check to see if the drive and
- * chipset is capable of DMA mode
+ * Check to see if the drive and chipset are capable of DMA mode.
  */
-
-static int sl82c105_check_drive (ide_drive_t *drive)
+static int sl82c105_ide_dma_check (ide_drive_t *drive)
 {
-   ide_hwif_t *hwif= HWIF(drive);
-
-   DBG((sl82c105_check_drive(drive:%s)\n, drive-name));
-
-   do {
-   struct hd_driveid *id = drive-id;
-
-   if (!drive-autodma)
-   break;
+   DBG((sl82c105_ide_dma_check(drive:%s)\n, drive-name));
 
-   if (!id || !(id-capability  1))
-   break;
-
-   /* Consult the list of known bad drives */
-   if (__ide_dma_bad_drive(drive))
-   break;
-
-   if (id-field_valid  2) {
-   if ((id-dma_mword  hwif-mwdma_mask) ||
-   (id-dma_1word  hwif-swdma_mask))
-   return 0;
-   }
-
-   if (__ide_dma_good_drive(drive)  id-eide_dma_time  150)
-   return 0;
-   } while (0);
+   if (ide_use_dma(drive)  config_for_dma(drive))
+   return 0;
 
return -1;
 }
@@ -236,7 +207,7 @@ static int sl82c105_ide_dma_lost_irq(ide
  * The generic IDE core will have disabled the BMEN bit before this
  * function is called.
  */
-static void sl82c105_ide_dma_start(ide_drive_t *drive)
+static void sl82c105_dma_start(ide_drive_t *drive)
 {
ide_hwif_t *hwif = HWIF(drive);
struct pci_dev *dev = hwif-pci_dev;
@@ -258,12 +229,18 @@ static int sl82c105_ide_dma_timeout(ide_
 
 static int sl82c105_ide_dma_on (ide_drive_t *drive)
 {
+   struct pci_dev *dev = HWIF(drive)-pci_dev;
+   int rc, reg = 0x44 + drive-dn * 4;
+
DBG((sl82c105_ide_dma_on(drive:%s)\n, drive-name));
 
-   if (config_for_dma(drive))
-   return 1;
-   printk(KERN_INFO %s: DMA enabled\n, drive-name);
-   return __ide_dma_on(drive);
+   rc = __ide_dma_on(drive);
+   if (rc == 0) {
+   pci_write_config_word(dev, reg, 0x0200);
+
+   printk(KERN_INFO %s: DMA enabled\n, drive-name);
+   }
+   return rc;
 }
 
 static void sl82c105_dma_off_quietly(ide_drive_t *drive)
@@ -376,7 +353,7 @@ static unsigned int __devinit init_chips
 }
 
 /*
- * Initialise the chip
+ * Initialise IDE channel
  */
 static void __devinit init_hwif_sl82c105(ide_hwif_t *hwif)
 {
@@ -396,11 +373,6 @@ static void __devinit init_hwif_sl82c105
hwif-drives[0].drive_data = hwif-drives[1].drive_data = 0x0909;
hwif-drives[0].autotune   = hwif-drives[1].autotune   = 1;
 
-   hwif-atapi_dma = 0;
-   hwif-mwdma_mask = 0;
-   hwif-swdma_mask = 0;
-   hwif-autodma = 0;
-
if 

Re: [RFT] libata hpa support

2007-02-23 Thread Alan
 A few questions are raised, libata SATA hasn't been resizing disks in the
 past, should we only disable HPA on PATA only, a la drivers/ide? In any

drivers/ide disable HPA on SATA ports it manages too.

 event, supporting HPA lets people who had partitions which extend beyond
 the protected area boot with libata PATA. Some policy decision needs to be
 made too... Right now, I'm just disabling it unilaterally.

Last time we had a Red Hat bitching session about it the main thing the
installer guys wanted was a way to find out what size the disk thought it
was before we twiddled with it, so that they can make an intelligent
attempt to handle partitioning in the case where for example a disk is
unpartitioned. A partitioned disk isn't so much of a problem as the
partitioning itself tells you the a lot about the state of disk and
intended HPA.

What do the Ubuntu installer/partitioning folk think ?

Alan
-
To unsubscribe from this list: send the line unsubscribe linux-ide in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[RFT] libata hpa support

2007-02-23 Thread Kyle McMartin
Rough first cut at Host Protected Area support for libata. Buyer beware, I
can't guarantee it won't club baby seals or kick your cat... In particular,
the non-LBA48 codepath hasn't been tested by me...

A few questions are raised, libata SATA hasn't been resizing disks in the
past, should we only disable HPA on PATA only, a la drivers/ide? In any
event, supporting HPA lets people who had partitions which extend beyond
the protected area boot with libata PATA. Some policy decision needs to be
made too... Right now, I'm just disabling it unilaterally.

I've added 0  to the patch where we would attempt to resize the disk to
mitigate the risk testing the reading portion of the code. I don't have a
laptop with a jumpered setting, so I've been testing by halving the disk size
as you can see in the #if 0 section of code. It seems to work and recover
alright. The SET_MAX_ADDRESS{,_EXT} is temporary, as when I reboot and look
at the disk I've been testing with, it's still full sized.

I'd appreciate a few more eyes on the code, and maybe even a tester
or two. (The box I'm beating on to test this has buggy ACPI, so I've not
had the opportunity to test resume, for instance.)

In particular, the taskfiles could use a perusing, I've noticed on my
box when the ICH8 controller is in AHCI mode, everything is fine, but when
driving it with ata_piix, the first read_native_max returns something that is
incorrect (but successive attempts work... very bizarre.)

Cheers,
Kyle M.

Signed-off-by: Kyle McMartin [EMAIL PROTECTED]

---
diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index 2cf8251..c3da3ea 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -804,6 +804,182 @@ void ata_id_c_string(const u16 *id, unsigned char *s,
*p = '\0';
 }
 
+static u64 ata_tf_to_lba48(struct ata_taskfile *tf)
+{
+   u64 sectors = 0;
+
+   sectors |= ((u64)(tf-hob_lbah  0xff))  40;
+   sectors |= ((u64)(tf-hob_lbam  0xff))  32;
+   sectors |= (tf-hob_lbal  0xff)  24;
+   sectors |= (tf-lbah  0xff)  16;
+   sectors |= (tf-lbam  0xff)  8;
+   sectors |= (tf-lbal  0xff);
+
+   return ++sectors;
+}
+
+static u64 ata_tf_to_lba(struct ata_taskfile *tf)
+{
+   u64 sectors = 0;
+
+   sectors |= (tf-device  0xff)  24;
+   sectors |= (tf-lbah  0xff)  16;
+   sectors |= (tf-lbam  0xff)  8;
+   sectors |= (tf-lbal  0xff);
+
+   return ++sectors;
+}
+
+static u64 ata_read_native_max_address_ext(struct ata_device *dev)
+{
+   unsigned int err;
+   struct ata_taskfile tf;
+
+   ata_tf_init(dev, tf);
+
+   tf.command = ATA_CMD_READ_NATIVE_MAX_EXT;
+   tf.flags |= ATA_TFLAG_DEVICE | ATA_TFLAG_LBA48 | ATA_TFLAG_ISADDR;
+   tf.protocol |= ATA_PROT_NODATA;
+   tf.device = 0x40;
+
+   err = ata_exec_internal(dev, tf, NULL, DMA_NONE, NULL, 0);
+   if (err)
+   return 0;
+
+   return ata_tf_to_lba48(tf);
+}
+
+static u64 ata_read_native_max_address(struct ata_device *dev)
+{
+   unsigned int err;
+   struct ata_taskfile tf;
+
+   ata_tf_init(dev, tf);
+
+   tf.command = ATA_CMD_READ_NATIVE_MAX;
+   tf.flags |= ATA_TFLAG_DEVICE | ATA_TFLAG_ISADDR;
+   tf.protocol |= ATA_PROT_NODATA;
+   tf.device = 0x40;
+
+   err = ata_exec_internal(dev, tf, NULL, DMA_NONE, NULL, 0);
+   if (err)
+   return 0;
+
+   return ata_tf_to_lba(tf);
+}
+
+static u64 ata_set_native_max_address_ext(struct ata_device *dev, u64 
new_sectors)
+{
+   unsigned int err;
+   struct ata_taskfile tf;
+
+   new_sectors--;
+
+   ata_tf_init(dev, tf);
+
+   tf.command = ATA_CMD_SET_MAX_EXT;
+   tf.flags |= ATA_TFLAG_DEVICE | ATA_TFLAG_LBA48 | ATA_TFLAG_ISADDR;
+   tf.protocol |= ATA_PROT_NODATA;
+   tf.device = 0x40;
+
+   tf.lbal = (new_sectors  0)  0xff;
+   tf.lbam = (new_sectors  8)  0xff;
+   tf.lbah = (new_sectors  16)  0xff;
+
+   tf.hob_lbal = (new_sectors  24)  0xff;
+   tf.hob_lbam = (new_sectors  32)  0xff;
+   tf.hob_lbah = (new_sectors  40)  0xff;
+
+   err = ata_exec_internal(dev, tf, NULL, DMA_NONE, NULL, 0);
+   if (err)
+   return 0;
+
+   return ata_tf_to_lba48(tf);
+}
+
+static u64 ata_set_native_max_address(struct ata_device *dev, u64 new_sectors)
+{
+   unsigned int err;
+   struct ata_taskfile tf;
+
+   new_sectors--;
+
+   ata_tf_init(dev, tf);
+
+   tf.command = ATA_CMD_SET_MAX;
+   tf.flags |= ATA_TFLAG_DEVICE | ATA_TFLAG_ISADDR;
+   tf.protocol |= ATA_PROT_NODATA;
+
+   tf.lbal = (new_sectors  0)  0xff;
+   tf.lbam = (new_sectors  8)  0xff;
+   tf.lbah = (new_sectors  16)  0xff;
+   tf.device = ((new_sectors  24)  0xff) | 0x40;
+
+   err = ata_exec_internal(dev, tf, NULL, DMA_NONE, NULL, 0);
+   if (err)
+   return 0;
+
+   return ata_tf_to_lba(tf);
+}
+
+static u64 ata_hpa_resize(struct ata_device *dev)
+{
+   u64 

Re: [RFT] libata hpa support

2007-02-23 Thread Colin Watson
Alan wrote:
  A few questions are raised, libata SATA hasn't been resizing disks in the
  past, should we only disable HPA on PATA only, a la drivers/ide? In any
 
 drivers/ide disable HPA on SATA ports it manages too.
 
  event, supporting HPA lets people who had partitions which extend beyond
  the protected area boot with libata PATA. Some policy decision needs to be
  made too... Right now, I'm just disabling it unilaterally.
 
 Last time we had a Red Hat bitching session about it the main thing the
 installer guys wanted was a way to find out what size the disk thought it
 was before we twiddled with it, so that they can make an intelligent
 attempt to handle partitioning in the case where for example a disk is
 unpartitioned. A partitioned disk isn't so much of a problem as the
 partitioning itself tells you the a lot about the state of disk and
 intended HPA.
 
 What do the Ubuntu installer/partitioning folk think ?

Suspiciously similar to the Red Hat installer folks. :-) Our partitioner
(and Debian's) asks libparted to create a partition table in memory
(ped_disk_new_fresh) and then works entirely based on what libparted
reports about that partition table before actually committing it. This
obviously only works if you can find out the size of the disk before
writing anything to it.

libparted uses ioctl(BLKGETSIZE[64]) to get the size of the disk at the
moment. Making that report the non-HPA size would be enough to make
libparted create new partition tables properly, but it would defeat the
purpose of Kyle's patch in that you probably couldn't use libparted to
work with existing partition tables extending beyond the HPA. Adding say
ioctl(BLKGETNONHPASIZE) or something and teaching libparted to create
new partition tables excluding the HPA but still be able to read
existing partition tables extending into the HPA seems like it'd be the
best approach, and sounds pretty straightforward. I'd be happy to do the
libparted side of that.

-- 
Colin Watson   [EMAIL PROTECTED]
-
To unsubscribe from this list: send the line unsubscribe linux-ide in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] (pata-2.6 fix queue) cmd64x: use interrupt status from MRDMODE register

2007-02-23 Thread Bartlomiej Zolnierkiewicz

On Friday 16 February 2007, Sergei Shtylyov wrote:
 Fold the parts of the ide_dma_end() methods indetical to __ide_dma_end() into 
 a
 mere call to it.
 Start using faster versions of the ide_dma_end() and ide_dma_test_irq() 
 methods
 for the PCI0646U and newer chips that have the duplicate interrupt status bits
 in the I/O mapped MRDMODE register, determing what methods to use at the 
 driver
 load time.  Do some cleanup/renaming in the old ide_dma_test_irq() method.
 
 While at it, fix minor issues with PCI0646 chipset reporting:
 
 - IRQ workaround enabled printed out not only for revision 0x01;
 
 - CMD646: chipset revision printed twice (by IDE core and the driver 
 itself);
 
 - empty/pointless switch cases for the chips other than PCI0646.
 
 Signed-off-by: Sergei Shtylyov [EMAIL PROTECTED]

applied
-
To unsubscribe from this list: send the line unsubscribe linux-ide in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] (pata-2.6 fix queue) cmd64x: add/fix enablebits

2007-02-23 Thread Bartlomiej Zolnierkiewicz

On Tuesday 20 February 2007, Sergei Shtylyov wrote:
 Hello.
 
 Bartlomiej Zolnierkiewicz wrote:
 
 The IDE core looks at the wrong bit when checking if the secondary channel 
 is
 enabled on PCI0646 -- CFR bit 8 is read-ahead disable, bit 3 is the correct 
 one.
 
  I guess that you meant CNTRL here?
 
 Yeah, and bit 7. :-

fixed, thanks
-
To unsubscribe from this list: send the line unsubscribe linux-ide in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 2.6.21-rc1 1/2] sata_vsc: factor the error and normal intr paths into separate routines

2007-02-23 Thread Jeff Garzik

Tejun Heo wrote:

Sorry to keep nagging about patch separation and your way could be okay
too, but IMHO this can be done better in one of the following ways.

1. Keep two patches.  One to break down the interrupt handler the other
to improve it.  In this case the first one shouldn't introduce major new
features but just focus on breaking down existing one.

2. Do it in single patch.  It seems the changes are localized enough.
Just name it something like 'reimplement irq handler' and list changes
in the commit message.

After seeing the change, I'm leaning toward #2.  Thanks.  :-)



These two sata_vsc changes should be done in a single patch.  add new 
stuff and use new stuff patches can be combined.  The types of 
patches that normally should be separated are logical changes like 
clean up irq handling or improve EH, changes that can be 
git-bisect'd usefully.


The changes look OK to me.

Jeff


-
To unsubscribe from this list: send the line unsubscribe linux-ide in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 2.6.21-rc1] sata_vsc: refactor vsc_sata_interrupt and hook up error handling

2007-02-23 Thread Dan Williams
Separate sata_vsc interrupt handling into a normal (per-port) path and an
error path with the addition of vsc_port_intr and vsc_error_intr
respectively.  The error path handles interrupt based
hotplug events which requires the definition of vsc_freeze and vsc_thaw.

Note: vsc_port_intr has a workaround for unexpected interrupts that occur
during polled commands.  This fixes a regression between 2.6.19 and 2.6.20.

Changes in take2:
* removed definition of invalid fis bit
* let standard ata-error-handling handle the serror register
* clear all unhandled interrupts
* revert changes to vsc_intr_mask_update (vsc_thaw enables all interrupts)
* use unlikely() for the pci-abort and not-our-interrupt cases in 
vsc_sata_interrupt

Changes in take3:
* Unify the add + hook-up patches into this single patch

[EMAIL PROTECTED]: clean up comments and suggestions]
Cc: Jeremy Higdon [EMAIL PROTECTED]
Signed-off-by: Dan Williams [EMAIL PROTECTED]
---

 drivers/ata/sata_vsc.c |  123 +---
 1 files changed, 74 insertions(+), 49 deletions(-)

diff --git a/drivers/ata/sata_vsc.c b/drivers/ata/sata_vsc.c
index 2fd037b..f0d86cb 100644
--- a/drivers/ata/sata_vsc.c
+++ b/drivers/ata/sata_vsc.c
@@ -98,10 +98,6 @@ enum {
  VSC_SATA_INT_PHY_CHANGE),
 };
 
-#define is_vsc_sata_int_err(port_idx, int_status) \
-(int_status  (VSC_SATA_INT_ERROR  (8 * port_idx)))
-
-
 static u32 vsc_sata_scr_read (struct ata_port *ap, unsigned int sc_reg)
 {
if (sc_reg  SCR_CONTROL)
@@ -119,6 +115,28 @@ static void vsc_sata_scr_write (struct ata_port *ap, 
unsigned int sc_reg,
 }
 
 
+static void vsc_freeze(struct ata_port *ap)
+{
+   void __iomem *mask_addr;
+
+   mask_addr = ap-host-iomap[VSC_MMIO_BAR] +
+   VSC_SATA_INT_MASK_OFFSET + ap-port_no;
+
+   writeb(0, mask_addr);
+}
+
+
+static void vsc_thaw(struct ata_port *ap)
+{
+   void __iomem *mask_addr;
+
+   mask_addr = ap-host-iomap[VSC_MMIO_BAR] +
+   VSC_SATA_INT_MASK_OFFSET + ap-port_no;
+
+   writeb(0xff, mask_addr);
+}
+
+
 static void vsc_intr_mask_update(struct ata_port *ap, u8 ctl)
 {
void __iomem *mask_addr;
@@ -203,6 +221,36 @@ static void vsc_sata_tf_read(struct ata_port *ap, struct 
ata_taskfile *tf)
 }
 }
 
+static inline void vsc_error_intr(u8 port_status, struct ata_port *ap)
+{
+   if (port_status  (VSC_SATA_INT_PHY_CHANGE | VSC_SATA_INT_ERROR_M))
+   ata_port_freeze(ap);
+   else
+   ata_port_abort(ap);
+}
+
+static void vsc_port_intr(u8 port_status, struct ata_port *ap)
+{
+   struct ata_queued_cmd *qc;
+   int handled = 0;
+
+   if (unlikely(port_status  VSC_SATA_INT_ERROR)) {
+   vsc_error_intr(port_status, ap);
+   return;
+   }
+
+   qc = ata_qc_from_tag(ap, ap-active_tag);
+   if (qc  likely(!(qc-tf.flags  ATA_TFLAG_POLLING)))
+   handled = ata_host_intr(ap, qc);
+
+   /* We received an interrupt during a polled command,
+* or some other spurious condition.  Interrupt reporting
+* with this hardware is fairly reliable so it is safe to
+* simply clear the interrupt
+*/
+   if (unlikely(!handled))
+   ata_chk_status(ap);
+}
 
 /*
  * vsc_sata_interrupt
@@ -214,59 +262,36 @@ static irqreturn_t vsc_sata_interrupt (int irq, void 
*dev_instance)
struct ata_host *host = dev_instance;
unsigned int i;
unsigned int handled = 0;
-   u32 int_status;
-
-   spin_lock(host-lock);
+   u32 status;
 
-   int_status = readl(host-iomap[VSC_MMIO_BAR] +
-  VSC_SATA_INT_STAT_OFFSET);
+   status = readl(host-iomap[VSC_MMIO_BAR] + VSC_SATA_INT_STAT_OFFSET);
 
-   for (i = 0; i  host-n_ports; i++) {
-   if (int_status  ((u32) 0xFF  (8 * i))) {
-   struct ata_port *ap;
+   if (unlikely(status == 0x || status == 0)) {
+   if (status)
+   dev_printk(KERN_ERR, host-dev,
+   : IRQ status == 0x, 
+   PCI fault or device removal?\n);
+   goto out;
+   }
 
-   ap = host-ports[i];
+   spin_lock(host-lock);
 
-   if (is_vsc_sata_int_err(i, int_status)) {
-   u32 err_status;
-   printk(KERN_DEBUG %s: ignoring 
interrupt(s)\n, __FUNCTION__);
-   err_status = ap ? vsc_sata_scr_read(ap, 
SCR_ERROR) : 0;
-   vsc_sata_scr_write(ap, SCR_ERROR, err_status);
-   handled++;
-   }
+   for (i = 0; i  host-n_ports; i++) {
+   u8 port_status = (status  (8 * i))  0xff;
+   if (port_status) {
+   struct ata_port *ap = host-ports[i];
 
if (ap  

Re: [PATCH] (pata-2.6 fix queue) sl82c105: DMA support code cleanup (take 3)

2007-02-23 Thread Bartlomiej Zolnierkiewicz

On Friday 23 February 2007, Sergei Shtylyov wrote:
 Fold the now equivalent code in the ide_dma_check() method into a mere call to
 ide_use_dma().  Make config_for_dma() return non-zero if DMA mode has been set
 and call it from the ide_dma_check() method instead of ide_dma_on().
 Also, defer writing the DMA timings to the chip registers until DMA is 
 actually
 turned on (and do not enable IORDY for DMA).  Remove unnecessary code from the
 init_hwif() method, improve its looks overall, and also rename the driver's
 ide_dma_check() and dma_start() methods.
 
 ---
 This version takes into account that the ide-use-fast-pio.patch had been 
 merged
 to mainline...

applied
-
To unsubscribe from this list: send the line unsubscribe linux-ide in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Patch: slight shrink of vt6421_init_addrs()

2007-02-23 Thread Adam J. Richter
On Fri, Feb 23, 2007 at 05:30:03AM -0500, Jeff Garzik wrote:
 Adam J. Richter wrote:
  Hi Tejun and Jeff,
  
  The discussion about the vt6421 problems caused me to glance
  at the code and try a very minor clean-up to vt6421_init_addrs().
  Please note that I don't have a vt6421, so this patch is UNTESTED.
  The patch does two things:
  
  1. Elminate the bmdma_addr variable, which was only used once,
 in an assignment to a field also named bmdma_addr.  So having
 a separate variable served no documentary purpose.  This change
 causes the value to be computed a little later, so please make
 sure that that is OK.  The effect is just to make the source
 code one line smaller.  The binary size is unchanged by this
 modification, at least on my x86 configuration, which has
 SMP and lots of debugging options activated.
  
  2. Add a variable ata_ports, to replace the six times the
 same value appeared to be computed.  I assume the compiler
 was smart enough to avoid most of the recomputation, but
 this change shrinks the .text by 7 bytes in my configuration,
 makes the routine more readable and reduces opportunities for
 typos.
  
  If it looks OK to everyone, please forward it upstream as
  appropriate.
  
  Adam Richter
 
 Patch seems OK in theory, except for two procedural stumbling blocks:
 
 1) always always always include a signed-off-by line in your kernel 
 patches.  see http://linux.yyz.us/patch-format.html or 
 Documentation/SubmittingPatches in the kernel tree.

OK.

 2) this was stirred a bit more by recent changes, so a rediff + resend 
 would be appreciated

The original patch applies perfectly cleanly to 2.6.21-rc1-git1, and
compiles without complaint, but here is a rediff specifically against
2.6.21-rc1-git1, using diff -up and in a/... vs. b/... format,
just to be more conformant to the documents you mentioned.  If there
is some other tree you wanted me to diff against, just let me know.

By the way, your remark that there have been recent changes to
sata_via.c is at least a hopeful sign, as I was about to look into
why why the vt6420 fix did not get into Linus's release candidate
2.6.21-rc1 or the latest git snapshot, 2.6.21-rc1-git1.  If there is
still a problem vt6421, I patch that only fixes vt6420 I think would
still be a big improvement.  Anyhow, if you're using a different
sata_via.c, I'm guessing you're about to submit a fix along those
lines.

Anyhow, here is my attempt at closer conformance to the two sets of
instructions you mentioned.  For the benefit of anyone tracking this
thread by subject, I have not changed the subject line to the format
specified in http://linux.yyz.us/patch-format.html, but I expect
to use that format in the future.





Developer's Certificate of Origin 1.1

By making a contribution to this project, I certify that:

(a) The contribution was created in whole or in part by me and I
have the right to submit it under the open source license
indicated in the file; or

(b) The contribution is based upon previous work that, to the best
of my knowledge, is covered under an appropriate open source
license and I have the right under that license to submit that
work with modifications, whether created in whole or in part
by me, under the same open source license (unless I am
permitted to submit under a different license), as indicated
in the file; or

(c) The contribution was provided directly to me by some other
person who certified (a), (b) or (c) and I have not modified
it.

(d) I understand and agree that this project and the contribution
are public and that a record of the contribution (including all
personal information I submit with it, including my sign-off) is
maintained indefinitely and may be redistributed consistent with
this project or the open source license(s) involved.

---

Signed-off-by: Adam J. Richter [EMAIL PROTECTED]


--- a/drivers/ata/sata_via.c2007-02-21 18:22:08.0 +0800
+++ b/drivers/ata/sata_via.c2007-02-24 07:44:20.0 +0800
@@ -407,16 +407,16 @@ static void vt6421_init_addrs(struct ata
  void __iomem * const *iomap, unsigned int port)
 {
void __iomem *reg_addr = iomap[port];
-   void __iomem *bmdma_addr = iomap[4] + (port * 8);
+   struct ata_ioports *ata_ports = probe_ent-port[port];
 
-   probe_ent-port[port].cmd_addr = reg_addr;
-   probe_ent-port[port].altstatus_addr =
-   probe_ent-port[port].ctl_addr = (void __iomem *)
+   ata_ports-cmd_addr = reg_addr;
+   ata_ports-altstatus_addr =
+   ata_ports-ctl_addr = (void __iomem *)
((unsigned long)(reg_addr + 8) | ATA_PCI_CTL_OFS);
-   

Re: PMP patches for more recent kernel?

2007-02-23 Thread Robin H. Johnson
On Sat, Feb 24, 2007 at 02:19:45AM +0100, Pierre Ossman wrote:
 I'm eager to test out your PMP patches as I'm currently sitting on a SATA disk
 rack that requires a PMP to be used. Your current set is from october though 
 and
 seems to require some work to get to apply on the current kernel. Do you have
 something newer I can play with?
I'm going to answer this question, because I'm getting 3-4 emails a week
now since I did the previous port of the PMP patches to 2.6.20rc2.

This was my previous instructional email.
http://www.mail-archive.com/linux-ide@vger.kernel.org/msg02533.html

That set of patches is intended for applying against the Linus GIT tree
as of approximately 2.6.20rc2, combined with the libata#upstream tree of
the same vintage.

They do NOT apply to anything newer, and definite work is needed in
porting them. I don't have time for such porting at present, so unless
somebody else ports them, I don't see them moving very fast at all.

Things would probably go much smooth if the hp_poll and ata_link stuff
got into jgarzik's tree, but I don't know when he will accept them.

-- 
Robin Hugh Johnson
Gentoo Linux Developer
E-Mail : [EMAIL PROTECTED]
GnuPG FP   : 11AC BA4F 4778 E3F6 E4ED  F38E B27B 944E 3488 4E85


pgpvjtPto15gL.pgp
Description: PGP signature


Re: [PATCH] sata_nv: kill old private BMDMA helper functions

2007-02-23 Thread Jeff Garzik

Robert Hancock wrote:

sata_nv implemented its own copies of the BMDMA helper functions for ADMA,
since the ADMA BMDMA status registers are PIO while the other registers
are MMIO, and this was the only way to handle this previously. Now that
we have iomap support, the standard routines should just work, so use them.
The only thing we need to override as far as ADMA and BMDMA is the
post_internal_cmd callback, where we should only call ata_post_internal_cmd
if we are in port-register mode.

Signed-off-by: Robert Hancock [EMAIL PROTECTED]


applied


-
To unsubscribe from this list: send the line unsubscribe linux-ide in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] sata_nv: complain on spurious completion notifiers

2007-02-23 Thread Jeff Garzik

Robert Hancock wrote:

Recently Tejun wrote a patch to ahci.c to make it raise a HSM violation
if the drive attempted to complete a tag that wasn't outstanding. We could
run into the same problem with sata_nv ADMA. This adds code to raise a HSM
violation error if the controller gives us a notifier tag that isn't
outstanding, since the drive may be issuing spurious completions.

Signed-off-by: Robert Hancock [EMAIL PROTECTED]


applied


-
To unsubscribe from this list: send the line unsubscribe linux-ide in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH]: pcmcia - spot slave decode flaws (for testing)

2007-02-23 Thread Jeff Garzik

Alan wrote:

If you've got a CF adapter or PCMCIA disc which shows up twice in libata
pata_pcmcia can you try this patch on top of the updates posted. It tries
to spot when the slave is a mirror of the master and to fix up problems
that causes.

Signed-off-by: Alan Cox [EMAIL PROTECTED]

diff -u --new-file --recursive --exclude-from /usr/src/exclude 
linux.vanilla-2.6.20-mm2/drivers/ata/pata_pcmcia.c 
linux-2.6.20-mm2/drivers/ata/pata_pcmcia.c
--- linux.vanilla-2.6.20-mm2/drivers/ata/pata_pcmcia.c  2007-02-20 
13:37:58.0 +
+++ linux-2.6.20-mm2/drivers/ata/pata_pcmcia.c  2007-02-20 14:28:13.0 
+
@@ -54,6 +54,39 @@
dev_node_t  node;
 };
 
+/**

+ * pcmcia_set_mode -   PCMCIA specific mode setup
+ * @ap: Port
+ * @r_failed_dev: Return pointer for failed device
+ *
+ * Perform the tuning and setup of the devices and timings, which
+ * for PCMCIA is the same as any other controller. We wrap it however
+ * as we need to spot hardware with incorrect or missing master/slave
+ * decode, which alas is embarrassingly common in the PC world
+ */
+ 
+static int pcmcia_set_mode(struct ata_port *ap, struct ata_device **r_failed_dev)

+{
+   struct ata_device *master = ap-device[0];
+   struct ata_device *slave = ap-device[1];
+   
+   if (!ata_dev_enabled(master) || !ata_dev_enabled(slave))
+   return ata_do_set_mode(ap, r_failed_dev);
+   
+   if (memcmp(master-id + ATA_ID_FW_REV,  slave-id + ATA_ID_FW_REV,
+  ATA_ID_FW_REV_LEN + ATA_ID_PROD_LEN) == 0)
+   {
+   /* Suspicious match, but could be two cards from
+  the same vendor - check serial */
+   if (memcmp(master-id + ATA_ID_SERNO, slave-id + ATA_ID_SERNO,
+  ATA_ID_SERNO_LEN) == 0  master-id[ATA_ID_SERNO] 
 8) {
+   ata_dev_printk(slave, KERN_WARNING, is a ghost device, 
ignoring.\n);
+   ata_dev_disable(slave);
+		}	 
+	}

+   return ata_do_set_mode(ap, r_failed_dev);


Code looks OK. Not applied due to for testing note.

General comment:  it might be nice to do this in the core, just as a 
sanity check for a variety of problems, past, present and future.



-
To unsubscribe from this list: send the line unsubscribe linux-ide in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH]: pcmcia - spot slave decode flaws (for testing)

2007-02-23 Thread Alan
 Code looks OK. Not applied due to for testing note.
 
 General comment:  it might be nice to do this in the core, just as a 
 sanity check for a variety of problems, past, present and future.

We tried that with old IDE and all hell broke loose. Lots of virtual disk
stuff and raid volumes have non-unique serial numbers. We even found a
case of identically serial numbered Maxtor drives.

It needs to stay in pcmcia, which is the only place we've seen the
duplication.



Alan
-
To unsubscribe from this list: send the line unsubscribe linux-ide in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] libata: add NCQ blacklist entries from Silicon Image Windows driver

2007-02-23 Thread Jeff Garzik

Robert Hancock wrote:

This patch adds in some NCQ blacklist entries taken from the Silicon
Image Windows drivers' .inf files for the 3124 and 3132 controllers.
These entries were marked as DisableSataQueueing. Assume these are
in their blacklist for a reason and disable NCQ on these drives.

Signed-off-by: Robert Hancock [EMAIL PROTECTED]

--- linux-2.6.21-rc1edit/drivers/ata/libata-core.c.prev2007-02-21 
22:23:05.0 -0600
+++ linux-2.6.21-rc1edit/drivers/ata/libata-core.c2007-02-21 
22:25:44.0 -0600

@@ -3269,6 +3269,13 @@ static const struct ata_blacklist_entry
/* Devices with NCQ limits */

+/* The following blacklist entries are taken from the Windows
+   driver .inf files for the Silicon Image 3124 and 3132. */
+{ Maxtor 7B250S0,BANC1B70,ATA_HORKAGE_NONCQ, },
+{ HTS541060G9SA00,MB3OC60D,ATA_HORKAGE_NONCQ, },
+{ HTS541080G9SA00,MB4OC60D,ATA_HORKAGE_NONCQ, },
+{ HTS541010G9SA00,MBZOC60D,ATA_HORKAGE_NONCQ, },



The general consensus of private emails seems to be that its ok to 
blacklist the Maxtor one, at least.


So let's go ahead and at least get that entry in.

Jeff


-
To unsubscribe from this list: send the line unsubscribe linux-ide in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 2.6.21-rc1] sata_vsc: refactor vsc_sata_interrupt and hook up error handling

2007-02-23 Thread Tejun Heo
Dan Williams wrote:
 Separate sata_vsc interrupt handling into a normal (per-port) path and an
 error path with the addition of vsc_port_intr and vsc_error_intr
 respectively.  The error path handles interrupt based
 hotplug events which requires the definition of vsc_freeze and vsc_thaw.
 
 Note: vsc_port_intr has a workaround for unexpected interrupts that occur
 during polled commands.  This fixes a regression between 2.6.19 and 2.6.20.
 
 Changes in take2:
 * removed definition of invalid fis bit
 * let standard ata-error-handling handle the serror register
 * clear all unhandled interrupts
 * revert changes to vsc_intr_mask_update (vsc_thaw enables all interrupts)
 * use unlikely() for the pci-abort and not-our-interrupt cases in 
 vsc_sata_interrupt
 
 Changes in take3:
 * Unify the add + hook-up patches into this single patch
 
 [EMAIL PROTECTED]: clean up comments and suggestions]
 Cc: Jeremy Higdon [EMAIL PROTECTED]
 Signed-off-by: Dan Williams [EMAIL PROTECTED]

Acked-by: Tejun Heo [EMAIL PROTECTED]

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


Re: Disk stuck in error recovery loop with AHCI

2007-02-23 Thread Tejun Heo
Jim Paris wrote:
 Still, it seems that some improvements could be made to the EH when
 this sort of thing happens.  For example, after speed down requested
 but no transfer mode left a few times in a row, maybe it would make
 sense to just fail the disk and give up.  That would have allowed
 higher layers like MD to recover.

That's SD retrying sector-by-sector a large request as libata doesn't
report proper failed sector info.  Please try 2.6.20.  md will fail the
drive after a few errors.

Patches to improve EH behavior further went into 2.6.21 devel tree and
some are pending.

Thanks.

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


Re: [PATCH 2.6.20] libata: Support HDIO_DRIVE_TASKFILE ioctl

2007-02-23 Thread Tejun Heo
Hello,

Anand Kulkarni wrote:
 Hi,
 
 Attached is a patch to allow HDIO_DRIVE_TASKFILE ioctl calls to libata
 devices so that arbitrary ATA commands that need PIO data transfers in
 either direction can be issued. I have tested it with linux kernel
 2.6.20. The format of the input arguments is the same as for the
 regular ATA HDIO_DRIVE_TASKFILE ioctl : Code that makes ioctl calls to
 ATA devices on /dev/hda can be sent as is to SATA devices on /dev/sda.
 
 Since this is the first time I am sending a patch, please let me know
 if I should do anything differently.

I was kind of hoping we could drop HDIO_DRIVE_TASKFILE.  Please take a
look at the HDIO_DRIVE_TASKFILE section of Documentation/ioctl/hdio.txt.
 That's one scary scary ioctl and we still haven't sorted out which are
features and which are bugs.

For backward compatibility HDIO_DRIVE_CMD and HDIO_DRIVE_TASK are
implemented and most applications seem to be happy with it.  If you need
real fine control over command execution, the recommended way is to use
SAT via SG_IO.

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


[PATCH] sata_mv: fix pci_enable_msi() error handling

2007-02-23 Thread Tejun Heo
intx should be turned on when pci_enable_msi() fails not when it
succeeds.  Fix it.

Signed-off-by: Tejun Heo [EMAIL PROTECTED]

diff --git a/drivers/ata/sata_mv.c b/drivers/ata/sata_mv.c
index 06867b9..d724bc7 100644
--- a/drivers/ata/sata_mv.c
+++ b/drivers/ata/sata_mv.c
@@ -2347,7 +2347,7 @@ static int mv_init_one(struct pci_dev *pdev, const struct 
pci_device_id *ent)
return rc;
 
/* Enable interrupts */
-   if (msi  !pci_enable_msi(pdev))
+   if (msi  pci_enable_msi(pdev))
pci_intx(pdev, 1);
 
mv_dump_pci_cfg(pdev, 0x68);
-
To unsubscribe from this list: send the line unsubscribe linux-ide in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


end to end error recovery musings

2007-02-23 Thread Ric Wheeler
In the IO/FS workshop, one idea we kicked around is the need to provide 
better and more specific error messages between the IO stack and the 
file system layer.


My group has been working to stabilize a relatively up to date libata + 
MD based box, so I can try to lay out at least one appliance like 
typical configuration to help frame the issue. We are working on a 
relatively large appliance, but you can buy similar home appliance (or 
build them) that use linux to provide a NAS in a box for end users.


The use case that we have is on an ICH6R/AHCI box with 4 large (500+ GB) 
drives, with some of the small system partitions on a 4-way RAID1 
device. The libata version we have is back port of 2.6.18 onto SLES10, 
so the error handling at the libata level is a huge improvement over 
what we had before.


Each box has a watchdog timer that can be set to fire after at most 2 
minutes.


(We have a second flavor of this box with an ICH5 and P-ATA drives using 
the non-libata drivers that has a similar use case).


Using the patches that Mark sent around recently for error injection, we 
inject media errors into one or more drives and try to see how smoothly 
error handling runs and, importantly, whether or not the error handling 
will complete before the watchdog fires and reboots the box.  If you 
want to be especially mean, inject errors into the RAID superblocks on 3 
out of the 4 drives.


We still have the following challenges:

   (1) read-ahead often means that we will  retry every bad sector at 
least twice from the file system level. The first time, the fs read 
ahead request triggers a speculative read that includes the bad sector 
(triggering the error handling mechanisms) right before the real 
application triggers a read does the same thing.  Not sure what the 
answer is here since read-ahead is obviously a huge win in the normal case.


   (2) the patches that were floating around on how to make sure that 
we effectively handle single sector errors in a large IO request are 
critical. On one hand, we want to combine adjacent IO requests into 
larger IO's whenever possible. On the other hand, when the combined IO 
fails, we need to isolate the error to the correct range, avoid 
reissuing a request that touches that sector again and communicate up 
the stack to file system/MD what really failed.  All of this needs to 
complete in tens of seconds, not multiple minutes.


   (3) The timeout values on the failed IO's need to be tuned well (as 
was discussed in an earlier linux-ide thread). We cannot afford to hang 
for 30 seconds, especially in the MD case, since you might need to fail 
more than one device for a single IO.  Prompt error prorogation (say 
that 4 times quickly!) can allow MD to mask the underlying errors as you 
would hope, hanging on too long will almost certainly cause a watchdog 
reboot...


   (4) The newish libata+SCSI stack is pretty good at handling disk 
errors, but adding in MD actually can reduce the reliability of your 
system unless you tune the error handling correctly.


We will follow up with specific issues as they arise, but I wanted to 
lay out a use case that can help frame part of the discussion.  I also 
want to encourage people to inject real disk errors with the Mark 
patches so we can share the pain ;-)


ric



-
To unsubscribe from this list: send the line unsubscribe linux-ide in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: end to end error recovery musings

2007-02-23 Thread H. Peter Anvin

Ric Wheeler wrote:


We still have the following challenges:

   (1) read-ahead often means that we will  retry every bad sector at 
least twice from the file system level. The first time, the fs read 
ahead request triggers a speculative read that includes the bad sector 
(triggering the error handling mechanisms) right before the real 
application triggers a read does the same thing.  Not sure what the 
answer is here since read-ahead is obviously a huge win in the normal case.




Probably the only sane thing to do is to remember the bad sectors and 
avoid attempting reading them; that would mean marking automatic 
versus explicitly requested requests to determine whether or not to 
filter them against a list of discovered bad blocks.


-hpa
-
To unsubscribe from this list: send the line unsubscribe linux-ide in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: end to end error recovery musings

2007-02-23 Thread Andreas Dilger
On Feb 23, 2007  16:03 -0800, H. Peter Anvin wrote:
 Ric Wheeler wrote:
(1) read-ahead often means that we will  retry every bad sector at 
 least twice from the file system level. The first time, the fs read 
 ahead request triggers a speculative read that includes the bad sector 
 (triggering the error handling mechanisms) right before the real 
 application triggers a read does the same thing.  Not sure what the 
 answer is here since read-ahead is obviously a huge win in the normal case.
 
 Probably the only sane thing to do is to remember the bad sectors and 
 avoid attempting reading them; that would mean marking automatic 
 versus explicitly requested requests to determine whether or not to 
 filter them against a list of discovered bad blocks.

And clearing this list when the sector is overwritten, as it will almost
certainly be relocated at the disk level.  For that matter, a huge win
would be to have the MD RAID layer rewrite only the bad sector (in hopes
of the disk relocating it) instead of failing the whiole disk.  Otherwise,
a few read errors on different disks in a RAID set can take the whole
system offline.  Apologies if this is already done in recent kernels...

Cheers, Andreas
--
Andreas Dilger
Principal Software Engineer
Cluster File Systems, Inc.

-
To unsubscribe from this list: send the line unsubscribe linux-ide in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: end to end error recovery musings

2007-02-23 Thread H. Peter Anvin

Andreas Dilger wrote:

And clearing this list when the sector is overwritten, as it will almost
certainly be relocated at the disk level.


Certainly if the overwrite is successful.

-hpa
-
To unsubscribe from this list: send the line unsubscribe linux-ide in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html