Messages in this thread | | | Date | Thu, 13 Apr 2023 11:02:17 +0530 | Subject | Re: [PATCH] ASoC: amd: Add check for acp config flags | From | syed saba kareem <> |
| |
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 <Syed.SabaKareem@amd.com> >> Reviewed-by: Vijendar Mukunda <Vijendar.Mukunda@amd.com> >> --- >> 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: >
| |