linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] ASoC: Intel: common: Replace sst_acpi_check_hid with acpi_dev_present
@ 2017-11-09 21:07 Jeremy Cline
  2017-11-10 11:33 ` Hans de Goede
  0 siblings, 1 reply; 3+ messages in thread
From: Jeremy Cline @ 2017-11-09 21:07 UTC (permalink / raw)
  To: Liam Girdwood, Mark Brown
  Cc: Jaroslav Kysela, Takashi Iwai, Hans de Goede, alsa-devel,
	linux-kernel, Jeremy Cline

Replace the sst_acpi_check_hid helper function added in
commit 915ae2b9f0fe ("ASoC: Intel: Create a helper to search for
matching machine") with the generic acpi_dev_present helper function
and remove the now unused sst_acpi_check_hid function. This should have
no functional change.

Signed-off-by: Jeremy Cline <jeremy@jcline.org>
---
 sound/soc/intel/common/sst-acpi.h       |  3 ---
 sound/soc/intel/common/sst-match-acpi.c | 32 ++------------------------------
 2 files changed, 2 insertions(+), 33 deletions(-)

diff --git a/sound/soc/intel/common/sst-acpi.h b/sound/soc/intel/common/sst-acpi.h
index afe9b87b8bd5..d7facac83b7e 100644
--- a/sound/soc/intel/common/sst-acpi.h
+++ b/sound/soc/intel/common/sst-acpi.h
@@ -43,9 +43,6 @@ static inline bool sst_acpi_find_package_from_hid(const u8 hid[ACPI_ID_LEN],
 /* acpi match */
 struct sst_acpi_mach *sst_acpi_find_machine(struct sst_acpi_mach *machines);
 
-/* acpi check hid */
-bool sst_acpi_check_hid(const u8 hid[ACPI_ID_LEN]);
-
 /* Descriptor for SST ASoC machine driver */
 struct sst_acpi_mach {
 	/* ACPI ID for the matching machine driver. Audio codec for instance */
diff --git a/sound/soc/intel/common/sst-match-acpi.c b/sound/soc/intel/common/sst-match-acpi.c
index 56d26f36a3cb..f8e1b92de79a 100644
--- a/sound/soc/intel/common/sst-match-acpi.c
+++ b/sound/soc/intel/common/sst-match-acpi.c
@@ -49,40 +49,12 @@ const char *sst_acpi_find_name_from_hid(const u8 hid[ACPI_ID_LEN])
 }
 EXPORT_SYMBOL_GPL(sst_acpi_find_name_from_hid);
 
-static acpi_status sst_acpi_mach_match(acpi_handle handle, u32 level,
-				       void *context, void **ret)
-{
-	unsigned long long sta;
-	acpi_status status;
-
-	*(bool *)context = true;
-	status = acpi_evaluate_integer(handle, "_STA", NULL, &sta);
-	if (ACPI_FAILURE(status) || !(sta & ACPI_STA_DEVICE_PRESENT))
-		*(bool *)context = false;
-
-	return AE_OK;
-}
-
-bool sst_acpi_check_hid(const u8 hid[ACPI_ID_LEN])
-{
-	acpi_status status;
-	bool found = false;
-
-	status = acpi_get_devices(hid, sst_acpi_mach_match, &found, NULL);
-
-	if (ACPI_FAILURE(status))
-		return false;
-
-	return found;
-}
-EXPORT_SYMBOL_GPL(sst_acpi_check_hid);
-
 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 (sst_acpi_check_hid(mach->id) == true) {
+		if (acpi_dev_present(mach->id, NULL, -1) == true) {
 			if (mach->machine_quirk == NULL)
 				return mach;
 
@@ -161,7 +133,7 @@ struct sst_acpi_mach *sst_acpi_codec_list(void *arg)
 		return mach;
 
 	for (i = 0; i < codec_list->num_codecs; i++) {
-		if (sst_acpi_check_hid(codec_list->codecs[i]) != true)
+		if (acpi_dev_present(codec_list->codecs[i], NULL, -1) != true)
 			return NULL;
 	}
 
-- 
2.14.3

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

* Re: [PATCH] ASoC: Intel: common: Replace sst_acpi_check_hid with acpi_dev_present
  2017-11-09 21:07 [PATCH] ASoC: Intel: common: Replace sst_acpi_check_hid with acpi_dev_present Jeremy Cline
@ 2017-11-10 11:33 ` Hans de Goede
  2017-11-10 14:39   ` [alsa-devel] " Pierre-Louis Bossart
  0 siblings, 1 reply; 3+ messages in thread
From: Hans de Goede @ 2017-11-10 11:33 UTC (permalink / raw)
  To: Jeremy Cline, Liam Girdwood, Mark Brown
  Cc: Jaroslav Kysela, Takashi Iwai, alsa-devel, linux-kernel

Hi,

On 09-11-17 22:07, Jeremy Cline wrote:
> Replace the sst_acpi_check_hid helper function added in
> commit 915ae2b9f0fe ("ASoC: Intel: Create a helper to search for
> matching machine") with the generic acpi_dev_present helper function
> and remove the now unused sst_acpi_check_hid function. This should have
> no functional change.
> 
> Signed-off-by: Jeremy Cline <jeremy@jcline.org>

I've just successfully tested this on an Assu T100HA:

Tested-by: Hans de Goede <hdegoede@redhat.com>
Reviewed-by: Hans de Goede <hdegoede@redhat.com>

Regards,

Hans



> ---
>   sound/soc/intel/common/sst-acpi.h       |  3 ---
>   sound/soc/intel/common/sst-match-acpi.c | 32 ++------------------------------
>   2 files changed, 2 insertions(+), 33 deletions(-)
> 
> diff --git a/sound/soc/intel/common/sst-acpi.h b/sound/soc/intel/common/sst-acpi.h
> index afe9b87b8bd5..d7facac83b7e 100644
> --- a/sound/soc/intel/common/sst-acpi.h
> +++ b/sound/soc/intel/common/sst-acpi.h
> @@ -43,9 +43,6 @@ static inline bool sst_acpi_find_package_from_hid(const u8 hid[ACPI_ID_LEN],
>   /* acpi match */
>   struct sst_acpi_mach *sst_acpi_find_machine(struct sst_acpi_mach *machines);
>   
> -/* acpi check hid */
> -bool sst_acpi_check_hid(const u8 hid[ACPI_ID_LEN]);
> -
>   /* Descriptor for SST ASoC machine driver */
>   struct sst_acpi_mach {
>   	/* ACPI ID for the matching machine driver. Audio codec for instance */
> diff --git a/sound/soc/intel/common/sst-match-acpi.c b/sound/soc/intel/common/sst-match-acpi.c
> index 56d26f36a3cb..f8e1b92de79a 100644
> --- a/sound/soc/intel/common/sst-match-acpi.c
> +++ b/sound/soc/intel/common/sst-match-acpi.c
> @@ -49,40 +49,12 @@ const char *sst_acpi_find_name_from_hid(const u8 hid[ACPI_ID_LEN])
>   }
>   EXPORT_SYMBOL_GPL(sst_acpi_find_name_from_hid);
>   
> -static acpi_status sst_acpi_mach_match(acpi_handle handle, u32 level,
> -				       void *context, void **ret)
> -{
> -	unsigned long long sta;
> -	acpi_status status;
> -
> -	*(bool *)context = true;
> -	status = acpi_evaluate_integer(handle, "_STA", NULL, &sta);
> -	if (ACPI_FAILURE(status) || !(sta & ACPI_STA_DEVICE_PRESENT))
> -		*(bool *)context = false;
> -
> -	return AE_OK;
> -}
> -
> -bool sst_acpi_check_hid(const u8 hid[ACPI_ID_LEN])
> -{
> -	acpi_status status;
> -	bool found = false;
> -
> -	status = acpi_get_devices(hid, sst_acpi_mach_match, &found, NULL);
> -
> -	if (ACPI_FAILURE(status))
> -		return false;
> -
> -	return found;
> -}
> -EXPORT_SYMBOL_GPL(sst_acpi_check_hid);
> -
>   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 (sst_acpi_check_hid(mach->id) == true) {
> +		if (acpi_dev_present(mach->id, NULL, -1) == true) {
>   			if (mach->machine_quirk == NULL)
>   				return mach;
>   
> @@ -161,7 +133,7 @@ struct sst_acpi_mach *sst_acpi_codec_list(void *arg)
>   		return mach;
>   
>   	for (i = 0; i < codec_list->num_codecs; i++) {
> -		if (sst_acpi_check_hid(codec_list->codecs[i]) != true)
> +		if (acpi_dev_present(codec_list->codecs[i], NULL, -1) != true)
>   			return NULL;
>   	}
>   
> 

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

* Re: [alsa-devel] [PATCH] ASoC: Intel: common: Replace sst_acpi_check_hid with acpi_dev_present
  2017-11-10 11:33 ` Hans de Goede
