Mark Brown wrote:
On Mon, Sep 14, 2009 at 12:00:25PM -0500, Lopez Cruz, Misael wrote:
+/* propietary formats */ +#define SND_SOC_DAIFMT_MCPDM 0x10 /* Texas Instruments McPDM */
This should really be split out into a separate patch. Are you absolutely positive that this is a proprietary interface that won't interoperate with standard PDM?
I think channel slot definition won't make it able to interoperate with other PDM interfaces. But I may be wrong.
+static void twl6030_power_up(struct snd_soc_codec *codec) +{
struct snd_soc_device *socdev = codec->socdev;struct twl6030_setup_data *setup = socdev->codec_data; +setup->codec_enable(1);That's interesting...?
The codec is turned on/off through an external line (i.e. with a gpio). Then, codec enable is board-dependent.
+static int twl6030_set_bias_level(struct snd_soc_codec *codec,
enum snd_soc_bias_level level) +{switch (level) {case SND_SOC_BIAS_ON:twl6030_power_up(codec);break;case SND_SOC_BIAS_PREPARE:twl6030_power_up(codec);break;case SND_SOC_BIAS_STANDBY:twl6030_power_up(codec);break;case SND_SOC_BIAS_OFF:twl6030_power_down(codec);break;Is there any reason not to just fold these functions into the bias management? It looks like the only caller and it'd save jumping around the file to find stuff.
For the moment, there is no reason. I thought it was more clear to have separate power_up/power_down functions, but I can merge them in bias management function.
/* power on device */twl6030_set_bias_level(codec, SND_SOC_BIAS_STANDBY); +twl6030_init_chip(codec);Is the the right ordering? I'd have expected to see the one time init stuff done prior to bringing up the power for the first time.
Yes, it's the right order. codec chip cannot be initialized if the codec is not already power-up, registers are not accesible before that.