On 30/07/2010, Takashi Iwai tiwai@suse.de wrote:
At Tue, 20 Jul 2010 19:44:29 -0300, Adrian Pardini wrote:
+static int snd_cm6206_headphone_source_get(struct snd_kcontrol *kcontrol, struct snd_ctl_elem_value *ucontrol)
Try to avoid too long lines. Once when you feed your patch to scripts/checkpatch.pl, you'll see it :)
Ah, I forgot that step. Yes, it promptly complained.
+static int snd_cm6206_headphone_source_put(struct snd_kcontrol *kcontrol, struct snd_ctl_elem_value *ucontrol) +{
- struct usb_mixer_interface *mixer = snd_kcontrol_chip(kcontrol);
- struct usb_device *dev = mixer->chip->dev;
Remove a blank line here.
done.
- u16 reg2 = mixer->cm6206reg2;
I think the corresponding change in mixer.h is missing?
Yes, it's missing. Is ok to cache the register value there? I'm doing it that way because it doesn't interfere with other functions of the board and also haven't managed yet to read it with an usb transfer.
- u16 idx = kcontrol->private_value = ucontrol->value.integer.value[0];
- reg2 = (u16)((((1<<7) | (idx<<5))<<8) | (reg2 & 0x1800));
We are no lispers. Please reduce parentheses. Also avoid unnecessary cast.
ok, is something like this more acceptable?
reg2 = ((1<<7 | idx<<5)<<8) | (reg2 & 0x1800);
Thanks a lot for your time.