Message-ID: <20241205002733.2183537-3-kuurtb@gmail.com>
Date: Wed,  4 Dec 2024 21:27:35 -0300
From: Kurt Borja <kuurtb@...il.com>
To: platform-driver-x86@...r.kernel.org
Cc: Dell.Client.Kernel@...l.com,
	hdegoede@...hat.com,
	ilpo.jarvinen@...ux.intel.com,
	linux-kernel@...r.kernel.org,
	mario.limonciello@....com,
	w_armin@....de,
	Kurt Borja <kuurtb@...il.com>
Subject: [RFC PATCH 00/21] alienware-wmi driver rework

Hi :)

This series are a follow-up to this discussion [1], in which I proposed
migrating the alienware-wmi driver to use:

1. State container driver model
2. Modern WMI driver design
3. Drop use of deprecated WMI methods

Of course, this was much harder than expected to do cleanly. Main
problem was that this driver "drives" two completely different devices
(I'm not referring to the WMI devices, which also happen to be two).

Throughout these series we will call these devices AlienFX and AWCC.

As a preamble
=============

AlienFX exposes a LED, hdmi, amplifier and deepsleep interface to
userspace through a platform device named "alienware-wmi". Historically
this driver handled this by leveraging on two WMI devices as a backend.
This devices named LEGACY and WMAX were very similar, the only
difference was that WMAX had more features, but share all features
LEGACY had. Although it's a stretch, it could be argued this WMI devices
are the "same", just different GUID.

Later Dell repurposed the WMAX WMI device to serve as a thermal control
interface for all newer "gaming" laptops. This new WMAX device has an
ACPI UID = "AWCC" (I discovered this recently). So it could also be
argued that old WMAX and AWCC WMAX are not the same device, just same
GUID.

This drivers manages all these features using deprecated WMI methods.

Approach I took for the rework
==============================

Parts 1-7 sort of containerize all AlienFX functionality under the
"alienware-wmi" platform driver so WMI drivers can prepare and register
a matching platform device from the probe.

Parts 8-12 create and register two WMI drivers for the LEGACY and WMAX
devices respectively. The code for these probes is VERY similar and
all "differences" are passed to the platform device via platform
specific data (platdata). Also AlienFX functionality is refactored to
use non-deprecated WMI methods.

Parts 13-17 migrate all AWCC methods to use non-deprecated WMI methods
and the state container driver model.

Parts 18-21 I splitted the alienware-wmi.c module into the different
features this driver manages.

alienware-wmi-base.c is in charge of initializing WMI drivers and
define some platform specific data, like operations (Part 10 for more
info). alienware-wmi-alienfx.c has all AlienFX functionality and
alienware-wmi-awcc.c has all AWCC functionality.

Coments
=======

This is still kind of a draft, but I did some testing and it works!

Of course I will do thorough testing and cleanup when I send the
non-RFC version. I just want to get some comments on the general
approach before proceeding further.

I think this is quite messy in it's current state so I apollogize.

@Mario Limonciello: I included the reviews you gave me on [2]. I
included some of those patches here, and dropped the ones that did not
make sense with this design. As this is another series let me know if
you want me to drop the tags!

@Armin Wolf: I don't like the amount of files I made. As the maintainer
of the wmi module, what do you think about making two independent
modules, one for AlienFX and one for AWCC. In order to not register two
drivers for the WMAX device the module init would check if the "AWCC"
UID is present.

The approach for that would be basically the same, and I think the
series would change very little.

I would like this a lot because I still think old and new WMAX devices
are different, but I couldn't find another example of where an OEM
repurposed a WMI device.

@Everyone: I know this is VERY long. Thank you so much for your time in
advance!

This series were made on top of the 'for-next' branch:

Commit c712e8fd9bf4 ("MAINTAINERS: Change AMD PMC driver status to "Supported"")

~ Kurt

[1] https://lore.kernel.org/platform-driver-x86/6m66cuivkzhcsvpjv4nunjyddqhr42bmjdhptu4bqm6rm7fvxf@qjwove4hg6gb/T/#u
[2] https://lore.kernel.org/platform-driver-x86/20241120163834.6446-3-kuurtb@gmail.com/

Kurt Borja (21):
  alienware-wmi: Modify parse_rgb() signature
  alienware-wmi: Move Lighting Control State
  alienware-wmi: Remove unnecessary check at module exit
  alienware-wmi: Improve sysfs groups creation
  alienware-wmi: Refactor rgb-zones sysfs group creation
  alienware-wmi: Add state container and alienfx_probe()
  alienware-wmi: Migrate to state container pattern
  alienware-wmi: Add WMI Drivers
  alienware-wmi: Initialize WMI drivers
  alienware-wmi: Add alienfx OPs to platdata
  alienware-wmi: Refactor LED control methods
  alienware-wmi: Refactor hdmi, amplifier, deepslp
  alienware-wmi: Add a state container for AWCC
  alienware-wmi: Migrate thermal methods to wmidev
  alienware-wmi: Refactor sysfs visibility methods
  alienware-wmi: Make running control state part of platdata
  alienware-wmi: Drop thermal methods dependency on quirks
  platform-x86: Add header file for alienware-wmi
  platform-x86: Rename alienare-wmi
  platform-x86: Split the alienware-wmi module
  platform-x86: Add config entries to alienware-wmi

 MAINTAINERS                                   |    3 +-
 drivers/platform/x86/dell/Kconfig             |   25 +-
 drivers/platform/x86/dell/Makefile            |    5 +-
 .../platform/x86/dell/alienware-wmi-alienfx.c |  531 +++++++
 .../platform/x86/dell/alienware-wmi-awcc.c    |  282 ++++
 .../platform/x86/dell/alienware-wmi-base.c    |  525 +++++++
 drivers/platform/x86/dell/alienware-wmi.c     | 1267 -----------------
 drivers/platform/x86/dell/alienware-wmi.h     |  141 ++
 8 files changed, 1505 insertions(+), 1274 deletions(-)
 create mode 100644 drivers/platform/x86/dell/alienware-wmi-alienfx.c
 create mode 100644 drivers/platform/x86/dell/alienware-wmi-awcc.c
 create mode 100644 drivers/platform/x86/dell/alienware-wmi-base.c
 delete mode 100644 drivers/platform/x86/dell/alienware-wmi.c
 create mode 100644 drivers/platform/x86/dell/alienware-wmi.h

-- 
2.47.1

Powered by blists - more mailing lists