Date: Mon, 12 Jun 2006 08:41:01 -0400 From: John Baldwin <john@baldwin.cx> To: Ian Dowse <iedowse@freebsd.org> Cc: cvs-src@freebsd.org, src-committers@freebsd.org, cvs-all@freebsd.org Subject: Re: cvs commit: src/sys/sys firmware.h src/sys/kern subr_firmware.c Message-ID: <200606120841.02447.john@baldwin.cx> In-Reply-To: <200606101704.k5AH479k065331@repoman.freebsd.org> References: <200606101704.k5AH479k065331@repoman.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Saturday 10 June 2006 13:04, Ian Dowse wrote:
> iedowse 2006-06-10 17:04:07 UTC
>
> FreeBSD src repository
>
> Modified files:
> sys/sys firmware.h
> sys/kern subr_firmware.c
> Log:
> Keep firmware images on the list until they have been unregistered
> with firmware_unregister(). Previously when the last driver reference
> had been dropped we would clear the list entry under the assumption
> that the firmware module was about to be unloaded, but this was not
> true if the firmware image had been loaded manually with kldload.
>
> This makes it possible to manually kldload firmware images as a
> workaround for drivers such as ipw that attempt to load firmware
> while resuming after a suspend.
>
> Reviewed by: mlaier (an earlier version of the patch)
I ran into additional problems with firmware(9) as well. One problem is that
if you kldload a firmware blob, you can kldunload it even while it is in
use(!) because the awk script doesn't include any error checking in the
mod_event function it generates. I've patched the awk script to fix this and
tested it. With this the kldunload of a firmware module that a driver
currently has a reference to now fails:
Index: fw_stub.awk
===================================================================
RCS file: /usr/cvs/src/sys/tools/fw_stub.awk,v
retrieving revision 1.2
diff -u -r1.2 fw_stub.awk
--- fw_stub.awk 30 Jan 2006 16:32:08 -0000 1.2
+++ fw_stub.awk 2 Jun 2006 16:00:42 -0000
@@ -125,7 +125,8 @@
printc("\nstatic int\n"\
opt_m "_fw_modevent(module_t mod, int type, void *unused)\
{\
- struct firmware *fp;\
+ struct firmware *fp, *parent;\
+ int error;\
switch (type) {\
case MOD_LOAD:");
@@ -136,11 +137,7 @@
# '-', '.' and '/' are converted to '_' by ld/objcopy
gsub(/-|\.|\//, "_", symb);
- if (file_i == 0)
- reg = "\t\tfp = ";
- else
- reg = "\t\t(void)";
-
+ reg = "\t\tfp = ";
reg = reg "firmware_register(\"" short "\", _binary_" symb "_start , ";
reg = reg "(size_t)(_binary_" symb "_end - _binary_" symb "_start), ";
reg = reg version ", ";
@@ -148,21 +145,37 @@
if (file_i == 0)
reg = reg "NULL);";
else
- reg = reg "fp);";
+ reg = reg "parent);";
printc(reg);
+
+ printc("\t\tif (fp == NULL)");
+ printc("\t\t\tgoto fail_" file_i ";");
+ if (file_i == 0)
+ printc("\t\tparent = fp;");
}
-printc("\t\treturn (0);\
- case MOD_UNLOAD:");
+printc("\t\treturn (0);");
+
+for (file_i = num_files - 1; file_i > 0; file_i--) {
+ printc("\tfail_" file_i ":")
+ printc("\t\t(void)firmware_unregister(\"" shortnames[file_i - 1] "\");");
+}
+
+printc("\tfail_0:");
+printc("\t\treturn (ENXIO);");
+
+printc("\tcase MOD_UNLOAD:");
for (file_i = 1; file_i < num_files; file_i++) {
- printc("\t\tfirmware_unregister(\"" shortnames[file_i] "\");");
+ printc("\t\terror = firmware_unregister(\"" shortnames[file_i] "\");");
+ printc("\t\tif (error)");
+ printc("\t\t\treturn (error);");
}
-printc("\t\tfirmware_unregister(\"" shortnames[0] "\");");
+printc("\t\terror = firmware_unregister(\"" shortnames[0] "\");");
-printc("\t\treturn (0);\
+printc("\t\treturn (error);\
}\
return (EINVAL);\
}\
--
John Baldwin
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200606120841.02447.john>