acpi: parse resources of not-present devices that are kept enabled

[guest-seuros]

Summary

acpi_probe_child() keeps PCI link devices, the RTC,
and docking stations enabled even when _STA reports them not present,
but skipped acpi_parse_resources() for them. With an empty resource
list, resource-based hint matching (BUS_HINT_DEVICE_UNIT) cannot wire
such a device to its hinted unit, and the hinted ISA device is then
created as a duplicate.

Modern AMI firmware reports the PNP0B00 RTC as not present while
handing timekeeping to the ACPI Time-and-Alarm device.

[ngie]

sys/dev/acpica/acpi.c

2584–2588

Could you please add references to the ACPI handles in the comment above? Thanks!

[guest-seuros]

sys/dev/acpica/acpi.c

2584–2588

Do you mean this page ?
https://uefi.org/specs/ACPI/6.5_A/05_ACPI_Software_Programming_Model.html

I copied it from the pdf

[ngie]

sys/dev/acpica/acpi.c

2587–2605

Maybe something like this? The PNP0C0F description seems a bit wordy in my suggestion TBH, but maybe something like that to communicate that the state of things with absolute clarity?
Alternatively, if you just create a map of the devices--either using constants or using the handles mentioned in the comment--then link to the actual spec, that would help the reader to understand what the code does (and maybe add more driver quirks as needed in the future).

[ngie]

Ugh. Didn't mean to "accept" the revision yet - I'm sorry :(.

[jhb]

This is probably ok. I found the original version a bit easier to read and extend, and maybe the way to handle that is to add a new helper function like so:

static bool
acpi_always_present(ACPI_HANDLE handle)
{
    /* Comment for PCI link devices */
    if (acpi_MatchHid(handle, "PNPOCOF")
      return (true);

   ...
  return (false);
}

Then in acpi_probe_child you end up with:

if (type == ACPI_TYPE_DEVICE && !acpi_DeviceIsPresent(child) &&
    !acpi_always_present(handle)) {
    device_disable(child);
    break;
}

sys/dev/acpica/acpi.c

2587–2605

Eh, I think the current comment is fine. The list in the comment is in the same order as the list of checks so it is already clear enough how they match up. Also, the added bit about PCI link devices "BIOS may not allow them to be used" I think isn't correct.

[guest-seuros]

address comments

[jhb]

sys/dev/acpica/acpi.c

2493

Cosmetic (and fine to just fix while pushing), but I think it's more extendable in the future if each of these conditions has a comment as it had in the old code) as then when you add a new exception here in the future, you only have to touch one place. It also addresses ngie@'s question about ensuring the comments line up with the relevant code. So something like:

/*
  * PCI link devices sometimes report "present" but not "functional"
  * (i.e. if disabled).  Always probe them since we may enable them
  * later.
  */
if (acpi_MatchHid(...))

(Then you also don't need to duplicate the HID strings in the comment since the comment is right next to the code.)