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