[PATCH] sound: pci: cmipci.c: Cleaning up function that are not used anywhere

[PATCH] sound: pci: cmipci.c: Cleaning up function that are not used anywhere

[Rickard Strandqvist]

Remove function snd_cmipci_pll_rmn() that is not used anywhere.

This was partially found by using a static code analysis program called cppcheck.

Signed-off-by: Rickard Strandqvist <rickard_strandqvist@spectrumdigital.se>
---
 sound/pci/cmipci.c |   41 -----------------------------------------
 1 file changed, 41 deletions(-)

diff --git a/sound/pci/cmipci.c b/sound/pci/cmipci.c
index 85ed403..e33edae 100644
--- a/sound/pci/cmipci.c
+++ b/sound/pci/cmipci.c
@@ -615,47 +615,6 @@ static unsigned int snd_cmipci_rate_freq(unsigned int rate)
 
 #ifdef USE_VAR48KRATE
 /*
- * Determine PLL values for frequency setup, maybe the CMI8338 (CMI8738???)
- * does it this way .. maybe not.  Never get any information from C-Media about
- * that <werner@suse.de>.
- */
-static int snd_cmipci_pll_rmn(unsigned int rate, unsigned int adcmult, int *r, int *m, int *n)
-{
-	unsigned int delta, tolerance;
-	int xm, xn, xr;
-
-	for (*r = 0; rate < CM_MAXIMUM_RATE/adcmult; *r += (1<<5))
-		rate <<= 1;
-	*n = -1;
-	if (*r > 0xff)
-		goto out;
-	tolerance = rate*CM_TOLERANCE_RATE;
-
-	for (xn = (1+2); xn < (0x1f+2); xn++) {
-		for (xm = (1+2); xm < (0xff+2); xm++) {
-			xr = ((CM_REFFREQ_XIN/adcmult) * xm) / xn;
-
-			if (xr < rate)
-				delta = rate - xr;
-			else
-				delta = xr - rate;
-
-			/*
-			 * If we found one, remember this,
-			 * and try to find a closer one
-			 */
-			if (delta < tolerance) {
-				tolerance = delta;
-				*m = xm - 2;
-				*n = xn - 2;
-			}
-		}
-	}
-out:
-	return (*n > -1);
-}
-
-/*
  * Program pll register bits, I assume that the 8 registers 0xf8 up to 0xff
  * are mapped onto the 8 ADC/DAC sampling frequency which can be chosen
  * at the register CM_REG_FUNCTRL1 (0x04).
-- 
1.7.10.4

Re: [PATCH] sound: pci: cmipci.c: Cleaning up function that are not used anywhere

[Bjorn Helgaas]

Hi Rickard,

Thanks a lot for fixing cruft like this.  It makes life better for everybody.

I'll let the maintainer of this driver take care of this patch, but
I'll just point out that your subject ("Cleaning up function ...") is
slightly ambiguous because sometimes "cleanup" means simplification,
not removal.  My personal preference would be "Remove unused
snd_cmipci_pll_rmn()" because it is unambiguous, shorter, and contains
a little more information.  That makes "git log --oneline" output more
 useful.

Bjorn

On Tue, Dec 2, 2014 at 4:12 PM, Rickard Strandqvist
<rickard_strandqvist@spectrumdigital.se> wrote:
> Remove function snd_cmipci_pll_rmn() that is not used anywhere.
>
> This was partially found by using a static code analysis program called cppcheck.
>
> Signed-off-by: Rickard Strandqvist <rickard_strandqvist@spectrumdigital.se>
> ---
>  sound/pci/cmipci.c |   41 -----------------------------------------
>  1 file changed, 41 deletions(-)
>
> diff --git a/sound/pci/cmipci.c b/sound/pci/cmipci.c
> index 85ed403..e33edae 100644
> --- a/sound/pci/cmipci.c
> +++ b/sound/pci/cmipci.c
> @@ -615,47 +615,6 @@ static unsigned int snd_cmipci_rate_freq(unsigned int rate)
>
>  #ifdef USE_VAR48KRATE
>  /*
> - * Determine PLL values for frequency setup, maybe the CMI8338 (CMI8738???)
> - * does it this way .. maybe not.  Never get any information from C-Media about
> - * that <werner@suse.de>.
> - */
> -static int snd_cmipci_pll_rmn(unsigned int rate, unsigned int adcmult, int *r, int *m, int *n)
> -{
> -       unsigned int delta, tolerance;
> -       int xm, xn, xr;
> -
> -       for (*r = 0; rate < CM_MAXIMUM_RATE/adcmult; *r += (1<<5))
> -               rate <<= 1;
> -       *n = -1;
> -       if (*r > 0xff)
> -               goto out;
> -       tolerance = rate*CM_TOLERANCE_RATE;
> -
> -       for (xn = (1+2); xn < (0x1f+2); xn++) {
> -               for (xm = (1+2); xm < (0xff+2); xm++) {
> -                       xr = ((CM_REFFREQ_XIN/adcmult) * xm) / xn;
> -
> -                       if (xr < rate)
> -                               delta = rate - xr;
> -                       else
> -                               delta = xr - rate;
> -
> -                       /*
> -                        * If we found one, remember this,
> -                        * and try to find a closer one
> -                        */
> -                       if (delta < tolerance) {
> -                               tolerance = delta;
> -                               *m = xm - 2;
> -                               *n = xn - 2;
> -                       }
> -               }
> -       }
> -out:
> -       return (*n > -1);
> -}
> -
> -/*
>   * Program pll register bits, I assume that the 8 registers 0xf8 up to 0xff
>   * are mapped onto the 8 ADC/DAC sampling frequency which can be chosen
>   * at the register CM_REG_FUNCTRL1 (0x04).
> --
> 1.7.10.4
>

Re: [PATCH] sound: pci: cmipci.c: Cleaning up function that are not used anywhere

[Takashi Iwai]

At Wed,  3 Dec 2014 00:12:55 +0100,
Rickard Strandqvist wrote:
> 
> Remove function snd_cmipci_pll_rmn() that is not used anywhere.

It's an experimental code that is currently disabled.  Keeping such a
code would be still meaningful if anyone wants to start a similar
experiment again.


Takashi

> This was partially found by using a static code analysis program called cppcheck.
> 
> Signed-off-by: Rickard Strandqvist <rickard_strandqvist@spectrumdigital.se>
> ---
>  sound/pci/cmipci.c |   41 -----------------------------------------
>  1 file changed, 41 deletions(-)
> 
> diff --git a/sound/pci/cmipci.c b/sound/pci/cmipci.c
> index 85ed403..e33edae 100644
> --- a/sound/pci/cmipci.c
> +++ b/sound/pci/cmipci.c
> @@ -615,47 +615,6 @@ static unsigned int snd_cmipci_rate_freq(unsigned int rate)
>  
>  #ifdef USE_VAR48KRATE
>  /*
> - * Determine PLL values for frequency setup, maybe the CMI8338 (CMI8738???)
> - * does it this way .. maybe not.  Never get any information from C-Media about
> - * that <werner@suse.de>.
> - */
> -static int snd_cmipci_pll_rmn(unsigned int rate, unsigned int adcmult, int *r, int *m, int *n)
> -{
> -	unsigned int delta, tolerance;
> -	int xm, xn, xr;
> -
> -	for (*r = 0; rate < CM_MAXIMUM_RATE/adcmult; *r += (1<<5))
> -		rate <<= 1;
> -	*n = -1;
> -	if (*r > 0xff)
> -		goto out;
> -	tolerance = rate*CM_TOLERANCE_RATE;
> -
> -	for (xn = (1+2); xn < (0x1f+2); xn++) {
> -		for (xm = (1+2); xm < (0xff+2); xm++) {
> -			xr = ((CM_REFFREQ_XIN/adcmult) * xm) / xn;
> -
> -			if (xr < rate)
> -				delta = rate - xr;
> -			else
> -				delta = xr - rate;
> -
> -			/*
> -			 * If we found one, remember this,
> -			 * and try to find a closer one
> -			 */
> -			if (delta < tolerance) {
> -				tolerance = delta;
> -				*m = xm - 2;
> -				*n = xn - 2;
> -			}
> -		}
> -	}
> -out:
> -	return (*n > -1);
> -}
> -
> -/*
>   * Program pll register bits, I assume that the 8 registers 0xf8 up to 0xff
>   * are mapped onto the 8 ADC/DAC sampling frequency which can be chosen
>   * at the register CM_REG_FUNCTRL1 (0x04).
> -- 
> 1.7.10.4
> 

Re: [PATCH] sound: pci: cmipci.c: Cleaning up function that are not used anywhere

[Rickard Strandqvist]

2014-12-03 7:55 GMT+01:00 Takashi Iwai <tiwai@suse.de>:
> At Wed,  3 Dec 2014 00:12:55 +0100,
> Rickard Strandqvist wrote:
>>
>> Remove function snd_cmipci_pll_rmn() that is not used anywhere.
>
> It's an experimental code that is currently disabled.  Keeping such a
> code would be still meaningful if anyone wants to start a similar
> experiment again.
>
>
> Takashi
>
>> This was partially found by using a static code analysis program called cppcheck.
>>
>> Signed-off-by: Rickard Strandqvist <rickard_strandqvist@spectrumdigital.se>
>> ---
>>  sound/pci/cmipci.c |   41 -----------------------------------------
>>  1 file changed, 41 deletions(-)
>>
>> diff --git a/sound/pci/cmipci.c b/sound/pci/cmipci.c
>> index 85ed403..e33edae 100644
>> --- a/sound/pci/cmipci.c
>> +++ b/sound/pci/cmipci.c
>> @@ -615,47 +615,6 @@ static unsigned int snd_cmipci_rate_freq(unsigned int rate)
>>
>>  #ifdef USE_VAR48KRATE
>>  /*
>> - * Determine PLL values for frequency setup, maybe the CMI8338 (CMI8738???)
>> - * does it this way .. maybe not.  Never get any information from C-Media about
>> - * that <werner@suse.de>.
>> - */
>> -static int snd_cmipci_pll_rmn(unsigned int rate, unsigned int adcmult, int *r, int *m, int *n)
>> -{
>> -     unsigned int delta, tolerance;
>> -     int xm, xn, xr;
>> -
>> -     for (*r = 0; rate < CM_MAXIMUM_RATE/adcmult; *r += (1<<5))
>> -             rate <<= 1;
>> -     *n = -1;
>> -     if (*r > 0xff)
>> -             goto out;
>> -     tolerance = rate*CM_TOLERANCE_RATE;
>> -
>> -     for (xn = (1+2); xn < (0x1f+2); xn++) {
>> -             for (xm = (1+2); xm < (0xff+2); xm++) {
>> -                     xr = ((CM_REFFREQ_XIN/adcmult) * xm) / xn;
>> -
>> -                     if (xr < rate)
>> -                             delta = rate - xr;
>> -                     else
>> -                             delta = xr - rate;
>> -
>> -                     /*
>> -                      * If we found one, remember this,
>> -                      * and try to find a closer one
>> -                      */
>> -                     if (delta < tolerance) {
>> -                             tolerance = delta;
>> -                             *m = xm - 2;
>> -                             *n = xn - 2;
>> -                     }
>> -             }
>> -     }
>> -out:
>> -     return (*n > -1);
>> -}
>> -
>> -/*
>>   * Program pll register bits, I assume that the 8 registers 0xf8 up to 0xff
>>   * are mapped onto the 8 ADC/DAC sampling frequency which can be chosen
>>   * at the register CM_REG_FUNCTRL1 (0x04).
>> --
>> 1.7.10.4


Hi

Maybe it had been appropriate to add some comment or something then....?

And thanks Bjorn, was a better idea to write like in the patch text.

Kind regards
Rickard Strandqvist

Re: [PATCH] sound: pci: cmipci.c: Cleaning up function that are not used anywhere

[Takashi Iwai]

At Wed, 3 Dec 2014 18:17:39 +0100,
Rickard Strandqvist wrote:
> 
> 2014-12-03 7:55 GMT+01:00 Takashi Iwai <tiwai@suse.de>:
> > At Wed,  3 Dec 2014 00:12:55 +0100,
> > Rickard Strandqvist wrote:
> >>
> >> Remove function snd_cmipci_pll_rmn() that is not used anywhere.
> >
> > It's an experimental code that is currently disabled.  Keeping such a
> > code would be still meaningful if anyone wants to start a similar
> > experiment again.
> >
> >
> > Takashi
> >
> >> This was partially found by using a static code analysis program called cppcheck.
> >>
> >> Signed-off-by: Rickard Strandqvist <rickard_strandqvist@spectrumdigital.se>
> >> ---
> >>  sound/pci/cmipci.c |   41 -----------------------------------------
> >>  1 file changed, 41 deletions(-)
> >>
> >> diff --git a/sound/pci/cmipci.c b/sound/pci/cmipci.c
> >> index 85ed403..e33edae 100644
> >> --- a/sound/pci/cmipci.c
> >> +++ b/sound/pci/cmipci.c
> >> @@ -615,47 +615,6 @@ static unsigned int snd_cmipci_rate_freq(unsigned int rate)
> >>
> >>  #ifdef USE_VAR48KRATE
> >>  /*
> >> - * Determine PLL values for frequency setup, maybe the CMI8338 (CMI8738???)
> >> - * does it this way .. maybe not.  Never get any information from C-Media about
> >> - * that <werner@suse.de>.
> >> - */
> >> -static int snd_cmipci_pll_rmn(unsigned int rate, unsigned int adcmult, int *r, int *m, int *n)
> >> -{
> >> -     unsigned int delta, tolerance;
> >> -     int xm, xn, xr;
> >> -
> >> -     for (*r = 0; rate < CM_MAXIMUM_RATE/adcmult; *r += (1<<5))
> >> -             rate <<= 1;
> >> -     *n = -1;
> >> -     if (*r > 0xff)
> >> -             goto out;
> >> -     tolerance = rate*CM_TOLERANCE_RATE;
> >> -
> >> -     for (xn = (1+2); xn < (0x1f+2); xn++) {
> >> -             for (xm = (1+2); xm < (0xff+2); xm++) {
> >> -                     xr = ((CM_REFFREQ_XIN/adcmult) * xm) / xn;
> >> -
> >> -                     if (xr < rate)
> >> -                             delta = rate - xr;
> >> -                     else
> >> -                             delta = xr - rate;
> >> -
> >> -                     /*
> >> -                      * If we found one, remember this,
> >> -                      * and try to find a closer one
> >> -                      */
> >> -                     if (delta < tolerance) {
> >> -                             tolerance = delta;
> >> -                             *m = xm - 2;
> >> -                             *n = xn - 2;
> >> -                     }
> >> -             }
> >> -     }
> >> -out:
> >> -     return (*n > -1);
> >> -}
> >> -
> >> -/*
> >>   * Program pll register bits, I assume that the 8 registers 0xf8 up to 0xff
> >>   * are mapped onto the 8 ADC/DAC sampling frequency which can be chosen
> >>   * at the register CM_REG_FUNCTRL1 (0x04).
> >> --
> >> 1.7.10.4
> 
> 
> Hi
> 
> Maybe it had been appropriate to add some comment or something then....?

It's inside the ifdef block that is already disabled...  Obviously for
experiments.


Takashi