Re: ML403 / ALSA driver for AC97 Controller

["Grant Likely" <grant.likely@secretlab.ca>]

On 7/21/07, Joachim F=F6rster <mls.JOFT@gmx.de> 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.
grant.likely@secretlab.ca
(403) 399-0195