On Wed, Oct 04, 2017 at 05:48:31PM -0500, Mario Limonciello wrote:
> All communication on individual GUIDs should occur in separate drivers.
> Allowing a driver to communicate with the bus to another GUID is just
> a hack that discourages drivers to adopt the bus model.
> 
> The information found from the WMI descriptor driver is now exported
> for use by other drivers.
> 
> Signed-off-by: Mario Limonciello <mario.limoncie...@dell.com>
> ---
>  MAINTAINERS                                |   5 +
>  drivers/platform/x86/Kconfig               |  12 +++
>  drivers/platform/x86/Makefile              |   1 +
>  drivers/platform/x86/dell-wmi-descriptor.c | 162 
> +++++++++++++++++++++++++++++
>  drivers/platform/x86/dell-wmi-descriptor.h |  18 ++++
>  drivers/platform/x86/dell-wmi.c            |  88 ++--------------
>  6 files changed, 205 insertions(+), 81 deletions(-)
>  create mode 100644 drivers/platform/x86/dell-wmi-descriptor.c
>  create mode 100644 drivers/platform/x86/dell-wmi-descriptor.h
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 08b96f77f618..659dbeec4191 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -4002,6 +4002,11 @@ M:     Pali Rohár <pali.ro...@gmail.com>
>  S:   Maintained
>  F:   drivers/platform/x86/dell-wmi.c
>  
> +DELL WMI DESCRIPTOR DRIVER
> +M:   Mario Limonciello <mario.limoncie...@dell.com>
> +S:   Maintained
> +F:   drivers/platform/x86/dell-wmi-descriptor.c
> +
>  DELTA ST MEDIA DRIVER
>  M:   Hugues Fruchet <hugues.fruc...@st.com>
>  L:   linux-me...@vger.kernel.org
> diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
> index 1f7959ff055c..bc347c326d87 100644
> --- a/drivers/platform/x86/Kconfig
> +++ b/drivers/platform/x86/Kconfig
> @@ -121,6 +121,7 @@ config DELL_WMI
>       depends on DMI
>       depends on INPUT
>       depends on ACPI_VIDEO || ACPI_VIDEO = n
> +     depends on DELL_WMI_DESCRIPTOR

We have to be careful with the use of "select", but I think in this case it
makes sense.

If you "select DELL_WMI_DESCRIPTOR" here, and maintain depends on
ACPI_WMI (not shown in context here), then...

>       select DELL_SMBIOS
>       select INPUT_SPARSEKMAP
>       ---help---
> @@ -129,6 +130,17 @@ config DELL_WMI
>         To compile this driver as a module, choose M here: the module will
>         be called dell-wmi.
>  
> +config DELL_WMI_DESCRIPTOR
> +     tristate "Dell WMI descriptor"
> +     depends on ACPI_WMI
> +     default ACPI_WMI

This default can be dropped, the dependency will be met if DELL_WMI can
be enabled...

> +     ---help---
> +       Say Y here if you want to be able to read the format of WMI
> +       communications on some Dell laptops, desktops and IoT gateways.
> +
> +       To compile this driver as a module, choose M here: the module will
> +       be called dell-wmi-descriptor.

... and this can be converted to an invisible option and eliminate some
Kconfig clutter.

...


> diff --git a/drivers/platform/x86/dell-wmi.c b/drivers/platform/x86/dell-wmi.c
> index 1366bccdf275..90d7e8e55e9b 100644
...
> @@ -376,7 +375,11 @@ static void dell_wmi_notify(struct wmi_device *wdev,
>        * So to prevent reading garbage from buffer we will process only first
>        * one event on devices with WMI interface version 0.
>        */
> -     if (priv->interface_version == 0 && buffer_entry < buffer_end)
> +     if (!dell_wmi_get_interface_version(&interface_version)) {

You've introduced a dependency on another module here and haven't
guaranteed that dell-wmi-descriptor will have been loaded prior to this
one.

You'll want to add something like:

#ifdef CONFIG_DELL_WMI_DESCRIPTOR_MODULE
        if (request_module("dell_wmi_descriptor"))
                /* FAIL */
#endif

During init.

-- 
Darren Hart
VMware Open Source Technology Center

Reply via email to