linux-kernel - Re: [PATCH] ASoC: amd: Add check for acp config flags

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Date:   Thu, 13 Apr 2023 11:02:17 +0530
From:   syed saba kareem <ssabakar@....com>
To:     "Limonciello, Mario" <mario.limonciello@....com>,
        Syed Saba Kareem <Syed.SabaKareem@....com>,
        broonie@...nel.org, alsa-devel@...a-project.org
Cc:     Vijendar.Mukunda@....com, Basavaraj.Hiregoudar@....com,
        Sunil-kumar.Dommati@....com, venkataprasad.potturu@....com,
        vsujithkumar.reddy@....com, Liam Girdwood <lgirdwood@...il.com>,
        Jaroslav Kysela <perex@...ex.cz>,
        Takashi Iwai <tiwai@...e.com>,
        Randy Dunlap <rdunlap@...radead.org>,
        Pierre-Louis Bossart <pierre-louis.bossart@...ux.intel.com>,
        Nathan Chancellor <nathan@...nel.org>,
        open list <linux-kernel@...r.kernel.org>
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 <Syed.SabaKareem@....com>
>> Reviewed-by: Vijendar Mukunda <Vijendar.Mukunda@....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:
>