[PATCH 00/10] ACPI: add device .notify methods

[PATCH 00/10] ACPI: add device .notify methods

[Bjorn Helgaas]

I'm trying to simplify ACPI drivers by reducing their dependence
on internal ACPI CA details.  One possibility seems to be moving
notify handler management from individual drivers into the
Linux/ACPI driver registration code.

These patches add support for an ACPI driver .notify() method.
If one is present, the Linux/ACPI code installs a generic notify
handler when the driver binds to a device.  The generic handler
passes the event on to the driver's .notify() method.  The
advantage is that the driver need not install or remove the
notify handler, and the .notify() method works in terms of a
struct acpi_device rather than an acpi_handle.

Note that these patches only affect device notifications, not
system notifications.  System notifications are primarily
bus-related events for things like hot-plug and wakeup.  The
device notifications are generally driver-specific things like
"device status changed," "button pressed," or "some method
needs to be re-evaluated."

Comments welcome.

Bjorn

---

Bjorn Helgaas (10):
      ACPI: WMI: use .notify method instead of installing handler directly
      sony-laptop: use .notify method instead of installing handler directly
      panasonic-laptop: use .notify method instead of installing handler directly
      fujitsu-laptop: use .notify method instead of installing hotkey handler directly
      fujitsu-laptop: use .notify method instead of installing handler directly
      ACPI: video: use .notify method instead of installing handler directly
      ACPI: thermal: use .notify method instead of installing handler directly
      ACPI: processor: use .notify method instead of installing handler directly
      ACPI: button: use .notify method instead of installing handler directly
      ACPI: support acpi_device_ops .notify methods


 drivers/acpi/button.c                   |   77 +++----------------------------
 drivers/acpi/processor_core.c           |   19 ++------
 drivers/acpi/scan.c                     |   71 +++++++++++++++++++++++++++++
 drivers/acpi/thermal.c                  |   27 ++---------
 drivers/acpi/video.c                    |   30 ++----------
 drivers/platform/x86/fujitsu-laptop.c   |   60 ++++--------------------
 drivers/platform/x86/panasonic-laptop.c |   26 ++--------
 drivers/platform/x86/sony-laptop.c      |   32 ++-----------
 drivers/platform/x86/wmi.c              |   15 +-----
 include/acpi/acpi_bus.h                 |    2 +
 include/acpi/acpi_drivers.h             |   10 ++++
 11 files changed, 126 insertions(+), 243 deletions(-)

[PATCH 01/10] ACPI: support acpi_device_ops .notify methods

[Bjorn Helgaas]

This patch adds support for ACPI device driver .notify() methods.  If
such a method is present, Linux/ACPI installs a handler for device
notifications (but not for system notifications such as Bus Check,
Device Check, etc).  When a device notification occurs, Linux/ACPI
passes it on to the driver's .notify() method.

In most cases, this removes the need for drivers to install their own
handlers for device-specific notifications.

For fixed hardware devices like some power and sleep buttons, there's
no notification value because there's no control method to execute a
Notify opcode.  When a fixed hardware device generates an event, we
handle it the same as a regular device notification, except we send
a ACPI_FIXED_HARDWARE_EVENT value.  This is outside the normal 0x0-0xff
range used by Notify opcodes.

Several drivers install their own handlers for system Bus Check and
Device Check notifications so they can support hot-plug.  This patch
doesn't affect that usage.

Signed-off-by: Bjorn Helgaas <bjorn.helgaas@hp.com>
Reviewed-by: Alex Chiang <achiang@hp.com>
---
 drivers/acpi/scan.c         |   71 +++++++++++++++++++++++++++++++++++++++++++
 include/acpi/acpi_bus.h     |    2 +
 include/acpi/acpi_drivers.h |   10 ++++++
 3 files changed, 83 insertions(+), 0 deletions(-)

diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
index b7308ef..20c23c0 100644
--- a/drivers/acpi/scan.c
+++ b/drivers/acpi/scan.c
@@ -359,6 +359,61 @@ static int acpi_device_uevent(struct device *dev, struct kobj_uevent_env *env)
 	return 0;
 }
 
+static void acpi_device_notify(acpi_handle handle, u32 event, void *data)
+{
+	struct acpi_device *device = data;
+
+	device->driver->ops.notify(device, event);
+}
+
+static acpi_status acpi_device_notify_fixed(void *data)
+{
+	struct acpi_device *device = data;
+
+	acpi_device_notify(device->handle, ACPI_FIXED_HARDWARE_EVENT, device);
+	return AE_OK;
+}
+
+static int acpi_device_install_notify_handler(struct acpi_device *device)
+{
+	acpi_status status;
+	char *hid;
+
+	hid = acpi_device_hid(device);
+	if (!strcmp(hid, ACPI_BUTTON_HID_POWERF))
+		status =
+		    acpi_install_fixed_event_handler(ACPI_EVENT_POWER_BUTTON,
+						     acpi_device_notify_fixed,
+						     device);
+	else if (!strcmp(hid, ACPI_BUTTON_HID_SLEEPF))
+		status =
+		    acpi_install_fixed_event_handler(ACPI_EVENT_SLEEP_BUTTON,
+						     acpi_device_notify_fixed,
+						     device);
+	else
+		status = acpi_install_notify_handler(device->handle,
+						     ACPI_DEVICE_NOTIFY,
+						     acpi_device_notify,
+						     device);
+
+	if (ACPI_FAILURE(status))
+		return -EINVAL;
+	return 0;
+}
+
+static void acpi_device_remove_notify_handler(struct acpi_device *device)
+{
+	if (!strcmp(acpi_device_hid(device), ACPI_BUTTON_HID_POWERF))
+		acpi_remove_fixed_event_handler(ACPI_EVENT_POWER_BUTTON,
+						acpi_device_notify_fixed);
+	else if (!strcmp(acpi_device_hid(device), ACPI_BUTTON_HID_SLEEPF))
+		acpi_remove_fixed_event_handler(ACPI_EVENT_SLEEP_BUTTON,
+						acpi_device_notify_fixed);
+	else
+		acpi_remove_notify_handler(device->handle, ACPI_DEVICE_NOTIFY,
+					   acpi_device_notify);
+}
+
 static int acpi_bus_driver_init(struct acpi_device *, struct acpi_driver *);
 static int acpi_start_single_object(struct acpi_device *);
 static int acpi_device_probe(struct device * dev)
