[PATCH 1/3] ASoC: add 88pm860x codec driver

On Tue, Aug 17, 2010 at 7:02 PM, Mark Brown
[off-list ref] wrote:

On Tue, Aug 17, 2010 at 06:44:44PM +0800, Haojian Zhuang wrote:

quoted

On Tue, Aug 17, 2010 at 5:58 PM, Mark Brown

quoted

quoted

quoted

+ ? ? switch (event) {
+ ? ? case SND_SOC_DAPM_PRE_PMU:
+ ? ? ? ? ? ? snd_soc_update_bits(codec, PM860X_ADC_EN_1, en1, en1);
+ ? ? ? ? ? ? snd_soc_update_bits(codec, PM860X_ADC_EN_2, en2, en2);

quoted

quoted

I still don't follow why you need a custom event for this.

quoted

Enabling both bit 0 of ADC_EN_1 and bit 5 of ADC_EN_2 can enable left
ADC. Enabling both bit 1 of ADC_EN_1 and bit 4 of ADC_EN_2 can enable
right ADC. I can't find any DAPM API can handle this. So I implement
the custom event.

As I said previously I would expect you to be using a DAPM supply widget
for this.

Supply widget should be always on while DAPM is working. How about
only enable left ADC?

quoted

quoted

quoted

+ ? ? /* set master/slave audio interface */
+ ? ? switch (fmt & SND_SOC_DAIFMT_MASTER_MASK) {
+ ? ? case SND_SOC_DAIFMT_CBM_CFM:
+ ? ? case SND_SOC_DAIFMT_CBM_CFS:
+ ? ? ? ? ? ? if (pm860x->dir == PM860X_CLK_DIR_OUT)
+ ? ? ? ? ? ? ? ? ? ? *inf |= PCM_INF2_MASTER;
+ ? ? ? ? ? ? else
+ ? ? ? ? ? ? ? ? ? ? return -EINVAL;
+ ? ? ? ? ? ? break;

quoted

quoted

You're setting the same register configuration for two different DAI
clock master configurations here. ?Presumably one of the settings is
inaccurate?

quoted

No, they're different registers. But offsets are same. So I just return pointer.

I can't associate your comment there with the code at all. ?The code
does nothing different for the two case statements and there's no other
code I can see.

You can find pm860x_pcm_set_dai_fmt() and pm860x_i2s_set_dai_fmt().
Both functions called set_dai_fmt(). In set_dai_fmt(), all bit
operations are based on variable, not registers. Register operation is
implemented in pcm_set_dai_fmt() and i2s_set_dai_fmt().

quoted

quoted

quoted

+ ? ? if (shrt & (SHORT_LO1 | SHORT_LO2))
+ ? ? ? ? ? ? report |= PM860X_SHORT_LINEOUT;
+ ? ? if (shrt & (SHORT_HS1 | SHORT_HS2))
+ ? ? ? ? ? ? report |= PM860X_SHORT_HEADSET;
+ ? ? dev_dbg(pm860x->codec->dev, "report:0x%x\n", report);
+ ? ? return IRQ_HANDLED;

quoted

quoted

It would seem better to just remove the interrupt handling support
entirely if you're not going to implement jack detection. ?Right now all
the curernt code will do is waste power by enabling the feature but
ignoring the result.

quoted

I need a document on illustrating jack on alsa. Could you share one?

There's a number of in tree examples - seach for snd_soc_jack.

OK. How to test it? Still no document?