Re: [PATCH 1/4] ASoC: TLV320AIC23B Support more sample rates

[Takashi Iwai <tiwai@suse.de>]

At Thu, 6 Nov 2008 10:49:06 +0000,
Mark Brown wrote:
> 
> On Wed, Nov 05, 2008 at 09:35:01PM +0100, Takashi Iwai wrote:
> > Mark Brown wrote:
> 
> > > +		return score;
> 
> > > +	return 0xffffffff;
> 
> > -1 would be simpler and safer (for possible missing f's).
> 
> I'm not sure that'd add to the clarity - the goal isn't to have all bits
> set, the goal is to have a much larger score than could ever otherwise
> be achieved so putting in a very large constant expresses that much more
> directly.

Also, regarding unsigned integer:

> +unsigned get_score(int adc, int adc_l, int adc_h, int need_adc,
> +		int dac, int dac_l, int dac_h, int need_dac)
> +{
> +	if ((adc >= adc_l) && (adc <= adc_h) &&
> +			(dac >= dac_l) && (dac <= dac_h)) {
> +		unsigned diff_adc = need_adc - adc;
> +		unsigned diff_dac = need_dac - dac;
> +		unsigned score;
> +		if (((int)diff_adc) < 0)
> +			diff_adc = -diff_adc;
> +		if (((int)diff_dac) < 0)
> +			diff_dac = -diff_dac;
> +		score = diff_adc + diff_dac;
> +		return score;

It's more readable when diff_adc, diff_dac are int so that you have no
ugly cast:
		int diff_adc = need_adc - adc;
		int diff_dac = need_dac - dac;
		unsigned score;
		if (diff_adc < 0)
			diff_adc = -diff_adc;
		if (diff_dac < 0)
			diff_dac = -diff_dac;
		score = diff_adc + diff_dac;
		return score;

And, even more readable with abs() (not surprisingly defined in
linux/kernel.h):
		int diff_adc = need_adc - adc;
		int diff_dac = need_dac - dac;
		return abs(diff_adc) + abs(diff_dac);

Takashi