All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH V2] ACPI / LPSS: Add device link for CHT SD card dependency on I2C
@ 2017-12-07  9:03 Adrian Hunter
  2017-12-14 14:16 ` Andy Shevchenko
  0 siblings, 1 reply; 6+ messages in thread
From: Adrian Hunter @ 2017-12-07  9:03 UTC (permalink / raw)
  To: Rafael J. Wysocki, Mika Westerberg
  Cc: Andy Shevchenko, linux-acpi, linux-kernel, Carlo Caione, Hans de Goede

Some Cherry Trail boards have a dependency between the SDHCI host
controller used for SD cards and an external PMIC accessed via I2C. Add a
device link between the SDHCI host controller (consumer) and the I2C
adapter (supplier).

This patch depends on a fix to devices links, namely commit 0ff26c662d5f
("driver core: Fix device link deferred probe"). And also either,
commit 126dbc6b49c8 ("PM: i2c-designware-platdrv: Clean up PM handling in
probe"), or patch "PM / runtime: Fix handling of suppliers with disabled
runtime PM".

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---


Changes in V2:

	Add a comment about why it is necessary to hardcode the links
	information in the code


 drivers/acpi/acpi_lpss.c | 146 +++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 146 insertions(+)

diff --git a/drivers/acpi/acpi_lpss.c b/drivers/acpi/acpi_lpss.c
index 7f2b02cc8ea1..9cfe6b71078b 100644
--- a/drivers/acpi/acpi_lpss.c
+++ b/drivers/acpi/acpi_lpss.c
@@ -290,6 +290,11 @@ static void bsw_pwm_setup(struct lpss_private_data *pdata)
 	{}
 };
 
+static const struct x86_cpu_id cht_cpu[] = {
+	ICPU(INTEL_FAM6_ATOM_AIRMONT),	/* Braswell, Cherry Trail */
+	{}
+};
+
 #else
 
 #define LPSS_ADDR(desc) (0UL)
@@ -427,6 +432,146 @@ static int register_device_clock(struct acpi_device *adev,
 	return 0;
 }
 
+struct lpss_device_links {
+	const char *supplier_hid;
+	const char *supplier_uid;
+	const char *consumer_hid;
+	const char *consumer_uid;
+	const struct x86_cpu_id *cpus;
+	u32 flags;
+};
+
+/*
+ * The _DEP method is used to identify dependencies but instead of creating
+ * device links for every handle in _DEP, only links in the following list are
+ * created. That is necessary because, in the general case, _DEP can refer to
+ * devices that might not have drivers, or that are on different buses, or where
+ * the supplier is not enumerated until after the consumer is probed.
+ */
+static const struct lpss_device_links lpss_device_links[] = {
+	{"808622C1", "7", "80860F14", "3", cht_cpu, DL_FLAG_PM_RUNTIME},
+};
+
+static bool hid_uid_match(const char *hid1, const char *uid1,
+			  const char *hid2, const char *uid2)
+{
+	return !strcmp(hid1, hid2) && uid1 && uid2 && !strcmp(uid1, uid2);
+}
+
+static bool acpi_lpss_is_supplier(struct acpi_device *adev,
+				  const struct lpss_device_links *link)
+{
+	return hid_uid_match(acpi_device_hid(adev), acpi_device_uid(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;
+};
+
+static int match_hid_uid(struct device *dev, void *data)
+{
+	struct acpi_device *adev = ACPI_COMPANION(dev);
+	struct hid_uid *id = data;
+
+	if (!adev)
+		return 0;
+
+	return hid_uid_match(acpi_device_hid(adev), acpi_device_uid(adev),
+			     id->hid, id->uid);
+}
+
+static struct device *acpi_lpss_find_device(const char *hid, const char *uid)
+{
+	struct hid_uid data = {
+		.hid = hid,
+		.uid = uid,
+	};
+
+	return bus_find_device(&platform_bus_type, NULL, &data, match_hid_uid);
+}
+
+static bool acpi_lpss_dep(struct acpi_device *adev, acpi_handle handle)
+{
+	struct acpi_handle_list dep_devices;
+	acpi_status status;
+	int i;
+
+	if (!acpi_has_method(adev->handle, "_DEP"))
+		return false;
+
+	status = acpi_evaluate_reference(adev->handle, "_DEP", NULL,
+					 &dep_devices);
+	if (ACPI_FAILURE(status)) {
+		dev_dbg(&adev->dev, "Failed to evaluate _DEP.\n");
+		return false;
+	}
+
+	for (i = 0; i < dep_devices.count; i++) {
+		if (dep_devices.handles[i] == handle)
+			return true;
+	}
+
+	return false;
+}
+
+static void acpi_lpss_link_consumer(struct device *dev1,
+				    const struct lpss_device_links *link)
+{
+	struct device *dev2;
+
+	dev2 = acpi_lpss_find_device(link->consumer_hid, link->consumer_uid);
+	if (!dev2)
+		return;
+
+	if (acpi_lpss_dep(ACPI_COMPANION(dev2), ACPI_HANDLE(dev1)))
+		device_link_add(dev2, dev1, link->flags);
+
+	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)
+{
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(lpss_device_links); i++) {
+		const struct lpss_device_links *link = &lpss_device_links[i];
+
+		if (link->cpus && !x86_match_cpu(link->cpus))
+			continue;
+
+		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);
+	}
+}
+
 static int acpi_lpss_create_device(struct acpi_device *adev,
 				   const struct acpi_device_id *id)
 {
@@ -500,6 +645,7 @@ static int acpi_lpss_create_device(struct acpi_device *adev,
 	adev->driver_data = pdata;
 	pdev = acpi_create_platform_device(adev, dev_desc->properties);
 	if (!IS_ERR_OR_NULL(pdev)) {
+		acpi_lpss_create_device_links(adev, pdev);
 		return 1;
 	}
 
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH V2] ACPI / LPSS: Add device link for CHT SD card dependency on I2C
  2017-12-07  9:03 [PATCH V2] ACPI / LPSS: Add device link for CHT SD card dependency on I2C Adrian Hunter
@ 2017-12-14 14:16 ` Andy Shevchenko
  2017-12-15  0:58   ` Rafael J. Wysocki
  2017-12-15  9:10   ` Adrian Hunter
  0 siblings, 2 replies; 6+ messages in thread
