2017-09-08 15:24:32

by Mario Limonciello

[permalink] [raw]
Subject: [PATCH v3] Add driver to force WMI Thunderbolt controller power status

Current implementations of Intel Thunderbolt controllers will go
into a low power mode when not in use.

Many machines containing these controllers also have a GPIO wired up
that can force the controller awake. This is offered via a ACPI-WMI
interface intended to be manipulated by a userspace utility.

This mechanism is provided by Intel to OEMs to include in BIOS.
It uses an industry wide GUID that is populated in a separate _WDG
entry with no binary MOF.

This interface allows software such as fwupd to wake up thunderbolt
controllers to query the firmware version or flash new firmware.

Signed-off-by: Mario Limonciello <[email protected]>
---
changes from v2 to v3:
* Fix typographical error
* Send KOBJ_CHANGE event

changes from v1 to v2:
* Add ABI documentation
* Update thunderbolt.rst
* Remove unnecessary cast
* Remove unneeded whitespace
* Adjust references of "Intel Wmi thunderbolt" ->
"Intel WMI thunderbolt force power"

.../testing/sysfs-platform-intel-wmi-thunderbolt | 11 +++
Documentation/admin-guide/thunderbolt.rst | 15 +++
MAINTAINERS | 5 +
drivers/platform/x86/Kconfig | 13 +++
drivers/platform/x86/Makefile | 1 +
drivers/platform/x86/intel-wmi-thunderbolt.c | 101 +++++++++++++++++++++
6 files changed, 146 insertions(+)
create mode 100644 Documentation/ABI/testing/sysfs-platform-intel-wmi-thunderbolt
create mode 100644 drivers/platform/x86/intel-wmi-thunderbolt.c

diff --git a/Documentation/ABI/testing/sysfs-platform-intel-wmi-thunderbolt b/Documentation/ABI/testing/sysfs-platform-intel-wmi-thunderbolt
new file mode 100644
index 0000000..bf3dedb
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-platform-intel-wmi-thunderbolt
@@ -0,0 +1,11 @@
+What: /sys/devices/platform/<platform>/force_power
+Date: September 2017
+KernelVersion: 4.14
+Contact: "Mario Limonciello" <[email protected]>
+Description:
+ Modify the platform force power state, influencing
+ Thunderbolt controllers to turn on or off when no
+ devices are connected (write-only)
+ There are two available states:
+ * 0 -> Force power disabled
+ * 1 -> Force power enabled
diff --git a/Documentation/admin-guide/thunderbolt.rst b/Documentation/admin-guide/thunderbolt.rst
index 6a4cd1f..dadcd66 100644
--- a/Documentation/admin-guide/thunderbolt.rst
+++ b/Documentation/admin-guide/thunderbolt.rst
@@ -197,3 +197,18 @@ information is missing.

To recover from this mode, one needs to flash a valid NVM image to the
host host controller in the same way it is done in the previous chapter.
+
+Forcing power
+-------------
+Many OEMs include a method that can be used to force the power of a
+thunderbolt controller to an "On" state even if nothing is connected.
+If supported by your machine this will be exposed by the WMI bus with
+a sysfs attribute called "force_power".
+
+For example the intel-wmi-thunderbolt driver exposes this attribute in:
+ /sys/devices/platform/PNP0C14:00/wmi_bus/wmi_bus-PNP0C14:00/86CCFD48-205E-4A77-9C48-2021CBEDE341/force_power
+
+ To force the power to on, write 1 to this attribute file.
+ To disable force power, write 0 to this attribute file.
+
+Note: it's currently not possible to query the force power state of a platform.
diff --git a/MAINTAINERS b/MAINTAINERS
index 1c3feff..9a6b73e 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -3949,6 +3949,11 @@ M: Pali Rohár <[email protected]>
S: Maintained
F: drivers/platform/x86/dell-wmi.c

+INTEL WMI THUNDERBOLT FORCE POWER DRIVER
+M: Mario Limonciello <[email protected]>
+S: Maintained
+F: drivers/platform/x86/intel-wmi-thunderbolt.c
+
DELTA ST MEDIA DRIVER
M: Hugues Fruchet <[email protected]>
L: [email protected]
diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
index 80b8795..f401ae4 100644
--- a/drivers/platform/x86/Kconfig
+++ b/drivers/platform/x86/Kconfig
@@ -658,6 +658,19 @@ config WMI_BMOF
To compile this driver as a module, choose M here: the module will
be called wmi-bmof.

+config INTEL_WMI_THUNDERBOLT
+ tristate "Intel WMI thunderbolt force power driver"
+ depends on ACPI_WMI
+ default ACPI_WMI
+ ---help---
+ Say Y here if you want to be able to use the WMI interface on select
+ systems to force the power control of Intel Thunderbolt controllers.
+ This is useful for updating the firmware when devices are not plugged
+ into the controller.
+
+ To compile this driver as a module, choose M here: the module will
+ be called intel-wmi-thunderbolt.
+
config MSI_WMI
tristate "MSI WMI extras"
depends on ACPI_WMI
diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile
index 91cec17..2b315d0 100644
--- a/drivers/platform/x86/Makefile
+++ b/drivers/platform/x86/Makefile
@@ -39,6 +39,7 @@ obj-$(CONFIG_PEAQ_WMI) += peaq-wmi.o
obj-$(CONFIG_SURFACE3_WMI) += surface3-wmi.o
obj-$(CONFIG_TOPSTAR_LAPTOP) += topstar-laptop.o
obj-$(CONFIG_WMI_BMOF) += wmi-bmof.o
+obj-$(CONFIG_INTEL_WMI_THUNDERBOLT) += intel-wmi-thunderbolt.o

