mbox series

[0/5] platform/x86: dell: Add new dell-wmi-ddv driver

Message ID 20220912125342.7395-1-W_Armin@gmx.de
Headers show
Series platform/x86: dell: Add new dell-wmi-ddv driver | expand

Message

Armin Wolf Sept. 12, 2022, 12:53 p.m. UTC 
This patch series adds a new driver for a WMI interface found in
many newer Dell machines. This interface allows to read battery
properties like temperature and the ePPID (Dell-specific), while
also providing fan and thermal sensor information.

The interface does support multiple batteries which are indetified
by an "index", which appears to be the batteries ACPI UID. Since
the interface also appears to omit any bounts checking of the
index, the ACPI battery hook mechanism is used to discover batteries.

Since the information returned when querying fan/thermal sensor
information is currently unknown, a debugfs entry is created to
allow for easier reverse engineering. The interface is likely
to be replaced by a proper hwmon interface in the future.

Since the driver can potentially be instantiated mutplie times,
the ACPI battery hook mechanism had to be extended.

The first two patches allow that battery hooks are not unloaded
if they return an error when a battery was added, so that they
can safely return -ENODEV.

The next two patches extend the battery hook mechanism to better
support drivers which can be instantiated mupltible times.

The last patch finally adds the new driver. It was called
dell-wmi-ddv since the interface is called "DDV" by Dell software,
likely meaning "Dell Data Vault".

The driver was tested, together with the changes made to the
ACPI battery driver, on a Dell Inspiron 3505. Other drivers
already using the battery hook mechanism where changed as well,
but could only be compile-tested due to missing hardware.

Armin Wolf (5):
  ACPI: battery: Do not unload battery hooks on single error
  ACPI: battery: Simplify battery_hook_unregister()
  ACPI: battery: Allow battery hooks to be registered multiple times.
  ACPI: battery: Allow for passing data to battery hooks.
  platform/x86: dell: Add new dell-wmi-ddv driver

 .../ABI/testing/debugfs-dell-wmi-ddv          |  21 +
 .../ABI/testing/sysfs-platform-dell-wmi-ddv   |  16 +
 MAINTAINERS                                   |   7 +
 drivers/acpi/battery.c                        |  59 ++-
 drivers/platform/x86/asus-wmi.c               |  20 +-
 drivers/platform/x86/dell/Kconfig             |  13 +
 drivers/platform/x86/dell/Makefile            |   1 +
 drivers/platform/x86/dell/dell-wmi-ddv.c      | 365 ++++++++++++++++++
 drivers/platform/x86/huawei-wmi.c             |  15 +-
 drivers/platform/x86/lg-laptop.c              |  14 +-
 drivers/platform/x86/system76_acpi.c          |  22 +-
 drivers/platform/x86/thinkpad_acpi.c          |  15 +-
 drivers/platform/x86/wmi.c                    |   1 +
 include/acpi/battery.h                        |  12 +-
 14 files changed, 504 insertions(+), 77 deletions(-)
 create mode 100644 Documentation/ABI/testing/debugfs-dell-wmi-ddv
 create mode 100644 Documentation/ABI/testing/sysfs-platform-dell-wmi-ddv
 create mode 100644 drivers/platform/x86/dell/dell-wmi-ddv.c

--
2.30.2

Comments

Mario Limonciello Sept. 13, 2022, 4:08 p.m. UTC | #1 
[Public]



> -----Original Message-----
> From: Armin Wolf <W_Armin@gmx.de>
> Sent: Tuesday, September 13, 2022 09:41
> To: Randy Dunlap <rdunlap@infradead.org>; hdegoede@redhat.com;
> markgross@kernel.org
> Cc: rafael@kernel.org; lenb@kernel.org; hmh@hmh.eng.br;
> matan@svgalib.org; corentin.chary@gmail.com; jeremy@system76.com;
> productdev@system76.com; platform-driver-x86@vger.kernel.org; linux-
> acpi@vger.kernel.org; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH 5/5] platform/x86: dell: Add new dell-wmi-ddv driver
> 
> Am 12.09.22 um 23:56 schrieb Randy Dunlap:
> 
> > Hi--
> >
> > On 9/12/22 05:53, Armin Wolf wrote:
> >> diff --git a/drivers/platform/x86/dell/Kconfig
> b/drivers/platform/x86/dell/Kconfig
> >> index 25421e061c47..209e63e347e2 100644
> >> --- a/drivers/platform/x86/dell/Kconfig
> >> +++ b/drivers/platform/x86/dell/Kconfig
> >> @@ -189,6 +189,19 @@ config DELL_WMI_DESCRIPTOR
> >>   	default n
> >>   	depends on ACPI_WMI
> >>
> >> +config DELL_WMI_DDV
> >> +	tristate "Dell WMI sensors Support"
> >> +	default m
> > You should (try to) justify default m, otherwise just
> > don't have a default for it.
> 
> I have chosen default m since many other Dell platform drivers are being
> default m. Since this driver is not essential for normal operation,
> i will drop default m then.

