On 10/12/2020 16:59, Rafael J. Wysocki wrote:
> On Thursday, December 10, 2020 5:07:56 PM CET Daniel Scally wrote:
>> On 10/12/2020 16:05, Rafael J. Wysocki wrote:
>>> All the adevs with matching _ADR also have both _STA and _HID
>>> unfortunately. Sorry; let me stop half-arsing this and show you
>>> something useful:
>>>
>>> [ 0.219953] acpi_find_child_device(PNP0A08:00, 0x00, false)
>>> [ 0.220818] INT3472:00: _STA 0x00, _ADR=0x00000000, _HID=INT3472
>>> [ 0.220821] INT3472:01: _STA 0x00, _ADR=0x00000000, _HID=INT3472
>>> [ 0.220870] INT3472:02: _STA 0x00, _ADR=0x00000000, _HID=INT3472
>>> [ 0.220892] INT3472:03: _STA 0x00, _ADR=0x00000000, _HID=INT3472
>>> [ 0.220916] INT3472:04: _STA 0x00, _ADR=0x00000000, _HID=INT3472
>>> [ 0.220941] INT3472:05: _STA 0x00, _ADR=0x00000000, _HID=INT3472
>>> [ 0.220965] INT3472:06: _STA 0x00, _ADR=0x00000000, _HID=INT3472
>>> [ 0.220990] INT3472:07: _STA 0x00, _ADR=0x00000000, _HID=INT3472
>>> These will be ignored with -ENODEV.
>>>
>>>> [ 0.221038] INT3472:08: _STA 0x0f, _ADR=0x00000000, _HID=INT3472
>>> For this acpi_find_child_device() will return FIND_CHILD_MIN_SCORE if
>>> I'm not mistaken.
>> It does - this is the one that binds, being the first.
>>>> [ 0.221051] OVTI5648:00: _STA 0x0f, _ADR=0x00000000, _HID=OVTI5648
>>>> [ 0.221061] INT3472:09: _STA 0x0f, _ADR=0x00000000, _HID=INT3472
>>>> [ 0.221070] OVTI2680:00: _STA 0x0f, _ADR=0x00000000, _HID=OVTI2680
>>> As well as for the three above.
>>>
>>>> [ 0.221079] INT3471:00: _STA 0x00, _ADR=0x00000000, _HID=INT3471
>>>> [ 0.221105] INT33BE:00: _STA 0x00, _ADR=0x00000000, _HID=INT33BE
>>>> [ 0.221130] INT3471:01: _STA 0x00, _ADR=0x00000000, _HID=INT3471
>>>> [ 0.221156] INT33BE:01: _STA 0x00, _ADR=0x00000000, _HID=INT33BE
>>> And the rest will be ignored.
>>>
>>>> That's the debug output I included for each adev that's assessed as a
>>>> child of PNP0A08:00. _STA, _ADR and _HID present for all, _ADR 0x00 for
>>>> all, _STA 0x0f for the 2 sensors and their PMIC's and 0x00 for the rest.
>>>> The same situation holds on both of my devices.
>>> So in fact we don't want to have an ACPI companion for (PNP0A08:00,
>>> 0x00, false).
>> Yeah, I think that's right
>>> This is a hostbridge special case and let me think about this for a while.
>> Sure - thanks very much for your help.
> I've come up with the following patch.
>
> Please let me know if it works for you.
>
> ---
> Index: linux-pm/drivers/pci/pci-acpi.c
> ===================================================================
> --- linux-pm.orig/drivers/pci/pci-acpi.c
> +++ linux-pm/drivers/pci/pci-acpi.c
> @@ -1162,14 +1162,32 @@ void acpi_pci_remove_bus(struct pci_bus
> static struct acpi_device *acpi_pci_find_companion(struct device *dev)
> {
> struct pci_dev *pci_dev = to_pci_dev(dev);
> + struct acpi_device *adev;
> bool check_children;
> u64 addr;
>
> check_children = pci_is_bridge(pci_dev);
> /* Please ref to ACPI spec for the syntax of _ADR */
> addr = (PCI_SLOT(pci_dev->devfn) << 16) | PCI_FUNC(pci_dev->devfn);
> - return acpi_find_child_device(ACPI_COMPANION(dev->parent), addr,
> + adev = acpi_find_child_device(ACPI_COMPANION(dev->parent), addr,
> check_children);
> + /*
> + * There may be ACPI device objects in the ACPI namesoace that are
> + * children of the device object representing the host bridge, but don't
> + * represent PCI devices. Both _HID and _ADR may be present for them,
> + * even though that is against the specification (for example, see
> + * Section 6.1 of ACPI 6.3), but in many cases the _ADR returns 0 which
> + * appears to indicate that they should not be taken into consideration
> + * as potential companions of PCI devices on the root bus.
> + *
> + * To catch this special case, disregard the returned device object if
> + * it has a valid _HID, addr is 0 and the PCI device at hand is on the
> + * root bus.
> + */
> + if (adev->pnp.type.platform_id && !addr && !pci_dev->bus->parent)
> + return NULL;
> +
> + return adev;
> }
>
> /**
>
Thanks - this needs to check adev for NULL too; acpi_find_child_device()
does return that sometimes. When changed to:
+ if (adev && adev->pnp.type.platform_id && !addr && !pci_dev->bus->parent)
+ return NULL;
Then it boots properly, and fixes the original problem. Also;
s/namesoace/namespace in the comment.
Really appreciate the help - thank you!
