linux-kernel - Re: [PATCH v3] ASoC: Intel: cirrus-common: Use UID to map correct amp to prefix

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] [thread-next>] [day] [month] [year] [list]
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;
>  }