[4.19.y-cip,24/57] ASoC: rsnd: rsnd_mod_name() handles both name and ID
diff mbox series

Message ID 1571295929-47286-25-git-send-email-biju.das@bp.renesas.com
State New
Headers show
Series
  • Audio improvements/SSIU BUSIF/
Related show

Commit Message

Biju Das Oct. 17, 2019, 7:04 a.m. UTC
From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

commit c0ea089dbad47a41ae30ad290766d7a6571c9802 upstream.

Current rsnd driver is using "%s[%d]" for mod name and ID,
but, this ID portion might confusable.
For example currently, CTU ID is 0 to 7, but using 00 to 13
(= 00, 01, 02, 03, 10, 11, 12, 13) is very best matching to datasheet.

In the future, we will support BUSIFn, but it will be more complicated
numbering. To avoid future confusable code, this patch modify
rsnd_mod_name() to return understandable name.

To avoid using pointless memory, it uses static char and snprintf,
thus, rsnd_mod_name() user should use it immediately, and shouldn't keep
its pointer.

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Signed-off-by: Mark Brown <broonie@kernel.org>
Signed-off-by: Biju Das <biju.das@bp.renesas.com>
---
 sound/soc/sh/rcar/core.c | 41 ++++++++++++++++++++++++++++++++---------
 sound/soc/sh/rcar/dma.c  | 21 ++++++++-------------
 sound/soc/sh/rcar/gen.c  | 12 ++++++------
 sound/soc/sh/rcar/rsnd.h |  2 +-
 sound/soc/sh/rcar/src.c  |  5 ++---
 sound/soc/sh/rcar/ssi.c  | 23 ++++++++---------------
 6 files changed, 57 insertions(+), 47 deletions(-)

Comments

Pavel Machek Oct. 20, 2019, 10:34 a.m. UTC | #1
Hi!

> In the future, we will support BUSIFn, but it will be more complicated
> numbering. To avoid future confusable code, this patch modify
> rsnd_mod_name() to return understandable name.
> 
> To avoid using pointless memory, it uses static char and snprintf,
> thus, rsnd_mod_name() user should use it immediately, and shouldn't keep
> its pointer.

No, don't do that. That's a dangerous interface.

> +#define MOD_NAME_SIZE 16
> +char *rsnd_mod_name(struct rsnd_mod *mod)
> +{
> +	static char name[MOD_NAME_SIZE];
> +
> +	/*
> +	 * Let's use same char to avoid pointlessness memory
> +	 * Thus, rsnd_mod_name() should be used immediately
> +	 * Don't keep pointer
> +	 */
> +	if ((mod)->ops->id_sub) {
> +		snprintf(name, MOD_NAME_SIZE, "%s[%d%d]",
> +			 mod->ops->name,
> +			 rsnd_mod_id(mod),
> +			 rsnd_mod_id_sub(mod));
> +	} else {
> +		snprintf(name, MOD_NAME_SIZE, "%s[%d]",
> +			 mod->ops->name,
> +			 rsnd_mod_id(mod));
> +	}
> +
> +	return name;
> +}

This is too hard to use correctly.

