Re: [Pull request] Support for wm9705 codec and two machines that use it.

[Ian Molton <ian@mnementh.co.uk>]

Mark Brown wrote:
> On Fri, Jan 16, 2009 at 03:46:19PM +0000, Ian Molton wrote:
> 
>> Mark, the changes requested are implemented below. There are two
>> patchsets, one for wm9705 and one for the other codec issues I found.
> 
> This looks good, thanks - I've applied it.  A couple of the changes
> appear to be pure formatting changes:

Thanks,

Comments below on your comments:

>> --- a/sound/soc/codecs/ad1980.c
>> +++ b/sound/soc/codecs/ad1980.c
>> @@ -109,7 +109,7 @@ static unsigned int ac97_read(struct snd_soc_codec  
>> *codec,
>>  	default:
>>  		reg = reg >> 1;
>>
>> -		if (reg >= (ARRAY_SIZE(ad1980_reg)))
>> +		if (reg >= ARRAY_SIZE(ad1980_reg))
>>  			return -EINVAL;
>>
>>  		return cache[reg];
> 
> Formatting changes only in this driver?  Not sure I really like the
> extra brackets.

Exactly - nor did I, so  Iremoved them :-)

>> diff --git a/sound/soc/codecs/wm8990.c b/sound/soc/codecs/wm8990.c
>> index 6b27786..f93c095 100644
>> --- a/sound/soc/codecs/wm8990.c
>> +++ b/sound/soc/codecs/wm8990.c
>> @@ -116,7 +116,7 @@ static inline unsigned int  
>> wm8990_read_reg_cache(struct snd_soc_codec *codec,
>>  	unsigned int reg)
>>  {
>>  	u16 *cache = codec->reg_cache;
>> -	BUG_ON(reg > (ARRAY_SIZE(wm8990_reg)) - 1);
>> +	BUG_ON(reg >= ARRAY_SIZE(wm8990_reg));
>>  	return cache[reg];
>>  }
>>
> 
> These two bits of code are equivalent...

Yes but one is clearer and more commonly used. Testing for 'more than 
one less than the array size' is just a bit... meh :-)

(plus its inconsistent with all the other drivers as it was)

-Ian