Cleanup and document the implementation of firmware(9) based on

a version that i posted earlier on the -current mailing list,
and subsequent feedback received.

The core of the change is just in sys/firmware.h and kern/subr_firmware.c,
while other files are just adaptation of the clients to the ABI change
(const-ification of some parameters and hiding of internal info,
so this is fully compatible at the binary level).

In detail:
- reduce the amount of information exported to clients in struct firmware,
  and constify the pointer;

- internally, document and simplify the implementation of the various
  functions, and make sure error conditions are dealt with properly.

The diffs are large, but the code is really straightforward now (i hope).

Note also that there is a subtle issue with the implementation of
firmware_register(): currently, as in the previous version, we just
store a reference to the 'imagename' argument, but we should rather
copy it because there is no guarantee that this is a static string.
I realised this while testing this code, but i prefer to fix it in
a later commit -- there is no regression with respect to the past.

Note, too, that the version in RELENG_6 has various bugs including
missing locks around the module release calls, mishandling of modules
loaded by /boot/loader, and so on, so an MFC is absolutely necessary
there.  I was just postponing it until this cleanup to avoid doing
things twice.

MFC after: 1 week

10 files changed, 296 insertions, 178 deletions

diff --git a/sys/arm/xscale/ixp425/ixp425_npe.c b/sys/arm/xscale/ixp425/ixp425_npe.c
index 0d1626b..c3e9dbd 100644
--- a/sys/arm/xscale/ixp425/ixp425_npe.c
+++ b/sys/arm/xscale/ixp425/ixp425_npe.c
@@ -422,7 +422,7 @@ ixpnpe_init(struct ixpnpe_softc *sc, const char *imageName, uint32_t imageId)
 {
     uint32_t imageSize;
     const uint32_t *imageCodePtr;
-    struct firmware *fw;
+    const struct firmware *fw;
     int error;
 
     DPRINTF(sc->sc_dev, "load %s, imageId 0x%08x\n", imageName, imageId);
diff --git a/sys/dev/ipw/if_ipw.c b/sys/dev/ipw/if_ipw.c
index 69761f6..7534949 100644
--- a/sys/dev/ipw/if_ipw.c
+++ b/sys/dev/ipw/if_ipw.c
@@ -1956,7 +1956,7 @@ ipw_init(void *priv)
 	struct ipw_softc *sc = priv;
 	struct ieee80211com *ic = &sc->sc_ic;
 	struct ifnet *ifp = ic->ic_ifp;
-	struct firmware *fp;
+	const struct firmware *fp;
 	const struct ipw_firmware_hdr *hdr;
 	const char *imagename, *fw;
 	int owned;
diff --git a/sys/dev/ipw/if_ipwvar.h b/sys/dev/ipw/if_ipwvar.h
index 3cfb77e..c97d6a8 100644
--- a/sys/dev/ipw/if_ipwvar.h
+++ b/sys/dev/ipw/if_ipwvar.h
@@ -99,7 +99,7 @@ struct ipw_softc {
 	bus_space_tag_t			sc_st;
 	bus_space_handle_t		sc_sh;
 	void 				*sc_ih;
-	struct firmware			*sc_firmware;
+	const struct firmware		*sc_firmware;
 
 	int				sc_tx_timer;
 
diff --git a/sys/dev/isp/isp_freebsd.h b/sys/dev/isp/isp_freebsd.h
index c9868ab..c8247e8 100644
--- a/sys/dev/isp/isp_freebsd.h
+++ b/sys/dev/isp/isp_freebsd.h
@@ -166,7 +166,7 @@ struct isposinfo {
 	struct callout_handle 	ldt;	/* loop down timer */
 	struct callout_handle	gdt;	/* gone device timer */
 #if __FreeBSD_version >= 500000  
-	struct firmware *	fw;
+	const struct firmware *	fw;
 	struct mtx		lock;
 	struct cv		kthread_cv;
 	union {
diff --git a/sys/dev/iwi/if_iwi.c b/sys/dev/iwi/if_iwi.c
index bf582aa..1f7f573 100644
--- a/sys/dev/iwi/if_iwi.c
+++ b/sys/dev/iwi/if_iwi.c
@@ -2169,7 +2169,7 @@ iwi_reset(struct iwi_softc *sc)
 static const struct iwi_firmware_ohdr *
 iwi_setup_ofw(struct iwi_softc *sc, struct iwi_fw *fw)
 {
-	struct firmware *fp = fw->fp;
+	const struct firmware *fp = fw->fp;
 	const struct iwi_firmware_ohdr *hdr;
 
 	if (fp->datasize < sizeof (struct iwi_firmware_ohdr)) {
@@ -2234,7 +2234,7 @@ iwi_get_firmware(struct iwi_softc *sc)
 {
 	struct ieee80211com *ic = &sc->sc_ic;
 	const struct iwi_firmware_hdr *hdr;
-	struct firmware *fp;
+	const struct firmware *fp;
 
 	/* invalidate cached firmware on mode change */
 	if (sc->fw_mode != ic->ic_opmode)
diff --git a/sys/dev/iwi/if_iwivar.h b/sys/dev/iwi/if_iwivar.h
index 3884a3a..6b5571e 100644
--- a/sys/dev/iwi/if_iwivar.h
+++ b/sys/dev/iwi/if_iwivar.h
@@ -108,7 +108,7 @@ struct iwi_node {
 };
 
 struct iwi_fw {
-	struct firmware		*fp;		/* image handle */
+	const struct firmware	*fp;		/* image handle */
 	const char		*data;		/* firmware image data */
 	size_t			size;		/* firmware image size */
 	const char		*name;		/* associated image name */
diff --git a/sys/dev/mxge/if_mxge.c b/sys/dev/mxge/if_mxge.c
index be8ab0e..52e64f0 100644
--- a/sys/dev/mxge/if_mxge.c
+++ b/sys/dev/mxge/if_mxge.c
@@ -512,7 +512,7 @@ mxge_validate_firmware(mxge_softc_t *sc, const mcp_gen_header_t *hdr)
 static int
 mxge_load_firmware_helper(mxge_softc_t *sc, uint32_t *limit)
 {
-	struct firmware *fw;
+	const struct firmware *fw;
 	const mcp_gen_header_t *hdr;
 	unsigned hdr_offset;
 	const char *fw_data;
diff --git a/sys/kern/subr_firmware.c b/sys/kern/subr_firmware.c
index 29964c4..388a331 100644
--- a/sys/kern/subr_firmware.c
+++ b/sys/kern/subr_firmware.c
@@ -42,40 +42,144 @@ __FBSDID("$FreeBSD$");
 #include <sys/proc.h>
 #include <sys/module.h>
 
+/*
+ * Loadable firmware support. See sys/sys/firmware.h and firmware(9)
+ * form more details on the subsystem.
+ *
+ * 'struct firmware' is the user-visible part of the firmware table.
+ * Additional internal information is stored in a 'struct priv_fw'
+ * (currently a static array). A slot is in use if FW_INUSE is true:
+ */
+
+#define FW_INUSE(p)	((p)->file != NULL || (p)->fw.name != NULL)
+
+/*
+ * fw.name != NULL when an image is registered; file != NULL for
+ * autoloaded images whose handling has not been completed.
+ *
+ * The state of a slot evolves as follows:
+ *	firmware_register	-->  fw.name = image_name
+ *	(autoloaded image)	-->  file = module reference
+ *	firmware_unregister	-->  fw.name = NULL
+ *	(unloadentry complete)	-->  file = NULL
+ *
+ * In order for the above to work, the 'file' field must remain
+ * unchanged in firmware_unregister().
+ *
+ * Images residing in the same module are linked to each other
+ * through the 'parent' argument of firmware_register().
+ * One image (typically, one with the same name as the module to let
+ * the autoloading mechanism work) is considered the parent image for
+ * all other images in the same module. Children affect the refcount
+ * on the parent image preventing improper unloading of the image itself.
+ */
+
+struct priv_fw {
+	int		refcnt;		/* reference count */
+
+	/*
+	 * parent entry, see above. Set on firmware_register(),
+	 * cleared on firmware_unregister().
+	 */
+	struct priv_fw	*parent;
+
+	int 		flags;	/* record FIRMWARE_UNLOAD requests */
+#define FW_UNLOAD	0x100
+
+	/*
+	 * 'file' is private info managed by the autoload/unload code.
+	 * Set at the end of firmware_get(), cleared only in the
+	 * firmware_task, so the latter can depend on its value even
+	 * while the lock is not held.
+	 */
+	linker_file_t   file;	/* module file, if autoloaded */
+
+	/*
+	 * 'fw' is the externally visible image information.
+	 * We do not make it the first field in priv_fw, to avoid the
+	 * temptation of casting pointers to each other.
+	 * Use PRIV_FW(fw) to get a pointer to the cointainer of fw.
+	 * Beware, PRIV_FW does not work for a NULL pointer.
+	 */
+	struct firmware	fw;	/* externally visible information */
+};
+
+/*
+ * PRIV_FW returns the pointer to the container of struct firmware *x.
+ * Cast to intptr_t to override the 'const' attribute of x
+ */
+#define PRIV_FW(x)	((struct priv_fw *)		\
+	((intptr_t)(x) - offsetof(struct priv_fw, fw)) )
+
+/*
+ * At the moment we use a static array as backing store for the registry.
+ * Should we move to a dynamic structure, keep in mind that we cannot
+ * reallocate the array because pointers are held externally.
+ * A list may work, though.
+ */
 #define	FIRMWARE_MAX	30
-static struct firmware firmware_table[FIRMWARE_MAX];
+static struct priv_fw firmware_table[FIRMWARE_MAX];
+
+/*
+ * module release are handled in a separate task as they might sleep.
+ */
 struct task firmware_task;
+
+/*
+ * This mutex protects accesses to the firmware table.
+ */
 struct mtx firmware_mtx;
 MTX_SYSINIT(firmware, &firmware_mtx, "firmware table", MTX_DEF);
 
 /*
+ * Helper function to lookup a name.
+ * As a side effect, it sets the pointer to a free slot, if any.
+ * This way we can concentrate most of the registry scanning in
+ * this function, which makes it easier to replace the registry
+ * with some other data structure.
+ */
+static struct priv_fw *
+lookup(const char *name, struct priv_fw **empty_slot)
+{
+	struct priv_fw *fp = NULL;
+	struct priv_fw *dummy;
+	int i;
+
+	if (empty_slot == NULL)
+		empty_slot = &dummy;
+	*empty_slot = NULL;
+	for (i = 0; i < FIRMWARE_MAX; i++) {
+		fp = &firmware_table[i];
+		if (fp->fw.name != NULL && strcasecmp(name, fp->fw.name) == 0)
+			break;
+		else if (!FW_INUSE(fp))
+			*empty_slot = fp;
+	}
+	return (i < FIRMWARE_MAX ) ? fp : NULL;
+}
+
+/*
  * Register a firmware image with the specified name.  The
  * image name must not already be registered.  If this is a
  * subimage then parent refers to a previously registered
  * image that this should be associated with.
  */
-struct firmware *
+const struct firmware *
 firmware_register(const char *imagename, const void *data, size_t datasize,
-    unsigned int version, struct firmware *parent)
+    unsigned int version, const struct firmware *parent)
 {
-	struct firmware *frp = NULL;
-	int i;
+	struct priv_fw *match, *frp;
 
 	mtx_lock(&firmware_mtx);
-	for (i = 0; i < FIRMWARE_MAX; i++) {
-		struct firmware *fp = &firmware_table[i];
-
-		if (fp->name == NULL) {
-			if (frp == NULL)
-				frp = fp;
-			continue;
-		}
-		if (strcasecmp(imagename, fp->name) == 0) {
-			mtx_unlock(&firmware_mtx);
-			printf("%s: image %s already registered!\n",
-				__func__, imagename);
-			return NULL;
-		}
+	/*
+	 * Do a lookup to make sure the name is unique or find a free slot.
+	 */
+	match = lookup(imagename, &frp);
+	if (match != NULL) {
+		mtx_unlock(&firmware_mtx);
+		printf("%s: image %s already registered!\n",
+			__func__, imagename);
+		return NULL;
 	}
 	if (frp == NULL) {
 		mtx_unlock(&firmware_mtx);
@@ -83,53 +187,20 @@ firmware_register(const char *imagename, const void *data, size_t datasize,
 		    __func__, imagename);
 		return NULL;
 	}
-	frp->name = imagename;
-	frp->data = data;
-	frp->datasize = datasize;
-	frp->version = version;
-	frp->refcnt = 0;
-	frp->flags = 0;
-	if (parent != NULL)
-		parent->refcnt++;
-	frp->parent = parent;
-	frp->file = NULL;
+	bzero(frp, sizeof(frp));	/* start from a clean record */
+	frp->fw.name = imagename;
+	frp->fw.data = data;
+	frp->fw.datasize = datasize;
+	frp->fw.version = version;
+	if (parent != NULL) {
+		frp->parent = PRIV_FW(parent);
+		frp->parent->refcnt++;
+	}
 	mtx_unlock(&firmware_mtx);
-
 	if (bootverbose)
 		printf("firmware: '%s' version %u: %zu bytes loaded at %p\n",
 		    imagename, version, datasize, data);
-
-	return frp;
-}
-
-static void
-clearentry(struct firmware *fp)
-{
-	KASSERT(fp->refcnt == 0, ("image %s refcnt %u", fp->name, fp->refcnt));
-	fp->name = NULL;
-	fp->file = NULL;
-	fp->data = NULL;
-	fp->datasize = 0;
-	fp->version = 0;
-	fp->flags = 0;
-	if (fp->parent != NULL) {	/* release parent reference */
-		fp->parent->refcnt--;
-		fp->parent = NULL;
-	}
-}
-
-static struct firmware *
-lookup(const char *name)
-{
-	struct firmware *fp;
-	int i;
-
-	for (i = 0; i < FIRMWARE_MAX; i++) {
-		fp = &firmware_table[i];
-		if (fp->name != NULL && strcasecmp(name, fp->name) == 0)
-			return fp;
-	}
-	return NULL;
+	return &frp->fw;
 }
 
 /*
@@ -140,62 +211,61 @@ lookup(const char *name)
 int
 firmware_unregister(const char *imagename)
 {
-	struct firmware *fp;
-	int refcnt = 0;
+	struct priv_fw *fp;
+	int err;
 
 	mtx_lock(&firmware_mtx);
-	/*
-	 * NB: it is ok for the lookup to fail; this can happen
-	 * when a module is unloaded on last reference and the
-	 * module unload handler unregister's each of it's
-	 * firmware images.
-	 */
-	fp = lookup(imagename);
-	if (fp != NULL) {
-		refcnt = fp->refcnt;
-		if (refcnt == 0)
-			clearentry(fp);
+	fp = lookup(imagename, NULL);
+	if (fp == NULL) {
+		/*
+		 * It is ok for the lookup to fail; this can happen
+		 * when a module is unloaded on last reference and the
+		 * module unload handler unregister's each of it's
+		 * firmware images.
+		 */
+		err = 0;
+	} else if (fp->refcnt != 0) {	/* cannot unregister */
+		err = EBUSY;
+	}  else {
+		linker_file_t   x = fp->file;	/* save value */
+
+		if (fp->parent != NULL)	/* release parent reference */
+			fp->parent->refcnt--;
+		/*
+		 * Clear the whole entry with bzero to make sure we
+		 * do not forget anything. Then restore 'file' which is
+		 * non-null for autoloaded images.
+		 */
+		bzero(fp, sizeof(struct priv_fw));
+		fp->file = x;
+		err = 0;
 	}
 	mtx_unlock(&firmware_mtx);
-	return (refcnt != 0 ? EBUSY : 0);
+	return err;
 }
 
 /*
  * Lookup and potentially load the specified firmware image.
- * If the firmware is not found in the registry attempt to
- * load a kernel module with the image name.  If the firmware
- * is located a reference is returned.  The caller must release
- * this reference for the image to be eligible for removal/unload.
+ * If the firmware is not found in the registry, try to load a kernel
+ * module named as the image name.
+ * If the firmware is located, a reference is returned. The caller must
+ * release this reference for the image to be eligible for removal/unload.
  */
-struct firmware *
+const struct firmware *
 firmware_get(const char *imagename)
 {
 	struct thread *td;
-	struct firmware *fp;
+	struct priv_fw *fp;
 	linker_file_t result;
-	int requested_load = 0;
 
-again:
 	mtx_lock(&firmware_mtx);
-	fp = lookup(imagename);
-	if (fp != NULL) {
-		if (requested_load)
-			fp->file = result;
-		fp->refcnt++;
-		mtx_unlock(&firmware_mtx);
-		return fp;
-	}
+	fp = lookup(imagename, NULL);
+	if (fp != NULL)
+		goto found;
 	/*
-	 * Image not present, try to load the module holding it
-	 * or if we already tried give up.
+	 * Image not present, try to load the module holding it.
 	 */
 	mtx_unlock(&firmware_mtx);
-	if (requested_load) {
-		printf("%s: failed to load firmware image %s\n",
-		    __func__, imagename);
-		(void) linker_release_module(imagename, NULL, NULL);
-		return NULL;
-	}
 	td = curthread;
 	if (priv_check(td, PRIV_FIRMWARE_LOAD) != 0 ||
 	    securelevel_gt(td->td_ucred, 0) != 0) {
@@ -204,69 +274,105 @@ again:
 		return NULL;
 	}
 	(void) linker_reference_module(imagename, NULL, &result);
-	requested_load = 1;
-	goto again;		/* sort of an Algol-style for loop */
-}
-
-static void
-unloadentry(void *unused1, int unused2)
-{
-	struct firmware *fp;
-	linker_file_t file;
-	int i, err;
-
+	/*
+	 * After loading the module, see if the image is registered now.
+	 */
 	mtx_lock(&firmware_mtx);
-	for (;;) {
-		/* Look for an unwanted entry that we explicitly loaded. */
-		for (i = 0; i < FIRMWARE_MAX; i++) {
-			fp = &firmware_table[i];
-			if (fp->name != NULL && fp->file != NULL &&
-			    fp->refcnt == 0 &&
-			    (fp->flags & FIRMWAREFLAG_KEEPKLDREF) == 0)
-				break;
-			fp = NULL;
-		}
-		if (fp == NULL)
-			break;
-		file = fp->file;
-		/* No longer explicitly loaded. */
-		fp->file = NULL;
+	fp = lookup(imagename, NULL);
+	if (fp == NULL) {
 		mtx_unlock(&firmware_mtx);
-
-		err = linker_release_module(NULL, NULL, file);
-
-		mtx_lock(&firmware_mtx);
-		if (err) {
-			/*
-			 * If linker_release_module() failed then we still
-			 * hold a reference on the module so it should not be
-			 * possible for it to go away or be re-registered.
-			 */
-			KASSERT(fp->file == NULL,
-			    ("firmware entry reused while referenced!"));
-			fp->file = file;
-		}
+		printf("%s: failed to load firmware image %s\n",
+			__func__, imagename);
+		(void) linker_release_module(imagename, NULL, NULL);
+		return NULL;
 	}
+	fp->file = result;	/* record the module identity */
+
+found:				/* common exit point on success */
+	fp->refcnt++;
 	mtx_unlock(&firmware_mtx);
+	return &fp->fw;
 }
 
 /*
- * Release a reference to a firmware image returned by
- * firmware_get.  The reference is released and if this is
- * the last reference to the firmware image the associated
- * module may be released/unloaded.
+ * Release a reference to a firmware image returned by firmware_get.
+ * The caller may specify, with the FIRMWARE_UNLOAD flag, its desire
+ * to release the resource, but the flag is only advisory.
+ *
+ * If this is the last reference to the firmware image, and this is an
+ * autoloaded module, wake up the firmware_task to figure out what to do
+ * with the associated module.
  */
 void
-firmware_put(struct firmware *fp, int flags)
+firmware_put(const struct firmware *p, int flags)
 {
+	struct priv_fw *fp = PRIV_FW(p);
+
 	mtx_lock(&firmware_mtx);
 	fp->refcnt--;
 	if (fp->refcnt == 0) {
-		if ((flags & FIRMWARE_UNLOAD) == 0)
-			fp->flags |= FIRMWAREFLAG_KEEPKLDREF;
+		if (flags & FIRMWARE_UNLOAD)
+			fp->flags |= FW_UNLOAD;
+		if (fp->file)
+			taskqueue_enqueue(taskqueue_thread, &firmware_task);
+	}
+	mtx_unlock(&firmware_mtx);
+}
+
+/*
+ * The body of the task in charge of unloading autoloaded modules
+ * that are not needed anymore.
+ * Images can be cross-linked so we may need to make multiple passes,
+ * but the time we spend in the loop is bounded because we clear entries
+ * as we touch them.
+ */
+static void
+unloadentry(void *unused1, int unused2)
+{
+	int limit = FIRMWARE_MAX;
+	int i;	/* current cycle */
+
+	mtx_lock(&firmware_mtx);
+	/*
+	 * Scan the table. limit is set to make sure we make another
+	 * full sweep after matching an entry that requires unloading.
+	 */
+	for (i = 0; i < limit; i++) {
+		struct priv_fw *fp;
+		int err;
+
+		fp = &firmware_table[i % FIRMWARE_MAX];
+		if (fp->fw.name == NULL || fp->file == NULL ||
+		    fp->refcnt != 0 || (fp->flags & FW_UNLOAD) == 0)
+			continue;
+
+		/*
+		 * Found an entry. Now:
+		 * 1. bump up limit to make sure we make another full round;
+		 * 2. clear FW_UNLOAD so we don't try this entry again.
+		 * 3. release the lock while trying to unload the module.
+		 * 'file' remains set so that the entry cannot be reused
+		 * in the meantime (it also means that fp->file will
+		 * not change while we release the lock).
+		 */
+		limit = i + FIRMWARE_MAX;	/* make another full round */
+		fp->flags &= ~FW_UNLOAD;	/* do not try again */
+
+		mtx_unlock(&firmware_mtx);
+		err = linker_release_module(NULL, NULL, fp->file);
+		mtx_lock(&firmware_mtx);
+
+		/*
+		 * We rely on the module to call firmware_unregister()
+		 * on unload to actually release the entry.
+		 * If err = 0 we can drop our reference as the system
+		 * accepted it. Otherwise unloading failed (e.g. the
+		 * module itself gave an error) so our reference is
+		 * still valid.
+		 */
+		if (err == 0)
+			fp->file = NULL; 
 	}
-	if (fp->file)
-		taskqueue_enqueue(taskqueue_thread, &firmware_task);
 	mtx_unlock(&firmware_mtx);
 }
 
@@ -276,21 +382,34 @@ firmware_put(struct firmware *fp, int flags)
 static int
 firmware_modevent(module_t mod, int type, void *unused)
 {
-	struct firmware *fp;
-	int i;
+	struct priv_fw *fp;
+	int i, err = EINVAL;
 
 	switch (type) {
 	case MOD_LOAD:
 		TASK_INIT(&firmware_task, 0, unloadentry, NULL);
 		return 0;
+
 	case MOD_UNLOAD:
+		/* request all autoloaded modules to be released */
+		mtx_lock(&firmware_mtx);
 		for (i = 0; i < FIRMWARE_MAX; i++) {
 			fp = &firmware_table[i];
-			fp->flags &= ~FIRMWAREFLAG_KEEPKLDREF;
+			fp->flags |= FW_UNLOAD;;
 		}
+		mtx_unlock(&firmware_mtx);
 		taskqueue_enqueue(taskqueue_thread, &firmware_task);
 		taskqueue_drain(taskqueue_thread, &firmware_task);
-		return 0;
+		for (i = 0; i < FIRMWARE_MAX; i++) {
+			fp = &firmware_table[i];
+			if (fp->fw.name != NULL) {
+				printf("%s: image %p ref %d still active slot %d\n",
+					__func__, fp->fw.name,
+					fp->refcnt,  i);
+				err = EINVAL;
+			}
+		}
+		return err;
 	}
 	return EINVAL;
 }
diff --git a/sys/sys/firmware.h b/sys/sys/firmware.h
index c199450..ce02831 100644
--- a/sys/sys/firmware.h
+++ b/sys/sys/firmware.h
@@ -30,36 +30,35 @@
 /*
  * Loadable firmware support.
  *
- * Firmware images are embedded in kernel loadable modules that can
+ * The firmware abstraction provides an interface for loading firmware
+ * images into the kernel and making them available to clients.
+ *
+ * Firmware images are usually embedded in kernel loadable modules that can
  * be loaded on-demand or pre-loaded as desired.  Modules may contain
  * one or more firmware images that are stored as opaque data arrays
- * and registered with a unique string name.  Consumers request
- * firmware by name with held references counted to use in disallowing
+ * and registered with a unique string name. Clients request
+ * firmware by name, and are returned a struct firmware * below on success.
+ * The kernel keeps track of references to firmware images to allow/prevent
  * module/data unload.
  *
- * When multiple images are stored in one module the one image is
+ * When multiple images are stored in one module, the first image is
  * treated as the master with the other images holding references
  * to it.  This means that to unload the module each dependent/subimage
  * must first have its references removed.
+ * In order for automatic loading to work, the master image must have
+ * the same name as the module it is embedded into.
  */
 struct firmware {
 	const char	*name;		/* system-wide name */
 	const void	*data;		/* location of image */
 	size_t		 datasize;	/* size of image in bytes */
 	unsigned int	 version;	/* version of the image */
-	int		 refcnt;	/* held references */
-	struct firmware *parent;	/* not null if a subimage */
-	linker_file_t	 file;		/* loadable module */
-	int		 flags;		/* FIRMWAREFLAG_ flags */
 };
 
-/* "flags" field definitions */
-#define FIRMWAREFLAG_KEEPKLDREF	0x0001	/* don't release KLD reference */
-
-struct firmware	*firmware_register(const char *, const void *, size_t,
-    unsigned int, struct firmware *);
-int		 firmware_unregister(const char *);
-struct firmware *firmware_get(const char *);
+const struct firmware	*firmware_register(const char *,
+	const void *, size_t, unsigned int, const struct firmware *);
+int	 firmware_unregister(const char *);
+const struct firmware *firmware_get(const char *);
 #define	FIRMWARE_UNLOAD		0x0001	/* unload if unreferenced */
-void		 firmware_put(struct firmware *, int);
+void		 firmware_put(const struct firmware *, int);
 #endif /* _SYS_FIRMWARE_H_ */
diff --git a/sys/tools/fw_stub.awk b/sys/tools/fw_stub.awk
index 3e4463e..40be8ad 100644
--- a/sys/tools/fw_stub.awk
+++ b/sys/tools/fw_stub.awk
@@ -125,7 +125,7 @@ for (file_i = 0; file_i < num_files; file_i++) {
 printc("\nstatic int\n"\
 opt_m "_fw_modevent(module_t mod, int type, void *unused)\
 {\
-	struct firmware *fp, *parent;\
+	const struct firmware *fp, *parent;\
 	int error;\
 	switch (type) {\
 	case MOD_LOAD:");