SoC codec read callback definition

SoC codec read callback definition

[Lopez Cruz, Misael]

read callback (of snd_soc_codec struct) is defined as a function
that returns unsigned int:

struct snd_soc_codec {
	...
	unsigned int (*read)(struct snd_soc_codec *, unsigned int);
	int (*write)(struct snd_soc_codec *, unsigned int, unsigned int);
      ...
}  

However, actual implementation of that callback in many CODEC drivers
may return a negative error value (i.e. when a register is out of cache
reg size). Should the callback definition be changed to return _int_
instead of _unsigned int_? Some drivers (like wm8900, wm9081) handle
invalid registers with BUG_ON, in that case it's not necessary to
return the negative error code.

What should be the best way to handle this? Or is it ok as currently
implemented? Well, it was just a thought.

-Misa

Re: SoC codec read callback definition

[Mark Brown]

On Thu, Jul 23, 2009 at 02:50:53AM -0500, Lopez Cruz, Misael wrote:

> However, actual implementation of that callback in many CODEC drivers
> may return a negative error value (i.e. when a register is out of cache
> reg size). Should the callback definition be changed to return _int_
> instead of _unsigned int_? Some drivers (like wm8900, wm9081) handle
> invalid registers with BUG_ON, in that case it's not necessary to
> return the negative error code.

There's two classes of error.  Reads from the hardware might possibly
fail at runtime, while reads from cache should never fail - an attempt
to read from an uncached register is a clear bug in the driver.  When
the code was originally written there were essentially no devices with
readback so the cache was the only option.

> What should be the best way to handle this? Or is it ok as currently
> implemented? Well, it was just a thought.

Ideally we'd have error codes but that would loose us a bit of data and
IIRC there's at least one codec in there with 32 bit registers which
would need to be handled.