Actually Dell drivers directory are a bit unique in this regard.  There is a special
top level boolean.  I would suggest to keep it as is.

Take a look at:
menuconfig X86_PLATFORM_DRIVERS_DELL

> 
> Armin Wolf
> 
> >> +	depends on ACPI_BATTERY
> >> +	depends on ACPI_WMI
> >> +	help
> >> +	  This option adds support for WMI-based sensors like
> >> +	  battery temperature sensors found on some Dell notebooks.
> >> +	  It also supports reading of the batteries ePPID.
> >> +
> >> +	  To compile this drivers as a module, choose M here: the module will
> >> +	  be called dell-wmi-ddv.
Armin Wolf Sept. 13, 2022, 4:45 p.m. UTC | #2 
Am 13.09.22 um 18:08 schrieb Limonciello, Mario:

> [Public]
>
>
>
>> -----Original Message-----
>> From: Armin Wolf <W_Armin@gmx.de>
>> Sent: Tuesday, September 13, 2022 09:41
>> To: Randy Dunlap <rdunlap@infradead.org>; hdegoede@redhat.com;
>> markgross@kernel.org
>> Cc: rafael@kernel.org; lenb@kernel.org; hmh@hmh.eng.br;
>> matan@svgalib.org; corentin.chary@gmail.com; jeremy@system76.com;
>> productdev@system76.com; platform-driver-x86@vger.kernel.org; linux-
>> acpi@vger.kernel.org; linux-kernel@vger.kernel.org
>> Subject: Re: [PATCH 5/5] platform/x86: dell: Add new dell-wmi-ddv driver
>>
>> Am 12.09.22 um 23:56 schrieb Randy Dunlap:
>>
>>> Hi--
>>>
>>> On 9/12/22 05:53, Armin Wolf wrote:
>>>> diff --git a/drivers/platform/x86/dell/Kconfig
>> b/drivers/platform/x86/dell/Kconfig
>>>> index 25421e061c47..209e63e347e2 100644
>>>> --- a/drivers/platform/x86/dell/Kconfig
>>>> +++ b/drivers/platform/x86/dell/Kconfig
>>>> @@ -189,6 +189,19 @@ config DELL_WMI_DESCRIPTOR
>>>>    	default n
>>>>    	depends on ACPI_WMI
>>>>
>>>> +config DELL_WMI_DDV
>>>> +	tristate "Dell WMI sensors Support"
>>>> +	default m
>>> You should (try to) justify default m, otherwise just
>>> don't have a default for it.
>> I have chosen default m since many other Dell platform drivers are being
>> default m. Since this driver is not essential for normal operation,
>> i will drop default m then.
> Actually Dell drivers directory are a bit unique in this regard.  There is a special
> top level boolean.  I would suggest to keep it as is.
>
> Take a look at:
> menuconfig X86_PLATFORM_DRIVERS_DELL

Ok.

Armin Wolf

