[PATCH] ASoC: amd: yc: Fix for enabling DMIC on acp6x via _DSD entry

Venkata Prasad Potturu posted 1 patch 1 year, 2 months ago
sound/soc/amd/yc/acp6x-mach.c | 18 +++++++++++++++++-
1 file changed, 17 insertions(+), 1 deletion(-)
[PATCH] ASoC: amd: yc: Fix for enabling DMIC on acp6x via _DSD entry
Posted by Venkata Prasad Potturu 1 year, 2 months ago
Add condition check to register ACP PDM sound card by reading
_WOV acpi entry.

Fixes: 5426f506b584 ("ASoC: amd: Add support for enabling DMIC on acp6x via _DSD")

Signed-off-by: Venkata Prasad Potturu <venkataprasad.potturu@amd.com>
---
 sound/soc/amd/yc/acp6x-mach.c | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/sound/soc/amd/yc/acp6x-mach.c b/sound/soc/amd/yc/acp6x-mach.c
index 438865d5e376..c301dac8b979 100644
--- a/sound/soc/amd/yc/acp6x-mach.c
+++ b/sound/soc/amd/yc/acp6x-mach.c
@@ -523,8 +523,14 @@ static int acp6x_probe(struct platform_device *pdev)
 	struct acp6x_pdm *machine = NULL;
 	struct snd_soc_card *card;
 	struct acpi_device *adev;
+	acpi_handle handle;
+	acpi_integer dmic_status;
 	int ret;
+	bool is_dmic_enable, wov_en;
 
+	/* IF WOV entry not found, enable dmic based on AcpDmicConnected entry*/
+	is_dmic_enable = false;
+	wov_en = true;
 	/* check the parent device's firmware node has _DSD or not */
 	adev = ACPI_COMPANION(pdev->dev.parent);
 	if (adev) {
@@ -532,9 +538,19 @@ static int acp6x_probe(struct platform_device *pdev)
 
 		if (!acpi_dev_get_property(adev, "AcpDmicConnected", ACPI_TYPE_INTEGER, &obj) &&
 		    obj->integer.value == 1)
-			platform_set_drvdata(pdev, &acp6x_card);
+			is_dmic_enable = true;
 	}
 
+	handle = ACPI_HANDLE(pdev->dev.parent);
+	ret = acpi_evaluate_integer(handle, "_WOV", NULL, &dmic_status);
+	if (!ACPI_FAILURE(ret))
+		wov_en = dmic_status;
+
+	if (is_dmic_enable && wov_en)
+		platform_set_drvdata(pdev, &acp6x_card);
+	else
+		return 0;
+
 	/* check for any DMI overrides */
 	dmi_id = dmi_first_match(yc_acp_quirk_table);
 	if (dmi_id)
-- 
2.39.2
Re: [PATCH] ASoC: amd: yc: Fix for enabling DMIC on acp6x via _DSD entry
Posted by Jaroslav Kysela 1 year, 2 months ago
On 27. 11. 24 12:22, Venkata Prasad Potturu wrote:
> Add condition check to register ACP PDM sound card by reading
> _WOV acpi entry.
> 
> Fixes: 5426f506b584 ("ASoC: amd: Add support for enabling DMIC on acp6x via _DSD")
> 
> Signed-off-by: Venkata Prasad Potturu <venkataprasad.potturu@amd.com>
> ---
>   sound/soc/amd/yc/acp6x-mach.c | 18 +++++++++++++++++-
>   1 file changed, 17 insertions(+), 1 deletion(-)
> 
> diff --git a/sound/soc/amd/yc/acp6x-mach.c b/sound/soc/amd/yc/acp6x-mach.c
> index 438865d5e376..c301dac8b979 100644
> --- a/sound/soc/amd/yc/acp6x-mach.c
> +++ b/sound/soc/amd/yc/acp6x-mach.c
> @@ -523,8 +523,14 @@ static int acp6x_probe(struct platform_device *pdev)
>   	struct acp6x_pdm *machine = NULL;
>   	struct snd_soc_card *card;
>   	struct acpi_device *adev;
> +	acpi_handle handle;
> +	acpi_integer dmic_status;
>   	int ret;
> +	bool is_dmic_enable, wov_en;
>   
> +	/* IF WOV entry not found, enable dmic based on AcpDmicConnected entry*/
> +	is_dmic_enable = false;
> +	wov_en = true;
>   	/* check the parent device's firmware node has _DSD or not */
>   	adev = ACPI_COMPANION(pdev->dev.parent);
>   	if (adev) {
> @@ -532,9 +538,19 @@ static int acp6x_probe(struct platform_device *pdev)
>   
>   		if (!acpi_dev_get_property(adev, "AcpDmicConnected", ACPI_TYPE_INTEGER, &obj) &&
>   		    obj->integer.value == 1)
> -			platform_set_drvdata(pdev, &acp6x_card);
> +			is_dmic_enable = true;
>   	}
>   
> +	handle = ACPI_HANDLE(pdev->dev.parent);
> +	ret = acpi_evaluate_integer(handle, "_WOV", NULL, &dmic_status);
> +	if (!ACPI_FAILURE(ret))
> +		wov_en = dmic_status;
> +
> +	if (is_dmic_enable && wov_en)
> +		platform_set_drvdata(pdev, &acp6x_card);
> +	else
> +		return 0;

The logic seems wrong. The overrides code bellow won't be executed when ACPI 
_WOV data are not set. I think that it may cause regressions for some BIOS 
versions.

Also, the return code should be probably -ENODEV.

					Jaroslav

