On 19/09/18 22:15, Hans de Goede wrote:
> Commit e6ce0ce34f65 ("ACPI / LPSS: Add device link for CHT SD card
> dependency on I2C") tries to add device links from consumer to
> supplier *and* the other way around.
That is not the intention. The intention is to add a link irrespective of
which device (consumer or supplier) is created last.
>
> When the consumer gets registered the supplier is not yet registered, so
> acpi_lpss_find_device() cannot find it and we never end up adding the
> supplier link. Although not intended this is a good thing, because the
> device links are dependencies and if we add a link in both directions the
> power-management core will not know which one to suspend/resume first.
Did that actually happen? It seems to me that could only happen if the _DEP
methods also pointed to each other.
>
> I've verified through debug printk-s that indeed only the consumer
> to supplier dependency gets added even before this commit removes
> the code to add the link the other way.
>
> Signed-off-by: Hans de Goede <hdegoede@xxxxxxxxxx>
> ---
> drivers/acpi/acpi_lpss.c | 25 -------------------------
> 1 file changed, 25 deletions(-)
>
> diff --git a/drivers/acpi/acpi_lpss.c b/drivers/acpi/acpi_lpss.c
> index 83875305b1d4..40a8cab81cbd 100644
> --- a/drivers/acpi/acpi_lpss.c
> +++ b/drivers/acpi/acpi_lpss.c
> @@ -486,13 +486,6 @@ static bool acpi_lpss_is_supplier(struct acpi_device *adev,
> link->supplier_hid, link->supplier_uid);
> }
>
> -static bool acpi_lpss_is_consumer(struct acpi_device *adev,
> - const struct lpss_device_links *link)
> -{
> - return hid_uid_match(acpi_device_hid(adev), acpi_device_uid(adev),
> - link->consumer_hid, link->consumer_uid);
> -}
> -
> struct hid_uid {
> const char *hid;
> const char *uid;
> @@ -559,21 +552,6 @@ static void acpi_lpss_link_consumer(struct device *dev1,
> put_device(dev2);
> }
>
> -static void acpi_lpss_link_supplier(struct device *dev1,
> - const struct lpss_device_links *link)
> -{
> - struct device *dev2;
> -
> - dev2 = acpi_lpss_find_device(link->supplier_hid, link->supplier_uid);
> - if (!dev2)
> - return;
> -
> - if (acpi_lpss_dep(ACPI_COMPANION(dev1), ACPI_HANDLE(dev2)))
> - device_link_add(dev1, dev2, link->flags);
> -
> - put_device(dev2);
> -}
> -
> static void acpi_lpss_create_device_links(struct acpi_device *adev,
> struct platform_device *pdev)
> {
> @@ -584,9 +562,6 @@ static void acpi_lpss_create_device_links(struct acpi_device *adev,
>
> if (acpi_lpss_is_supplier(adev, link))
> acpi_lpss_link_consumer(&pdev->dev, link);
> -
> - if (acpi_lpss_is_consumer(adev, link))
> - acpi_lpss_link_supplier(&pdev->dev, link);
> }
> }
>
>
