diff mbox series

[4.19.y-cip,06/57] ASoC: rsnd: ssi: Check runtime channel number rather than hw_params

Message ID 1571295929-47286-7-git-send-email-biju.das@bp.renesas.com (mailing list archive)
State Changes Requested
Headers show
Series Audio improvements/SSIU BUSIF/ | expand

Commit Message

Biju Das Oct. 17, 2019, 7:04 a.m. UTC
From: Jiada Wang <jiada_wang@mentor.com>

commit 599da084e041b877ef89211dcbb4c7bd8380049d upstream.

The number of channel handled by SSI maybe differs from the one set
in hw_params, currently SSI checks hw_params's channel number,
and constrains to use same channel number, when it is being
used by multiple clients.

This patch corrects to check runtime channel number rather
than channel number set in hw_params.

Signed-off-by: Jiada Wang <jiada_wang@mentor.com>
Signed-off-by: Timo Wischer <twischer@de.adit-jv.com>
[kuninori: adjust to upstreaming]
Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Tested-by: Hiroyuki Yokoyama <hiroyuki.yokoyama.vx@renesas.com>
Signed-off-by: Mark Brown <broonie@kernel.org>
Signed-off-by: Biju Das <biju.das@bp.renesas.com>
---
 sound/soc/sh/rcar/ssi.c | 27 +++++++--------------------
 1 file changed, 7 insertions(+), 20 deletions(-)

Comments

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

> commit 599da084e041b877ef89211dcbb4c7bd8380049d upstream.
> 
> The number of channel handled by SSI maybe differs from the one set
> in hw_params, currently SSI checks hw_params's channel number,
> and constrains to use same channel number, when it is being
> used by multiple clients.
> 
> This patch corrects to check runtime channel number rather
> than channel number set in hw_params.

This also changes the error return here from -EIO to -EINVAL. I assume
that's okay, but I wanted to double check.

Best regards,
								Pavel


> @@ -307,6 +307,11 @@ static int rsnd_ssi_master_clk_start(struct rsnd_mod *mod,
>  			return -EINVAL;
>  		}
>  
> +		if (ssi->chan != chan) {
> +			dev_err(dev, "SSI parent/child should use same chan\n");
> +			return -EINVAL;
> +		}
> +
>  		return 0;
>  	}
>  
> @@ -519,24 +524,6 @@ static int rsnd_ssi_hw_params(struct rsnd_mod *mod,
> -	if (ssi->usrcnt) {
> -		/*
> -		 * Already working.
> -		 * It will happen if SSI has parent/child connection.
> -		 * it is error if child <-> parent SSI uses
> -		 * different channels.
> -		 */
> -		if (ssi->chan != chan)
> -			return -EIO;
> -	}
Biju Das Oct. 21, 2019, 12:10 p.m. UTC | #2
Hi Pavel,

Thanks for the feedback.

> Subject: Re: [PATCH 4.19.y-cip 06/57] ASoC: rsnd: ssi: Check runtime channel
> number rather than hw_params
> 
> Hi!
> 
> > commit 599da084e041b877ef89211dcbb4c7bd8380049d upstream.
> >
> > The number of channel handled by SSI maybe differs from the one set in
> > hw_params, currently SSI checks hw_params's channel number, and
> > constrains to use same channel number, when it is being used by
> > multiple clients.
> >
> > This patch corrects to check runtime channel number rather than
> > channel number set in hw_params.
> 
> This also changes the error return here from -EIO to -EINVAL. I assume that's
> okay, but I wanted to double check.
> 

Do you prefer -EIO rather than -EINVAL?  For me, -EINVAL make sense here.

Cheers,
Biju

> > +		if (ssi->chan != chan) {
> > +			dev_err(dev, "SSI parent/child should use same
> chan\n");
> > +			return -EINVAL;
> > +		}
> > +
> >  		return 0;
> >  	}
> >
> > @@ -519,24 +524,6 @@ static int rsnd_ssi_hw_params(struct rsnd_mod
> *mod,
> > -	if (ssi->usrcnt) {
> > -		/*
> > -		 * Already working.
> > -		 * It will happen if SSI has parent/child connection.
> > -		 * it is error if child <-> parent SSI uses
> > -		 * different channels.
> > -		 */
> > -		if (ssi->chan != chan)
> > -			return -EIO;
> > -	}
> 
> --
> DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
diff mbox series

Patch

diff --git a/sound/soc/sh/rcar/ssi.c b/sound/soc/sh/rcar/ssi.c
index acb77ae..06d340a 100644
--- a/sound/soc/sh/rcar/ssi.c
+++ b/sound/soc/sh/rcar/ssi.c
@@ -307,6 +307,11 @@  static int rsnd_ssi_master_clk_start(struct rsnd_mod *mod,
 			return -EINVAL;
 		}
 
+		if (ssi->chan != chan) {
+			dev_err(dev, "SSI parent/child should use same chan\n");
+			return -EINVAL;
+		}
+
 		return 0;
 	}
 
@@ -334,6 +339,7 @@  static int rsnd_ssi_master_clk_start(struct rsnd_mod *mod,
 			SCKD | SWSD | CKDV(idx);
 	ssi->wsr = CONT;
 	ssi->rate = rate;
+	ssi->chan = chan;
 
 	dev_dbg(dev, "%s[%d] outputs %u Hz\n",
 		rsnd_mod_name(mod),
@@ -359,6 +365,7 @@  static void rsnd_ssi_master_clk_stop(struct rsnd_mod *mod,
 
 	ssi->cr_clk	= 0;
 	ssi->rate	= 0;
+	ssi->chan	= 0;
 
 	rsnd_adg_ssi_clk_stop(mod);
 }
@@ -506,9 +513,7 @@  static int rsnd_ssi_hw_params(struct rsnd_mod *mod,
 			      struct snd_pcm_substream *substream,
 			      struct snd_pcm_hw_params *params)
 {
-	struct rsnd_ssi *ssi = rsnd_mod_to_ssi(mod);
 	struct rsnd_dai *rdai = rsnd_io_to_rdai(io);
-	int chan = params_channels(params);
 	unsigned int fmt_width = snd_pcm_format_width(params_format(params));
 
 	if (fmt_width > rdai->chan_width) {
@@ -519,24 +524,6 @@  static int rsnd_ssi_hw_params(struct rsnd_mod *mod,
 		return -EINVAL;
 	}
 
-	/*
-	 * snd_pcm_ops::hw_params will be called *before*
-	 * snd_soc_dai_ops::trigger. Thus, ssi->usrcnt is 0
-	 * in 1st call.
-	 */
-	if (ssi->usrcnt) {
-		/*
-		 * Already working.
-		 * It will happen if SSI has parent/child connection.
-		 * it is error if child <-> parent SSI uses
-		 * different channels.
-		 */
-		if (ssi->chan != chan)
-			return -EIO;
-	}
-
-	ssi->chan = chan;
-
 	return 0;
 }