Re: Regression in asus-wmi due to fan curve patches

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

 



Hi,

On 2/5/22 11:10, Abhijeet Viswa wrote:
> Hi,
> 
> Firstly, apologies if I have included/excluded the wrong mailing list or persons in this email. This is my first time doing this and I've tried my best to make sure it is accurate.

No worries, it looks like you've done a pretty good job at picking the
right people + lists. And even if you didn't with regressions like this
*the* most important thing is to get the word out quickly, so thank
you for doing that!

> I am facing a regression in the mainline of the kernel (commit 0457e5153e0e8420134f60921349099e907264ca) with the asus-wmi platform driver. The driver fails to load with the following dmesg:
> 
>     asus-nb-wmi: probe of asus-nb-wmi failed with error -61
> 
> I have an ASUS TUF FX506 laptop.
> 
> I traced the regression to the method fan_curve_get_factory_default. It calls a WMI method which is expected to return a data buffer. However, if the device does not support fan curve method it is supposed to return the integer error code ASUS_WMI_UNSUPPORTED_METHOD.
> 
> However, my laptop returns a value 0 to indicate that the method is not supported:
> 
>                 If ((IIA0 == 0x00110024))
>                 {
>                     Return (Zero)
>                 }
> 
>                 If ((IIA0 == 0x00110025))
>                 {
>                     Return (Zero)
>                 }
> 
> This means that on lines 395-407 in the method asus_wmi_evaluate_method_buf, the if condition err == 0 evaluates to true an -ENODATA (-61) is returned.
> 
>         case ACPI_TYPE_INTEGER:
>                 err = (u32)obj->integer.value;
> 
>                 if (err == ASUS_WMI_UNSUPPORTED_METHOD)
>                         err = -ENODEV;
>                 /*
>                  * At least one method returns a 0 with no buffer if no arg
>                  * is provided, such as ASUS_WMI_DEVID_CPU_FAN_CURVE
>                  */
>                 if (err == 0)
>                         err = -ENODATA;
>                 break;
> 
> I am not sure the extent of ASUS laptops that are affected. TUF series laptops do not support fan curve control and so I presume many of them are affected by this regression.
> 
> I have tested a patch which selectively ignores the -ENODATA error code when probing for fan curve control. However, I'm not sure if this is the right way to do things and hence have no included the patch here.

That sounds like it is the right way to do it, can you share your patch please ?

And in the case I end up using your patch as is, may I add a:

Signed-off-by: Abhijeet Viswa <abhijeetviswa@xxxxxxxxx>

To the patch when committing it?

By adding this line you indicate that you are the author of the code and
are submitting it under the existing license for the file which you are
modifying (typically GPL-2.0) or that you have permission from the author
to submit it under this license. See:

https://www.kernel.org/doc/html/latest/process/submitting-patches.html#sign-your-work-the-developer-s-certificate-of-origin

> Once again, I apologize if this email is not how things are normally done and would love to hear feedback on the same.

Once again, this is the right way, so no worries and thank you for reporting this.

Regards,

Hans




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

  Powered by Linux