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. > > 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
