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 |
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; > - }
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 --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; }