[alsa-devel] [PATCH 4/7] Alchemy: DB1200 AC97+I2S audio support.

[Manuel Lauss]

Hi Mark!

On Mon, Jun 8, 2009 at 11:25 AM, Mark
Brown<broonie at opensource.wolfsonmicro.com> wrote:
> On Sun, Jun 07, 2009 at 08:39:01PM +0200, Manuel Lauss wrote:
>> Replaces the sample Alchemy PSC AC97 machine code with a DB1200 machine
>> driver with AC97 and I2S support.
>
> Acked-by: Mark Brown <broonie at opensource.wolfsonmicro.com>
>
> If this is going to go in during the merge window I'm happy for it to go
> in via the MIPS tree but otherwise I'd much rather it goes via ASoC in
> case API changes cause merge issues.  I don't know what you prefer,
> Ralf?

I'd rather have it all go through the MIPS tree; this is the only patch of
7 which touches files outside it, and it depends on another one to
apply cleanly.


> We could also split the MIPS and ASoC parts into separate patches if
> that helps.
>
>> +/* it sucks that the ASoC headers are not under include/ */
>> +#include "../codecs/ac97.h"
>> +#include "../codecs/wm8731.h"
>
> This is because they're internal to ASoC - having them out of include
> should set off big red warning flags for something outside ASoC is
> looking at them.  If there are things that should be referenced outside
> ASoC they should be in a separate header in include/sound like the
> WM9081 platform data.

I'd actually love to move this file to the actual board code, however due
to the way ASoC is organized this isn't at all possible.  This is one of two
things I don't like about ASoC: the machine registration is extremely awkward
compared to other platform drivers. (the other being that ASoC doesn't
support multiple machines [i.e. I could actually run both AC97 and I2S
simultaneously one of my boards, as independent sound cards])


>> +static int db1200_ac97_init(struct snd_soc_codec *codec)
>> +{
>> +     snd_soc_dapm_sync(codec);
>> +     return 0;
>> +}
>
> This could be removed but it does no harm.

I'll  drop it.


>> +/*-------------------------  COMMON PART  ---------------------------*/
>> +
>> +static struct resource psc1_res[] = {
>> +     [0] = {
>> +             .start  = CPHYSADDR(PSC1_BASE_ADDR),
>> +             .end    = CPHYSADDR(PSC1_BASE_ADDR) + 0x000fffff,
>> +             .flags  = IORESOURCE_MEM,
>
> If you conver the I2S driver to use the standard device probing model
> this could all me moved into the architecture code rather than placed in
> machine drivers.

Again, I'd love to, but can't: the AC97/I2S/DBDMA drivers extract base address
and DMA information from the platform device resource structure;  however
I can't just copy the resource info from the this db1200_sound platform_driver
to the soc_audio platform driver because the driver core complains about
resource conflicts (two platform_devices sharing the same resources).
Unless I missed a flag which needs to be passed to the resource.flags member?

Thanks!
        Manuel Lauss