[PATCH 0/4] platform/x86: wmi: Add subsystem documentation

[PATCH 0/4] platform/x86: wmi: Add subsystem documentation

[Armin Wolf]

Currently, there is no recent documentation available for writing WMI
drivers using the modern bus-based interface. This leads to developers
using the deprecated GUID-based interface when developing new drivers,
causing issues with notification handling when multiple WMI devices sharing
the same notification ID are present. There is also no way for WMI
drivers to add device specific documentation at the moment.
Add documentation for the WMI subsystem to solve those issues. The
device specific documentation currently onyl include documentation for
the wmi-bmof driver, but more is expected to follow.

Armin Wolf (4):
  platform/x86: wmi: Add kernel doc comments
  platform/x86: wmi: Mark GUID-based WMI interface as deprecated
  platform/x86: wmi: Add documentation
  platform/x86: wmi: Add device specific documentation

 .../ABI/stable/sysfs-platform-wmi-bmof        |  7 ++
 Documentation/driver-api/index.rst            |  1 +
 Documentation/driver-api/wmi.rst              | 19 ++++
 Documentation/subsystem-apis.rst              |  1 +
 Documentation/wmi/acpi-interface.rst          | 86 +++++++++++++++++++
 Documentation/wmi/devices/index.rst           | 22 +++++
 Documentation/wmi/devices/wmi-bmof.rst        | 22 +++++
 Documentation/wmi/index.rst                   | 19 ++++
 MAINTAINERS                                   |  9 ++
 drivers/platform/x86/wmi.c                    | 63 ++++++++++----
 include/linux/wmi.h                           | 41 ++++++++-
 11 files changed, 272 insertions(+), 18 deletions(-)
 create mode 100644 Documentation/ABI/stable/sysfs-platform-wmi-bmof
 create mode 100644 Documentation/driver-api/wmi.rst
 create mode 100644 Documentation/wmi/acpi-interface.rst
 create mode 100644 Documentation/wmi/devices/index.rst
 create mode 100644 Documentation/wmi/devices/wmi-bmof.rst
 create mode 100644 Documentation/wmi/index.rst

--
2.30.2

[PATCH 1/4] platform/x86: wmi: Add kernel doc comments

[Armin Wolf]

Add kernel doc comments useful for documenting the functions/structs
used to interact with the WMI driver core.

Signed-off-by: Armin Wolf <W_Armin@gmx.de>
---
 drivers/platform/x86/wmi.c | 51 +++++++++++++++++++++++++++++++-------
 include/linux/wmi.h        | 41 +++++++++++++++++++++++++++---
 2 files changed, 80 insertions(+), 12 deletions(-)