From: Andy Shevchenko @ 2017-12-14 14:16 UTC (permalink / raw)
  To: Adrian Hunter, Rafael J. Wysocki, Mika Westerberg
  Cc: linux-acpi, linux-kernel, Carlo Caione, Hans de Goede

On Thu, 2017-12-07 at 11:03 +0200, Adrian Hunter wrote:
> Some Cherry Trail boards have a dependency between the SDHCI host
> controller used for SD cards and an external PMIC accessed via I2C.
> Add a
> device link between the SDHCI host controller (consumer) and the I2C
> adapter (supplier).
> 
> This patch depends on a fix to devices links, namely commit
> 0ff26c662d5f
> ("driver core: Fix device link deferred probe"). And also either,
> commit 126dbc6b49c8 ("PM: i2c-designware-platdrv: Clean up PM handling
> in
> probe"), or patch "PM / runtime: Fix handling of suppliers with
> disabled
> runtime PM".
> 

Fine with me, though I think below comment worth to address.

>  
> +static const struct x86_cpu_id cht_cpu[] = {
> +	ICPU(INTEL_FAM6_ATOM_AIRMONT),	/* Braswell, Cherry
> Trail */
> +	{}
> +};

I would rather to modify ICPU() macro to accept driver data where we
just pass an unsigned long value to be assigned as lpss_quirks and
introduce another quirk.

> +
> +		if (link->cpus && !x86_match_cpu(link->cpus))
> +			continue;

...thus, 

if (!(lpss_quirks & LPSS_QUIRK_NEED_DEVICE_LINKS))
 continue;


-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH V2] ACPI / LPSS: Add device link for CHT SD card dependency on I2C
  2017-12-14 14:16 ` Andy Shevchenko
@ 2017-12-15  0:58   ` Rafael J. Wysocki
  2017-12-15 10:03     ` Andy Shevchenko
  2017-12-15  9:10   ` Adrian Hunter
  1 sibling, 1 reply; 6+ messages in thread
