[alsa-devel] [PATCH] usb-audio: Add mixer control for Digidesign Mbox 1 clock source
This patch provides a mixer control for selecting the clock source of the Digidesign Mbox 1 to either internal clock or S/PDIF external. Trial and error and bus snooping were the only way to get this information, but it works on the hardware.
Hi Damien,
On Oct 31 2014 10:57, Damien Zammit wrote:
This patch provides a mixer control for selecting the clock source of the Digidesign Mbox 1 to either internal clock or S/PDIF external. Trial and error and bus snooping were the only way to get this information, but it works on the hardware.
This patch includes lines over 80 characters. Furthermore, these lines include inappropriate white-space for indentation. How about indenting with tab only and add more line-breaks between each function parameter?
$ ./scripts/checkpatch.pl /tmp/0001-mbox1-spdif.patch WARNING: line over 80 characters #134: FILE: sound/usb/mixer_quirks.c:641: + usb_rcvctrlpipe(mixer->chip->dev, 0), 0x81,
WARNING: line over 80 characters #135: FILE: sound/usb/mixer_quirks.c:642: + USB_DIR_IN | USB_TYPE_CLASS | USB_RECIP_INTERFACE,
WARNING: line over 80 characters #140: FILE: sound/usb/mixer_quirks.c:647: + usb_rcvctrlpipe(mixer->chip->dev, 0), 0x81,
WARNING: line over 80 characters #141: FILE: sound/usb/mixer_quirks.c:648: + USB_DIR_IN | USB_TYPE_CLASS | USB_RECIP_ENDPOINT,
WARNING: line over 80 characters #159: FILE: sound/usb/mixer_quirks.c:666: + usb_rcvctrlpipe(mixer->chip->dev, 0), 0x81,
WARNING: line over 80 characters #160: FILE: sound/usb/mixer_quirks.c:667: + USB_DIR_IN | USB_TYPE_CLASS | USB_RECIP_ENDPOINT,
WARNING: line over 80 characters #165: FILE: sound/usb/mixer_quirks.c:672: + usb_rcvctrlpipe(mixer->chip->dev, 0), 0x81,
WARNING: line over 80 characters #166: FILE: sound/usb/mixer_quirks.c:673: + USB_DIR_IN | USB_TYPE_CLASS | USB_RECIP_ENDPOINT,
total: 0 errors, 8 warnings, 226 lines checked
/tmp/0001-mbox1-spdif.patch has style problems, please review.
If any of these errors are false positives, please report them to the maintainer, see CHECKPATCH in MAINTAINERS.
Regards
Takashi Sakamoto o-takashi@sakamocchi.jp
Hi, sorry for the style problems. See attached for better version.
Damien
On 31/10/14 14:29, Takashi Sakamoto wrote:
Hi Damien,
On Oct 31 2014 10:57, Damien Zammit wrote:
This patch provides a mixer control for selecting the clock source of the Digidesign Mbox 1 to either internal clock or S/PDIF external. Trial and error and bus snooping were the only way to get this information, but it works on the hardware.
This patch includes lines over 80 characters. Furthermore, these lines include inappropriate white-space for indentation. How about indenting with tab only and add more line-breaks between each function parameter?
$ ./scripts/checkpatch.pl /tmp/0001-mbox1-spdif.patch WARNING: line over 80 characters #134: FILE: sound/usb/mixer_quirks.c:641:
usb_rcvctrlpipe(mixer->chip->dev,0), 0x81,
WARNING: line over 80 characters #135: FILE: sound/usb/mixer_quirks.c:642:
USB_DIR_IN | USB_TYPE_CLASS |USB_RECIP_INTERFACE,
WARNING: line over 80 characters #140: FILE: sound/usb/mixer_quirks.c:647:
usb_rcvctrlpipe(mixer->chip->dev,0), 0x81,
WARNING: line over 80 characters #141: FILE: sound/usb/mixer_quirks.c:648:
USB_DIR_IN | USB_TYPE_CLASS |USB_RECIP_ENDPOINT,
WARNING: line over 80 characters #159: FILE: sound/usb/mixer_quirks.c:666:
usb_rcvctrlpipe(mixer->chip->dev,0), 0x81,
WARNING: line over 80 characters #160: FILE: sound/usb/mixer_quirks.c:667:
USB_DIR_IN | USB_TYPE_CLASS |USB_RECIP_ENDPOINT,
WARNING: line over 80 characters #165: FILE: sound/usb/mixer_quirks.c:672:
usb_rcvctrlpipe(mixer->chip->dev,0), 0x81,
WARNING: line over 80 characters #166: FILE: sound/usb/mixer_quirks.c:673:
USB_DIR_IN | USB_TYPE_CLASS |USB_RECIP_ENDPOINT,
total: 0 errors, 8 warnings, 226 lines checked
/tmp/0001-mbox1-spdif.patch has style problems, please review.
If any of these errors are false positives, please report them to the maintainer, see CHECKPATCH in MAINTAINERS.
Regards
Takashi Sakamoto o-takashi@sakamocchi.jp
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.htm...
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@sakamocchi.jp
Hi Takashi,
On 31/10/14 15:28, Takashi Sakamoto wrote:
See attached patch.
I have applied your changes, patch 3 attached.
I am also working on another patch that will provide duplex support with a locked frequency at 48kHz. I already have it working on the hardware but there is some code refactoring required before I can submit it again for review.
Damien
On Oct 31 2014 13:28, Takashi Sakamoto wrote:
2.please use 'const char *const' for immutable arrays for immutable strings:
static const char *texts[2] = {"Internal","S/PDIF"};
Oops. I mistook to use immutable as 'may not be changed'. It should be 'mutable'...
Regards
Takashi Sakamoto o-takashi@sakamocchi.jp
participants (2)
-
Damien Zammit -
Takashi Sakamoto