| Message ID | 1480501450-10780-2-git-send-email-arnaud.pouliquen@st.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi, On Nov 30 2016 19:24, Arnaud Pouliquen wrote: > In case of several instances of the same PCM control (e.g IEC controls). > Application should be able to address the control using the > device field number, according to the PCM character device. > This patch allows to link DAI PCM controls to the PCM device. > During DAI_link probe, PCM controls are added after device field is forced > to the PCM device number. > > Signed-off-by: Arnaud Pouliquen <arnaud.pouliquen@st.com> > --- > include/sound/soc-dai.h | 4 ++++ > sound/soc/soc-core.c | 35 +++++++++++++++++++++++++++++++++++ > 2 files changed, 39 insertions(+) > > diff --git a/include/sound/soc-dai.h b/include/sound/soc-dai.h > index 200e1f0..3ff1a86 100644 > --- a/include/sound/soc-dai.h > +++ b/include/sound/soc-dai.h > @@ -273,6 +273,10 @@ struct snd_soc_dai_driver { > /* probe ordering - for components with runtime dependencies */ > int probe_order; > int remove_order; > + > + /* Optional PCM controls to bind to PCM device on DAIs link*/ In this patchset, you newly request developers to pay enough attention to 'iface' field of identification information for the control element set. This is logically correct, however such kind of special requirement should be described in code comment for the other developers to start using this feature easily. > + const struct snd_kcontrol_new *pcm_controls; > + int num_pcm_controls; > }; > > /* > diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c > index aaab26a..7b79a31 100644 > --- a/sound/soc/soc-core.c > +++ b/sound/soc/soc-core.c > @@ -1598,6 +1598,32 @@ 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, ret; > + > + 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 (kctl.iface != SNDRV_CTL_ELEM_IFACE_PCM || > + rtd->dai_link->no_pcm) { > + dev_err(dais[i]->dev, > + "Failed to bind control: %s\n", > + kctl.name); It's better to output name of the DAI driver, to make it easy to identify which driver causes this issue, as well. > + return -EINVAL; > + } > + kctl.device = rtd->pcm->device; > + ret = snd_soc_add_dai_controls(dais[i], &kctl, 1); > + if (ret < 0) > + return ret; > + } > + } > + > + return 0; > +} > + > static int soc_link_dai_widgets(struct snd_soc_card *card, > struct snd_soc_dai_link *dai_link, > struct snd_soc_pcm_runtime *rtd) > @@ -1709,6 +1735,15 @@ static int soc_probe_link_dais(struct snd_soc_card *card, > dai_link->stream_name, ret); > return ret; > } > + > + /* Bind DAIs pcm controls to the PCM device */ > + ret = soc_link_dai_pcm_controls(&cpu_dai, 1, rtd); > + if (ret < 0) > + return ret; > + ret = soc_link_dai_pcm_controls(rtd->codec_dais, > + rtd->num_codecs, rtd); > + if (ret < 0) > + return ret; > } else { > INIT_DELAYED_WORK(&rtd->delayed_work, > codec2codec_close_delayed_work); Regards Takashi Sakamoto
Hi, Takashi (Sakamoto): Thanks for the remarks, i will integrate them in V5 Liam, Marc, Takashi (Iwai), I was wondering if you have any comment and/or feedback about this patch-set. That's would help to determine if i have to continue to evolve the patch-set in this way, or if you see bocking point in the concept. Regards, Arnaud On 12/05/2016 12:48 PM, Takashi Sakamoto wrote: > Hi, > > On Nov 30 2016 19:24, Arnaud Pouliquen wrote: >> In case of several instances of the same PCM control (e.g IEC controls). >> Application should be able to address the control using the >> device field number, according to the PCM character device. >> This patch allows to link DAI PCM controls to the PCM device. >> During DAI_link probe, PCM controls are added after device field is forced >> to the PCM device number. >> >> Signed-off-by: Arnaud Pouliquen <arnaud.pouliquen@st.com> >> --- >> include/sound/soc-dai.h | 4 ++++ >> sound/soc/soc-core.c | 35 +++++++++++++++++++++++++++++++++++ >> 2 files changed, 39 insertions(+) >> >> diff --git a/include/sound/soc-dai.h b/include/sound/soc-dai.h >> index 200e1f0..3ff1a86 100644 >> --- a/include/sound/soc-dai.h >> +++ b/include/sound/soc-dai.h >> @@ -273,6 +273,10 @@ struct snd_soc_dai_driver { >> /* probe ordering - for components with runtime dependencies */ >> int probe_order; >> int remove_order; >> + >> + /* Optional PCM controls to bind to PCM device on DAIs link*/ > > In this patchset, you newly request developers to pay enough attention > to 'iface' field of identification information for the control element > set. This is logically correct, however such kind of special requirement > should be described in code comment for the other developers to start > using this feature easily. > >> + const struct snd_kcontrol_new *pcm_controls; >> + int num_pcm_controls; >> }; >> >> /* >> diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c >> index aaab26a..7b79a31 100644 >> --- a/sound/soc/soc-core.c >> +++ b/sound/soc/soc-core.c >> @@ -1598,6 +1598,32 @@ 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, ret; >> + >> + 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 (kctl.iface != SNDRV_CTL_ELEM_IFACE_PCM || >> + rtd->dai_link->no_pcm) { >> + dev_err(dais[i]->dev, >> + "Failed to bind control: %s\n", >> + kctl.name); > > It's better to output name of the DAI driver, to make it easy to > identify which driver causes this issue, as well. > >> + return -EINVAL; >> + } >> + kctl.device = rtd->pcm->device; >> + ret = snd_soc_add_dai_controls(dais[i], &kctl, 1); >> + if (ret < 0) >> + return ret; >> + } >> + } >> + >> + return 0; >> +} >> + >> static int soc_link_dai_widgets(struct snd_soc_card *card, >> struct snd_soc_dai_link *dai_link, >> struct snd_soc_pcm_runtime *rtd) >> @@ -1709,6 +1735,15 @@ static int soc_probe_link_dais(struct snd_soc_card *card, >> dai_link->stream_name, ret); >> return ret; >> } >> + >> + /* Bind DAIs pcm controls to the PCM device */ >> + ret = soc_link_dai_pcm_controls(&cpu_dai, 1, rtd); >> + if (ret < 0) >> + return ret; >> + ret = soc_link_dai_pcm_controls(rtd->codec_dais, >> + rtd->num_codecs, rtd); >> + if (ret < 0) >> + return ret; >> } else { >> INIT_DELAYED_WORK(&rtd->delayed_work, >> codec2codec_close_delayed_work); > > Regards > > Takashi Sakamoto >
On Mon, 05 Dec 2016 14:51:23 +0100, Arnaud Pouliquen wrote: > > Hi, > > Takashi (Sakamoto): Thanks for the remarks, i will integrate them in V5 > > Liam, Marc, Takashi (Iwai), > I was wondering if you have any comment and/or feedback about this > patch-set. That's would help to determine if i have to continue to > evolve the patch-set in this way, or if you see bocking point in the > concept. I see no big issue there as the changes look fairly straightforward. thanks, Takashi > > Regards, > > Arnaud > > On 12/05/2016 12:48 PM, Takashi Sakamoto wrote: > > Hi, > > > > On Nov 30 2016 19:24, Arnaud Pouliquen wrote: > >> In case of several instances of the same PCM control (e.g IEC controls). > >> Application should be able to address the control using the > >> device field number, according to the PCM character device. > >> This patch allows to link DAI PCM controls to the PCM device. > >> During DAI_link probe, PCM controls are added after device field is forced > >> to the PCM device number. > >> > >> Signed-off-by: Arnaud Pouliquen <arnaud.pouliquen@st.com> > >> --- > >> include/sound/soc-dai.h | 4 ++++ > >> sound/soc/soc-core.c | 35 +++++++++++++++++++++++++++++++++++ > >> 2 files changed, 39 insertions(+) > >> > >> diff --git a/include/sound/soc-dai.h b/include/sound/soc-dai.h > >> index 200e1f0..3ff1a86 100644 > >> --- a/include/sound/soc-dai.h > >> +++ b/include/sound/soc-dai.h > >> @@ -273,6 +273,10 @@ struct snd_soc_dai_driver { > >> /* probe ordering - for components with runtime dependencies */ > >> int probe_order; > >> int remove_order; > >> + > >> + /* Optional PCM controls to bind to PCM device on DAIs link*/ > > > > In this patchset, you newly request developers to pay enough attention > > to 'iface' field of identification information for the control element > > set. This is logically correct, however such kind of special requirement > > should be described in code comment for the other developers to start > > using this feature easily. > > > >> + const struct snd_kcontrol_new *pcm_controls; > >> + int num_pcm_controls; > >> }; > >> > >> /* > >> diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c > >> index aaab26a..7b79a31 100644 > >> --- a/sound/soc/soc-core.c > >> +++ b/sound/soc/soc-core.c > >> @@ -1598,6 +1598,32 @@ 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, ret; > >> + > >> + 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 (kctl.iface != SNDRV_CTL_ELEM_IFACE_PCM || > >> + rtd->dai_link->no_pcm) { > >> + dev_err(dais[i]->dev, > >> + "Failed to bind control: %s\n", > >> + kctl.name); > > > > It's better to output name of the DAI driver, to make it easy to > > identify which driver causes this issue, as well. > > > >> + return -EINVAL; > >> + } > >> + kctl.device = rtd->pcm->device; > >> + ret = snd_soc_add_dai_controls(dais[i], &kctl, 1); > >> + if (ret < 0) > >> + return ret; > >> + } > >> + } > >> + > >> + return 0; > >> +} > >> + > >> static int soc_link_dai_widgets(struct snd_soc_card *card, > >> struct snd_soc_dai_link *dai_link, > >> struct snd_soc_pcm_runtime *rtd) > >> @@ -1709,6 +1735,15 @@ static int soc_probe_link_dais(struct snd_soc_card *card, > >> dai_link->stream_name, ret); > >> return ret; > >> } > >> + > >> + /* Bind DAIs pcm controls to the PCM device */ > >> + ret = soc_link_dai_pcm_controls(&cpu_dai, 1, rtd); > >> + if (ret < 0) > >> + return ret; > >> + ret = soc_link_dai_pcm_controls(rtd->codec_dais, > >> + rtd->num_codecs, rtd); > >> + if (ret < 0) > >> + return ret; > >> } else { > >> INIT_DELAYED_WORK(&rtd->delayed_work, > >> codec2codec_close_delayed_work); > > > > Regards > > > > Takashi Sakamoto > > >
diff --git a/include/sound/soc-dai.h b/include/sound/soc-dai.h index 200e1f0..3ff1a86 100644 --- a/include/sound/soc-dai.h +++ b/include/sound/soc-dai.h @@ -273,6 +273,10 @@ struct snd_soc_dai_driver { /* probe ordering - for components with runtime dependencies */ int probe_order; int remove_order; + + /* Optional PCM controls to bind to PCM device on DAIs link*/ + const struct snd_kcontrol_new *pcm_controls; + int num_pcm_controls; }; /* diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c index aaab26a..7b79a31 100644 --- a/sound/soc/soc-core.c +++ b/sound/soc/soc-core.c @@ -1598,6 +1598,32 @@ 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, ret; + + 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 (kctl.iface != SNDRV_CTL_ELEM_IFACE_PCM || + rtd->dai_link->no_pcm) { + dev_err(dais[i]->dev, + "Failed to bind control: %s\n", + kctl.name); + return -EINVAL; + } + kctl.device = rtd->pcm->device; + ret = snd_soc_add_dai_controls(dais[i], &kctl, 1); + if (ret < 0) + return ret; + } + } + + return 0; +} + static int soc_link_dai_widgets(struct snd_soc_card *card, struct snd_soc_dai_link *dai_link, struct snd_soc_pcm_runtime *rtd) @@ -1709,6 +1735,15 @@ static int soc_probe_link_dais(struct snd_soc_card *card, dai_link->stream_name, ret); return ret; } + + /* Bind DAIs pcm controls to the PCM device */ + ret = soc_link_dai_pcm_controls(&cpu_dai, 1, rtd); + if (ret < 0) + return ret; + ret = soc_link_dai_pcm_controls(rtd->codec_dais, + rtd->num_codecs, rtd); + if (ret < 0) + return ret; } else { INIT_DELAYED_WORK(&rtd->delayed_work, codec2codec_close_delayed_work);
In case of several instances of the same PCM control (e.g IEC controls). Application should be able to address the control using the device field number, according to the PCM character device. This patch allows to link DAI PCM controls to the PCM device. During DAI_link probe, PCM controls are added after device field is forced to the PCM device number. Signed-off-by: Arnaud Pouliquen <arnaud.pouliquen@st.com> --- include/sound/soc-dai.h | 4 ++++ sound/soc/soc-core.c | 35 +++++++++++++++++++++++++++++++++++ 2 files changed, 39 insertions(+)