@ 2017-11-10 14:39   ` Pierre-Louis Bossart
  0 siblings, 0 replies; 3+ messages in thread
From: Pierre-Louis Bossart @ 2017-11-10 14:39 UTC (permalink / raw)
  To: Hans de Goede, Jeremy Cline, Liam Girdwood, Mark Brown
  Cc: linux-kernel, alsa-devel, Takashi Iwai



On 11/10/2017 05:33 AM, Hans de Goede wrote:
> Hi,
>
> On 09-11-17 22:07, Jeremy Cline wrote:
>> Replace the sst_acpi_check_hid helper function added in
>> commit 915ae2b9f0fe ("ASoC: Intel: Create a helper to search for
>> matching machine") with the generic acpi_dev_present helper function
>> and remove the now unused sst_acpi_check_hid function. This should have
>> no functional change.
>>
>> Signed-off-by: Jeremy Cline <jeremy@jcline.org>
>
> I've just successfully tested this on an Assu T100HA:
>
> Tested-by: Hans de Goede <hdegoede@redhat.com>
> Reviewed-by: Hans de Goede <hdegoede@redhat.com>

I don't mind this change, but I'd also like to see a bit more testing 
with weird devices to check that the code is indeed equivalent (not 
obvious from a quick code inspection). Surface 3 was broken recently due 
to lack of testing of a change that 'should have been functionally 
equivalent'.

Also this patch will not apply, the code was changed to remove any 
reference to the sst driver, you need to rebase against broonie/for-next.

>
> Regards,
>
> Hans
>
>
>
>> ---
>>   sound/soc/intel/common/sst-acpi.h       |  3 ---
>>   sound/soc/intel/common/sst-match-acpi.c | 32 
>> ++------------------------------
>>   2 files changed, 2 insertions(+), 33 deletions(-)
>>
>> diff --git a/sound/soc/intel/common/sst-acpi.h 
>> b/sound/soc/intel/common/sst-acpi.h
>> index afe9b87b8bd5..d7facac83b7e 100644
>> --- a/sound/soc/intel/common/sst-acpi.h
>> +++ b/sound/soc/intel/common/sst-acpi.h
>> @@ -43,9 +43,6 @@ static inline bool 
>> sst_acpi_find_package_from_hid(const u8 hid[ACPI_ID_LEN],
>>   /* acpi match */
>>   struct sst_acpi_mach *sst_acpi_find_machine(struct sst_acpi_mach 
>> *machines);
>>   -/* acpi check hid */
>> -bool sst_acpi_check_hid(const u8 hid[ACPI_ID_LEN]);
>> -
>>   /* Descriptor for SST ASoC machine driver */
>>   struct sst_acpi_mach {
>>       /* ACPI ID for the matching machine driver. Audio codec for 
>> instance */
>> diff --git a/sound/soc/intel/common/sst-match-acpi.c 
>> b/sound/soc/intel/common/sst-match-acpi.c
>> index 56d26f36a3cb..f8e1b92de79a 100644
>> --- a/sound/soc/intel/common/sst-match-acpi.c
>> +++ b/sound/soc/intel/common/sst-match-acpi.c
>> @@ -49,40 +49,12 @@ const char *sst_acpi_find_name_from_hid(const u8 
>> hid[ACPI_ID_LEN])
>>   }
>>   EXPORT_SYMBOL_GPL(sst_acpi_find_name_from_hid);
>>   -static acpi_status sst_acpi_mach_match(acpi_handle handle, u32 level,
>> -                       void *context, void **ret)
>> -{
>> -    unsigned long long sta;
>> -    acpi_status status;
>> -
>> -    *(bool *)context = true;
>> -    status = acpi_evaluate_integer(handle, "_STA", NULL, &sta);
>> -    if (ACPI_FAILURE(status) || !(sta & ACPI_STA_DEVICE_PRESENT))
>> -        *(bool *)context = false;
>> -
>> -    return AE_OK;
>> -}
>> -
>> -bool sst_acpi_check_hid(const u8 hid[ACPI_ID_LEN])
>> -{
>> -    acpi_status status;
>> -    bool found = false;
>> -
>> -    status = acpi_get_devices(hid, sst_acpi_mach_match, &found, NULL);
>> -
>> -    if (ACPI_FAILURE(status))
>> -        return false;
>> -
>> -    return found;
>> -}
>> -EXPORT_SYMBOL_GPL(sst_acpi_check_hid);
>> -
>>   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 (sst_acpi_check_hid(mach->id) == true) {
>> +        if (acpi_dev_present(mach->id, NULL, -1) == true) {
>>               if (mach->machine_quirk == NULL)
>>                   return mach;
>>   @@ -161,7 +133,7 @@ struct sst_acpi_mach *sst_acpi_codec_list(void 
>> *arg)
>>           return mach;
>>         for (i = 0; i < codec_list->num_codecs; i++) {
>> -        if (sst_acpi_check_hid(codec_list->codecs[i]) != true)
>> +        if (acpi_dev_present(codec_list->codecs[i], NULL, -1) != true)
>>               return NULL;
>>       }
>>
> _______________________________________________
> Alsa-devel mailing list
> Alsa-devel@alsa-project.org
> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

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

end of thread, other threads:[~2017-11-10 14:39 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-11-09 21:07 [PATCH] ASoC: Intel: common: Replace sst_acpi_check_hid with acpi_dev_present Jeremy Cline
2017-11-10 11:33 ` Hans de Goede
2017-11-10 14:39   ` [alsa-devel] " Pierre-Louis Bossart

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).