On 01/08/2013 03:22 PM, Lucas Stach wrote:

Am Dienstag, den 08.01.2013, 15:10 -0700 schrieb Stephen Warren:

...

On 01/04/2013 06:18 PM, Lucas Stach wrote:

...

This adds the driver for the Tegra 2x AC97 host controller.

...

...

diff --git a/sound/soc/tegra/tegra20_ac97.c b/sound/soc/tegra/tegra20_ac97.c

...

...

+static inline void tegra20_ac97_start_playback(struct tegra20_ac97 *ac97) +{

• regmap_update_bits(ac97->regmap, TEGRA20_AC97_FIFO1_SCR,

• 	   TEGRA20_AC97_FIFO_SCR_PB_QRT_MT_EN,
  

• 	   TEGRA20_AC97_FIFO_SCR_PB_QRT_MT_EN);
  

• regmap_update_bits(ac97->regmap, TEGRA20_AC97_CTRL,

• 	   TEGRA20_AC97_CTRL_PCM_DAC_EN |
  

• 	   TEGRA20_AC97_CTRL_STM_EN,
  

• 	   TEGRA20_AC97_CTRL_PCM_DAC_EN |
  

• 	   TEGRA20_AC97_CTRL_STM_EN);
  

+}

That sets both PCM_DAC_EN and STM_EN, but ...

...

+static inline void tegra20_ac97_stop_playback(struct tegra20_ac97 *ac97) +{

• regmap_update_bits(ac97->regmap, TEGRA20_AC97_FIFO1_SCR,

• 	   TEGRA20_AC97_FIFO_SCR_PB_QRT_MT_EN, 0);
  

• regmap_update_bits(ac97->regmap, TEGRA20_AC97_CTRL,

• 	   TEGRA20_AC97_CTRL_PCM_DAC_EN, 0);
  

+}

... that only clears DAC_EN. Should it clear STM_EN too?

To be honest I have no idea what STM really is, seems to be some form of packed format selection. If not set playback outputs only more or less random noise. Only PCM_EN controls FIFO and DMA operations though, so it's ok to just disable this to stop playback.

I'd be tempted to just hard-code those bits on in probe() then, but it's not a big deal, so not worth spinning the patch for that.

I guess I meant to say, Reviewed-by: Stephen Warren swarren@nvidia.com