> WMI is logically a bus: the WMI driver binds to an ACPI node (or
> more than one), and each instance of the WMI driver enumerates its
> children and hopes that drivers will attach to the children that are
> useful.
>
> This patch gives WMI a driver model bus type and the ability to
> match to drivers. The bus itself is a device in the new "wmi_bus"
> class, and all of the individual WMI devices are slotted into the
> device hierarchy correctly.
>
> Signed-off-by: Andy Lutomirski <luto@xxxxxxxxxx>
> ---
> drivers/platform/x86/wmi.c | 198 +++++++++++++++++++++++++++++++++++----------
> include/linux/wmi.h | 47 +++++++++++
> 2 files changed, 202 insertions(+), 43 deletions(-)
> create mode 100644 include/linux/wmi.h
>
> diff --git a/drivers/platform/x86/wmi.c b/drivers/platform/x86/wmi.c
> index dcb34a5f5669..2fa9493bd35c 100644
> --- a/drivers/platform/x86/wmi.c
> +++ b/drivers/platform/x86/wmi.c
> @@ -37,14 +37,13 @@
> #include <linux/acpi.h>
> #include <linux/slab.h>
> #include <linux/module.h>
> +#include <linux/wmi.h>
>
> ACPI_MODULE_NAME("wmi");
> MODULE_AUTHOR("Carlos Corbacho");
> MODULE_DESCRIPTION("ACPI-WMI Mapping Driver");
> MODULE_LICENSE("GPL");
>
> -#define ACPI_WMI_CLASS "wmi"
> -
> static LIST_HEAD(wmi_block_list);
>
> struct guid_block {
> @@ -61,12 +60,12 @@ struct guid_block {
> };
>
> struct wmi_block {
> + struct wmi_device dev;
> struct list_head list;
> struct guid_block gblock;
> struct acpi_device *acpi_device;
> wmi_notify_handler handler;
> void *handler_data;
> - struct device dev;
> };
>
>
> @@ -101,8 +100,8 @@ static const struct acpi_device_id wmi_device_ids[] = {
> MODULE_DEVICE_TABLE(acpi, wmi_device_ids);
>
> static struct acpi_driver acpi_wmi_driver = {
> - .name = "wmi",
> - .class = ACPI_WMI_CLASS,
> + .name = "acpi-wmi",
> + .owner = THIS_MODULE,
> .ids = wmi_device_ids,
> .ops = {
> .add = acpi_wmi_add,
> @@ -623,77 +622,146 @@ bool wmi_has_guid(const char *guid_string)
> }
> EXPORT_SYMBOL_GPL(wmi_has_guid);
>
> +static struct wmi_block *dev_to_wblock(struct device *dev)
> +{
> + return container_of(dev, struct wmi_block, dev.dev);
> +}
> +
> +static struct wmi_device *dev_to_wdev(struct device *dev)
> +{
> + return container_of(dev, struct wmi_device, dev);
> +}
> +
> /*
> * sysfs interface
> */
> static ssize_t modalias_show(struct device *dev, struct device_attribute *attr,
> char *buf)
> {
> - struct wmi_block *wblock;
> -
> - wblock = dev_get_drvdata(dev);
> - if (!wblock) {
> - strcat(buf, "\n");
> - return strlen(buf);
> - }
> + struct wmi_block *wblock = dev_to_wblock(dev);
>
> return sprintf(buf, "wmi:%pUL\n", wblock->gblock.guid);
> }
> static DEVICE_ATTR_RO(modalias);
>
> +static ssize_t guid_show(struct device *dev, struct device_attribute *attr,
> + char *buf)
> +{
> + struct wmi_block *wblock = dev_to_wblock(dev);
> +
> + return sprintf(buf, "%pUL\n", wblock->gblock.guid);
> +}
> +static DEVICE_ATTR_RO(guid);
> +
> static struct attribute *wmi_attrs[] = {
> &dev_attr_modalias.attr,
> + &dev_attr_guid.attr,
> NULL,
> };
> ATTRIBUTE_GROUPS(wmi);
>
> static int wmi_dev_uevent(struct device *dev, struct kobj_uevent_env *env)
> {
> - char guid_string[37];
> -
> - struct wmi_block *wblock;
> + struct wmi_block *wblock = dev_to_wblock(dev);
>
> - if (add_uevent_var(env, "MODALIAS="))
> + if (add_uevent_var(env, "MODALIAS=wmi:%pUL", wblock->gblock.guid))
> return -ENOMEM;
>
> - wblock = dev_get_drvdata(dev);
> - if (!wblock)
> + if (add_uevent_var(env, "WMI_GUID=%pUL", wblock->gblock.guid))
> return -ENOMEM;
>
> - sprintf(guid_string, "%pUL", wblock->gblock.guid);
> + return 0;
> +}
> +
> +static void wmi_dev_release(struct device *dev)
> +{
> + struct wmi_block *wblock = dev_to_wblock(dev);
> +
> + kfree(wblock);
> +}
> +
> +static int wmi_dev_match(struct device *dev, struct device_driver *driver)
> +{
> + struct wmi_driver *wmi_driver =
> + container_of(driver, struct wmi_driver, driver);
> + struct wmi_block *wblock = dev_to_wblock(dev);
> + const struct wmi_device_id *id = wmi_driver->id_table;
>
> - strcpy(&env->buf[env->buflen - 1], "wmi:");
> - memcpy(&env->buf[env->buflen - 1 + 4], guid_string, 36);
> - env->buflen += 40;
> + while (id->guid_string) {
> + u8 tmp[16], driver_guid[16];
> +
> + wmi_parse_guid(id->guid_string, tmp);
> + wmi_swap_bytes(tmp, driver_guid);
> + if (!memcmp(driver_guid, wblock->gblock.guid, 16))
> + return 1;
> +
> + id++;
> + }
>
> return 0;
> }
>
> -static void wmi_dev_free(struct device *dev)
> +int wmi_dev_probe(struct device *dev)
Can be static.
> {
> - struct wmi_block *wmi_block = container_of(dev, struct wmi_block, dev);
> + struct wmi_block *wblock = dev_to_wblock(dev);
> + struct wmi_driver *wdriver =
> + container_of(dev->driver, struct wmi_driver, driver);
> + int ret = 0;
> +
> + if (ACPI_FAILURE(wmi_method_enable(wblock, 1)))
> + dev_warn(dev, "failed to enable device -- probing anyway\n");
> +
> + if (wdriver->probe) {
> + ret = wdriver->probe(dev_to_wdev(dev));
> + if (ret != 0 && ACPI_FAILURE(wmi_method_enable(wblock, 0)))
> + dev_warn(dev, "failed to disable device\n");
> + }
> +
> + return ret;
> +}
> +
> +int wmi_dev_remove(struct device *dev)
Can be static.
> +{
> + struct wmi_block *wblock = dev_to_wblock(dev);
> + struct wmi_driver *wdriver =
> + container_of(dev->driver, struct wmi_driver, driver);
> + int ret = 0;
> +
> + if (wdriver->remove)
> + ret = wdriver->remove(dev_to_wdev(dev));
> +
> + if (ACPI_FAILURE(wmi_method_enable(wblock, 0)))
> + dev_warn(dev, "failed to disable device\n");
>
> - kfree(wmi_block);
> + return ret;
> }
>
> -static struct class wmi_class = {
> +static struct class wmi_bus_class = {
> + .name = "wmi_bus",
> +};
> +
> +static struct bus_type wmi_bus_type = {
> .name = "wmi",
> - .dev_release = wmi_dev_free,
> - .dev_uevent = wmi_dev_uevent,
> .dev_groups = wmi_groups,
> + .match = wmi_dev_match,
> + .uevent = wmi_dev_uevent,
> + .probe = wmi_dev_probe,
> + .remove = wmi_dev_remove,
> };
>
> -static int wmi_create_device(const struct guid_block *gblock,
> +static int wmi_create_device(struct device *wmi_bus_dev,
> + const struct guid_block *gblock,
> struct wmi_block *wblock,
> struct acpi_device *device)
> {
> - wblock->dev.class = &wmi_class;
> + wblock->dev.dev.bus = &wmi_bus_type;
> + wblock->dev.dev.parent = wmi_bus_dev;
>
> - dev_set_name(&wblock->dev, "%pUL", gblock->guid);
> + dev_set_name(&wblock->dev.dev, "%pUL", gblock->guid);
>
> - dev_set_drvdata(&wblock->dev, wblock);
> + wblock->dev.dev.release = wmi_dev_release;
>
> - return device_register(&wblock->dev);
> + return device_register(&wblock->dev.dev);
> }
>
> static void wmi_free_devices(struct acpi_device *device)
> @@ -704,8 +772,8 @@ static void wmi_free_devices(struct acpi_device *device)
> list_for_each_entry_safe(wblock, next, &wmi_block_list, list) {
> if (wblock->acpi_device == device) {
> list_del(&wblock->list);
> - if (wblock->dev.class)
> - device_unregister(&wblock->dev);
> + if (wblock->dev.dev.bus)
> + device_unregister(&wblock->dev.dev);
> else
> kfree(wblock);
> }
> @@ -737,7 +805,7 @@ static bool guid_already_parsed(struct acpi_device *device,
> /*
> * Parse the _WDG method for the GUID data blocks
> */
> -static int parse_wdg(struct acpi_device *device)
> +static int parse_wdg(struct device *wmi_bus_dev, struct acpi_device *device)
> {
> struct acpi_buffer out = {ACPI_ALLOCATE_BUFFER, NULL};
> union acpi_object *obj;
> @@ -781,7 +849,8 @@ static int parse_wdg(struct acpi_device *device)
> for device creation.
> */
> if (!guid_already_parsed(device, gblock[i].guid)) {
> - retval = wmi_create_device(&gblock[i], wblock, device);
> + retval = wmi_create_device(wmi_bus_dev, &gblock[i],
> + wblock, device);
> if (retval) {
> wmi_free_devices(device);
> goto out_free_pointer;
> @@ -881,12 +950,15 @@ static int acpi_wmi_remove(struct acpi_device *device)
> acpi_remove_address_space_handler(device->handle,
> ACPI_ADR_SPACE_EC, &acpi_wmi_ec_space_handler);
> wmi_free_devices(device);
> + device_unregister((struct device *)acpi_driver_data(device));
> + device->driver_data = NULL;
>
> return 0;
> }
>
> static int acpi_wmi_add(struct acpi_device *device)
> {
> + struct device *wmi_bus_dev;
> acpi_status status;
> int error;
>
> @@ -899,14 +971,26 @@ static int acpi_wmi_add(struct acpi_device *device)
> return -ENODEV;
> }
>
> - error = parse_wdg(device);
> + wmi_bus_dev = device_create(&wmi_bus_class, &device->dev, MKDEV(0, 0),
> + NULL, "wmi_bus-%s", dev_name(&device->dev));
> + if (IS_ERR(wmi_bus_dev)) {
> + error = PTR_ERR(wmi_bus_dev);
> + goto err_remove_handler;
> + }
> + device->driver_data = wmi_bus_dev;
> +
> + error = parse_wdg(wmi_bus_dev, device);
> if (error) {
> pr_err("Failed to parse WDG method\n");
> - goto err_remove_handler;
> + goto err_remove_busdev;
> }
>
> return 0;
>
> +err_remove_busdev:
> + device_unregister(wmi_bus_dev);
> + put_device(wmi_bus_dev);
I believe this put_device() call is excessive. device_create() calls
device_initialize() and device_add(); device_unregister() calls
device_del() and put_device(), so it should already take care of
removing all references to wmi_bus_dev. Am I missing something?
--
Best regards,
Michał Kępień
--
To unsubscribe from this list: send the line "unsubscribe platform-driver-x86" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
