firmware_class: skip timeout in request_firmware during boot

[Randy Dunlap]

On Wed, 25 Jul 2007 14:38:18 +0200 Javier Pello wrote:

> Hi,
> 
> I have prepared a patch that makes request_firmware skip the usual
> grace period that it gives firmware images to show up if it determines
> that userspace was not notified at all.
> 
> The situation is the following: I have a device that requires firmware
> to be loaded during initialisation. However, the driver is built into
> the kernel, so it tries to initialise before the root filesystem is
> mounted. This results in _request_firmware sending an event to userspace
> and patiently waiting for the firmware image to appear (which it cannot
> do) until the loading_timeout timer kicks in a minute later, ending up
> in -ENOENT. The wait is done synchronously, so the whole kernel is
> stopped in the meanwhile; this means a useless boot delay of one minute.
> 
> The attached patch fixes this by making _request_firmware check whether
> the firmware loading event was succesfully sent to userspace, and skip
> the timeout if it has not. The patch comes in two parts:
> 
> 1. The first part changes kobject_uevent_env in lib/kobject_uevent.c
> to report a failure if both netlink_broadcast (if applicable) and
> call_usermodehelper fail to send the event to userspace. Nothing in
> the kernel seems to care about the return value of kobject_uevent_env,
> so this should not break anything.
> 
> 2. The second part changes _request_firmware in
> drivers/base/firmware_class.c to actually check the return value of
> kobject_uevent and skip the loading_timeout delay if the loading event
> was not delivered to userspace at all.
> 
> The patches apply cleanly against 2.6.22. Any suggestions or feedback
> will be welcome.

Hi,
How is this different from request_firmware_nowait()?


> ==========
> From: Javier Pello <javier.pello at urjc.es>
> 
> kobject_uevent: return an error if event was not delivered to userspace
> 
> Make kobject_uevent_env return an error to the caller if the event was
> not delivered to userspace either via netlink or via uevent_helper.
> 
> Signed-off-by: Javier Pello <javier.pello at urjc.es>
> 
> ---
> diff -u linux-2.6.22/lib/kobject_uevent.c linux-2.6.22-p/lib/kobject_uevent.c
> --- linux-2.6.22/lib/kobject_uevent.c	2007-07-09 01:32:17.000000000 +0200
> +++ linux-2.6.22-p/lib/kobject_uevent.c	2007-07-18 20:17:10.000000000 +0200
> @@ -196,7 +196,8 @@
>  			}
>  
>  			NETLINK_CB(skb).dst_group = 1;
> -			netlink_broadcast(uevent_sock, skb, 0, 1, GFP_KERNEL);
> +			retval = netlink_broadcast(uevent_sock, skb, 0, 1,
> +						GFP_KERNEL);
>  		}
>  	}
>  #endif
> @@ -208,7 +209,15 @@
>  		argv [0] = uevent_helper;
>  		argv [1] = (char *)subsystem;
>  		argv [2] = NULL;
> -		call_usermodehelper (argv[0], argv, envp, 0);
> +#if defined(CONFIG_NET)
> +		if (retval) {
> +			retval = call_usermodehelper (argv[0], argv, envp, 0);
> +		} else {
> +			call_usermodehelper (argv[0], argv, envp, 0);
> +		}
> +#else
> +		retval = call_usermodehelper (argv[0], argv, envp, 0);
> +#endif
>  	}
>  
>  exit:
> 
> ==========
> From: Javier Pello <javier.pello at urjc.es>
> 
> request_firmware: skip timeout if userspace was not notified
> 
> Stop _request_firmware from setting up a timer and waiting for
> the firmware image to appear if kobject_uevent returns an error
> (meaning userspace was not notified at all).
> 
> This prevents useless delays if there is no userspace tool to
> provide the firmware image (eg during boot).
> 
> Signed-off-by: Javier Pello <javier.pello at urjc.es>
> 
> ---
> diff -u linux-2.6.22/drivers/base/firmware_class.c linux-2.6.22-p/drivers/base/firmware_class.c
> --- linux-2.6.22/drivers/base/firmware_class.c	2007-07-09 01:32:17.000000000 +0200
> +++ linux-2.6.22-p/drivers/base/firmware_class.c	2007-07-18 20:06:53.000000000 +0200
> @@ -420,8 +420,12 @@
>  			add_timer(&fw_priv->timeout);
>  		}
>  
> -		kobject_uevent(&f_dev->kobj, KOBJ_ADD);
> -		wait_for_completion(&fw_priv->completion);
> +		retval = kobject_uevent(&f_dev->kobj, KOBJ_ADD);
> +		if (retval) {
> +			fw_load_abort(fw_priv);
> +		} else {
> +			wait_for_completion(&fw_priv->completion);
> +		}

Kernel coding style is no braces on single-command blocks...
(here and above).

>  		set_bit(FW_STATUS_DONE, &fw_priv->status);
>  		del_timer_sync(&fw_priv->timeout);
>  	} else
> ==========

---
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***