> @@ -762,12 +759,10 @@ static int rsnd_dma_alloc(struct rsnd_dai_stream *io, struct rsnd_mod *mod,
>  	if (ret < 0)
>  		return ret;
>  
> -	dev_dbg(dev, "%s[%d] %s[%d] -> %s[%d]\n",
> -		rsnd_mod_name(*dma_mod), rsnd_mod_id(*dma_mod),
> +	dev_dbg(dev, "%s %s -> %s\n",
> +		rsnd_mod_name(*dma_mod),
>  		rsnd_mod_name(mod_from ? mod_from : &mem),
> -		rsnd_mod_id  (mod_from ? mod_from : &mem),
> -		rsnd_mod_name(mod_to   ? mod_to   : &mem),
> -		rsnd_mod_id  (mod_to   ? mod_to   : &mem));
> +		rsnd_mod_name(mod_to   ? mod_to   : &mem));
>  
>  	ret = attach(io, dma, mod_from, mod_to);
>  	if (ret < 0)

For example this usage is not correct.

And yes, I took a look at mainline, and no, having 5 static buffers
is not really an acceptable solution.

Best regards,
								Pavel
Biju Das Oct. 21, 2019, 1:26 p.m. UTC | #2
Hi Pavel,

Thanks for the feedback.

> Subject: Re: [PATCH 4.19.y-cip 24/57] ASoC: rsnd: rsnd_mod_name() handles
> both name and ID
> 
> Hi!
> 
> > In the future, we will support BUSIFn, but it will be more complicated
> > numbering. To avoid future confusable code, this patch modify
> > rsnd_mod_name() to return understandable name.
> >
> > To avoid using pointless memory, it uses static char and snprintf,
> > thus, rsnd_mod_name() user should use it immediately, and shouldn't
> > keep its pointer.
> 
> No, don't do that. That's a dangerous interface.

Can you please elaborate more on the dangerous interface ?

> > +#define MOD_NAME_SIZE 16
> > +char *rsnd_mod_name(struct rsnd_mod *mod) {
> > +	static char name[MOD_NAME_SIZE];
> > +
> > +	/*
> > +	 * Let's use same char to avoid pointlessness memory
> > +	 * Thus, rsnd_mod_name() should be used immediately
> > +	 * Don't keep pointer
> > +	 */
> > +	if ((mod)->ops->id_sub) {
> > +		snprintf(name, MOD_NAME_SIZE, "%s[%d%d]",
> > +			 mod->ops->name,
> > +			 rsnd_mod_id(mod),
> > +			 rsnd_mod_id_sub(mod));
> > +	} else {
> > +		snprintf(name, MOD_NAME_SIZE, "%s[%d]",
> > +			 mod->ops->name,
> > +			 rsnd_mod_id(mod));
> > +	}
> > +
> > +	return name;
> > +}
> 
> This is too hard to use correctly.

Can you please suggest the correct way of doing this?

> > @@ -762,12 +759,10 @@ static int rsnd_dma_alloc(struct rsnd_dai_stream
> *io, struct rsnd_mod *mod,
> >  	if (ret < 0)
> >  		return ret;
> >
> > -	dev_dbg(dev, "%s[%d] %s[%d] -> %s[%d]\n",
> > -		rsnd_mod_name(*dma_mod), rsnd_mod_id(*dma_mod),
> > +	dev_dbg(dev, "%s %s -> %s\n",
> > +		rsnd_mod_name(*dma_mod),
> >  		rsnd_mod_name(mod_from ? mod_from : &mem),
> > -		rsnd_mod_id  (mod_from ? mod_from : &mem),
> > -		rsnd_mod_name(mod_to   ? mod_to   : &mem),
> > -		rsnd_mod_id  (mod_to   ? mod_to   : &mem));
> > +		rsnd_mod_name(mod_to   ? mod_to   : &mem));
> >
> >  	ret = attach(io, dma, mod_from, mod_to);
> >  	if (ret < 0)
> 
> For example this usage is not correct.
> 
> And yes, I took a look at mainline, and no, having 5 static buffers is not really
> an acceptable solution.
> 
Cheers,
Biju
Pavel Machek Nov. 18, 2019, 3:34 p.m. UTC | #3
Hi!

> > > In the future, we will support BUSIFn, but it will be more complicated
> > > numbering. To avoid future confusable code, this patch modify
> > > rsnd_mod_name() to return understandable name.
> > >
> > > To avoid using pointless memory, it uses static char and snprintf,
> > > thus, rsnd_mod_name() user should use it immediately, and shouldn't
> > > keep its pointer.
> > 
> > No, don't do that. That's a dangerous interface.
> 
> Can you please elaborate more on the dangerous interface ?
> 
> > > +#define MOD_NAME_SIZE 16
> > > +char *rsnd_mod_name(struct rsnd_mod *mod) {
> > > +	static char name[MOD_NAME_SIZE];
> > > +
> > > +	/*
> > > +	 * Let's use same char to avoid pointlessness memory
> > > +	 * Thus, rsnd_mod_name() should be used immediately
> > > +	 * Don't keep pointer
> > > +	 */
> > > +	if ((mod)->ops->id_sub) {
> > > +		snprintf(name, MOD_NAME_SIZE, "%s[%d%d]",
> > > +			 mod->ops->name,
> > > +			 rsnd_mod_id(mod),
> > > +			 rsnd_mod_id_sub(mod));
> > > +	} else {
> > > +		snprintf(name, MOD_NAME_SIZE, "%s[%d]",
> > > +			 mod->ops->name,
> > > +			 rsnd_mod_id(mod));
> > > +	}
> > > +
> > > +	return name;
> > > +}
> > 
> > This is too hard to use correctly.
> 
> Can you please suggest the correct way of doing this?

void rsnd_mod_name(char *buf, struct rsnd_mod *mod) {} would be usual
way of doing it. Semantics would be similar to sprintf().

Best regards,
							Pavel
Biju Das Nov. 18, 2019, 3:41 p.m. UTC | #4
+ Morimoto-San

> Subject: Re: [PATCH 4.19.y-cip 24/57] ASoC: rsnd: rsnd_mod_name() handles
> both name and ID
> 
> Hi!
> 
> > > > In the future, we will support BUSIFn, but it will be more
> > > > complicated numbering. To avoid future confusable code, this patch
> > > > modify
> > > > rsnd_mod_name() to return understandable name.
> > > >
> > > > To avoid using pointless memory, it uses static char and snprintf,
> > > > thus, rsnd_mod_name() user should use it immediately, and
> > > > shouldn't keep its pointer.
> > >
> > > No, don't do that. That's a dangerous interface.
> >
> > Can you please elaborate more on the dangerous interface ?
> >
> > > > +#define MOD_NAME_SIZE 16
> > > > +char *rsnd_mod_name(struct rsnd_mod *mod) {
> > > > +	static char name[MOD_NAME_SIZE];
> > > > +
> > > > +	/*
> > > > +	 * Let's use same char to avoid pointlessness memory
> > > > +	 * Thus, rsnd_mod_name() should be used immediately
> > > > +	 * Don't keep pointer
> > > > +	 */
> > > > +	if ((mod)->ops->id_sub) {
> > > > +		snprintf(name, MOD_NAME_SIZE, "%s[%d%d]",
> > > > +			 mod->ops->name,
> > > > +			 rsnd_mod_id(mod),
> > > > +			 rsnd_mod_id_sub(mod));
> > > > +	} else {
> > > > +		snprintf(name, MOD_NAME_SIZE, "%s[%d]",
> > > > +			 mod->ops->name,
> > > > +			 rsnd_mod_id(mod));
> > > > +	}
> > > > +
> > > > +	return name;
> > > > +}
> > >
> > > This is too hard to use correctly.
> >
> > Can you please suggest the correct way of doing this?
> 
> void rsnd_mod_name(char *buf, struct rsnd_mod *mod) {} would be usual
> way of doing it. Semantics would be similar to sprintf().

Morimoto-San, 
	Please share your opinion on this.

Regards,
Biju
Kuninori Morimoto Nov. 19, 2019, 1:03 a.m. UTC | #5
Hi Biju

> > > > > In the future, we will support BUSIFn, but it will be more
> > > > > complicated numbering. To avoid future confusable code, this patch
> > > > > modify
> > > > > rsnd_mod_name() to return understandable name.
> > > > >
> > > > > To avoid using pointless memory, it uses static char and snprintf,
> > > > > thus, rsnd_mod_name() user should use it immediately, and
> > > > > shouldn't keep its pointer.
> > > >
> > > > No, don't do that. That's a dangerous interface.
> > >
> > > Can you please elaborate more on the dangerous interface ?
> > >
> > > > > +#define MOD_NAME_SIZE 16
> > > > > +char *rsnd_mod_name(struct rsnd_mod *mod) {
> > > > > +	static char name[MOD_NAME_SIZE];
> > > > > +
> > > > > +	/*
> > > > > +	 * Let's use same char to avoid pointlessness memory
> > > > > +	 * Thus, rsnd_mod_name() should be used immediately
> > > > > +	 * Don't keep pointer
> > > > > +	 */
> > > > > +	if ((mod)->ops->id_sub) {
> > > > > +		snprintf(name, MOD_NAME_SIZE, "%s[%d%d]",
> > > > > +			 mod->ops->name,
> > > > > +			 rsnd_mod_id(mod),
> > > > > +			 rsnd_mod_id_sub(mod));
> > > > > +	} else {
> > > > > +		snprintf(name, MOD_NAME_SIZE, "%s[%d]",
> > > > > +			 mod->ops->name,
> > > > > +			 rsnd_mod_id(mod));
> > > > > +	}
> > > > > +
> > > > > +	return name;
> > > > > +}
> > > >
> > > > This is too hard to use correctly.
> > >
> > > Can you please suggest the correct way of doing this?
> > 
> > void rsnd_mod_name(char *buf, struct rsnd_mod *mod) {} would be usual
> > way of doing it. Semantics would be similar to sprintf().
> 
> Morimoto-San, 
> 	Please share your opinion on this.

rsnd_mod_name() is used for debug or error case,
and not used in Hi frequency.
At least, it doesn't used over 5 (= MOD_NAME_NUM) times in the same time.
So, it is no problem *at this point*.
But, indeed it is including potential issue...

Thank you for your help !!
Best regards
---
Kuninori Morimoto

Patch
diff mbox series

diff --git a/sound/soc/sh/rcar/core.c b/sound/soc/sh/rcar/core.c
index 93848f4..6023fb3 100644
--- a/sound/soc/sh/rcar/core.c
+++ b/sound/soc/sh/rcar/core.c
@@ -123,8 +123,8 @@  void rsnd_mod_make_sure(struct rsnd_mod *mod, enum rsnd_mod_type type)
 		struct rsnd_priv *priv = rsnd_mod_to_priv(mod);
 		struct device *dev = rsnd_priv_to_dev(priv);
 
-		dev_warn(dev, "%s[%d] is not your expected module\n",
-			 rsnd_mod_name(mod), rsnd_mod_id(mod));
+		dev_warn(dev, "%s is not your expected module\n",
+			 rsnd_mod_name(mod));
 	}
 }
 
