Re: [PATCH 4/4] ASoC: clean up cache accesser

[Dimitris Papastamos <dp@opensource.wolfsonmicro.com>]

On Mon, 2010-12-20 at 17:05 +0100, Takashi Iwai wrote:
> +static inline bool set_cache_val(void *base, unsigned int idx,
> +				unsigned int val, unsigned int word_size)
> +{
> +	if (word_size == 1) {
> +		u8 *cache = base;
> +		if (cache[idx] == val)
> +			return true;
> +		cache[idx] = val;
> +	} else {
> +		u16 *cache = base;
> +		if (cache[idx] == val)
> +			return true;
> +		cache[idx] = val;
> +	}
> +	return false;
> +}

If word_size is anything other than 1 byte, the above else will try to
handle it and assume it is 16 bits.  I'd expect for an explicit check
for word_size == 2.  A switch statement would perhaps be preferred for
legibility.  It'd perhaps be wise to simply die via BUG() or similar if
an unsupported word size was passed in.

> +static inline unsigned int get_cache_val(const void *base, unsigned int idx,
> +					 unsigned int word_size)
> +{
> +	if (word_size == 1) {
> +		const u8 *cache = base;
> +		return cache[idx];
> +	} else {
> +		const u16 *cache = base;
> +		return cache[idx];
> +	}
> +}

Same here.

The rest looks good.

Thanks,
Dimitrios