Re: Workaround for some broken BIOSes that forgot to enable ATA channels [patch]

In message: <20030414100702.GC22229_at_vega.vega.com>
            Maxim Sobolev <sobomax_at_portaone.com> writes:
: Attached please find a patch, which workaround a bug found in
: some BIOSes, which forget to enable ATA channels properly.
: This results in ATA driver not attaching properly and inability
: to use disk devices.

This patch is fundamentally flawed.  Driver have no business directly
enabling I/O port or memory ranges.  All drivers would need it.  It
also uses the wrong API to do so.

However, I'm curious why setting pci_enable_io_modes to 1 doesn't work
for you:

static int pci_enable_io_modes = 1;
TUNABLE_INT("hw.pci.enable_io_modes", (int *)&pci_enable_io_modes);
SYSCTL_INT(_hw_pci, OID_AUTO, enable_io_modes, CTLFLAG_RW,
    &pci_enable_io_modes, 1,
    "Enable I/O and memory bits in the config register.  Some BIOSes do not\n\
enable these bits correctly.  We'd like to do this all the time, but there\n\
are some peripherals that this causes problems with.");

However, I'm starting to think that the right place for this isn't
where the pci bus detects the resources (which is where the about
tunable/sysctl controls), but rather at driver allocation time:

Index: pci.c
===================================================================
RCS file: /home/ncvs/src/sys/dev/pci/pci.c,v
retrieving revision 1.212
diff -u -r1.212 pci.c
--- pci.c	19 Feb 2003 05:47:09 -0000	1.212
+++ pci.c	15 Apr 2003 14:23:03 -0000
_at__at_ -1301,21 +1301,33 _at__at_
 	 * XXX add support here for SYS_RES_IOPORT and SYS_RES_MEMORY
 	 */
 	if (device_get_parent(child) == dev) {
-		/*
-		 * If the child device doesn't have an interrupt routed
-		 * and is deserving of an interrupt, try to assign it one.
-		 */
-		if ((type == SYS_RES_IRQ) &&
-		    !PCI_INTERRUPT_VALID(cfg->intline) &&
-		    (cfg->intpin != 0)) {
-			cfg->intline = PCIB_ROUTE_INTERRUPT(
-				device_get_parent(dev), child, cfg->intpin);
-			if (PCI_INTERRUPT_VALID(cfg->intline)) {
-				pci_write_config(child, PCIR_INTLINE,
-				    cfg->intline, 1);
-				resource_list_add(rl, SYS_RES_IRQ, 0,
-				    cfg->intline, cfg->intline, 1);
+		switch (type) {
+		case SYS_RES_IRQ:
+			/*
+			 * If the child device doesn't have an
+			 * interrupt routed and is deserving of an
+			 * interrupt, try to assign it one.
+			 */
+			if (!PCI_INTERRUPT_VALID(cfg->intline) &&
+			    (cfg->intpin != 0)) {
+				cfg->intline = PCIB_ROUTE_INTERRUPT(
+				    device_get_parent(dev), child, cfg->intpin);
+				if (PCI_INTERRUPT_VALID(cfg->intline)) {
+					pci_write_config(child, PCIR_INTLINE,
+					    cfg->intline, 1);
+					resource_list_add(rl, SYS_RES_IRQ, 0,
+					    cfg->intline, cfg->intline, 1);
+				}
 			}
+			break;
+		case SYS_RES_IOPORT:
+		case SYS_RES_MEMORY:
+			/*
+			 * Enable the I/O mode.  We should also be allocating
+			 * resources too. XXX
+			 */
+			pci_enable_io_method(dev, child, type);
+			break;
 		}
 	}
 

Warner