[v6] FROMLIST: ASoC: Intel: sof_cs42l42: adding support for ADL configuration and BT offload audio

[Terry Chen]

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 at linux.intel.com> 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 at wistron.corp-partner.google.com>
> > (am from https://patchwork.kernel.org/patch/12845884/)
> > (also found at
> https://lore.kernel.org/r/20220511075522.1764114-1-terry_chen@wistron.corp-partner.google.com
> )
>
> 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.
>