@@ -371,6 +426,20 @@ static int acpi_device_probe(struct device * dev)
 	if (!ret) {
 		if (acpi_dev->bus_ops.acpi_op_start)
 			acpi_start_single_object(acpi_dev);
+
+		if (acpi_drv->ops.notify) {
+			ret = acpi_device_install_notify_handler(acpi_dev);
+			if (ret) {
+				if (acpi_drv->ops.stop)
+					acpi_drv->ops.stop(acpi_dev,
+						   acpi_dev->removal_type);
+				if (acpi_drv->ops.remove)
+					acpi_drv->ops.remove(acpi_dev,
+						     acpi_dev->removal_type);
+				return ret;
+			}
+		}
+
 		ACPI_DEBUG_PRINT((ACPI_DB_INFO,
 			"Found driver [%s] for device [%s]\n",
 			acpi_drv->name, acpi_dev->pnp.bus_id));
@@ -385,6 +454,8 @@ static int acpi_device_remove(struct device * dev)
 	struct acpi_driver *acpi_drv = acpi_dev->driver;
 
 	if (acpi_drv) {
+		if (acpi_drv->ops.notify)
+			acpi_device_remove_notify_handler(acpi_dev);
 		if (acpi_drv->ops.stop)
 			acpi_drv->ops.stop(acpi_dev, acpi_dev->removal_type);
 		if (acpi_drv->ops.remove)
diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
index 08ec60c..a222851 100644
--- a/include/acpi/acpi_bus.h
+++ b/include/acpi/acpi_bus.h
@@ -95,6 +95,7 @@ typedef int (*acpi_op_suspend) (struct acpi_device * device,
 typedef int (*acpi_op_resume) (struct acpi_device * device);
 typedef int (*acpi_op_bind) (struct acpi_device * device);
 typedef int (*acpi_op_unbind) (struct acpi_device * device);
+typedef void (*acpi_op_notify) (struct acpi_device * device, u32 event);
 
 struct acpi_bus_ops {
 	u32 acpi_op_add:1;
@@ -110,6 +111,7 @@ struct acpi_device_ops {
 	acpi_op_resume resume;
 	acpi_op_bind bind;
 	acpi_op_unbind unbind;
+	acpi_op_notify notify;
 };
 
 struct acpi_driver {
diff --git a/include/acpi/acpi_drivers.h b/include/acpi/acpi_drivers.h
index 241d227..0352c8f 100644
--- a/include/acpi/acpi_drivers.h
+++ b/include/acpi/acpi_drivers.h
@@ -67,6 +67,16 @@
 #define ACPI_BAY_HID			"LNXIOBAY"
 #define ACPI_DOCK_HID			"LNXDOCK"
 
+/*
+ * For fixed hardware buttons, we fabricate acpi_devices with HID
+ * ACPI_BUTTON_HID_POWERF or ACPI_BUTTON_HID_SLEEPF.  Fixed hardware
+ * signals only an event; it doesn't supply a notification value.
+ * To allow drivers to treat notifications from fixed hardware the
+ * same as those from real devices, we turn the events into this
+ * notification value.
+ */
+#define ACPI_FIXED_HARDWARE_EVENT	0x100
+
 /* --------------------------------------------------------------------------
                                        PCI
    -------------------------------------------------------------------------- */

Re: [PATCH 01/10] ACPI: support acpi_device_ops .notify methods

[Thomas Renninger]

Hi Bjorn,

On Monday 30 March 2009 19:48:13 Bjorn Helgaas wrote:
> This patch adds support for ACPI device driver .notify() methods.  If
> such a method is present, Linux/ACPI installs a handler for device
> notifications (but not for system notifications such as Bus Check,
> Device Check, etc).  When a device notification occurs, Linux/ACPI
> passes it on to the driver's .notify() method.
I sent more or less the same some years ago.
Thanks a lot for finally cleaning this up!

> In most cases, this removes the need for drivers to install their own
> handlers for device-specific notifications.
> 
> For fixed hardware devices like some power and sleep buttons, there's
> no notification value because there's no control method to execute a
> Notify opcode.  When a fixed hardware device generates an event, we
> handle it the same as a regular device notification, except we send
> a ACPI_FIXED_HARDWARE_EVENT value.  This is outside the normal 0x0-0xff
> range used by Notify opcodes.
> 
> Several drivers install their own handlers for system Bus Check and
> Device Check notifications so they can support hot-plug.  This patch
> doesn't affect that usage.
Getting rid of these will be the tricky part.
When I looked at it start/stop already was defined, but nobody used it.
IMO start/stop is not needed and hotplug capable device drivers can
handle things themselves in the relevant notify case.
Most risky part probably will be to register devices which are not present
(to get rid of the own system bus handlers).
I could imagine it will just work. The device drivers have to be able to
handle it. E.g. check in add() or notify() whether _STA is present and if not,
don't touch the device's functions. This should mostly be done for hotplug
capable drivers and not necessary for others (additional sanity checking for
_STA being not present is a good idea, though).

I remember these two guys helped me testing on memory hotplug.
They only had a simulator, but might want to give the latest kernel a try if
you come to clean up acpi_memhotplug.c:
kamezawa.hiroyu@jp.fujitsu.com
Yasunori Goto <y-goto@jp.fujitsu.com>

AFAIK, we also have memory hotplug capable machines somewhere, tell
me if I shall test something.

Thanks a lot,

    Thomas

> Signed-off-by: Bjorn Helgaas <bjorn.helgaas@hp.com>
> Reviewed-by: Alex Chiang <achiang@hp.com>
> ---
>  drivers/acpi/scan.c         |   71 +++++++++++++++++++++++++++++++++++++++++++
>  include/acpi/acpi_bus.h     |    2 +
>  include/acpi/acpi_drivers.h |   10 ++++++
>  3 files changed, 83 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
> index b7308ef..20c23c0 100644
> --- a/drivers/acpi/scan.c
> +++ b/drivers/acpi/scan.c
> @@ -359,6 +359,61 @@ static int acpi_device_uevent(struct device *dev, struct kobj_uevent_env *env)
>  	return 0;
>  }
>  
> +static void acpi_device_notify(acpi_handle handle, u32 event, void *data)
> +{
> +	struct acpi_device *device = data;
> +
> +	device->driver->ops.notify(device, event);
> +}
> +
> +static acpi_status acpi_device_notify_fixed(void *data)
> +{
> +	struct acpi_device *device = data;
> +
> +	acpi_device_notify(device->handle, ACPI_FIXED_HARDWARE_EVENT, device);
> +	return AE_OK;
> +}
> +
> +static int acpi_device_install_notify_handler(struct acpi_device *device)
> +{
> +	acpi_status status;
> +	char *hid;
> +
> +	hid = acpi_device_hid(device);
> +	if (!strcmp(hid, ACPI_BUTTON_HID_POWERF))
> +		status =
> +		    acpi_install_fixed_event_handler(ACPI_EVENT_POWER_BUTTON,
> +						     acpi_device_notify_fixed,
> +						     device);
> +	else if (!strcmp(hid, ACPI_BUTTON_HID_SLEEPF))
> +		status =
> +		    acpi_install_fixed_event_handler(ACPI_EVENT_SLEEP_BUTTON,
> +						     acpi_device_notify_fixed,
> +						     device);
> +	else
> +		status = acpi_install_notify_handler(device->handle,
> +						     ACPI_DEVICE_NOTIFY,
> +						     acpi_device_notify,
> +						     device);
> +
> +	if (ACPI_FAILURE(status))
> +		return -EINVAL;
> +	return 0;
> +}
> +
> +static void acpi_device_remove_notify_handler(struct acpi_device *device)
> +{
> +	if (!strcmp(acpi_device_hid(device), ACPI_BUTTON_HID_POWERF))
> +		acpi_remove_fixed_event_handler(ACPI_EVENT_POWER_BUTTON,
> +						acpi_device_notify_fixed);
> +	else if (!strcmp(acpi_device_hid(device), ACPI_BUTTON_HID_SLEEPF))
> +		acpi_remove_fixed_event_handler(ACPI_EVENT_SLEEP_BUTTON,
> +						acpi_device_notify_fixed);
> +	else
> +		acpi_remove_notify_handler(device->handle, ACPI_DEVICE_NOTIFY,
> +					   acpi_device_notify);
> +}
> +
>  static int acpi_bus_driver_init(struct acpi_device *, struct acpi_driver *);
>  static int acpi_start_single_object(struct acpi_device *);
>  static int acpi_device_probe(struct device * dev)
> @@ -371,6 +426,20 @@ static int acpi_device_probe(struct device * dev)
>  	if (!ret) {
>  		if (acpi_dev->bus_ops.acpi_op_start)
>  			acpi_start_single_object(acpi_dev);
> +
> +		if (acpi_drv->ops.notify) {
> +			ret = acpi_device_install_notify_handler(acpi_dev);
> +			if (ret) {
> +				if (acpi_drv->ops.stop)
> +					acpi_drv->ops.stop(acpi_dev,
> +						   acpi_dev->removal_type);
> +				if (acpi_drv->ops.remove)
> +					acpi_drv->ops.remove(acpi_dev,
> +						     acpi_dev->removal_type);
> +				return ret;
> +			}
> +		}
> +
>  		ACPI_DEBUG_PRINT((ACPI_DB_INFO,
>  			"Found driver [%s] for device [%s]\n",
>  			acpi_drv->name, acpi_dev->pnp.bus_id));
> @@ -385,6 +454,8 @@ static int acpi_device_remove(struct device * dev)
>  	struct acpi_driver *acpi_drv = acpi_dev->driver;
>  
>  	if (acpi_drv) {
> +		if (acpi_drv->ops.notify)
> +			acpi_device_remove_notify_handler(acpi_dev);
>  		if (acpi_drv->ops.stop)
>  			acpi_drv->ops.stop(acpi_dev, acpi_dev->removal_type);
>  		if (acpi_drv->ops.remove)
> diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
> index 08ec60c..a222851 100644
> --- a/include/acpi/acpi_bus.h
> +++ b/include/acpi/acpi_bus.h
> @@ -95,6 +95,7 @@ typedef int (*acpi_op_suspend) (struct acpi_device * device,
>  typedef int (*acpi_op_resume) (struct acpi_device * device);
>  typedef int (*acpi_op_bind) (struct acpi_device * device);
>  typedef int (*acpi_op_unbind) (struct acpi_device * device);
> +typedef void (*acpi_op_notify) (struct acpi_device * device, u32 event);
>  
>  struct acpi_bus_ops {
>  	u32 acpi_op_add:1;
> @@ -110,6 +111,7 @@ struct acpi_device_ops {
>  	acpi_op_resume resume;
>  	acpi_op_bind bind;
>  	acpi_op_unbind unbind;
> +	acpi_op_notify notify;
>  };
>  
>  struct acpi_driver {
> diff --git a/include/acpi/acpi_drivers.h b/include/acpi/acpi_drivers.h
> index 241d227..0352c8f 100644
> --- a/include/acpi/acpi_drivers.h
> +++ b/include/acpi/acpi_drivers.h
> @@ -67,6 +67,16 @@
>  #define ACPI_BAY_HID			"LNXIOBAY"
>  #define ACPI_DOCK_HID			"LNXDOCK"
>  
> +/*
> + * For fixed hardware buttons, we fabricate acpi_devices with HID
> + * ACPI_BUTTON_HID_POWERF or ACPI_BUTTON_HID_SLEEPF.  Fixed hardware
> + * signals only an event; it doesn't supply a notification value.
> + * To allow drivers to treat notifications from fixed hardware the
> + * same as those from real devices, we turn the events into this
> + * notification value.
> + */
> +#define ACPI_FIXED_HARDWARE_EVENT	0x100
> +
>  /* --------------------------------------------------------------------------
>                                         PCI
>     -------------------------------------------------------------------------- */
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH 01/10] ACPI: support acpi_device_ops .notify methods

[Bjorn Helgaas]

On Thursday 02 April 2009 07:56:28 am Thomas Renninger wrote:
> On Monday 30 March 2009 19:48:13 Bjorn Helgaas wrote:
> > This patch adds support for ACPI device driver .notify() methods.  If
> > such a method is present, Linux/ACPI installs a handler for device
> > notifications (but not for system notifications such as Bus Check,
> > Device Check, etc).  When a device notification occurs, Linux/ACPI
> > passes it on to the driver's .notify() method.
> I sent more or less the same some years ago.
> Thanks a lot for finally cleaning this up!

Hi Thomas,

Oh, sorry, I didn't know that, or I would have given you some credit :-)
In fact, if you have a URL, I'll add a pointer to the changelog.  I
always like to leave breadcrumbs to help future research.

> > Several drivers install their own handlers for system Bus Check and
> > Device Check notifications so they can support hot-plug.  This patch
> > doesn't affect that usage.
> Getting rid of these will be the tricky part.
> When I looked at it start/stop already was defined, but nobody used it.
> IMO start/stop is not needed and hotplug capable device drivers can
> handle things themselves in the relevant notify case.

I'd really like to get rid of the bus/device check notification stuff
in the drivers eventually.  IMHO, the core Linux/ACPI code should
field those notifications and just call the driver .add() and .remove()
methods as necessary.

But you're right, it's going to be quite tricky.  I'm looking at getting
rid of .start() right now, because that's a major complication.  The
biggest user looks like acpiphp, and that is going to be a mess to
straighten out.

> I remember these two guys helped me testing on memory hotplug.
> They only had a simulator, but might want to give the latest kernel a try if
> you come to clean up acpi_memhotplug.c:
> kamezawa.hiroyu@jp.fujitsu.com
> Yasunori Goto <y-goto@jp.fujitsu.com>
> 
> AFAIK, we also have memory hotplug capable machines somewhere, tell
> me if I shall test something.

Great, thanks for the pointers.  I have some HP boxes that should
support some of this hotplug if I can dig out the right tools to
kick things off.  It works on HP-UX, so we *ought* to be able to
play with it on Linux, too.

Bjorn

Re: [PATCH 01/10] ACPI: support acpi_device_ops .notify methods

[Yasunori Goto]

> On Thursday 02 April 2009 07:56:28 am Thomas Renninger wrote:
> > On Monday 30 March 2009 19:48:13 Bjorn Helgaas wrote:
> > > This patch adds support for ACPI device driver .notify() methods.  If
> > > such a method is present, Linux/ACPI installs a handler for device
> > > notifications (but not for system notifications such as Bus Check,
> > > Device Check, etc).  When a device notification occurs, Linux/ACPI
> > > passes it on to the driver's .notify() method.
> > I sent more or less the same some years ago.
> > Thanks a lot for finally cleaning this up!
> 
> Hi Thomas,
> 
> Oh, sorry, I didn't know that, or I would have given you some credit :-)
> In fact, if you have a URL, I'll add a pointer to the changelog.  I
> always like to leave breadcrumbs to help future research.
> 
> > > Several drivers install their own handlers for system Bus Check and
> > > Device Check notifications so they can support hot-plug.  This patch
> > > doesn't affect that usage.
> > Getting rid of these will be the tricky part.
> > When I looked at it start/stop already was defined, but nobody used it.
> > IMO start/stop is not needed and hotplug capable device drivers can
> > handle things themselves in the relevant notify case.
> 
> I'd really like to get rid of the bus/device check notification stuff
> in the drivers eventually.  IMHO, the core Linux/ACPI code should
> field those notifications and just call the driver .add() and .remove()
> methods as necessary.
> 
> But you're right, it's going to be quite tricky.  I'm looking at getting
> rid of .start() right now, because that's a major complication.  The
> biggest user looks like acpiphp, and that is going to be a mess to
> straighten out.
> 
> > I remember these two guys helped me testing on memory hotplug.
> > They only had a simulator, but might want to give the latest kernel a try if
> > you come to clean up acpi_memhotplug.c:
> > kamezawa.hiroyu@jp.fujitsu.com
> > Yasunori Goto <y-goto@jp.fujitsu.com>

I can use real machine which can memory hotplug.
In our case, notify method is called for container device (NUMA node),
and memory and processor devices are added under acpi_bus_scan().

I'll book it to test this patch in next week.
It must be good test. :-)

Bye.



-- 
Yasunori Goto 

Re: [PATCH 01/10] ACPI: support acpi_device_ops .notify methods

[Thomas Renninger]

On Friday 03 April 2009 02:23:16 Yasunori Goto wrote:
> > On Thursday 02 April 2009 07:56:28 am Thomas Renninger wrote:
> > > On Monday 30 March 2009 19:48:13 Bjorn Helgaas wrote:
> > > > This patch adds support for ACPI device driver .notify() methods.  If
> > > > such a method is present, Linux/ACPI installs a handler for device
> > > > notifications (but not for system notifications such as Bus Check,
> > > > Device Check, etc).  When a device notification occurs, Linux/ACPI
> > > > passes it on to the driver's .notify() method.
> > > I sent more or less the same some years ago.
> > > Thanks a lot for finally cleaning this up!
> > 
> > Hi Thomas,
> > 
> > Oh, sorry, I didn't know that, or I would have given you some credit :-)
> > In fact, if you have a URL, I'll add a pointer to the changelog.  I
> > always like to leave breadcrumbs to help future research.
> > 
> > > > Several drivers install their own handlers for system Bus Check and
> > > > Device Check notifications so they can support hot-plug.  This patch
> > > > doesn't affect that usage.
> > > Getting rid of these will be the tricky part.
> > > When I looked at it start/stop already was defined, but nobody used it.
> > > IMO start/stop is not needed and hotplug capable device drivers can
> > > handle things themselves in the relevant notify case.
> > 
> > I'd really like to get rid of the bus/device check notification stuff
> > in the drivers eventually.  IMHO, the core Linux/ACPI code should
> > field those notifications and just call the driver .add() and .remove()
> > methods as necessary.
> > 
> > But you're right, it's going to be quite tricky.  I'm looking at getting
> > rid of .start() right now, because that's a major complication.  The
> > biggest user looks like acpiphp, and that is going to be a mess to
> > straighten out.
> > 
> > > I remember these two guys helped me testing on memory hotplug.
> > > They only had a simulator, but might want to give the latest kernel a try if
> > > you come to clean up acpi_memhotplug.c:
> > > kamezawa.hiroyu@jp.fujitsu.com
> > > Yasunori Goto <y-goto@jp.fujitsu.com>
> 
> I can use real machine which can memory hotplug.
> In our case, notify method is called for container device (NUMA node),
> and memory and processor devices are added under acpi_bus_scan().
> 
> I'll book it to test this patch in next week.
> It must be good test. :-)

Better wait a bit or ask Bjorn.
I expect this is only the beginning of his cleanups and the tricky
parts will still come.

Thanks,

      Thomas

Re: [PATCH 01/10] ACPI: support acpi_device_ops .notify methods

[Bjorn Helgaas]

On Thursday 02 April 2009 06:23:16 pm Yasunori Goto wrote:
> > On Thursday 02 April 2009 07:56:28 am Thomas Renninger wrote:
> > > I remember these two guys helped me testing on memory hotplug.
> > > They only had a simulator, but might want to give the latest kernel a try if
> > > you come to clean up acpi_memhotplug.c:
> > > kamezawa.hiroyu@jp.fujitsu.com
> > > Yasunori Goto <y-goto@jp.fujitsu.com>
> 
> I can use real machine which can memory hotplug.
> In our case, notify method is called for container device (NUMA node),
> and memory and processor devices are added under acpi_bus_scan().
> 
> I'll book it to test this patch in next week.
> It must be good test. :-)

Thomas is right; I don't think it's worth your time to do much testing
right now.  These patches only affect device notifications, and I think
the container/memory/processor stuff you're referring to depends on the
system notifications.  So this series should not affect the hotplug
path.

I am working on some changes to the hotplug path, but that looks much
more difficult, so it'll probably be a while before anything comes of
it.  Thanks for the tip; I'll make sure to CC: you when I post patches.

Bjorn

Re: [PATCH 01/10] ACPI: support acpi_device_ops .notify methods

[Yasunori Goto]

> On Thursday 02 April 2009 06:23:16 pm Yasunori Goto wrote:
> > > On Thursday 02 April 2009 07:56:28 am Thomas Renninger wrote:
> > > > I remember these two guys helped me testing on memory hotplug.
> > > > They only had a simulator, but might want to give the latest kernel a try if
> > > > you come to clean up acpi_memhotplug.c:
> > > > kamezawa.hiroyu@jp.fujitsu.com
> > > > Yasunori Goto <y-goto@jp.fujitsu.com>
> > 
> > I can use real machine which can memory hotplug.
> > In our case, notify method is called for container device (NUMA node),
> > and memory and processor devices are added under acpi_bus_scan().
> > 
> > I'll book it to test this patch in next week.
> > It must be good test. :-)
> 
> Thomas is right; I don't think it's worth your time to do much testing
> right now.  These patches only affect device notifications, and I think
> the container/memory/processor stuff you're referring to depends on the
> system notifications.  So this series should not affect the hotplug
> path.

Ah, OK. I misunderstood something.
Sorry for noise.

> 
> I am working on some changes to the hotplug path, but that looks much
> more difficult, so it'll probably be a while before anything comes of
> it.  Thanks for the tip; I'll make sure to CC: you when I post patches.




-- 
Yasunori Goto 

Re: [PATCH 01/10] ACPI: support acpi_device_ops .notify methods

[Thomas Renninger]

On Thursday 02 April 2009 17:03:19 Bjorn Helgaas wrote:
> On Thursday 02 April 2009 07:56:28 am Thomas Renninger wrote:
> > On Monday 30 March 2009 19:48:13 Bjorn Helgaas wrote:
> > > This patch adds support for ACPI device driver .notify() methods.  If
> > > such a method is present, Linux/ACPI installs a handler for device
> > > notifications (but not for system notifications such as Bus Check,
> > > Device Check, etc).  When a device notification occurs, Linux/ACPI
> > > passes it on to the driver's .notify() method.
> > I sent more or less the same some years ago.
> > Thanks a lot for finally cleaning this up!
> 
> Hi Thomas,
> 
> Oh, sorry, I didn't know that, or I would have given you some credit :-)
> In fact, if you have a URL, I'll add a pointer to the changelog.  I
> always like to leave breadcrumbs to help future research.
I only find the acpi_memhotplug things, but I remember I suggested
(maybe privately?) to introduce a .notify callback.
Hmm, I even had some code, but got cold feet when I realized that not
present devices have to call .add or .start functions...
While this is not much worth crediting :), I just want to give you the
feedback I missed, IMO you are going the right way.

    Thomas

[PATCH 02/10] ACPI: button: use .notify method instead of installing handler directly

[Bjorn Helgaas]

This patch adds a .notify() method.  The presence of .notify() causes
Linux/ACPI to manage event handlers and notify handlers on our behalf,
so we don't have to install and remove them ourselves.

Note that events from fixed hardware buttons now show up as a special
notify event, so to preserve user-space backward compatibility, we
convert that back to ACPI_BUTTON_NOTIFY_STATUS.

Signed-off-by: Bjorn Helgaas <bjorn.helgaas@hp.com>
Reviewed-by: Alex Chiang <achiang@hp.com>
CC: Alexey Starikovskiy <alexey.y.starikovskiy@linux.intel.com>
---
 drivers/acpi/button.c |   77 +++++--------------------------------------------
 1 files changed, 8 insertions(+), 69 deletions(-)

diff --git a/drivers/acpi/button.c b/drivers/acpi/button.c
index 171fd91..2f8e0ff 100644
--- a/drivers/acpi/button.c
+++ b/drivers/acpi/button.c
@@ -78,6 +78,7 @@ MODULE_DEVICE_TABLE(acpi, button_device_ids);
 static int acpi_button_add(struct acpi_device *device);
 static int acpi_button_remove(struct acpi_device *device, int type);
 static int acpi_button_resume(struct acpi_device *device);
+static void acpi_button_notify(struct acpi_device *device, u32 event);
 static int acpi_button_info_open_fs(struct inode *inode, struct file *file);
 static int acpi_button_state_open_fs(struct inode *inode, struct file *file);
 
@@ -89,6 +90,7 @@ static struct acpi_driver acpi_button_driver = {
 		.add = acpi_button_add,
 		.resume = acpi_button_resume,
 		.remove = acpi_button_remove,
+		.notify = acpi_button_notify,
 	},
 };
 
@@ -265,15 +267,18 @@ static int acpi_lid_send_state(struct acpi_button *button)
 	return 0;
 }
 
-static void acpi_button_notify(acpi_handle handle, u32 event, void *data)
+static void acpi_button_notify(struct acpi_device *device, u32 event)
 {
-	struct acpi_button *button = data;
+	struct acpi_button *button = acpi_driver_data(device);
 	struct input_dev *input;
 
 	if (!button || !button->device)
 		return;
 
 	switch (event) {
+	case ACPI_FIXED_HARDWARE_EVENT:
+		event = ACPI_BUTTON_NOTIFY_STATUS;
+		/* fall through */
 	case ACPI_BUTTON_NOTIFY_STATUS:
 		input = button->input;
 		if (button->type == ACPI_BUTTON_TYPE_LID) {
@@ -300,46 +305,6 @@ static void acpi_button_notify(acpi_handle handle, u32 event, void *data)
 	return;
 }
 
-static acpi_status acpi_button_notify_fixed(void *data)
-{
-	struct acpi_button *button = data;
-
-	if (!button)
-		return AE_BAD_PARAMETER;
-
-	acpi_button_notify(button->device->handle, ACPI_BUTTON_NOTIFY_STATUS, button);
-
-	return AE_OK;
-}
-
-static int acpi_button_install_notify_handlers(struct acpi_button *button)
-{
-	acpi_status status;
-
-	switch (button->type) {
-	case ACPI_BUTTON_TYPE_POWERF:
-		status =
-		    acpi_install_fixed_event_handler(ACPI_EVENT_POWER_BUTTON,
-						     acpi_button_notify_fixed,
-						     button);
-		break;
-	case ACPI_BUTTON_TYPE_SLEEPF:
-		status =
-		    acpi_install_fixed_event_handler(ACPI_EVENT_SLEEP_BUTTON,
-						     acpi_button_notify_fixed,
-						     button);
-		break;
-	default:
-		status = acpi_install_notify_handler(button->device->handle,
-						     ACPI_DEVICE_NOTIFY,
-						     acpi_button_notify,
-						     button);
-		break;
-	}
-
-	return ACPI_FAILURE(status) ? -ENODEV : 0;
-}
-
 static int acpi_button_resume(struct acpi_device *device)
 {
 	struct acpi_button *button;
@@ -351,25 +316,6 @@ static int acpi_button_resume(struct acpi_device *device)
 	return 0;
 }
 
-static void acpi_button_remove_notify_handlers(struct acpi_button *button)
-{
-	switch (button->type) {
-	case ACPI_BUTTON_TYPE_POWERF:
-		acpi_remove_fixed_event_handler(ACPI_EVENT_POWER_BUTTON,
-						acpi_button_notify_fixed);
-		break;
-	case ACPI_BUTTON_TYPE_SLEEPF:
-		acpi_remove_fixed_event_handler(ACPI_EVENT_SLEEP_BUTTON,
-						acpi_button_notify_fixed);
-		break;
-	default:
-		acpi_remove_notify_handler(button->device->handle,
-					   ACPI_DEVICE_NOTIFY,
-					   acpi_button_notify);
-		break;
-	}
-}
-
 static int acpi_button_add(struct acpi_device *device)
 {
 	int error;
@@ -434,10 +380,6 @@ static int acpi_button_add(struct acpi_device *device)
 	if (error)
 		goto err_free_input;
 
-	error = acpi_button_install_notify_handlers(button);
-	if (error)
-		goto err_remove_fs;
-
 	snprintf(button->phys, sizeof(button->phys),
 		 "%s/button/input0", acpi_device_hid(device));
 
@@ -468,7 +410,7 @@ static int acpi_button_add(struct acpi_device *device)
 
 	error = input_register_device(input);
 	if (error)
-		goto err_remove_handlers;
+		goto err_remove_fs;
 	if (button->type == ACPI_BUTTON_TYPE_LID)
 		acpi_lid_send_state(button);
 
@@ -487,8 +429,6 @@ static int acpi_button_add(struct acpi_device *device)
 
 	return 0;
 
- err_remove_handlers:
-	acpi_button_remove_notify_handlers(button);
  err_remove_fs:
 	acpi_button_remove_fs(device);
  err_free_input:
@@ -507,7 +447,6 @@ static int acpi_button_remove(struct acpi_device *device, int type)
 
 	button = acpi_driver_data(device);
 
-	acpi_button_remove_notify_handlers(button);
 	acpi_button_remove_fs(device);
 	input_unregister_device(button->input);
 	kfree(button);

[PATCH 03/10] ACPI: processor: use .notify method instead of installing handler directly

[Bjorn Helgaas]

This patch adds a .notify() method.  The presence of .notify() causes
Linux/ACPI to manage event handlers and notify handlers on our behalf,
so we don't have to install and remove them ourselves.

Signed-off-by: Bjorn Helgaas <bjorn.helgaas@hp.com>
CC: Zhang Rui <rui.zhang@intel.com>
CC: Zhao Yakui <yakui.zhao@intel.com>
CC: Venki Pallipadi <venkatesh.pallipadi@intel.com>
CC: Anil S Keshavamurthy <anil.s.keshavamurthy@intel.com>
---
 drivers/acpi/processor_core.c |   19 ++++---------------
 1 files changed, 4 insertions(+), 15 deletions(-)

diff --git a/drivers/acpi/processor_core.c b/drivers/acpi/processor_core.c
index 0cc2fd3..0e97d70 100644
--- a/drivers/acpi/processor_core.c
+++ b/drivers/acpi/processor_core.c
@@ -82,7 +82,7 @@ static int acpi_processor_add(struct acpi_device *device);
 static int acpi_processor_start(struct acpi_device *device);
 static int acpi_processor_remove(struct acpi_device *device, int type);
 static int acpi_processor_info_open_fs(struct inode *inode, struct file *file);
-static void acpi_processor_notify(acpi_handle handle, u32 event, void *data);
+static void acpi_processor_notify(struct acpi_device *device, u32 event);
 static acpi_status acpi_processor_hotadd_init(acpi_handle handle, int *p_cpu);
 static int acpi_processor_handle_eject(struct acpi_processor *pr);
 
@@ -104,6 +104,7 @@ static struct acpi_driver acpi_processor_driver = {
 		.start = acpi_processor_start,
 		.suspend = acpi_processor_suspend,
 		.resume = acpi_processor_resume,
+		.notify = acpi_processor_notify,
 		},
 };
 
@@ -666,7 +667,6 @@ static DEFINE_PER_CPU(void *, processor_device_array);
 static int __cpuinit acpi_processor_start(struct acpi_device *device)
 {
 	int result = 0;
-	acpi_status status = AE_OK;
 	struct acpi_processor *pr;
 	struct sys_device *sysdev;
 
@@ -703,9 +703,6 @@ static int __cpuinit acpi_processor_start(struct acpi_device *device)
 	if (sysfs_create_link(&device->dev.kobj, &sysdev->kobj, "sysdev"))
 		return -EFAULT;
 
-	status = acpi_install_notify_handler(pr->handle, ACPI_DEVICE_NOTIFY,
-					     acpi_processor_notify, pr);
-
 	/* _PDC call should be done before doing anything else (if reqd.). */
 	arch_acpi_processor_init_pdc(pr);
 	acpi_processor_set_pdc(pr);
@@ -751,18 +748,14 @@ static int __cpuinit acpi_processor_start(struct acpi_device *device)
 	return result;
 }
 
-static void acpi_processor_notify(acpi_handle handle, u32 event, void *data)
+static void acpi_processor_notify(struct acpi_device *device, u32 event)
 {
-	struct acpi_processor *pr = data;
-	struct acpi_device *device = NULL;
+	struct acpi_processor *pr = acpi_driver_data(device);
 	int saved;
 
 	if (!pr)
 		return;
 
-	if (acpi_bus_get_device(pr->handle, &device))
-		return;
-
 	switch (event) {
 	case ACPI_PROCESSOR_NOTIFY_PERFORMANCE:
 		saved = pr->performance_platform_limit;
@@ -841,7 +834,6 @@ static int acpi_processor_add(struct acpi_device *device)
 
 static int acpi_processor_remove(struct acpi_device *device, int type)
 {
-	acpi_status status = AE_OK;
 	struct acpi_processor *pr = NULL;
 
 
@@ -860,9 +852,6 @@ static int acpi_processor_remove(struct acpi_device *device, int type)
 
 	acpi_processor_power_exit(pr, device);
 
-	status = acpi_remove_notify_handler(pr->handle, ACPI_DEVICE_NOTIFY,
-					    acpi_processor_notify);
-
 	sysfs_remove_link(&device->dev.kobj, "sysdev");
 
 	acpi_processor_remove_fs(device);

[PATCH 04/10] ACPI: thermal: use .notify method instead of installing handler directly

[Bjorn Helgaas]

This patch adds a .notify() method.  The presence of .notify() causes
Linux/ACPI to manage event handlers and notify handlers on our behalf,
so we don't have to install and remove them ourselves.

Signed-off-by: Bjorn Helgaas <bjorn.helgaas@hp.com>
CC: Zhang Rui <rui.zhang@intel.com>
---
 drivers/acpi/thermal.c |   27 ++++-----------------------
 1 files changed, 4 insertions(+), 23 deletions(-)

diff --git a/drivers/acpi/thermal.c b/drivers/acpi/thermal.c
index 99e6f1f..3a7b0c7 100644
--- a/drivers/acpi/thermal.c
+++ b/drivers/acpi/thermal.c
@@ -98,6 +98,7 @@ MODULE_PARM_DESC(psv, "Disable or override all passive trip points.");
 static int acpi_thermal_add(struct acpi_device *device);
 static int acpi_thermal_remove(struct acpi_device *device, int type);
 static int acpi_thermal_resume(struct acpi_device *device);
+static void acpi_thermal_notify(struct acpi_device *device, u32 event);
 static int acpi_thermal_state_open_fs(struct inode *inode, struct file *file);
 static int acpi_thermal_temp_open_fs(struct inode *inode, struct file *file);
 static int acpi_thermal_trip_open_fs(struct inode *inode, struct file *file);
@@ -123,6 +124,7 @@ static struct acpi_driver acpi_thermal_driver = {
 		.add = acpi_thermal_add,
 		.remove = acpi_thermal_remove,
 		.resume = acpi_thermal_resume,
+		.notify = acpi_thermal_notify,
 		},
 };
 
@@ -1579,17 +1581,14 @@ static int acpi_thermal_remove_fs(struct acpi_device *device)
                                  Driver Interface
    -------------------------------------------------------------------------- */
 
-static void acpi_thermal_notify(acpi_handle handle, u32 event, void *data)
+static void acpi_thermal_notify(struct acpi_device *device, u32 event)
 {
-	struct acpi_thermal *tz = data;
-	struct acpi_device *device = NULL;
+	struct acpi_thermal *tz = acpi_driver_data(device);
 
 
 	if (!tz)
 		return;
 
-	device = tz->device;
-
 	switch (event) {
 	case ACPI_THERMAL_NOTIFY_TEMPERATURE:
 		acpi_thermal_check(tz);
@@ -1613,8 +1612,6 @@ static void acpi_thermal_notify(acpi_handle handle, u32 event, void *data)
 				  "Unsupported event [0x%x]\n", event));
 		break;
 	}
-
-	return;
 }
 
 static int acpi_thermal_get_info(struct acpi_thermal *tz)
@@ -1652,7 +1649,6 @@ static int acpi_thermal_get_info(struct acpi_thermal *tz)
 static int acpi_thermal_add(struct acpi_device *device)
 {
 	int result = 0;
-	acpi_status status = AE_OK;
 	struct acpi_thermal *tz = NULL;
 
 
@@ -1689,21 +1685,11 @@ static int acpi_thermal_add(struct acpi_device *device)
 
 	acpi_thermal_check(tz);
 
-	status = acpi_install_notify_handler(device->handle,
-					     ACPI_DEVICE_NOTIFY,
-					     acpi_thermal_notify, tz);
-	if (ACPI_FAILURE(status)) {
-		result = -ENODEV;
-		goto remove_fs;
-	}
-
 	printk(KERN_INFO PREFIX "%s [%s] (%ld C)\n",
 	       acpi_device_name(device), acpi_device_bid(device),
 	       KELVIN_TO_CELSIUS(tz->temperature));
 	goto end;
 
-remove_fs:
-	acpi_thermal_remove_fs(device);
 unregister_thermal_zone:
 	thermal_zone_device_unregister(tz->thermal_zone);
 free_memory:
@@ -1714,7 +1700,6 @@ end:
 
 static int acpi_thermal_remove(struct acpi_device *device, int type)
 {
-	acpi_status status = AE_OK;
 	struct acpi_thermal *tz = NULL;
 
 
@@ -1732,10 +1717,6 @@ static int acpi_thermal_remove(struct acpi_device *device, int type)
 	/* deferred task may reinsert timer */
 	del_timer_sync(&(tz->timer));
 
-	status = acpi_remove_notify_handler(device->handle,
-					    ACPI_DEVICE_NOTIFY,
-					    acpi_thermal_notify);
-
 	/* Terminate policy */
 	if (tz->trips.passive.flags.valid && tz->trips.passive.flags.enabled) {
 		tz->trips.passive.flags.enabled = 0;

[PATCH 05/10] ACPI: video: use .notify method instead of installing handler directly

[Bjorn Helgaas]

This patch adds a .notify() method.  The presence of .notify() causes
Linux/ACPI to manage event handlers and notify handlers on our behalf,
so we don't have to install and remove them ourselves.

Signed-off-by: Bjorn Helgaas <bjorn.helgaas@hp.com>
CC: Zhang Rui <rui.zhang@intel.com>
---
 drivers/acpi/video.c |   30 +++++-------------------------
 1 files changed, 5 insertions(+), 25 deletions(-)

diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c
index bb5ed05..1bea502 100644
--- a/drivers/acpi/video.c
+++ b/drivers/acpi/video.c
@@ -77,6 +77,7 @@ module_param(brightness_switch_enabled, bool, 0644);
 static int acpi_video_bus_add(struct acpi_device *device);
 static int acpi_video_bus_remove(struct acpi_device *device, int type);
 static int acpi_video_resume(struct acpi_device *device);
+static void acpi_video_bus_notify(struct acpi_device *device, u32 event);
 
 static const struct acpi_device_id video_device_ids[] = {
 	{ACPI_VIDEO_HID, 0},
@@ -92,6 +93,7 @@ static struct acpi_driver acpi_video_bus = {
 		.add = acpi_video_bus_add,
 		.remove = acpi_video_bus_remove,
 		.resume = acpi_video_resume,
+		.notify = acpi_video_bus_notify,
 		},
 };
 
@@ -1850,17 +1852,15 @@ static int acpi_video_bus_stop_devices(struct acpi_video_bus *video)
 	return acpi_video_bus_DOS(video, 0, 1);
 }
 
-static void acpi_video_bus_notify(acpi_handle handle, u32 event, void *data)
+static void acpi_video_bus_notify(struct acpi_device *device, u32 event)
 {
-	struct acpi_video_bus *video = data;
-	struct acpi_device *device = NULL;
+	struct acpi_video_bus *video = acpi_driver_data(device);
 	struct input_dev *input;
 	int keycode;
 
 	if (!video)
 		return;
 
-	device = video->device;
 	input = video->input;
 
 	switch (event) {
@@ -1991,7 +1991,6 @@ static int acpi_video_resume(struct acpi_device *device)
 
 static int acpi_video_bus_add(struct acpi_device *device)
 {
-	acpi_status status;
 	struct acpi_video_bus *video;
 	struct input_dev *input;
 	int error;
@@ -2033,20 +2032,10 @@ static int acpi_video_bus_add(struct acpi_device *device)
 	acpi_video_bus_get_devices(video, device);
 	acpi_video_bus_start_devices(video);
 
-	status = acpi_install_notify_handler(device->handle,
-					     ACPI_DEVICE_NOTIFY,
-					     acpi_video_bus_notify, video);
-	if (ACPI_FAILURE(status)) {
-		printk(KERN_ERR PREFIX
-				  "Error installing notify handler\n");
-		error = -ENODEV;
-		goto err_stop_video;
-	}
-
 	video->input = input = input_allocate_device();
 	if (!input) {
 		error = -ENOMEM;
-		goto err_uninstall_notify;
+		goto err_stop_video;
 	}
 
 	snprintf(video->phys, sizeof(video->phys),
@@ -2082,9 +2071,6 @@ static int acpi_video_bus_add(struct acpi_device *device)
 
  err_free_input_dev:
 	input_free_device(input);
- err_uninstall_notify:
-	acpi_remove_notify_handler(device->handle, ACPI_DEVICE_NOTIFY,
-				   acpi_video_bus_notify);
  err_stop_video:
 	acpi_video_bus_stop_devices(video);
 	acpi_video_bus_put_devices(video);
@@ -2099,7 +2085,6 @@ static int acpi_video_bus_add(struct acpi_device *device)
 
 static int acpi_video_bus_remove(struct acpi_device *device, int type)
 {
-	acpi_status status = 0;
 	struct acpi_video_bus *video = NULL;
 
 
@@ -2109,11 +2094,6 @@ static int acpi_video_bus_remove(struct acpi_device *device, int type)
 	video = acpi_driver_data(device);
 
 	acpi_video_bus_stop_devices(video);
-
-	status = acpi_remove_notify_handler(video->device->handle,
-					    ACPI_DEVICE_NOTIFY,
-					    acpi_video_bus_notify);
-
 	acpi_video_bus_put_devices(video);
 	acpi_video_bus_remove_fs(device);
 

[PATCH 06/10] fujitsu-laptop: use .notify method instead of installing handler directly

[Bjorn Helgaas]

This patch adds a .notify() method.  The presence of .notify() causes
Linux/ACPI to manage event handlers and notify handlers on our behalf,
so we don't have to install and remove them ourselves.

Signed-off-by: Bjorn Helgaas <bjorn.helgaas@hp.com>
CC: Jonathan Woithe <jwoithe@physics.adelaide.edu.au>
CC: Tony Vroon <tony@linx.net>
---
 drivers/platform/x86/fujitsu-laptop.c |   28 ++++------------------------
 1 files changed, 4 insertions(+), 24 deletions(-)

diff --git a/drivers/platform/x86/fujitsu-laptop.c b/drivers/platform/x86/fujitsu-laptop.c
index 45940f3..10f8796 100644
--- a/drivers/platform/x86/fujitsu-laptop.c
+++ b/drivers/platform/x86/fujitsu-laptop.c
@@ -203,7 +203,7 @@ struct led_classdev kblamps_led = {
 static u32 dbg_level = 0x03;
 #endif
 
-static void acpi_fujitsu_notify(acpi_handle handle, u32 event, void *data);
+static void acpi_fujitsu_notify(struct acpi_device *device, u32 event);
 
 /* Fujitsu ACPI interface function */
 
@@ -658,7 +658,6 @@ static struct dmi_system_id fujitsu_dmi_table[] = {
 
 static int acpi_fujitsu_add(struct acpi_device *device)
 {
-	acpi_status status;
 	acpi_handle handle;
 	int result = 0;
 	int state = 0;
@@ -673,20 +672,10 @@ static int acpi_fujitsu_add(struct acpi_device *device)
 	sprintf(acpi_device_class(device), "%s", ACPI_FUJITSU_CLASS);
 	device->driver_data = fujitsu;
 
-	status = acpi_install_notify_handler(device->handle,
-					     ACPI_DEVICE_NOTIFY,
-					     acpi_fujitsu_notify, fujitsu);
-
-	if (ACPI_FAILURE(status)) {
-		printk(KERN_ERR "Error installing notify handler\n");
-		error = -ENODEV;
-		goto err_stop;
-	}
-
 	fujitsu->input = input = input_allocate_device();
 	if (!input) {
 		error = -ENOMEM;
-		goto err_uninstall_notify;
+		goto err_stop;
 	}
 
 	snprintf(fujitsu->phys, sizeof(fujitsu->phys),
@@ -743,9 +732,6 @@ static int acpi_fujitsu_add(struct acpi_device *device)
 end:
 err_free_input_dev:
 	input_free_device(input);
-err_uninstall_notify:
-	acpi_remove_notify_handler(device->handle, ACPI_DEVICE_NOTIFY,
-				   acpi_fujitsu_notify);
 err_stop:
 
 	return result;
@@ -753,7 +739,6 @@ err_stop:
 
 static int acpi_fujitsu_remove(struct acpi_device *device, int type)
 {
-	acpi_status status;
 	struct fujitsu_t *fujitsu = NULL;
 
 	if (!device || !acpi_driver_data(device))
@@ -761,10 +746,6 @@ static int acpi_fujitsu_remove(struct acpi_device *device, int type)
 
 	fujitsu = acpi_driver_data(device);
 
-	status = acpi_remove_notify_handler(fujitsu->acpi_handle,
-					    ACPI_DEVICE_NOTIFY,
-					    acpi_fujitsu_notify);
-
 	if (!device || !acpi_driver_data(device))
 		return -EINVAL;
 
@@ -775,7 +756,7 @@ static int acpi_fujitsu_remove(struct acpi_device *device, int type)
 
 /* Brightness notify */
 
-static void acpi_fujitsu_notify(acpi_handle handle, u32 event, void *data)
+static void acpi_fujitsu_notify(struct acpi_device *device, u32 event)
 {
 	struct input_dev *input;
 	int keycode;
@@ -829,8 +810,6 @@ static void acpi_fujitsu_notify(acpi_handle handle, u32 event, void *data)
 		input_report_key(input, keycode, 0);
 		input_sync(input);
 	}
-
-	return;
 }
 
 /* ACPI device for hotkey handling */
@@ -1107,6 +1086,7 @@ static struct acpi_driver acpi_fujitsu_driver = {
 	.ops = {
 		.add = acpi_fujitsu_add,
 		.remove = acpi_fujitsu_remove,
+		.notify = acpi_fujitsu_notify,
 		},
 };
 

Re: [PATCH 06/10] fujitsu-laptop: use .notify method instead of

[Jonathan Woithe]

> This patch adds a .notify() method.  The presence of .notify() causes
> Linux/ACPI to manage event handlers and notify handlers on our behalf,
> so we don't have to install and remove them ourselves.
> 
> Signed-off-by: Bjorn Helgaas <bjorn.helgaas@hp.com>
> CC: Jonathan Woithe <jwoithe@physics.adelaide.edu.au>
> CC: Tony Vroon <tony@linx.net>

I will check this out overnight and comment tomorrow.

Regards
  jonathan

Re: [PATCH 06/10] fujitsu-laptop: use .notify method instead of installing handler directly

[Tony Vroon]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On Mon, 30 Mar 2009 11:48:39 -0600
Bjorn Helgaas <bjorn.helgaas@hp.com> wrote:
> This patch adds a .notify() method.  The presence of .notify() causes
> Linux/ACPI to manage event handlers and notify handlers on our behalf,
> so we don't have to install and remove them ourselves.
 
> Signed-off-by: Bjorn Helgaas <bjorn.helgaas@hp.com>
> CC: Jonathan Woithe <jwoithe@physics.adelaide.edu.au>
Acked-By: Tony Vroon <tony@linx.net>

Dock/lid/radio notifications continue to be delivered with this
patchset applied to 2.6.29; test platform is a Fujitsu-Siemens Lifebook
S6420 [FJNB1E6] with BIOS 1.18 (01/09/2009).
The code also gets much easier to read. Thanks for this Bjorn.

Regards,
Tony V.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.11 (GNU/Linux)

iEYEARECAAYFAknSjUEACgkQp5vW4rUFj5rMLACfe+2d3Jcol4X9J99cA+bEILQZ
b50AmwTq7Dkawfh4MxrhzYgAjTIrwVBu
=6sWC
-----END PGP SIGNATURE-----

Re: [PATCH 06/10] fujitsu-laptop: use .notify method instead of installing handler directly

[Bjorn Helgaas]

On Tuesday 31 March 2009 03:38:05 pm Tony Vroon wrote:
> On Mon, 30 Mar 2009 11:48:39 -0600
> Bjorn Helgaas <bjorn.helgaas@hp.com> wrote:
> > This patch adds a .notify() method.  The presence of .notify() causes
> > Linux/ACPI to manage event handlers and notify handlers on our behalf,
> > so we don't have to install and remove them ourselves.
> 
> > Signed-off-by: Bjorn Helgaas <bjorn.helgaas@hp.com>
> > CC: Jonathan Woithe <jwoithe@physics.adelaide.edu.au>
> Acked-By: Tony Vroon <tony@linx.net>
> 
> Dock/lid/radio notifications continue to be delivered with this
> patchset applied to 2.6.29; test platform is a Fujitsu-Siemens Lifebook
> S6420 [FJNB1E6] with BIOS 1.18 (01/09/2009).
> The code also gets much easier to read. Thanks for this Bjorn.

Thanks a lot for testing this, Tony!  If you don't object, I'll also
add a "Tested-By: Tony Vroon <tony@linx.net>" and the config to these
two patches.

Bjorn

Re: [PATCH 06/10] fujitsu-laptop: use .notify method instead of installing handler directly

[Tony Vroon]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On Tue, 31 Mar 2009 16:09:11 -0600
Bjorn Helgaas <bjorn.helgaas@hp.com> wrote:
> If you don't object, I'll also add a "Tested-By: Tony Vroon <tony@linx.net>" and the config to these
> two patches.

Go for it.

> Bjorn

Regards,
Tony V.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.11 (GNU/Linux)

iEYEARECAAYFAknSlI0ACgkQp5vW4rUFj5ptGACgh27f4uxL9bBxrIAbXY/iauux
5LUAnA8Axkjg6zh5xc65dkBLA3A+lQ0O
=Y/Ti
-----END PGP SIGNATURE-----

Re: [PATCH 06/10] fujitsu-laptop: use .notify method instead of installing handler directly

[Jonathan Woithe]

> This patch adds a .notify() method.  The presence of .notify() causes
> Linux/ACPI to manage event handlers and notify handlers on our behalf,
> so we don't have to install and remove them ourselves.
> 
> Signed-off-by: Bjorn Helgaas <bjorn.helgaas@hp.com>
> CC: Jonathan Woithe <jwoithe@physics.adelaide.edu.au>
> CC: Tony Vroon <tony@linx.net>

This patch looks fine to me for a Fujitsu S7020 laptop.  Thanks Bjorn.

Acked-by: Jonathan Woithe <jwoithe@physics.adelaide.edu.au>

Regards
  jonathan

> ---
>  drivers/platform/x86/fujitsu-laptop.c |   28 ++++------------------------
>  1 files changed, 4 insertions(+), 24 deletions(-)
> 
> diff --git a/drivers/platform/x86/fujitsu-laptop.c b/drivers/platform/x86/fujitsu-laptop.c
> index 45940f3..10f8796 100644
> --- a/drivers/platform/x86/fujitsu-laptop.c
> +++ b/drivers/platform/x86/fujitsu-laptop.c
> @@ -203,7 +203,7 @@ struct led_classdev kblamps_led = {
>  static u32 dbg_level = 0x03;
>  #endif
>  
> -static void acpi_fujitsu_notify(acpi_handle handle, u32 event, void *data);
> +static void acpi_fujitsu_notify(struct acpi_device *device, u32 event);
>  
>  /* Fujitsu ACPI interface function */
>  
> @@ -658,7 +658,6 @@ static struct dmi_system_id fujitsu_dmi_table[] = {
>  
>  static int acpi_fujitsu_add(struct acpi_device *device)
>  {
> -	acpi_status status;
>  	acpi_handle handle;
>  	int result = 0;
>  	int state = 0;
> @@ -673,20 +672,10 @@ static int acpi_fujitsu_add(struct acpi_device *device)
>  	sprintf(acpi_device_class(device), "%s", ACPI_FUJITSU_CLASS);
>  	device->driver_data = fujitsu;
>  
> -	status = acpi_install_notify_handler(device->handle,
> -					     ACPI_DEVICE_NOTIFY,
> -					     acpi_fujitsu_notify, fujitsu);
> -
> -	if (ACPI_FAILURE(status)) {
> -		printk(KERN_ERR "Error installing notify handler\n");
> -		error = -ENODEV;
> -		goto err_stop;
> -	}
> -
>  	fujitsu->input = input = input_allocate_device();
>  	if (!input) {
>  		error = -ENOMEM;
> -		goto err_uninstall_notify;
> +		goto err_stop;
>  	}
>  
>  	snprintf(fujitsu->phys, sizeof(fujitsu->phys),
> @@ -743,9 +732,6 @@ static int acpi_fujitsu_add(struct acpi_device *device)
>  end:
>  err_free_input_dev:
>  	input_free_device(input);
> -err_uninstall_notify:
> -	acpi_remove_notify_handler(device->handle, ACPI_DEVICE_NOTIFY,
> -				   acpi_fujitsu_notify);
>  err_stop:
>  
>  	return result;
> @@ -753,7 +739,6 @@ err_stop:
>  
>  static int acpi_fujitsu_remove(struct acpi_device *device, int type)
>  {
> -	acpi_status status;
>  	struct fujitsu_t *fujitsu = NULL;
>  
>  	if (!device || !acpi_driver_data(device))
> @@ -761,10 +746,6 @@ static int acpi_fujitsu_remove(struct acpi_device *device, int type)
>  
>  	fujitsu = acpi_driver_data(device);
>  
> -	status = acpi_remove_notify_handler(fujitsu->acpi_handle,
> -					    ACPI_DEVICE_NOTIFY,
> -					    acpi_fujitsu_notify);
> -
>  	if (!device || !acpi_driver_data(device))
>  		return -EINVAL;
>  
> @@ -775,7 +756,7 @@ static int acpi_fujitsu_remove(struct acpi_device *device, int type)
>  
>  /* Brightness notify */
>  
> -static void acpi_fujitsu_notify(acpi_handle handle, u32 event, void *data)
> +static void acpi_fujitsu_notify(struct acpi_device *device, u32 event)
>  {
>  	struct input_dev *input;
>  	int keycode;
> @@ -829,8 +810,6 @@ static void acpi_fujitsu_notify(acpi_handle handle, u32 event, void *data)
>  		input_report_key(input, keycode, 0);
>  		input_sync(input);
>  	}
> -
> -	return;
>  }
>  
>  /* ACPI device for hotkey handling */
> @@ -1107,6 +1086,7 @@ static struct acpi_driver acpi_fujitsu_driver = {
>  	.ops = {
>  		.add = acpi_fujitsu_add,
>  		.remove = acpi_fujitsu_remove,
> +		.notify = acpi_fujitsu_notify,
>  		},
>  };
>  
> 

[PATCH 07/10] fujitsu-laptop: use .notify method instead of installing hotkey handler directly

[Bjorn Helgaas]

This patch adds a .notify() method.  The presence of .notify() causes
Linux/ACPI to manage event handlers and notify handlers on our behalf,
so we don't have to install and remove them ourselves.

Signed-off-by: Bjorn Helgaas <bjorn.helgaas@hp.com>
CC: Jonathan Woithe <jwoithe@physics.adelaide.edu.au>
CC: Tony Vroon <tony@linx.net>
---
 drivers/platform/x86/fujitsu-laptop.c |   32 +++++---------------------------
 1 files changed, 5 insertions(+), 27 deletions(-)

diff --git a/drivers/platform/x86/fujitsu-laptop.c b/drivers/platform/x86/fujitsu-laptop.c
index 10f8796..218b9a1 100644
--- a/drivers/platform/x86/fujitsu-laptop.c
+++ b/drivers/platform/x86/fujitsu-laptop.c
@@ -174,8 +174,7 @@ struct fujitsu_hotkey_t {
 
 static struct fujitsu_hotkey_t *fujitsu_hotkey;
 
-static void acpi_fujitsu_hotkey_notify(acpi_handle handle, u32 event,
-				       void *data);
+static void acpi_fujitsu_hotkey_notify(struct acpi_device *device, u32 event);
 
 #ifdef CONFIG_LEDS_CLASS
 static enum led_brightness logolamp_get(struct led_classdev *cdev);
@@ -816,7 +815,6 @@ static void acpi_fujitsu_notify(struct acpi_device *device, u32 event)
 
 static int acpi_fujitsu_hotkey_add(struct acpi_device *device)
 {
-	acpi_status status;
 	acpi_handle handle;
 	int result = 0;
 	int state = 0;
@@ -833,17 +831,6 @@ static int acpi_fujitsu_hotkey_add(struct acpi_device *device)
 	sprintf(acpi_device_class(device), "%s", ACPI_FUJITSU_CLASS);
 	device->driver_data = fujitsu_hotkey;
 
-	status = acpi_install_notify_handler(device->handle,
-					     ACPI_DEVICE_NOTIFY,
-					     acpi_fujitsu_hotkey_notify,
-					     fujitsu_hotkey);
-
-	if (ACPI_FAILURE(status)) {
-		printk(KERN_ERR "Error installing notify handler\n");
-		error = -ENODEV;
-		goto err_stop;
-	}
-
 	/* kfifo */
 	spin_lock_init(&fujitsu_hotkey->fifo_lock);
 	fujitsu_hotkey->fifo =
@@ -858,7 +845,7 @@ static int acpi_fujitsu_hotkey_add(struct acpi_device *device)
 	fujitsu_hotkey->input = input = input_allocate_device();
 	if (!input) {
 		error = -ENOMEM;
-		goto err_uninstall_notify;
+		goto err_free_fifo;
 	}
 
 	snprintf(fujitsu_hotkey->phys, sizeof(fujitsu_hotkey->phys),
@@ -954,9 +941,7 @@ static int acpi_fujitsu_hotkey_add(struct acpi_device *device)
 end:
 err_free_input_dev:
 	input_free_device(input);
-err_uninstall_notify:
-	acpi_remove_notify_handler(device->handle, ACPI_DEVICE_NOTIFY,
-				   acpi_fujitsu_hotkey_notify);
+err_free_fifo:
 	kfifo_free(fujitsu_hotkey->fifo);
 err_stop:
 
@@ -965,7 +950,6 @@ err_stop:
 
 static int acpi_fujitsu_hotkey_remove(struct acpi_device *device, int type)
 {
-	acpi_status status;
 	struct fujitsu_hotkey_t *fujitsu_hotkey = NULL;
 
 	if (!device || !acpi_driver_data(device))
@@ -973,10 +957,6 @@ static int acpi_fujitsu_hotkey_remove(struct acpi_device *device, int type)
 
 	fujitsu_hotkey = acpi_driver_data(device);
 
-	status = acpi_remove_notify_handler(fujitsu_hotkey->acpi_handle,
-					    ACPI_DEVICE_NOTIFY,
-					    acpi_fujitsu_hotkey_notify);
-
 	fujitsu_hotkey->acpi_handle = NULL;
 
 	kfifo_free(fujitsu_hotkey->fifo);
@@ -984,8 +964,7 @@ static int acpi_fujitsu_hotkey_remove(struct acpi_device *device, int type)
 	return 0;
 }
 
-static void acpi_fujitsu_hotkey_notify(acpi_handle handle, u32 event,
-				       void *data)
+static void acpi_fujitsu_hotkey_notify(struct acpi_device *device, u32 event)
 {
 	struct input_dev *input;
 	int keycode, keycode_r;
@@ -1068,8 +1047,6 @@ static void acpi_fujitsu_hotkey_notify(acpi_handle handle, u32 event,
 		input_sync(input);
 		break;
 	}
-
-	return;
 }
 
 /* Initialization */
@@ -1102,6 +1079,7 @@ static struct acpi_driver acpi_fujitsu_hotkey_driver = {
 	.ops = {
 		.add = acpi_fujitsu_hotkey_add,
 		.remove = acpi_fujitsu_hotkey_remove,
+		.notify = acpi_fujitsu_hotkey_notify,
 		},
 };
 

Re: [PATCH 07/10] fujitsu-laptop: use .notify method instead of installing hotkey handler directly

[Tony Vroon]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On Mon, 30 Mar 2009 11:48:44 -0600
Bjorn Helgaas <bjorn.helgaas@hp.com> wrote:

> This patch adds a .notify() method.  The presence of .notify() causes
> Linux/ACPI to manage event handlers and notify handlers on our behalf,
> so we don't have to install and remove them ourselves.
> 
> Signed-off-by: Bjorn Helgaas <bjorn.helgaas@hp.com>
> CC: Jonathan Woithe <jwoithe@physics.adelaide.edu.au>
Acked-By: Tony Vroon <tony@linx.net>

Application panel button events continue to be delivered with
this patchset applied to 2.6.29; test platform is a Fujitsu-Siemens
Lifebook S6420 [FJNB1E6] with BIOS 1.18 (01/09/2009).
The code also gets much easier to read. Thanks for this Bjorn.

Regards,
Tony V.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.11 (GNU/Linux)

iEYEARECAAYFAknSjaoACgkQp5vW4rUFj5oqgwCgoLLIsu6csN2j/XTpXO3cs+Aw
vtgAnRMc3C2kyLbzy5RidX7FLqewGW6M
=BoMb
-----END PGP SIGNATURE-----

Re: [PATCH 07/10] fujitsu-laptop: use .notify method instead of installing handler directly

[Jonathan Woithe]

> This patch adds a .notify() method.  The presence of .notify() causes
> Linux/ACPI to manage event handlers and notify handlers on our behalf,
> so we don't have to install and remove them ourselves.
> 
> Signed-off-by: Bjorn Helgaas <bjorn.helgaas@hp.com>
> CC: Jonathan Woithe <jwoithe@physics.adelaide.edu.au>
> CC: Tony Vroon <tony@linx.net>

This patch also looks to be fine (machine is S7020).  Thanks again Bjorn.

Acked-by: Jonathan Woithe <jwoithe@physics.adelaide.edu.au>

Regards
  jonathan

> ---
>  drivers/platform/x86/fujitsu-laptop.c |   32 +++++---------------------------
>  1 files changed, 5 insertions(+), 27 deletions(-)
> 
> diff --git a/drivers/platform/x86/fujitsu-laptop.c b/drivers/platform/x86/fujitsu-laptop.c
> index 10f8796..218b9a1 100644
> --- a/drivers/platform/x86/fujitsu-laptop.c
> +++ b/drivers/platform/x86/fujitsu-laptop.c
> @@ -174,8 +174,7 @@ struct fujitsu_hotkey_t {
>  
>  static struct fujitsu_hotkey_t *fujitsu_hotkey;
>  
> -static void acpi_fujitsu_hotkey_notify(acpi_handle handle, u32 event,
> -				       void *data);
> +static void acpi_fujitsu_hotkey_notify(struct acpi_device *device, u32 event);
>  
>  #ifdef CONFIG_LEDS_CLASS
>  static enum led_brightness logolamp_get(struct led_classdev *cdev);
> @@ -816,7 +815,6 @@ static void acpi_fujitsu_notify(struct acpi_device *device, u32 event)
>  
>  static int acpi_fujitsu_hotkey_add(struct acpi_device *device)
>  {
> -	acpi_status status;
>  	acpi_handle handle;
>  	int result = 0;
>  	int state = 0;
> @@ -833,17 +831,6 @@ static int acpi_fujitsu_hotkey_add(struct acpi_device *device)
>  	sprintf(acpi_device_class(device), "%s", ACPI_FUJITSU_CLASS);
>  	device->driver_data = fujitsu_hotkey;
>  
> -	status = acpi_install_notify_handler(device->handle,
> -					     ACPI_DEVICE_NOTIFY,
> -					     acpi_fujitsu_hotkey_notify,
> -					     fujitsu_hotkey);
> -
> -	if (ACPI_FAILURE(status)) {
> -		printk(KERN_ERR "Error installing notify handler\n");
> -		error = -ENODEV;
> -		goto err_stop;
> -	}
> -
>  	/* kfifo */
>  	spin_lock_init(&fujitsu_hotkey->fifo_lock);
>  	fujitsu_hotkey->fifo =
> @@ -858,7 +845,7 @@ static int acpi_fujitsu_hotkey_add(struct acpi_device *device)
>  	fujitsu_hotkey->input = input = input_allocate_device();
>  	if (!input) {
>  		error = -ENOMEM;
> -		goto err_uninstall_notify;
> +		goto err_free_fifo;
>  	}
>  
>  	snprintf(fujitsu_hotkey->phys, sizeof(fujitsu_hotkey->phys),
> @@ -954,9 +941,7 @@ static int acpi_fujitsu_hotkey_add(struct acpi_device *device)
>  end:
>  err_free_input_dev:
>  	input_free_device(input);
> -err_uninstall_notify:
> -	acpi_remove_notify_handler(device->handle, ACPI_DEVICE_NOTIFY,
> -				   acpi_fujitsu_hotkey_notify);
> +err_free_fifo:
>  	kfifo_free(fujitsu_hotkey->fifo);
>  err_stop:
>  
> @@ -965,7 +950,6 @@ err_stop:
>  
>  static int acpi_fujitsu_hotkey_remove(struct acpi_device *device, int type)
>  {
> -	acpi_status status;
>  	struct fujitsu_hotkey_t *fujitsu_hotkey = NULL;
>  
>  	if (!device || !acpi_driver_data(device))
> @@ -973,10 +957,6 @@ static int acpi_fujitsu_hotkey_remove(struct acpi_device *device, int type)
>  
>  	fujitsu_hotkey = acpi_driver_data(device);
>  
> -	status = acpi_remove_notify_handler(fujitsu_hotkey->acpi_handle,
> -					    ACPI_DEVICE_NOTIFY,
> -					    acpi_fujitsu_hotkey_notify);
> -
>  	fujitsu_hotkey->acpi_handle = NULL;
>  
>  	kfifo_free(fujitsu_hotkey->fifo);
> @@ -984,8 +964,7 @@ static int acpi_fujitsu_hotkey_remove(struct acpi_device *device, int type)
>  	return 0;
>  }
>  
> -static void acpi_fujitsu_hotkey_notify(acpi_handle handle, u32 event,
> -				       void *data)
> +static void acpi_fujitsu_hotkey_notify(struct acpi_device *device, u32 event)
>  {
>  	struct input_dev *input;
>  	int keycode, keycode_r;
> @@ -1068,8 +1047,6 @@ static void acpi_fujitsu_hotkey_notify(acpi_handle handle, u32 event,
>  		input_sync(input);
>  		break;
>  	}
> -
> -	return;
>  }
>  
>  /* Initialization */
> @@ -1102,6 +1079,7 @@ static struct acpi_driver acpi_fujitsu_hotkey_driver = {
>  	.ops = {
>  		.add = acpi_fujitsu_hotkey_add,
>  		.remove = acpi_fujitsu_hotkey_remove,
> +		.notify = acpi_fujitsu_hotkey_notify,
>  		},
>  };
>  
> 

[PATCH 08/10] panasonic-laptop: use .notify method instead of installing handler directly

[Bjorn Helgaas]

This patch adds a .notify() method.  The presence of .notify() causes
Linux/ACPI to manage event handlers and notify handlers on our behalf,
so we don't have to install and remove them ourselves.

Signed-off-by: Bjorn Helgaas <bjorn.helgaas@hp.com>
CC: Harald Welte <laforge@gnumonks.org>
---
 drivers/platform/x86/panasonic-laptop.c |   26 +++++---------------------
 1 files changed, 5 insertions(+), 21 deletions(-)

diff --git a/drivers/platform/x86/panasonic-laptop.c b/drivers/platform/x86/panasonic-laptop.c
index c47a44d..6356031 100644
--- a/drivers/platform/x86/panasonic-laptop.c
+++ b/drivers/platform/x86/panasonic-laptop.c
@@ -176,6 +176,7 @@ enum SINF_BITS { SINF_NUM_BATTERIES = 0,
 static int acpi_pcc_hotkey_add(struct acpi_device *device);
 static int acpi_pcc_hotkey_remove(struct acpi_device *device, int type);
 static int acpi_pcc_hotkey_resume(struct acpi_device *device);
+static void acpi_pcc_hotkey_notify(struct acpi_device *device, u32 event);
 
 static const struct acpi_device_id pcc_device_ids[] = {
 	{ "MAT0012", 0},
@@ -193,6 +194,7 @@ static struct acpi_driver acpi_pcc_driver = {
 				.add =		acpi_pcc_hotkey_add,
 				.remove =	acpi_pcc_hotkey_remove,
 				.resume =       acpi_pcc_hotkey_resume,
+				.notify =	acpi_pcc_hotkey_notify,
 			},
 };
 
@@ -526,9 +528,9 @@ static void acpi_pcc_generate_keyinput(struct pcc_acpi *pcc)
 	return;
 }
 
-static void acpi_pcc_hotkey_notify(acpi_handle handle, u32 event, void *data)
+static void acpi_pcc_hotkey_notify(struct acpi_device *device, u32 event)
 {
-	struct pcc_acpi *pcc = (struct pcc_acpi *) data;
+	struct pcc_acpi *pcc = acpi_driver_data(device);
 
 	switch (event) {
 	case HKEY_NOTIFY:
@@ -598,7 +600,6 @@ static int acpi_pcc_hotkey_resume(struct acpi_device *device)
 
 static int acpi_pcc_hotkey_add(struct acpi_device *device)
 {
-	acpi_status status;
 	struct pcc_acpi *pcc;
 	int num_sifr, result;
 
@@ -639,22 +640,11 @@ static int acpi_pcc_hotkey_add(struct acpi_device *device)
 		goto out_sinf;
 	}
 
-	/* initialize hotkey input device */
-	status = acpi_install_notify_handler(pcc->handle, ACPI_DEVICE_NOTIFY,
-					     acpi_pcc_hotkey_notify, pcc);
-
-	if (ACPI_FAILURE(status)) {
-		ACPI_DEBUG_PRINT((ACPI_DB_ERROR,
-				  "Error installing notify handler\n"));
-		result = -ENODEV;
-		goto out_input;
-	}
-
 	/* initialize backlight */
 	pcc->backlight = backlight_device_register("panasonic", NULL, pcc,
 						   &pcc_backlight_ops);
 	if (IS_ERR(pcc->backlight))
-		goto out_notify;
+		goto out_input;
 
 	if (!acpi_pcc_retrieve_biosdata(pcc, pcc->sinf)) {
 		ACPI_DEBUG_PRINT((ACPI_DB_ERROR,
@@ -679,9 +669,6 @@ static int acpi_pcc_hotkey_add(struct acpi_device *device)
 
 out_backlight:
 	backlight_device_unregister(pcc->backlight);
-out_notify:
-	acpi_remove_notify_handler(pcc->handle, ACPI_DEVICE_NOTIFY,
-				   acpi_pcc_hotkey_notify);
 out_input:
 	input_unregister_device(pcc->input_dev);
 	/* no need to input_free_device() since core input API refcount and
@@ -722,9 +709,6 @@ static int acpi_pcc_hotkey_remove(struct acpi_device *device, int type)
 
 	backlight_device_unregister(pcc->backlight);
 
-	acpi_remove_notify_handler(pcc->handle, ACPI_DEVICE_NOTIFY,
-				   acpi_pcc_hotkey_notify);
-
 	input_unregister_device(pcc->input_dev);
 	/* no need to input_free_device() since core input API refcount and
 	 * free()s the device */

[PATCH 09/10] sony-laptop: use .notify method instead of installing handler directly

[Bjorn Helgaas]

This patch adds a .notify() method.  The presence of .notify() causes
Linux/ACPI to manage event handlers and notify handlers on our behalf,
so we don't have to install and remove them ourselves.

Signed-off-by: Bjorn Helgaas <bjorn.helgaas@hp.com>
CC: Mattia Dongili <malattia@linux.it>
---
 drivers/platform/x86/sony-laptop.c |   32 +++++---------------------------
 1 files changed, 5 insertions(+), 27 deletions(-)

diff --git a/drivers/platform/x86/sony-laptop.c b/drivers/platform/x86/sony-laptop.c
index bc8996c..be98b1f 100644
--- a/drivers/platform/x86/sony-laptop.c
+++ b/drivers/platform/x86/sony-laptop.c
@@ -895,7 +895,7 @@ static const struct dmi_system_id sony_nc_ids[] = {
 /*
  * ACPI callbacks
  */
-static void sony_acpi_notify(acpi_handle handle, u32 event, void *data)
+static void sony_nc_notify(struct acpi_device *device, u32 event)
 {
 	struct sony_nc_event *evmap;
 	u32 ev = event;
@@ -913,8 +913,8 @@ static void sony_acpi_notify(acpi_handle handle, u32 event, void *data)
 		 * TODO: we may want to do the same for the older GHKE -need
 		 *       dmi list- so this snippet may become one more callback.
 		 */
-		if (acpi_callsetfunc(handle, "SN07", 0x0202, &result) < 0)
-			dprintk("sony_acpi_notify, unable to decode event 0x%.2x\n", ev);
+		if (acpi_callsetfunc(sony_nc_acpi_handle, "SN07", 0x0202, &result) < 0)
+			dprintk("sony_nc_notify, unable to decode event 0x%.2x\n", ev);
 		else
 			ev = result & 0xFF;
 	}
@@ -927,7 +927,7 @@ static void sony_acpi_notify(acpi_handle handle, u32 event, void *data)
 			}
 		}
 
-	dprintk("sony_acpi_notify, event: 0x%.2x\n", ev);
+	dprintk("sony_nc_notify, event: 0x%.2x\n", ev);
 	sony_laptop_report_input_event(ev);
 	acpi_bus_generate_proc_event(sony_nc_acpi_device, 1, ev);
 }
@@ -1032,15 +1032,6 @@ static int sony_nc_add(struct acpi_device *device)
 		goto outwalk;
 	}
 
-	status = acpi_install_notify_handler(sony_nc_acpi_handle,
-					     ACPI_DEVICE_NOTIFY,
-					     sony_acpi_notify, NULL);
-	if (ACPI_FAILURE(status)) {
-		printk(KERN_WARNING DRV_PFX "unable to install notify handler (%u)\n", status);
-		result = -ENODEV;
-		goto outinput;
-	}
-
 	if (acpi_video_backlight_support()) {
 		printk(KERN_INFO DRV_PFX "brightness ignored, must be "
 		       "controlled by ACPI video driver\n");
@@ -1121,13 +1112,6 @@ static int sony_nc_add(struct acpi_device *device)
 	if (sony_backlight_device)
 		backlight_device_unregister(sony_backlight_device);
 
-	status = acpi_remove_notify_handler(sony_nc_acpi_handle,
-					    ACPI_DEVICE_NOTIFY,
-					    sony_acpi_notify);
-	if (ACPI_FAILURE(status))
-		printk(KERN_WARNING DRV_PFX "unable to remove notify handler\n");
-
-      outinput:
 	sony_laptop_remove_input();
 
       outwalk:
@@ -1136,7 +1120,6 @@ static int sony_nc_add(struct acpi_device *device)
 
 static int sony_nc_remove(struct acpi_device *device, int type)
 {
-	acpi_status status;
 	struct sony_nc_value *item;
 
 	if (sony_backlight_device)
@@ -1144,12 +1127,6 @@ static int sony_nc_remove(struct acpi_device *device, int type)
 
 	sony_nc_acpi_device = NULL;
 
-	status = acpi_remove_notify_handler(sony_nc_acpi_handle,
-					    ACPI_DEVICE_NOTIFY,
-					    sony_acpi_notify);
-	if (ACPI_FAILURE(status))
-		printk(KERN_WARNING DRV_PFX "unable to remove notify handler\n");
-
 	for (item = sony_nc_values; item->name; ++item) {
 		device_remove_file(&sony_pf_device->dev, &item->devattr);
 	}
@@ -1182,6 +1159,7 @@ static struct acpi_driver sony_nc_driver = {
 		.add = sony_nc_add,
 		.remove = sony_nc_remove,
 		.resume = sony_nc_resume,
+		.notify = sony_nc_notify,
 		},
 };
 

Re: [PATCH 09/10] sony-laptop: use .notify method instead of installing handler directly

[Mattia Dongili]

On Mon, Mar 30, 2009 at 11:48:54AM -0600, Bjorn Helgaas wrote:
> This patch adds a .notify() method.  The presence of .notify() causes
> Linux/ACPI to manage event handlers and notify handlers on our behalf,
> so we don't have to install and remove them ourselves.
> 
> Signed-off-by: Bjorn Helgaas <bjorn.helgaas@hp.com>
> CC: Mattia Dongili <malattia@linux.it>

looks fine to me, the only thing is that the first 3 hunks may not apply
(cleanly?) due to a few recent changes not yet in Linus tree (but in
Len's).

Feel free to add my
Signed-off-by: Mattia Dongili <malattia@linux.it>

> ---
>  drivers/platform/x86/sony-laptop.c |   32 +++++---------------------------
>  1 files changed, 5 insertions(+), 27 deletions(-)
> 
> diff --git a/drivers/platform/x86/sony-laptop.c b/drivers/platform/x86/sony-laptop.c
> index bc8996c..be98b1f 100644
> --- a/drivers/platform/x86/sony-laptop.c
> +++ b/drivers/platform/x86/sony-laptop.c
> @@ -895,7 +895,7 @@ static const struct dmi_system_id sony_nc_ids[] = {
>  /*
>   * ACPI callbacks
>   */
> -static void sony_acpi_notify(acpi_handle handle, u32 event, void *data)
> +static void sony_nc_notify(struct acpi_device *device, u32 event)
>  {
>  	struct sony_nc_event *evmap;
>  	u32 ev = event;
> @@ -913,8 +913,8 @@ static void sony_acpi_notify(acpi_handle handle, u32 event, void *data)
>  		 * TODO: we may want to do the same for the older GHKE -need
>  		 *       dmi list- so this snippet may become one more callback.
>  		 */
> -		if (acpi_callsetfunc(handle, "SN07", 0x0202, &result) < 0)
> -			dprintk("sony_acpi_notify, unable to decode event 0x%.2x\n", ev);
> +		if (acpi_callsetfunc(sony_nc_acpi_handle, "SN07", 0x0202, &result) < 0)
> +			dprintk("sony_nc_notify, unable to decode event 0x%.2x\n", ev);
>  		else
>  			ev = result & 0xFF;
>  	}
> @@ -927,7 +927,7 @@ static void sony_acpi_notify(acpi_handle handle, u32 event, void *data)
>  			}
>  		}
>  
> -	dprintk("sony_acpi_notify, event: 0x%.2x\n", ev);
> +	dprintk("sony_nc_notify, event: 0x%.2x\n", ev);
>  	sony_laptop_report_input_event(ev);
>  	acpi_bus_generate_proc_event(sony_nc_acpi_device, 1, ev);
>  }
> @@ -1032,15 +1032,6 @@ static int sony_nc_add(struct acpi_device *device)
>  		goto outwalk;
>  	}
>  
> -	status = acpi_install_notify_handler(sony_nc_acpi_handle,
> -					     ACPI_DEVICE_NOTIFY,
> -					     sony_acpi_notify, NULL);
> -	if (ACPI_FAILURE(status)) {
> -		printk(KERN_WARNING DRV_PFX "unable to install notify handler (%u)\n", status);
> -		result = -ENODEV;
> -		goto outinput;
> -	}
> -
>  	if (acpi_video_backlight_support()) {
>  		printk(KERN_INFO DRV_PFX "brightness ignored, must be "
>  		       "controlled by ACPI video driver\n");
> @@ -1121,13 +1112,6 @@ static int sony_nc_add(struct acpi_device *device)
>  	if (sony_backlight_device)
>  		backlight_device_unregister(sony_backlight_device);
>  
> -	status = acpi_remove_notify_handler(sony_nc_acpi_handle,
> -					    ACPI_DEVICE_NOTIFY,
> -					    sony_acpi_notify);
> -	if (ACPI_FAILURE(status))
> -		printk(KERN_WARNING DRV_PFX "unable to remove notify handler\n");
> -
> -      outinput:
>  	sony_laptop_remove_input();
>  
>        outwalk:
> @@ -1136,7 +1120,6 @@ static int sony_nc_add(struct acpi_device *device)
>  
>  static int sony_nc_remove(struct acpi_device *device, int type)
>  {
> -	acpi_status status;
>  	struct sony_nc_value *item;
>  
>  	if (sony_backlight_device)
> @@ -1144,12 +1127,6 @@ static int sony_nc_remove(struct acpi_device *device, int type)
>  
>  	sony_nc_acpi_device = NULL;
>  
> -	status = acpi_remove_notify_handler(sony_nc_acpi_handle,
> -					    ACPI_DEVICE_NOTIFY,
> -					    sony_acpi_notify);
> -	if (ACPI_FAILURE(status))
> -		printk(KERN_WARNING DRV_PFX "unable to remove notify handler\n");
> -
>  	for (item = sony_nc_values; item->name; ++item) {
>  		device_remove_file(&sony_pf_device->dev, &item->devattr);
>  	}
> @@ -1182,6 +1159,7 @@ static struct acpi_driver sony_nc_driver = {
>  		.add = sony_nc_add,
>  		.remove = sony_nc_remove,
>  		.resume = sony_nc_resume,
> +		.notify = sony_nc_notify,
>  		},
>  };
>  
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
-- 
mattia
:wq!

Re: [PATCH 09/10] sony-laptop: use .notify method instead of installing handler directly

[Mattia Dongili]

On Tue, Mar 31, 2009 at 10:46:13PM +0900, Mattia Dongili wrote:
> On Mon, Mar 30, 2009 at 11:48:54AM -0600, Bjorn Helgaas wrote:
> > This patch adds a .notify() method.  The presence of .notify() causes
> > Linux/ACPI to manage event handlers and notify handlers on our behalf,
> > so we don't have to install and remove them ourselves.
> > 
> > Signed-off-by: Bjorn Helgaas <bjorn.helgaas@hp.com>
> > CC: Mattia Dongili <malattia@linux.it>
> 
> looks fine to me, the only thing is that the first 3 hunks may not apply
> (cleanly?) due to a few recent changes not yet in Linus tree (but in
> Len's).
> 
> Feel free to add my
> Signed-off-by: Mattia Dongili <malattia@linux.it>
yeah well, should be
Acked-by: Mattia Dongili <malattia@linux.it>
-- 
mattia
:wq!

[PATCH 10/10] ACPI: WMI: use .notify method instead of installing handler directly

[Bjorn Helgaas]

This patch adds a .notify() method.  The presence of .notify() causes
Linux/ACPI to manage event handlers and notify handlers on our behalf,
so we don't have to install and remove them ourselves.

Signed-off-by: Bjorn Helgaas <bjorn.helgaas@hp.com>
CC: Carlos Corbacho <carlos@strangeworlds.co.uk>
---
 drivers/platform/x86/wmi.c |   15 +++------------
 1 files changed, 3 insertions(+), 12 deletions(-)

diff --git a/drivers/platform/x86/wmi.c b/drivers/platform/x86/wmi.c
index 2f269e1..043b208 100644
--- a/drivers/platform/x86/wmi.c
+++ b/drivers/platform/x86/wmi.c
@@ -81,6 +81,7 @@ static struct wmi_block wmi_blocks;
 
 static int acpi_wmi_remove(struct acpi_device *device, int type);
 static int acpi_wmi_add(struct acpi_device *device);
+static void acpi_wmi_notify(struct acpi_device *device, u32 event);
 
 static const struct acpi_device_id wmi_device_ids[] = {
 	{"PNP0C14", 0},
@@ -96,6 +97,7 @@ static struct acpi_driver acpi_wmi_driver = {
 	.ops = {
 		.add = acpi_wmi_add,
 		.remove = acpi_wmi_remove,
+		.notify = acpi_wmi_notify,
 		},
 };
 
@@ -643,12 +645,11 @@ acpi_wmi_ec_space_handler(u32 function, acpi_physical_address address,
 	}
 }
 
-static void acpi_wmi_notify(acpi_handle handle, u32 event, void *data)
+static void acpi_wmi_notify(struct acpi_device *device, u32 event)
 {
 	struct guid_block *block;
 	struct wmi_block *wblock;
 	struct list_head *p;
-	struct acpi_device *device = data;
 
 	list_for_each(p, &wmi_blocks.list) {
 		wblock = list_entry(p, struct wmi_block, list);
@@ -669,9 +670,6 @@ static void acpi_wmi_notify(acpi_handle handle, u32 event, void *data)
 
 static int acpi_wmi_remove(struct acpi_device *device, int type)
 {
-	acpi_remove_notify_handler(device->handle, ACPI_DEVICE_NOTIFY,
-		acpi_wmi_notify);
-
 	acpi_remove_address_space_handler(device->handle,
 				ACPI_ADR_SPACE_EC, &acpi_wmi_ec_space_handler);
 
@@ -683,13 +681,6 @@ static int __init acpi_wmi_add(struct acpi_device *device)
 	acpi_status status;
 	int result = 0;
 
-	status = acpi_install_notify_handler(device->handle, ACPI_DEVICE_NOTIFY,
-		acpi_wmi_notify, device);
-	if (ACPI_FAILURE(status)) {
-		printk(KERN_ERR PREFIX "Error installing notify handler\n");
-		return -ENODEV;
-	}
-
 	status = acpi_install_address_space_handler(device->handle,
 						    ACPI_ADR_SPACE_EC,
 						    &acpi_wmi_ec_space_handler,