Re: [PATCH v2 2/3] hwmon: (asus_wmi_ec_sensors) Support B550 Asus WMI.

From: Eugene Shalygin
Date: Sun Oct 10 2021 - 14:58:01 EST


Günter,

On Sun, 10 Oct 2021 at 16:33, Guenter Roeck <linux@xxxxxxxxxxxx> wrote:
> > Why do you believe they are more reliable? How does it cover more motherboards?
> >
>
> You said yourself below: "I know the naive reading from the ACPI EC registers
> leads to problems (fans get stuck, etc.)".

I also know what the WMI functions (BREC, RSIO, WSIO, RHWM, ) do, as I
see their source. They lock the mutex (\AMW0.ASMX) and through
multi-level mapping access EC or Super I/O chip registers. I know all
the hardware access WMI functions begin with acquiring the same mutex,
and those accessing the SIO acquire one more mutex
(\_SB.PCI0.SBRG.SIO1.MUT0). Thus locking the same mutex and accessing
the hardware directly is safe.

By naive I meant reading EC registers without acquiring the ACPI mutex.

> Something in the WMI code is obviously broken and, ultimately, will need
> to get fixed. I don't know if that something is on the ASUS side or on the
> kernel side, or on the interface between the two. A single WMI call taking
> 1 second is way too long and strongly suggests that some timeout is involved.
> Not using WMI because of that just seems wrong.

On that machine a single reading of the EC register (i.e. a call to
ec_read()) takes approx. 14 ms. The timeout is probably right here.

>From that the 990 ms delay of BREC is understandable, because for each
register it performs 4 EC transactions: read current bank,
unconditionally switch current bank, read register, switch to the old
bank. When I ask it to read 14 registers, it needs 42 ec_transaction()
calls which would sum up to 600 ms alone. In the new approach I switch
EC banks only when needed (currently only 2 times including rollback
to the previous bank) and save a lot of ec_transaction() calls. It
would help, however, to extend the EC interface in the acpi/ec.c to
allow for block reads in a single transaction.

I don't know why ec_transaction() takes so long, but in any case the
API of the WMI BREC function dictates it to perform slowly because it
needs to do a bank switch for each item in the input array, while the
direct reading allows us to avoid that bloating.

Regards,
Eugene