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

[Arjan van de Ven <arjan@infradead.org>]

On Wed, 7 Jan 2009 22:12:56 +0800
Yong Wang <yong.y.wang@linux.intel.com> wrote:

> 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@linux.intel.com>
> > 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@linux.intel.com>
> > ---
> >  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.

ok I've dropped the ACPI part for now; I'll work with you and Len to
see how we can solve the ACPI side, but for now it seems to have too
many issues

> 
> -Yong


-- 
Arjan van de Ven 	Intel Open Source Technology Centre
For development, discussion and tips for power savings, 
visit http://www.lesswatts.org