[Alsa-devel] [RFC 4/7] snd-aoa: add codecs

[Takashi Iwai]

At Tue, 30 May 2006 14:43:16 +0200,
Johannes Berg wrote:
> 
> On Mon, 2006-05-29 at 14:08 +0200, Takashi Iwai wrote:
> > > +	if (reg != ONYX_REG_CONTROL) {
> > > +		*value = onyx->cache[reg-FIRSTREGISTER];
> > > +		return 0;
> > > +	}
> > > +	v = i2c_smbus_read_byte_data(&onyx->i2c, reg);
> > > +	if (v < 0)
> > > +		return -1;
> > > +	*value = (u8)v;
> > > +	onyx->cache[ONYX_REG_CONTROL-FIRSTREGISTER] = *value;
> > 
> > Isn't it "reg - FIRSTREGISTER" ?
> 
> Nah, look at the first line I quoted :)

Ah, OK it's anyway same...

> > > +	/* FIXME: we could be checking if anything changed */
> > > +	mutex_unlock(&onyx->mutex);
> > > +
> > > +	return 1;
> > 
> > The put callback is supposed to return 0 if the values are unchanged
> > (although most apps ignore the return value).
> 
> Does it have to? This way there's an event, but...

Yes, in principle.  As mentioned, it works even without the check,
though.

> > > +static int tas_snd_capture_source_info(struct snd_kcontrol *kcontrol,
> > > +	struct snd_ctl_elem_info *uinfo)
> > > +{
> > > +	static char* texts[] = { "Line-In", "Microphone" };
> > 
> > char *texts[]
> 
> Any particular reason?

Well, I meant the position of asterisk to follow the conventional C
coding style.  Of course it should be static.

> > > +	aoa_snd_ctl_add(snd_ctl_new1(&volume_control, tas));
> > 
> > Error checks please.
> 
> What should it do on such errors?

The driver should give up the initialization and fail to load.
Usually the error from snd_ctl_add() is critical, either no memory or
a duplicated control element is found.


Takashi