# toshiba_acpi must link after wmi to ensure that wmi devices are found
# before toshiba_acpi initializes
diff --git a/drivers/platform/x86/intel-wmi-thunderbolt.c b/drivers/platform/x86/intel-wmi-thunderbolt.c
new file mode 100644
index 0000000..32fb6cc
--- /dev/null
+++ b/drivers/platform/x86/intel-wmi-thunderbolt.c
@@ -0,0 +1,101 @@
+/*
+ * WMI Thunderbolt driver
+ *
+ * Copyright (C) 2017 Dell Inc. All Rights Reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 as published
+ * by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include <linux/acpi.h>
+#include <linux/device.h>
+#include <linux/fs.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/string.h>
+#include <linux/sysfs.h>
+#include <linux/types.h>
+#include <linux/wmi.h>
+
+#define INTEL_WMI_THUNDERBOLT_GUID "86CCFD48-205E-4A77-9C48-2021CBEDE341"
+
+static ssize_t force_power_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct acpi_buffer input;
+ acpi_status status;
+ u8 mode;
+
+ input.length = sizeof(u8);
+ input.pointer = &mode;
+ mode = hex_to_bin(buf[0]);
+ if (mode == 0 || mode == 1) {
+ status = wmi_evaluate_method(INTEL_WMI_THUNDERBOLT_GUID, 0, 1,
+ &input, NULL);
+ if (ACPI_FAILURE(status)) {
+ pr_err("intel-wmi-thunderbolt: failed setting %s\n",
+ buf);
+ return -ENODEV;
+ }
+ } else {
+ pr_err("intel-wmi-thunderbolt: unsupported mode: %d", mode);
+ }
+ return count;
+}
+
+static DEVICE_ATTR_WO(force_power);
+
+static struct attribute *tbt_attrs[] = {
+ &dev_attr_force_power.attr,
+ NULL
+};
+
+static const struct attribute_group tbt_attribute_group = {
+ .attrs = tbt_attrs,
+};
+
+static int intel_wmi_thunderbolt_probe(struct wmi_device *wdev)
+{
+ int ret;
+
+ ret = sysfs_create_group(&wdev->dev.kobj, &tbt_attribute_group);
+ kobject_uevent(&wdev->dev.kobj, KOBJ_CHANGE);
+ return ret;
+}
+
+static int intel_wmi_thunderbolt_remove(struct wmi_device *wdev)
+{
+ sysfs_remove_group(&wdev->dev.kobj, &tbt_attribute_group);
+ kobject_uevent(&wdev->dev.kobj, KOBJ_CHANGE);
+ return 0;
+}
+
+static const struct wmi_device_id intel_wmi_thunderbolt_id_table[] = {
+ { .guid_string = INTEL_WMI_THUNDERBOLT_GUID },
+ { },
+};
+
+static struct wmi_driver intel_wmi_thunderbolt_driver = {
+ .driver = {
+ .name = "intel-wmi-thunderbolt",
+ },
+ .probe = intel_wmi_thunderbolt_probe,
+ .remove = intel_wmi_thunderbolt_remove,
+ .id_table = intel_wmi_thunderbolt_id_table,
+};
+
+module_wmi_driver(intel_wmi_thunderbolt_driver);
+
+MODULE_ALIAS("wmi:" INTEL_WMI_THUNDERBOLT_GUID);
+MODULE_AUTHOR("Mario Limonciello <[email protected]>");
+MODULE_DESCRIPTION("Intel WMI Thunderbolt force power driver");
+MODULE_LICENSE("GPL");
--
2.7.4


2017-09-09 07:09:34

by Mika Westerberg

[permalink] [raw]
Subject: Re: [PATCH v3] Add driver to force WMI Thunderbolt controller power status

On Fri, Sep 08, 2017 at 10:23:11AM -0500, Mario Limonciello wrote:
> Current implementations of Intel Thunderbolt controllers will go
> into a low power mode when not in use.
>
> Many machines containing these controllers also have a GPIO wired up
> that can force the controller awake. This is offered via a ACPI-WMI
> interface intended to be manipulated by a userspace utility.
>
> This mechanism is provided by Intel to OEMs to include in BIOS.
> It uses an industry wide GUID that is populated in a separate _WDG
> entry with no binary MOF.
>
> This interface allows software such as fwupd to wake up thunderbolt
> controllers to query the firmware version or flash new firmware.
>
> Signed-off-by: Mario Limonciello <[email protected]>

Reviewed-by: Mika Westerberg <[email protected]>

2017-09-09 18:39:47

by Yehezkel Bernat

[permalink] [raw]
Subject: Re: [PATCH v3] Add driver to force WMI Thunderbolt controller power status

(Now in plain text. Sorry about that.)

On Sat, Sep 9, 2017 at 10:09 AM, Mika Westerberg
<[email protected]> wrote:
> On Fri, Sep 08, 2017 at 10:23:11AM -0500, Mario Limonciello wrote:
>> Current implementations of Intel Thunderbolt controllers will go
>> into a low power mode when not in use.
>>
>> Many machines containing these controllers also have a GPIO wired up
>> that can force the controller awake. This is offered via a ACPI-WMI
>> interface intended to be manipulated by a userspace utility.
>>
>> This mechanism is provided by Intel to OEMs to include in BIOS.
>> It uses an industry wide GUID that is populated in a separate _WDG
>> entry with no binary MOF.
>>
>> This interface allows software such as fwupd to wake up thunderbolt
>> controllers to query the firmware version or flash new firmware.
>>
>> Signed-off-by: Mario Limonciello <[email protected]>
>
> Reviewed-by: Mika Westerberg <[email protected]>

FWIW,

Reviewed-by: Yehezkel Bernat <[email protected]>

2017-09-11 21:45:03

by Darren Hart

[permalink] [raw]
Subject: Re: [PATCH v3] Add driver to force WMI Thunderbolt controller power status

On Fri, Sep 08, 2017 at 10:23:11AM -0500, Mario Limonciello wrote:
> Current implementations of Intel Thunderbolt controllers will go
> into a low power mode when not in use.
>
> Many machines containing these controllers also have a GPIO wired up
> that can force the controller awake. This is offered via a ACPI-WMI
> interface intended to be manipulated by a userspace utility.
>
> This mechanism is provided by Intel to OEMs to include in BIOS.
> It uses an industry wide GUID that is populated in a separate _WDG
> entry with no binary MOF.
>
> This interface allows software such as fwupd to wake up thunderbolt
> controllers to query the firmware version or flash new firmware.
>
> Signed-off-by: Mario Limonciello <[email protected]>

Queued for testing, thanks everyone.

--
Darren Hart
VMware Open Source Technology Center

2017-09-12 19:31:48

by Mario Limonciello

[permalink] [raw]
Subject: RE: [PATCH v3] Add driver to force WMI Thunderbolt controller power status

> -----Original Message-----
> From: Darren Hart [mailto:[email protected]]
> Sent: Monday, September 11, 2017 4:45 PM
> To: Limonciello, Mario <[email protected]>
> Cc: LKML <[email protected]>; [email protected];
> Richard Hughes <[email protected]>; Yehezkel Bernat
> <[email protected]>; Mika Westerberg <[email protected]>
> Subject: Re: [PATCH v3] Add driver to force WMI Thunderbolt controller power
> status
>
> On Fri, Sep 08, 2017 at 10:23:11AM -0500, Mario Limonciello wrote:
> > Current implementations of Intel Thunderbolt controllers will go
> > into a low power mode when not in use.
> >
> > Many machines containing these controllers also have a GPIO wired up
> > that can force the controller awake. This is offered via a ACPI-WMI
> > interface intended to be manipulated by a userspace utility.
> >
> > This mechanism is provided by Intel to OEMs to include in BIOS.
> > It uses an industry wide GUID that is populated in a separate _WDG
> > entry with no binary MOF.
> >
> > This interface allows software such as fwupd to wake up thunderbolt
> > controllers to query the firmware version or flash new firmware.
> >
> > Signed-off-by: Mario Limonciello <[email protected]>
>
> Queued for testing, thanks everyone.
>
> --
> Darren Hart
> VMware Open Source Technology Center

Darren,

Thanks.

FYI to those that would like to test this, the associated userspace
code that is paired with this was just merged to fwupd master.
https://github.com/hughsie/fwupd/commit/8f17e1ccf4f68b3fb7015a41acc4cbb784c1f776

It's done in a way that if another GUID ever needs to be added for force-power
it will be no changes for userspace, and if another driver is introduced it will
be minimal changes (what drivers the code matches on is hardcoded).

If you would like to experiment with it, instructions for building fwupd
are available here:
https://github.com/hughsie/fwupd/wiki/Compilation

If you find any problems, feel free to file an issue with fwupd on Github.

Thanks,


2017-09-13 17:29:17

by Lukas Wunner

[permalink] [raw]
Subject: Re: [PATCH v3] Add driver to force WMI Thunderbolt controller power status

Sorry, late to the party.

On Fri, Sep 08, 2017 at 10:23:11AM -0500, Mario Limonciello wrote:
> + mode = hex_to_bin(buf[0]);
> + if (mode == 0 || mode == 1) {
> + status = wmi_evaluate_method(INTEL_WMI_THUNDERBOLT_GUID, 0, 1,
> + &input, NULL);
> + if (ACPI_FAILURE(status)) {
> + pr_err("intel-wmi-thunderbolt: failed setting %s\n",
> + buf);
> + return -ENODEV;
> + }
> + } else {
> + pr_err("intel-wmi-thunderbolt: unsupported mode: %d", mode);
> + }
> + return count;
> +}

Seems odd to allow user space to fill the log by writing invalid data
to sysfs, likewise that success is returned in the else case.
I'd drop both pr_err() and return -EINVAL in the else case.


> +static const struct wmi_device_id intel_wmi_thunderbolt_id_table[] = {
> + { .guid_string = INTEL_WMI_THUNDERBOLT_GUID },
> + { },
> +};

I'm not familiar with WMI, but don't you need a MODULE_DEVICE_TABLE here?
How does user space know which module to load upon receiving the uevent?

Thanks,

Lukas

2017-09-14 07:42:21

by Mario Limonciello

[permalink] [raw]
Subject: RE: [PATCH v3] Add driver to force WMI Thunderbolt controller power status

> -----Original Message-----
> From: Lukas Wunner [mailto:[email protected]]
> Sent: Wednesday, September 13, 2017 12:20 PM
> To: Limonciello, Mario <[email protected]>
> Cc: [email protected]; LKML <[email protected]>; platform-driver-
> [email protected]; Richard Hughes <[email protected]>; Yehezkel Bernat
> <[email protected]>; Mika Westerberg <[email protected]>
> Subject: Re: [PATCH v3] Add driver to force WMI Thunderbolt controller power
> status
>
> Sorry, late to the party.
>
> On Fri, Sep 08, 2017 at 10:23:11AM -0500, Mario Limonciello wrote:
> > + mode = hex_to_bin(buf[0]);
> > + if (mode == 0 || mode == 1) {
> > + status = wmi_evaluate_method(INTEL_WMI_THUNDERBOLT_GUID,
> 0, 1,
> > + &input, NULL);
> > + if (ACPI_FAILURE(status)) {
> > + pr_err("intel-wmi-thunderbolt: failed setting %s\n",
> > + buf);
> > + return -ENODEV;
> > + }
> > + } else {
> > + pr_err("intel-wmi-thunderbolt: unsupported mode: %d", mode);
> > + }
> > + return count;
> > +}
>
> Seems odd to allow user space to fill the log by writing invalid data
> to sysfs, likewise that success is returned in the else case.
> I'd drop both pr_err() and return -EINVAL in the else case.
>

Seems fine to me. As Darren already queued the patch, I'll send a follow up patch
to fix these two cases.

>
> > +static const struct wmi_device_id intel_wmi_thunderbolt_id_table[] = {
> > + { .guid_string = INTEL_WMI_THUNDERBOLT_GUID },
> > + { },
> > +};
>
> I'm not familiar with WMI, but don't you need a MODULE_DEVICE_TABLE here?
> How does user space know which module to load upon receiving the uevent?

Some macros for WMI bus devices.
https://github.com/torvalds/linux/blob/e0f25a3f2d052e36ff67a9b4db835c3e27e950d8/include/linux/wmi.h#L55
https://github.com/torvalds/linux/blob/master/include/linux/device.h#L1487


2017-09-14 09:21:32

by Lukas Wunner

[permalink] [raw]
Subject: Re: [PATCH v3] Add driver to force WMI Thunderbolt controller power status

On Thu, Sep 14, 2017 at 06:42:03AM +0000, [email protected] wrote:
> > On Fri, Sep 08, 2017 at 10:23:11AM -0500, Mario Limonciello wrote:
> > > +static const struct wmi_device_id intel_wmi_thunderbolt_id_table[] = {
> > > + { .guid_string = INTEL_WMI_THUNDERBOLT_GUID },
> > > + { },
> > > +};
> >
> > I'm not familiar with WMI, but don't you need a MODULE_DEVICE_TABLE here?
> > How does user space know which module to load upon receiving the uevent?
>
> Some macros for WMI bus devices.
> https://github.com/torvalds/linux/blob/e0f25a3f2d052e36ff67a9b4db835c3e27e950d8/include/linux/wmi.h#L55
> https://github.com/torvalds/linux/blob/master/include/linux/device.h#L1487

No, the init and exit hooks defined by this macro are executed
*after* the module has been loaded. The question was, how does
the module get loaded in the first place?

Looking at drivers/platform/x86/wmi.c:wmi_dev_uevent() it seems that
a modalias consisting of "wmi:" followed by the GUID is sent to udevd.
For udevd to then load the module, I suspect you need to add a
MODULE_DEVICE_TABLE(wmi, ...) to your driver.

Thanks,

Lukas

2017-09-14 14:53:51

by Mario Limonciello

[permalink] [raw]
Subject: RE: [PATCH v3] Add driver to force WMI Thunderbolt controller power status

> -----Original Message-----
> From: Lukas Wunner [mailto:[email protected]]
> Sent: Thursday, September 14, 2017 4:14 AM
> To: Limonciello, Mario <[email protected]>
> Cc: [email protected]; [email protected]; platform-driver-
> [email protected]; [email protected]; [email protected];
> [email protected]
> Subject: Re: [PATCH v3] Add driver to force WMI Thunderbolt controller power
> status
>
> On Thu, Sep 14, 2017 at 06:42:03AM +0000, [email protected] wrote:
> > > On Fri, Sep 08, 2017 at 10:23:11AM -0500, Mario Limonciello wrote:
> > > > +static const struct wmi_device_id intel_wmi_thunderbolt_id_table[] = {
> > > > + { .guid_string = INTEL_WMI_THUNDERBOLT_GUID },
> > > > + { },
> > > > +};
> > >
> > > I'm not familiar with WMI, but don't you need a MODULE_DEVICE_TABLE here?
> > > How does user space know which module to load upon receiving the uevent?
> >
> > Some macros for WMI bus devices.
> >
> https://github.com/torvalds/linux/blob/e0f25a3f2d052e36ff67a9b4db835c3e27e9
> 50d8/include/linux/wmi.h#L55
> > https://github.com/torvalds/linux/blob/master/include/linux/device.h#L1487
>
> No, the init and exit hooks defined by this macro are executed
> *after* the module has been loaded. The question was, how does
> the module get loaded in the first place?
>
> Looking at drivers/platform/x86/wmi.c:wmi_dev_uevent() it seems that
> a modalias consisting of "wmi:" followed by the GUID is sent to udevd.
> For udevd to then load the module, I suspect you need to add a
> MODULE_DEVICE_TABLE(wmi, ...) to your driver.

Ah, you're looking for this code from the WMI bus driver:
https://github.com/torvalds/linux/blob/master/drivers/platform/x86/wmi.c#L724

That happens when the bus is initialized.


2017-09-14 15:03:46

by Mika Westerberg

[permalink] [raw]
Subject: Re: [PATCH v3] Add driver to force WMI Thunderbolt controller power status

On Thu, Sep 14, 2017 at 02:52:27PM +0000, [email protected] wrote:
> > Looking at drivers/platform/x86/wmi.c:wmi_dev_uevent() it seems that
> > a modalias consisting of "wmi:" followed by the GUID is sent to udevd.
> > For udevd to then load the module, I suspect you need to add a
> > MODULE_DEVICE_TABLE(wmi, ...) to your driver.
>
> Ah, you're looking for this code from the WMI bus driver:
> https://github.com/torvalds/linux/blob/master/drivers/platform/x86/wmi.c#L724
>
> That happens when the bus is initialized.

That's right you get the uevent and whatnot but Lucas means that if you
don't have MODULE_DEVICE_TABLE(wmi, ...) in the driver, udev cannot load
the module automatically when the device appears.

2017-09-14 15:14:22

by Mika Westerberg

[permalink] [raw]
Subject: Re: [PATCH v3] Add driver to force WMI Thunderbolt controller power status

On Thu, Sep 14, 2017 at 05:59:19PM +0300, Mika Westerberg wrote:
> On Thu, Sep 14, 2017 at 02:52:27PM +0000, [email protected] wrote:
> > > Looking at drivers/platform/x86/wmi.c:wmi_dev_uevent() it seems that
> > > a modalias consisting of "wmi:" followed by the GUID is sent to udevd.
> > > For udevd to then load the module, I suspect you need to add a
> > > MODULE_DEVICE_TABLE(wmi, ...) to your driver.
> >
> > Ah, you're looking for this code from the WMI bus driver:
> > https://github.com/torvalds/linux/blob/master/drivers/platform/x86/wmi.c#L724
> >
> > That happens when the bus is initialized.
>
> That's right you get the uevent and whatnot but Lucas means that if you
> don't have MODULE_DEVICE_TABLE(wmi, ...) in the driver, udev cannot load
> the module automatically when the device appears.

I meant to say Lukas, not Lucas. Sorry about that.

2017-09-15 07:44:15

by Lukas Wunner

[permalink] [raw]
Subject: Re: [PATCH v3] Add driver to force WMI Thunderbolt controller power status

On Thu, Sep 14, 2017 at 05:59:19PM +0300, Mika Westerberg wrote:
> On Thu, Sep 14, 2017 at 02:52:27PM +0000, [email protected] wrote:
> > > Looking at drivers/platform/x86/wmi.c:wmi_dev_uevent() it seems that
> > > a modalias consisting of "wmi:" followed by the GUID is sent to udevd.
> > > For udevd to then load the module, I suspect you need to add a
> > > MODULE_DEVICE_TABLE(wmi, ...) to your driver.
> >
> > Ah, you're looking for this code from the WMI bus driver:
> > https://github.com/torvalds/linux/blob/master/drivers/platform/x86/wmi.c#L724
> >
> > That happens when the bus is initialized.
>
> That's right you get the uevent and whatnot but Lucas means that if you
> don't have MODULE_DEVICE_TABLE(wmi, ...) in the driver, udev cannot load
> the module automatically when the device appears.

Digging a bit deeper I notice the wmi drivers seem to solve this by
directly declaring a MODULE_ALIAS(), which is also present in Mario's
driver. Mario, have you tested if auto-loading works if compiled as
a module? If so, sorry for the noise.

Thanks,

Lukas

2017-09-15 16:37:19

by Mario Limonciello

[permalink] [raw]
Subject: RE: [PATCH v3] Add driver to force WMI Thunderbolt controller power status

> -----Original Message-----
> From: Lukas Wunner [mailto:[email protected]]
> Sent: Friday, September 15, 2017 2:45 AM
> To: Mika Westerberg <[email protected]>
> Cc: Limonciello, Mario <[email protected]>; [email protected];
> [email protected]; [email protected];
> [email protected]; [email protected]
> Subject: Re: [PATCH v3] Add driver to force WMI Thunderbolt controller power
> status
>
> On Thu, Sep 14, 2017 at 05:59:19PM +0300, Mika Westerberg wrote:
> > On Thu, Sep 14, 2017 at 02:52:27PM +0000, [email protected] wrote:
> > > > Looking at drivers/platform/x86/wmi.c:wmi_dev_uevent() it seems that
> > > > a modalias consisting of "wmi:" followed by the GUID is sent to udevd.
> > > > For udevd to then load the module, I suspect you need to add a
> > > > MODULE_DEVICE_TABLE(wmi, ...) to your driver.
> > >
> > > Ah, you're looking for this code from the WMI bus driver:
> > >
> https://github.com/torvalds/linux/blob/master/drivers/platform/x86/wmi.c#L724
> > >
> > > That happens when the bus is initialized.
> >
> > That's right you get the uevent and whatnot but Lucas means that if you
> > don't have MODULE_DEVICE_TABLE(wmi, ...) in the driver, udev cannot load
> > the module automatically when the device appears.
>
> Digging a bit deeper I notice the wmi drivers seem to solve this by
> directly declaring a MODULE_ALIAS(), which is also present in Mario's
> driver. Mario, have you tested if auto-loading works if compiled as
> a module? If so, sorry for the noise.
>

Yes, I had tested that and that's why I was really baffled at needing to add
MODULE_DEVICE_TABLE. I was going to dig further into this today, but I'm
glad you figured it out.