Date: Wed, 24 Dec 2003 18:00:46 -0700 (MST) From: "M. Warner Losh" <imp@bsdimp.com> To: michael@lestinsky.de Cc: freebsd-current@freebsd.org Subject: Re: [RELENG_5_2] Cardbus trouble Message-ID: <20031224.180046.83980462.imp@bsdimp.com> In-Reply-To: <20031222220014.GA659@mobi.lestinsky.de> References: <20031222220014.GA659@mobi.lestinsky.de>
next in thread | previous in thread | raw e-mail | index | archive | help
In message: <20031222220014.GA659@mobi.lestinsky.de>
Michael Lestinsky <michael@lestinsky.de> writes:
: I just managed to bring my cardbus to life. Well sort of. There is one
: Problem left: Cards are not properly detected. When booting with an
: inserted card, they are gone from dmesg:
This smells like a problem that I've been chasing for a while. I have
some patches that might help. The unsupported range stuff really
shouldn't work (since PCI is a heirarchical bus), but clearly it does
so I'm looking for a good explaination about what I misunderstand
about PCI bus that allows it to work.
You may try setting hw.cbb.start_memory as well.
Warner
If you like to live dangerously, maybe you can try the following
patches for me. "It works for me" on my Dell inspiron 8000. There
are issues with the patch, but they would only bite you down stream
(eg, if you unload a driver, eject a card or something like that).
They are in my p4 tree, as well as the p4 power tree (since they have
power code intertwingled into it).
diff -u /dell/imp/FreeBSD/src/sys/dev/pci/pci.c ./pci.c
--- /dell/imp/FreeBSD/src/sys/dev/pci/pci.c Tue Dec 23 19:01:22 2003
+++ ./pci.c Tue Dec 23 22:37:01 2003
@@ -68,7 +68,8 @@
static int pci_porten(device_t pcib, int b, int s, int f);
static int pci_memen(device_t pcib, int b, int s, int f);
-static int pci_add_map(device_t pcib, int b, int s, int f, int reg,
+static int pci_add_map(device_t pcib, device_t bus, device_t dev,
+ int b, int s, int f, int reg,
struct resource_list *rl);
static void pci_add_resources(device_t pcib, device_t bus,
device_t dev);
@@ -82,13 +83,15 @@
static void pci_hdrtypedata(device_t pcib, int b, int s, int f,
pcicfgregs *cfg);
static void pci_read_extcap(device_t pcib, pcicfgregs *cfg);
+static void pci_cfg_restore(device_t, struct pci_devinfo *);
+static void pci_cfg_save(device_t, struct pci_devinfo *, int);
static device_method_t pci_methods[] = {
/* Device interface */
DEVMETHOD(device_probe, pci_probe),
DEVMETHOD(device_attach, pci_attach),
DEVMETHOD(device_shutdown, bus_generic_shutdown),
- DEVMETHOD(device_suspend, bus_generic_suspend),
+ DEVMETHOD(device_suspend, pci_suspend),
DEVMETHOD(device_resume, pci_resume),
/* Bus interface */
@@ -96,7 +99,7 @@
DEVMETHOD(bus_probe_nomatch, pci_probe_nomatch),
DEVMETHOD(bus_read_ivar, pci_read_ivar),
DEVMETHOD(bus_write_ivar, pci_write_ivar),
- DEVMETHOD(bus_driver_added, bus_generic_driver_added),
+ DEVMETHOD(bus_driver_added, pci_driver_added),
DEVMETHOD(bus_setup_intr, bus_generic_setup_intr),
DEVMETHOD(bus_teardown_intr, bus_generic_teardown_intr),
@@ -613,6 +616,7 @@
return (EINVAL);
}
pci_set_command_bit(dev, child, bit);
+ /* Some devices seem to need a brief stall here, what do to? */
command = PCI_READ_CONFIG(dev, child, PCIR_COMMAND, 2);
if (command & bit)
return (0);
@@ -719,11 +723,12 @@
* register is a 32bit map register or 2 if it is a 64bit register.
*/
static int
-pci_add_map(device_t pcib, int b, int s, int f, int reg,
- struct resource_list *rl)
+pci_add_map(device_t pcib, device_t bus, device_t dev,
+ int b, int s, int f, int reg, struct resource_list *rl)
{
uint32_t map;
uint64_t base;
+ uint64_t start, end, count;
uint8_t ln2size;
uint8_t ln2range;
uint32_t testval;
@@ -732,20 +737,31 @@
map = PCIB_READ_CONFIG(pcib, b, s, f, reg, 4);
- if (map == 0 || map == 0xffffffff)
- return (1); /* skip invalid entry */
-
PCIB_WRITE_CONFIG(pcib, b, s, f, reg, 0xffffffff, 4);
testval = PCIB_READ_CONFIG(pcib, b, s, f, reg, 4);
PCIB_WRITE_CONFIG(pcib, b, s, f, reg, map, 4);
- base = pci_mapbase(map);
if (pci_maptype(map) & PCI_MAPMEM)
type = SYS_RES_MEMORY;
else
type = SYS_RES_IOPORT;
ln2size = pci_mapsize(testval);
ln2range = pci_maprange(testval);
+ base = pci_mapbase(map);
+
+ /*
+ * For I/O registers, if bottom bit is set, and the next bit up
+ * isn't clear, we know we have a BAR that doesn't conform to the
+ * spec, so ignore it. Also, sanity check the size of the data
+ * areas to the type of memory involved.
+ */
+ if ((testval & 0x1) == 0x1 &&
+ (testval & 0x2) != 0)
+ return (1);
+ if ((type == SYS_RES_MEMORY && ln2size < 5) ||
+ (type == SYS_RES_IOPORT && ln2size < 3))
+ return (1);
+
if (ln2range == 64) {
/* Read the other half of a 64bit map register */
base |= (uint64_t) PCIB_READ_CONFIG(pcib, b, s, f, reg + 4, 4) << 32;
@@ -787,9 +803,23 @@
if (type == SYS_RES_MEMORY && !pci_memen(pcib, b, s, f))
return (1);
}
- resource_list_add(rl, type, reg, base, base + (1 << ln2size) - 1,
- (1 << ln2size));
+ /*
+ * If base is 0, then we have problems. It is best to ignore
+ * such entires for the moment. They don't decode anything (at
+ * least on some hardware), so cause no problems.
+ */
+ if (base == 0)
+ return 1;
+ start = base;
+ end = base + (1 << ln2size) - 1;
+ count = 1 << ln2size;
+ resource_list_add(rl, type, reg, start, end, count);
+ /*
+ * Not quite sure what to do on failure of allocating the resource
+ * since I can postulate several right answers.
+ */
+ resource_list_alloc(rl, bus, dev, type, ®, start, end, count, 0);
return ((ln2range == 64) ? 2 : 1);
}
@@ -806,13 +836,13 @@
s = cfg->slot;
f = cfg->func;
for (i = 0; i < cfg->nummaps;) {
- i += pci_add_map(pcib, b, s, f, PCIR_BAR(i), rl);
+ i += pci_add_map(pcib, bus, dev, b, s, f, PCIR_BAR(i), rl);
}
for (q = &pci_quirks[0]; q->devid; q++) {
if (q->devid == ((cfg->device << 16) | cfg->vendor)
&& q->type == PCI_QUIRK_MAP_REG)
- pci_add_map(pcib, b, s, f, q->arg1, rl);
+ pci_add_map(pcib, bus, dev, b, s, f, q->arg1, rl);
}
if (cfg->intpin > 0 && PCI_INTERRUPT_VALID(cfg->intline)) {
@@ -873,6 +903,7 @@
pcib = device_get_parent(bus);
dinfo->cfg.dev = device_add_child(bus, NULL, -1);
device_set_ivars(dinfo->cfg.dev, dinfo);
+ pci_cfg_restore(dinfo->cfg.dev, dinfo);
pci_add_resources(pcib, bus, dinfo->cfg.dev);
pci_print_verbose(dinfo);
}
@@ -907,6 +938,52 @@
return (bus_generic_attach(dev));
}
+int
+pci_suspend(device_t dev)
+{
+ int numdevs;
+ device_t *devlist;
+ device_t child;
+ struct pci_devinfo *dinfo;
+ int i;
+
+ /*
+ * Save the pci configuration space for each child. We don't need
+ * to do this, unless the BIOS suspend code powers down the bus and
+ * the devices on the bus.
+ */
+ device_get_children(dev, &devlist, &numdevs);
+ for (i = 0; i < numdevs; i++) {
+ child = devlist[i];
+ dinfo = (struct pci_devinfo *) device_get_ivars(child);
+ pci_cfg_save(child, dinfo, 0);
+ }
+ free(devlist, M_TEMP);
+ return (bus_generic_suspend(dev));
+}
+
+int
+pci_resume(device_t dev)
+{
+ int numdevs;
+ device_t *devlist;
+ device_t child;
+ struct pci_devinfo *dinfo;
+ int i;
+
+ /*
+ * Restore the pci configuration space for each child.
+ */
+ device_get_children(dev, &devlist, &numdevs);
+ for (i = 0; i < numdevs; i++) {
+ child = devlist[i];
+ dinfo = (struct pci_devinfo *) device_get_ivars(child);
+ pci_cfg_restore(child, dinfo);
+ }
+ free(devlist, M_TEMP);
+ return (bus_generic_resume(dev));
+}
+
static void
pci_load_vendor_data(void)
{
@@ -922,6 +999,34 @@
}
}
+void
+pci_driver_added(device_t dev, driver_t *driver)
+{
+ int numdevs;
+ device_t *devlist;
+ device_t child;
+ struct pci_devinfo *dinfo;
+ int i;
+
+ device_printf(dev, "driver added\n");
+ DEVICE_IDENTIFY(driver, dev);
+ device_get_children(dev, &devlist, &numdevs);
+ for (i = 0; i < numdevs; i++) {
+ child = devlist[i];
+ if (device_get_state(child) != DS_NOTPRESENT)
+ continue;
+ dinfo = device_get_ivars(child);
+ pci_print_verbose(dinfo);
+/*XXX???*/ /* resource_list_init(&dinfo->cfg.resources); */
+ printf("pci%d:%d:%d: reprobing on driver added\n",
+ dinfo->cfg.bus, dinfo->cfg.slot, dinfo->cfg.func);
+ pci_cfg_restore(child, dinfo);
+ if (device_probe_and_attach(child) != 0)
+ pci_cfg_save(child, dinfo, 1);
+ }
+ free(devlist, M_TEMP);
+}
+
int
pci_print_child(device_t dev, device_t child)
{
@@ -1047,6 +1152,7 @@
}
printf(" at device %d.%d (no driver attached)\n",
pci_get_slot(child), pci_get_function(child));
+ pci_cfg_save(child, (struct pci_devinfo *) device_get_ivars(child), 1);
return;
}
@@ -1332,6 +1438,7 @@
{
struct pci_devinfo *dinfo = device_get_ivars(child);
struct resource_list *rl = &dinfo->resources;
+ struct resource_list_entry *rle;
pcicfgregs *cfg = &dinfo->cfg;
/*
@@ -1363,16 +1470,34 @@
if (*rid < PCIR_BAR(cfg->nummaps)) {
/*
* Enable the I/O mode. We should
- * also be allocating resources
- * too. XXX
+ * also be assigning resources too
+ * when none are present. The
+ * resource_list_alloc kind of sorta does
+ * this...
*/
if (PCI_ENABLE_IO(dev, child, type))
return (NULL);
}
break;
}
+ /*
+ * If we've already allocated the resource, then
+ * return it now. But first we may need to activate
+ * it, since we don't allocate the resource as active
+ * above. Normally this would be done down in the
+ * nexus, but since we short-circuit that path we have
+ * to do its job here. Not sure if we should free the
+ * resource if it fails to activate.
+ */
+ rle = resource_list_find(rl, type, *rid);
+ if (rle != NULL && rle->res != NULL) {
+ if ((flags & RF_ACTIVE) &&
+ bus_generic_activate_resource(dev, child, type,
+ *rid, rle->res))
+ return NULL;
+ return (rle->res);
+ }
}
-
return (resource_list_alloc(rl, dev, child, type, rid,
start, end, count, flags));
}
@@ -1416,13 +1541,9 @@
struct resource_list *
pci_get_resource_list (device_t dev, device_t child)
{
- struct pci_devinfo * dinfo = device_get_ivars(child);
- struct resource_list * rl = &dinfo->resources;
-
- if (!rl)
- return (NULL);
+ struct pci_devinfo *dinfo = device_get_ivars(child);
- return (rl);
+ return (&dinfo->resources);
}
uint32_t
@@ -1447,7 +1568,7 @@
}
int
-pci_child_location_str_method(device_t cbdev, device_t child, char *buf,
+pci_child_location_str_method(device_t dev, device_t child, char *buf,
size_t buflen)
{
struct pci_devinfo *dinfo;
@@ -1459,7 +1580,7 @@
}
int
-pci_child_pnpinfo_str_method(device_t cbdev, device_t child, char *buf,
+pci_child_pnpinfo_str_method(device_t dev, device_t child, char *buf,
size_t buflen)
{
struct pci_devinfo *dinfo;
@@ -1506,34 +1627,39 @@
return (0);
}
-int
-pci_resume(device_t dev)
+static void
+pci_cfg_restore(device_t dev, struct pci_devinfo *dinfo)
{
- int numdevs;
- int i;
- device_t *children;
- device_t child;
- struct pci_devinfo *dinfo;
- pcicfgregs *cfg;
+#define NBAR 7
+ int i;
+ uint32_t bar[NBAR];
- device_get_children(dev, &children, &numdevs);
+ if (dinfo->cfg.hdrtype != 0)
+ return;
+ for (i = 0; i < NBAR; i++)
+ bar[i] = pci_read_config(dev, PCIR_MAPS + i * 4, 4);
+ printf("pci%d:%d:%d: setting power state D0\n", dinfo->cfg.bus,
+ dinfo->cfg.slot, dinfo->cfg.func);
+ pci_set_powerstate(dev, PCI_POWERSTATE_D0);
+ for (i = 0; i < NBAR; i++)
+ pci_write_config(dev, PCIR_MAPS + i * 4, bar[i], 4);
+ pci_write_config(dev, PCIR_INTLINE, dinfo->cfg.intline, 1);
+ pci_write_config(dev, PCIR_INTPIN, dinfo->cfg.intpin, 1);
+ pci_write_config(dev, PCIR_MINGNT, dinfo->cfg.mingnt, 1);
+ pci_write_config(dev, PCIR_MAXLAT, dinfo->cfg.maxlat, 1);
+}
- for (i = 0; i < numdevs; i++) {
- child = children[i];
+static void
+pci_cfg_save(device_t dev, struct pci_devinfo *dinfo, int setstate)
+{
+ uint32_t cls;
- dinfo = device_get_ivars(child);
- cfg = &dinfo->cfg;
- if (cfg->intpin > 0 && PCI_INTERRUPT_VALID(cfg->intline)) {
- cfg->intline = PCI_ASSIGN_INTERRUPT(dev, child);
- if (PCI_INTERRUPT_VALID(cfg->intline)) {
- pci_write_config(child, PCIR_INTLINE,
- cfg->intline, 1);
- }
- }
+ if (dinfo->cfg.hdrtype != 0)
+ return;
+ cls = pci_get_class(dev);
+ if (setstate && cls != PCIC_DISPLAY && cls != PCIC_MEMORY) {
+ pci_set_powerstate(dev, PCI_POWERSTATE_D3);
+ printf("pci%d:%d:%d: setting power state D3\n", dinfo->cfg.bus,
+ dinfo->cfg.slot, dinfo->cfg.func);
}
-
- free(children, M_TEMP);
-
- return (bus_generic_resume(dev));
}
-
diff -u /dell/imp/FreeBSD/src/sys/dev/pci/pci_private.h ./pci_private.h
--- /dell/imp/FreeBSD/src/sys/dev/pci/pci_private.h Sun Oct 19 11:32:23 2003
+++ ./pci_private.h Sat Sep 27 12:22:09 2003
@@ -41,6 +41,7 @@
void pci_add_children(device_t dev, int busno, size_t dinfo_size);
void pci_add_child(device_t bus, struct pci_devinfo *dinfo);
+void pci_driver_added(device_t dev, driver_t *driver);
int pci_print_child(device_t dev, device_t child);
void pci_probe_nomatch(device_t dev, device_t child);
int pci_read_ivar(device_t dev, device_t child, int which,
@@ -74,4 +75,5 @@
char *buf, size_t buflen);
int pci_assign_interrupt_method(device_t dev, device_t child);
int pci_resume(device_t dev);
+int pci_suspend(device_t dev);
#endif /* _PCI_PRIVATE_H_ */
diff -u /dell/imp/FreeBSD/src/sys/dev/pci/pcivar.h ./pcivar.h
--- /dell/imp/FreeBSD/src/sys/dev/pci/pcivar.h Sun Oct 19 11:32:23 2003
+++ ./pcivar.h Sat Sep 27 11:22:44 2003
@@ -66,8 +66,13 @@
uint16_t msi_data; /* Location of MSI data word */
};
-/* config header information common to all header types */
+/* Additional data saved on power events */
+struct pci_save
+{
+ uint32_t bar[7]; /* 7 bars to save */
+};
+/* config header information common to all header types */
typedef struct pcicfg {
struct device *dev; /* device which owns this */
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20031224.180046.83980462.imp>