From mboxrd@z Thu Jan 1 00:00:00 1970 From: Luben Tuikov Subject: Re: [PATCH] Add proper module ID tables to Adaptec aic7[9x]xx drivers Date: Thu, 07 Oct 2004 14:08:46 -0400 Sender: linux-scsi-owner@vger.kernel.org Message-ID: <4165862E.1040904@adaptec.com> References: <41644BD5.7030807@tls.msk.ru> <20041006202543.GA22794@wotan.suse.de> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii; format=flowed Content-Transfer-Encoding: 7bit Return-path: Received: from magic.adaptec.com ([216.52.22.17]:5820 "EHLO magic.adaptec.com") by vger.kernel.org with ESMTP id S267618AbUJGSI6 (ORCPT ); Thu, 7 Oct 2004 14:08:58 -0400 In-Reply-To: <20041006202543.GA22794@wotan.suse.de> List-Id: linux-scsi@vger.kernel.org To: Andi Kleen Cc: Michael Tokarev , Sergey Vlasov , linux-scsi@vger.kernel.org Andi Kleen wrote: > On Wed, Oct 06, 2004 at 11:47:33PM +0400, Michael Tokarev wrote: > >>[Answering to an old post -- rehashing the topic. >> I didn't trim the email. See my comments below.] >> >>On Mon, 21 Jun 2004 20:10:47 +0400, Sergey Vlasov wrote: >> >>>On Mon, Jun 21, 2004 at 06:24:40PM +0200, Andi Kleen wrote: >>> >>> >>>>On Mon, 21 Jun 2004 17:44:08 +0400 >>>>Sergey Vlasov wrote: >>>> >>>> >>>>>On Mon, 21 Jun 2004 16:14:41 +0200 Andi Kleen wrote: >>>>> >>>>> >>>>> >>>>>>This is needed for 2.6 hotplug where the driver is autoloaded. When you >>>>>>have >>>>>>multiple conflicting entries the hotplug module loader usually loads >>>>>>the first one listed, which may be correct or may be not. >>>>>> >>>>>>With these changes the drivers announce the correct PCI IDs. >>>>> >>>>>Unfortunately, the patch does not seem to be correct :( >>>>> >>>>>struct pci_device_id does not have the mask field, therefore the >>>>>ID_9005_GENERIC_MASK restriction cannot be specified other than by >>>>>listing all 16 possible IDs as separate entries. Your patch adds only >>>>>one entry, thus losing 15 other possible IDs. >>>> >>>>Hmm, good point. Thanks for catching this. >>>> >>>>Here is a new patch. Does this one look better? >>> >>>It fixes the above problem, but it's hard to say that the patch is >>>correct without carefully checking all the tables. I'm trying to >>>hack up something to autogenerate the pci_device_id table from the >>>aic7xxx internal table. >>> >>>BTW, ID_AIC7810 and ID_AIC7815 probably should not be in the PCI ID >>>table at all - ahc_raid_setup() just prints "RAID functionality >>>unsupported" for them. And some more IDs generated by ID16 are >>>really rejected by the driver due to the (ID_9005_SISL_ID, >>>ID_9005_SISL_MASK) exclusion entry. >> >>I found several "OEM" (on-board) aic79xx controllers (mostly on >>HP boxes) that breaks after the above-mentioned change, which >>found it's way into 2.6.8 kernel. For example, HP ProLiant ML > > > I don't think it's merged in mainline, no. Or at least 2.6.9rc3 > doesn't have it. > > > >>machines and others. The end-result is that the driver, which >>worked just fine before, does not "detect" the card anymore. >> >>On one of such machines, AIC7902 U320 (rev 03) card is shown >>as device=9005:801f (identified by the driver), but subsystem >>id is 103c:103c, wich gets interpreted by pci.ids as >>"Hewlett-Packard Company: Unknown device 103c". It does >>not look like a right thing to do, but HP knows better it >>seems. >> >>Also, I tried to guess what all those new PCI ID macros does >>in the driver, but that's quite a challenge: deeply-nested >>macros with non-obvious bit manipulation... ;) So I can't > > > They just try to match large groups without too much typing.... > > >>produce a patch right now for this problem. Either way, >>the resulting PCI table does not look right: >> >>$ fgrep aic79xx /lib/modules/`uname -r`/modules.pcimap | wc -l >>32 >>$ fgrep aic79xx /lib/modules/`uname -r`/modules.pcimap | sort -u | wc -l >>15 >> >>Devices 9005:800f and 9005:801e are listed with *:* subsystem, >>while the rest (incl. 9005:801f -- the one shown above) specifies >>several non-wildcard subsystem entries, nothing to match 103c >>(HP). > > > If I remember the patch correctly it only matches the Adaptec vendor id. > > For HP there will need to be an special entry (probably needs some > reverse engineering from the original code) > > >>Maybe just list all subsystems as wildcards? > > > No, i don't think that's a good idea. > > At least the vendors should be listed explicitely. > Otherwise hotplug autoloading is unhappy because bogus modules > get loaded all the time. > > I can go over it again at some point, but currently I don't > have too much time, so it would be good if someone else would > tackle it. > > Luben, can you just do a proper pci_id table please? You probably > know the requirements best. Yes, no problem. (It'd be tricky but I'll try to augment it properly.) Luben > -Andi >