Hi, On 8/28/20 11:49 PM, Vasiliy Kupriakov wrote: > Some of the new ASUS laptops like TUF Gaming FA706II do not set bit > indicating presence of method. Instead, just the fan speed is returned. > > Implement ugly way to check if for the cpu fan. > It seems that in other TUF models SPEC version and SFUN are the same. > So it checks these two values, then tries to read fan speed, and last, > checks that fan speed is in adequate range. > > Signed-off-by: Vasiliy Kupriakov <rublag-ns@xxxxxxxxx> I am worried that this patch is going to cause issues on some systems, not only does it register a bunch of files in the hwmon class device (which would be ok-ish since those won't do anything unless used), but it also causes asus_fan_set_auto() to get called during driver init. I know that you have tried to limit this to only affected devices by adding this check: if (asus->spec != 0x80001 || !(asus->sfun & 0x400000)) But doing a quick search for dmesg output on Asus devices it seems that that check will actually be true on many many devices I'm afraid. Also see the recent history of asus-wmi related to SW_TABLET_MODE support: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/log/drivers/platform/x86/asus-wmi.c Specifically: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=b0dbd97de1f1fd6b3c9a7bb8f7c795bba7e169d8 https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=1797d588af15174d4a4e7159dac8c800538e4f8c Where the first commit took a generic approach like you are doing here; and the second commit switches to a DMI based allow list. It seems that the ASUS_WMI_DSTS_PRESENCE_BIT simply is not very reliable (in either direction) and I wish we had some knowledge what the sfun fields actually means... I think for now that unfortunately the best way forward with this patch; and with your 3th patch too is to use DMI based allow-listing for using the FAN_TYPE_SPEC83 fan on devices where the ASUS_WMI_DSTS_PRESENCE_BIT is not set. What is also interesting is that your WMI versions (spec) seems to be 8.1 where as the SPEC83 stands for 8.3, so I guess that this interface was actually introduced before the 8.3 WMI API version. Regards, Hans > --- > drivers/platform/x86/asus-wmi.c | 34 +++++++++++++++++++++++++++++++++ > 1 file changed, 34 insertions(+) > > diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c > index 71559d429ba0..82505307ec17 100644 > --- a/drivers/platform/x86/asus-wmi.c > +++ b/drivers/platform/x86/asus-wmi.c > @@ -1309,6 +1309,38 @@ static bool asus_wmi_has_agfn_fan(struct asus_wmi *asus) > || (!asus->sfun && !(value & ASUS_WMI_DSTS_PRESENCE_BIT))); > } > > +/* > + * Check for fan availability. Some of the newer laptops don't set > + * the ASUS_WMI_DSTS_PRESENCE_BIT. This is ad hoc solution. It compares > + * few attributes to constants found in DSDT, then tries to read fan speed > + */ > +static bool asus_wmi_has_fan(struct asus_wmi *asus, u32 dev_id) > +{ > + int status; > + u32 value; > + > + /* > + * On multiple TUF laptops with similar DSDT interface > + * for fans, these two values are as following. > + * On some older laptops, they are different. > + */ > + if (asus->spec != 0x80001 || !(asus->sfun & 0x400000)) > + return false; > + > + status = asus_wmi_get_devstate(asus, dev_id, &value); > + if (status != 0 || value == ASUS_WMI_UNSUPPORTED_METHOD) > + return false; > + > + /* > + * Check that fan RPM is in adequate range (0 <= RPM <= 10000) > + * Note that firmware returns RPM/100. > + */ > + if (value > 100) > + return false; > + > + return true; > +} > + > static int asus_fan_set_auto(struct asus_wmi *asus) > { > int status; > @@ -1613,6 +1645,8 @@ static int asus_wmi_fan_init(struct asus_wmi *asus) > asus->fan_type = FAN_TYPE_SPEC83; > else if (asus_wmi_has_agfn_fan(asus)) > asus->fan_type = FAN_TYPE_AGFN; > + else if (asus_wmi_has_fan(asus, ASUS_WMI_DEVID_CPU_FAN_CTRL)) > + asus->fan_type = FAN_TYPE_SPEC83; > > if (asus->fan_type == FAN_TYPE_NONE) > return -ENODEV; >
