[alsa-devel] New alsa lowlevel driver for the SE-200PCI and SE-90PCI
sh_okada at d4.dion.ne.jp
sh_okada at d4.dion.ne.jp
Fri Oct 12 17:05:43 CEST 2007
>Well, the current code still doesn't follow properly.
>Below are some nitpickings.
Thank you for a detailed check.
A new one put my HP
http://www.d4.dion.ne.jp/~sh_okada/misc/alsa-SE200PCI.patch-3.tgz
>You don't need braces around a single-line if, for example,
>
>> if (ice->eeprom.subvendor == VT1724_SUBDEVICE_SE90PCI) {
>> /* nothing to do */
>> }
>> else if (ice->eeprom.subvendor == VT1724_SUBDEVICE_SE200PCI) {
>> se200pci_add_controls(ice);
>> }
>
>should be
>
>> if (ice->eeprom.subvendor == VT1724_SUBDEVICE_SE90PCI)
>> /* nothing to do */
>> else if (ice->eeprom.subvendor == VT1724_SUBDEVICE_SE200PCI)
>> se200pci_add_controls(ice);
This becomes a compile error,
Because when omitting it
if (A) else if (B) func();
It needs ';' or '{}'
like
if (A) ; else if (B) func();
or
if (A) {} else if (B) func();
Therefore, "/* nothing to do */" cannot be written by this style.
When forcibly writing It becomes tricky.
if (A)
/* nothing to do */;
else if (B)
func();
or
if (A) {
/* nothing to do */
} else if (B)
func();
So I wrote as
/* nothing to do for VT1724_SUBDEVICE_SE90PCI */
if (ice->eeprom.subvendor == VT1724_SUBDEVICE_SE200PCI)
err = se200pci_add_controls(ice);
>> 2 The name of the control
>
>Now looks better. One remaining thing is you're using an enum control
>for "ACG Capture Switch". If it's a switch, you should use a boolean
>type.
AGC has some modes, but now coded one mode.
It makes it to such a style, Because it is likely to be going
to increase it in the future.
now OFF, ON(type=1)
future OFF, ON(type=1), ON(type=2),...
Thanks
More information about the Alsa-devel
mailing list