Re: [PATCH 1/7] ACPI / LPSS: Only add device links from consumer to supplier

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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);
>  	}
>  }
>  
> 




[Index of Archives]     [Linux IBM ACPI]     [Linux Power Management]     [Linux Kernel]     [Linux Laptop]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Device Mapper]     [Linux Resources]

  Powered by Linux