Re: acpi_wmi noisy without EC

On 17.12.2020 11:24, Yuri Pankov wrote:
> Yuri Pankov wrote:
>> On Tue, Nov 17, 2020, at 4:00 PM, Vladimir Kondratyev wrote:
>>> On 2020-11-17 15:29, Yuri Pankov wrote:
>>>> On Tue, Nov 17, 2020, at 11:07 AM, Vladimir Kondratyev wrote:
>>>>> On 2020-11-17 10:57, Vladimir Kondratyev wrote:
>>>>>> On 2020-11-17 03:00, Yuri Pankov wrote:
>>>>>>> I have started seeing the following on boot since some time:
>>>>>>>
>>>>>>> acpi_wmi0: <ACPI-WMI mapping> on acpi0
>>>>>>> acpi_wmi0: cannot find EC device
>>>>>>> device_attach: acpi_wmi0 attach returned 6
>>>>>>> acpi_wmi0: <ACPI-WMI mapping> on acpi0
>>>>>>> acpi_wmi0: cannot find EC device
>>>>>>> device_attach: acpi_wmi0 attach returned 6
>>>>>>> acpi_wmi0: <ACPI-WMI mapping> on acpi0
>>>>>>> acpi_wmi0: cannot find EC device
>>>>>>> device_attach: acpi_wmi0 attach returned 6
>>>>>>> acpi_wmi0: <ACPI-WMI mapping> on acpi0
>>>>>>> acpi_wmi0: cannot find EC device
>>>>>>> device_attach: acpi_wmi0 attach returned 6
>>>>>>>
>>>>>>> Likely following this commit:
>>>>>>>
>>>>>>> commit 708d048ccfdacf6199cc08a56aa05a9c899441fd
>>>>>>> Author: Vladimir Kondratyev <wulf_at_FreeBSD.org>
>>>>>>> Date:   Sat Oct 31 22:19:39 2020 +0000
>>>>>>>
>>>>>>>      acpi_wmi(4): Add ACPI_PNP_INFO
>>>>>>>
>>>>>>> While the reason is obvious -- there's no EC in this system
>>>>>>> (Gigabyte
>>>>>>> X299X AORUS MASTER desktop motherboard), at least searching the
>>>>>>> `acpidump -dt` output doesn't show any PNP0C09 entries -- it
>>>>>>> certainly
>>>>>>> looks like "something is broken" when first noticed.  I wonder if we
>>>>>>> could/should handle this gracefully -- no EC, do nothing, simply
>>>>>>> exit?
>>>>>>
>>>>>> Following patch should ignore missing EC like Linux does. Could you
>>>>>> test it?
>>>>>>
>>>>>> diff --git a/sys/dev/acpi_support/acpi_wmi.c
>>>>>> b/sys/dev/acpi_support/acpi_wmi.c
>>>>>> index 379cfd1705f1..efae96cdcc9a 100644
>>>>>> --- a/sys/dev/acpi_support/acpi_wmi.c
>>>>>> +++ b/sys/dev/acpi_support/acpi_wmi.c
>>>>>> _at__at_ -246,7 +246,7 _at__at_ acpi_wmi_attach(device_t dev)
>>>>>>   if ((sc->ec_dev = devclass_get_device(devclass_find("acpi_ec"), 0))
>>>>>>       == NULL)
>>>>>>   device_printf(dev, "cannot find EC device\n");
>>>>>> - else if (ACPI_FAILURE((status =
>>>>>> AcpiInstallNotifyHandler(sc->wmi_handle,
>>>>>> + if (ACPI_FAILURE((status = AcpiInstallNotifyHandler(sc->wmi_handle,
>>>>>>       ACPI_DEVICE_NOTIFY, acpi_wmi_notify_handler, sc))))
>>>>>>   device_printf(sc->wmi_dev, "couldn't install notify handler -
>>>>>> %s\n",
>>>>>>       AcpiFormatException(status));
>>>>>> _at__at_ -701,6 +701,8 _at__at_ acpi_wmi_ec_handler(UINT32 function,
>>>>>> ACPI_PHYSICAL_ADDRESS address,
>>>>>>   return (AE_BAD_PARAMETER);
>>>>>>   if (address + (width / 8) - 1 > 0xFF)
>>>>>>   return (AE_BAD_ADDRESS);
>>>>>> + if (sc->ec_dev == NULL)
>>>>>> + return (AE_NOT_FOUND);
>>>>>>   if (function == ACPI_READ)
>>>>>>   *value = 0;
>>>>>>   ec_addr = address;
>>>>>
>>>>> _at_#_at_##! Web client ate all the tabs.
>>>>>
>>>>> Patch is in attachment.
>>>>
>>>> Output changed, though it's still somewhat noisy -- I guess there
>>>> isn't a way to NOT report the device that we are not going to attach
>>>> to, or do that e.g. only for verbose boot?
>>>>
>>>> acpi_wmi0: <ACPI-WMI mapping> on acpi0
>>>> acpi_wmi0: cannot find EC device
>>>> acpi_wmi0: Embedded MOF found
>>>> ACPI: \134GSA1.WQCC: 1 arguments were passed to a non-method ACPI
>>>> object (Buffer) (20201113/nsarguments-361)
>>>> acpi_wmi1: <ACPI-WMI mapping> on acpi0
>>>> acpi_wmi1: cannot find EC device
>>>> acpi_wmi2: <ACPI-WMI mapping> on acpi0
>>>> acpi_wmi2: cannot find EC device
>>>> acpi_wmi3: <ACPI-WMI mapping> on acpi0
>>>> acpi_wmi3: cannot find EC device
>>>
>>> acpi_wmi does not try to attach to EC node (PNP0C09). It only queries it
>>> in OpRegion handler.
>>> WMI's _HID/_CID is PNP0C14. According to your output, acpi_wmi has
>>> successfully attached to 4 nodes.
>>
>> Oh great, I misunderstood then.  And indeed, sysctl -b
>> dev.acpi_wmi.0.bmof | bmf2mof provides some interesting information. 
>> All other 3 instances do not though.  In any case, it seems to work now.
>>
>>> Verbosity can be reduced with attached patch if current level is too
>>> high for you.
>>
>> Works for me both ways, I simply had the wrong impression that if we
>> don't have EC, we can't attach at all.
> 
> Could you commit this, or is it incomplete fix?

I did some tests with ACER ACPI extras which left functional after
OPregion handler had been disabled, so I think, fix is complete.

I have created a phabricator review: https://reviews.freebsd.org/D27653