linux-kernel - Re: [PATCH] [v2] ASoC: Intel: sof_cs42l42: adding support for ADL configuration and BT offload audio

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>] [day] [month] [year] [list]

Message-ID: <a71c4a2c-06f9-faa7-07ee-783ee7f136ec@linux.intel.com>
Date:   Wed, 11 May 2022 09:02:58 -0500
From:   Pierre-Louis Bossart <pierre-louis.bossart@...ux.intel.com>
To:     Terry Chen <terry_chen@...tron.corp-partner.google.com>
Cc:     alsa-devel@...a-project.org, cezary.rojewski@...el.com,
        liam.r.girdwood@...ux.intel.com, yang.jie@...ux.intel.com,
        broonie@...nel.org, perex@...ex.cz, tiwai@...e.com,
        brent.lu@...el.com, cujomalainey@...omium.org,
        Sean Paul <seanpaul@...omium.org>, casey.g.bowman@...el.com,
        Mark Hsieh <mark_hsieh@...tron.corp-partner.google.com>,
        vamshi.krishna.gopal@...el.com, Mac Chiang <mac.chiang@...el.com>,
        kai.vehmanen@...ux.intel.com, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] [v2] ASoC: Intel: sof_cs42l42: adding support for ADL
 configuration and BT offload audio



On 5/11/22 01:33, Terry Chen wrote:
> Hi Pierre-Louis
> 
>> @@ -522,6 +578,14 @@ static struct snd_soc_dai_link *sof_card_dai_links_create(struct device *dev,
>>                               goto devm_err;
>>                       }
>>                       break;
>> +             case LINK_BT:
>> +                     ret = create_bt_offload_dai_links(dev, links, cpus, &id, ssp_bt);
>> +                     if (ret < 0) {
>> +                             dev_err(dev, "fail to create bt offload dai links, ret %d\n",
>> +                                     ret);
> 
> For this point, we just follow Intel member to write for this coding
> style. The other component also was the same style.

the magic of copy-paste, eh? Please update this, thanks.

>     > @@ -384,6 +384,14 @@ struct snd_soc_acpi_mach
>     snd_soc_acpi_intel_adl_machines[] = {
>     >               .sof_fw_filename = "sof-adl.ri",
>     >               .sof_tplg_filename = "sof-adl-cs35l41.tplg",
>     >       },
>     > +     {
>     > +             .id = "10134242",
>     > +             .drv_name = "adl_mx98360a_cs4242",
>     > +             .machine_quirk = snd_soc_acpi_codec_list,
>     > +             .quirk_data = &adl_max98360a_amp,
>     > +             .sof_fw_filename = "sof-adl.ri",
> 
>     This  also was the same style with others.

No, it's not a matter of style but rather that this field was *REMOVED*,
this cannot possibly compile.

see commit a6264056b39ee ("ASoC: soc-acpi: remove sof_fw_filename
")

If you had submitted this patch through the SOF tree, you would have
seen a compilation error.

> 
>     > +             .sof_tplg_filename = "sof-adl-max98360a-rt5682.tplg",
> 
>     Why would you refer to a topology that uses a different codec?
> 
> 
>  Because Intel college use the same naming style for the same audio codec.

It's bad practice to use the same topology name for different platforms
based on different codecs. One evolution of the topology would impact an
unrelated platform. Please use a symlink or duplicate the topology with
a different name, this is not future-proof and will be problematic for
releases.