diff mbox series

ASoC: amd: yc: Fix the wrong return value

Message ID 20241209101320.970571-1-venkataprasad.potturu@amd.com (mailing list archive)
State Superseded
Headers show
Series ASoC: amd: yc: Fix the wrong return value | expand

Commit Message

potturu venkata prasad Dec. 9, 2024, 10:13 a.m. UTC
With the current implementation, when ACP driver fails to read
ACPI _WOV entry then the DMI overrides code won't invoke,
may cause regressions for some BIOS versions.

Add a condition check to jump to check the DMI entries
incase of ACP driver fail to read ACPI _WOV method.

Fixes: 4095cf872084 (ASoC: amd: yc: Fix for enabling DMIC on acp6x via _DSD entry)

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

Comments

Markus Elfring Dec. 9, 2024, 1:46 p.m. UTC | #1
> Add a condition check to jump to check the DMI entries
> incase of ACP driver fail to read ACPI _WOV method.

  in case?             failure?


…
> +++ b/sound/soc/amd/yc/acp6x-mach.c
> @@ -578,14 +578,19 @@ static int acp6x_probe(struct platform_device *pdev)
>
>  	handle = ACPI_HANDLE(pdev->dev.parent);
>  	ret = acpi_evaluate_integer(handle, "_WOV", NULL, &dmic_status);
> -	if (!ACPI_FAILURE(ret))
> +	if (!ACPI_FAILURE(ret)) {
>  		wov_en = dmic_status;
> +		if (!wov_en)
> +			return -ENODEV;
> +	} else {
> +		/* Incase of ACPI method read failure then jump to check_dmi_entry */
> +		goto check_dmi_entry;
> +	}
>
>  	if (is_dmic_enable && wov_en)
>  		platform_set_drvdata(pdev, &acp6x_card);
…

Is there a need to adjust another condition check accordingly?

Regards,
Markus
potturu venkata prasad Dec. 10, 2024, 6:24 a.m. UTC | #2
On 12/9/24 19:16, Markus Elfring wrote:
Hi Markus,
> …
>> Add a condition check to jump to check the DMI entries
>> incase of ACP driver fail to read ACPI _WOV method.
>    in case?             failure?
It's a continuous statement.
"Added a condition check to jump into check_dmi_entry when ACP driver 
fail to read ACPI _WOV method".
>
>
> …
>> +++ b/sound/soc/amd/yc/acp6x-mach.c
>> @@ -578,14 +578,19 @@ static int acp6x_probe(struct platform_device *pdev)
>>
>>   	handle = ACPI_HANDLE(pdev->dev.parent);
>>   	ret = acpi_evaluate_integer(handle, "_WOV", NULL, &dmic_status);
>> -	if (!ACPI_FAILURE(ret))
>> +	if (!ACPI_FAILURE(ret)) {
>>   		wov_en = dmic_status;
>> +		if (!wov_en)
>> +			return -ENODEV;
>> +	} else {
>> +		/* Incase of ACPI method read failure then jump to check_dmi_entry */
>> +		goto check_dmi_entry;
>> +	}
>>
>>   	if (is_dmic_enable && wov_en)
>>   		platform_set_drvdata(pdev, &acp6x_card);
> …
>
> Is there a need to adjust another condition check accordingly?
No Markus, not required.
>
> Regards,
> Markus
Markus Elfring Dec. 10, 2024, 7:28 a.m. UTC | #3
>> …
>>> +++ b/sound/soc/amd/yc/acp6x-mach.c
>>> @@ -578,14 +578,19 @@ static int acp6x_probe(struct platform_device *pdev)
>>>
>>>       handle = ACPI_HANDLE(pdev->dev.parent);
>>>       ret = acpi_evaluate_integer(handle, "_WOV", NULL, &dmic_status);
>>> -    if (!ACPI_FAILURE(ret))
>>> +    if (!ACPI_FAILURE(ret)) {
>>>           wov_en = dmic_status;
>>> +        if (!wov_en)
>>> +            return -ENODEV;
>>> +    } else {
>>> +        /* Incase of ACPI method read failure then jump to check_dmi_entry */
>>> +        goto check_dmi_entry;
>>> +    }
>>>
>>>       if (is_dmic_enable && wov_en)
>>>           platform_set_drvdata(pdev, &acp6x_card);
>> …
>>
>> Is there a need to adjust another condition check accordingly?
> No Markus, not required.

