Re: ML403 / ALSA driver for AC97 Controller

["Joachim Förster" <mls.JOFT@gmx.de>]

Hi Grant,

On Sat, 2007-07-21 at 14:17 -0600, Grant Likely wrote:
> 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

About a week ago I finished work on the last four issues and in last
days I created several patches against different kernel trees. ATM I
have three patches, against:

tag v2.6.22 (Linus)
branch master (Linus)
branch virtex-dev (your v2.6.22 based branch)

I had to make different patches, because one version didn't apply
cleanly to the other branches, due to differences in a Kconfig file e.g.
and above all in the virtex_devices.c file.

Now, my question is: Which one should I post to the mailing list? (after
testing these patches - I haven't got a chance yet to test them on real
hardware - compilation is ok).

One more thing: I made two parts, one patch adds the driver and the
other one makes the registration with the platform bus. Is this ok? (I
saw this scheme in your virtex-dev branch.)

> 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.

Meanwhile, I had a _very_ short look into ASoC ... and I don't really
know ... My driver already uses the AC97 Layer of ALSA, so in some way
the codec is already separated from the controller. Xilinx' AC97
Controller Reference does have some very bad impact on the codec which
forced me to implement codec register shadowing ... hmmmm, I have to
look at it (ASoC) again - as soon as there is more (free)time ...

 Joachim