Re: kern/38056: ahcisata fails to attach with a ServerWorks HT-1000 SATA

[Simon Burge <simonb%NetBSD.org@localhost>]

The following reply was made to PR kern/38056; it has been noted by GNATS.

From: Simon Burge <simonb%NetBSD.org@localhost>
To: jnemeth%victoria.tc.ca@localhost (John Nemeth)
Cc: Manuel Bouyer <bouyer%antioche.eu.org@localhost>, 
gnats-bugs%NetBSD.org@localhost,
    kern-bug-people%NetBSD.org@localhost, netbsd-bugs%NetBSD.org@localhost
Subject: Re: kern/38056: ahcisata fails to attach with a ServerWorks HT-1000 
SATA 
Date: Tue, 19 Feb 2008 15:03:15 +1100

 [ Not sure why part of John's reply got trimmed from this but
   is OK in gnats ]
 
 John Nemeth wrote:
 
 > On Jul 12,  7:09am, Simon Burge wrote:
 > } Manuel Bouyer wrote:
 > }
 > } > I suspect it's a side effect of support for AHCI RAID controllers.
 > } > What does pcidump say for this device ?
 > } >
 > } > ahcisata should probably not match any controller announcing a RAID 
 > subclass
 > .
 > } > We should use a table of compatible devices for RAID class.
 > 
 >      I think I may have found the bug.
 > sys/dev/pci/ahcisata_pci:ahci_pci_match() looks like this:
 > 
 >         int ret = 0;
 > 
 >         if (PCI_CLASS(pa->pa_class) == PCI_CLASS_MASS_STORAGE &&
 >             ((PCI_SUBCLASS(pa->pa_class) == PCI_SUBCLASS_MASS_STORAGE_SATA &&
 >              PCI_INTERFACE(pa->pa_class) == PCI_INTERFACE_SATA_AHCI) ||
 >              PCI_SUBCLASS(pa->pa_class) == PCI_SUBCLASS_MASS_STORAGE_RAID)) {
 >                 /* check if the chip is in ahci mode */
 >                 if (pci_mapreg_map(pa, AHCI_PCI_ABAR,
 >                     PCI_MAPREG_TYPE_MEM | PCI_MAPREG_MEM_TYPE_32BIT, 0,
 >                     &regt, &regh, NULL, &size) != 0)
 >                         return (0);
 >                 if (bus_space_read_4(regt, regh, AHCI_GHC) & AHCI_GHC_AE)
 >                         ret = 3;
 >                 bus_space_unmap(regt, regh, size);
 >                 return (3);
 >         }
 > 
 >         return (ret);
 > 
 >      Notice that after bus_space_read_4(), it does bus_space_unmap()
 > followed by return (3) regardless of what the AHCI_GHC register
 > contained.  I think that "return (3)" should be "return (ret)", that
 > way it will only return 3 if AHCI_GHC & AHCI_GHC_AE != 0.  This bug has
 > been there since rev. 1.1, but would have been uncovered by rev. 1.3 on
 > Feb. 11th, which added the check against
 > PCI_SUBCLASS_MASS_STORAGE_RAID.
 
 Using "return (ret)" fixes this for me - svwsata(4) attaches and
 ahcisata(4) doesn't attempt to.
 
        svwsata0 at pci1 dev 14 function 0
        svwsata0: ServerWorks HT-1000 SATA Controller (rev. 0x00)
        svwsata0: using irq 11 for native-PCI interrupt
        atabus0 at svwsata0 channel 0
        atabus1 at svwsata0 channel 1
        atabus2 at svwsata0 channel 2
        atabus3 at svwsata0 channel 3
        ...
        svwsata0 port 0: device present, speed: 1.5Gb/s
        wd0 at atabus0 drive 0: <FUJITSU MHT2080BH>
        wd0: 76319 MB, 155061 cyl, 16 head, 63 sec, 512 bytes/sect x 156301488 
sectors
        svwsata0 port 1: PHY offline
        svwsata0 port 2: PHY offline
        svwsata0 port 3: PHY offline
 
 Manuel - OK for John to commit that fix?
 
 Cheers,
 Simon.