[alsa-devel] [PATCH V1 06/13] Allow a custom ASOC fabric driver with soc-of-simple

[Mark Brown]

On Wed, May 13, 2009 at 09:59:12PM -0400, Jon Smirl wrote:

> Allow a custom ASOC fabric driver with soc-of-simple. Register multiple DAIs. If no custom fabric driver, supply a default one so that ASOC binding will complete.

Hrm.  I'm not sure the way you're providing a default behaviour here is
going to work (see comments below).  Would it not be better to have
something in the OF tree that specifies non-default behaviour?
Obviously that opens the whole can of worms with OpenFirmware not being
able to cope with machine drivers well, though that's probably going to
have to be dealt with at some point.

Also, note that it's spelt 'ASoC'.  I'm fairly sure I've mentioned this
to you before.

> +#define SOC_OF_SIMPLE_MAX_DAI 2
> +

Why not make this dynamic?  It's also surprising to see this in the
header, it should be an implementation detail.

> +int of_snd_soc_register_cpu_dai(struct device_node *node,
> +				 struct snd_soc_dai *cpu_dai, int count);
> +

ASoC is moving towards having all DAIs register equivalently.  Might be
as well to consider supporting this here too, though the core isn't
quite there yet.

> +int of_snd_soc_register_default_fabric(void);

> +#include <sound/soc-of-simple.h>
>  
>  static int __init alsa_sound_last_init(void)
>  {
>  	int idx, ok = 0;
> -	
> +
> +#if defined(CONFIG_SND_SOC_OF_SIMPLE)
> +	of_snd_soc_register_default_fabric();
> +#endif 	

This approach relies on having things built in or being able to load the
modules before the alsa_sound_last_init() is called.  That's not
something that's going to be reliably doable for all systems - netbooted
development systems are a common example here.

>  #if defined(CONFIG_SND_SOC_OF_SIMPLE)
>  	/* Tell the of_soc helper about this codec */
> -	of_snd_soc_register_codec(&aic26_soc_codec_dev, aic26, &aic26_dai,
> +	of_snd_soc_register_codec(&aic26_soc_codec_dev, aic26, &aic26_dai, 1,
>  				  spi->dev.archdata.of_node);
>  #endif

If, as previously discussed, this were merged with the core you could
pick most of this data out of the snd_soc_codec.

> +	of_soc->card.name = (char *)template_name;

This cast is suspect..

> -	if ((!of_soc->device.codec_data) || (!of_soc->platform_node))
> +	if ((!of_soc->device.codec_data) || (!of_soc->cpu_dai_node) ||
> +									!of_soc->card.platform || !of_soc->card.name)

Looks like something went very wrong with the indentation here.

> +	pr_info("registering ASoC platform driver: %s\n", platform->name);

pr_debug().