Re: alienware-wmi rework RFC

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Am 20.11.24 um 16:04 schrieb Kurt Borja:

On Wed, Nov 20, 2024 at 01:06:57PM +0100, Armin Wolf wrote:
Am 19.11.24 um 05:29 schrieb Kurt Borja:

On Mon, Nov 18, 2024 at 09:54:25PM -0600, Mario Limonciello wrote:
Loop Dell Client Kernel M/B for any comments.

On 11/18/2024 21:47, Kurt Borja wrote:
Hi!

I'm planning on migrating the alienware-wmi driver to the new WMI
interface, as it's currently using the deprecated one.
🎉
I like this :)

My plan is to:

rename alienware-wmi.c -> alienware-wmi-base.c
create alienware-wmi.h
create alienware-wmi-legacy
create alienware-wmi-wmax

The last two files would not be independent modules, just includes for
the base module. The base module would be in charge of initializing the
platform driver plus the correct wmi_driver backend, but the wmi probes
would register the platform device. This would be very similar to what
other dell drivers already do. Aditionally I want to migrate everything
to the state container design pattern.

I would do this in such a way that the legacy and new code would be
completely independent of each other (i.e. different state containters,
dmi checks, etc).
As the original author of this driver when I was at Dell I'll add some
comments.

The 'legacy' code was very narrowly focused for supporting a handful of
hardware specifically for lighting control.  One was the original Alienware
steam machine, and then a few generations of the X51.

I don't know how much of the driver continues to work on hardware since
then.  Maybe Dell guys I added to CC can comment on how much of this has
stuck around over the years and keeps working.
My guess is that none of it works on new models. The LEGACY wmi device
is not longer included on new machines, as all lighting control is done
through an EC and the WMAX device was repurposed to fan/thermal control.
I say this based on exploring quite a few acpidumps and a couple RGB
control Windows open source alternatives.

Pros:
    - Modern interfaces and design patterns
    - This is compatible with Mario's upcoming platform profile changes as
      the WMAX device would hold a reference to the platform device
    - Would not break compatibility as legacy code is independent
    - Easier to understand and develop in the future

Cons:
    - Initialy alienware-wmi-base.c would be almost completely legacy code,
      as new features don't require a platform device (yet), so
      alienware-wmi-base would basically just register the wmax wmi driver
      on newer machines
    - With this design users would not be able to completely exclude legacy
      code with CONFIG parameters
I wonder if you're better off just having the legacy driver as it's own
kernel object?  If it only supports a handful of systems, most people won't
need it compiled.
Yes! I'd like to do this but unfortunately some user space applications
might depend on attributes being available to a platform device named
"alienware-wmi". This is why I wanted to have a unified "alienware-wmi"
platform driver.

Thank you for your feedback!
It see, that is unfortunate. In this case having a single driver which handles the platform device
and calls the right initialization function from the other two files sounds like a good choice to me.
I have another idea:

rename alienware-wmi.c -> alienware-alienfx.c
create alienware-wmi-wmax.c

Both independent modules that can be included/excluded with config
parameters, etc. alienware-alienfx would register the platform driver
and the wmi legacy device driver, while alienware-wmi-wmax would do
everything independently and only register an "alienware-wmi" platform
device if loaded on a 'legacy' laptop model.

Of course I'd make sure a platform device with the same name is not loaded
twice.

Something about this doesn't feel right but I don't know what it is.
However it is less files and less memory footprint, because ideally,
'legacy' code would not even load on newer models.

I thought about this because Mario said legacy interface is only
supported by a handful of devices and it seems like a waste of resources
to load unsupported interfaces.

I think the WMI driver for the WMAX interface still has to always register the platform device
since nearly all custom attributes (deepsleep, etc) work on the WMAX interface.

I would prefer to have a single driver which when using the legacy WMI interface will only provide
lighting features (correct me if the legacy WMI interface supports more features) but will provide
all features when using the WMAX WMI interface.

The overhead of handling the legacy WMI interface should be acceptable since it only provides lighting
services, so a lot of the code can be shared with the lighting code for the WMAX WMI interface.

Making support for the legacy WMI interface configurable when building the kernel also allows people to
trim the resource consumption should they not want legacy support.

Thanks,
Armin Wolf

Thanks,
Armin Wolf

After this I want to add HWMON and sparse keymap capabilities to the
wmax interface.
🎉

I'm sure there are things I'm not seeing so feedback is greatly
appreciated!

Regards,
Kurt





[Index of Archives]     [Linux Kernel Development]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux