diff mbox series

[v2,2/2] ASoC: soc-pcm: makes snd_soc_dpcm_can_be_xxx() local function

Message ID 87ikvua9sf.wl-kuninori.morimoto.gx@renesas.com (mailing list archive)
State Superseded
Headers show
Series ASoC: soc-pcm: makes snd_soc_dpcm_can_be_xxx() local function | expand

Commit Message

Kuninori Morimoto Aug. 21, 2024, 2:29 a.m. UTC 
No driver is calling snd_soc_dpcm_can_be_xxx() functions. We don't need
to have EXPORT_SYMBOL_GPL() for them. Let's makes it static function.
One note is that snd_soc_dpcm_fe_can_update() is not used in upstream.
Use #if-endif and keep it for future support.

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
 include/sound/soc-dpcm.h | 18 ------------------
 sound/soc/soc-pcm.c      | 23 ++++++++++-------------
 2 files changed, 10 insertions(+), 31 deletions(-)

Comments

Mark Brown Aug. 21, 2024, 1:13 p.m. UTC | #1 
On Wed, Aug 21, 2024 at 02:29:04AM +0000, Kuninori Morimoto wrote:
> No driver is calling snd_soc_dpcm_can_be_xxx() functions. We don't need
> to have EXPORT_SYMBOL_GPL() for them. Let's makes it static function.
> One note is that snd_soc_dpcm_fe_can_update() is not used in upstream.
> Use #if-endif and keep it for future support.

We should just delete the code if it's not needed, it can always be
re-added later.
Kuninori Morimoto Aug. 21, 2024, 11:23 p.m. UTC | #2 
Hi Mark

> > No driver is calling snd_soc_dpcm_can_be_xxx() functions. We don't need
> > to have EXPORT_SYMBOL_GPL() for them. Let's makes it static function.
> > One note is that snd_soc_dpcm_fe_can_update() is not used in upstream.
> > Use #if-endif and keep it for future support.
> 
> We should just delete the code if it's not needed, it can always be
> re-added later.

Thanks.
The reason why this patch try to keep the code is that 2 functions
(for fe/be) determination methods are unique. But be is used, fe is not
used.

	snd_soc_dpcm_fe_can_update()
	snd_soc_dpcm_be_can_update()

Of cource we can remove it and re-added later, but I'm afraid the person
who will re-add it might not noticed that there was a code before, and
re-add different determination methods, etc.
To avoid such things, this patch disables it by #if-endif.

But I can post v3 patch which doesn't have unneeded code.
Please let me know which one is better.

Thank you for your help !!

Best regards
---
Kuninori Morimoto
Mark Brown Aug. 21, 2024, 11:38 p.m. UTC | #3 
On Wed, Aug 21, 2024 at 11:23:24PM +0000, Kuninori Morimoto wrote:

> > > No driver is calling snd_soc_dpcm_can_be_xxx() functions. We don't need
> > > to have EXPORT_SYMBOL_GPL() for them. Let's makes it static function.
> > > One note is that snd_soc_dpcm_fe_can_update() is not used in upstream.
> > > Use #if-endif and keep it for future support.

> > We should just delete the code if it's not needed, it can always be
> > re-added later.

> The reason why this patch try to keep the code is that 2 functions
> (for fe/be) determination methods are unique. But be is used, fe is not
> used.

> 	snd_soc_dpcm_fe_can_update()
> 	snd_soc_dpcm_be_can_update()

> Of cource we can remove it and re-added later, but I'm afraid the person
> who will re-add it might not noticed that there was a code before, and
> re-add different determination methods, etc.
> To avoid such things, this patch disables it by #if-endif.

> But I can post v3 patch which doesn't have unneeded code.
> Please let me know which one is better.

I can see the symmetry argument, I don't know how other people working
more with DPCM feel though?  I think either the function should be
removed or it should be left with in #if 0, it's those that are the
issue with the change.
Kuninori Morimoto Aug. 21, 2024, 11:50 p.m. UTC | #4 
Hi

> > Of cource we can remove it and re-added later, but I'm afraid the person
> > who will re-add it might not noticed that there was a code before, and
> > re-add different determination methods, etc.
> > To avoid such things, this patch disables it by #if-endif.
(snip)
> I can see the symmetry argument, I don't know how other people working
> more with DPCM feel though?

Actually, I'm reading DPCM code, but unfortunately I still can't
understand details...

Thank you for your help !!

Best regards
---
Kuninori Morimoto
diff mbox series

Patch

diff --git a/include/sound/soc-dpcm.h b/include/sound/soc-dpcm.h
index 773f2db8c31c..c6fb350b4b06 100644
--- a/include/sound/soc-dpcm.h
+++ b/include/sound/soc-dpcm.h
@@ -113,24 +113,6 @@  struct snd_soc_dpcm_runtime {
 #define for_each_dpcm_be_rollback(fe, stream, _dpcm)			\
 	list_for_each_entry_continue_reverse(_dpcm, &(fe)->dpcm[stream].be_clients, list_be)
 
-/* can this BE stop and free */
-int snd_soc_dpcm_can_be_free_stop(struct snd_soc_pcm_runtime *fe,
-		struct snd_soc_pcm_runtime *be, int stream);
-
-/* can this BE perform a hw_params() */
-int snd_soc_dpcm_can_be_params(struct snd_soc_pcm_runtime *fe,
-		struct snd_soc_pcm_runtime *be, int stream);
-
-/* can this BE perform prepare */
-int snd_soc_dpcm_can_be_prepared(struct snd_soc_pcm_runtime *fe,
-				 struct snd_soc_pcm_runtime *be, int stream);
-
-/* is the current PCM operation for this FE ? */
-int snd_soc_dpcm_fe_can_update(struct snd_soc_pcm_runtime *fe, int stream);
-
-/* is the current PCM operation for this BE ? */
-int snd_soc_dpcm_be_can_update(struct snd_soc_pcm_runtime *fe,
-		struct snd_soc_pcm_runtime *be, int stream);
 
 /* get the substream for this BE */
 struct snd_pcm_substream *
diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c
index 667d8dff1314..c421eb663a3c 100644
--- a/sound/soc/soc-pcm.c
+++ b/sound/soc/soc-pcm.c
@@ -50,16 +50,17 @@  static inline int _soc_pcm_ret(struct snd_soc_pcm_runtime *rtd,
 }
 
 /* is the current PCM operation for this FE ? */
-int snd_soc_dpcm_fe_can_update(struct snd_soc_pcm_runtime *fe, int stream)
+#if 0
+static int snd_soc_dpcm_fe_can_update(struct snd_soc_pcm_runtime *fe, int stream)
 {
 	if (fe->dpcm[stream].runtime_update == SND_SOC_DPCM_UPDATE_FE)
 		return 1;
 	return 0;
 }
-EXPORT_SYMBOL_GPL(snd_soc_dpcm_fe_can_update);
+#endif
 
 /* is the current PCM operation for this BE ? */
-int snd_soc_dpcm_be_can_update(struct snd_soc_pcm_runtime *fe,
+static int snd_soc_dpcm_be_can_update(struct snd_soc_pcm_runtime *fe,
 			       struct snd_soc_pcm_runtime *be, int stream)
 {
 	if ((fe->dpcm[stream].runtime_update == SND_SOC_DPCM_UPDATE_FE) ||
@@ -68,7 +69,6 @@  int snd_soc_dpcm_be_can_update(struct snd_soc_pcm_runtime *fe,
 		return 1;
 	return 0;
 }
-EXPORT_SYMBOL_GPL(snd_soc_dpcm_be_can_update);
 
 static int snd_soc_dpcm_check_state(struct snd_soc_pcm_runtime *fe,
 				    struct snd_soc_pcm_runtime *be,
@@ -103,8 +103,8 @@  static int snd_soc_dpcm_check_state(struct snd_soc_pcm_runtime *fe,
  * We can only hw_free, stop, pause or suspend a BE DAI if any of it's FE
  * are not running, paused or suspended for the specified stream direction.
  */
-int snd_soc_dpcm_can_be_free_stop(struct snd_soc_pcm_runtime *fe,
-				  struct snd_soc_pcm_runtime *be, int stream)
+static int snd_soc_dpcm_can_be_free_stop(struct snd_soc_pcm_runtime *fe,
+					 struct snd_soc_pcm_runtime *be, int stream)
 {
 	const enum snd_soc_dpcm_state state[] = {
 		SND_SOC_DPCM_STATE_START,
@@ -114,14 +114,13 @@  int snd_soc_dpcm_can_be_free_stop(struct snd_soc_pcm_runtime *fe,
 
 	return snd_soc_dpcm_check_state(fe, be, stream, state, ARRAY_SIZE(state));
 }
-EXPORT_SYMBOL_GPL(snd_soc_dpcm_can_be_free_stop);
 
 /*
  * We can only change hw params a BE DAI if any of it's FE are not prepared,
  * running, paused or suspended for the specified stream direction.
  */
-int snd_soc_dpcm_can_be_params(struct snd_soc_pcm_runtime *fe,
-			       struct snd_soc_pcm_runtime *be, int stream)
+static int snd_soc_dpcm_can_be_params(struct snd_soc_pcm_runtime *fe,
+				      struct snd_soc_pcm_runtime *be, int stream)
 {
 	const enum snd_soc_dpcm_state state[] = {
 		SND_SOC_DPCM_STATE_START,
@@ -132,14 +131,13 @@  int snd_soc_dpcm_can_be_params(struct snd_soc_pcm_runtime *fe,
 
 	return snd_soc_dpcm_check_state(fe, be, stream, state, ARRAY_SIZE(state));
 }
-EXPORT_SYMBOL_GPL(snd_soc_dpcm_can_be_params);
 
 /*
  * We can only prepare a BE DAI if any of it's FE are not prepared,
  * running or paused for the specified stream direction.
  */
-int snd_soc_dpcm_can_be_prepared(struct snd_soc_pcm_runtime *fe,
-				 struct snd_soc_pcm_runtime *be, int stream)
+static int snd_soc_dpcm_can_be_prepared(struct snd_soc_pcm_runtime *fe,
+					struct snd_soc_pcm_runtime *be, int stream)
 {
 	const enum snd_soc_dpcm_state state[] = {
 		SND_SOC_DPCM_STATE_START,
@@ -149,7 +147,6 @@  int snd_soc_dpcm_can_be_prepared(struct snd_soc_pcm_runtime *fe,
 
 	return snd_soc_dpcm_check_state(fe, be, stream, state, ARRAY_SIZE(state));
 }
-EXPORT_SYMBOL_GPL(snd_soc_dpcm_can_be_prepared);
 
 #define DPCM_MAX_BE_USERS	8