First step in cleaning up CIS parsing and /dev/cardbus*.cis: remove

redundant malloc/free.  Add comments about how this should really be
done.  Fix an overly verbose comment about under 1MB mapping: go ahead
and set the bits, but we ignore them.

3 files changed, 42 insertions, 38 deletions

diff --git a/sys/dev/cardbus/cardbus_cis.c b/sys/dev/cardbus/cardbus_cis.c
index a8cfa25..9fe7477 100644
--- a/sys/dev/cardbus/cardbus_cis.c
+++ b/sys/dev/cardbus/cardbus_cis.c
@@ -318,29 +318,27 @@ decode_tuple_bar(device_t cbdev, device_t child, int id,
 	if (type == SYS_RES_MEMORY) {
 		if (reg & TPL_BAR_REG_PREFETCHABLE)
 			dinfo->mprefetchable |= (1 << PCI_RID2BAR(bar));
-#if 0
 		/*
-		 * XXX: It appears from a careful reading of the spec
-		 * that we're not supposed to honor this when the bridge
-		 * is not on the main system bus.  PCI spec doesn't appear
-		 * to allow for memory ranges not listed in the bridge's
-		 * decode range to be decoded.  The PC Card spec seems to
-		 * indicate that this should only be done on x86 based
-		 * machines, which seems to imply that on non-x86 machines
-		 * the adddresses can be anywhere.  This further implies that
-		 * since the hardware can do it on non-x86 machines, it should
-		 * be able to do it on x86 machines.  Therefore, we can and
-		 * should ignore this hint.  Furthermore, the PC Card spec
-		 * recommends always allocating memory above 1MB, contradicting
-		 * the other part of the PC Card spec.
+		 * The PC Card spec says we're only supposed to honor this
+		 * hint when the cardbus bridge is a child of pci0 (the main
+		 * bus).  The PC Card spec seems to indicate that this should
+		 * only be done on x86 based machines, which suggests that on
+		 * non-x86 machines the adddresses can be anywhere.  Since the
+		 * hardware can do it on non-x86 machines, it should be able
+		 * to do it on x86 machines too.  Therefore, we can and should
+		 * ignore this hint.  Furthermore, the PC Card spec recommends
+		 * always allocating memory above 1MB, contradicting the other
+		 * part of the PC Card spec, it seems.  We make note of it,
+		 * but otherwise don't use this information.
 		 *
-		 * NetBSD ignores this bit, but it also ignores the
-		 * prefetchable bit too, so that's not an indication of
-		 * correctness.
+		 * Some Realtek cards have this set in their CIS, but fail
+		 * to actually work when mapped this way, and experience
+		 * has shown ignoring this big to be a wise choice.
+		 *
+		 * XXX We should cite chapter and verse for standard refs.
 		 */
 		if (reg & TPL_BAR_REG_BELOW1MB)
 			dinfo->mbelow1mb |= (1 << PCI_RID2BAR(bar));
-#endif
 	}
 
 	return (0);
diff --git a/sys/dev/cardbus/cardbus_device.c b/sys/dev/cardbus/cardbus_device.c
index c03a18a..a409e0f 100644
--- a/sys/dev/cardbus/cardbus_device.c
+++ b/sys/dev/cardbus/cardbus_device.c
@@ -96,13 +96,17 @@ cardbus_build_cis(device_t cbdev, device_t child, int id,
 	 * CISTPL_END is a special case, it has no length field.
 	 */
 	if (id == CISTPL_END) {
-		if (cis->len + 1 > sizeof(cis->buffer))
+		if (cis->len + 1 > sizeof(cis->buffer)) {
+			cis->len = 0;
 			return (ENOSPC);
+		}
 		cis->buffer[cis->len++] = id;
 		return (0);
 	}
-	if (cis->len + 2 + len > sizeof(cis->buffer))
+	if (cis->len + 2 + len > sizeof(cis->buffer)) {
+		cis->len = 0;
 		return (ENOSPC);
+	}
 	cis->buffer[cis->len++] = id;
 	cis->buffer[cis->len++] = len;
 	for (i = 0; i < len; i++)
@@ -110,6 +114,18 @@ cardbus_build_cis(device_t cbdev, device_t child, int id,
 	return (0);
 }
 
+static int
+cardbus_device_buffer_cis(device_t parent, device_t child)
+{
+	struct cardbus_softc *sc;
+	struct tuple_callbacks cb[] = {
+		{CISTPL_GENERIC, "GENERIC", cardbus_build_cis}
+	};
+
+	sc = device_get_softc(parent);
+	return (cardbus_parse_cis(parent, child, cb, &sc->sc_cis));
+}
+
 static	int
 cardbus_open(struct cdev *dev, int oflags, int devtype, struct thread *td)
 {
@@ -117,9 +133,6 @@ cardbus_open(struct cdev *dev, int oflags, int devtype, struct thread *td)
 	device_t *kids;
 	int cnt, err;
 	struct cardbus_softc *sc;
-	struct tuple_callbacks cb[] = {
-		{CISTPL_GENERIC, "GENERIC", cardbus_build_cis}
-	};
 
 	sc = dev->si_drv1;
 	if (sc->sc_cis_open)
@@ -128,21 +141,17 @@ cardbus_open(struct cdev *dev, int oflags, int devtype, struct thread *td)
 	err = device_get_children(parent, &kids, &cnt);
 	if (err)
 		return err;
+	sc->sc_cis.len = 0;
 	if (cnt == 0) {
 		free(kids, M_TEMP);
 		sc->sc_cis_open++;
-		sc->sc_cis = NULL;
 		return (0);
 	}
 	child = kids[0];
 	free(kids, M_TEMP);
-	sc->sc_cis = malloc(sizeof(*sc->sc_cis), M_TEMP, M_ZERO | M_WAITOK);
-	err = cardbus_parse_cis(parent, child, cb, sc->sc_cis);
-	if (err) {
-		free(sc->sc_cis, M_TEMP);
-		sc->sc_cis = NULL;
+	err = cardbus_device_buffer_cis(parent, child);
+	if (err)
 		return (err);
-	}
 	sc->sc_cis_open++;
 	return (0);
 }
@@ -153,8 +162,6 @@ cardbus_close(struct cdev *dev, int fflags, int devtype, struct thread *td)
 	struct cardbus_softc *sc;
 
 	sc = dev->si_drv1;
-	free(sc->sc_cis, M_TEMP);
-	sc->sc_cis = NULL;
 	sc->sc_cis_open = 0;
 	return (0);
 }
@@ -173,8 +180,8 @@ cardbus_read(struct cdev *dev, struct uio *uio, int ioflag)
 
 	sc = dev->si_drv1;
 	/* EOF */
-	if (sc->sc_cis == NULL || uio->uio_offset > sc->sc_cis->len)
+	if (uio->uio_offset >= sc->sc_cis.len)
 		return (0);
-	return (uiomove(sc->sc_cis->buffer + uio->uio_offset,
-	  MIN(uio->uio_resid, sc->sc_cis->len - uio->uio_offset), uio));
+	return (uiomove(sc->sc_cis.buffer + uio->uio_offset,
+	  MIN(uio->uio_resid, sc->sc_cis.len - uio->uio_offset), uio));
 }
diff --git a/sys/dev/cardbus/cardbusvar.h b/sys/dev/cardbus/cardbusvar.h
index 7132884..ca778c2 100644
--- a/sys/dev/cardbus/cardbusvar.h
+++ b/sys/dev/cardbus/cardbusvar.h
@@ -34,7 +34,6 @@ struct cardbus_devinfo
 	struct pci_devinfo pci;
 	uint8_t        mprefetchable; /* bit mask of prefetchable BARs */
 	uint8_t        mbelow1mb; /* bit mask of BARs which require below 1Mb */
-	uint8_t        ibelow1mb; /* bit mask of BARs which require below 1Mb */
 	uint16_t	mfrid;		/* manufacturer id */
 	uint16_t	prodid;		/* product id */
 	u_int		funcid;		/* function id */
@@ -54,10 +53,10 @@ struct cis_buffer
 
 struct cardbus_softc 
 {
-	/* XXX need mutex XXX */
 	device_t	sc_dev;
+	/* The following fields should in be in struct cardbus_devinfo */
 	struct cdev 	*sc_cisdev;
-	struct cis_buffer *sc_cis;
+	struct cis_buffer sc_cis;
 	int		sc_cis_open;
 };