Hi Pierre We upload v7 patch to follow the coding style as other components. Thanks On Wed, May 18, 2022 at 10:02 AM Pierre-Louis Bossart < pierre-louis.bossart@xxxxxxxxxxxxxxx> wrote: > > > On 5/17/22 20:31, Terry Chen wrote: > > To be able to do driver data for adl_mx98360a_cs4242 which supports > > two max98360a speaker amplifiers on SSP1 and cs42l42 headphone codec > > on SSP0 running on ADL platform. Also add the capability to machine > driver > > of creating DAI Link for BT offload. Although BT offload always uses SSP2 > > port but we reserve the flexibility to assign the port number in macro. > > > > Signed-off-by: Terry Chen <terry_chen@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx> > > (am from https://patchwork.kernel.org/patch/12845884/) > > (also found at > https://lore.kernel.org/r/20220511075522.1764114-1-terry_chen@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx > ) > > not sure what this is about, what's the point of adding information on > the v5 in the v6 patch? > > > > > --- > > sound/soc/intel/boards/sof_cs42l42.c | 92 ++++++++++++++++++- > > .../intel/common/soc-acpi-intel-adl-match.c | 7 ++ > > 2 files changed, 95 insertions(+), 4 deletions(-) > > > > diff --git a/sound/soc/intel/boards/sof_cs42l42.c > b/sound/soc/intel/boards/sof_cs42l42.c > > index ce78c18798876..2efffc7933479 100644 > > --- a/sound/soc/intel/boards/sof_cs42l42.c > > +++ b/sound/soc/intel/boards/sof_cs42l42.c > > @@ -41,8 +41,13 @@ > > #define SOF_CS42L42_DAILINK_MASK (GENMASK(24, 10)) > > #define SOF_CS42L42_DAILINK(link1, link2, link3, link4, link5) \ > > ((((link1) | ((link2) << 3) | ((link3) << 6) | ((link4) << 9) | > ((link5) << 12)) << SOF_CS42L42_DAILINK_SHIFT) & SOF_CS42L42_DAILINK_MASK) > > -#define SOF_MAX98357A_SPEAKER_AMP_PRESENT BIT(25) > > -#define SOF_MAX98360A_SPEAKER_AMP_PRESENT BIT(26) > > +#define SOF_BT_OFFLOAD_PRESENT BIT(25) > > +#define SOF_CS42L42_SSP_BT_SHIFT 26 > > +#define SOF_CS42L42_SSP_BT_MASK (GENMASK(28, 26)) > > +#define SOF_CS42L42_SSP_BT(quirk) \ > > + (((quirk) << SOF_CS42L42_SSP_BT_SHIFT) & SOF_CS42L42_SSP_BT_MASK) > > +#define SOF_MAX98357A_SPEAKER_AMP_PRESENT BIT(29) > > +#define SOF_MAX98360A_SPEAKER_AMP_PRESENT BIT(30) > > > > enum { > > LINK_NONE = 0, > > @@ -50,6 +55,7 @@ enum { > > LINK_SPK = 2, > > LINK_DMIC = 3, > > LINK_HDMI = 4, > > + LINK_BT = 5, > > }; > > > > /* Default: SSP2 */ > > @@ -278,6 +284,13 @@ static struct snd_soc_dai_link_component > dmic_component[] = { > > } > > }; > > > > +static struct snd_soc_dai_link_component dummy_component[] = { > > + { > > + .name = "snd-soc-dummy", > > + .dai_name = "snd-soc-dummy-dai", > > + } > > +}; > > + > > static int create_spk_amp_dai_links(struct device *dev, > > struct snd_soc_dai_link *links, > > struct snd_soc_dai_link_component > *cpus, > > @@ -467,9 +480,56 @@ static int create_hdmi_dai_links(struct device *dev, > > return -ENOMEM; > > } > > > > +static int create_bt_offload_dai_links(struct device *dev, > > + struct snd_soc_dai_link *links, > > + struct snd_soc_dai_link_component > *cpus, > > + int *id, int ssp_bt) > > +{ > > + int ret = 0; > > either you remove this useless init... > > > + > > + /* bt offload */ > > + if (!(sof_cs42l42_quirk & SOF_BT_OFFLOAD_PRESENT)) > > + return 0; > > + > > + links[*id].name = devm_kasprintf(dev, GFP_KERNEL, "SSP%d-BT", > > + ssp_bt); > > + if (!links[*id].name) { > > + ret = -ENOMEM; > > + goto devm_err; > > + } > > + > > + links[*id].id = *id; > > + links[*id].codecs = dummy_component; > > + links[*id].num_codecs = ARRAY_SIZE(dummy_component); > > + links[*id].platforms = platform_component; > > + links[*id].num_platforms = ARRAY_SIZE(platform_component); > > + > > + links[*id].dpcm_playback = 1; > > + links[*id].dpcm_capture = 1; > > + links[*id].no_pcm = 1; > > + links[*id].cpus = &cpus[*id]; > > + links[*id].num_cpus = 1; > > + > > + links[*id].cpus->dai_name = devm_kasprintf(dev, GFP_KERNEL, > > + "SSP%d Pin", > > + ssp_bt); > > + if (!links[*id].cpus->dai_name) { > > + ret = -ENOMEM; > > + goto devm_err; > > + } > > + > > + (*id)++; > > + > > + return 0; > > ... or you remove the return 0; > > pick one. > > > + > > +devm_err: > > + return ret; > > +} > > + > > > }, > > + { > > + .id = "10134242", > > + .drv_name = "adl_mx98360a_cs4242", > > + .machine_quirk = snd_soc_acpi_codec_list, > > + .quirk_data = &adl_max98360a_amp, > > + .sof_tplg_filename = "sof-adl-max98360a-rt5682.tplg", > > No, I've told this before in previous reviews: do not use a topology > name that was designed for a different platform, this is not > maintainable. If the topologies happen to be the same, either generate > them twice or use a symlink. >
