| Message ID | 87wo4ry3bz.wl-kuninori.morimoto.gx@renesas.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | ASoC: soc-component: collect component functions | expand |
On Mon, 2020-06-01 at 10:36 +0900, Kuninori Morimoto wrote: > From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com> > > component related function should be implemented at > soc-component.c. > This patch moves soc-compress soc_compr_components_open() > to soc-component as snd_soc_component_compr_open(). Morimoto-san, This is change is justified in one way but it also feels like maybe because the functions are so specific to compr devices, it is best to leave them here. Maybe others should also chime in. Thanks, Ranjani
Hi Ranjani Cc Mark > > From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com> > > > > component related function should be implemented at > > soc-component.c. > > This patch moves soc-compress soc_compr_components_open() > > to soc-component as snd_soc_component_compr_open(). > Morimoto-san, > > This is change is justified in one way but it also feels like maybe > because the functions are so specific to compr devices, it is best to > leave them here. Maybe others should also chime in. Hmm.. I'm not sure. Such kind of functions are already has been changed... Let's ask to Maintainer. Mark, what do you think ? Thank you for your help !! Best regards --- Kuninori Morimoto
On Tue, Jun 02, 2020 at 08:16:00AM +0900, Kuninori Morimoto wrote: > > > From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com> > > > component related function should be implemented at > > > soc-component.c. > > > This patch moves soc-compress soc_compr_components_open() > > > to soc-component as snd_soc_component_compr_open(). > > This is change is justified in one way but it also feels like maybe > > because the functions are so specific to compr devices, it is best to > > leave them here. Maybe others should also chime in. > Hmm.. I'm not sure. > Such kind of functions are already has been changed... > Let's ask to Maintainer. > Mark, what do you think ? I don't think there's one right answer here. I guess if we have to pick we should do the same as is being done for PCM for compressed open but either option is fine I think.
On Mon, 2020-06-01 at 10:36 +0900, Kuninori Morimoto wrote: > From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com> > > component related function should be implemented at > soc-component.c. > This patch moves soc-compress soc_compr_components_open() > to soc-component as snd_soc_component_compr_open(). > > Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com> > --- > include/sound/soc-component.h | 2 ++ > sound/soc/soc-component.c | 23 +++++++++++++++++++++++ > sound/soc/soc-compress.c | 31 ++----------------------------- > 3 files changed, 27 insertions(+), 29 deletions(-) > > diff --git a/include/sound/soc-component.h b/include/sound/soc- > component.h > index bb26d55a9289..4f82839948d6 100644 > --- a/include/sound/soc-component.h > +++ b/include/sound/soc-component.h > @@ -436,6 +436,8 @@ int snd_soc_component_of_xlate_dai_id(struct > snd_soc_component *component, > int snd_soc_component_of_xlate_dai_name(struct snd_soc_component > *component, > struct of_phandle_args *args, > const char **dai_name); > +int snd_soc_component_compr_open(struct snd_compr_stream *cstream, > + struct snd_soc_component **last); > > int snd_soc_pcm_component_pointer(struct snd_pcm_substream > *substream); > int snd_soc_pcm_component_ioctl(struct snd_pcm_substream *substream, > diff --git a/sound/soc/soc-component.c b/sound/soc/soc-component.c > index 150b02be0219..c2a6046a6380 100644 > --- a/sound/soc/soc-component.c > +++ b/sound/soc/soc-component.c > @@ -384,6 +384,29 @@ > EXPORT_SYMBOL_GPL(snd_soc_component_exit_regmap); > > #endif > > +int snd_soc_component_compr_open(struct snd_compr_stream *cstream, > + struct snd_soc_component **last) > +{ > + struct snd_soc_pcm_runtime *rtd = cstream->private_data; > + struct snd_soc_component *component; > + int i, ret; > + > + for_each_rtd_components(rtd, i, component) { > + if (component->driver->compress_ops && > + component->driver->compress_ops->open) { > + ret = component->driver->compress_ops- > >open(component, cstream); > + if (ret < 0) { > + *last = component; > + return soc_component_ret(component, > ret); > + } > + } > + } > + > + *last = NULL; > + return 0; > +} > +EXPORT_SYMBOL_GPL(snd_soc_component_compr_open); > + > int snd_soc_pcm_component_pointer(struct snd_pcm_substream > *substream) > { > struct snd_soc_pcm_runtime *rtd = substream->private_data; > diff --git a/sound/soc/soc-compress.c b/sound/soc/soc-compress.c > index 4984b6a2c370..2a0d554013a4 100644 > --- a/sound/soc/soc-compress.c > +++ b/sound/soc/soc-compress.c > @@ -22,33 +22,6 @@ > #include <sound/soc-link.h> > #include <linux/pm_runtime.h> > > -static int soc_compr_components_open(struct snd_compr_stream > *cstream, > - struct snd_soc_component **last) > -{ > - struct snd_soc_pcm_runtime *rtd = cstream->private_data; > - struct snd_soc_component *component; > - int i, ret; > - > - for_each_rtd_components(rtd, i, component) { > - if (!component->driver->compress_ops || > - !component->driver->compress_ops->open) > - continue; > - > - ret = component->driver->compress_ops->open(component, > cstream); > - if (ret < 0) { > - dev_err(component->dev, > - "Compress ASoC: can't open platform %s: > %d\n", > - component->name, ret); > - > - *last = component; > - return ret; > - } > - } > - > - *last = NULL; > - return 0; > -} > - > static int soc_compr_components_free(struct snd_compr_stream > *cstream, > struct snd_soc_component *last) > { > @@ -92,7 +65,7 @@ static int soc_compr_open(struct snd_compr_stream > *cstream) > if (ret < 0) > goto out; > > - ret = soc_compr_components_open(cstream, &component); > + ret = snd_soc_component_compr_open(cstream, &component); If you do decide to keep your changes to move all these functions to soc-component.c, we need to include soc-component.h in soc-compress.c isnt it? Thanks, Ranjani
Hi Mark, Ranjani Thank you for your feedback > > > This is change is justified in one way but it also feels like maybe > > > because the functions are so specific to compr devices, it is best to > > > leave them here. Maybe others should also chime in. (snip) > I don't think there's one right answer here. I guess if we have to pick > we should do the same as is being done for PCM for compressed open but > either option is fine I think. I re-checked the patch. The original code is soc_compr_xxx() and doing compress things. I agree to keeping original code at original file. I will remove these patched from v2 patch-set. Thank you for your help !! Best regards --- Kuninori Morimoto
Hi Mark, Ranjani again > > > > This is change is justified in one way but it also feels like maybe > > > > because the functions are so specific to compr devices, it is best to > > > > leave them here. Maybe others should also chime in. > (snip) > > I don't think there's one right answer here. I guess if we have to pick > > we should do the same as is being done for PCM for compressed open but > > either option is fine I think. > > I re-checked the patch. > The original code is soc_compr_xxx() and doing compress things. > I agree to keeping original code at original file. > I will remove these patched from v2 patch-set. I re-re-checked. I will drop this patch this time, but might post it again in the future. Because we already have same/similar functon for DAI (= snd_soc_dai_compr_xxx()). And I plan to add snd_soc_component_xxx() specific future, and compr can use it. Thank you for your help !! Best regards --- Kuninori Morimoto
diff --git a/include/sound/soc-component.h b/include/sound/soc-component.h index bb26d55a9289..4f82839948d6 100644 --- a/include/sound/soc-component.h +++ b/include/sound/soc-component.h @@ -436,6 +436,8 @@ int snd_soc_component_of_xlate_dai_id(struct snd_soc_component *component, int snd_soc_component_of_xlate_dai_name(struct snd_soc_component *component, struct of_phandle_args *args, const char **dai_name); +int snd_soc_component_compr_open(struct snd_compr_stream *cstream, + struct snd_soc_component **last); int snd_soc_pcm_component_pointer(struct snd_pcm_substream *substream); int snd_soc_pcm_component_ioctl(struct snd_pcm_substream *substream, diff --git a/sound/soc/soc-component.c b/sound/soc/soc-component.c index 150b02be0219..c2a6046a6380 100644 --- a/sound/soc/soc-component.c +++ b/sound/soc/soc-component.c @@ -384,6 +384,29 @@ EXPORT_SYMBOL_GPL(snd_soc_component_exit_regmap); #endif +int snd_soc_component_compr_open(struct snd_compr_stream *cstream, + struct snd_soc_component **last) +{ + struct snd_soc_pcm_runtime *rtd = cstream->private_data; + struct snd_soc_component *component; + int i, ret; + + for_each_rtd_components(rtd, i, component) { + if (component->driver->compress_ops && + component->driver->compress_ops->open) { + ret = component->driver->compress_ops->open(component, cstream); + if (ret < 0) { + *last = component; + return soc_component_ret(component, ret); + } + } + } + + *last = NULL; + return 0; +} +EXPORT_SYMBOL_GPL(snd_soc_component_compr_open); + int snd_soc_pcm_component_pointer(struct snd_pcm_substream *substream) { struct snd_soc_pcm_runtime *rtd = substream->private_data; diff --git a/sound/soc/soc-compress.c b/sound/soc/soc-compress.c index 4984b6a2c370..2a0d554013a4 100644 --- a/sound/soc/soc-compress.c +++ b/sound/soc/soc-compress.c @@ -22,33 +22,6 @@ #include <sound/soc-link.h> #include <linux/pm_runtime.h> -static int soc_compr_components_open(struct snd_compr_stream *cstream, - struct snd_soc_component **last) -{ - struct snd_soc_pcm_runtime *rtd = cstream->private_data; - struct snd_soc_component *component; - int i, ret; - - for_each_rtd_components(rtd, i, component) { - if (!component->driver->compress_ops || - !component->driver->compress_ops->open) - continue; - - ret = component->driver->compress_ops->open(component, cstream); - if (ret < 0) { - dev_err(component->dev, - "Compress ASoC: can't open platform %s: %d\n", - component->name, ret); - - *last = component; - return ret; - } - } - - *last = NULL; - return 0; -} - static int soc_compr_components_free(struct snd_compr_stream *cstream, struct snd_soc_component *last) { @@ -92,7 +65,7 @@ static int soc_compr_open(struct snd_compr_stream *cstream) if (ret < 0) goto out; - ret = soc_compr_components_open(cstream, &component); + ret = snd_soc_component_compr_open(cstream, &component); if (ret < 0) goto machine_err; @@ -170,7 +143,7 @@ static int soc_compr_open_fe(struct snd_compr_stream *cstream) if (ret < 0) goto out; - ret = soc_compr_components_open(cstream, &component); + ret = snd_soc_component_compr_open(cstream, &component); if (ret < 0) goto open_err;