[alsa-devel] SoC codec read callback definition
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
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.
participants (2)
-
Lopez Cruz, Misael -
Mark Brown