From: Rafael J. Wysocki @ 2017-12-15  0:58 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Adrian Hunter, Rafael J. Wysocki, Mika Westerberg,
	ACPI Devel Maling List, Linux Kernel Mailing List, Carlo Caione,
	Hans de Goede

On Thu, Dec 14, 2017 at 3:16 PM, Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
> On Thu, 2017-12-07 at 11:03 +0200, Adrian Hunter wrote:
>> Some Cherry Trail boards have a dependency between the SDHCI host
>> controller used for SD cards and an external PMIC accessed via I2C.
>> Add a
>> device link between the SDHCI host controller (consumer) and the I2C
>> adapter (supplier).
>>
>> This patch depends on a fix to devices links, namely commit
>> 0ff26c662d5f
>> ("driver core: Fix device link deferred probe"). And also either,
>> commit 126dbc6b49c8 ("PM: i2c-designware-platdrv: Clean up PM handling
>> in
>> probe"), or patch "PM / runtime: Fix handling of suppliers with
>> disabled
>> runtime PM".
>>
>
> Fine with me, though I think below comment worth to address.
>
>>
>> +static const struct x86_cpu_id cht_cpu[] = {
>> +     ICPU(INTEL_FAM6_ATOM_AIRMONT),  /* Braswell, Cherry
>> Trail */
>> +     {}
>> +};
>
> I would rather to modify ICPU() macro to accept driver data where we
> just pass an unsigned long value to be assigned as lpss_quirks and
> introduce another quirk.

Not really.

There are many instances of ICPU() already in the tree and updating
all of them is just not worth it.

If you can make the code cleaner without modifying that macro, go for it.

Thanks,
Rafael

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH V2] ACPI / LPSS: Add device link for CHT SD card dependency on I2C
  2017-12-14 14:16 ` Andy Shevchenko
  2017-12-15  0:58   ` Rafael J. Wysocki
@ 2017-12-15  9:10   ` Adrian Hunter
  2017-12-15 12:00     ` Adrian Hunter
  1 sibling, 1 reply; 6+ messages in thread
From: Adrian Hunter @ 2017-12-15  9:10 UTC (permalink / raw)
  To: Andy Shevchenko, Rafael J. Wysocki, Mika Westerberg
  Cc: linux-acpi, linux-kernel, Carlo Caione, Hans de Goede

On 14/12/17 16:16, Andy Shevchenko wrote:
> On Thu, 2017-12-07 at 11:03 +0200, Adrian Hunter wrote:
>> Some Cherry Trail boards have a dependency between the SDHCI host
>> controller used for SD cards and an external PMIC accessed via I2C.
>> Add a
>> device link between the SDHCI host controller (consumer) and the I2C
>> adapter (supplier).
>>
>> This patch depends on a fix to devices links, namely commit
>> 0ff26c662d5f
>> ("driver core: Fix device link deferred probe"). And also either,
>> commit 126dbc6b49c8 ("PM: i2c-designware-platdrv: Clean up PM handling
>> in
>> probe"), or patch "PM / runtime: Fix handling of suppliers with
>> disabled
>> runtime PM".
>>
> 
> Fine with me, though I think below comment worth to address.
> 
>>  
>> +static const struct x86_cpu_id cht_cpu[] = {
>> +	ICPU(INTEL_FAM6_ATOM_AIRMONT),	/* Braswell, Cherry
>> Trail */
>> +	{}
>> +};
> 
> I would rather to modify ICPU() macro to accept driver data where we
> just pass an unsigned long value to be assigned as lpss_quirks and
> introduce another quirk.
> 
>> +
>> +		if (link->cpus && !x86_match_cpu(link->cpus))
>> +			continue;
> 
> ...thus, 
> 
> if (!(lpss_quirks & LPSS_QUIRK_NEED_DEVICE_LINKS))
>  continue;

The intention is to associate the cpu with the link information i.e. that
link is needed on that cpu.  What you are proposing is slightly different.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH V2] ACPI / LPSS: Add device link for CHT SD card dependency on I2C
  2017-12-15  0:58   ` Rafael J. Wysocki
@ 2017-12-15 10:03     ` Andy Shevchenko
  0 siblings, 0 replies; 6+ messages in thread
