Re: acpi_wmi noisy without EC

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?