>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 _______________________________________________ Alsa-devel mailing list Alsa-devel@xxxxxxxxxxxxxxxx http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
