Re: [PATCH 5/7] ACPI / hotplug: Introduce user space interface for hotplug profiles

["Rafael J. Wysocki" <rjw@sisk.pl>]

On Monday, February 25, 2013 11:13:59 AM Toshi Kani wrote:
> On Sun, 2013-02-17 at 16:24 +0100, Rafael J. Wysocki wrote:
> > From: Rafael J. Wysocki <rafael.j.wysocki@inel.com>
> > 
> > Introduce user space interface for manipulating hotplug profiles
> > associated with ACPI scan handlers.
> > 
> > The interface consists of sysfs directories under
> > /sys/firmware/acpi/hotplug/, one for each hotplug profile, containing
> > attributes allowing user space to manipulate the enabled, uevents,
> > and autoeject fields of the corresponding profile.  In particular,
> > switching the enabled attribute from 'unset' to 'set' will cause
> > the common hotplug notify handler to be installed for all ACPI
> > namespace objects representing devices matching the scan handler
> > associated with the given hotplug profile (and analogously for the
> > converse switch).
> > 
> > Drivers willing to use the new user space interface should add their
> > ACPI scan handlers with the help of new funtion
> > acpi_scan_add_handler_with_hotplug().
> > 
> > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@inel.com>
> > ---
> >  Documentation/ABI/testing/sysfs-firmware-acpi |   42 +++++++++
> >  drivers/acpi/internal.h                       |    9 +
> >  drivers/acpi/scan.c                           |   73 +++++++++++++++
> >  drivers/acpi/sysfs.c                          |  120 ++++++++++++++++++++++++++
> >  include/acpi/acpi_bus.h                       |    7 +
> >  5 files changed, 251 insertions(+)
>  :
> > Index: test/drivers/acpi/scan.c
> > ===================================================================
> > --- test.orig/drivers/acpi/scan.c
> > +++ test/drivers/acpi/scan.c
> > @@ -63,6 +63,19 @@ int acpi_scan_add_handler(struct acpi_sc
> >  	return 0;
> >  }
> >  
> > +int acpi_scan_add_handler_with_hotplug(struct acpi_scan_handler *handler,
> > +				       const char *hotplug_profile_name)
> > +{
> > +	int error;
> > +
> > +	error = acpi_scan_add_handler(handler);
> > +	if (error)
> > +		return error;
> > +
> > +	acpi_sysfs_add_hotplug_profile(&handler->hotplug, hotplug_profile_name);
> > +	return 0;
> > +}
> > +
> >  /*
> >   * Creates hid/cid(s) string needed for modalias and uevent
> >   * e.g. on a device with hid:IBM0001 and cid:ACPI0001 you get:
> > @@ -1675,6 +1688,66 @@ static bool acpi_scan_handler_matching(s
> >  	return false;
> >  }
> >  
> > +static acpi_status acpi_scan_hotplug_modify(acpi_handle handle,
> > +					    u32 lvl_not_used, void *data,
> > +					    void **ret_not_used)
> > +{
> > +	struct acpi_scan_handler *handler = data;
> > +	struct acpi_device_info *info;
> > +	bool match = false;
> > +
> > +	if (ACPI_FAILURE(acpi_get_object_info(handle, &info)))
> > +		return AE_OK;
> > +
> > +	if (info->valid & ACPI_VALID_HID) {
> > +		char *idstr = info->hardware_id.string;
> > +		match = acpi_scan_handler_matching(handler, idstr, NULL);
> > +	}
> > +	kfree(info);
> > +	if (!match)
> > +		return AE_OK;
> > +
> > +	if (handler->hotplug.enabled)
> > +		acpi_remove_notify_handler(handle, ACPI_SYSTEM_NOTIFY,
> > +					   acpi_hotplug_notify_cb);
> > +	else
> > +		acpi_install_notify_handler(handle, ACPI_SYSTEM_NOTIFY,
> > +					    acpi_hotplug_notify_cb, NULL);
> 
> I found that the action for "enabled" attribute is incorrect.  When
> enabled, it should install the notify handler, and remove otherwise.

Well, thanks for spotting this!

Rafael


-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.