Re: [’PATCH’ 2/3] ASoC: codecs: add ne w SoundWire-based SN624x

Pierre-Louis Bossart <[email protected]>
Newsgroups org.kernel.vger.linux-sound
Message-ID <[email protected]> 
> +static const struct snd_soc_dapm_route sn6242_sdca_map[] = {
> +	{ "Headphone", NULL, "sn6242 HP" },
> +	{ "sn6242 MIC2", NULL, "Headset Mic" },
> +};
> +
> +static struct snd_soc_jack_pin senary_sdca_jack_pins[] = {
> +	{
> +		.pin    = "Headphone",
> +		.mask   = SND_JACK_HEADPHONE,
> +	},
> +	{
> +		.pin    = "Headset Mic",
> +		.mask   = SND_JACK_MICROPHONE,
> +	},
> +};
> +
> +static const char * const need_sdca_suffix[] = {
> +	"sn6242", "sn6244", "sn6247"
> +};
> +
> +int asoc_sdw_senary_sdca_jack_rtd_init(struct snd_soc_pcm_runtime *rtd, struct snd_soc_dai *dai)
> +{
> +	struct snd_soc_card *card = rtd->card;
> +	struct snd_soc_dapm_context *dapm = snd_soc_card_to_dapm(card);
> +	struct asoc_sdw_mc_private *ctx = snd_soc_card_get_drvdata(card);
> +	struct snd_soc_component *component;
> +	struct snd_soc_jack *jack;
> +	int ret;
> +	int i;
> +
> +	component = dai->component;
> +	card->components = devm_kasprintf(card->dev, GFP_KERNEL,
> +					  "%s hs:%s",
> +					  card->components, component->name_prefix);
> +	if (!card->components)
> +		return -ENOMEM;
> +
> +	for (i = 0; i < ARRAY_SIZE(need_sdca_suffix); i++) {
> +		if (strstr(component->name_prefix, need_sdca_suffix[i])) {
> +			/* Add -sdca suffix for existing UCMs */
> +			card->components = devm_kasprintf(card->dev, GFP_KERNEL,
> +							  "%s-sdca", card->components);
> +			if (!card->components)
> +				return -ENOMEM;
> +			break;
> +		}
> +	}
> +
> +	if (strstr(component->name_prefix, "sn6242")) {
> +		ret = snd_soc_dapm_add_routes(dapm, sn6242_sdca_map,
> +					      ARRAY_SIZE(sn6242_sdca_map));
> +	} else if (strstr(component->name_prefix, "sn6244")) {
> +		ret = snd_soc_dapm_add_routes(dapm, sn6242_sdca_map,
> +					      ARRAY_SIZE(sn6242_sdca_map));
> +	} else if (strstr(component->name_prefix, "sn6247")) {
> +		ret = snd_soc_dapm_add_routes(dapm, sn6242_sdca_map,
> +					      ARRAY_SIZE(sn6242_sdca_map));

looks like all branches do the same thing, consider refactoring all this...

> +	} else {
> +		dev_err(card->dev, "%s is not supported\n", component->name_prefix);
> +		return -EINVAL;
> +	}
> +
> +	if (ret) {
> +		dev_err(card->dev, "senary sdca jack map addition failed: %d\n", ret);
> +		return ret;
> +	}
> +
> +	ret = snd_soc_card_jack_new_pins(rtd->card, "Headset Jack",
> +					 SND_JACK_HEADSET | SND_JACK_BTN_0 |
> +					 SND_JACK_BTN_1 | SND_JACK_BTN_2 |
> +					 SND_JACK_BTN_3,
> +					 &ctx->sdw_headset,
> +					 senary_sdca_jack_pins,
> +					 ARRAY_SIZE(senary_sdca_jack_pins));
> +	if (ret) {
> +		dev_err(rtd->card->dev, "Headset Jack creation failed: %d\n",
> +			ret);
> +		return ret;
> +	}
> +
> +	jack = &ctx->sdw_headset;
> +
> +	snd_jack_set_key(jack->jack, SND_JACK_BTN_0, KEY_PLAYPAUSE);
> +	snd_jack_set_key(jack->jack, SND_JACK_BTN_1, KEY_VOICECOMMAND);
> +	snd_jack_set_key(jack->jack, SND_JACK_BTN_2, KEY_VOLUMEUP);
> +	snd_jack_set_key(jack->jack, SND_JACK_BTN_3, KEY_VOLUMEDOWN);
> +
> +	ret = snd_soc_component_set_jack(component, jack, NULL);
> +
> +	if (ret)
> +		dev_err(rtd->card->dev, "Headset Jack call-back failed: %d\n",
> +			ret);
> +
> +	return ret;
> +}
> +EXPORT_SYMBOL_NS(asoc_sdw_senary_sdca_jack_rtd_init, "SND_SOC_SDW_UTILS");
> +
> +int asoc_sdw_senary_sdca_jack_exit(struct snd_soc_card *card, struct snd_soc_dai_link *dai_link)
> +{
> +	struct asoc_sdw_mc_private *ctx = snd_soc_card_get_drvdata(card);
> +
> +	if (!ctx->headset_codec_dev)
> +		return 0;
> +
> +	if (!SOC_SDW_JACK_JDSRC(ctx->mc_quirk))
> +		return 0;
> +
> +	device_remove_software_node(ctx->headset_codec_dev);
> +	put_device(ctx->headset_codec_dev);
> +	ctx->headset_codec_dev = NULL;
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_NS(asoc_sdw_senary_sdca_jack_exit, "SND_SOC_SDW_UTILS");
> +
> +int asoc_sdw_senary_sdca_jack_init(struct snd_soc_card *card,
> +			       struct snd_soc_dai_link *dai_links,
> +			       struct asoc_sdw_codec_info *info,
> +			       bool playback)
> +{
> +	struct asoc_sdw_mc_private *ctx = snd_soc_card_get_drvdata(card);
> +	struct device *sdw_dev;
> +	int ret;
> +
> +	/*
> +	 * Jack detection should be only initialized once for headsets since
> +	 * the playback/capture is sharing the same jack
> +	 */
> +	if (ctx->headset_codec_dev)
> +		return 0;
> +
> +	sdw_dev = bus_find_device_by_name(&sdw_bus_type, NULL, dai_links->codecs[0].name);
> +	if (!sdw_dev)
> +		return -EPROBE_DEFER;
> +
> +	ret = senary_sdca_jack_add_codec_device_props(sdw_dev, ctx->mc_quirk);
> +	if (ret < 0) {
> +		put_device(sdw_dev);
> +		return ret;
> +	}
> +	ctx->headset_codec_dev = sdw_dev;
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_NS(asoc_sdw_senary_sdca_jack_init, "SND_SOC_SDW_UTILS");

quite a few lines from this file are just copy-pasted, is it time to try
and share these helpers?

> diff --git a/sound/soc/sdw_utils/soc_sdw_utils.c b/sound/soc/sdw_utils/soc_sdw_utils.c
> index d8db8fc5313e..603e57a4f09c 100644
> --- a/sound/soc/sdw_utils/soc_sdw_utils.c
> +++ b/sound/soc/sdw_utils/soc_sdw_utils.c
> @@ -1228,6 +1228,148 @@ struct asoc_sdw_codec_info codec_info_list[] = {
>  		},
>  		.dai_num = 1,
>  	},
> +	{
> +		.part_id = 0x6244,
> +		.name_prefix = "sn6242",
> +		.ignore_internal_dmic = true,
> +		.dais = {
> +			{
> +				.direction = {true, true},
> +				.dai_name = "sn6242-sdca-aif",
> +				.dai_type = SOC_SDW_DAI_TYPE_JACK,
> +				.dailink = {SOC_SDW_JACK_OUT_DAI_ID, SOC_SDW_JACK_IN_DAI_ID},
> +				.init = asoc_sdw_senary_sdca_jack_init,
> +				.exit = asoc_sdw_senary_sdca_jack_exit,
> +				.rtd_init = asoc_sdw_senary_sdca_jack_rtd_init,
> +				.controls = generic_jack_controls,
> +				.num_controls = ARRAY_SIZE(generic_jack_controls),
> +				.widgets = generic_jack_widgets,
> +				.num_widgets = ARRAY_SIZE(generic_jack_widgets),
> +			},
> +			{
> +				.direction = {true, false},
> +				.dai_name = "sn6242-sdca-aif2",
> +				.component_name = "sn6242",

not sure why there is a component_name only for capture?

> +				.dai_type = SOC_SDW_DAI_TYPE_AMP,
> +				.dailink = {SOC_SDW_AMP_OUT_DAI_ID, SOC_SDW_UNUSED_DAI_ID},
> +				.init = asoc_sdw_senary_amp_init,
> +				.exit = asoc_sdw_senary_amp_exit,
> +				.rtd_init = asoc_sdw_senary_sdca_spk_rtd_init,
> +				.controls = generic_spk_controls,
> +				.num_controls = ARRAY_SIZE(generic_spk_controls),
> +				.widgets = generic_spk_widgets,
> +				.num_widgets = ARRAY_SIZE(generic_spk_widgets),
> +				.quirk = SOC_SDW_CODEC_SPKR,
> +				.quirk_exclude = true,
> +			},
> +			{
> +				.direction = {false, true},
> +				.dai_name = "sn6242-sdca-aif3",
> +				.dai_type = SOC_SDW_DAI_TYPE_MIC,
> +				.dailink = {SOC_SDW_UNUSED_DAI_ID, SOC_SDW_DMIC_DAI_ID},
> +				.rtd_init = asoc_sdw_senary_dmic_rtd_init,
> +				.quirk = SOC_SDW_CODEC_MIC,
> +				.quirk_exclude = true,
> +			},
> +		},
> +		.dai_num = 3,
> +	},
> +	{
> +		.part_id = 0x6242,
> +		.name_prefix = "sn6242",
> +		.ignore_internal_dmic = true,
> +		.dais = {
> +			{
> +				.direction = {true, true},
> +				.dai_name = "sn6242-sdca-aif",
> +				.dai_type = SOC_SDW_DAI_TYPE_JACK,
> +				.dailink = {SOC_SDW_JACK_OUT_DAI_ID, SOC_SDW_JACK_IN_DAI_ID},
> +				.init = asoc_sdw_senary_sdca_jack_init,
> +				.exit = asoc_sdw_senary_sdca_jack_exit,
> +				.rtd_init = asoc_sdw_senary_sdca_jack_rtd_init,
> +				.controls = generic_jack_controls,
> +				.num_controls = ARRAY_SIZE(generic_jack_controls),
> +				.widgets = generic_jack_widgets,
> +				.num_widgets = ARRAY_SIZE(generic_jack_widgets),
> +			},
> +			{
> +				.direction = {true, false},
> +				.dai_name = "sn6242-sdca-aif2",
> +				.component_name = "sn6242",
> +				.dai_type = SOC_SDW_DAI_TYPE_AMP,
> +				.dailink = {SOC_SDW_AMP_OUT_DAI_ID, SOC_SDW_UNUSED_DAI_ID},
> +				.init = asoc_sdw_senary_amp_init,
> +				.exit = asoc_sdw_senary_amp_exit,
> +				.rtd_init = asoc_sdw_senary_sdca_spk_rtd_init,
> +				.controls = generic_spk_controls,
> +				.num_controls = ARRAY_SIZE(generic_spk_controls),
> +				.widgets = generic_spk_widgets,
> +				.num_widgets = ARRAY_SIZE(generic_spk_widgets),
> +				.quirk = SOC_SDW_CODEC_SPKR,
> +				.quirk_exclude = true,
> +			},
> +			{
> +				.direction = {false, true},
> +				.dai_name = "sn6242-sdca-aif3",
> +				.dai_type = SOC_SDW_DAI_TYPE_MIC,
> +				.dailink = {SOC_SDW_UNUSED_DAI_ID, SOC_SDW_DMIC_DAI_ID},
> +				.rtd_init = asoc_sdw_senary_dmic_rtd_init,
> +				.quirk = SOC_SDW_CODEC_MIC,
> +				.quirk_exclude = true,
> +			},
> +		},
> +		.dai_num = 3,
> +	},
> +	{
> +		.part_id = 0x6247,
> +		.name_prefix = "sn6242",
> +		.ignore_internal_dmic = true,
> +		.dais = {
> +			{
> +				.direction = {true, true},
> +				.dai_name = "sn6242-sdca-aif",
> +				.dai_type = SOC_SDW_DAI_TYPE_JACK,
> +				.dailink = {SOC_SDW_JACK_OUT_DAI_ID, SOC_SDW_JACK_IN_DAI_ID},
> +				.init = asoc_sdw_senary_sdca_jack_init,
> +				.exit = asoc_sdw_senary_sdca_jack_exit,
> +				.rtd_init = asoc_sdw_senary_sdca_jack_rtd_init,
> +				.controls = generic_jack_controls,
> +				.num_controls = ARRAY_SIZE(generic_jack_controls),
> +				.widgets = generic_jack_widgets,
> +				.num_widgets = ARRAY_SIZE(generic_jack_widgets),
> +			},
> +			{
> +				.direction = {true, false},
> +				.dai_name = "sn6242-sdca-aif2",
> +				.component_name = "sn6242",
> +				.dai_type = SOC_SDW_DAI_TYPE_AMP,
> +				.dailink = {SOC_SDW_AMP_OUT_DAI_ID, SOC_SDW_UNUSED_DAI_ID},
> +				.init = asoc_sdw_senary_amp_init,
> +				.exit = asoc_sdw_senary_amp_exit,
> +				.rtd_init = asoc_sdw_senary_sdca_spk_rtd_init,
> +				.controls = generic_spk_controls,
> +				.num_controls = ARRAY_SIZE(generic_spk_controls),
> +				.widgets = generic_spk_widgets,
> +				.num_widgets = ARRAY_SIZE(generic_spk_widgets),
> +				.quirk = SOC_SDW_CODEC_SPKR,
> +				.quirk_exclude = true,
> +			},
> +			{
> +				.direction = {false, true},
> +				.dai_name = "sn6242-sdca-aif3",
> +				.dai_type = SOC_SDW_DAI_TYPE_MIC,
> +				.dailink = {SOC_SDW_UNUSED_DAI_ID, SOC_SDW_DMIC_DAI_ID},
> +				.rtd_init = asoc_sdw_senary_dmic_rtd_init,
> +				.widgets = generic_dmic_widgets,
> +				.num_widgets = ARRAY_SIZE(generic_dmic_widgets),
> +				.controls = generic_dmic_controls,
> +				.num_controls = ARRAY_SIZE(generic_dmic_controls),
> +				.quirk = SOC_SDW_CODEC_MIC,
> +				.quirk_exclude = true,
> +			},
> +		},
> +		.dai_num = 3,

all those 3 dais look identical, the only difference is the part_id.
Isn't there a better way to define and represent these dais?

> +	},
>  };
>  EXPORT_SYMBOL_NS(codec_info_list, "SND_SOC_SDW_UTILS");
>
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.