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 :)
> I'd define a constant for 128.
#define OFFSET_BECAUSE_ALSA_USERSPACE_PROGRAMS_SUCK 128
;)
Just kidding, will do.
> Fold lines to fit with 80 columns (heh, blaming other one's code is
> easy :)
Heh. I thought I'd Lindent'ed most of it.
> > + /* 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...
> > +static u8 initial_values[] = {
>
> Should have ARRAY_SIZE(register_map) since this size must be identical
> with register_map.
Good point.
> > +#define ADDCTL(n) \
> > + do { \
> > + ctl = snd_ctl_new1(&n, onyx); \
> > + if (ctl) { \
> > + ctl->id.device = \
> > + onyx->codec.soundbus_dev->pcm->device; \
> > + aoa_snd_ctl_add(ctl); \
>
> No error check?
Hmm, yeah, I guess it should have error checks. Will revisit that.
> Use strlcpy, or MAX_CODEC_NAME_LEN-1. Similar lines are found in
> tas driver too.
Right.
> Looks a bit too hacky. IMO, it's better to define a struct
Maybe I'll just get rid of it completely, it's never hooked up as far as
I know. But yeah, I guess you're right.
> > +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?
> > +/* I need help here!
>
> Use ifdef. Nested comments are bad.
I know. Temporary code, probably going away completely.
> > + aoa_snd_ctl_add(snd_ctl_new1(&volume_control, tas));
>
> Error checks please.
What should it do on such errors?
Thanks,
johannes
Attachment:
signature.asc
Description: This is a digitally signed message part
_______________________________________________ Alsa-devel mailing list Alsa-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.sourceforge.net/lists/listinfo/alsa-devel