Can it be that the expression part “&& wov_en” became redundant with the proposed
source code adjustment?

Regards,
Markus
potturu venkata prasad Dec. 10, 2024, 8:37 a.m. UTC | #4
On 12/10/24 12:58, Markus Elfring wrote:
>>> …
>>>> +++ b/sound/soc/amd/yc/acp6x-mach.c
>>>> @@ -578,14 +578,19 @@ static int acp6x_probe(struct platform_device *pdev)
>>>>
>>>>        handle = ACPI_HANDLE(pdev->dev.parent);
>>>>        ret = acpi_evaluate_integer(handle, "_WOV", NULL, &dmic_status);
>>>> -    if (!ACPI_FAILURE(ret))
>>>> +    if (!ACPI_FAILURE(ret)) {
>>>>            wov_en = dmic_status;
>>>> +        if (!wov_en)
>>>> +            return -ENODEV;
>>>> +    } else {
>>>> +        /* Incase of ACPI method read failure then jump to check_dmi_entry */
>>>> +        goto check_dmi_entry;
>>>> +    }
>>>>
>>>>        if (is_dmic_enable && wov_en)
>>>>            platform_set_drvdata(pdev, &acp6x_card);
>>> …
>>>
>>> Is there a need to adjust another condition check accordingly?
>> No Markus, not required.
> Can it be that the expression part “&& wov_en” became redundant with the proposed
> source code adjustment?
Agreed, will remove && wov_en  and then send v2 patch.
>
> Regards,
> Markus
Mark Brown Dec. 10, 2024, 2:51 p.m. UTC | #5
On Mon, 09 Dec 2024 15:43:17 +0530, Venkata Prasad Potturu wrote:
> With the current implementation, when ACP driver fails to read
> ACPI _WOV entry then the DMI overrides code won't invoke,
> may cause regressions for some BIOS versions.
> 
> Add a condition check to jump to check the DMI entries
> incase of ACP driver fail to read ACPI _WOV method.
> 
> [...]

Applied to

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

Thanks!

[1/1] ASoC: amd: yc: Fix the wrong return value
      commit: 984795e76def5c903724b8d6a8228e356bbdf2af

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
diff mbox series

Patch

diff --git a/sound/soc/amd/yc/acp6x-mach.c b/sound/soc/amd/yc/acp6x-mach.c
index e38c5885dadf..13af1d485eb0 100644
--- a/sound/soc/amd/yc/acp6x-mach.c
+++ b/sound/soc/amd/yc/acp6x-mach.c
@@ -578,14 +578,19 @@  static int acp6x_probe(struct platform_device *pdev)
 
 	handle = ACPI_HANDLE(pdev->dev.parent);
 	ret = acpi_evaluate_integer(handle, "_WOV", NULL, &dmic_status);
-	if (!ACPI_FAILURE(ret))
+	if (!ACPI_FAILURE(ret)) {
 		wov_en = dmic_status;
+		if (!wov_en)
+			return -ENODEV;
+	} else {
+		/* Incase of ACPI method read failure then jump to check_dmi_entry */
+		goto check_dmi_entry;
+	}
 
 	if (is_dmic_enable && wov_en)
 		platform_set_drvdata(pdev, &acp6x_card);
-	else
-		return 0;
 
+check_dmi_entry:
 	/* check for any DMI overrides */
 	dmi_id = dmi_first_match(yc_acp_quirk_table);
 	if (dmi_id)