2023-04-12 09:27:09

by Saba Kareem, Syed

[permalink] [raw]
Subject: [PATCH] ASoC: amd: Add check for acp config flags

We have SOF and generic ACP support enabled for Rembrandt and
pheonix platforms on some machines. Since we have same PCI id
used for probing, add check for machine configuration flag to
avoid conflict with newer pci drivers. Such machine flag has
been initialized via dmi match on few Chrome machines. If no
flag is specified probe and register older platform device.

Signed-off-by: Syed Saba Kareem <[email protected]>
Reviewed-by: Vijendar Mukunda <[email protected]>
---
sound/soc/amd/Kconfig | 2 ++
sound/soc/amd/ps/acp63.h | 2 ++
sound/soc/amd/ps/pci-ps.c | 8 +++++++-
sound/soc/amd/yc/acp6x.h | 3 +++
sound/soc/amd/yc/pci-acp6x.c | 8 +++++++-
5 files changed, 21 insertions(+), 2 deletions(-)

diff --git a/sound/soc/amd/Kconfig b/sound/soc/amd/Kconfig
index c88ebd84bdd5..08e42082f5e9 100644
--- a/sound/soc/amd/Kconfig
+++ b/sound/soc/amd/Kconfig
@@ -90,6 +90,7 @@ config SND_SOC_AMD_VANGOGH_MACH

config SND_SOC_AMD_ACP6x
tristate "AMD Audio Coprocessor-v6.x Yellow Carp support"
+ select SND_AMD_ACP_CONFIG
depends on X86 && PCI
help
This option enables Audio Coprocessor i.e ACP v6.x support on
@@ -130,6 +131,7 @@ config SND_SOC_AMD_RPL_ACP6x

