Hi,
On 09/21/2018 03:01 PM, Adrian Hunter wrote:
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.
Ah I see an upon rereading the code you are right.
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.
You are right again.
So I guess that this patch can be dropped.
Note that for the new GPU -> PMIC I2C controller consumer -> supplier link this
patchset adds we rely on probe ordering. Relying on probe ordering works
for the SDCARD -> PMIC I2C too, so we could keep this patch as it does
simplify the code. If we do that the commit message needs to be corrected
of course.
Regards,
Hans
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);
}
}
