[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <1e2c7dd9-0c28-17eb-b166-3bfaf65bbfe4@linux.intel.com>
Date: Mon, 1 Aug 2022 10:25:14 -0500
From: Pierre-Louis Bossart <pierre-louis.bossart@...ux.intel.com>
To: Stefan Binding <sbinding@...nsource.cirrus.com>,
Mark Brown <broonie@...nel.org>,
Liam Girdwood <lgirdwood@...il.com>,
Brent Lu <brent.lu@...el.com>, xliu <xiang.liu@...rus.com>
Cc: Vitaly Rodionov <vitalyr@...nsource.cirrus.com>,
patches@...nsource.cirrus.com, alsa-devel@...a-project.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3] ASoC: Intel: cirrus-common: Use UID to map correct amp
to prefix
On 8/1/22 04:40, Stefan Binding wrote:
> Since the order of the amps in the ACPI determines the device name,
> and the ACPI order may change depending on hardware configuration,
> use UID to dynamically compute the dai links, allowing dynamic
> assignment of the name_prefix.
>
> The UIDs for these amps in ACPI are fixed, and map to a name_prefix,
> where:
> UID 0x0 -> WL
> UID 0x1 -> WR
> UID 0x2 -> TL
> UID 0x3 -> TR
>
> Signed-off-by: Stefan Binding <sbinding@...nsource.cirrus.com>
> Signed-off-by: Vitaly Rodionov <vitalyr@...nsource.cirrus.com>
Thanks for the updates, LGTM
Acked-by: Pierre-Louis Bossart <pierre-louis.bossart@...ux.intel.com>
> ---
> sound/soc/intel/boards/sof_cirrus_common.c | 92 ++++++++++++----------
> 1 file changed, 49 insertions(+), 43 deletions(-)
>
> diff --git a/sound/soc/intel/boards/sof_cirrus_common.c b/sound/soc/intel/boards/sof_cirrus_common.c
> index f4192df962d6..6e39eda77385 100644
> --- a/sound/soc/intel/boards/sof_cirrus_common.c
> +++ b/sound/soc/intel/boards/sof_cirrus_common.c
> @@ -10,6 +10,9 @@
> #include "../../codecs/cs35l41.h"
> #include "sof_cirrus_common.h"
>
> +#define CS35L41_HID "CSC3541"
> +#define CS35L41_MAX_AMPS 4
> +
> /*
> * Cirrus Logic CS35L41/CS35L53
> */
> @@ -35,50 +38,12 @@ static const struct snd_soc_dapm_route cs35l41_dapm_routes[] = {
> {"TR Spk", NULL, "TR SPK"},
> };
>
> -static struct snd_soc_dai_link_component cs35l41_components[] = {
> - {
> - .name = CS35L41_DEV0_NAME,
> - .dai_name = CS35L41_CODEC_DAI,
> - },
> - {
> - .name = CS35L41_DEV1_NAME,
> - .dai_name = CS35L41_CODEC_DAI,
> - },
> - {
> - .name = CS35L41_DEV2_NAME,
> - .dai_name = CS35L41_CODEC_DAI,
> - },
> - {
> - .name = CS35L41_DEV3_NAME,
> - .dai_name = CS35L41_CODEC_DAI,
> - },
> -};
> +static struct snd_soc_dai_link_component cs35l41_components[CS35L41_MAX_AMPS];
>
> /*
> * Mapping between ACPI instance id and speaker position.
> - *
> - * Four speakers:
> - * 0: Tweeter left, 1: Woofer left
> - * 2: Tweeter right, 3: Woofer right
> */
> -static struct snd_soc_codec_conf cs35l41_codec_conf[] = {
> - {
> - .dlc = COMP_CODEC_CONF(CS35L41_DEV0_NAME),
> - .name_prefix = "TL",
> - },
> - {
> - .dlc = COMP_CODEC_CONF(CS35L41_DEV1_NAME),
> - .name_prefix = "WL",
> - },
> - {
> - .dlc = COMP_CODEC_CONF(CS35L41_DEV2_NAME),
> - .name_prefix = "TR",
> - },
> - {
> - .dlc = COMP_CODEC_CONF(CS35L41_DEV3_NAME),
> - .name_prefix = "WR",
> - },
> -};
> +static struct snd_soc_codec_conf cs35l41_codec_conf[CS35L41_MAX_AMPS];
>
> static int cs35l41_init(struct snd_soc_pcm_runtime *rtd)
> {
> @@ -117,10 +82,10 @@ static int cs35l41_init(struct snd_soc_pcm_runtime *rtd)
> static const struct {
> unsigned int rx[2];
> } cs35l41_channel_map[] = {
> - {.rx = {0, 1}}, /* TL */
> {.rx = {0, 1}}, /* WL */
> - {.rx = {1, 0}}, /* TR */
> {.rx = {1, 0}}, /* WR */
> + {.rx = {0, 1}}, /* TL */
> + {.rx = {1, 0}}, /* TR */
> };
>
> static int cs35l41_hw_params(struct snd_pcm_substream *substream,
> @@ -175,10 +140,51 @@ static const struct snd_soc_ops cs35l41_ops = {
> .hw_params = cs35l41_hw_params,
> };
>
> +static const char * const cs35l41_name_prefixes[] = { "WL", "WR", "TL", "TR" };
> +
> +/*
> + * Expected UIDs are integers (stored as strings).
> + * UID Mapping is fixed:
> + * UID 0x0 -> WL
> + * UID 0x1 -> WR
> + * UID 0x2 -> TL
> + * UID 0x3 -> TR
> + * Note: If there are less than 4 Amps, UIDs still map to WL/WR/TL/TR. Dynamic code will only create
> + * dai links for UIDs which exist, and ignore non-existant ones. Only 2 or 4 amps are expected.
> + * Return number of codecs found.
> + */
> +static int cs35l41_compute_codec_conf(void)
> +{
> + const char * const uid_strings[] = { "0", "1", "2", "3" };
> + unsigned int uid, sz = 0;
> + struct acpi_device *adev;
> + struct device *physdev;
> +
> + for (uid = 0; uid < CS35L41_MAX_AMPS; uid++) {
> + adev = acpi_dev_get_first_match_dev(CS35L41_HID, uid_strings[uid], -1);
> + if (!adev) {
> + pr_devel("Cannot find match for HID %s UID %u (%s)\n", CS35L41_HID, uid,
> + cs35l41_name_prefixes[uid]);
> + continue;
> + }
> + physdev = get_device(acpi_get_first_physical_node(adev));
> + cs35l41_components[sz].name = dev_name(physdev);
> + cs35l41_components[sz].dai_name = CS35L41_CODEC_DAI;
> + cs35l41_codec_conf[sz].dlc.name = dev_name(physdev);
> + cs35l41_codec_conf[sz].name_prefix = cs35l41_name_prefixes[uid];
> + acpi_dev_put(adev);
> + sz++;
> + }
> +
> + if (sz != 2 && sz != 4)
> + pr_warn("Invalid number of cs35l41 amps found: %d, expected 2 or 4\n", sz);
> + return sz;
> +}
> +
> void cs35l41_set_dai_link(struct snd_soc_dai_link *link)
> {
> + link->num_codecs = cs35l41_compute_codec_conf();
> link->codecs = cs35l41_components;
> - link->num_codecs = ARRAY_SIZE(cs35l41_components);
> link->init = cs35l41_init;
> link->ops = &cs35l41_ops;
> }
Powered by blists - more mailing lists