@@ -137,6 +137,30 @@  struct dma_chan *rsnd_mod_dma_req(struct rsnd_dai_stream *io,
 	return mod->ops->dma_req(io, mod);
 }
 
+#define MOD_NAME_SIZE 16
+char *rsnd_mod_name(struct rsnd_mod *mod)
+{
+	static char name[MOD_NAME_SIZE];
+
+	/*
+	 * Let's use same char to avoid pointlessness memory
+	 * Thus, rsnd_mod_name() should be used immediately
+	 * Don't keep pointer
+	 */
+	if ((mod)->ops->id_sub) {
+		snprintf(name, MOD_NAME_SIZE, "%s[%d%d]",
+			 mod->ops->name,
+			 rsnd_mod_id(mod),
+			 rsnd_mod_id_sub(mod));
+	} else {
+		snprintf(name, MOD_NAME_SIZE, "%s[%d]",
+			 mod->ops->name,
+			 rsnd_mod_id(mod));
+	}
+
+	return name;
+}
+
 u32 *rsnd_mod_get_status(struct rsnd_mod *mod,
 			 struct rsnd_dai_stream *io,
 			 enum rsnd_mod_type type)
@@ -494,15 +518,14 @@  static int rsnd_status_update(u32 *status,
 						__rsnd_mod_shift_##fn,	\
 						__rsnd_mod_add_##fn,	\
 						__rsnd_mod_call_##fn);	\
-		rsnd_dbg_dai_call(dev, "%s[%d]\t0x%08x %s\n",		\
-			rsnd_mod_name(mod), rsnd_mod_id(mod), *status,	\
+		rsnd_dbg_dai_call(dev, "%s\t0x%08x %s\n",		\
+			rsnd_mod_name(mod), *status,	\
 			(func_call && (mod)->ops->fn) ? #fn : "");	\
 		if (func_call && (mod)->ops->fn)			\
 			tmp = (mod)->ops->fn(mod, io, param);		\
 		if (tmp && (tmp != -EPROBE_DEFER))			\
-			dev_err(dev, "%s[%d] : %s error %d\n",		\
-				rsnd_mod_name(mod), rsnd_mod_id(mod),	\
-						     #fn, tmp);		\
+			dev_err(dev, "%s : %s error %d\n",		\
+				rsnd_mod_name(mod), #fn, tmp);		\
 		ret |= tmp;						\
 	}								\
 	ret;								\
@@ -529,8 +552,8 @@  int rsnd_dai_connect(struct rsnd_mod *mod,
 
 	io->mod[type] = mod;
 
-	dev_dbg(dev, "%s[%d] is connected to io (%s)\n",
-		rsnd_mod_name(mod), rsnd_mod_id(mod),
+	dev_dbg(dev, "%s is connected to io (%s)\n",
+		rsnd_mod_name(mod),
 		rsnd_io_is_play(io) ? "Playback" : "Capture");
 
 	return 0;
diff --git a/sound/soc/sh/rcar/dma.c b/sound/soc/sh/rcar/dma.c
index e5c30ee..5daa6c9 100644
--- a/sound/soc/sh/rcar/dma.c
+++ b/sound/soc/sh/rcar/dma.c
@@ -174,8 +174,8 @@  static int rsnd_dmaen_start(struct rsnd_mod *mod,
 	cfg.src_addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES;
 	cfg.dst_addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES;
 
-	dev_dbg(dev, "%s[%d] %pad -> %pad\n",
-		rsnd_mod_name(mod), rsnd_mod_id(mod),
+	dev_dbg(dev, "%s %pad -> %pad\n",
+		rsnd_mod_name(mod),
 		&cfg.src_addr, &cfg.dst_addr);
 
 	ret = dmaengine_slave_config(dmaen->chan, &cfg);
@@ -369,8 +369,7 @@  static u32 rsnd_dmapp_get_id(struct rsnd_dai_stream *io,
 	if ((!entry) || (size <= id)) {
 		struct device *dev = rsnd_priv_to_dev(rsnd_io_to_priv(io));
 
-		dev_err(dev, "unknown connection (%s[%d])\n",
-			rsnd_mod_name(mod), rsnd_mod_id(mod));
+		dev_err(dev, "unknown connection (%s)\n", rsnd_mod_name(mod));
 
 		/* use non-prohibited SRS number as error */
 		return 0x00; /* SSI00 */
@@ -692,12 +691,10 @@  static void rsnd_dma_of_path(struct rsnd_mod *this,
 		*mod_to		= mod[1];
 	}
 
-	dev_dbg(dev, "module connection (this is %s[%d])\n",
-		rsnd_mod_name(this), rsnd_mod_id(this));
+	dev_dbg(dev, "module connection (this is %s)\n", rsnd_mod_name(this));
 	for (i = 0; i <= idx; i++) {
-		dev_dbg(dev, "  %s[%d]%s\n",
+		dev_dbg(dev, "  %s%s\n",
 			rsnd_mod_name(mod[i] ? mod[i] : &mem),
-			rsnd_mod_id  (mod[i] ? mod[i] : &mem),
 			(mod[i] == *mod_from) ? " from" :
 			(mod[i] == *mod_to)   ? " to" : "");
 	}
@@ -762,12 +759,10 @@  static int rsnd_dma_alloc(struct rsnd_dai_stream *io, struct rsnd_mod *mod,
 	if (ret < 0)
 		return ret;
 
-	dev_dbg(dev, "%s[%d] %s[%d] -> %s[%d]\n",
-		rsnd_mod_name(*dma_mod), rsnd_mod_id(*dma_mod),
+	dev_dbg(dev, "%s %s -> %s\n",
+		rsnd_mod_name(*dma_mod),
 		rsnd_mod_name(mod_from ? mod_from : &mem),
-		rsnd_mod_id  (mod_from ? mod_from : &mem),
-		rsnd_mod_name(mod_to   ? mod_to   : &mem),
-		rsnd_mod_id  (mod_to   ? mod_to   : &mem));
+		rsnd_mod_name(mod_to   ? mod_to   : &mem));
 
 	ret = attach(io, dma, mod_from, mod_to);
 	if (ret < 0)
diff --git a/sound/soc/sh/rcar/gen.c b/sound/soc/sh/rcar/gen.c
index 1f7881cc..ca63940 100644
--- a/sound/soc/sh/rcar/gen.c
+++ b/sound/soc/sh/rcar/gen.c
@@ -83,8 +83,8 @@  u32 rsnd_read(struct rsnd_priv *priv,
 
 	regmap_fields_read(gen->regs[reg], rsnd_mod_id(mod), &val);
 
-	dev_dbg(dev, "r %s[%d] - %-18s (%4d) : %08x\n",
-		rsnd_mod_name(mod), rsnd_mod_id(mod),
+	dev_dbg(dev, "r %s - %-18s (%4d) : %08x\n",
+		rsnd_mod_name(mod),
 		rsnd_reg_name(gen, reg), reg, val);
 
 	return val;
@@ -102,8 +102,8 @@  void rsnd_write(struct rsnd_priv *priv,
 
 	regmap_fields_force_write(gen->regs[reg], rsnd_mod_id(mod), data);
 
-	dev_dbg(dev, "w %s[%d] - %-18s (%4d) : %08x\n",
-		rsnd_mod_name(mod), rsnd_mod_id(mod),
+	dev_dbg(dev, "w %s - %-18s (%4d) : %08x\n",
+		rsnd_mod_name(mod),
 		rsnd_reg_name(gen, reg), reg, data);
 }
 
@@ -119,8 +119,8 @@  void rsnd_bset(struct rsnd_priv *priv, struct rsnd_mod *mod,
 	regmap_fields_force_update_bits(gen->regs[reg],
 					rsnd_mod_id(mod), mask, data);
 
-	dev_dbg(dev, "b %s[%d] - %-18s (%4d) : %08x/%08x\n",
-		rsnd_mod_name(mod), rsnd_mod_id(mod),
+	dev_dbg(dev, "b %s - %-18s (%4d) : %08x/%08x\n",
+		rsnd_mod_name(mod),
 		rsnd_reg_name(gen, reg), reg, data, mask);
 
 }
diff --git a/sound/soc/sh/rcar/rsnd.h b/sound/soc/sh/rcar/rsnd.h
index fdf007a..28bd90a 100644
--- a/sound/soc/sh/rcar/rsnd.h
+++ b/sound/soc/sh/rcar/rsnd.h
@@ -377,7 +377,6 @@  struct rsnd_mod {
 #define __rsnd_mod_call_pointer		0
 
 #define rsnd_mod_to_priv(mod)	((mod)->priv)
-#define rsnd_mod_name(mod)	((mod)->ops->name)
 #define rsnd_mod_power_on(mod)	clk_enable((mod)->clk)
 #define rsnd_mod_power_off(mod)	clk_disable((mod)->clk)
 #define rsnd_mod_get(ip)	(&(ip)->mod)
@@ -400,6 +399,7 @@  u32 *rsnd_mod_get_status(struct rsnd_mod *mod,
 int rsnd_mod_id(struct rsnd_mod *mod);
 int rsnd_mod_id_raw(struct rsnd_mod *mod);
 int rsnd_mod_id_sub(struct rsnd_mod *mod);
+char *rsnd_mod_name(struct rsnd_mod *mod);
 struct rsnd_mod *rsnd_mod_next(int *iterator,
 			       struct rsnd_dai_stream *io,
 			       enum rsnd_mod_type *array,
diff --git a/sound/soc/sh/rcar/src.c b/sound/soc/sh/rcar/src.c
index 7de7afd..bdc0595 100644
--- a/sound/soc/sh/rcar/src.c
+++ b/sound/soc/sh/rcar/src.c
@@ -349,9 +349,8 @@  static bool rsnd_src_error_occurred(struct rsnd_mod *mod)
 	status0 = rsnd_mod_read(mod, SCU_SYS_STATUS0);
 	status1 = rsnd_mod_read(mod, SCU_SYS_STATUS1);
 	if ((status0 & val0) || (status1 & val1)) {
-		rsnd_dbg_irq_status(dev, "%s[%d] err status : 0x%08x, 0x%08x\n",
-			rsnd_mod_name(mod), rsnd_mod_id(mod),
-			status0, status1);
+		rsnd_dbg_irq_status(dev, "%s err status : 0x%08x, 0x%08x\n",
+			rsnd_mod_name(mod), status0, status1);
 
 		ret = true;
 	}
diff --git a/sound/soc/sh/rcar/ssi.c b/sound/soc/sh/rcar/ssi.c
index bf14207..81d3864 100644
--- a/sound/soc/sh/rcar/ssi.c
+++ b/sound/soc/sh/rcar/ssi.c
@@ -181,8 +181,7 @@  static void rsnd_ssi_status_check(struct rsnd_mod *mod,
 		udelay(5);
 	}
 
-	dev_warn(dev, "%s[%d] status check failed\n",
-		 rsnd_mod_name(mod), rsnd_mod_id(mod));
+	dev_warn(dev, "%s status check failed\n", rsnd_mod_name(mod));
 }
 
 static u32 rsnd_ssi_multi_slaves(struct rsnd_dai_stream *io)
@@ -346,9 +345,7 @@  static int rsnd_ssi_master_clk_start(struct rsnd_mod *mod,
 	ssi->rate = rate;
 	ssi->chan = chan;
 
-	dev_dbg(dev, "%s[%d] outputs %u Hz\n",
-		rsnd_mod_name(mod),
-		rsnd_mod_id(mod), rate);
+	dev_dbg(dev, "%s outputs %u Hz\n", rsnd_mod_name(mod), rate);
 
 	return 0;
 }
@@ -494,8 +491,7 @@  static int rsnd_ssi_quit(struct rsnd_mod *mod,
 		return 0;
 
 	if (!ssi->usrcnt) {
-		dev_err(dev, "%s[%d] usrcnt error\n",
-			rsnd_mod_name(mod), rsnd_mod_id(mod));
+		dev_err(dev, "%s usrcnt error\n", rsnd_mod_name(mod));
 		return -EIO;
 	}
 
@@ -654,8 +650,8 @@  static void __rsnd_ssi_interrupt(struct rsnd_mod *mod,
 
 	/* DMA only */
 	if (is_dma && (status & (UIRQ | OIRQ))) {
-		rsnd_dbg_irq_status(dev, "%s[%d] err status : 0x%08x\n",
-			rsnd_mod_name(mod), rsnd_mod_id(mod), status);
+		rsnd_dbg_irq_status(dev, "%s err status : 0x%08x\n",
+			rsnd_mod_name(mod), status);
 
 		stop = true;
 	}
@@ -964,8 +960,7 @@  static int rsnd_ssi_fallback(struct rsnd_mod *mod,
 	 */
 	mod->ops = &rsnd_ssi_pio_ops;
 
-	dev_info(dev, "%s[%d] fallback to PIO mode\n",
-		 rsnd_mod_name(mod), rsnd_mod_id(mod));
+	dev_info(dev, "%s fallback to PIO mode\n", rsnd_mod_name(mod));
 
 	return 0;
 }
@@ -1085,15 +1080,13 @@  static void __rsnd_ssi_parse_hdmi_connection(struct rsnd_priv *priv,
 	/* HDMI0 */
 	if (strstr(remote_node->full_name, "hdmi@fead0000")) {
 		rsnd_flags_set(ssi, RSND_SSI_HDMI0);
-		dev_dbg(dev, "%s[%d] connected to HDMI0\n",
-			 rsnd_mod_name(mod), rsnd_mod_id(mod));
+		dev_dbg(dev, "%s connected to HDMI0\n", rsnd_mod_name(mod));
 	}
 
 	/* HDMI1 */
 	if (strstr(remote_node->full_name, "hdmi@feae0000")) {
 		rsnd_flags_set(ssi, RSND_SSI_HDMI1);
-		dev_dbg(dev, "%s[%d] connected to HDMI1\n",
-			rsnd_mod_name(mod), rsnd_mod_id(mod));
+		dev_dbg(dev, "%s connected to HDMI1\n", rsnd_mod_name(mod));
 	}
 }