Bug 14345

Summary:	Version 1.27 of the 'src/sys/dev/ata/ata-all.c' prevents HPT366 matching
Product:	Base System	Reporter:	nnd <nnd>
Component:	kern	Assignee:	Søren Schmidt <sos>
Status:	Closed FIXED
Severity:	Affects Only Me
Priority:	Normal
Version:	Unspecified
Hardware:	Any
OS:	Any

Description nnd 1999-10-15 10:10:00 UTC

	Doug Rabson's commit to the file in question with the
  Log:
  Don't match non-IDE devices in probe.

prevents at least HighPoint HPT366 IDE controller from matching.
Kernel with version 1.26 file successfully recognizes and works
with the disk on this controller on the Abit's BP6 motherboard.
The reason for this is that motherbord (or PCI-BIOS ?) reports
this controller class as 'class=01-80-00' i.e. class=PCIC_STORGE
ans subclass=PCIS_OTHER. This does'nt prevent 'ata_pciattach'
from attaching HPT366 controller.

	Loking at the diff between -r1.26 and -r1.27 I dont
understand the purpose of the patch at all. Additional test
at the beginning of the 'ata_pcimatch' function can only return
NULL where previous version return non-NULL for one of the
"supported chipsets" or "unsupported but known chipsets".
But if there is such a chipset - is'nt it better to change
it's 'case' instead of the general (pre)condition ?

Fix: 

Revert this file to the -r1.26
How-To-Repeat: 
	Take an FreeBSD-current system with HPT366 controller
and kernels with the -r1.26 and -r1.27 of the 'src/sys/dev/ata/ata_all.c'.
Then the first one will work with this controller and the second
not recognized it at all.

Comment 1 Sheldon Hearn freebsd_committer

1999-10-15 10:19:55 UTC

Responsible Changed
From-To: freebsd-bugs->sos

Over to the driver's maintainer.

Comment 2 Doug Rabson 1999-10-15 22:18:39 UTC

On 15 Oct 1999 nnd@mail.nsk.ru wrote:

> 
> >Number:         14345
> >Category:       kern
> >Synopsis:       Version 1.27 of the 'src/sys/dev/ata/ata-all.c' prevents HPT366 matching
> >Confidential:   no
> >Severity:       serious
> >Priority:       high
> >Responsible:    freebsd-bugs
> >State:          open
> >Quarter:        
> >Keywords:       
> >Date-Required:
> >Class:          sw-bug
> >Submitter-Id:   current-users
> >Arrival-Date:   Fri Oct 15 02:10:00 PDT 1999
> >Closed-Date:
> >Last-Modified:
> >Originator:     Nickolay N. Dudorov
> >Release:        FreeBSD-current i386
> >Organization:
> STE Infoteka
> >Environment:
> 
> 	FreeBSD-current after 1999/10/13 11:56:50 PDT
> 
> >Description:
> 
> 	Doug Rabson's commit to the file in question with the
>   Log:
>   Don't match non-IDE devices in probe.
> 
> prevents at least HighPoint HPT366 IDE controller from matching.
> Kernel with version 1.26 file successfully recognizes and works
> with the disk on this controller on the Abit's BP6 motherboard.
> The reason for this is that motherbord (or PCI-BIOS ?) reports
> this controller class as 'class=01-80-00' i.e. class=PCIC_STORGE
> ans subclass=PCIS_OTHER. This does'nt prevent 'ata_pciattach'
> from attaching HPT366 controller.
> 
> 	Loking at the diff between -r1.26 and -r1.27 I dont
> understand the purpose of the patch at all. Additional test
> at the beginning of the 'ata_pcimatch' function can only return
> NULL where previous version return non-NULL for one of the
> "supported chipsets" or "unsupported but known chipsets".
> But if there is such a chipset - is'nt it better to change
> it's 'case' instead of the general (pre)condition ?

The reason I needed this is because at least one supported chipset
(Cypress 82c693) has several functions including pci-isa bridge, usb
bridge and ata. The broken part for this particular chipset is that the
same deviceid/vendorid is reported for all functions and the drivers need
to check the device class to avoid matching for the wrong function.

Probably the correct workaround is to just check for PCIC_STORAGE. This
patch should work for both broken chipsets:

Index: ata-all.c
===================================================================
RCS file: /home/ncvs/src/sys/dev/ata/ata-all.c,v
retrieving revision 1.27
diff -u -r1.27 ata-all.c
--- ata-all.c	1999/10/13 18:56:49	1.27
+++ ata-all.c	1999/10/15 21:17:57
@@ -181,8 +181,7 @@
 static const char *
 ata_pcimatch(device_t dev)
 {
-    if (pci_get_class(dev) != PCIC_STORAGE ||
-	(pci_get_subclass(dev) != PCIS_STORAGE_IDE))
+    if (pci_get_class(dev) != PCIC_STORAGE)
 	return NULL;
 
     switch (pci_get_devid(dev)) {


--
Doug Rabson				Mail:  dfr@nlsystems.com
Nonlinear Systems Ltd.			Phone: +44 181 442 9037

Comment 3 dfr freebsd_committer

1999-10-16 13:25:19 UTC

State Changed
From-To: open->closed

Fixed in version 1.28 of ata-all.c