>> Armin Wolf
>>
>>>> +	depends on ACPI_BATTERY
>>>> +	depends on ACPI_WMI
>>>> +	help
>>>> +	  This option adds support for WMI-based sensors like
>>>> +	  battery temperature sensors found on some Dell notebooks.
>>>> +	  It also supports reading of the batteries ePPID.
>>>> +
>>>> +	  To compile this drivers as a module, choose M here: the module will
>>>> +	  be called dell-wmi-ddv.
Randy Dunlap Sept. 13, 2022, 6:27 p.m. UTC | #3 
On 9/13/22 09:08, Limonciello, Mario wrote:
> [Public]
> 
> 
> 
>> -----Original Message-----
>> From: Armin Wolf <W_Armin@gmx.de>
>> Sent: Tuesday, September 13, 2022 09:41
>> To: Randy Dunlap <rdunlap@infradead.org>; hdegoede@redhat.com;
>> markgross@kernel.org
>> Cc: rafael@kernel.org; lenb@kernel.org; hmh@hmh.eng.br;
>> matan@svgalib.org; corentin.chary@gmail.com; jeremy@system76.com;
>> productdev@system76.com; platform-driver-x86@vger.kernel.org; linux-
>> acpi@vger.kernel.org; linux-kernel@vger.kernel.org
>> Subject: Re: [PATCH 5/5] platform/x86: dell: Add new dell-wmi-ddv driver
>>
>> Am 12.09.22 um 23:56 schrieb Randy Dunlap:
>>
>>> Hi--
>>>
>>> On 9/12/22 05:53, Armin Wolf wrote:
>>>> diff --git a/drivers/platform/x86/dell/Kconfig
>> b/drivers/platform/x86/dell/Kconfig
>>>> index 25421e061c47..209e63e347e2 100644
>>>> --- a/drivers/platform/x86/dell/Kconfig
>>>> +++ b/drivers/platform/x86/dell/Kconfig
>>>> @@ -189,6 +189,19 @@ config DELL_WMI_DESCRIPTOR
>>>>   	default n
>>>>   	depends on ACPI_WMI
>>>>
>>>> +config DELL_WMI_DDV
>>>> +	tristate "Dell WMI sensors Support"
>>>> +	default m
>>> You should (try to) justify default m, otherwise just
>>> don't have a default for it.
>>
>> I have chosen default m since many other Dell platform drivers are being
>> default m. Since this driver is not essential for normal operation,
>> i will drop default m then.
> 
> Actually Dell drivers directory are a bit unique in this regard.  There is a special
> top level boolean.  I would suggest to keep it as is.
> 
> Take a look at:
> menuconfig X86_PLATFORM_DRIVERS_DELL
> 

So all of those "default m" and "default y" drivers are *needed*
as opposed to desirable?

>>
>> Armin Wolf
>>
>>>> +	depends on ACPI_BATTERY
>>>> +	depends on ACPI_WMI
>>>> +	help
>>>> +	  This option adds support for WMI-based sensors like
>>>> +	  battery temperature sensors found on some Dell notebooks.
>>>> +	  It also supports reading of the batteries ePPID.
>>>> +
>>>> +	  To compile this drivers as a module, choose M here: the module will
>>>> +	  be called dell-wmi-ddv.

thanks.
Mario Limonciello Sept. 13, 2022, 6:30 p.m. UTC | #4 
On 9/13/2022 13:27, Randy Dunlap wrote:
> 
> 
> On 9/13/22 09:08, Limonciello, Mario wrote:
>> [Public]
>>
>>
>>
>>> -----Original Message-----
>>> From: Armin Wolf <W_Armin@gmx.de>
>>> Sent: Tuesday, September 13, 2022 09:41
>>> To: Randy Dunlap <rdunlap@infradead.org>; hdegoede@redhat.com;
>>> markgross@kernel.org
>>> Cc: rafael@kernel.org; lenb@kernel.org; hmh@hmh.eng.br;
>>> matan@svgalib.org; corentin.chary@gmail.com; jeremy@system76.com;
>>> productdev@system76.com; platform-driver-x86@vger.kernel.org; linux-
>>> acpi@vger.kernel.org; linux-kernel@vger.kernel.org
>>> Subject: Re: [PATCH 5/5] platform/x86: dell: Add new dell-wmi-ddv driver
>>>
>>> Am 12.09.22 um 23:56 schrieb Randy Dunlap:
>>>
>>>> Hi--
>>>>
>>>> On 9/12/22 05:53, Armin Wolf wrote:
>>>>> diff --git a/drivers/platform/x86/dell/Kconfig
>>> b/drivers/platform/x86/dell/Kconfig
>>>>> index 25421e061c47..209e63e347e2 100644
>>>>> --- a/drivers/platform/x86/dell/Kconfig
>>>>> +++ b/drivers/platform/x86/dell/Kconfig
>>>>> @@ -189,6 +189,19 @@ config DELL_WMI_DESCRIPTOR
>>>>>    	default n
>>>>>    	depends on ACPI_WMI
>>>>>
>>>>> +config DELL_WMI_DDV
>>>>> +	tristate "Dell WMI sensors Support"
>>>>> +	default m
>>>> You should (try to) justify default m, otherwise just
>>>> don't have a default for it.
>>>
>>> I have chosen default m since many other Dell platform drivers are being
>>> default m. Since this driver is not essential for normal operation,
>>> i will drop default m then.
>>
>> Actually Dell drivers directory are a bit unique in this regard.  There is a special
>> top level boolean.  I would suggest to keep it as is.
>>
>> Take a look at:
>> menuconfig X86_PLATFORM_DRIVERS_DELL
>>
> 
> So all of those "default m" and "default y" drivers are *needed*
> as opposed to desirable?
> 

