On 7/21/07, Joachim Förster <[EMAIL PROTECTED]> wrote: > Hi all, Thanks Joachim, this is interesting work. I'd like to see this get into mainline.
A few initial comments: 1. You should post this code as a patch against the kernel tree. Links to tarballs are not the best way to get code reviewed. Post your patches to both the ALSA and linuxppc-dev mailing lists. 2. (get this out of the way quickly) Coding standard doesn't match the kernel (indent w/ tabs, keep lines < 80 chars, etc). You should run your code through 'scripts/Lindent' in the kernel tree. That will sort out many of the whitespace issues. 3. In the same vein, c++ style comments need to be scrubbed. 4. Do not directly include xparameters.h in your driver. The driver should get it's data from the platform bus registration (of the of_device registration when we move to arch/powerpc). 5. struct 'platform_device devices[]' needs to be moved to arch/ppc/sysdev/virtex_devices.c 6. Have you looked into the new ALSA infrastructure which separates Codecs from controllers (in sound/soc)? It might be a good idea to go that route for this driver. That being said, it looks like good work. Please cc me when you send the next version. Thanks, g. --- Grant Likely, B.Sc., P.Eng. Secret Lab Technologies Ltd. [EMAIL PROTECTED] (403) 399-0195 _______________________________________________ Linuxppc-embedded mailing list [email protected] https://ozlabs.org/mailman/listinfo/linuxppc-embedded