-- 
Jaroslav Kysela <perex@perex.cz>
Linux Sound Maintainer; ALSA Project; Red Hat, Inc.
Re: [PATCH] ASoC: amd: yc: Fix for enabling DMIC on acp6x via _DSD entry
Posted by potturu venkata prasad 1 year, 2 months ago
On 11/27/24 18:06, Jaroslav Kysela wrote:
> On 27. 11. 24 12:22, Venkata Prasad Potturu wrote:
>> Add condition check to register ACP PDM sound card by reading
>> _WOV acpi entry.
>>
>> Fixes: 5426f506b584 ("ASoC: amd: Add support for enabling DMIC on 
>> acp6x via _DSD")
>>
>> Signed-off-by: Venkata Prasad Potturu <venkataprasad.potturu@amd.com>
>> ---
>>   sound/soc/amd/yc/acp6x-mach.c | 18 +++++++++++++++++-
>>   1 file changed, 17 insertions(+), 1 deletion(-)
>>
>> diff --git a/sound/soc/amd/yc/acp6x-mach.c 
>> b/sound/soc/amd/yc/acp6x-mach.c
>> index 438865d5e376..c301dac8b979 100644
>> --- a/sound/soc/amd/yc/acp6x-mach.c
>> +++ b/sound/soc/amd/yc/acp6x-mach.c
>> @@ -523,8 +523,14 @@ static int acp6x_probe(struct platform_device 
>> *pdev)
>>       struct acp6x_pdm *machine = NULL;
>>       struct snd_soc_card *card;
>>       struct acpi_device *adev;
>> +    acpi_handle handle;
>> +    acpi_integer dmic_status;
>>       int ret;
>> +    bool is_dmic_enable, wov_en;
>>   +    /* IF WOV entry not found, enable dmic based on 
>> AcpDmicConnected entry*/
>> +    is_dmic_enable = false;
>> +    wov_en = true;
>>       /* check the parent device's firmware node has _DSD or not */
>>       adev = ACPI_COMPANION(pdev->dev.parent);
>>       if (adev) {
>> @@ -532,9 +538,19 @@ static int acp6x_probe(struct platform_device 
>> *pdev)
>>             if (!acpi_dev_get_property(adev, "AcpDmicConnected", 
>> ACPI_TYPE_INTEGER, &obj) &&
>>               obj->integer.value == 1)
>> -            platform_set_drvdata(pdev, &acp6x_card);
>> +            is_dmic_enable = true;
>>       }
>>   +    handle = ACPI_HANDLE(pdev->dev.parent);
>> +    ret = acpi_evaluate_integer(handle, "_WOV", NULL, &dmic_status);
>> +    if (!ACPI_FAILURE(ret))
>> +        wov_en = dmic_status;
>> +
>> +    if (is_dmic_enable && wov_en)
>> +        platform_set_drvdata(pdev, &acp6x_card);
>> +    else
>> +        return 0;
>
> The logic seems wrong. The overrides code bellow won't be executed 
> when ACPI _WOV data are not set. I think that it may cause regressions 
> for some BIOS versions.
>
> Also, the return code should be probably -ENODEV.
>
>                     Jaroslav
Okay Jaroslav,

We will correct it and resend v2 patch.
Thanks.

Re: [PATCH] ASoC: amd: yc: Fix for enabling DMIC on acp6x via _DSD entry
Posted by Mark Brown 1 year, 2 months ago
On Wed, Nov 27, 2024 at 10:34:32PM +0530, potturu venkata prasad wrote:
> On 11/27/24 18:06, Jaroslav Kysela wrote:

> > The logic seems wrong. The overrides code bellow won't be executed when
> > ACPI _WOV data are not set. I think that it may cause regressions for
> > some BIOS versions.

> > Also, the return code should be probably -ENODEV.

> Okay Jaroslav,

> We will correct it and resend v2 patch.

Given that I applied this before Jaroslav's review please send an
incremental patch that fixes the issue.
Re: [PATCH] ASoC: amd: yc: Fix for enabling DMIC on acp6x via _DSD entry
Posted by potturu venkata prasad 1 year, 2 months ago
On 11/27/24 23:03, Mark Brown wrote:
> On Wed, Nov 27, 2024 at 10:34:32PM +0530, potturu venkata prasad wrote:
>> On 11/27/24 18:06, Jaroslav Kysela wrote:
>>> The logic seems wrong. The overrides code bellow won't be executed when
>>> ACPI _WOV data are not set. I think that it may cause regressions for
>>> some BIOS versions.
>>> Also, the return code should be probably -ENODEV.
>> Okay Jaroslav,
>> We will correct it and resend v2 patch.
> Given that I applied this before Jaroslav's review please send an
> incremental patch that fixes the issue.
Sorry, I missed it, will send incremental patch.

Thanks.
Re: [PATCH] ASoC: amd: yc: Fix for enabling DMIC on acp6x via _DSD entry
Posted by Mark Brown 1 year, 2 months ago
On Wed, 27 Nov 2024 16:52:25 +0530, Venkata Prasad Potturu wrote:
> Add condition check to register ACP PDM sound card by reading
> _WOV acpi entry.
> 
> Fixes: 5426f506b584 ("ASoC: amd: Add support for enabling DMIC on acp6x via _DSD")
> 
> 

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next

Thanks!

[1/1] ASoC: amd: yc: Fix for enabling DMIC on acp6x via _DSD entry
      commit: 4095cf872084ecfdfdb0e681f3e9ff9745acfa75

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark