Complicated problem with ACPI probe / enumeration ordering

[Hans de Goede <hdegoede@xxxxxxxxxx>]

Hi All,

While working on this bug:
https://bugzilla.redhat.com/show_bug.cgi?id=1665610

I encountered a DSDT which contains the bits:

Starting at line 6271:

Device (SDHB) {
    ...
    Name (_DDN, "Intel(R) SDIO Controller - 80862295")
    ...
    Device (RTLW) {
                Method (_STA, 0, NotSerialized)  // _STA: Status
                {
                    Local0 = Zero
                    If ((^^^^GPO1.AMMR == One))
                    {
                        If ((^^^^GPO1.WID0 == One))
                        {
                            Local0 = One
                        }

                        If ((^^^^GPO1.WID1 == One))
                        {
                            Local0 += 0x02
                        }
                    }

                    If ((Local0 == Zero))
                    {
                        Return (Zero)
                    }
                    ElseIf ((Local0 == One))
                    {
                        Return (0x0F)
                    }
                    ElseIf ((Local0 == 0x02))
                    {
                        Return (Zero)
                    }
                    Else
                    {
                        Return (Zero)
                    }
                }
     }
     ...
}

There are other subdevices for if the device has a Broadcom wifi module
instead of a Realtek one, which return 0xf from _STA when
Local0 == Zero.

And then there is this bit of AML for the Bluetooth part of the
broadcom/wifi module:

Starting at line 9779:

        Scope (URT1)
        {
            Device (BTH3)
            {
                Method (_HID, 0, NotSerialized)  // _HID: Hardware ID
                {
                    Local0 = Zero
                    If ((^^^^GPO1.AMMR == One))
                    {
                        If ((^^^^GPO1.WID0 == One))
                        {
                            Local0 = One
                        }
                        If ((^^^^GPO1.WID1 == One))
                        {
                            Local0 += 0x02
                        }
                    }

                    If ((Local0 == Zero))
                    {
                        Return ("BCM2E5B")
                    }
                    ElseIf ((Local0 == One))
                    {
                        Return ("OBDA0623")
                    }
                    ElseIf ((Local0 == 0x02))
                    {
                        Return ("BCM2E74")
                    }
                    Else
                    {
                        Return ("BCM2E5B")
                    }
                }
            }
        }

What is happening here is that two GPIOs are connected to
(solder?) jumpers which indicate which type of wifi module
is installed. And instead of the UEFI firmware reading this
at boot and then setting an ACPI variable accordingly as is
typically done for options configurable through the setup
menu, the reading of the GPIOs is done from the AML code.

The problem here is that we evaluate the _STA and _HID
methods while enumerating the ACPI devices, which we do in
the order they are listed in the AML code.

The GPIO devices start at line 14974, so these are not enumerated
yet when we execute the _STA and _HID methods listed above, so
they alwys take the Local0 == Zero option, while on the device from:
https://bugzilla.redhat.com/show_bug.cgi?id=1665610
The Local0 == One option is the right one.

I've provided the user with a DSDT override which hardcodes the
Local0 == One results for these _STA and _HID methods and that
fixes wifi and bluetooth not working on this device.

###

So the above hopefully explains the problem. Now the question is
should we fix this and if so, how?  I believe that we are likely
to see this some construct elsewhere although it is certainly not
common I don't think this one device will be unique case so I do
believe that we should fix this.

As for how, thinking a bit about this I see 2 approaches:

1) Doing the scanning in 2 phases, we could use the
level parameter to acpi_bus_check_add to make it skip
non top-level ACPI nodes on a first run and then do a second
run adding the child nodes, which at least in this case
are the ones having this problem.

2) Have a special acpi_init_gpio() function which
calls acpi_walk_namespace(ACPI_ROOT_OBJECT, ...) using
a special helper which checks for known GPIO acpi_ids and
then calls acpi_add_single_object for those. And then call
acpi_init_gpio() from acpi_scan_init() before calling
acpi_bus_scan(), thus registering the GPIOs early.

Solution 1 is more generic, but solution 2 has the biug
advantage that it is more limited in scope and thus has
a lot smaller chance of causing unwanted side-effects.

I myself am leaning towards solution 2, because 1 would
change the entire enumeration order everywhere and even
though that should not matter I'm afraid that it will
matter (as this bug shows) and that doing so will cause
issues somewhere.

So before I spend time on implementing solution 2 I would
like to hear from you if that sounds acceptable (on first
reading, the proof is in the actual pudding of course)
and/or if you have different/better ideas how to deal
with this?

Regards,

Hans