Re: [PATCH v3 1/2] ASoC: core: allow DAI PCM controls bound to PCM device

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,

On Nov 29 2016 00:18, Arnaud Pouliquen wrote:
diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
index 4afa8db..ace83c9 100644
--- a/sound/soc/soc-core.c
+++ b/sound/soc/soc-core.c
@@ -1552,6 +1552,25 @@ static int soc_probe_dai(struct snd_soc_dai *dai, int order)
 	return 0;
 }

+static int soc_link_dai_pcm_controls(struct snd_soc_dai **dais, int num_dais,
+				     struct snd_soc_pcm_runtime *rtd)
+{
+	struct snd_kcontrol_new kctl;
+	int i, j, err;
+
+	for (i = 0; i < num_dais; ++i) {
+		for (j = 0; j < dais[i]->driver->num_pcm_controls; j++) {
+			kctl = dais[i]->driver->pcm_controls[j];
+			if (!rtd->dai_link->no_pcm)
+				kctl.device = rtd->pcm->device;

For the above codes, please request some comment to the other developers
working for ALSA SoC part. I'm not so experienced developer for this
part, sorry.
I think something is missing here, to return an error if following
condition is not respected: iface == SNDRV_CTL_ELEM_IFACE_PCM
I'm going to add a test.

When you have unclear points, it's better to ask them to the person who knows it. Furthermore, you attempt to change core codes of ALSA SoC part. Enough deliberation is preferable.

Liam Girdwood committed to the 'no_pcm' feature in below patch, and he is still active in this subsystem (I met him this month). You could request him to review.
https://git.kernel.org/cgit/linux/kernel/git/tiwai/sound.git/commit/?id=01d7584cd2e5a93a2b959c9dddaa0d93ec205404

+			if (snd_soc_add_dai_controls(dais[i], &kctl, 1))
+				return err;

Return value from snd_soc_add_dai_controls() should be assigned to the
'err' variable, I think.

Oops, stupid mistake... thanks! i will correct it in my next version

OK.

In the other part of this subsystem, the first parameter of helper
functions typically represents a 'subject'. In this context, the subject
is PCM runtime specialized for ALSA SoC part, which get some new control
element sets for the PCM runtime. Therefore, it's better to move the
'rtd' parameter to the first argument. (This is not so strong demand,
and somewhat depends on developers' taste. Furthermore, I have no
self-confidence to tell my intension to you correctly in English...)

I understand your point. Nevertheless, i would not see the PCM runtime
as subject, but the DAI. In this way, I was inspired by the
soc_link_dai_widgets helper that is also in the subsytem...
if rtd is the subject, i think soc_link_dai_pcm_controls should
also be renamed snd_soc_runtime_link_dai_ctls (or something like that)

Fair enough. I might had a confusion due to complexity of ALSA SoC part against ALSA control core, sorry.

But i'm not expert in ASoC semantic, so based on my answer, please
confirm me you point of view, i will integrate it in my V4 with other fixes.

Go a head.


Regards

Takashi Sakamoto
_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxx
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel



[Index of Archives]     [ALSA User]     [Linux Audio Users]     [Kernel Archive]     [Asterisk PBX]     [Photo Sharing]     [Linux Sound]     [Video 4 Linux]     [Gimp]     [Yosemite News]

  Powered by Linux