| Submitter | Peter Ujfalusi |
|---|---|
| Date | 2009-11-02 12:34:55 |
| Message ID | <1257165295-9352-6-git-send-email-peter.ujfalusi@nokia.com> |
| Download | mbox | patch |
| Permalink | /patch/57006/ |
| State | Awaiting Upstream, archived |
| Headers | show |
Comments
On Mon, Nov 02, 2009 at 02:34:55PM +0200, Peter Ujfalusi wrote: > APLL_CTL register is configured by the twl4030-codec MFD > driver. > Remove code, which makes changes in the APLL_CTL register, > and replace those with checks against the configured > audio_mclk configuration done in the MFD driver. This all looks mostly OK. It might be nice to combine this patch with the change to the MFD driver, simply because when the previous patch said "move" but didn't remove anything from the CODEC driver it seemed a bit surprising. The MFD part of this is queued in ASoC ATM so it shouldn't cause cross tree issues. > - twl4030_write(codec, TWL4030_REG_APLL_CTL, apll_ctrl); > + if ((freq / 1000) != twl4030->sysclk) { > + dev_err(codec->dev, > + "Mismatch in APLL infrequency %u (configred: %u)\n", > + freq, twl4030->sysclk * 1000); Typos in the log message here (and similarly for the other DAI). > - if (infreq != TWL4030_APLL_INFREQ_26000KHZ) { > + if (twl4030->sysclk != 26000) { > printk(KERN_ERR "TWL4030 voice startup: " > "MCLK is not 26MHz, call set_sysclk() on init\n"); > return -EINVAL; Is that advice still appropriate if things are specified via the codec device now? > @@ -2233,6 +2215,7 @@ static int __devinit twl4030_codec_probe(struct platform_device *pdev) > twl4030_codec = codec; > > /* Set the defaults, and power up the codec */ > + twl4030->sysclk = twl4030_codec_get_mclk() / 1000; > twl4030_init_chip(codec); > codec->bias_level = SND_SOC_BIAS_OFF; > twl4030_set_bias_level(codec, SND_SOC_BIAS_STANDBY); twl4030_codec_get_mclk() feels like it ought to take a parameter saying which twl4030, though the chances of having multiple twl4030s in one system are remote so it's not a real issue. -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Monday 02 November 2009 19:27:20 ext Mark Brown wrote: > On Mon, Nov 02, 2009 at 02:34:55PM +0200, Peter Ujfalusi wrote: > > APLL_CTL register is configured by the twl4030-codec MFD > > driver. > > Remove code, which makes changes in the APLL_CTL register, > > and replace those with checks against the configured > > audio_mclk configuration done in the MFD driver. > > This all looks mostly OK. It might be nice to combine this patch with > the change to the MFD driver, simply because when the previous patch > said "move" but didn't remove anything from the CODEC driver it seemed a > bit surprising. The MFD part of this is queued in ASoC ATM so it > shouldn't cause cross tree issues. I just wanted to keep patches for different subsystems separate, but I can either merge those two, or change the commit message in patch 3, that it will be not misleading. > > > - twl4030_write(codec, TWL4030_REG_APLL_CTL, apll_ctrl); > > + if ((freq / 1000) != twl4030->sysclk) { > > + dev_err(codec->dev, > > + "Mismatch in APLL infrequency %u (configred: %u)\n", > > + freq, twl4030->sysclk * 1000); > > Typos in the log message here (and similarly for the other DAI). Will fix those. > > > - if (infreq != TWL4030_APLL_INFREQ_26000KHZ) { > > + if (twl4030->sysclk != 26000) { > > printk(KERN_ERR "TWL4030 voice startup: " > > "MCLK is not 26MHz, call set_sysclk() on init\n"); > > return -EINVAL; > > Is that advice still appropriate if things are specified via the > codec device now? Correct, it is not correct. > > > @@ -2233,6 +2215,7 @@ static int __devinit twl4030_codec_probe(struct > > platform_device *pdev) twl4030_codec = codec; > > > > /* Set the defaults, and power up the codec */ > > + twl4030->sysclk = twl4030_codec_get_mclk() / 1000; > > twl4030_init_chip(codec); > > codec->bias_level = SND_SOC_BIAS_OFF; > > twl4030_set_bias_level(codec, SND_SOC_BIAS_STANDBY); > > twl4030_codec_get_mclk() feels like it ought to take a parameter saying > which twl4030, though the chances of having multiple twl4030s in one > system are remote so it's not a real issue. We have discussed this, but, yes the chances of having more than one twl in a system is unlikely.
Patch
diff --git a/sound/soc/codecs/twl4030.c b/sound/soc/codecs/twl4030.c index c0b47df..67cde72 100644 --- a/sound/soc/codecs/twl4030.c +++ b/sound/soc/codecs/twl4030.c @@ -214,7 +214,8 @@ static void twl4030_init_chip(struct snd_soc_codec *codec) /* set all audio section registers to reasonable defaults */ for (i = TWL4030_REG_OPTION; i <= TWL4030_REG_MISC_SET_2; i++) - twl4030_write(codec, i, cache[i]); + if (i != TWL4030_REG_APLL_CTL) + twl4030_write(codec, i, cache[i]); } @@ -1753,30 +1754,23 @@ static int twl4030_set_dai_sysclk(struct snd_soc_dai *codec_dai, { struct snd_soc_codec *codec = codec_dai->codec; struct twl4030_priv *twl4030 = codec->private_data; - u8 apll_ctrl; - apll_ctrl = twl4030_read_reg_cache(codec, TWL4030_REG_APLL_CTL); - apll_ctrl &= ~TWL4030_APLL_INFREQ; switch (freq) { case 19200000: - apll_ctrl |= TWL4030_APLL_INFREQ_19200KHZ; - twl4030->sysclk = 19200; - break; case 26000000: - apll_ctrl |= TWL4030_APLL_INFREQ_26000KHZ; - twl4030->sysclk = 26000; - break; case 38400000: - apll_ctrl |= TWL4030_APLL_INFREQ_38400KHZ; - twl4030->sysclk = 38400; break; default: - printk(KERN_ERR "TWL4030 set sysclk: unknown rate %d\n", - freq); + dev_err(codec->dev, "Unsupported frequency %u\n", freq); return -EINVAL; } - twl4030_write(codec, TWL4030_REG_APLL_CTL, apll_ctrl); + if ((freq / 1000) != twl4030->sysclk) { + dev_err(codec->dev, + "Mismatch in APLL infrequency %u (configred: %u)\n", + freq, twl4030->sysclk * 1000); + return -EINVAL; + } return 0; } @@ -1874,16 +1868,13 @@ static int twl4030_voice_startup(struct snd_pcm_substream *substream, struct snd_soc_pcm_runtime *rtd = substream->private_data; struct snd_soc_device *socdev = rtd->socdev; struct snd_soc_codec *codec = socdev->card->codec; - u8 infreq; + struct twl4030_priv *twl4030 = codec->private_data; u8 mode; /* If the system master clock is not 26MHz, the voice PCM interface is * not avilable. */ - infreq = twl4030_read_reg_cache(codec, TWL4030_REG_APLL_CTL) - & TWL4030_APLL_INFREQ; - - if (infreq != TWL4030_APLL_INFREQ_26000KHZ) { + if (twl4030->sysclk != 26000) { printk(KERN_ERR "TWL4030 voice startup: " "MCLK is not 26MHz, call set_sysclk() on init\n"); return -EINVAL; @@ -1958,22 +1949,18 @@ static int twl4030_voice_set_dai_sysclk(struct snd_soc_dai *codec_dai, int clk_id, unsigned int freq, int dir) { struct snd_soc_codec *codec = codec_dai->codec; - u8 apll_ctrl; + struct twl4030_priv *twl4030 = codec->private_data; - apll_ctrl = twl4030_read_reg_cache(codec, TWL4030_REG_APLL_CTL); - apll_ctrl &= ~TWL4030_APLL_INFREQ; - switch (freq) { - case 26000000: - apll_ctrl |= TWL4030_APLL_INFREQ_26000KHZ; - break; - default: - printk(KERN_ERR "TWL4030 voice set sysclk: unknown rate %d\n", - freq); + if (freq != 26000000) { + dev_err(codec->dev, "Unsupported frequency %u\n", freq); + return -EINVAL; + } + if ((freq / 1000) != twl4030->sysclk) { + dev_err(codec->dev, + "Mismatch in APLL infrequency %u (configred: %u)\n", + freq, twl4030->sysclk * 1000); return -EINVAL; } - - twl4030_write(codec, TWL4030_REG_APLL_CTL, apll_ctrl); - return 0; } @@ -2131,17 +2118,14 @@ static int twl4030_soc_probe(struct platform_device *pdev) if (setup) { unsigned char hs_pop; - if (setup->sysclk) - twl4030->sysclk = setup->sysclk; - else - twl4030->sysclk = 26000; + if (setup->sysclk != twl4030->sysclk) + dev_warn(&pdev->dev, "sysclk mismatch (%u vs %u)\n", + twl4030->sysclk, setup->sysclk); hs_pop = twl4030_read_reg_cache(codec, TWL4030_REG_HS_POPN_SET); hs_pop &= ~TWL4030_RAMP_DELAY; hs_pop |= (setup->ramp_delay_value << 2); twl4030_write_reg_cache(codec, TWL4030_REG_HS_POPN_SET, hs_pop); - } else { - twl4030->sysclk = 26000; } /* register pcms */ @@ -2191,10 +2175,8 @@ static int __devinit twl4030_codec_probe(struct platform_device *pdev) struct twl4030_priv *twl4030; int ret; - if (!pdata || !(pdata->audio_mclk == 19200000 || - pdata->audio_mclk == 26000000 || - pdata->audio_mclk == 38400000)) { - dev_err(&pdev->dev, "Invalid platform_data\n"); + if (!pdata) { + dev_err(&pdev->dev, "platform_data is missing\n"); return -EINVAL; } @@ -2233,6 +2215,7 @@ static int __devinit twl4030_codec_probe(struct platform_device *pdev) twl4030_codec = codec; /* Set the defaults, and power up the codec */ + twl4030->sysclk = twl4030_codec_get_mclk() / 1000; twl4030_init_chip(codec); codec->bias_level = SND_SOC_BIAS_OFF; twl4030_set_bias_level(codec, SND_SOC_BIAS_STANDBY);
APLL_CTL register is configured by the twl4030-codec MFD driver. Remove code, which makes changes in the APLL_CTL register, and replace those with checks against the configured audio_mclk configuration done in the MFD driver. Signed-off-by: Peter Ujfalusi <peter.ujfalusi@nokia.com> --- sound/soc/codecs/twl4030.c | 69 ++++++++++++++++--------------------------- 1 files changed, 26 insertions(+), 43 deletions(-)