Re: [PATCH 4/6] fastboot: make ACPI bus drivers probe asynchronous

[Yong Wang]

On Mon, Jan 05, 2009 at 08:12:39PM -0800, Arjan van de Ven wrote:
> 
> >From 34c47dd38f49e29eb7cd7fa2078efc6b8b258bb9 Mon Sep 17 00:00:00 2001
> From: Arjan van de Ven <arjan@xxxxxxxxxxxxxxx>
> Date: Sun, 4 Jan 2009 05:32:28 -0800
> Subject: [PATCH] fastboot: make ACPI bus drivers probe asynchronous
> 
> the various ACPI bus drivers have non-overlapping devices and can
> each be run asynchronous. Some of the ACPI drivers (especially the
> battery one, but others as well) can take quite a long time to probe.
> 
> Signed-off-by: Arjan van de Ven <arjan@xxxxxxxxxxxxxxx>
> ---
>  drivers/acpi/scan.c |   23 ++++++++++++++++-------
>  1 files changed, 16 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
> index 39b7233..a9e542d 100644
> --- a/drivers/acpi/scan.c
> +++ b/drivers/acpi/scan.c
> @@ -8,6 +8,7 @@
>  #include <linux/acpi.h>
>  #include <linux/signal.h>
>  #include <linux/kthread.h>
> +#include <linux/async.h>
>  
>  #include <acpi/acpi_drivers.h>
>  #include <acpi/acinterp.h>	/* for acpi_ex_eisa_id_to_string() */
> @@ -578,6 +579,19 @@ static int acpi_start_single_object(struct acpi_device *device)
>  	return result;
>  }
>  
> +static void acpi_bus_register_async(void *data, async_cookie_t cookie)
> +{
> +	int ret;
> +	struct acpi_driver *driver = data;
> +	driver->drv.name = driver->name;
> +	driver->drv.bus = &acpi_bus_type;
> +	driver->drv.owner = driver->owner;
> +
> +	async_synchronize_cookie(cookie);
> +
> +	ret = driver_register(&driver->drv);
> +	WARN_ON(ret != 0);
> +}
>  /**
>   * acpi_bus_register_driver - register a driver with the ACPI bus
>   * @driver: driver being registered
> @@ -588,16 +602,11 @@ static int acpi_start_single_object(struct acpi_device *device)
>   */
>  int acpi_bus_register_driver(struct acpi_driver *driver)
>  {
> -	int ret;
>  
>  	if (acpi_disabled)
>  		return -ENODEV;
> -	driver->drv.name = driver->name;
> -	driver->drv.bus = &acpi_bus_type;
> -	driver->drv.owner = driver->owner;
> -
> -	ret = driver_register(&driver->drv);
> -	return ret;
> +	async_schedule(acpi_bus_register_async, driver);
> +	return 0;
>  }
>  
>  EXPORT_SYMBOL(acpi_bus_register_driver);

For ACPI battery, button and thermal drivers whose init funtion immediately ends
right after calling acpi_bus_register_driver, it might be OK. But for other drivers
whose init function has more work to do and depends on acpi_bus_register_driver
having *really* done its job, it is certainly not OK. In the latter case, the
'return 0' in acpi_bus_register_driver is effectively cheating those drivers that
acpi_bus_register_driver has already done its job which is unfortunately not
the case. Example drivers include eeepc_laptop and asus_laptop. At least I am
seeing a kernel oops on eeepc because of that.

-Yong
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/