diff mbox series

[-next] ASoC: fsl_micfil: Remove set but not used variable 'osr'

Message ID 20190417150915.37968-1-yuehaibing@huawei.com (mailing list archive)
State Not Applicable
Headers show
Series [-next] ASoC: fsl_micfil: Remove set but not used variable 'osr' | expand

Checks

Context Check Description
snowpatch_ozlabs/apply_patch success Successfully applied on branch next (8c2ffd9174779014c3fe1f96d9dc3641d9175f00)
snowpatch_ozlabs/build-ppc64le success Build succeeded
snowpatch_ozlabs/build-ppc64be success Build succeeded
snowpatch_ozlabs/build-ppc64e success Build succeeded
snowpatch_ozlabs/build-pmac32 success Build succeeded
snowpatch_ozlabs/checkpatch warning total: 0 errors, 1 warnings, 0 checks, 12 lines checked

Commit Message

Yue Haibing April 17, 2019, 3:09 p.m. UTC
From: YueHaibing <yuehaibing@huawei.com>

Fixes gcc '-Wunused-but-set-variable' warning:

sound/soc/fsl/fsl_micfil.c: In function 'get_clk_div':
sound/soc/fsl/fsl_micfil.c:154:6: warning: variable 'osr' set but not used [-Wunused-but-set-variable]

It is never used since introduction in
commit 47a70e6fc9a8 ("ASoC: Add MICFIL SoC Digital Audio Interface driver.")

Signed-off-by: YueHaibing <yuehaibing@huawei.com>
---
 sound/soc/fsl/fsl_micfil.c | 3 ---
 1 file changed, 3 deletions(-)

Comments

Daniel Baluta April 17, 2019, 3:25 p.m. UTC | #1
Hi Yue,

Looks good to me. Just one question for Cosmin:

On Wed, Apr 17, 2019 at 6:10 PM Yue Haibing <yuehaibing@huawei.com> wrote:
>
> From: YueHaibing <yuehaibing@huawei.com>
>
> Fixes gcc '-Wunused-but-set-variable' warning:
>
> sound/soc/fsl/fsl_micfil.c: In function 'get_clk_div':
> sound/soc/fsl/fsl_micfil.c:154:6: warning: variable 'osr' set but not used [-Wunused-but-set-variable]
>
> It is never used since introduction in
> commit 47a70e6fc9a8 ("ASoC: Add MICFIL SoC Digital Audio Interface driver.")
>
> Signed-off-by: YueHaibing <yuehaibing@huawei.com>
> ---
>  sound/soc/fsl/fsl_micfil.c | 3 ---
>  1 file changed, 3 deletions(-)
>
> diff --git a/sound/soc/fsl/fsl_micfil.c b/sound/soc/fsl/fsl_micfil.c
> index 40c07e7..f7f2d29 100644
> --- a/sound/soc/fsl/fsl_micfil.c
> +++ b/sound/soc/fsl/fsl_micfil.c
> @@ -151,12 +151,9 @@ static inline int get_clk_div(struct fsl_micfil *micfil,
>  {
>         u32 ctrl2_reg;
>         long mclk_rate;
> -       int osr;
>         int clk_div;
>
>         regmap_read(micfil->regmap, REG_MICFIL_CTRL2, &ctrl2_reg);

I noticed now that we also read ctrl2_reg without using it. Is this as intended?

> -       osr = 16 - ((ctrl2_reg & MICFIL_CTRL2_CICOSR_MASK)
> -                   >> MICFIL_CTRL2_CICOSR_SHIFT);
>
>         mclk_rate = clk_get_rate(micfil->mclk);
>
> --
> 2.7.4
>
>
Nicolin Chen April 17, 2019, 6:35 p.m. UTC | #2
On Wed, Apr 17, 2019 at 06:25:11PM +0300, Daniel Baluta wrote:

> Looks good to me. Just one question for Cosmin:

> > diff --git a/sound/soc/fsl/fsl_micfil.c b/sound/soc/fsl/fsl_micfil.c
> > index 40c07e7..f7f2d29 100644
> > --- a/sound/soc/fsl/fsl_micfil.c
> > +++ b/sound/soc/fsl/fsl_micfil.c
> > @@ -151,12 +151,9 @@ static inline int get_clk_div(struct fsl_micfil *micfil,
> >  {
> >         u32 ctrl2_reg;
> >         long mclk_rate;
> > -       int osr;
> >         int clk_div;
> >
> >         regmap_read(micfil->regmap, REG_MICFIL_CTRL2, &ctrl2_reg);
> 
> I noticed now that we also read ctrl2_reg without using it. Is this as intended?

For Cosmin,

Same question here.

Btw, I just noticed that the initial patch of adding this driver
did not seemly include most of FSL maintainers/reviewers. Please
run get_maintainer.pl to add all listed reviewers when sending a
change to the maillist.

Thanks
Nicolin
Cosmin-Gabriel Samoila April 17, 2019, 6:58 p.m. UTC | #3
On Wed, 2019-04-17 at 11:35 -0700, Nicolin Chen wrote:
> On Wed, Apr 17, 2019 at 06:25:11PM +0300, Daniel Baluta wrote:
> 
> > Looks good to me. Just one question for Cosmin:
> > > diff --git a/sound/soc/fsl/fsl_micfil.c
> > > b/sound/soc/fsl/fsl_micfil.c
> > > index 40c07e7..f7f2d29 100644
> > > --- a/sound/soc/fsl/fsl_micfil.c
> > > +++ b/sound/soc/fsl/fsl_micfil.c
> > > @@ -151,12 +151,9 @@ static inline int get_clk_div(struct
> > > fsl_micfil *micfil,
> > >  {
> > >         u32 ctrl2_reg;
> > >         long mclk_rate;
> > > -       int osr;
> > >         int clk_div;
> > > 
> > >         regmap_read(micfil->regmap, REG_MICFIL_CTRL2,
> > > &ctrl2_reg);
> > 
> > I noticed now that we also read ctrl2_reg without using it. Is this
> > as intended?
> 
> For Cosmin,
> 
> Same question here.
> 
> Btw, I just noticed that the initial patch of adding this driver
> did not seemly include most of FSL maintainers/reviewers. Please
> run get_maintainer.pl to add all listed reviewers when sending a
> change to the maillist.
> 
> Thanks
> Nicolin

Hello,

The regmap_read is not used anymore in this implementation and I've
forgot to remove it - it can be safetly removed.

Nicolin, I am pretty sure I've ran the get_maintainer.pl script but
I will pay more attention next time... sorry if I forgot to add you.

Best regards,
Cosmin
Nicolin Chen April 17, 2019, 7:14 p.m. UTC | #4
On Wed, Apr 17, 2019 at 09:58:37PM +0300, gabrielcsmo@gmail.com wrote:

> Nicolin, I am pretty sure I've ran the get_maintainer.pl script but
> I will pay more attention next time... sorry if I forgot to add you.

It's okay for that one as I trust Mark's review anyway. Just
adding us would offload some of his burden, especially for a
device specific change.

Thanks
diff mbox series

Patch

diff --git a/sound/soc/fsl/fsl_micfil.c b/sound/soc/fsl/fsl_micfil.c
index 40c07e7..f7f2d29 100644
--- a/sound/soc/fsl/fsl_micfil.c
+++ b/sound/soc/fsl/fsl_micfil.c
@@ -151,12 +151,9 @@  static inline int get_clk_div(struct fsl_micfil *micfil,
 {
 	u32 ctrl2_reg;
 	long mclk_rate;
-	int osr;
 	int clk_div;
 
 	regmap_read(micfil->regmap, REG_MICFIL_CTRL2, &ctrl2_reg);
-	osr = 16 - ((ctrl2_reg & MICFIL_CTRL2_CICOSR_MASK)
-		    >> MICFIL_CTRL2_CICOSR_SHIFT);
 
 	mclk_rate = clk_get_rate(micfil->mclk);