diff --git a/drivers/platform/x86/wmi.c b/drivers/platform/x86/wmi.c
index d81319a502ef..e4b41dca70c7 100644
--- a/drivers/platform/x86/wmi.c
+++ b/drivers/platform/x86/wmi.c
@@ -248,7 +248,9 @@ static acpi_status get_event_data(const struct wmi_block *wblock, struct acpi_bu
  * @wdev: A wmi bus device from a driver
  * @length: Required buffer size
  *
- * Allocates memory needed for buffer, stores the buffer size in that memory
+ * Allocates memory needed for buffer, stores the buffer size in that memory.
+ *
+ * Return: 0 on success or a negative error code for failure.
  */
 int set_required_buffer_size(struct wmi_device *wdev, u64 length)
 {
@@ -269,7 +271,9 @@ EXPORT_SYMBOL_GPL(set_required_buffer_size);
  * @in: Buffer containing input for the method call
  * @out: Empty buffer to return the method results
  *
- * Call an ACPI-WMI method
+ * Call an ACPI-WMI method, the caller must free @out.
+ *
+ * Return: acpi_status signaling success or error.
  */
 acpi_status wmi_evaluate_method(const char *guid_string, u8 instance, u32 method_id,
 				const struct acpi_buffer *in, struct acpi_buffer *out)
@@ -294,7 +298,9 @@ EXPORT_SYMBOL_GPL(wmi_evaluate_method);
  * @in: Buffer containing input for the method call
  * @out: Empty buffer to return the method results
  *
- * Call an ACPI-WMI method
+ * Call an ACPI-WMI method, the caller must free @out.
+ *
+ * Return: acpi_status signaling success or error.
  */
 acpi_status wmidev_evaluate_method(struct wmi_device *wdev, u8 instance, u32 method_id,
 				   const struct acpi_buffer *in, struct acpi_buffer *out)
@@ -411,7 +417,9 @@ static acpi_status __query_block(struct wmi_block *wblock, u8 instance,
  * @instance: Instance index
  * @out: Empty buffer to return the contents of the data block to
  *
- * Return the contents of an ACPI-WMI data block to a buffer
+ * Query a ACPI-WMI block, the caller must free @out.
+ *
+ * Return: ACPI object containing the content of the WMI block.
  */
 acpi_status wmi_query_block(const char *guid_string, u8 instance,
 			    struct acpi_buffer *out)
@@ -427,6 +435,15 @@ acpi_status wmi_query_block(const char *guid_string, u8 instance,
 }
 EXPORT_SYMBOL_GPL(wmi_query_block);

+/**
+ * wmidev_block_query - Return contents of a WMI block
+ * @wdev: A wmi bus device from a driver
+ * @instance: Instance index
+ *
+ * Query a ACPI-WMI block, the caller must free the result.
+ *
+ * Return: ACPI object containing the content of the WMI block.
+ */
 union acpi_object *wmidev_block_query(struct wmi_device *wdev, u8 instance)
 {
 	struct acpi_buffer out = { ACPI_ALLOCATE_BUFFER, NULL };
@@ -445,7 +462,9 @@ EXPORT_SYMBOL_GPL(wmidev_block_query);
  * @instance: Instance index
  * @in: Buffer containing new values for the data block
  *
- * Write the contents of the input buffer to an ACPI-WMI data block
+ * Write the contents of the input buffer to an ACPI-WMI data block.
+ *
+ * Return: acpi_status signaling success or error.
  */
 acpi_status wmi_set_block(const char *guid_string, u8 instance,
 			  const struct acpi_buffer *in)
@@ -555,6 +574,8 @@ static void wmi_notify_debug(u32 value, void *context)
  * @data: Data to be returned to handler when event is fired
  *
  * Register a handler for events sent to the ACPI-WMI mapper device.
+ *
+ * Return: acpi_status signaling success or error.
  */
 acpi_status wmi_install_notify_handler(const char *guid,
 				       wmi_notify_handler handler,
@@ -597,6 +618,8 @@ EXPORT_SYMBOL_GPL(wmi_install_notify_handler);
  * @guid: 36 char string of the form fa50ff2b-f2e8-45de-83fa-65417f2f49ba
  *
  * Unregister handler for events sent to the ACPI-WMI mapper device.
+ *
+ * Return: acpi_status signaling success or error.
  */
 acpi_status wmi_remove_notify_handler(const char *guid)
 {
@@ -641,9 +664,11 @@ EXPORT_SYMBOL_GPL(wmi_remove_notify_handler);
  * wmi_get_event_data - Get WMI data associated with an event
  *
  * @event: Event to find
- * @out: Buffer to hold event data. out->pointer should be freed with kfree()
+ * @out: Buffer to hold event data
+ *
+ * Get extra data associated with an WMI event, the caller needs to free @out.
  *
- * Returns extra data associated with an event in WMI.
+ * Return: acpi_status signaling success or error.
  */
 acpi_status wmi_get_event_data(u32 event, struct acpi_buffer *out)
 {
@@ -664,7 +689,9 @@ EXPORT_SYMBOL_GPL(wmi_get_event_data);
  * wmi_has_guid - Check if a GUID is available
  * @guid_string: 36 char string of the form fa50ff2b-f2e8-45de-83fa-65417f2f49ba
  *
- * Check if a given GUID is defined by _WDG
+ * Check if a given GUID is defined by _WDG.
+ *
+ * Return: True if GUID is available, false otherwise.
  */
 bool wmi_has_guid(const char *guid_string)
 {
@@ -678,7 +705,7 @@ EXPORT_SYMBOL_GPL(wmi_has_guid);
  *
  * Find the _UID of ACPI device associated with this WMI GUID.
  *
- * Return: The ACPI _UID field value or NULL if the WMI GUID was not found
+ * Return: The ACPI _UID field value or NULL if the WMI GUID was not found.
  */
 char *wmi_get_acpi_device_uid(const char *guid_string)
 {
@@ -1454,6 +1481,12 @@ int __must_check __wmi_driver_register(struct wmi_driver *driver,
 }
 EXPORT_SYMBOL(__wmi_driver_register);

+/**
+ * wmi_driver_unregister() - Unregister a WMI driver
+ * @driver: WMI driver to unregister
+ *
+ * Unregisters a WMI driver from the WMI bus.
+ */
 void wmi_driver_unregister(struct wmi_driver *driver)
 {
 	driver_unregister(&driver->driver);
diff --git a/include/linux/wmi.h b/include/linux/wmi.h
index b88d7b58e61e..88f66b12eef9 100644
--- a/include/linux/wmi.h
+++ b/include/linux/wmi.h
@@ -13,25 +13,44 @@
 #include <linux/mod_devicetable.h>
 #include <uapi/linux/wmi.h>

+/**
+ * struct wmi_device - WMI device structure
+ * @dev: Device associated with this WMI device
+ * @setable: True for devices implementing the Set Control Method
+ *
+ * This represents WMI devices discovered by the WMI driver core.
+ */
 struct wmi_device {
 	struct device dev;

-	 /* True for data blocks implementing the Set Control Method */
+	/* private: used by the WMI driver core */
 	bool setable;
 };

-/* evaluate the ACPI method associated with this device */
 extern acpi_status wmidev_evaluate_method(struct wmi_device *wdev,
 					  u8 instance, u32 method_id,
 					  const struct acpi_buffer *in,
 					  struct acpi_buffer *out);

-/* Caller must kfree the result. */
 extern union acpi_object *wmidev_block_query(struct wmi_device *wdev,
 					     u8 instance);

 extern int set_required_buffer_size(struct wmi_device *wdev, u64 length);

+/**
+ * struct wmi_driver - WMI driver structure
+ * @driver: Driver model structure
+ * @id_table: List of WMI GUIDs supported by this driver
+ * @no_notify_data: WMI events provide no event data
+ * @probe: Callback for device binding
+ * @remove: Callback for device unbinding
+ * @notify: Callback for receiving WMI events
+ * @filter_callback: Callback for filtering device IOCTLs
+ *
+ * This represents WMI drivers which handle WMI devices.
+ * @filter_callback is only necessary for drivers which
+ * want to set up a WMI IOCTL interface.
+ */
 struct wmi_driver {
 	struct device_driver driver;
 	const struct wmi_device_id *id_table;
@@ -47,8 +66,24 @@ struct wmi_driver {
 extern int __must_check __wmi_driver_register(struct wmi_driver *driver,
 					      struct module *owner);
 extern void wmi_driver_unregister(struct wmi_driver *driver);
+
+/**
+ * wmi_driver_register() - Helper macro ro register a WMI driver
+ * @driver: wmi_driver struct
+ *
+ * Helper macro for registering a WMI driver. It automatically passes
+ * THIS_MODULE to the underlying function.
+ */
 #define wmi_driver_register(driver) __wmi_driver_register((driver), THIS_MODULE)

+/**
+ * module_wmi_driver() - Helper macro to register/unregister a WMI driver
+ * @__wmi_driver: wmi_driver struct
+ *
+ * Helper macro for WMI drivers which do not do anything special in module
+ * init/exit. This eliminates a lot of boilerplate. Each module may only
+ * use this macro once, and calling it replaces module_init() and module_exit().
+ */
 #define module_wmi_driver(__wmi_driver) \
 	module_driver(__wmi_driver, wmi_driver_register, \
 		      wmi_driver_unregister)
--
2.30.2

[PATCH 2/4] platform/x86: wmi: Mark GUID-based WMI interface as deprecated

[Armin Wolf]

The WMI driver core supports a more mordern bus-based interface for
interacting with WMI devices. The older GUID-based interface depends
on each WMI GUID and notification id being unique on a given system,
which turned out is not the case.
Mark the older interface as deprecated since new WMI drivers should
use the bus-based interface to avoid this issues.

Signed-off-by: Armin Wolf <W_Armin@gmx.de>
---
 drivers/platform/x86/wmi.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/platform/x86/wmi.c b/drivers/platform/x86/wmi.c
index e4b41dca70c7..ae654487718e 100644
--- a/drivers/platform/x86/wmi.c
+++ b/drivers/platform/x86/wmi.c
@@ -264,7 +264,7 @@ int set_required_buffer_size(struct wmi_device *wdev, u64 length)
 EXPORT_SYMBOL_GPL(set_required_buffer_size);

 /**
- * wmi_evaluate_method - Evaluate a WMI method
+ * wmi_evaluate_method - Evaluate a WMI method (deprecated)
  * @guid_string: 36 char string of the form fa50ff2b-f2e8-45de-83fa-65417f2f49ba
  * @instance: Instance index
  * @method_id: Method ID to call
@@ -457,7 +457,7 @@ union acpi_object *wmidev_block_query(struct wmi_device *wdev, u8 instance)
 EXPORT_SYMBOL_GPL(wmidev_block_query);

 /**
- * wmi_set_block - Write to a WMI block
+ * wmi_set_block - Write to a WMI block (deprecated)
  * @guid_string: 36 char string of the form fa50ff2b-f2e8-45de-83fa-65417f2f49ba
  * @instance: Instance index
  * @in: Buffer containing new values for the data block
@@ -568,7 +568,7 @@ static void wmi_notify_debug(u32 value, void *context)
 }

 /**
- * wmi_install_notify_handler - Register handler for WMI events
+ * wmi_install_notify_handler - Register handler for WMI events (deprecated)
  * @guid: 36 char string of the form fa50ff2b-f2e8-45de-83fa-65417f2f49ba
  * @handler: Function to handle notifications
  * @data: Data to be returned to handler when event is fired
@@ -614,7 +614,7 @@ acpi_status wmi_install_notify_handler(const char *guid,
 EXPORT_SYMBOL_GPL(wmi_install_notify_handler);

 /**
- * wmi_remove_notify_handler - Unregister handler for WMI events
+ * wmi_remove_notify_handler - Unregister handler for WMI events (deprecated)
  * @guid: 36 char string of the form fa50ff2b-f2e8-45de-83fa-65417f2f49ba
  *
  * Unregister handler for events sent to the ACPI-WMI mapper device.
@@ -661,7 +661,7 @@ acpi_status wmi_remove_notify_handler(const char *guid)
 EXPORT_SYMBOL_GPL(wmi_remove_notify_handler);

 /**
- * wmi_get_event_data - Get WMI data associated with an event
+ * wmi_get_event_data - Get WMI data associated with an event (deprecated)
  *
  * @event: Event to find
  * @out: Buffer to hold event data
@@ -700,7 +700,7 @@ bool wmi_has_guid(const char *guid_string)
 EXPORT_SYMBOL_GPL(wmi_has_guid);

 /**
- * wmi_get_acpi_device_uid() - Get _UID name of ACPI device that defines GUID
+ * wmi_get_acpi_device_uid() - Get _UID name of ACPI device that defines GUID (deprecated)
  * @guid_string: 36 char string of the form fa50ff2b-f2e8-45de-83fa-65417f2f49ba
  *
  * Find the _UID of ACPI device associated with this WMI GUID.
--
2.30.2

[PATCH 3/4] platform/x86: wmi: Add documentation

[Armin Wolf]

Add documentation for the WMI subsystem. The documentation describes
both the ACPI WMI interface and the driver API for interacting with
the WMI driver core. The information regarding the ACPI interface
where retrieved from the Ubuntu kernel references and the
Windows driver samples available on GitHub. The documentation is
supposed to help driver developers writing WMI drivers, as many
modern machines designed to run Windows provide a ACPI WMI interface.

Signed-off-by: Armin Wolf <W_Armin@gmx.de>
---
 Documentation/driver-api/index.rst   |  1 +
 Documentation/driver-api/wmi.rst     | 19 ++++++
 Documentation/subsystem-apis.rst     |  1 +
 Documentation/wmi/acpi-interface.rst | 86 ++++++++++++++++++++++++++++
 Documentation/wmi/index.rst          | 18 ++++++
 MAINTAINERS                          |  2 +
 include/linux/wmi.h                  |  2 +-
 7 files changed, 128 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/driver-api/wmi.rst
 create mode 100644 Documentation/wmi/acpi-interface.rst
 create mode 100644 Documentation/wmi/index.rst

diff --git a/Documentation/driver-api/index.rst b/Documentation/driver-api/index.rst
index ff9aa1afdc62..1e16a40da3ba 100644
--- a/Documentation/driver-api/index.rst
+++ b/Documentation/driver-api/index.rst
@@ -113,6 +113,7 @@ available subsections can be seen below.
    xillybus
    zorro
    hte/index
+   wmi

 .. only::  subproject and html

diff --git a/Documentation/driver-api/wmi.rst b/Documentation/driver-api/wmi.rst
new file mode 100644
index 000000000000..06cecbe36afd
--- /dev/null
+++ b/Documentation/driver-api/wmi.rst
@@ -0,0 +1,19 @@
+.. SPDX-License-Identifier: GPL-2.0-or-later
+
+==============
+WMI Driver API
+==============
+
+The WMI driver core supports a more modern bus-based interface for interacting with WMI devices,
+and an older GUID-based interface. The later interface is considered to be deprecated, so new
+WMI drivers should generally avoid it since it has some issues with multiple WMI devices and
+events sharing the same GUIDs and/or notification IDs. The modern bus-based interface instead
+maps each WMI device to a :c:type:`struct wmi_device <wmi_device>`, so it supports WMI devices
+sharing GUIDs and/or notification IDs. Drivers can then register a :c:type:`struct wmi_driver <wmi_driver>`,
+which will get bound to compatible WMI devices by the driver core.
+
+.. kernel-doc:: include/linux/wmi.h
+   :internal:
+
+.. kernel-doc:: drivers/platform/x86/wmi.c
+   :export:
diff --git a/Documentation/subsystem-apis.rst b/Documentation/subsystem-apis.rst
index b51f38527e14..69f5e4d53bad 100644
--- a/Documentation/subsystem-apis.rst
+++ b/Documentation/subsystem-apis.rst
@@ -57,3 +57,4 @@ needed).
    scheduler/index
    mhi/index
    peci/index
+   wmi/index
diff --git a/Documentation/wmi/acpi-interface.rst b/Documentation/wmi/acpi-interface.rst
new file mode 100644
index 000000000000..c0afdb6c5885
--- /dev/null
+++ b/Documentation/wmi/acpi-interface.rst
@@ -0,0 +1,86 @@
+.. SPDX-License-Identifier: GPL-2.0-or-later
+
+==================
+ACPI WMI interface
+==================
+
+The ACPI WMI interface is a proprietary extension of the ACPI specification made by Microsoft
+to allow hardware vendors to embed WMI (Windows Management Instrumentation) objects inside
+their ACPI firmware. Typical functions implemented over ACPI WMI are hotkey events on modern
+notebooks and configuration of BIOS options.
+
+PNP0C14 ACPI device
+-------------------
+
+Discovery of WMI objects is handled by defining ACPI devices with a PNP ID of ``PNP0C14``.
+These devices will contain a set of ACPI buffers and methods used for mapping and execution
+of WMI methods and/or queries. If there exist multiple of such devices, then each device is
+required to have a unique ACPI UID.
+
+_WDG buffer
+-----------
+
+The ``_WDG`` buffer is used to discover WMI objects and is required to be static. Its internal
+structure consists of data blocks with a size of 20 bytes, containing the following data:
+
+======= =============== =====================================================
+Offset  Size (in bytes) Content
+======= =============== =====================================================
+0x00    16              128 bit Variant 2 object GUID.
+0x10    2               2 character method ID or single byte notification ID.
+0x12    1               Object instance count.
+0x13    1               Object flags.
+======= =============== =====================================================
+
+The WMI object flags control whether the method or notification ID is used:
+
+- 0x1: Data block usage is expensive and must be explicitly enabled/disabled.
+- 0x2: Data block contains WMI methods.
+- 0x4: Data block contains ASCIZ string.
+- 0x8: Data block describes a WMI event, use notification ID instead of method ID.
+
+Each WMI object GUID can appear multiple times inside a system. The method/notification ID
+is used to construct the ACPI method names used for interacting with the WMI object.
+
+WQxx ACPI methods
+-----------------
+
+If a data block does not contain WMI methods, then its content can be retrieved by this required
+ACPI methods. The last two characters of the ACPI method name are the method ID of the data block
+to query. Their single parameter is a integer describing the instance which should be queried. This
+parameter can be omitted if the data block contains only a single instance.
+
+WSxx ACPI methods
+-----------------
+
+Similar to the ``WQxx`` ACPI methods, except that it is optional and takes an additional buffer
+as its second argument. The instance argument also cannot be omitted.
+
+WMxx ACPI methods
+-----------------
+
+Used for executing WMI methods associated with a data block. The last two characters of the ACPI method
+name are the method ID of the data block containing the WMI methods. Their first parameter is a integer
+describing the instance which methods should be executed. The second parameter is a integer describing
+the WMI method ID to execute, and the third parameter is a buffer containing the WMI method parameters.
+If the data block is marked as containing a ASCIZ string, then this buffer should contain a ASCIZ string.
+The ACPI method will return the result of the executed WMI method.
+
+WExx ACPI methods
+-----------------
+
+Used for optionally enabling/disabling WMI events, the last two characters of the ACPI method are the
+notification ID of the data block describing the WMI event as hexadecimal value. Their first parameter
+is an integer with a value of 0 if the WMI event should be disabled, other values will enable the WMI event.
+
+WCxx ACPI methods
+-----------------
+Similar to the ``WExx`` ACPI methods, except that it controls data collection instead of events
+and thus the last two characters of the ACPI method name are the method ID of the data block
+to enable/disable.
+
+_WED ACPI method
+----------------
+
+Used to retrieve additional WMI event data, its single parameter is a integer holding the
+notification ID of the event.
diff --git a/Documentation/wmi/index.rst b/Documentation/wmi/index.rst
new file mode 100644
index 000000000000..b29933a86380
--- /dev/null
+++ b/Documentation/wmi/index.rst
@@ -0,0 +1,18 @@
+.. SPDX-License-Identifier: GPL-2.0-or-later
+
+=============
+WMI Subsystem
+=============
+
+.. toctree::
+   :maxdepth: 1
+
+   acpi-interface
+
+.. only::  subproject and html
+
+
+   Indices
+   =======
+
+   * :ref:`genindex`
diff --git a/MAINTAINERS b/MAINTAINERS
index 0c9011f5fc17..979d37176429 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -449,6 +449,8 @@ F:	include/linux/acpi_viot.h
 ACPI WMI DRIVER
 L:	platform-driver-x86@vger.kernel.org
 S:	Orphan
+F:	Documentation/driver-api/wmi.rst
+F:	Documentation/wmi/
 F:	drivers/platform/x86/wmi.c
 F:	include/uapi/linux/wmi.h

diff --git a/include/linux/wmi.h b/include/linux/wmi.h
index 88f66b12eef9..87822effdf3c 100644
--- a/include/linux/wmi.h
+++ b/include/linux/wmi.h
@@ -49,7 +49,7 @@ extern int set_required_buffer_size(struct wmi_device *wdev, u64 length);
  *
  * This represents WMI drivers which handle WMI devices.
  * @filter_callback is only necessary for drivers which
- * want to set up a WMI IOCTL interface.
+ * want to set up a WMI IOCTL interface
  */
 struct wmi_driver {
 	struct device_driver driver;
--
2.30.2

[PATCH 4/4] platform/x86: wmi: Add device specific documentation

[Armin Wolf]

Add a place for device-specific documentation of WMI drivers.
The first entry is documentation for the wmi-bmof driver, with
additional documentation being expected to follow.

Signed-off-by: Armin Wolf <W_Armin@gmx.de>
---
 .../ABI/stable/sysfs-platform-wmi-bmof        |  7 ++++++
 Documentation/wmi/devices/index.rst           | 22 +++++++++++++++++++
 Documentation/wmi/devices/wmi-bmof.rst        | 22 +++++++++++++++++++
 Documentation/wmi/index.rst                   |  1 +
 MAINTAINERS                                   |  7 ++++++
 5 files changed, 59 insertions(+)
 create mode 100644 Documentation/ABI/stable/sysfs-platform-wmi-bmof
 create mode 100644 Documentation/wmi/devices/index.rst
 create mode 100644 Documentation/wmi/devices/wmi-bmof.rst

diff --git a/Documentation/ABI/stable/sysfs-platform-wmi-bmof b/Documentation/ABI/stable/sysfs-platform-wmi-bmof
new file mode 100644
index 000000000000..a786504b6027
--- /dev/null
+++ b/Documentation/ABI/stable/sysfs-platform-wmi-bmof
@@ -0,0 +1,7 @@
+What:		/sys/bus/wmi/devices/05901221-D566-11D1-B2F0-00A0C9062910[-X]/bmof
+Date:		Jun 2017
+KernelVersion:	4.13
+Description:
+		Binary MOF metadata used to decribe the details of available ACPI WMI interfaces.
+
+		See Documentation/wmi/devices/wmi-bmof.rst for details.
diff --git a/Documentation/wmi/devices/index.rst b/Documentation/wmi/devices/index.rst
new file mode 100644
index 000000000000..c08735a9d7df
--- /dev/null
+++ b/Documentation/wmi/devices/index.rst
@@ -0,0 +1,22 @@
+.. SPDX-License-Identifier: GPL-2.0-or-later
+
+=============================
+Driver-specific Documentation
+=============================
+
+This section provides information about various devices supported by
+the Linux kernel, their protocols and driver details.
+
+.. toctree::
+   :maxdepth: 1
+   :numbered:
+   :glob:
+
+   *
+
+.. only:: subproject and html
+
+   Indices
+   =======
+
+   * :ref:`genindex`
diff --git a/Documentation/wmi/devices/wmi-bmof.rst b/Documentation/wmi/devices/wmi-bmof.rst
new file mode 100644
index 000000000000..b558fa46190c
--- /dev/null
+++ b/Documentation/wmi/devices/wmi-bmof.rst
@@ -0,0 +1,22 @@
+.. SPDX-License-Identifier: GPL-2.0-only
+
+==============================
+WMI embedded Binary MOF driver
+==============================
+
+Introduction
+============
+
+Many machines embed WMI Binary MOF metadata used to describe the details of their ACPI WMI interfaces.
+The data can be decoded with tools like `bmfdec <https://github.com/pali/bmfdec>`_ to obtain a
+human readable WMI interface description, which is useful for developing new WMI drivers.
+
+The Binary MOF data can be retrieved from the ``bmof`` sysfs attribute of the associated WMI device.
+Please note that multiple WMI devices containing Binary MOF data can exist on a given system.
+
+WMI interface
+=============
+
+The Binary MOF WMI device is identified by the WMI GUID ``05901221-D566-11D1-B2F0-00A0C9062910``.
+The Binary MOF can be obtained by doing a WMI data block query. The result is then returned as
+an ACPI buffer with a variable size.
diff --git a/Documentation/wmi/index.rst b/Documentation/wmi/index.rst
index b29933a86380..537cff188e14 100644
--- a/Documentation/wmi/index.rst
+++ b/Documentation/wmi/index.rst
@@ -8,6 +8,7 @@ WMI Subsystem
    :maxdepth: 1

    acpi-interface
+   devices/index

 .. only::  subproject and html

diff --git a/MAINTAINERS b/MAINTAINERS
index 979d37176429..4d5b1f6d77f6 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -22556,6 +22556,13 @@ L:	linux-wireless@vger.kernel.org
 S:	Odd fixes
 F:	drivers/net/wireless/wl3501*

+WMI BINARY MOF DRIVER
+L:	platform-drivers-x86@vger.kernel.org
+S:	Orphan
+F:	Documentation/ABI/stable/sysfs-platform-wmi-bmof
+F:	Documentation/wmi/devices/wmi-bmof.rst
+F:	drivers/platform/x86/wmi-bmof.c
+
 WOLFSON MICROELECTRONICS DRIVERS
 L:	patches@opensource.cirrus.com
 S:	Supported
--
2.30.2

Re: [PATCH 4/4] platform/x86: wmi: Add device specific documentation

[Randy Dunlap]

Hi,

On 4/20/23 16:32, Armin Wolf wrote:
> diff --git a/Documentation/wmi/devices/wmi-bmof.rst b/Documentation/wmi/devices/wmi-bmof.rst
> new file mode 100644
> index 000000000000..b558fa46190c
> --- /dev/null
> +++ b/Documentation/wmi/devices/wmi-bmof.rst
> @@ -0,0 +1,22 @@
> +.. SPDX-License-Identifier: GPL-2.0-only
> +
> +==============================
> +WMI embedded Binary MOF driver
> +==============================
> +

Please tell the reader what MOF means.

It would be good in drivers/platform/x86/Kconfig also did that.

> +Introduction
> +============
> +
> +Many machines embed WMI Binary MOF metadata used to describe the details of their ACPI WMI interfaces.
> +The data can be decoded with tools like `bmfdec <https://github.com/pali/bmfdec>`_ to obtain a
> +human readable WMI interface description, which is useful for developing new WMI drivers.
> +
> +The Binary MOF data can be retrieved from the ``bmof`` sysfs attribute of the associated WMI device.
> +Please note that multiple WMI devices containing Binary MOF data can exist on a given system.
> +
> +WMI interface
> +=============
> +
> +The Binary MOF WMI device is identified by the WMI GUID ``05901221-D566-11D1-B2F0-00A0C9062910``.
> +The Binary MOF can be obtained by doing a WMI data block query. The result is then returned as
> +an ACPI buffer with a variable size.

-- 
~Randy

Re: [PATCH 3/4] platform/x86: wmi: Add documentation

[Randy Dunlap]

Hi--

On 4/20/23 16:32, Armin Wolf wrote:
> Add documentation for the WMI subsystem. The documentation describes
> both the ACPI WMI interface and the driver API for interacting with
> the WMI driver core. The information regarding the ACPI interface
> where retrieved from the Ubuntu kernel references and the

  were?  I would say "was".

> Windows driver samples available on GitHub. The documentation is
> supposed to help driver developers writing WMI drivers, as many
> modern machines designed to run Windows provide a ACPI WMI interface.

                                                  an ACPI

For the Documentation/ files, AFAIK, we still have an 80-column preferred
limit.  coding-style.rst does not say anything about Documentation/ being
exempt from that limit.

> 
> Signed-off-by: Armin Wolf <W_Armin@gmx.de>
> ---
>  Documentation/driver-api/index.rst   |  1 +
>  Documentation/driver-api/wmi.rst     | 19 ++++++
>  Documentation/subsystem-apis.rst     |  1 +
>  Documentation/wmi/acpi-interface.rst | 86 ++++++++++++++++++++++++++++
>  Documentation/wmi/index.rst          | 18 ++++++
>  MAINTAINERS                          |  2 +
>  include/linux/wmi.h                  |  2 +-
>  7 files changed, 128 insertions(+), 1 deletion(-)
>  create mode 100644 Documentation/driver-api/wmi.rst
>  create mode 100644 Documentation/wmi/acpi-interface.rst
>  create mode 100644 Documentation/wmi/index.rst
> 
> diff --git a/Documentation/driver-api/index.rst b/Documentation/driver-api/index.rst
> index ff9aa1afdc62..1e16a40da3ba 100644
> --- a/Documentation/driver-api/index.rst
> +++ b/Documentation/driver-api/index.rst
> @@ -113,6 +113,7 @@ available subsections can be seen below.
>     xillybus
>     zorro
>     hte/index
> +   wmi
> 
>  .. only::  subproject and html
> 
> diff --git a/Documentation/driver-api/wmi.rst b/Documentation/driver-api/wmi.rst
> new file mode 100644
> index 000000000000..06cecbe36afd
> --- /dev/null
> +++ b/Documentation/driver-api/wmi.rst
> @@ -0,0 +1,19 @@
> +.. SPDX-License-Identifier: GPL-2.0-or-later
> +
> +==============
> +WMI Driver API
> +==============
> +
> +The WMI driver core supports a more modern bus-based interface for interacting with WMI devices,
> +and an older GUID-based interface. The later interface is considered to be deprecated, so new

ITYM                                      latter

> +WMI drivers should generally avoid it since it has some issues with multiple WMI devices and
> +events sharing the same GUIDs and/or notification IDs. The modern bus-based interface instead
> +maps each WMI device to a :c:type:`struct wmi_device <wmi_device>`, so it supports WMI devices
> +sharing GUIDs and/or notification IDs. Drivers can then register a :c:type:`struct wmi_driver <wmi_driver>`,
> +which will get bound to compatible WMI devices by the driver core.

s/get/be/ preferably.

> +
> +.. kernel-doc:: include/linux/wmi.h
> +   :internal:
> +
> +.. kernel-doc:: drivers/platform/x86/wmi.c
> +   :export:
> diff --git a/Documentation/subsystem-apis.rst b/Documentation/subsystem-apis.rst
> index b51f38527e14..69f5e4d53bad 100644
> --- a/Documentation/subsystem-apis.rst
> +++ b/Documentation/subsystem-apis.rst
> @@ -57,3 +57,4 @@ needed).
>     scheduler/index
>     mhi/index
>     peci/index
> +   wmi/index
> diff --git a/Documentation/wmi/acpi-interface.rst b/Documentation/wmi/acpi-interface.rst
> new file mode 100644
> index 000000000000..c0afdb6c5885
> --- /dev/null
> +++ b/Documentation/wmi/acpi-interface.rst
> @@ -0,0 +1,86 @@
> +.. SPDX-License-Identifier: GPL-2.0-or-later
> +
> +==================
> +ACPI WMI interface
> +==================
> +
> +The ACPI WMI interface is a proprietary extension of the ACPI specification made by Microsoft
> +to allow hardware vendors to embed WMI (Windows Management Instrumentation) objects inside
> +their ACPI firmware. Typical functions implemented over ACPI WMI are hotkey events on modern
> +notebooks and configuration of BIOS options.
> +
> +PNP0C14 ACPI device
> +-------------------
> +
> +Discovery of WMI objects is handled by defining ACPI devices with a PNP ID of ``PNP0C14``.
> +These devices will contain a set of ACPI buffers and methods used for mapping and execution
> +of WMI methods and/or queries. If there exist multiple of such devices, then each device is
> +required to have a unique ACPI UID.
> +
> +_WDG buffer
> +-----------
> +
> +The ``_WDG`` buffer is used to discover WMI objects and is required to be static. Its internal
> +structure consists of data blocks with a size of 20 bytes, containing the following data:
> +
> +======= =============== =====================================================
> +Offset  Size (in bytes) Content
> +======= =============== =====================================================
> +0x00    16              128 bit Variant 2 object GUID.
> +0x10    2               2 character method ID or single byte notification ID.
> +0x12    1               Object instance count.
> +0x13    1               Object flags.
> +======= =============== =====================================================
> +
> +The WMI object flags control whether the method or notification ID is used:
> +
> +- 0x1: Data block usage is expensive and must be explicitly enabled/disabled.
> +- 0x2: Data block contains WMI methods.
> +- 0x4: Data block contains ASCIZ string.
> +- 0x8: Data block describes a WMI event, use notification ID instead of method ID.
> +
> +Each WMI object GUID can appear multiple times inside a system. The method/notification ID
> +is used to construct the ACPI method names used for interacting with the WMI object.
> +
> +WQxx ACPI methods
> +-----------------
> +
> +If a data block does not contain WMI methods, then its content can be retrieved by this required
> +ACPI methods. The last two characters of the ACPI method name are the method ID of the data block

   ACPI method.

> +to query. Their single parameter is a integer describing the instance which should be queried. This

                                       an integer

> +parameter can be omitted if the data block contains only a single instance.
> +
> +WSxx ACPI methods
> +-----------------
> +
> +Similar to the ``WQxx`` ACPI methods, except that it is optional and takes an additional buffer
> +as its second argument. The instance argument also cannot be omitted.
> +
> +WMxx ACPI methods
> +-----------------
> +
> +Used for executing WMI methods associated with a data block. The last two characters of the ACPI method
> +name are the method ID of the data block containing the WMI methods. Their first parameter is a integer

                                                                                                 an integer

> +describing the instance which methods should be executed. The second parameter is a integer describing
> +the WMI method ID to execute, and the third parameter is a buffer containing the WMI method parameters.
> +If the data block is marked as containing a ASCIZ string, then this buffer should contain a ASCIZ string.

                                             an ASCIZ string,                                an ASCIZ string.

> +The ACPI method will return the result of the executed WMI method.
> +
> +WExx ACPI methods
> +-----------------
> +
> +Used for optionally enabling/disabling WMI events, the last two characters of the ACPI method are the
> +notification ID of the data block describing the WMI event as hexadecimal value. Their first parameter
> +is an integer with a value of 0 if the WMI event should be disabled, other values will enable the WMI event.
> +
> +WCxx ACPI methods
> +-----------------
> +Similar to the ``WExx`` ACPI methods, except that it controls data collection instead of events
> +and thus the last two characters of the ACPI method name are the method ID of the data block
> +to enable/disable.
> +
> +_WED ACPI method
> +----------------
> +
> +Used to retrieve additional WMI event data, its single parameter is a integer holding the
> +notification ID of the event.
> diff --git a/Documentation/wmi/index.rst b/Documentation/wmi/index.rst
> new file mode 100644
> index 000000000000..b29933a86380
> --- /dev/null
> +++ b/Documentation/wmi/index.rst
> @@ -0,0 +1,18 @@
> +.. SPDX-License-Identifier: GPL-2.0-or-later
> +
> +=============
> +WMI Subsystem
> +=============
> +
> +.. toctree::
> +   :maxdepth: 1
> +
> +   acpi-interface
> +
> +.. only::  subproject and html
> +
> +
> +   Indices
> +   =======
> +
> +   * :ref:`genindex`
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 0c9011f5fc17..979d37176429 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -449,6 +449,8 @@ F:	include/linux/acpi_viot.h
>  ACPI WMI DRIVER
>  L:	platform-driver-x86@vger.kernel.org
>  S:	Orphan
> +F:	Documentation/driver-api/wmi.rst
> +F:	Documentation/wmi/
>  F:	drivers/platform/x86/wmi.c
>  F:	include/uapi/linux/wmi.h
> 
> diff --git a/include/linux/wmi.h b/include/linux/wmi.h
> index 88f66b12eef9..87822effdf3c 100644
> --- a/include/linux/wmi.h
> +++ b/include/linux/wmi.h
> @@ -49,7 +49,7 @@ extern int set_required_buffer_size(struct wmi_device *wdev, u64 length);
>   *
>   * This represents WMI drivers which handle WMI devices.
>   * @filter_callback is only necessary for drivers which
> - * want to set up a WMI IOCTL interface.
> + * want to set up a WMI IOCTL interface

Nothing wrong with the ending '.' there.

>   */
>  struct wmi_driver {
>  	struct device_driver driver;
> --
> 2.30.2
> 

-- 
~Randy

Re: [PATCH 1/4] platform/x86: wmi: Add kernel doc comments

[Randy Dunlap]

Hi,

On 4/20/23 16:32, Armin Wolf wrote:
> Add kernel doc comments useful for documenting the functions/structs
> used to interact with the WMI driver core.
> 
> Signed-off-by: Armin Wolf <W_Armin@gmx.de>
> ---
>  drivers/platform/x86/wmi.c | 51 +++++++++++++++++++++++++++++++-------
>  include/linux/wmi.h        | 41 +++++++++++++++++++++++++++---
>  2 files changed, 80 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/platform/x86/wmi.c b/drivers/platform/x86/wmi.c
> index d81319a502ef..e4b41dca70c7 100644
> --- a/drivers/platform/x86/wmi.c
> +++ b/drivers/platform/x86/wmi.c
> @@ -248,7 +248,9 @@ static acpi_status get_event_data(const struct wmi_block *wblock, struct acpi_bu
>   * @wdev: A wmi bus device from a driver
>   * @length: Required buffer size
>   *
> - * Allocates memory needed for buffer, stores the buffer size in that memory
> + * Allocates memory needed for buffer, stores the buffer size in that memory.
> + *
> + * Return: 0 on success or a negative error code for failure.
>   */
>  int set_required_buffer_size(struct wmi_device *wdev, u64 length)
>  {
> @@ -269,7 +271,9 @@ EXPORT_SYMBOL_GPL(set_required_buffer_size);
>   * @in: Buffer containing input for the method call
>   * @out: Empty buffer to return the method results
>   *
> - * Call an ACPI-WMI method
> + * Call an ACPI-WMI method, the caller must free @out.
> + *
> + * Return: acpi_status signaling success or error.
>   */
>  acpi_status wmi_evaluate_method(const char *guid_string, u8 instance, u32 method_id,
>  				const struct acpi_buffer *in, struct acpi_buffer *out)
> @@ -294,7 +298,9 @@ EXPORT_SYMBOL_GPL(wmi_evaluate_method);
>   * @in: Buffer containing input for the method call
>   * @out: Empty buffer to return the method results
>   *
> - * Call an ACPI-WMI method
> + * Call an ACPI-WMI method, the caller must free @out.
> + *
> + * Return: acpi_status signaling success or error.
>   */
>  acpi_status wmidev_evaluate_method(struct wmi_device *wdev, u8 instance, u32 method_id,
>  				   const struct acpi_buffer *in, struct acpi_buffer *out)
> @@ -411,7 +417,9 @@ static acpi_status __query_block(struct wmi_block *wblock, u8 instance,
>   * @instance: Instance index
>   * @out: Empty buffer to return the contents of the data block to
>   *
> - * Return the contents of an ACPI-WMI data block to a buffer
> + * Query a ACPI-WMI block, the caller must free @out.
> + *
> + * Return: ACPI object containing the content of the WMI block.
>   */
>  acpi_status wmi_query_block(const char *guid_string, u8 instance,
>  			    struct acpi_buffer *out)
> @@ -427,6 +435,15 @@ acpi_status wmi_query_block(const char *guid_string, u8 instance,
>  }
>  EXPORT_SYMBOL_GPL(wmi_query_block);
> 
> +/**
> + * wmidev_block_query - Return contents of a WMI block
> + * @wdev: A wmi bus device from a driver
> + * @instance: Instance index
> + *
> + * Query a ACPI-WMI block, the caller must free the result.

      Query an ACPI-WMI block,

> + *
> + * Return: ACPI object containing the content of the WMI block.
> + */
>  union acpi_object *wmidev_block_query(struct wmi_device *wdev, u8 instance)
>  {
>  	struct acpi_buffer out = { ACPI_ALLOCATE_BUFFER, NULL };
> @@ -445,7 +462,9 @@ EXPORT_SYMBOL_GPL(wmidev_block_query);
>   * @instance: Instance index
>   * @in: Buffer containing new values for the data block
>   *
> - * Write the contents of the input buffer to an ACPI-WMI data block
> + * Write the contents of the input buffer to an ACPI-WMI data block.
> + *
> + * Return: acpi_status signaling success or error.
>   */
>  acpi_status wmi_set_block(const char *guid_string, u8 instance,
>  			  const struct acpi_buffer *in)
> @@ -555,6 +574,8 @@ static void wmi_notify_debug(u32 value, void *context)
>   * @data: Data to be returned to handler when event is fired
>   *
>   * Register a handler for events sent to the ACPI-WMI mapper device.
> + *
> + * Return: acpi_status signaling success or error.
>   */
>  acpi_status wmi_install_notify_handler(const char *guid,
>  				       wmi_notify_handler handler,
> @@ -597,6 +618,8 @@ EXPORT_SYMBOL_GPL(wmi_install_notify_handler);
>   * @guid: 36 char string of the form fa50ff2b-f2e8-45de-83fa-65417f2f49ba
>   *
>   * Unregister handler for events sent to the ACPI-WMI mapper device.
> + *
> + * Return: acpi_status signaling success or error.
>   */
>  acpi_status wmi_remove_notify_handler(const char *guid)
>  {
> @@ -641,9 +664,11 @@ EXPORT_SYMBOL_GPL(wmi_remove_notify_handler);
>   * wmi_get_event_data - Get WMI data associated with an event
>   *
>   * @event: Event to find
> - * @out: Buffer to hold event data. out->pointer should be freed with kfree()
> + * @out: Buffer to hold event data
> + *
> + * Get extra data associated with an WMI event, the caller needs to free @out.
>   *
> - * Returns extra data associated with an event in WMI.
> + * Return: acpi_status signaling success or error.
>   */
>  acpi_status wmi_get_event_data(u32 event, struct acpi_buffer *out)
>  {
> @@ -664,7 +689,9 @@ EXPORT_SYMBOL_GPL(wmi_get_event_data);
>   * wmi_has_guid - Check if a GUID is available
>   * @guid_string: 36 char string of the form fa50ff2b-f2e8-45de-83fa-65417f2f49ba
>   *
> - * Check if a given GUID is defined by _WDG
> + * Check if a given GUID is defined by _WDG.
> + *
> + * Return: True if GUID is available, false otherwise.
>   */
>  bool wmi_has_guid(const char *guid_string)
>  {
> @@ -678,7 +705,7 @@ EXPORT_SYMBOL_GPL(wmi_has_guid);
>   *
>   * Find the _UID of ACPI device associated with this WMI GUID.
>   *
> - * Return: The ACPI _UID field value or NULL if the WMI GUID was not found
> + * Return: The ACPI _UID field value or NULL if the WMI GUID was not found.
>   */
>  char *wmi_get_acpi_device_uid(const char *guid_string)
>  {
> @@ -1454,6 +1481,12 @@ int __must_check __wmi_driver_register(struct wmi_driver *driver,
>  }
>  EXPORT_SYMBOL(__wmi_driver_register);
> 
> +/**
> + * wmi_driver_unregister() - Unregister a WMI driver
> + * @driver: WMI driver to unregister
> + *
> + * Unregisters a WMI driver from the WMI bus.
> + */
>  void wmi_driver_unregister(struct wmi_driver *driver)
>  {
>  	driver_unregister(&driver->driver);
> diff --git a/include/linux/wmi.h b/include/linux/wmi.h
> index b88d7b58e61e..88f66b12eef9 100644
> --- a/include/linux/wmi.h
> +++ b/include/linux/wmi.h
> @@ -13,25 +13,44 @@
>  #include <linux/mod_devicetable.h>
>  #include <uapi/linux/wmi.h>
> 
> +/**
> + * struct wmi_device - WMI device structure
> + * @dev: Device associated with this WMI device
> + * @setable: True for devices implementing the Set Control Method

private: fields are not normally documented in kernel-doc.

> + *
> + * This represents WMI devices discovered by the WMI driver core.
> + */
>  struct wmi_device {
>  	struct device dev;
> 
> -	 /* True for data blocks implementing the Set Control Method */
> +	/* private: used by the WMI driver core */
>  	bool setable;
>  };
> 
> -/* evaluate the ACPI method associated with this device */
>  extern acpi_status wmidev_evaluate_method(struct wmi_device *wdev,
>  					  u8 instance, u32 method_id,
>  					  const struct acpi_buffer *in,
>  					  struct acpi_buffer *out);
> 
> -/* Caller must kfree the result. */
>  extern union acpi_object *wmidev_block_query(struct wmi_device *wdev,
>  					     u8 instance);
> 
>  extern int set_required_buffer_size(struct wmi_device *wdev, u64 length);
> 
> +/**
> + * struct wmi_driver - WMI driver structure
> + * @driver: Driver model structure
> + * @id_table: List of WMI GUIDs supported by this driver
> + * @no_notify_data: WMI events provide no event data
> + * @probe: Callback for device binding
> + * @remove: Callback for device unbinding
> + * @notify: Callback for receiving WMI events
> + * @filter_callback: Callback for filtering device IOCTLs
> + *
> + * This represents WMI drivers which handle WMI devices.
> + * @filter_callback is only necessary for drivers which
> + * want to set up a WMI IOCTL interface.
> + */
>  struct wmi_driver {
>  	struct device_driver driver;
>  	const struct wmi_device_id *id_table;
> @@ -47,8 +66,24 @@ struct wmi_driver {
>  extern int __must_check __wmi_driver_register(struct wmi_driver *driver,
>  					      struct module *owner);
>  extern void wmi_driver_unregister(struct wmi_driver *driver);
> +
> +/**
> + * wmi_driver_register() - Helper macro ro register a WMI driver

s/ro/to/

> + * @driver: wmi_driver struct
> + *
> + * Helper macro for registering a WMI driver. It automatically passes
> + * THIS_MODULE to the underlying function.
> + */
>  #define wmi_driver_register(driver) __wmi_driver_register((driver), THIS_MODULE)
> 
> +/**
> + * module_wmi_driver() - Helper macro to register/unregister a WMI driver
> + * @__wmi_driver: wmi_driver struct
> + *
> + * Helper macro for WMI drivers which do not do anything special in module
> + * init/exit. This eliminates a lot of boilerplate. Each module may only
> + * use this macro once, and calling it replaces module_init() and module_exit().
> + */
>  #define module_wmi_driver(__wmi_driver) \
>  	module_driver(__wmi_driver, wmi_driver_register, \
>  		      wmi_driver_unregister)
> --

Reviewed-by: Randy Dunlap <rdunlap@infradead.org>
Tested-by: Randy Dunlap <rdunlap@infradead.org>

Thanks.
-- 
~Randy

Re: [PATCH 1/4] platform/x86: wmi: Add kernel doc comments

[Armin Wolf]

Am 22.04.23 um 04:54 schrieb Randy Dunlap:

> Hi,
>
> On 4/20/23 16:32, Armin Wolf wrote:
>> Add kernel doc comments useful for documenting the functions/structs
>> used to interact with the WMI driver core.
>>
>> Signed-off-by: Armin Wolf <W_Armin@gmx.de>
>> ---
>>   drivers/platform/x86/wmi.c | 51 +++++++++++++++++++++++++++++++-------
>>   include/linux/wmi.h        | 41 +++++++++++++++++++++++++++---
>>   2 files changed, 80 insertions(+), 12 deletions(-)
>>
>> diff --git a/drivers/platform/x86/wmi.c b/drivers/platform/x86/wmi.c
>> index d81319a502ef..e4b41dca70c7 100644
>> --- a/drivers/platform/x86/wmi.c
>> +++ b/drivers/platform/x86/wmi.c
>> @@ -248,7 +248,9 @@ static acpi_status get_event_data(const struct wmi_block *wblock, struct acpi_bu
>>    * @wdev: A wmi bus device from a driver
>>    * @length: Required buffer size
>>    *
>> - * Allocates memory needed for buffer, stores the buffer size in that memory
>> + * Allocates memory needed for buffer, stores the buffer size in that memory.
>> + *
>> + * Return: 0 on success or a negative error code for failure.
>>    */
>>   int set_required_buffer_size(struct wmi_device *wdev, u64 length)
>>   {
>> @@ -269,7 +271,9 @@ EXPORT_SYMBOL_GPL(set_required_buffer_size);
>>    * @in: Buffer containing input for the method call
>>    * @out: Empty buffer to return the method results
>>    *
>> - * Call an ACPI-WMI method
>> + * Call an ACPI-WMI method, the caller must free @out.
>> + *
>> + * Return: acpi_status signaling success or error.
>>    */
>>   acpi_status wmi_evaluate_method(const char *guid_string, u8 instance, u32 method_id,
>>   				const struct acpi_buffer *in, struct acpi_buffer *out)
>> @@ -294,7 +298,9 @@ EXPORT_SYMBOL_GPL(wmi_evaluate_method);
>>    * @in: Buffer containing input for the method call
>>    * @out: Empty buffer to return the method results
>>    *
>> - * Call an ACPI-WMI method
>> + * Call an ACPI-WMI method, the caller must free @out.
>> + *
>> + * Return: acpi_status signaling success or error.
>>    */
>>   acpi_status wmidev_evaluate_method(struct wmi_device *wdev, u8 instance, u32 method_id,
>>   				   const struct acpi_buffer *in, struct acpi_buffer *out)
>> @@ -411,7 +417,9 @@ static acpi_status __query_block(struct wmi_block *wblock, u8 instance,
>>    * @instance: Instance index
>>    * @out: Empty buffer to return the contents of the data block to
>>    *
>> - * Return the contents of an ACPI-WMI data block to a buffer
>> + * Query a ACPI-WMI block, the caller must free @out.
>> + *
>> + * Return: ACPI object containing the content of the WMI block.
>>    */
>>   acpi_status wmi_query_block(const char *guid_string, u8 instance,
>>   			    struct acpi_buffer *out)
>> @@ -427,6 +435,15 @@ acpi_status wmi_query_block(const char *guid_string, u8 instance,
>>   }
>>   EXPORT_SYMBOL_GPL(wmi_query_block);
>>
>> +/**
>> + * wmidev_block_query - Return contents of a WMI block
>> + * @wdev: A wmi bus device from a driver
>> + * @instance: Instance index
>> + *
>> + * Query a ACPI-WMI block, the caller must free the result.
>        Query an ACPI-WMI block,
>
>> + *
>> + * Return: ACPI object containing the content of the WMI block.
>> + */
>>   union acpi_object *wmidev_block_query(struct wmi_device *wdev, u8 instance)
>>   {
>>   	struct acpi_buffer out = { ACPI_ALLOCATE_BUFFER, NULL };
>> @@ -445,7 +462,9 @@ EXPORT_SYMBOL_GPL(wmidev_block_query);
>>    * @instance: Instance index
>>    * @in: Buffer containing new values for the data block
>>    *
>> - * Write the contents of the input buffer to an ACPI-WMI data block
>> + * Write the contents of the input buffer to an ACPI-WMI data block.
>> + *
>> + * Return: acpi_status signaling success or error.
>>    */
>>   acpi_status wmi_set_block(const char *guid_string, u8 instance,
>>   			  const struct acpi_buffer *in)
>> @@ -555,6 +574,8 @@ static void wmi_notify_debug(u32 value, void *context)
>>    * @data: Data to be returned to handler when event is fired
>>    *
>>    * Register a handler for events sent to the ACPI-WMI mapper device.
>> + *
>> + * Return: acpi_status signaling success or error.
>>    */
>>   acpi_status wmi_install_notify_handler(const char *guid,
>>   				       wmi_notify_handler handler,
>> @@ -597,6 +618,8 @@ EXPORT_SYMBOL_GPL(wmi_install_notify_handler);
>>    * @guid: 36 char string of the form fa50ff2b-f2e8-45de-83fa-65417f2f49ba
>>    *
>>    * Unregister handler for events sent to the ACPI-WMI mapper device.
>> + *
>> + * Return: acpi_status signaling success or error.
>>    */
>>   acpi_status wmi_remove_notify_handler(const char *guid)
>>   {
>> @@ -641,9 +664,11 @@ EXPORT_SYMBOL_GPL(wmi_remove_notify_handler);
>>    * wmi_get_event_data - Get WMI data associated with an event
>>    *
>>    * @event: Event to find
>> - * @out: Buffer to hold event data. out->pointer should be freed with kfree()
>> + * @out: Buffer to hold event data
>> + *
>> + * Get extra data associated with an WMI event, the caller needs to free @out.
>>    *
>> - * Returns extra data associated with an event in WMI.
>> + * Return: acpi_status signaling success or error.
>>    */
>>   acpi_status wmi_get_event_data(u32 event, struct acpi_buffer *out)
>>   {
>> @@ -664,7 +689,9 @@ EXPORT_SYMBOL_GPL(wmi_get_event_data);
>>    * wmi_has_guid - Check if a GUID is available
>>    * @guid_string: 36 char string of the form fa50ff2b-f2e8-45de-83fa-65417f2f49ba
>>    *
>> - * Check if a given GUID is defined by _WDG
>> + * Check if a given GUID is defined by _WDG.
>> + *
>> + * Return: True if GUID is available, false otherwise.
>>    */
>>   bool wmi_has_guid(const char *guid_string)
>>   {
>> @@ -678,7 +705,7 @@ EXPORT_SYMBOL_GPL(wmi_has_guid);
>>    *
>>    * Find the _UID of ACPI device associated with this WMI GUID.
>>    *
>> - * Return: The ACPI _UID field value or NULL if the WMI GUID was not found
>> + * Return: The ACPI _UID field value or NULL if the WMI GUID was not found.
>>    */
>>   char *wmi_get_acpi_device_uid(const char *guid_string)
>>   {
>> @@ -1454,6 +1481,12 @@ int __must_check __wmi_driver_register(struct wmi_driver *driver,
>>   }
>>   EXPORT_SYMBOL(__wmi_driver_register);
>>
>> +/**
>> + * wmi_driver_unregister() - Unregister a WMI driver
>> + * @driver: WMI driver to unregister
>> + *
>> + * Unregisters a WMI driver from the WMI bus.
>> + */
>>   void wmi_driver_unregister(struct wmi_driver *driver)
>>   {
>>   	driver_unregister(&driver->driver);
>> diff --git a/include/linux/wmi.h b/include/linux/wmi.h
>> index b88d7b58e61e..88f66b12eef9 100644
>> --- a/include/linux/wmi.h
>> +++ b/include/linux/wmi.h
>> @@ -13,25 +13,44 @@
>>   #include <linux/mod_devicetable.h>
>>   #include <uapi/linux/wmi.h>
>>
>> +/**
>> + * struct wmi_device - WMI device structure
>> + * @dev: Device associated with this WMI device
>> + * @setable: True for devices implementing the Set Control Method
> private: fields are not normally documented in kernel-doc.

Hi,

since @setable is only used internally by the WMI driver core, i thought it might
be beneficial to exclude it from the normal driver interface documentation and only
use it for subsystem-internal documentation.

Armin Wolf

>> + *
>> + * This represents WMI devices discovered by the WMI driver core.
>> + */
>>   struct wmi_device {
>>   	struct device dev;
>>
>> -	 /* True for data blocks implementing the Set Control Method */
>> +	/* private: used by the WMI driver core */
>>   	bool setable;
>>   };
>>
>> -/* evaluate the ACPI method associated with this device */
>>   extern acpi_status wmidev_evaluate_method(struct wmi_device *wdev,
>>   					  u8 instance, u32 method_id,
>>   					  const struct acpi_buffer *in,
>>   					  struct acpi_buffer *out);
>>
>> -/* Caller must kfree the result. */
>>   extern union acpi_object *wmidev_block_query(struct wmi_device *wdev,
>>   					     u8 instance);
>>
>>   extern int set_required_buffer_size(struct wmi_device *wdev, u64 length);
>>
>> +/**
>> + * struct wmi_driver - WMI driver structure
>> + * @driver: Driver model structure
>> + * @id_table: List of WMI GUIDs supported by this driver
>> + * @no_notify_data: WMI events provide no event data
>> + * @probe: Callback for device binding
>> + * @remove: Callback for device unbinding
>> + * @notify: Callback for receiving WMI events
>> + * @filter_callback: Callback for filtering device IOCTLs
>> + *
>> + * This represents WMI drivers which handle WMI devices.
>> + * @filter_callback is only necessary for drivers which
>> + * want to set up a WMI IOCTL interface.
>> + */
>>   struct wmi_driver {
>>   	struct device_driver driver;
>>   	const struct wmi_device_id *id_table;
>> @@ -47,8 +66,24 @@ struct wmi_driver {
>>   extern int __must_check __wmi_driver_register(struct wmi_driver *driver,
>>   					      struct module *owner);
>>   extern void wmi_driver_unregister(struct wmi_driver *driver);
>> +
>> +/**
>> + * wmi_driver_register() - Helper macro ro register a WMI driver
> s/ro/to/
>
>> + * @driver: wmi_driver struct
>> + *
>> + * Helper macro for registering a WMI driver. It automatically passes
>> + * THIS_MODULE to the underlying function.
>> + */
>>   #define wmi_driver_register(driver) __wmi_driver_register((driver), THIS_MODULE)
>>
>> +/**
>> + * module_wmi_driver() - Helper macro to register/unregister a WMI driver
>> + * @__wmi_driver: wmi_driver struct
>> + *
>> + * Helper macro for WMI drivers which do not do anything special in module
>> + * init/exit. This eliminates a lot of boilerplate. Each module may only
>> + * use this macro once, and calling it replaces module_init() and module_exit().
>> + */
>>   #define module_wmi_driver(__wmi_driver) \
>>   	module_driver(__wmi_driver, wmi_driver_register, \
>>   		      wmi_driver_unregister)
>> --
> Reviewed-by: Randy Dunlap <rdunlap@infradead.org>
> Tested-by: Randy Dunlap <rdunlap@infradead.org>
>
> Thanks.

Re: [PATCH 1/4] platform/x86: wmi: Add kernel doc comments

[Randy Dunlap]

On 4/24/23 14:20, Armin Wolf wrote:
> Am 22.04.23 um 04:54 schrieb Randy Dunlap:
> 
>> Hi,
>>
>> On 4/20/23 16:32, Armin Wolf wrote:
>>> Add kernel doc comments useful for documenting the functions/structs
>>> used to interact with the WMI driver core.
>>>
>>> Signed-off-by: Armin Wolf <W_Armin@gmx.de>
>>> ---
>>>   drivers/platform/x86/wmi.c | 51 +++++++++++++++++++++++++++++++-------
>>>   include/linux/wmi.h        | 41 +++++++++++++++++++++++++++---
>>>   2 files changed, 80 insertions(+), 12 deletions(-)
>>>

>>> +/**
>>> + * struct wmi_device - WMI device structure
>>> + * @dev: Device associated with this WMI device
>>> + * @setable: True for devices implementing the Set Control Method
>> private: fields are not normally documented in kernel-doc.
> 
> Hi,
> 
> since @setable is only used internally by the WMI driver core, i thought it might
> be beneficial to exclude it from the normal driver interface documentation and only
> use it for subsystem-internal documentation.

OK then. :)

-- 
~Randy