[alsa-devel] [PATCH] usb-audio: Add mixer control for Digidesign Mbox 1 clock source

[Takashi Sakamoto]

Hi Damien,

On Oct 31 2014 13:02, Damien Zammit wrote:
> Hi, sorry for the style problems.  See attached for better version.

4 points.

1. please use white-spaces or tabs for indentation to align parameters 
to function-start blacket:

 > +static int snd_mbox1_switch_put(struct snd_kcontrol *kctl,
 > +                                       struct snd_ctl_elem_value 
*ucontrol)

 > +static int snd_mbox1_switch_info(struct snd_kcontrol *kcontrol,
 > +                                       struct snd_ctl_elem_info *uinfo)

 > +static int snd_mbox1_switch_put(struct snd_kcontrol *kctl,
 > +                                       struct snd_ctl_elem_value 
*ucontrol)

2.please use 'const char *const' for immutable arrays for immutable strings:
 > +       static const char *texts[2] = {"Internal",
 > +                                      "S/PDIF"
 > +       };
 > +

3. I think in your case, snd_ctl_enum_info() is available in struct 
snd_kcontrol_new.info() callback. Please read this thread:
[alsa-devel] [PATCH 00/43] Spread usage of snd_ctl_elem_info()
http://mailman.alsa-project.org/pipermail/alsa-devel/2014-October/082573.html

4. I think 'err' local variable in snd_mbox1_create_sync_switch() can be 
removed because it's assign and evaluated at once:

 > +       err = snd_ctl_add(mixer->chip->card, kctl);
 > +       if (err < 0)
 > +               return err;
 > +
 > +       return 0;

See attached patch.


Regards

Takashi Sakamoto
o-takashi at sakamocchi.jp
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Comments-for-mbox1-spdif-2.patch.patch
Type: text/x-patch
Size: 2570 bytes
Desc: not available
URL: <http://mailman.alsa-project.org/pipermail/alsa-devel/attachments/20141031/b86a9902/attachment.bin>