It was supposed to be a convenience option, it's first introduced in 
f1e1ea516721d1.

So if you have a Dell laptop you set the one option and then get 
defaults for all those modules.

>>>
>>> Armin Wolf
>>>
>>>>> +	depends on ACPI_BATTERY
>>>>> +	depends on ACPI_WMI
>>>>> +	help
>>>>> +	  This option adds support for WMI-based sensors like
>>>>> +	  battery temperature sensors found on some Dell notebooks.
>>>>> +	  It also supports reading of the batteries ePPID.
>>>>> +
>>>>> +	  To compile this drivers as a module, choose M here: the module will
>>>>> +	  be called dell-wmi-ddv.
> 
> thanks.
Randy Dunlap Sept. 13, 2022, 6:37 p.m. UTC | #5 
On 9/13/22 11:30, Limonciello, Mario wrote:
> On 9/13/2022 13:27, Randy Dunlap wrote:
>>
>>
>> On 9/13/22 09:08, Limonciello, Mario wrote:
>>> [Public]
>>>
>>>
>>>
>>>> -----Original Message-----
>>>> From: Armin Wolf <W_Armin@gmx.de>
>>>> Sent: Tuesday, September 13, 2022 09:41
>>>> To: Randy Dunlap <rdunlap@infradead.org>; hdegoede@redhat.com;
>>>> markgross@kernel.org
>>>> Cc: rafael@kernel.org; lenb@kernel.org; hmh@hmh.eng.br;
>>>> matan@svgalib.org; corentin.chary@gmail.com; jeremy@system76.com;
>>>> productdev@system76.com; platform-driver-x86@vger.kernel.org; linux-
>>>> acpi@vger.kernel.org; linux-kernel@vger.kernel.org
>>>> Subject: Re: [PATCH 5/5] platform/x86: dell: Add new dell-wmi-ddv driver
>>>>
>>>> Am 12.09.22 um 23:56 schrieb Randy Dunlap:
>>>>
>>>>> Hi--
>>>>>
>>>>> On 9/12/22 05:53, Armin Wolf wrote:
>>>>>> diff --git a/drivers/platform/x86/dell/Kconfig
>>>> b/drivers/platform/x86/dell/Kconfig
>>>>>> index 25421e061c47..209e63e347e2 100644
>>>>>> --- a/drivers/platform/x86/dell/Kconfig
>>>>>> +++ b/drivers/platform/x86/dell/Kconfig
>>>>>> @@ -189,6 +189,19 @@ config DELL_WMI_DESCRIPTOR
>>>>>>        default n
>>>>>>        depends on ACPI_WMI
>>>>>>
>>>>>> +config DELL_WMI_DDV
>>>>>> +    tristate "Dell WMI sensors Support"
>>>>>> +    default m
>>>>> You should (try to) justify default m, otherwise just
>>>>> don't have a default for it.
>>>>
>>>> I have chosen default m since many other Dell platform drivers are being
>>>> default m. Since this driver is not essential for normal operation,
>>>> i will drop default m then.
>>>
>>> Actually Dell drivers directory are a bit unique in this regard.  There is a special
>>> top level boolean.  I would suggest to keep it as is.
>>>
>>> Take a look at:
>>> menuconfig X86_PLATFORM_DRIVERS_DELL
>>>
>>
>> So all of those "default m" and "default y" drivers are *needed*
>> as opposed to desirable?
>>
> 
> It was supposed to be a convenience option, it's first introduced in f1e1ea516721d1.
> 
> So if you have a Dell laptop you set the one option and then get defaults for all those modules.

oh well. whatever.

thanks.

>>>>
>>>> Armin Wolf
>>>>
>>>>>> +    depends on ACPI_BATTERY
>>>>>> +    depends on ACPI_WMI
>>>>>> +    help
>>>>>> +      This option adds support for WMI-based sensors like
>>>>>> +      battery temperature sensors found on some Dell notebooks.
>>>>>> +      It also supports reading of the batteries ePPID.
>>>>>> +
>>>>>> +      To compile this drivers as a module, choose M here: the module will
>>>>>> +      be called dell-wmi-ddv.
>>
>> thanks.
>