From owner-freebsd-current@FreeBSD.ORG Tue Apr 15 07:23:55 2003 Return-Path: Delivered-To: freebsd-current@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id 5D00C37B401; Tue, 15 Apr 2003 07:23:55 -0700 (PDT) Received: from harmony.village.org (rover.bsdimp.com [204.144.255.66]) by mx1.FreeBSD.org (Postfix) with ESMTP id 1333043F85; Tue, 15 Apr 2003 07:23:52 -0700 (PDT) (envelope-from imp@bsdimp.com) Received: from localhost (warner@rover2.village.org [10.0.0.1]) by harmony.village.org (8.12.8/8.12.3) with ESMTP id h3FENjA7062899; Tue, 15 Apr 2003 08:23:51 -0600 (MDT) (envelope-from imp@bsdimp.com) Date: Tue, 15 Apr 2003 08:23:21 -0600 (MDT) Message-Id: <20030415.082321.05356303.imp@bsdimp.com> To: sobomax@portaone.com From: "M. Warner Losh" In-Reply-To: <20030414100702.GC22229@vega.vega.com> References: <20030414100702.GC22229@vega.vega.com> X-Mailer: Mew version 2.1 on Emacs 21.2 / Mule 5.0 (SAKAKI) Mime-Version: 1.0 Content-Type: Text/Plain; charset=us-ascii Content-Transfer-Encoding: 7bit cc: current@freebsd.org Subject: Re: Workaround for some broken BIOSes that forgot to enable ATA channels [patch] X-BeenThere: freebsd-current@freebsd.org X-Mailman-Version: 2.1.1 Precedence: list List-Id: Discussions about the use of FreeBSD-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 15 Apr 2003 14:23:55 -0000 In message: <20030414100702.GC22229@vega.vega.com> Maxim Sobolev 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 @@ -1301,21 +1301,33 @@ * 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