| Message ID | 87y305932s.wl-kuninori.morimoto.gx@renesas.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | ASoC: cleanup patches for soc-core | expand |
On 2019-08-07 03:31, Kuninori Morimoto wrote: > From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com> > > snd_soc_component_set_jack() is used for both setting/clearing. > Setting purpose is used under each driver. > Hence, clearing purpose should be used under each driver, not soc-core. > > soc-core shouldn't touch it even though its purpose was for clearing, > otherwise, code becomes very confusable. > This patch removes snd_soc_component_set_jack() from soc-core.c > > Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com> > --- > v1 -> v2 > > - no change > > sound/soc/soc-core.c | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c > index 80703618..e708095 100644 > --- a/sound/soc/soc-core.c > +++ b/sound/soc/soc-core.c > @@ -938,7 +938,6 @@ static int soc_bind_dai_link(struct snd_soc_card *card, > > static void soc_cleanup_component(struct snd_soc_component *component) > { > - snd_soc_component_set_jack(component, NULL, NULL); > list_del(&component->card_list); > snd_soc_dapm_free(snd_soc_component_get_dapm(component)); > soc_cleanup_component_debugfs(component); > This has been added lately for a reason - reload/ unload series. Amadeusz, could you comment on this change?
On Wed, 7 Aug 2019 20:49:09 +0200 Cezary Rojewski <cezary.rojewski@intel.com> wrote: > On 2019-08-07 03:31, Kuninori Morimoto wrote: > > From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com> > > > > snd_soc_component_set_jack() is used for both setting/clearing. > > Setting purpose is used under each driver. > > Hence, clearing purpose should be used under each driver, not > > soc-core. > > > > soc-core shouldn't touch it even though its purpose was for > > clearing, otherwise, code becomes very confusable. > > This patch removes snd_soc_component_set_jack() from soc-core.c > > > > Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com> > > --- > > v1 -> v2 > > > > - no change > > > > sound/soc/soc-core.c | 1 - > > 1 file changed, 1 deletion(-) > > > > diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c > > index 80703618..e708095 100644 > > --- a/sound/soc/soc-core.c > > +++ b/sound/soc/soc-core.c > > @@ -938,7 +938,6 @@ static int soc_bind_dai_link(struct > > snd_soc_card *card, > > static void soc_cleanup_component(struct snd_soc_component > > *component) { > > - snd_soc_component_set_jack(component, NULL, NULL); > > list_del(&component->card_list); > > snd_soc_dapm_free(snd_soc_component_get_dapm(component)); > > soc_cleanup_component_debugfs(component); > > > > This has been added lately for a reason - reload/ unload series. > Amadeusz, could you comment on this change? This was done on assumption that we want to always make sure that it is cleaned up, independent of if driver author accidentally forgets to do this. We can of course add handler to our driver to do this, first version of patch actually did this, before we decided on global option. Amadeusz
Hi > > > From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com> > > > > > > snd_soc_component_set_jack() is used for both setting/clearing. > > > Setting purpose is used under each driver. > > > Hence, clearing purpose should be used under each driver, not > > > soc-core. > > > > > > soc-core shouldn't touch it even though its purpose was for > > > clearing, otherwise, code becomes very confusable. > > > This patch removes snd_soc_component_set_jack() from soc-core.c > > > > > > Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com> > > > --- > > > v1 -> v2 > > > > > > - no change > > > > > > sound/soc/soc-core.c | 1 - > > > 1 file changed, 1 deletion(-) > > > > > > diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c > > > index 80703618..e708095 100644 > > > --- a/sound/soc/soc-core.c > > > +++ b/sound/soc/soc-core.c > > > @@ -938,7 +938,6 @@ static int soc_bind_dai_link(struct > > > snd_soc_card *card, > > > static void soc_cleanup_component(struct snd_soc_component > > > *component) { > > > - snd_soc_component_set_jack(component, NULL, NULL); > > > list_del(&component->card_list); > > > snd_soc_dapm_free(snd_soc_component_get_dapm(component)); > > > soc_cleanup_component_debugfs(component); > > > > > > > This has been added lately for a reason - reload/ unload series. > > Amadeusz, could you comment on this change? > > This was done on assumption that we want to always make sure that it is > cleaned up, independent of if driver author accidentally forgets to do > this. > > We can of course add handler to our driver to do this, first version of > patch actually did this, before we decided on global option. I double-checked framework. *All* drivers which are using snd_soc_component_set_jack() doesn't care reset jack. I think it should be done under driver, not framework, but this patch seems have big effect. OK, let's skip it. Thank you for your help !! Best regards --- Kuninori Morimoto
On Tue, Aug 20, 2019 at 01:24:11PM +0900, Kuninori Morimoto wrote: > > We can of course add handler to our driver to do this, first version of > > patch actually did this, before we decided on global option. > I double-checked framework. > *All* drivers which are using snd_soc_component_set_jack() > doesn't care reset jack. > I think it should be done under driver, not framework, > but this patch seems have big effect. > OK, let's skip it. Yes, I think the current code is better for robustness - we've got a few things like this where the core will do cleanup even if it's neater to do it in the driver since it makes it harder for there to be mistakes.
Hi Mark > > I double-checked framework. > > *All* drivers which are using snd_soc_component_set_jack() > > doesn't care reset jack. > > I think it should be done under driver, not framework, > > but this patch seems have big effect. > > OK, let's skip it. > > Yes, I think the current code is better for robustness - we've got a few > things like this where the core will do cleanup even if it's neater to > do it in the driver since it makes it harder for there to be mistakes. I agree we want to have robustness. To make clear that it is for robustness, I think it is better to have such message/comment on code :) Thank you for your help !! Best regards --- Kuninori Morimoto
On Thu, Aug 29, 2019 at 09:16:08AM +0900, Kuninori Morimoto wrote: > I agree we want to have robustness. > To make clear that it is for robustness, > I think it is better to have such message/comment on code :) Sure, that sounds sensible.
diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c index 80703618..e708095 100644 --- a/sound/soc/soc-core.c +++ b/sound/soc/soc-core.c @@ -938,7 +938,6 @@ static int soc_bind_dai_link(struct snd_soc_card *card, static void soc_cleanup_component(struct snd_soc_component *component) { - snd_soc_component_set_jack(component, NULL, NULL); list_del(&component->card_list); snd_soc_dapm_free(snd_soc_component_get_dapm(component)); soc_cleanup_component_debugfs(component);