On Fri, 30 May 2025 at 23:16, Armin Wolf <W_Armin@xxxxxx> wrote: > > Am 30.05.25 um 22:50 schrieb Antheas Kapenekakis: > > > On Mon, 19 May 2025 at 04:38, Armin Wolf <W_Armin@xxxxxx> wrote: > >> Am 11.05.25 um 22:44 schrieb Antheas Kapenekakis: > >> > >>> This draft patch series brings into parity the msi-wmi-platform driver with > >>> the MSI Center M Windows application for the MSI Claw (all models). > >>> Unfortunately, MSI Center M and this interface do not have a discovery API, > >>> necessitating the introduction of a quirk system. > >>> > >>> While this patch series is fully functional and tested, there are still > >>> some issues that need to be addressed: > >>> - Armin notes we need to disable fan curve support by default and quirk > >>> it as well, as it is not supported on all models. However, the way > >>> PWM enable ops work, this makes it a bit difficult, so I would like > >>> some suggestions on how to rework this. > >>> - It turns out that to fully disable the fan curve, we have to restore > >>> the default fan values. This is also what is done on the OEM software. > >>> For this, the last patch in the series is used, which is a bit dirty. > >>> > >>> Sleep was tested with all values being preserved during S0iX (platform > >>> profile, fan curve, PL1/PL2), so we do not need suspend/resume hooks, at > >>> least for the Claw devices. > >>> > >>> For PL1/PL2, we use firmware-attributes. So for that I +cc Kurt since if > >>> his new high level interface is merged beforehand, we can use that instead. > >> Overall the patch series looks promising, however the suspend/resume handling > >> and the quirk system still needs some work. > >> > >> If you wish i can provide you with a patch for the EC-based quirk system. You > >> can then structure your exiting patches around that. > > Hi, > > Sorry I have been busy with personal life. I will try to get back to > > this in 1-2 weeks. > > > > I have three minor concerns that mirror each other with using an EC based check. > > > > 1) First is that we use boardname on the userspace side to check for > > the Claw. Therefore, using the EC ID kernel side introduces a failure > > point I am not very fond of. 2) Second is that collecting the IDs from > > users might prove more difficult 3) userspace software from MSI uses > > boardname as well. > > Actually the EC ID contains the board name (among other data). I envisioned that we > rely on the board name reported by the EC instead of the board name reported over SMBIOS. > This would allow us to better support model variations that share a common board name. > > Maybe we can still expose some data (EC ID, debugfs interface) even if a given board is > not whitelisted. This way users can easily retrieve the EC ID with the board name even > on unknown boards. Would a hybrid approach be an option perhaps? In my mind, Id say an info message in dmesg if the board is not supported should be enough. That's what MSI-EC does. Are there any other platform drivers that bind to EC ID? Antheas > Thanks, > Armin Wolf > > > Could we use a hybrid approach perhaps? What do you think? > > > > Antheas > > > >> Thanks, > >> Armin Wolf > >> > >>> Antheas Kapenekakis (8): > >>> platform/x86: msi-wmi-platform: Add unlocked msi_wmi_platform_query > >>> platform/x86: msi-wmi-platform: Add quirk system > >>> platform/x86: msi-wmi-platform: Add platform profile through shift > >>> mode > >>> platform/x86: msi-wmi-platform: Add PL1/PL2 support via firmware > >>> attributes > >>> platform/x86: msi-wmi-platform: Add charge_threshold support > >>> platform/x86: msi-wmi-platform: Drop excess fans in dual fan devices > >>> platform/x86: msi-wmi-platform: Update header text > >>> platform/x86: msi-wmi-platform: Restore fan curves on PWM disable and > >>> unload > >>> > >>> Armin Wolf (2): > >>> platform/x86: msi-wmi-platform: Use input buffer for returning result > >>> platform/x86: msi-wmi-platform: Add support for fan control > >>> > >>> .../wmi/devices/msi-wmi-platform.rst | 26 + > >>> drivers/platform/x86/Kconfig | 3 + > >>> drivers/platform/x86/msi-wmi-platform.c | 1181 ++++++++++++++++- > >>> 3 files changed, 1156 insertions(+), 54 deletions(-) > >>> > >>> > >>> base-commit: 62b1dcf2e7af3dc2879d1a39bf6823c99486a8c2