Linux Sound subsystem development
 help / color / mirror / Atom feed
From: Jaroslav Kysela <perex@perex.cz>
To: Venkata Prasad Potturu <venkataprasad.potturu@amd.com>,
	broonie@kernel.org, alsa-devel@alsa-project.org
Cc: Vijendar.Mukunda@amd.com, Basavaraj.Hiregoudar@amd.com,
	Sunil-kumar.Dommati@amd.com, syed.sabakareem@amd.com,
	mario.limonciello@amd.com, "Liam Girdwood" <lgirdwood@gmail.com>,
	"Takashi Iwai" <tiwai@suse.com>,
	"Tim Crawford" <tcrawford@system76.com>,
	"Attila Tőkés" <attitokes@gmail.com>,
	"Techno Mooney" <techno.mooney@gmail.com>,
	"Jeremy Soller" <jeremy@system76.com>,
	"Malcolm Hart" <malcolm@5harts.com>,
	"open list:SOUND - SOC LAYER / DYNAMIC AUDIO POWER MANAGEM..."
	<linux-sound@vger.kernel.org>,
	"open list" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] ASoC: amd: yc: Fix for enabling DMIC on acp6x via _DSD entry
Date: Wed, 27 Nov 2024 13:36:10 +0100	[thread overview]
Message-ID: <a6036171-2dfd-4296-9fe4-242c306e5449@perex.cz> (raw)
In-Reply-To: <20241127112227.227106-1-venkataprasad.potturu@amd.com>

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.

  parent reply	other threads:[~2024-11-27 12:42 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-11-27 11:22 [PATCH] ASoC: amd: yc: Fix for enabling DMIC on acp6x via _DSD entry Venkata Prasad Potturu
2024-11-27 12:13 ` Mark Brown
2024-11-27 12:36 ` Jaroslav Kysela [this message]
2024-11-27 17:04   ` potturu venkata prasad
2024-11-27 17:33     ` Mark Brown
2024-11-27 17:37       ` potturu venkata prasad

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=a6036171-2dfd-4296-9fe4-242c306e5449@perex.cz \
    --to=perex@perex.cz \
    --cc=Basavaraj.Hiregoudar@amd.com \
    --cc=Sunil-kumar.Dommati@amd.com \
    --cc=Vijendar.Mukunda@amd.com \
    --cc=alsa-devel@alsa-project.org \
    --cc=attitokes@gmail.com \
    --cc=broonie@kernel.org \
    --cc=jeremy@system76.com \
    --cc=lgirdwood@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-sound@vger.kernel.org \
    --cc=malcolm@5harts.com \
    --cc=mario.limonciello@amd.com \
    --cc=syed.sabakareem@amd.com \
    --cc=tcrawford@system76.com \
    --cc=techno.mooney@gmail.com \
    --cc=tiwai@suse.com \
    --cc=venkataprasad.potturu@amd.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox