From owner-freebsd-current@FreeBSD.ORG Sun Jun 15 07:07:50 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 BF0BF37B401 for ; Sun, 15 Jun 2003 07:07:50 -0700 (PDT) Received: from postfix4-1.free.fr (postfix4-1.free.fr [213.228.0.62]) by mx1.FreeBSD.org (Postfix) with ESMTP id 70CC743F3F for ; Sun, 15 Jun 2003 07:07:49 -0700 (PDT) (envelope-from nsouch@free.fr) Received: from armor.fastether (nas-cbv-7-62-147-153-119.dial.proxad.net [62.147.153.119]) by postfix4-1.free.fr (Postfix) with SMTP id 06CB13ED33 for ; Sun, 15 Jun 2003 16:07:43 +0200 (CEST) Received: (qmail 5666 invoked by uid 1001); 15 Jun 2003 16:22:14 -0000 Date: Sun, 15 Jun 2003 16:22:14 +0000 From: Nicolas Souchu To: Terry Lambert Message-ID: <20030615162214.A5630@armor.free.fr> References: <20030602222009.A16160@armor.free.fr> <20030603175430.GA4039@tombstone.localnet.gomerbud.com> <20030604072931.E33869@armor.free.fr> <20030604072907.GA10742@tombstone.localnet.gomerbud.com> <3EDF26D2.C93A93DF@mindspring.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline User-Agent: Mutt/1.2.5i In-Reply-To: <3EDF26D2.C93A93DF@mindspring.com>; from tlambert2@mindspring.com on Thu, Jun 05, 2003 at 04:17:38AM -0700 cc: "David P. Reese Jr." cc: current@freebsd.org Subject: Patch review [was: Re: viapropm doesnt like sys/dev/pci.c rev 1.214] 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: Sun, 15 Jun 2003 14:07:51 -0000 On Thu, Jun 05, 2003 at 04:17:38AM -0700, Terry Lambert wrote: > How about RF_DONTCHECK or RF_ALWAYSWORKS? It better implies > what's happening here, since you're going to assume success in > the face of diagnostics to the contrary. > > So instead of: > > if (flag) > return (0); > command = PCI_READ_CONFIG(dev, child, PCIR_COMMAND, 2); > if (command & bit) > return (0); > device_printf(child, "failed to enable %s mapping!\n", error); > return (ENXIO); > > You do: > > command = PCI_READ_CONFIG(dev, child, PCIR_COMMAND, 2); > if ((command & bit) || flag) > return (0); > device_printf(child, "failed to enable %s mapping!\n", error); > return (ENXIO); > > Yeah, I know the disctinction is subtle, but there migh be other > PCI_READ_CONFIG() results later that people care about, besides > just this one bit, which *do* work on some other chip with the > same issue. Sounds good like that? (ignore more changes in amdpm.c. Just consider that RF_DONTCHECK was added to the resource allocation). Note that AMD-768 PM has the same flaw as the VIA chipset. Index: dev/cardbus/cardbus_cis.c =================================================================== RCS file: /home/ncvs/src/sys/dev/cardbus/cardbus_cis.c,v retrieving revision 1.37 diff -u -r1.37 cardbus_cis.c --- dev/cardbus/cardbus_cis.c 24 May 2003 23:23:41 -0000 1.37 +++ dev/cardbus/cardbus_cis.c 15 Jun 2003 16:05:16 -0000 @@ -457,7 +457,7 @@ * Mark the appropriate bit in the PCI command register so that * device drivers will know which type of BARs can be used. */ - pci_enable_io(child, type); + pci_enable_io(child, type, 0); return (0); } @@ -624,7 +624,7 @@ rman_get_start(res) | ((*rid == CARDBUS_ROM_REG)? CARDBUS_ROM_ENABLE : 0), 4); - PCI_ENABLE_IO(cbdev, child, SYS_RES_MEMORY); + PCI_ENABLE_IO(cbdev, child, SYS_RES_MEMORY, 0); /* Flip to the right ROM image if CIS is in ROM */ if (CARDBUS_CIS_SPACE(*start) == CARDBUS_CIS_ASI_ROM) { Index: dev/hifn/hifn7751.c =================================================================== RCS file: /home/ncvs/src/sys/dev/hifn/hifn7751.c,v retrieving revision 1.13 diff -u -r1.13 hifn7751.c --- dev/hifn/hifn7751.c 11 Mar 2003 22:47:06 -0000 1.13 +++ dev/hifn/hifn7751.c 15 Jun 2003 16:03:43 -0000 @@ -616,7 +616,7 @@ /* reenable busmastering */ pci_enable_busmaster(dev); - pci_enable_io(dev, HIFN_RES); + pci_enable_io(dev, HIFN_RES, 0); /* reinitialize interface if necessary */ if (ifp->if_flags & IFF_UP) Index: dev/pci/pci.c =================================================================== RCS file: /home/ncvs/src/sys/dev/pci/pci.c,v retrieving revision 1.214 diff -u -r1.214 pci.c --- dev/pci/pci.c 16 Apr 2003 03:15:08 -0000 1.214 +++ dev/pci/pci.c 15 Jun 2003 15:25:57 -0000 @@ -583,7 +583,7 @@ } int -pci_enable_io_method(device_t dev, device_t child, int space) +pci_enable_io_method(device_t dev, device_t child, int space, u_int flags) { u_int16_t command; u_int16_t bit; @@ -607,7 +607,7 @@ } pci_set_command_bit(dev, child, bit); command = PCI_READ_CONFIG(dev, child, PCIR_COMMAND, 2); - if (command & bit) + if ((command & bit) || (flags & RF_DONTCHECK)) return (0); device_printf(child, "failed to enable %s mapping!\n", error); return (ENXIO); @@ -1365,7 +1365,7 @@ * Enable the I/O mode. We should also be allocating * resources too. XXX */ - if (PCI_ENABLE_IO(dev, child, type)) + if (PCI_ENABLE_IO(dev, child, type, flags)) return (NULL); break; } Index: dev/pci/pci_if.m =================================================================== RCS file: /home/ncvs/src/sys/dev/pci/pci_if.m,v retrieving revision 1.5 diff -u -r1.5 pci_if.m --- dev/pci/pci_if.m 16 Apr 2003 03:15:08 -0000 1.5 +++ dev/pci/pci_if.m 15 Jun 2003 15:23:23 -0000 @@ -70,6 +70,7 @@ device_t dev; device_t child; int space; + u_int flags; }; METHOD int disable_io { Index: dev/pci/pci_private.h =================================================================== RCS file: /home/ncvs/src/sys/dev/pci/pci_private.h,v retrieving revision 1.8 diff -u -r1.8 pci_private.h --- dev/pci/pci_private.h 16 Apr 2003 03:15:08 -0000 1.8 +++ dev/pci/pci_private.h 15 Jun 2003 15:27:55 -0000 @@ -56,7 +56,8 @@ int reg, u_int32_t val, int width); int pci_enable_busmaster_method(device_t dev, device_t child); int pci_disable_busmaster_method(device_t dev, device_t child); -int pci_enable_io_method(device_t dev, device_t child, int space); +int pci_enable_io_method(device_t dev, device_t child, int space, + u_int flags); int pci_disable_io_method(device_t dev, device_t child, int space); struct resource *pci_alloc_resource(device_t dev, device_t child, int type, int *rid, u_long start, u_long end, u_long count, Index: dev/pci/pcivar.h =================================================================== RCS file: /home/ncvs/src/sys/dev/pci/pcivar.h,v retrieving revision 1.61 diff -u -r1.61 pcivar.h --- dev/pci/pcivar.h 16 Apr 2003 03:15:08 -0000 1.61 +++ dev/pci/pcivar.h 15 Jun 2003 15:24:02 -0000 @@ -262,9 +262,9 @@ } static __inline int -pci_enable_io(device_t dev, int space) +pci_enable_io(device_t dev, int space, u_int flags) { - return(PCI_ENABLE_IO(device_get_parent(dev), dev, space)); + return(PCI_ENABLE_IO(device_get_parent(dev), dev, space, flags)); } static __inline int Index: pci/amdpm.c =================================================================== RCS file: /home/ncvs/src/sys/pci/amdpm.c,v retrieving revision 1.6 diff -u -r1.6 amdpm.c --- pci/amdpm.c 15 Apr 2003 06:37:29 -0000 1.6 +++ pci/amdpm.c 15 Jun 2003 15:31:21 -0000 @@ -66,6 +66,7 @@ #define AMDPM_VENDORID_AMD 0x1022 #define AMDPM_DEVICEID_AMD756PM 0x740b +#define AMDPM_DEVICEID_AMD768PM 0x7443 /* nVidia nForce chipset */ #define AMDPM_VENDORID_NVIDIA 0x10de @@ -141,20 +142,17 @@ static int amdpm_probe(device_t dev) { - u_long base; - - if ((pci_get_vendor(dev) == AMDPM_VENDORID_AMD) && - (pci_get_device(dev) == AMDPM_DEVICEID_AMD756PM)) { - device_set_desc(dev, "AMD 756 Power Management Controller"); - - /* - * We have to do this, since the BIOS won't give us the - * resource info (not mine, anyway). - */ - base = pci_read_config(dev, AMDPCI_PMBASE, 4); - base &= 0xff00; - bus_set_resource(dev, SYS_RES_IOPORT, AMDPCI_PMBASE, base, 256); - return (0); + if ((pci_get_vendor(dev) == AMDPM_VENDORID_AMD)) { + switch (pci_get_device(dev)) { + case AMDPM_DEVICEID_AMD756PM: + device_set_desc(dev, "AMD 756 Power Management Controller"); + return (0); + case AMDPM_DEVICEID_AMD768PM: + device_set_desc(dev, "AMD 768 Power Management Controller"); + return (0); + default: + break; + } } return ENXIO; } @@ -164,6 +162,15 @@ { struct amdpm_softc *amdpm_sc = device_get_softc(dev); u_char val_b; + u_long base; + + /* + * We have to do this, since the BIOS won't give us the + * resource info (not mine, anyway). + */ + base = pci_read_config(dev, AMDPCI_PMBASE, 4); + base &= 0xff00; + bus_set_resource(dev, SYS_RES_IOPORT, AMDPCI_PMBASE, base, 256); /* Enable I/O block access */ val_b = pci_read_config(dev, AMDPCI_GEN_CONFIG_PM, 1); @@ -171,7 +178,7 @@ /* Allocate I/O space */ amdpm_sc->rid = AMDPCI_PMBASE; - amdpm_sc->res = bus_alloc_resource(dev, SYS_RES_IOPORT, &amdpm_sc->rid, 0, ~0, 1, RF_ACTIVE); + amdpm_sc->res = bus_alloc_resource(dev, SYS_RES_IOPORT, &amdpm_sc->rid, 0, ~0, 1, RF_ACTIVE | RF_DONTCHECK); if (amdpm_sc->res == NULL) { device_printf(dev, "could not map i/o space\n"); @@ -211,22 +218,9 @@ static int nfpm_probe(device_t dev) { - u_long base; - if ((pci_get_vendor(dev) == AMDPM_VENDORID_NVIDIA) && (pci_get_device(dev) == AMDPM_DEVICEID_NF_SMB)) { device_set_desc(dev, "nForce SMBus Controller"); - - /* - * We have to do this, since the BIOS won't give us the - * resource info (not mine, anyway). - */ - base = pci_read_config(dev, NFPCI_PMBASE, 4); - base &= 0xff00; - bus_set_resource(dev, SYS_RES_IOPORT, NFPCI_PMBASE, base, 256); - - pm_reg_offset = 0x00; - return (0); } return ENXIO; @@ -237,6 +231,17 @@ { struct amdpm_softc *amdpm_sc = device_get_softc(dev); u_char val_b; + u_long base; + + /* + * We have to do this, since the BIOS won't give us the + * resource info (not mine, anyway). + */ + base = pci_read_config(dev, NFPCI_PMBASE, 4); + base &= 0xff00; + bus_set_resource(dev, SYS_RES_IOPORT, NFPCI_PMBASE, base, 256); + + pm_reg_offset = 0x00; /* Enable I/O block access */ val_b = pci_read_config(dev, AMDPCI_GEN_CONFIG_PM, 1); @@ -244,7 +249,7 @@ /* Allocate I/O space */ amdpm_sc->rid = NFPCI_PMBASE; - amdpm_sc->res = bus_alloc_resource(dev, SYS_RES_IOPORT, &amdpm_sc->rid, 0, ~0, 1, RF_ACTIVE); + amdpm_sc->res = bus_alloc_resource(dev, SYS_RES_IOPORT, &amdpm_sc->rid, 0, ~0, 1, RF_ACTIVE | RF_DONTCHECK); if (amdpm_sc->res == NULL) { device_printf(dev, "could not map i/o space\n"); Index: pci/if_dc.c =================================================================== RCS file: /home/ncvs/src/sys/pci/if_dc.c,v retrieving revision 1.108 diff -u -r1.108 if_dc.c --- pci/if_dc.c 15 May 2003 16:53:29 -0000 1.108 +++ pci/if_dc.c 15 Jun 2003 16:04:06 -0000 @@ -3760,7 +3760,7 @@ /* reenable busmastering */ pci_enable_busmaster(dev); - pci_enable_io(dev, DC_RES); + pci_enable_io(dev, DC_RES, 0); /* reinitialize interface if necessary */ if (ifp->if_flags & IFF_UP) Index: pci/if_rl.c =================================================================== RCS file: /home/ncvs/src/sys/pci/if_rl.c,v retrieving revision 1.98 diff -u -r1.98 if_rl.c --- pci/if_rl.c 21 Apr 2003 18:34:04 -0000 1.98 +++ pci/if_rl.c 15 Jun 2003 16:04:21 -0000 @@ -1894,7 +1894,7 @@ /* reenable busmastering */ pci_enable_busmaster(dev); - pci_enable_io(dev, RL_RES); + pci_enable_io(dev, RL_RES, 0); /* reinitialize interface if necessary */ if (ifp->if_flags & IFF_UP) Index: sys/rman.h =================================================================== RCS file: /home/ncvs/src/sys/sys/rman.h,v retrieving revision 1.19 diff -u -r1.19 rman.h --- sys/rman.h 12 Feb 2003 07:00:59 -0000 1.19 +++ sys/rman.h 15 Jun 2003 15:21:02 -0000 @@ -43,6 +43,7 @@ #define RF_WANTED 0x0010 /* somebody is waiting for this resource */ #define RF_FIRSTSHARE 0x0020 /* first in sharing list */ #define RF_PREFETCHABLE 0x0040 /* resource is prefetchable */ +#define RF_DONTCHECK 0x0080 /* don't perform extra checks on the resource */ #define RF_ALIGNMENT_SHIFT 10 /* alignment size bit starts bit 10 */ #define RF_ALIGNMENT_MASK (0x003F << RF_ALIGNMENT_SHIFT) -- Nicholas Souchu - nsouch@free.fr - nsouch@FreeBSD.org