From: Andy Shevchenko @ 2017-12-15 10:03 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Adrian Hunter, Rafael J. Wysocki, Mika Westerberg,
	ACPI Devel Maling List, Linux Kernel Mailing List, Carlo Caione,
	Hans de Goede

On Fri, 2017-12-15 at 01:58 +0100, Rafael J. Wysocki wrote:
> On Thu, Dec 14, 2017 at 3:16 PM, Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:
> > On Thu, 2017-12-07 at 11:03 +0200, Adrian Hunter wrote:
> > > 

> > > 
> > > +static const struct x86_cpu_id cht_cpu[] = {
> > > +     ICPU(INTEL_FAM6_ATOM_AIRMONT),  /* Braswell, Cherry
> > > Trail */
> > > +     {}
> > > +};
> > 
> > I would rather to modify ICPU() macro to accept driver data where we
> > just pass an unsigned long value to be assigned as lpss_quirks and
> > introduce another quirk.
> 
> Not really.
> 
> There are many instances of ICPU() already in the tree and updating
> all of them is just not worth it.

These macros all over the code are local.
I'm talking about one which is defined inside acpi_lpss.c.

-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH V2] ACPI / LPSS: Add device link for CHT SD card dependency on I2C
  2017-12-15  9:10   ` Adrian Hunter
@ 2017-12-15 12:00     ` Adrian Hunter
  0 siblings, 0 replies; 6+ messages in thread
From: Adrian Hunter @ 2017-12-15 12:00 UTC (permalink / raw)
  To: Andy Shevchenko, Rafael J. Wysocki, Mika Westerberg
  Cc: linux-acpi, linux-kernel, Carlo Caione, Hans de Goede

On 15/12/17 11:10, Adrian Hunter wrote:
> On 14/12/17 16:16, Andy Shevchenko wrote:
>> On Thu, 2017-12-07 at 11:03 +0200, Adrian Hunter wrote:
>>> Some Cherry Trail boards have a dependency between the SDHCI host
>>> controller used for SD cards and an external PMIC accessed via I2C.
>>> Add a
>>> device link between the SDHCI host controller (consumer) and the I2C
>>> adapter (supplier).
>>>
>>> This patch depends on a fix to devices links, namely commit
>>> 0ff26c662d5f
>>> ("driver core: Fix device link deferred probe"). And also either,
>>> commit 126dbc6b49c8 ("PM: i2c-designware-platdrv: Clean up PM handling
>>> in
>>> probe"), or patch "PM / runtime: Fix handling of suppliers with
>>> disabled
>>> runtime PM".
>>>
>>
>> Fine with me, though I think below comment worth to address.
>>
>>>  
>>> +static const struct x86_cpu_id cht_cpu[] = {
>>> +	ICPU(INTEL_FAM6_ATOM_AIRMONT),	/* Braswell, Cherry
>>> Trail */
>>> +	{}
>>> +};
>>
>> I would rather to modify ICPU() macro to accept driver data where we
>> just pass an unsigned long value to be assigned as lpss_quirks and
>> introduce another quirk.
>>
>>> +
>>> +		if (link->cpus && !x86_match_cpu(link->cpus))
>>> +			continue;
>>
>> ...thus, 
>>
>> if (!(lpss_quirks & LPSS_QUIRK_NEED_DEVICE_LINKS))
>>  continue;
> 
> The intention is to associate the cpu with the link information i.e. that
> link is needed on that cpu.  What you are proposing is slightly different.
> 

Spoke with Andy and decided the cpu check could be removed altogether for
now, so I will send a V3 shortly.

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2017-12-15 12:00 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-12-07  9:03 [PATCH V2] ACPI / LPSS: Add device link for CHT SD card dependency on I2C Adrian Hunter
2017-12-14 14:16 ` Andy Shevchenko
2017-12-15  0:58   ` Rafael J. Wysocki
2017-12-15 10:03     ` Andy Shevchenko
2017-12-15  9:10   ` Adrian Hunter
2017-12-15 12:00     ` Adrian Hunter

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.