| Message ID | 878spuilqd.wl-kuninori.morimoto.gx@renesas.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | ASoC: soc-core cleanup - step 4 | expand |
On 10/8/19 11:30 PM, Kuninori Morimoto wrote: > From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com> > > ALSA SoC has 2 similar but diffarent implementation. different > snd_soc_register_dai() is used from topology > snd_soc_register_dais() is used from snd_soc_add_component() > > Because of this kind of confusable naming, and duplicated confusing > implementation, code is very unreadale. unreadable > We shouldn't have duplicated and confusable implementation. confusing > snd_soc_register_dai() is now used from topology. > But, to reduce duplicated code, it will be used from soc-core, too. > To prepare for it, this patch adds missing parameter legacy_dai_naming > to snd_soc_register_dai(). It doesn't look like this series reduces the confusion between snd_soc_register_dai() and snd_soc_register_dais() ? maybe for the latter case since it's a static function we don't want the entire prefix then? > > Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com> > --- > include/sound/soc.h | 3 ++- > sound/soc/soc-core.c | 5 +++-- > sound/soc/soc-topology.c | 2 +- > 3 files changed, 6 insertions(+), 4 deletions(-) > > diff --git a/include/sound/soc.h b/include/sound/soc.h > index 320dcd4..827322a 100644 > --- a/include/sound/soc.h > +++ b/include/sound/soc.h > @@ -1327,7 +1327,8 @@ struct snd_soc_dai_link *snd_soc_find_dai_link(struct snd_soc_card *card, > const char *stream_name); > > int snd_soc_register_dai(struct snd_soc_component *component, > - struct snd_soc_dai_driver *dai_drv); > + struct snd_soc_dai_driver *dai_drv, > + bool legacy_dai_naming); > > struct snd_soc_dai *snd_soc_find_dai( > const struct snd_soc_dai_link_component *dlc); > diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c > index be4e1b5..3a16868 100644 > --- a/sound/soc/soc-core.c > +++ b/sound/soc/soc-core.c > @@ -2611,7 +2611,8 @@ static struct snd_soc_dai *soc_add_dai(struct snd_soc_component *component, > * will be freed in the component cleanup. > */ > int snd_soc_register_dai(struct snd_soc_component *component, > - struct snd_soc_dai_driver *dai_drv) > + struct snd_soc_dai_driver *dai_drv, > + bool legacy_dai_naming) > { > struct snd_soc_dapm_context *dapm = > snd_soc_component_get_dapm(component); > @@ -2625,7 +2626,7 @@ int snd_soc_register_dai(struct snd_soc_component *component, > } > > lockdep_assert_held(&client_mutex); > - dai = soc_add_dai(component, dai_drv, false); > + dai = soc_add_dai(component, dai_drv, legacy_dai_naming); > if (!dai) > return -ENOMEM; > > diff --git a/sound/soc/soc-topology.c b/sound/soc/soc-topology.c > index 0fd0329..b6e1456 100644 > --- a/sound/soc/soc-topology.c > +++ b/sound/soc/soc-topology.c > @@ -1842,7 +1842,7 @@ static int soc_tplg_dai_create(struct soc_tplg *tplg, > list_add(&dai_drv->dobj.list, &tplg->comp->dobj_list); > > /* register the DAI to the component */ > - return snd_soc_register_dai(tplg->comp, dai_drv); > + return snd_soc_register_dai(tplg->comp, dai_drv, false); > } > > static void set_link_flags(struct snd_soc_dai_link *link, >
Hi Pierre-Louis Thank you for your feedback > > snd_soc_register_dai() is now used from topology. > > But, to reduce duplicated code, it will be used from soc-core, too. > > To prepare for it, this patch adds missing parameter legacy_dai_naming > > to snd_soc_register_dai(). > > It doesn't look like this series reduces the confusion between > snd_soc_register_dai() and snd_soc_register_dais() ? > > maybe for the latter case since it's a static function we don't want > the entire prefix then? Maybe my explain is not so good... The point is that, in general people think like below from naming. Other functions are this style. => int snd_soc_register_dai() { ... } int snd_soc_register_dais() { for(..) { => snd_soc_register_dai() } } But in reality is like this int snd_soc_register_dai() { /* almost same but different code */ } snd_soc_register_dais() { /* almost same but different code */ } To avoid duplicate code and confusion, this patchset try to implement "general" style. But needs some preparation. I will fix log and English. Thank you for your help !! Best regards --- Kuninori Morimoto
On 10/10/19 8:07 PM, Kuninori Morimoto wrote: > > Hi Pierre-Louis > > Thank you for your feedback > >>> snd_soc_register_dai() is now used from topology. >>> But, to reduce duplicated code, it will be used from soc-core, too. >>> To prepare for it, this patch adds missing parameter legacy_dai_naming >>> to snd_soc_register_dai(). >> >> It doesn't look like this series reduces the confusion between >> snd_soc_register_dai() and snd_soc_register_dais() ? >> >> maybe for the latter case since it's a static function we don't want >> the entire prefix then? > > Maybe my explain is not so good... > The point is that, in general people think like below from naming. > Other functions are this style. > > => int snd_soc_register_dai() > { > ... > } > > int snd_soc_register_dais() > { > for(..) { > => snd_soc_register_dai() > } > } > > But in reality is like this > > int snd_soc_register_dai() > { > /* almost same but different code */ > } > > snd_soc_register_dais() > { > /* almost same but different code */ > } > > To avoid duplicate code and confusion, > this patchset try to implement "general" style. > But needs some preparation. > > I will fix log and English. I get your point, what I was trying to suggest is that we only use the full snd_soc prefix for non-static functions that can be called by other files, for static functions we can use just the soc prefix. It's a convention that helps the reader understand what is local and what is common/shared.
diff --git a/include/sound/soc.h b/include/sound/soc.h index 320dcd4..827322a 100644 --- a/include/sound/soc.h +++ b/include/sound/soc.h @@ -1327,7 +1327,8 @@ struct snd_soc_dai_link *snd_soc_find_dai_link(struct snd_soc_card *card, const char *stream_name); int snd_soc_register_dai(struct snd_soc_component *component, - struct snd_soc_dai_driver *dai_drv); + struct snd_soc_dai_driver *dai_drv, + bool legacy_dai_naming); struct snd_soc_dai *snd_soc_find_dai( const struct snd_soc_dai_link_component *dlc); diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c index be4e1b5..3a16868 100644 --- a/sound/soc/soc-core.c +++ b/sound/soc/soc-core.c @@ -2611,7 +2611,8 @@ static struct snd_soc_dai *soc_add_dai(struct snd_soc_component *component, * will be freed in the component cleanup. */ int snd_soc_register_dai(struct snd_soc_component *component, - struct snd_soc_dai_driver *dai_drv) + struct snd_soc_dai_driver *dai_drv, + bool legacy_dai_naming) { struct snd_soc_dapm_context *dapm = snd_soc_component_get_dapm(component); @@ -2625,7 +2626,7 @@ int snd_soc_register_dai(struct snd_soc_component *component, } lockdep_assert_held(&client_mutex); - dai = soc_add_dai(component, dai_drv, false); + dai = soc_add_dai(component, dai_drv, legacy_dai_naming); if (!dai) return -ENOMEM; diff --git a/sound/soc/soc-topology.c b/sound/soc/soc-topology.c index 0fd0329..b6e1456 100644 --- a/sound/soc/soc-topology.c +++ b/sound/soc/soc-topology.c @@ -1842,7 +1842,7 @@ static int soc_tplg_dai_create(struct soc_tplg *tplg, list_add(&dai_drv->dobj.list, &tplg->comp->dobj_list); /* register the DAI to the component */ - return snd_soc_register_dai(tplg->comp, dai_drv); + return snd_soc_register_dai(tplg->comp, dai_drv, false); } static void set_link_flags(struct snd_soc_dai_link *link,