Use acpi_dev_present()

PATCH In review Lukas Wunner 5 participants 10 messages Jan 17, 2016 – Mar 13, 2016

Subsystems (8)

Sound ACPI ASoC x86 platform drivers +4 more subsystems

lore

Share & tools

lore

Lukas Wunner

author

2016-01-17 20:49 UTC #0 lore raw

cover Use acpi_dev_present()

Hi Darren,

the acpi_dev_present() API has now landed in Linus' tree. Thus, after Linus' tree gets merged back into yours, it would be possible to use the API in the pdx86 drivers as per the following patches.

I've also pushed these to GitHub in case anyone prefers perusing them in a browser:
https://github.com/l1k/linux/commits/acpi_dev_present_pdx86

This is a repost of patches submitted in November, the only change is one line added to the commit messages to reference the commit which introduces the API:
http://thread.gmane.org/gmane.linux.alsa.devel/147414/focus=8004
http://thread.gmane.org/gmane.linux.alsa.devel/147414/focus=8005

Thanks,

Lukas

Lukas Wunner (2):
eeepc-wmi: Use acpi_dev_present()
acer-wmi: Use acpi_dev_present()

 drivers/platform/x86/acer-wmi.c  | 16 ++++------------
 drivers/platform/x86/eeepc-wmi.c | 24 ++----------------------
 2 files changed, 6 insertions(+), 34 deletions(-)

Signature · 1 line

1.8.5.2 (Apple Git-48)

Lukas Wunner

author

2016-01-17 20:50 UTC #1 lore raw

PATCH 1/2 eeepc-wmi: Use acpi_dev_present()

Use shiny new acpi_dev_present() and remove all the boilerplate to search for a particular ACPI device. No functional change. Cf. 2d12b6b381ba ("ACPI / utils: Add acpi_dev_present()").

Cc Corentin Chary <[email protected]>

Signed-off-by Lukas Wunner <[email protected]>

Acked-by Darren Hart <[email protected]>

---
 drivers/platform/x86/eeepc-wmi.c | 24 ++----------------------
 1 file changed, 2 insertions(+), 22 deletions(-)

drivers/platform/x86/eeepc-wmi.c +2 ~~-22~~

index 14fd2ec..e1490db 100644
@@ -204,30 +204,10 @@ static void eeepc_wmi_key_filter(struct asus_wmi_driver *asus_wmi, int *code,
 	}
 }
 
