cvs commit: src/sys/sys firmware.h src/sys/kern subr_firmware.c

[John Baldwin]

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