config SND_SOC_AMD_PS
tristate "AMD Audio Coprocessor-v6.3 Pink Sardine support"
+ select SND_AMD_ACP_CONFIG
depends on X86 && PCI && ACPI
help
This option enables Audio Coprocessor i.e ACP v6.3 support on
diff --git a/sound/soc/amd/ps/acp63.h b/sound/soc/amd/ps/acp63.h
index 6bf29b520511..dd36790b25ae 100644
--- a/sound/soc/amd/ps/acp63.h
+++ b/sound/soc/amd/ps/acp63.h
@@ -111,3 +111,5 @@ struct acp63_dev_data {
u16 pdev_count;
u16 pdm_dev_index;
};
+
+int snd_amd_acp_find_config(struct pci_dev *pci);
diff --git a/sound/soc/amd/ps/pci-ps.c b/sound/soc/amd/ps/pci-ps.c
index 688a1d4643d9..afddb9a77ba4 100644
--- a/sound/soc/amd/ps/pci-ps.c
+++ b/sound/soc/amd/ps/pci-ps.c
@@ -247,11 +247,17 @@ static int snd_acp63_probe(struct pci_dev *pci,
{
struct acp63_dev_data *adata;
u32 addr;
- u32 irqflags;
+ u32 irqflags, flag;
int val;
int ret;

irqflags = IRQF_SHARED;
+
+ /* Return if acp config flag is defined */
+ flag = snd_amd_acp_find_config(pci);
+ if (flag)
+ return -ENODEV;
+
/* Pink Sardine device check */
switch (pci->revision) {
case 0x63:
diff --git a/sound/soc/amd/yc/acp6x.h b/sound/soc/amd/yc/acp6x.h
index 036207568c04..2de7d1edf00b 100644
--- a/sound/soc/amd/yc/acp6x.h
+++ b/sound/soc/amd/yc/acp6x.h
@@ -105,3 +105,6 @@ static inline void acp6x_writel(u32 val, void __iomem *base_addr)
{
writel(val, base_addr - ACP6x_PHY_BASE_ADDRESS);
}
+
+int snd_amd_acp_find_config(struct pci_dev *pci);
+
diff --git a/sound/soc/amd/yc/pci-acp6x.c b/sound/soc/amd/yc/pci-acp6x.c
index 77c5fa1f7af1..7af6a349b1d4 100644
--- a/sound/soc/amd/yc/pci-acp6x.c
+++ b/sound/soc/amd/yc/pci-acp6x.c
@@ -149,10 +149,16 @@ static int snd_acp6x_probe(struct pci_dev *pci,
int index = 0;
int val = 0x00;
u32 addr;
- unsigned int irqflags;
+ unsigned int irqflags, flag;
int ret;

irqflags = IRQF_SHARED;
+
+ /* Return if acp config flag is defined */
+ flag = snd_amd_acp_find_config(pci);
+ if (flag)
+ return -ENODEV;
+
/* Yellow Carp device check */
switch (pci->revision) {
case 0x60:
--
2.25.1


2023-04-12 15:03:01

by Mario Limonciello

[permalink] [raw]
Subject: Re: [PATCH] ASoC: amd: Add check for acp config flags

On 4/12/2023 04:16, Syed Saba Kareem wrote:
> We have SOF and generic ACP support enabled for Rembrandt and
> pheonix platforms on some machines. Since we have same PCI id

s,pheonix,Phoenix,
s,have same,have the same,

> used for probing, add check for machine configuration flag to

You should mention here that the PCI ID is the same but the revision
is different to make this clearer for the commit message.

> avoid conflict with newer pci drivers. Such machine flag has
> been initialized via dmi match on few Chrome machines. If no
> flag is specified probe and register older platform device.
>
> Signed-off-by: Syed Saba Kareem <[email protected]>
> Reviewed-by: Vijendar Mukunda <[email protected]>
> ---
> sound/soc/amd/Kconfig | 2 ++
> sound/soc/amd/ps/acp63.h | 2 ++
> sound/soc/amd/ps/pci-ps.c | 8 +++++++-
> sound/soc/amd/yc/acp6x.h | 3 +++
> sound/soc/amd/yc/pci-acp6x.c | 8 +++++++-
> 5 files changed, 21 insertions(+), 2 deletions(-)
>
> diff --git a/sound/soc/amd/Kconfig b/sound/soc/amd/Kconfig
> index c88ebd84bdd5..08e42082f5e9 100644
> --- a/sound/soc/amd/Kconfig
> +++ b/sound/soc/amd/Kconfig
> @@ -90,6 +90,7 @@ config SND_SOC_AMD_VANGOGH_MACH
>
> config SND_SOC_AMD_ACP6x
> tristate "AMD Audio Coprocessor-v6.x Yellow Carp support"
> + select SND_AMD_ACP_CONFIG
> depends on X86 && PCI
> help
> This option enables Audio Coprocessor i.e ACP v6.x support on
> @@ -130,6 +131,7 @@ config SND_SOC_AMD_RPL_ACP6x
>
> config SND_SOC_AMD_PS
> tristate "AMD Audio Coprocessor-v6.3 Pink Sardine support"
> + select SND_AMD_ACP_CONFIG

Whitespace looks not aligned here. I think it's a mix of tabs and
spaces in this section.

> depends on X86 && PCI && ACPI
> help
> This option enables Audio Coprocessor i.e ACP v6.3 support on
> diff --git a/sound/soc/amd/ps/acp63.h b/sound/soc/amd/ps/acp63.h
> index 6bf29b520511..dd36790b25ae 100644
> --- a/sound/soc/amd/ps/acp63.h
> +++ b/sound/soc/amd/ps/acp63.h
> @@ -111,3 +111,5 @@ struct acp63_dev_data {
> u16 pdev_count;
> u16 pdm_dev_index;
> };
> +
> +int snd_amd_acp_find_config(struct pci_dev *pci);
> diff --git a/sound/soc/amd/ps/pci-ps.c b/sound/soc/amd/ps/pci-ps.c
> index 688a1d4643d9..afddb9a77ba4 100644
> --- a/sound/soc/amd/ps/pci-ps.c
> +++ b/sound/soc/amd/ps/pci-ps.c
> @@ -247,11 +247,17 @@ static int snd_acp63_probe(struct pci_dev *pci,
> {
> struct acp63_dev_data *adata;
> u32 addr;
> - u32 irqflags;
> + u32 irqflags, flag; > int val;
> int ret;
>
> irqflags = IRQF_SHARED;
> +
> + /* Return if acp config flag is defined */
> + flag = snd_amd_acp_find_config(pci);

This 'flag' variable seems unnecessary if it's just used in one place
and never evaluated.

> + if (flag)
> + return -ENODEV;
> +
> /* Pink Sardine device check */
> switch (pci->revision) {
> case 0x63:
> diff --git a/sound/soc/amd/yc/acp6x.h b/sound/soc/amd/yc/acp6x.h
> index 036207568c04..2de7d1edf00b 100644
> --- a/sound/soc/amd/yc/acp6x.h
> +++ b/sound/soc/amd/yc/acp6x.h
> @@ -105,3 +105,6 @@ static inline void acp6x_writel(u32 val, void __iomem *base_addr)
> {
> writel(val, base_addr - ACP6x_PHY_BASE_ADDRESS);
> }
> +
> +int snd_amd_acp_find_config(struct pci_dev *pci);
> +
> diff --git a/sound/soc/amd/yc/pci-acp6x.c b/sound/soc/amd/yc/pci-acp6x.c
> index 77c5fa1f7af1..7af6a349b1d4 100644
> --- a/sound/soc/amd/yc/pci-acp6x.c
> +++ b/sound/soc/amd/yc/pci-acp6x.c
> @@ -149,10 +149,16 @@ static int snd_acp6x_probe(struct pci_dev *pci,
> int index = 0;
> int val = 0x00;
> u32 addr;
> - unsigned int irqflags;
> + unsigned int irqflags, flag;
> int ret;
>
> irqflags = IRQF_SHARED;
> +
> + /* Return if acp config flag is defined */
> + flag = snd_amd_acp_find_config(pci);

This 'flag' variable seems unnecessary if it's just used in one place
and never evaluated.

> + if (flag)
> + return -ENODEV;
> +
> /* Yellow Carp device check */
> switch (pci->revision) {
> case 0x60:

2023-04-12 16:50:39

by Mark Brown

[permalink] [raw]
Subject: Re: [PATCH] ASoC: amd: Add check for acp config flags

On Wed, 12 Apr 2023 14:46:16 +0530, Syed Saba Kareem wrote:
> We have SOF and generic ACP support enabled for Rembrandt and
> pheonix platforms on some machines. Since we have same PCI id
> used for probing, add check for machine configuration flag to
> avoid conflict with newer pci drivers. Such machine flag has
> been initialized via dmi match on few Chrome machines. If no
> flag is specified probe and register older platform device.
>
> [...]

Applied to

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

Thanks!

[1/1] ASoC: amd: Add check for acp config flags
commit: bddcfb0802eb69b0f51293eab5db33d344c0262f

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

2023-04-13 05:47:22

by syed saba kareem

[permalink] [raw]
Subject: Re: [PATCH] ASoC: amd: Add check for acp config flags


On 4/12/23 20:30, Limonciello, Mario wrote:
> On 4/12/2023 04:16, Syed Saba Kareem wrote:
>> We have SOF and generic ACP support enabled for Rembrandt and
>> pheonix platforms on some machines. Since we have same PCI id
>
> s,pheonix,Phoenix,
> s,have same,have the same,
>
>> used for probing, add check for machine configuration flag to
>
> You should mention here that the PCI ID is the same but the revision
> is different to make this clearer for the commit message.
>
>> avoid conflict with newer pci drivers. Such machine flag has
>> been initialized via dmi match on few Chrome machines. If no
>> flag is specified probe and register older platform device.
>>
>> Signed-off-by: Syed Saba Kareem <[email protected]>
>> Reviewed-by: Vijendar Mukunda <[email protected]>
>> ---
>>   sound/soc/amd/Kconfig        | 2 ++
>>   sound/soc/amd/ps/acp63.h     | 2 ++
>>   sound/soc/amd/ps/pci-ps.c    | 8 +++++++-
>>   sound/soc/amd/yc/acp6x.h     | 3 +++
>>   sound/soc/amd/yc/pci-acp6x.c | 8 +++++++-
>>   5 files changed, 21 insertions(+), 2 deletions(-)
>>
>> diff --git a/sound/soc/amd/Kconfig b/sound/soc/amd/Kconfig
>> index c88ebd84bdd5..08e42082f5e9 100644
>> --- a/sound/soc/amd/Kconfig
>> +++ b/sound/soc/amd/Kconfig
>> @@ -90,6 +90,7 @@ config SND_SOC_AMD_VANGOGH_MACH
>>     config SND_SOC_AMD_ACP6x
>>       tristate "AMD Audio Coprocessor-v6.x Yellow Carp support"
>> +    select SND_AMD_ACP_CONFIG
>>       depends on X86 && PCI
>>       help
>>         This option enables Audio Coprocessor i.e ACP v6.x support on
>> @@ -130,6 +131,7 @@ config SND_SOC_AMD_RPL_ACP6x
>>     config SND_SOC_AMD_PS
>>           tristate "AMD Audio Coprocessor-v6.3 Pink Sardine support"
>> +    select SND_AMD_ACP_CONFIG
>
> Whitespace looks not aligned here.  I think it's a mix of tabs and
> spaces in this section.
> It is aligned ,may be due to addition of plus symbol in the patch it
> is looking like misaligned.
>>           depends on X86 && PCI && ACPI
>>           help
>>             This option enables Audio Coprocessor i.e ACP v6.3
>> support on
>> diff --git a/sound/soc/amd/ps/acp63.h b/sound/soc/amd/ps/acp63.h
>> index 6bf29b520511..dd36790b25ae 100644
>> --- a/sound/soc/amd/ps/acp63.h
>> +++ b/sound/soc/amd/ps/acp63.h
>> @@ -111,3 +111,5 @@ struct acp63_dev_data {
>>       u16 pdev_count;
>>       u16 pdm_dev_index;
>>   };
>> +
>> +int snd_amd_acp_find_config(struct pci_dev *pci);
>> diff --git a/sound/soc/amd/ps/pci-ps.c b/sound/soc/amd/ps/pci-ps.c
>> index 688a1d4643d9..afddb9a77ba4 100644
>> --- a/sound/soc/amd/ps/pci-ps.c
>> +++ b/sound/soc/amd/ps/pci-ps.c
>> @@ -247,11 +247,17 @@ static int snd_acp63_probe(struct pci_dev *pci,
>>   {
>>       struct acp63_dev_data *adata;
>>       u32 addr;
>> -    u32 irqflags;
>> +    u32 irqflags, flag; >       int val;
>>       int ret;
>>         irqflags = IRQF_SHARED;
>> +
>> +    /* Return if acp config flag is defined */
>> +    flag = snd_amd_acp_find_config(pci);
>
> This 'flag' variable seems unnecessary if it's just used in one place
> and never evaluated.
>
>> +    if (flag)
>> +        return -ENODEV;
>> +
>>       /* Pink Sardine device check */
>>       switch (pci->revision) {
>>       case 0x63:
>> diff --git a/sound/soc/amd/yc/acp6x.h b/sound/soc/amd/yc/acp6x.h
>> index 036207568c04..2de7d1edf00b 100644
>> --- a/sound/soc/amd/yc/acp6x.h
>> +++ b/sound/soc/amd/yc/acp6x.h
>> @@ -105,3 +105,6 @@ static inline void acp6x_writel(u32 val, void
>> __iomem *base_addr)
>>   {
>>       writel(val, base_addr - ACP6x_PHY_BASE_ADDRESS);
>>   }
>> +
>> +int snd_amd_acp_find_config(struct pci_dev *pci);
>> +
>> diff --git a/sound/soc/amd/yc/pci-acp6x.c b/sound/soc/amd/yc/pci-acp6x.c
>> index 77c5fa1f7af1..7af6a349b1d4 100644
>> --- a/sound/soc/amd/yc/pci-acp6x.c
>> +++ b/sound/soc/amd/yc/pci-acp6x.c
>> @@ -149,10 +149,16 @@ static int snd_acp6x_probe(struct pci_dev *pci,
>>       int index = 0;
>>       int val = 0x00;
>>       u32 addr;
>> -    unsigned int irqflags;
>> +    unsigned int irqflags, flag;
>>       int ret;
>>         irqflags = IRQF_SHARED;
>> +
>> +    /* Return if acp config flag is defined */
>> +    flag = snd_amd_acp_find_config(pci);
>
> This 'flag' variable seems unnecessary if it's just used in one place
> and never evaluated.
>
>> +    if (flag)
>> +        return -ENODEV;
>> +
>>       /* Yellow Carp device check */
>>       switch (pci->revision) {
>>       case 0x60:
>