-static acpi_status eeepc_wmi_parse_device(acpi_handle handle, u32 level,
-						 void *context, void **retval)
-{
-	pr_warn("Found legacy ATKD device (%s)\n", EEEPC_ACPI_HID);
-	*(bool *)context = true;
-	return AE_CTRL_TERMINATE;
-}
-
-static int eeepc_wmi_check_atkd(void)
-{
-	acpi_status status;
-	bool found = false;
-
-	status = acpi_get_devices(EEEPC_ACPI_HID, eeepc_wmi_parse_device,
-				  &found, NULL);
-
-	if (ACPI_FAILURE(status) || !found)
-		return 0;
-	return -1;
-}
-
 static int eeepc_wmi_probe(struct platform_device *pdev)
 {
-	if (eeepc_wmi_check_atkd()) {
+	if (acpi_dev_present(EEEPC_ACPI_HID)) {
+		pr_warn("Found legacy ATKD device (%s)\n", EEEPC_ACPI_HID);
 		pr_warn("WMI device present, but legacy ATKD device is also "
 			"present and enabled\n");
 		pr_warn("You probably booted with acpi_osi=\"Linux\" or "

Signature · 1 line

1.8.5.2 (Apple Git-48)

Lukas Wunner

author

2016-01-17 20:50 UTC #2 lore raw

PATCH 2/2 acer-wmi: Use acpi_dev_present()

In reply to Lukas Wunner

Use shiny new acpi_dev_present() and remove all the boilerplate to search for a particular ACPI device. No functional change. Cf. 2d12b6b381ba ("ACPI / utils: Add acpi_dev_present()").

Cc Lee, Chun-Yi <[email protected]>

Signed-off-by Lukas Wunner <[email protected]>

Acked-by Darren Hart <[email protected]>

---
 drivers/platform/x86/acer-wmi.c | 16 ++++------------
 1 file changed, 4 insertions(+), 12 deletions(-)

drivers/platform/x86/acer-wmi.c +4 ~~-12~~

index 1062fa4..3deadd5 100644
@@ -793,15 +793,6 @@ static acpi_status __init AMW0_find_mailled(void)
 	return AE_OK;
 }
 
-static int AMW0_set_cap_acpi_check_device_found __initdata;
-
-static acpi_status __init AMW0_set_cap_acpi_check_device_cb(acpi_handle handle,
-	u32 level, void *context, void **retval)
-{
-	AMW0_set_cap_acpi_check_device_found = 1;
-	return AE_OK;
-}
-
 static const struct acpi_device_id norfkill_ids[] __initconst = {
 	{ "VPC2004", 0},
 	{ "IBM0068", 0},

@@ -816,9 +807,10 @@ static int __init AMW0_set_cap_acpi_check_device(void)
 	const struct acpi_device_id *id;
 
 	for (id = norfkill_ids; id->id[0]; id++)
-		acpi_get_devices(id->id, AMW0_set_cap_acpi_check_device_cb,
-				NULL, NULL);
-	return AMW0_set_cap_acpi_check_device_found;
+		if (acpi_dev_present(id->id))
+			return true;
+
+	return false;
 }
 
 static acpi_status __init AMW0_set_capabilities(void)

Signature · 1 line

1.8.5.2 (Apple Git-48)

joeyli

maintainer Reviewed-by on v1 2/2

2016-01-18 03:48 UTC #3 lore raw

Re: [PATCH 2/2] acer-wmi: Use acpi_dev_present()

Hi Lukas,

Quoted 7 lines · Lukas Wunner

Use shiny new acpi_dev_present() and remove all the boilerplate to search for a particular ACPI device. No functional change. Cf. 2d12b6b381ba ("ACPI / utils: Add acpi_dev_present()").
Cc Lee, Chun-Yi <[email protected]>
Signed-off-by Lukas Wunner <[email protected]>
Acked-by Darren Hart <[email protected]>

Reviewed-by Lee, Chun-Yi <[email protected]>

Thanks a lot!
Joey Lee

Quoted 41 lines

---
 drivers/platform/x86/acer-wmi.c | 16 ++++------------
 1 file changed, 4 insertions(+), 12 deletions(-)

drivers/platform/x86/acer-wmi.c +4 ~~-12~~

index 1062fa4..3deadd5 100644
@@ -793,15 +793,6 @@ static acpi_status __init AMW0_find_mailled(void)
 	return AE_OK;
 }
 
-static int AMW0_set_cap_acpi_check_device_found __initdata;
-
-static acpi_status __init AMW0_set_cap_acpi_check_device_cb(acpi_handle handle,
-	u32 level, void *context, void **retval)
-{
-	AMW0_set_cap_acpi_check_device_found = 1;
-	return AE_OK;
-}
-
 static const struct acpi_device_id norfkill_ids[] __initconst = {
 	{ "VPC2004", 0},
 	{ "IBM0068", 0},

@@ -816,9 +807,10 @@ static int __init AMW0_set_cap_acpi_check_device(void)
 	const struct acpi_device_id *id;
 
 	for (id = norfkill_ids; id->id[0]; id++)
-		acpi_get_devices(id->id, AMW0_set_cap_acpi_check_device_cb,
-				NULL, NULL);
-	return AMW0_set_cap_acpi_check_device_found;
+		if (acpi_dev_present(id->id))
+			return true;
+
+	return false;
 }
 
 static acpi_status __init AMW0_set_capabilities(void)

Signature · 1 line

1.8.5.2 (Apple Git-48)

Darren Hart

2016-01-19 21:12 UTC #4 lore raw

In reply to Lukas Wunner

Quoted 11 lines · Lukas Wunner

Hi Darren,
the acpi_dev_present() API has now landed in Linus' tree. Thus, after Linus' tree gets merged back into yours, it would be possible to use the API in the pdx86 drivers as per the following patches.
I've also pushed these to GitHub in case anyone prefers perusing them in a browser:
https://github.com/l1k/linux/commits/acpi_dev_present_pdx86

Quoted 5 lines

This is a repost of patches submitted in November, the only change is one line added to the commit messages to reference the commit which introduces the API:
http://thread.gmane.org/gmane.linux.alsa.devel/147414/focus=8004
http://thread.gmane.org/gmane.linux.alsa.devel/147414/focus=8005

Who's tree did the API make it in through? That would likely be the best tree to pull these 2 patches in from.

Robert, Lv, Rafael? Would one of you prefer to take these 2 patches using the new API?

Alternatively, I can create a second pull request with a newer base after my first pull request to Linus this week. It just seems smoother to go through the tree that introduced the API if done during the same merge window.

Quoted 18 lines

Thanks,
Lukas
Lukas Wunner (2):
eeepc-wmi: Use acpi_dev_present()
acer-wmi: Use acpi_dev_present()
 drivers/platform/x86/acer-wmi.c  | 16 ++++------------
 drivers/platform/x86/eeepc-wmi.c | 24 ++----------------------
 2 files changed, 6 insertions(+), 34 deletions(-)
Signature · 1 line
1.8.5.2 (Apple Git-48)

Signature · 2 lines

Darren Hart
Intel Open Source Technology Center

Rafael J. Wysocki

2016-01-19 21:58 UTC #5 lore raw

Quoted 18 lines · Darren Hart

Quoted 11 lines · Lukas Wunner
Hi Darren,
the acpi_dev_present() API has now landed in Linus' tree. Thus, after Linus' tree gets merged back into yours, it would be possible to use the API in the pdx86 drivers as per the following patches.
I've also pushed these to GitHub in case anyone prefers perusing them in a browser:
https://github.com/l1k/linux/commits/acpi_dev_present_pdx86
Quoted 5 lines
This is a repost of patches submitted in November, the only change is one line added to the commit messages to reference the commit which introduces the API:
http://thread.gmane.org/gmane.linux.alsa.devel/147414/focus=8004
http://thread.gmane.org/gmane.linux.alsa.devel/147414/focus=8005

Quoted 5 lines

Who's tree did the API make it in through? That would likely be the best tree to pull these 2 patches in from.
Robert, Lv, Rafael? Would one of you prefer to take these 2 patches using the new API?

It was my tree and I can take these patches, but in that case I'd like the function's name to be changed as discussed elsewhere.

Executive summary is that we have acpi_dev_present() and acpi_device_is_present() now and they serve different purposes which is kind of confusing. Moreover, acpi_dev_present() doesn't check if the device is actually present, so I would like it to be renamed to acpi_device_found() or similar.

Thanks,
Rafael

Lukas Wunner

author

2016-02-11 18:31 UTC #6 lore raw

Hi Rafael,

thanks a lot for your patience.

Quoted 25 lines · Rafael J. Wysocki

Quoted 18 lines · Darren Hart
Quoted 11 lines · Lukas Wunner
Hi Darren,
the acpi_dev_present() API has now landed in Linus' tree. Thus, after Linus' tree gets merged back into yours, it would be possible to use the API in the pdx86 drivers as per the following patches.
I've also pushed these to GitHub in case anyone prefers perusing them in a browser:
https://github.com/l1k/linux/commits/acpi_dev_present_pdx86
Quoted 5 lines
This is a repost of patches submitted in November, the only change is one line added to the commit messages to reference the commit which introduces the API:
http://thread.gmane.org/gmane.linux.alsa.devel/147414/focus=8004
http://thread.gmane.org/gmane.linux.alsa.devel/147414/focus=8005
Quoted 5 lines
Who's tree did the API make it in through? That would likely be the best tree to pull these 2 patches in from.
Robert, Lv, Rafael? Would one of you prefer to take these 2 patches using the new API?

Quoted 7 lines

It was my tree and I can take these patches, but in that case I'd like the function's name to be changed as discussed elsewhere.
Executive summary is that we have acpi_dev_present() and acpi_device_is_present() now and they serve different purposes which is kind of confusing. Moreover, acpi_dev_present() doesn't check if the device is actually present, so I would like it to be renamed to acpi_device_found() or similar.

There are 4 users of acpi_dev_present in linux-next (3 in sound/soc/intel/, 1 in include/linux/apple-gmux.h). I expect 1 other user to appear in i915.

I've created a patch (based on linux-next and included below as an RFC) to rename acpi_dev_present to acpi_dev_found in the function declaration as well as at all call sites. I've also rebased the 2 pdx86 patches onto this and pushed the branch to GitHub:
https://github.com/l1k/linux/commits/acpi_dev_found

My plan is currently to wait until all users are merged into 4.6, then rebase my branch onto Linus' tree and post the resulting patches. This will be either late during the 4.6 merge window or immediately after it has closed. You could then either pick up the patches for 4.6 or 4.7, whichever you prefer.

If you'd prefer a different way of moving forward or would like something changed in the patch below, please let me know and I will be happy to adjust accordingly.

Thanks again,

Lukas

-- >8 --
Subject: [RFC] ACPI / utils: Rename acpi_dev_present()

acpi_dev_present() was originally named after pci_dev_present() to signify the similarity of the two functions.

However Rafael J. Wysocki pointed out that the exported function acpi_dev_present() is easily confused with the non-exported acpi_device_is_present(). Additionally in ACPI parlance the term "present" usually refers to the "device is present" bit returned by the _STA control method, yet acpi_dev_present() merely checks presence in the namespace. It does not invoke _STA at all, let alone check the "device is present" bit.

As suggested by Rafael, rename the function to acpi_dev_found() and adjust all existing call sites.

Cc "Rafael J. Wysocki" <[email protected]>

Signed-off-by Lukas Wunner <[email protected]>

---
 drivers/acpi/utils.c                         | 6 +++---
 include/acpi/acpi_bus.h                      | 2 +-
 include/linux/apple-gmux.h                   | 2 +-
 sound/soc/intel/boards/cht_bsw_max98090_ti.c | 2 +-
 sound/soc/intel/boards/cht_bsw_rt5645.c      | 2 +-
 sound/soc/intel/common/sst-match-acpi.c      | 2 +-
 6 files changed, 8 insertions(+), 8 deletions(-)

drivers/acpi/utils.c +3 -3

index f2f9873..41b80ff 100644
@@ -711,7 +711,7 @@ bool acpi_check_dsm(acpi_handle handle, const u8 *uuid, int rev, u64 funcs)
 EXPORT_SYMBOL(acpi_check_dsm);
 
 /**
- * acpi_dev_present - Detect presence of a given ACPI device in the system.
+ * acpi_dev_found - Detect presence of a given ACPI device in the namespace.
  * @hid: Hardware ID of the device.
  *
  * Return %true if the device was present at the moment of invocation.

@@ -723,7 +723,7 @@ EXPORT_SYMBOL(acpi_check_dsm);
  * instead). Calling from module_init() is fine (which is synonymous
  * with device_initcall()).
  */
-bool acpi_dev_present(const char *hid)
+bool acpi_dev_found(const char *hid)
 {
 	struct acpi_device_bus_id *acpi_device_bus_id;
 	bool found = false;

@@ -738,7 +738,7 @@ bool acpi_dev_present(const char *hid)
 
 	return found;
 }
-EXPORT_SYMBOL(acpi_dev_present);
+EXPORT_SYMBOL(acpi_dev_found);
 
 /*
  * acpi_backlight= handling, this is done here rather then in video_detect.c

include/acpi/acpi_bus.h +1 -1

index 14362a8..a84fd15 100644
@@ -87,7 +87,7 @@ acpi_evaluate_dsm_typed(acpi_handle handle, const u8 *uuid, int rev, int func,
 	  .package.elements = (eles)			\
 	}
 
-bool acpi_dev_present(const char *hid);
+bool acpi_dev_found(const char *hid);
 
 #ifdef CONFIG_ACPI

include/linux/apple-gmux.h +1 -1

index b2d32e0..714186d 100644
@@ -35,7 +35,7 @@
  */
 static inline bool apple_gmux_present(void)
 {
-	return acpi_dev_present(GMUX_ACPI_HID);
+	return acpi_dev_found(GMUX_ACPI_HID);
 }
 
 #else  /* !CONFIG_APPLE_GMUX */

sound/soc/intel/boards/cht_bsw_max98090_ti.c +1 -1

index e609f08..ac60b04 100644
@@ -296,7 +296,7 @@ static int snd_cht_mc_probe(struct platform_device *pdev)
 	if (!drv)
 		return -ENOMEM;
 
-	drv->ts3a227e_present = acpi_dev_present("104C227E");
+	drv->ts3a227e_present = acpi_dev_found("104C227E");
 	if (!drv->ts3a227e_present) {
 		/* no need probe TI jack detection chip */
 		snd_soc_card_cht.aux_dev = NULL;

sound/soc/intel/boards/cht_bsw_rt5645.c +1 -1

index e6cf800..edcf130 100644
@@ -346,7 +346,7 @@ static int snd_cht_mc_probe(struct platform_device *pdev)
 		return -ENOMEM;
 
 	for (i = 0; i < ARRAY_SIZE(snd_soc_cards); i++) {
-		if (acpi_dev_present(snd_soc_cards[i].codec_id)) {
+		if (acpi_dev_found(snd_soc_cards[i].codec_id)) {
 			dev_dbg(&pdev->dev,
 				"found codec %s\n", snd_soc_cards[i].codec_id);
 			card = snd_soc_cards[i].soc_card;

sound/soc/intel/common/sst-match-acpi.c +1 -1

index 0b8ee04..e45b278 100644
@@ -21,7 +21,7 @@ struct sst_acpi_mach *sst_acpi_find_machine(struct sst_acpi_mach *machines)
 	struct sst_acpi_mach *mach;
 
 	for (mach = machines; mach->id[0]; mach++)
-		if (acpi_dev_present(mach->id))
+		if (acpi_dev_found(mach->id))
 			return mach;
 
 	return NULL;

Signature · 1 line

1.8.5.2 (Apple Git-48)

Rafael J. Wysocki

maintainer

2016-03-09 22:21 UTC #7 lore raw

Quoted 53 lines · Lukas Wunner

Hi Rafael,
thanks a lot for your patience.
Quoted 25 lines · Rafael J. Wysocki
Quoted 18 lines · Darren Hart
Quoted 11 lines · Lukas Wunner
Hi Darren,
the acpi_dev_present() API has now landed in Linus' tree. Thus, after Linus' tree gets merged back into yours, it would be possible to use the API in the pdx86 drivers as per the following patches.
I've also pushed these to GitHub in case anyone prefers perusing them in a browser:
https://github.com/l1k/linux/commits/acpi_dev_present_pdx86
Quoted 5 lines
This is a repost of patches submitted in November, the only change is one line added to the commit messages to reference the commit which introduces the API:
http://thread.gmane.org/gmane.linux.alsa.devel/147414/focus=8004
http://thread.gmane.org/gmane.linux.alsa.devel/147414/focus=8005
Quoted 5 lines
Who's tree did the API make it in through? That would likely be the best tree to pull these 2 patches in from.
Robert, Lv, Rafael? Would one of you prefer to take these 2 patches using the new API?
Quoted 7 lines
It was my tree and I can take these patches, but in that case I'd like the function's name to be changed as discussed elsewhere.
Executive summary is that we have acpi_dev_present() and acpi_device_is_present() now and they serve different purposes which is kind of confusing. Moreover, acpi_dev_present() doesn't check if the device is actually present, so I would like it to be renamed to acpi_device_found() or similar.
There are 4 users of acpi_dev_present in linux-next (3 in sound/soc/intel/, 1 in include/linux/apple-gmux.h). I expect 1 other user to appear in i915.
I've created a patch (based on linux-next and included below as an RFC) to rename acpi_dev_present to acpi_dev_found in the function declaration as well as at all call sites. I've also rebased the 2 pdx86 patches onto this and pushed the branch to GitHub:
https://github.com/l1k/linux/commits/acpi_dev_found
My plan is currently to wait until all users are merged into 4.6, then rebase my branch onto Linus' tree and post the resulting patches. This will be either late during the 4.6 merge window or immediately after it has closed. You could then either pick up the patches for 4.6 or 4.7, whichever you prefer.

Quoted 3 lines

If you'd prefer a different way of moving forward or would like something changed in the patch below, please let me know and I will be happy to adjust accordingly.

Thanks for doing this!

Your plan seems workable to me, so please go ahead with it.

Thanks,
Rafael

Lukas Wunner

author

2016-03-12 17:30 UTC #8 lore raw

Hi Rafael,

I've noticed only now that the PCI core has 2 separate functions for detecting presence of a device, pci_dev_present() which only searches the list of enumerated devices, and pci_device_is_present(), which tests actual presence by reading from the device's config space.

Thus it would actually be consistent to have acpi_dev_present() (only searches namespace) and acpi_device_is_present() (calls _STA).

But I assume you still want it renamed, right? Just to clarify.

Thanks,

Lukas

Quoted 64 lines · Rafael J. Wysocki

Quoted 53 lines · Lukas Wunner
Hi Rafael,
thanks a lot for your patience.
Quoted 25 lines · Rafael J. Wysocki
Quoted 18 lines · Darren Hart
Quoted 11 lines · Lukas Wunner
Hi Darren,
the acpi_dev_present() API has now landed in Linus' tree. Thus, after Linus' tree gets merged back into yours, it would be possible to use the API in the pdx86 drivers as per the following patches.
I've also pushed these to GitHub in case anyone prefers perusing them in a browser:
https://github.com/l1k/linux/commits/acpi_dev_present_pdx86
Quoted 5 lines
This is a repost of patches submitted in November, the only change is one line added to the commit messages to reference the commit which introduces the API:
http://thread.gmane.org/gmane.linux.alsa.devel/147414/focus=8004
http://thread.gmane.org/gmane.linux.alsa.devel/147414/focus=8005
Quoted 5 lines
Who's tree did the API make it in through? That would likely be the best tree to pull these 2 patches in from.
Robert, Lv, Rafael? Would one of you prefer to take these 2 patches using the new API?
Quoted 7 lines
It was my tree and I can take these patches, but in that case I'd like the function's name to be changed as discussed elsewhere.
Executive summary is that we have acpi_dev_present() and acpi_device_is_present() now and they serve different purposes which is kind of confusing. Moreover, acpi_dev_present() doesn't check if the device is actually present, so I would like it to be renamed to acpi_device_found() or similar.
There are 4 users of acpi_dev_present in linux-next (3 in sound/soc/intel/, 1 in include/linux/apple-gmux.h). I expect 1 other user to appear in i915.
I've created a patch (based on linux-next and included below as an RFC) to rename acpi_dev_present to acpi_dev_found in the function declaration as well as at all call sites. I've also rebased the 2 pdx86 patches onto this and pushed the branch to GitHub:
https://github.com/l1k/linux/commits/acpi_dev_found
My plan is currently to wait until all users are merged into 4.6, then rebase my branch onto Linus' tree and post the resulting patches. This will be either late during the 4.6 merge window or immediately after it has closed. You could then either pick up the patches for 4.6 or 4.7, whichever you prefer.
Quoted 3 lines
If you'd prefer a different way of moving forward or would like something changed in the patch below, please let me know and I will be happy to adjust accordingly.
Thanks for doing this!
Your plan seems workable to me, so please go ahead with it.
Thanks,
Rafael

Rafael J. Wysocki

maintainer

2016-03-13 01:50 UTC #9 lore raw

Quoted 11 lines · Lukas Wunner

Hi Rafael,
I've noticed only now that the PCI core has 2 separate functions for detecting presence of a device, pci_dev_present() which only searches the list of enumerated devices, and pci_device_is_present(), which tests actual presence by reading from the device's config space.
Thus it would actually be consistent to have acpi_dev_present() (only searches namespace) and acpi_device_is_present() (calls _STA).
But I assume you still want it renamed, right?

Right.

Thanks,
Rafael