[PATCH 1/3] UBUNTU: SAUCE: Allow registration of handler to multiple WMI events with same GUID

Eric Miao eric.miao at canonical.com
Sun Nov 21 12:30:21 UTC 2010


On Sat, Nov 20, 2010 at 3:16 AM, Colin King <colin.king at canonical.com> wrote:
> From: Colin Ian King <colin.king at canonical.com>
>
> BugLink: http://bugs.launchpad.net/bugs/676997
>
> WMI data blocks can contain WMI events with the same GUID but with
> different notifiy_ids.  This patch enables a single event handler
> to be registered and unregistered against all events against with
> the same GUID.  The event handler is passed the notify_id of these
> events and hence can differentiate between the differen events. The
> patch also ensures we only register and unregister a device per
> unique GUID.
>
> The original registration implementation just matched on the first
> event with the matching GUID and left the other events with out a
> handler.
>
> Signed-off-by: Eric Miao <eric.miao at canonical.com>
> Signed-off-by: Colin Ian King <colin.king at canonical.com>

Colin,

Thanks for the patch, it is very straight-forward and clean, much more
carefully considered in overall.

Acked.

> ---
>  drivers/platform/x86/wmi.c |  131 ++++++++++++++++++++++++++------------------
>  1 files changed, 78 insertions(+), 53 deletions(-)
>
> diff --git a/drivers/platform/x86/wmi.c b/drivers/platform/x86/wmi.c
> index e4eaa14..70526ca 100644
> --- a/drivers/platform/x86/wmi.c
> +++ b/drivers/platform/x86/wmi.c
> @@ -68,6 +68,7 @@ struct wmi_block {
>        wmi_notify_handler handler;
>        void *handler_data;
>        struct device *dev;
> +       bool first_instance;
>  };
>
>  static struct wmi_block wmi_blocks;
> @@ -556,21 +557,34 @@ acpi_status wmi_install_notify_handler(const char *guid,
>  wmi_notify_handler handler, void *data)
>  {
>        struct wmi_block *block;
> -       acpi_status status;
> +       acpi_status status = AE_NOT_EXIST;
> +       char tmp[16], guid_input[16];
> +       struct list_head *p;
>
>        if (!guid || !handler)
>                return AE_BAD_PARAMETER;
>
> -       if (!find_guid(guid, &block))
> -               return AE_NOT_EXIST;
> +       wmi_parse_guid(guid, tmp);
> +       wmi_swap_bytes(tmp, guid_input);
> +
> +       list_for_each(p, &wmi_blocks.list) {
> +               acpi_status wmi_status;
> +               block = list_entry(p, struct wmi_block, list);
>
> -       if (block->handler && block->handler != wmi_notify_debug)
> -               return AE_ALREADY_ACQUIRED;
> +               if (memcmp(block->gblock.guid, guid_input, 16) == 0) {
> +                       if (block->handler &&
> +                           block->handler != wmi_notify_debug)
> +                               return AE_ALREADY_ACQUIRED;
>
> -       block->handler = handler;
> -       block->handler_data = data;
> +                       block->handler = handler;
> +                       block->handler_data = data;
>
> -       status = wmi_method_enable(block, 1);
> +                       wmi_status = wmi_method_enable(block, 1);
> +                       if ((wmi_status != AE_OK) ||
> +                           ((wmi_status == AE_OK) && (status == AE_NOT_EXIST)))
> +                               status = wmi_status;
> +               }
> +       }
>
>        return status;
>  }
> @@ -584,23 +598,38 @@ EXPORT_SYMBOL_GPL(wmi_install_notify_handler);
>  acpi_status wmi_remove_notify_handler(const char *guid)
>  {
>        struct wmi_block *block;
> -       acpi_status status = AE_OK;
> +       acpi_status status = AE_NOT_EXIST;
> +       char tmp[16], guid_input[16];
> +       struct list_head *p;
>
>        if (!guid)
>                return AE_BAD_PARAMETER;
>
> -       if (!find_guid(guid, &block))
> -               return AE_NOT_EXIST;
> +       wmi_parse_guid(guid, tmp);
> +       wmi_swap_bytes(tmp, guid_input);
> +
> +       list_for_each(p, &wmi_blocks.list) {
> +               acpi_status wmi_status;
> +               block = list_entry(p, struct wmi_block, list);
>
> -       if (!block->handler || block->handler == wmi_notify_debug)
> -               return AE_NULL_ENTRY;
> +               if (memcmp(block->gblock.guid, guid_input, 16) == 0) {
> +                       if (!block->handler ||
> +                           block->handler == wmi_notify_debug)
> +                               return AE_NULL_ENTRY;
>
> -       if (debug_event) {
> -               block->handler = wmi_notify_debug;
> -       } else {
> -               status = wmi_method_enable(block, 0);
> -               block->handler = NULL;
> -               block->handler_data = NULL;
> +                       if (debug_event) {
> +                               block->handler = wmi_notify_debug;
> +                               status = AE_OK;
> +                       } else {
> +                               wmi_status = wmi_method_enable(block, 0);
> +                               block->handler = NULL;
> +                               block->handler_data = NULL;
> +                               if ((wmi_status != AE_OK) ||
> +                                   ((wmi_status == AE_OK) &&
> +                                    (status == AE_NOT_EXIST)))
> +                                       status = wmi_status;
> +                       }
> +               }
>        }
>        return status;
>  }
> @@ -717,28 +746,34 @@ static int wmi_create_devs(void)
>        /* Create devices for all the GUIDs */
>        list_for_each(p, &wmi_blocks.list) {
>                wblock = list_entry(p, struct wmi_block, list);
> +               /*
> +                * Only create device on first instance, as subsequent
> +                * instances share the same GUID and we need to avoid
> +                * creating multiple devices with the same GUID
> +                */
> +               if (wblock->first_instance) {
> +                       guid_dev = kzalloc(sizeof(struct device), GFP_KERNEL);
> +                       if (!guid_dev)
> +                               return -ENOMEM;
>
> -               guid_dev = kzalloc(sizeof(struct device), GFP_KERNEL);
> -               if (!guid_dev)
> -                       return -ENOMEM;
> -
> -               wblock->dev = guid_dev;
> +                       wblock->dev = guid_dev;
>
> -               guid_dev->class = &wmi_class;
> -               dev_set_drvdata(guid_dev, wblock);
> +                       guid_dev->class = &wmi_class;
> +                       dev_set_drvdata(guid_dev, wblock);
>
> -               gblock = &wblock->gblock;
> +                       gblock = &wblock->gblock;
>
> -               wmi_gtoa(gblock->guid, guid_string);
> -               dev_set_name(guid_dev, guid_string);
> +                       wmi_gtoa(gblock->guid, guid_string);
> +                       dev_set_name(guid_dev, guid_string);
>
> -               result = device_register(guid_dev);
> -               if (result)
> -                       return result;
> +                       result = device_register(guid_dev);
> +                       if (result)
> +                               return result;
>
> -               result = device_create_file(guid_dev, &dev_attr_modalias);
> -               if (result)
> -                       return result;
> +                       result = device_create_file(guid_dev, &dev_attr_modalias);
> +                       if (result)
> +                               return result;
> +               }
>        }
>
>        return 0;
> @@ -755,12 +790,14 @@ static void wmi_remove_devs(void)
>        list_for_each(p, &wmi_blocks.list) {
>                wblock = list_entry(p, struct wmi_block, list);
>
> -               guid_dev = wblock->dev;
> -               gblock = &wblock->gblock;
> +               if (wblock->first_instance) {
> +                       guid_dev = wblock->dev;
> +                       gblock = &wblock->gblock;
>
> -               device_remove_file(guid_dev, &dev_attr_modalias);
> +                       device_remove_file(guid_dev, &dev_attr_modalias);
>
> -               device_unregister(guid_dev);
> +                       device_unregister(guid_dev);
> +               }
>        }
>  }
>
> @@ -831,19 +868,6 @@ static __init acpi_status parse_wdg(acpi_handle handle)
>                return AE_NO_MEMORY;
>
>        for (i = 0; i < total; i++) {
> -               /*
> -                 Some WMI devices, like those for nVidia hooks, have a
> -                 duplicate GUID. It's not clear what we should do in this
> -                 case yet, so for now, we'll just ignore the duplicate.
> -                 Anyone who wants to add support for that device can come
> -                 up with a better workaround for the mess then.
> -               */
> -               if (guid_already_parsed(gblock[i].guid) == true) {
> -                       wmi_gtoa(gblock[i].guid, guid_string);
> -                       printk(KERN_INFO PREFIX "Skipping duplicate GUID %s\n",
> -                               guid_string);
> -                       continue;
> -               }
>                if (debug_dump_wdg)
>                        wmi_dump_wdg(&gblock[i]);
>
> @@ -851,6 +875,7 @@ static __init acpi_status parse_wdg(acpi_handle handle)
>                if (!wblock)
>                        return AE_NO_MEMORY;
>
> +               wblock->first_instance = !guid_already_parsed(gblock[i].guid);
>                wblock->gblock = gblock[i];
>                wblock->handle = handle;
>                if (debug_event) {
> --
> 1.7.0.4
>
>
> --
> kernel-team mailing list
> kernel-team at lists.ubuntu.com
> https://lists.ubuntu.com/mailman/listinfo/kernel-team
>


More information about the kernel-team mailing list