| Message ID | 87ef1w6w8o.wl-kuninori.morimoto.gx@renesas.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | ASoC: use modern style for aux_dev | expand |
On Wed, Aug 7, 2019 at 11:03 PM Kuninori Morimoto < kuninori.morimoto.gx@renesas.com> wrote: > > From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com> > > It is easy to read code if it is cleanly using paired function/naming, > like start <-> stop, register <-> unregister, etc, etc. > But, current ALSA SoC code is very random, unbalance, not paired, etc. > It is easy to create bug at the such code, and it will be difficult to > debug. > > soc-core.c has soc_bind_aux_dev(), but, there is no its paired > soc_unbind_aux_dev(). This patch adds it. > Morimoto-san, I'm not sure it quite improves readability to just have list_del in unbind_aux_dev(). bin_aux_dev() does more than just adding to the card_aux_list(). So in fact, I think this change makes it look unbalanced. The existing code for aux_dev looks quite similar to what bind_dai_link() and remove_dai_link() do and they are quite intuitive. Thanks, Ranjani > > Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com> > --- > sound/soc/soc-core.c | 7 ++++++- > 1 file changed, 6 insertions(+), 1 deletion(-) > > diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c > index f8a5464..5e3b907 100644 > --- a/sound/soc/soc-core.c > +++ b/sound/soc/soc-core.c > @@ -1527,6 +1527,11 @@ static int soc_probe_link_dais(struct snd_soc_card > *card, > return ret; > } > > +static void soc_unbind_aux_dev(struct snd_soc_component *component) > +{ > + list_del(&component->card_aux_list); > +} > + > static int soc_bind_aux_dev(struct snd_soc_card *card, > struct snd_soc_aux_dev *aux_dev) > { > @@ -1578,7 +1583,7 @@ static void soc_remove_aux_devices(struct > snd_soc_card *card) > if (comp->driver->remove_order == order) { > soc_remove_component(comp); > /* remove it from the card's aux_comp_list > */ > - list_del(&comp->card_aux_list); > + soc_unbind_aux_dev(comp); > } > } > } > -- > 2.7.4 > > _______________________________________________ > Alsa-devel mailing list > Alsa-devel@alsa-project.org > https://mailman.alsa-project.org/mailman/listinfo/alsa-devel >
Hi Sridharan Thank you for your feedback > From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com> > > It is easy to read code if it is cleanly using paired function/naming, > like start <-> stop, register <-> unregister, etc, etc. > But, current ALSA SoC code is very random, unbalance, not paired, etc. > It is easy to create bug at the such code, and it will be difficult to > debug. > > soc-core.c has soc_bind_aux_dev(), but, there is no its paired > soc_unbind_aux_dev(). This patch adds it. > > Morimoto-san, > I'm not sure it quite improves readability to just have list_del in unbind_aux_dev(). bin_aux_dev() does more than just > adding to the card_aux_list(). So in fact, I think this change makes it look unbalanced. Hmm... But, bind_aux_dev() is doing 1) find target component 2) setup component->init 3) add card_aux_list We can ignore 1) for unbind. This patch is for 3). I guess we can ignore 2), but can handle it. How about this ? static void soc_unbind_aux_dev(struct snd_soc_component *component) { => component->init = NULL; list_del(&component->card_aux_list); } > The existing code for aux_dev looks quite similar to what bind_dai_link() and remove_dai_link() do and they are quite > intuitive. I guess "add"_dai_link instead of "bind"_dai_link ? Thank you for your help !! Best regards --- Kuninori Morimoto
diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c index f8a5464..5e3b907 100644 --- a/sound/soc/soc-core.c +++ b/sound/soc/soc-core.c @@ -1527,6 +1527,11 @@ static int soc_probe_link_dais(struct snd_soc_card *card, return ret; } +static void soc_unbind_aux_dev(struct snd_soc_component *component) +{ + list_del(&component->card_aux_list); +} + static int soc_bind_aux_dev(struct snd_soc_card *card, struct snd_soc_aux_dev *aux_dev) { @@ -1578,7 +1583,7 @@ static void soc_remove_aux_devices(struct snd_soc_card *card) if (comp->driver->remove_order == order) { soc_remove_component(comp); /* remove it from the card's aux_comp_list */ - list_del(&comp->card_aux_list); + soc_unbind_aux_dev(comp); } } }