lkml.org 
[lkml]   [2017]   [Oct]   [4]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH v4 05/14] platform/x86: dell-wmi-descriptor: split WMI descriptor into it's own driver
    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.limonciello@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.rohar@gmail.com>
    > S: Maintained
    > F: drivers/platform/x86/dell-wmi.c
    >
    > +DELL WMI DESCRIPTOR DRIVER
    > +M: Mario Limonciello <mario.limonciello@dell.com>
    > +S: Maintained
    > +F: drivers/platform/x86/dell-wmi-descriptor.c
    > +
    > DELTA ST MEDIA DRIVER
    > M: Hugues Fruchet <hugues.fruchet@st.com>
    > L: linux-media@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

    \
     
     \ /
      Last update: 2017-10-05 03:10    [W:4.553 / U:0.336 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site