[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Bug#996816: hw-detect: check-missing-firmware generates modprobe errors



Package: hw-detect
Version: 1.147
Severity: minor
Tags: d-i patch
X-Debbugs-Cc: kibi@debian.org

Debian Install System Team,

Revisiting "#973733 - RTW88_8821ce module fails to find firmware during
install and must be reloaded":
        https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=973733#61

On Mon Jul 26, 2021, Cyril Brulebois wrote:

> ...
> I've kept reloading the module determined initially, even if it could
> probably be skipped if the actual module name is different: it's likely
> to generate two modprobe errors (once for unloading, once for loading)
> but oh well… I could also have good for the `modprobe -q` option but
> better keep all logs, just in case something strange happens…
> ...
> (A variation would be to move the first modprobe dance below the loop,
> and its execution depend on whether actual_module was ever set.)
> ...

First, thank you for your successful efforts to greatly improve
the Bullseye installation experience!

I have created a variation on that variation, with some improvements:
  1) Sit out the first modprobe dance to avoid generating faux-pas errors.
     This assumes that every driver has a corresponding module.
  2) Rename variables in the remove/reload section for improved clarity:
     module -> driver; driver -> drv_path; actual_module -> module
  3) Remove unused get_module function
  4) Fix UUOC
  5) Change "! -n" to "-z"
  6) Fix comment typo

Tested patch is attached.

Thank you!
Daniel Lewart
Urbana, Illinois
--- check-missing-firmware.orig	2021-07-28 02:05:05.000000000 -0500
+++ check-missing-firmware.sh	2021-10-14 00:00:00.000000000 -0500
@@ -20,23 +20,6 @@
 	log-output -t check-missing-firmware "$@"
 }
 
-# Not all drivers register themselves if firmware is missing; in that
-# case determine the module via the device's modalias.
-get_module () {
-	local devpath=$1
-
-	if [ -d $devpath/driver ]; then
-		# The real path of the destination of the driver/module
-		# symlink should be something like "/sys/module/e100"
-		basename $(readlink -f $devpath/driver/module) || true
-	elif [ -e $devpath/modalias ]; then
-		modalias="$(cat $devpath/modalias)"
-		# Take the last module returned by modprobe
-		modprobe --show-depends "$modalias" 2>/dev/null | \
-			sed -n -e '$s#^.*/\([^.]*\)\.ko.*$#\1#p'
-	fi
-}
-
 # Some modules only try to load firmware once brought up. So bring up and
 # then down any interfaces specified by ethdetect.
 upnics() {
@@ -77,12 +60,12 @@
 		# Transform [foo] into \[foo\] to make it possible to search for
 		# "^$tspattern" (-F for fixed string doesn't play well with ^ to
 		# anchor the pattern on the left):
-		tspattern=$(cat $dmesg_ts | sed 's,\[,\\[,;s,\],\\],')
+		tspattern=$(sed 's,\[,\\[,;s,\],\\],' $dmesg_ts)
 		log "looking at dmesg again, restarting from timestamp: $(cat $dmesg_ts)"
 
 		# Find the line number for the first match, empty if not found:
 		ln=$(grep -n "^$tspattern" $dmesg_file |sed 's/:.*//'|head -n 1)
-		if [ ! -z "$ln" ]; then
+		if [ -n "$ln" ]; then
 			log "timestamp found, truncating dmesg accordingly"
 			sed -i "1,$ln d" $dmesg_file
 		else
@@ -110,7 +93,7 @@
 	modules=""
 	files=""
 
-	# Parse dmesg using a started parttern to detect firmware
+	# Parse dmesg using a started pattern to detect firmware
 	# files the kernel drivers look for (#725714):
 	fwlist=/tmp/check-missing-firmware-dmesg.list
 	get_fresh_dmesg | sed -rn 's/^(\[[^]]*\] )?([^ ]+) [^ ]+: firmware: failed to load ([^ ]+) .*/\2 \3/p' > $fwlist
@@ -288,24 +271,22 @@
 	fi
 
 	# remove and reload modules so they see the new firmware
-	for module in $modules; do
-		if ! nic_is_configured $module; then
-			log "removing and loading kernel module $module"
-			log_output modprobe -r $module || true
-			log_output modprobe $module || true
-
+	for driver in $modules; do
+		if ! nic_is_configured $driver; then
 			# iterate to avoid dealing with multiplicity explicitly:
-			for driver in $(find /sys/bus/*/drivers -name "$module"); do
-				# module name mentioned in dmesg might differ from the actual module
+			for drv_path in $(find /sys/bus/*/drivers -name "$driver"); do
+				# driver name mentioned in dmesg might differ from the module
 				# (rtw_8821ce vs. rtw88_8821ce, see #973733); also beware of the
 				# module symlink, it doesn't always exist:
-				if [ -e "$driver/module" ]; then
-					actual_module=$(basename $(readlink -f "$driver/module"))
-					if [ "$actual_module" != "$module" ]; then
-						log "removing and loading kernel module $actual_module as well (actual module for $module)"
-						log_output modprobe -r $actual_module || true
-						log_output modprobe $actual_module || true
+				if [ -e "$drv_path/module" ]; then
+					module=$(basename $(readlink -f "$drv_path/module"))
+					if [ "$module" = "$driver" ]; then
+						log "removing and loading kernel module $module"
+					else
+						log "removing and loading kernel module $module (driver $driver)"
 					fi
+					log_output modprobe -r $module || true
+					log_output modprobe $module || true
 				fi
 			done
 		fi

Reply to: