Gentoo Websites Logo
Go to: Gentoo Home Documentation Forums Lists Bugs Planet Store Wiki Get Gentoo!
Bug 39186 - silicon image sata driver siimage.c bug in gentoo-dev-sources 2.6.1-r1 and earlier
Summary: silicon image sata driver siimage.c bug in gentoo-dev-sources 2.6.1-r1 and ea...
Status: RESOLVED WONTFIX
Alias: None
Product: Gentoo Linux
Classification: Unclassified
Component: [OLD] Core system (show other bugs)
Hardware: AMD64 Linux
: High normal (vote)
Assignee: John Mylchreest (RETIRED)
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2004-01-23 12:25 UTC by B. Allyn Fay
Modified: 2004-01-28 09:57 UTC (History)
1 user (show)

See Also:
Package list:
Runtime testing required: ---


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description B. Allyn Fay 2004-01-23 12:25:57 UTC
The Silicon Image driver siimage.c in gentoo-dev-sources 2.6.1-r1 isn't careful enough about applying it's "pessimistic Seagate errata fix". That is, this older version of the driver sets:
hwif->rqsize = 15

When really it should only do so if it has a broken Seagate device attached. 

Otherwise, it should set:
hwif->rqsize = 128

as noted in the patch below.

This leads to very poor performance for all sata devices connected to the bus, e.g. hdparm -t shows 20 MB/sec with old driver and 55 MB/sec with the new one.

Reproducible: Always
Steps to Reproduce:
1.
2.
3.




Here's a patch between gentoo-dev-sources 2.6.1-r1 and mm-sources 2.6.1-mm5:

--- linux-2.6.1-gentoo-r1/drivers/ide/pci/siimage.c     2004-01-23
14:44:52.000000000 -0500
+++ linux-2.6.1-mm5/drivers/ide/pci/siimage.c   2004-01-23 14:36:58.000000000 -0500
@@ -1,5 +1,5 @@
 /*
- * linux/drivers/ide/pci/siimage.c             Version 1.06    June 11, 2003
+ * linux/drivers/ide/pci/siimage.c             Version 1.07    Nov 30, 2003
  *
  * Copyright (C) 2001-2002     Andre Hedrick <andre@linux-ide.org>
  * Copyright (C) 2003          Red Hat <alan@redhat.com>
@@ -767,20 +767,17 @@
   
 static void proc_reports_siimage (struct pci_dev *dev, u8 clocking, const char
*name)
 {
-       if(pdev_is_sata(dev))
-               goto sata_skip;
-
-       printk(KERN_INFO "%s: BASE CLOCK ", name);
-       clocking &= 0x03;
-       switch(clocking) {
-               case 0x03: printk("DISABLED !\n"); break;
-               case 0x02: printk("== 2X PCI \n"); break;
-               case 0x01: printk("== 133 \n"); break;
-               case 0x00: printk("== 100 \n"); break;
+       if (!pdev_is_sata(dev)) {
+               printk(KERN_INFO "%s: BASE CLOCK ", name);
+               clocking &= 0x03;
+               switch (clocking) {
+                       case 0x03: printk("DISABLED!\n"); break;
+                       case 0x02: printk("== 2X PCI\n"); break;
+                       case 0x01: printk("== 133\n"); break;
+                       case 0x00: printk("== 100\n"); break;
+               }
        }
  
-sata_skip:
-
 #if defined(DISPLAY_SIIMAGE_TIMINGS) && defined(CONFIG_PROC_FS)
        siimage_devs[n_siimage_devs++] = dev;
  
@@ -1047,6 +1044,27 @@
        hwif->mmio                      = 2;
 }
  
+static int is_dev_seagate_sata(ide_drive_t *drive)
+{
+       const char *s = &drive->id->model[0];
+       unsigned len;
+
+       if (!drive->present)
+               return 0;
+
+       len = strnlen(s, sizeof(drive->id->model));
+
+       if ((len > 4) && (!memcmp(s, "ST", 2))) {
+               if ((!memcmp(s + len - 2, "AS", 2)) ||
+                   (!memcmp(s + len - 3, "ASL", 3))) {
+                       printk(KERN_INFO "%s: applying pessimistic Seagate "
+                                        "errata fix\n", drive->name);
+                       return 1;
+               }
+       }
+       return 0;
+}
+
 /**
  *     init_iops_siimage       -       set up iops
  *     @hwif: interface to set up
@@ -1068,7 +1086,7 @@
        hwif->hwif_data = 0;
  
        hwif->rqsize = 128;
-       if (is_sata(hwif))
+       if (is_sata(hwif) && is_dev_seagate_sata(&hwif->drives[0]))
                hwif->rqsize = 15;
  
        if (pci_get_drvdata(dev) == NULL)
Comment 1 Bob Johnson (RETIRED) gentoo-dev 2004-01-23 16:48:12 UTC
id check that patch out pretty good, i cant boot -mm on 
my siimage sata
vanilla is fine
Comment 2 B. Allyn Fay 2004-01-23 19:05:18 UTC
Here's the original patch... the relevant parts come from 2.6.1-mm2:

ftp://ftp.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.1/2.6.1-mm2/broken-out/ide-siimage-seagate.patch

I'd like to see the hdparm from that vanilla driver...
Comment 3 Brad House 2004-01-25 08:58:09 UTC
think this is fixed in gentoo-dev-sources-2.6.2_rc1
reopen if not.
Comment 4 B. Allyn Fay 2004-01-25 11:07:57 UTC
It's not in gentoo-dev-sources-2.6.2_rc1.
Comment 5 Brad House 2004-01-27 20:14:23 UTC
just realized this is a patch to the ide driver.
why are you not using the actual sata driver under SCSI lowlevel
drivers? Please use that one, we won't patch the ide/ata driver
as the newer libata sata drivers under scsi lowlevel are by far
the preferred. 
Please try gentoo-dev-sources-2.6.2_rc1 it is hardmasked, but it
works (_rc2 is out, so it probably will never be officially released).

-Brad
Comment 6 Bob Johnson (RETIRED) gentoo-dev 2004-01-27 20:19:50 UTC
actually libata sucks for some controllers, siimage being one of them.
But the ide patch is still broke
Comment 7 B. Allyn Fay 2004-01-28 09:57:44 UTC
According to Jeff Garzik's post on 1-15 to the linux-kernel mailing list regarding Silicon Image support in libata:

* Silicon Image support is still considered to be unstable. drivers/ide's siimage.c is preferred.

I don't see anything to contradict this in the changelog for 2.6.2-rc1 or 2.6.2-rc2, and this driver has been known to give hard lockups from heavy disk access.