| Message ID | 1456820357-2545-3-git-send-email-arnaud.pouliquen@st.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Mar 01, 2016 at 09:19:15AM +0100, Arnaud Pouliquen wrote: > Some Controls defined in DAI need to be associated to PCM device (e.g. IEC60958). > > This allows to perform post initialization in DAI after PCM device creation. Rather than add an explicit callback and make drivers open code this I'd prefer to see us either just move the control creation entirely (I can't immediately think of a particular need for the current ordering) or add a data based mechanism like we currently have. Why do we need to do this via a callback?
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 On 03/02/2016 05:03 AM, Mark Brown wrote: > On Tue, Mar 01, 2016 at 09:19:15AM +0100, Arnaud Pouliquen wrote: >> Some Controls defined in DAI need to be associated to PCM device >> (e.g. IEC60958). >> >> This allows to perform post initialization in DAI after PCM >> device creation. > > Rather than add an explicit callback and make drivers open code > this I'd prefer to see us either just move the control creation > entirely (I can't immediately think of a particular need for the > current ordering) or add a data based mechanism like we currently > have. Why do we need to do this via a callback? > For time being I identify only a need to link controls to pcm device. So no other justification to implement the pcm_new callback. I implemented the pcm_new callback because straightforward way to handle generic iec958 control proposed in [PATCH v3 1/4] ALSA: pcm: add IEC958 channel status control helper Here is an alternative but with higher impact in soc-core: 1)Create 2 helper functions in soc-core: - snd_soc_add_dai_pcm_controls: =>register a control that needs to be linked to pcm device. if dai probed(pcm device exist) => link control to pcm device else postpone link creation in soc_probe_link_dais - snd_soc_add_dai_iec_control same but based on snd_pcm_create_iec958_ctrl 2) add 2 news fields in snd-soc-dai struct struct snd_kcontrol *pcm_kctl /* list control that will be linked to pcm device*/ struct snd_pcm_iec958_params *params /* if not null iec control is created */ Abandon helper function for iec control should simplify implementation but that also makes sense to have helper for this generic control... Is is sometime that should be more reasonable? Thanks Arnaud -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.17 (GNU/Linux) iQIcBAEBAgAGBQJW1rM1AAoJEP0ZQ+DAfqbfTnYP/3ILQb0WXjIEel9u40UcT5/z eN6kZSurKaNJq0kX3XpORa7USBct0jxXdgaQEsxMMZxLcc2nkJuxOMgHjtEb/EDj 337mRT+TPG/pdg2e/AH+rafNsUSSgZfuyl1LudLmgxasEVk2cTEJTmz287LmNK9O CSp5U8gs0zGnKiUcEdqgSHLdJYNxJJ87kD7i3d510fx1koYjDPxfgxUnhqIKiqTP S9mEC6y0UqD7RfTVBPdYTHk8qGtJjrPvhuR1I8HiEfa/YUonFvvxxP7ec5ic56cv GQ0PYRLhUkH+45OIoB46rfvu2yCNtFbTqBC64QIV8x7FhbI8gLqkMAzvtmyKE2uk sZ9ZWri9HYpo9QNmVfFyqHoR7UcXKI4XmSZ7eVAcAPFlTf9K3dnPYpNPOeJ+lUoH bAj0I+Rq/rpPVEJnhggSqQPeDk6jyFffxZqIx4A3UEIuwgNtIv2QlWzQ7FhTwuGJ FQM94xqfIT5V7e75FPUkPC5ktTCcXFhg7ClLk3frP4kjXzFKg5BcvRsfT1ZyHSd2 hYRwKIqIp8nsgiM5LYO3ujAz3UJDDtbP9dPE4G+foXokvQWsFl15lh2+4Knm1SNv jZ3MXL/PDaTQydTQ8Hs+jL9Xclpk/wFyIgDI9JYHiA1htuWnCvVF6GZCxBBup2jC R1wZ8q8kjN6JFjyaWgYl =gmEu -----END PGP SIGNATURE-----
On Wed, Mar 02, 2016 at 10:32:37AM +0100, Arnaud Pouliquen wrote: > Here is an alternative but with higher impact in soc-core: > 1)Create 2 helper functions in soc-core: > - snd_soc_add_dai_pcm_controls: > =>register a control that needs to be linked to pcm device. > if dai probed(pcm device exist) => link control to pcm device > else postpone link creation in soc_probe_link_dais > - snd_soc_add_dai_iec_control > same but based on snd_pcm_create_iec958_ctrl The whole point is to avoid functions... > 2) add 2 news fields in snd-soc-dai struct > struct snd_kcontrol *pcm_kctl /* list control that will be linked to > pcm device*/ > struct snd_pcm_iec958_params *params /* if not null iec control is > created */ > Abandon helper function for iec control should simplify implementation > but that also makes sense to have helper for this generic control... This seems better though I'm confused about what exactly these would be, I've not looked at the first patch since it seemed to have problems.
diff --git a/include/sound/soc-dai.h b/include/sound/soc-dai.h index 964b7de..6969c83 100644 --- a/include/sound/soc-dai.h +++ b/include/sound/soc-dai.h @@ -205,6 +205,13 @@ struct snd_soc_dai_ops { */ snd_pcm_sframes_t (*delay)(struct snd_pcm_substream *, struct snd_soc_dai *); + + /* + * function called by soc_probe_link_dais to post initialize DAI + * after pcm device creation. + * As example, can be used to link a controls to the pcm device + */ + int (*pcm_new)(struct snd_soc_pcm_runtime *, struct snd_soc_dai *); }; /* diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c index 790ee2b..3899ce7 100644 --- a/sound/soc/soc-core.c +++ b/sound/soc/soc-core.c @@ -1587,6 +1587,7 @@ static int soc_probe_link_dais(struct snd_soc_card *card, { struct snd_soc_dai_link *dai_link = rtd->dai_link; struct snd_soc_dai *cpu_dai = rtd->cpu_dai; + const struct snd_soc_dai_ops *ops; int i, ret; dev_dbg(card->dev, "ASoC: probe %s dai link %d late %d\n", @@ -1662,6 +1663,19 @@ static int soc_probe_link_dais(struct snd_soc_card *card, } } + for (i = 0; i < rtd->num_codecs; i++) { + ops = rtd->codec_dais[i]->driver->ops; + if (ops->pcm_new) + ret = ops->pcm_new(rtd, rtd->codec_dais[i]); + if (ret) + return ret; + } + ops = cpu_dai->driver->ops; + if (ops->pcm_new) + ret = ops->pcm_new(rtd, cpu_dai); + if (ret) + return ret; + return 0; }
Some Controls defined in DAI need to be associated to PCM device (e.g. IEC60958). This allows to perform post initialization in DAI after PCM device creation. Signed-off-by: Arnaud Pouliquen <arnaud.pouliquen@st.com> --- include/sound/soc-dai.h | 7 +++++++ sound/soc/soc-core.c | 14 ++++++++++++++ 2 files changed, 21 insertions(+)