| Message ID | 1405008673-15994-1-git-send-email-s.nawrocki@samsung.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, Jul 10, 2014 at 06:11:13PM +0200, Sylwester Nawrocki wrote: > Currently configuration of the CDCLK pad is being overwritten in > the i2s_shutdown() callback in order to gate the SoC output clock. Applied, thanks.
Hi Sylwester, On Thu, Jul 10, 2014 at 10:11 AM, Sylwester Nawrocki <s.nawrocki@samsung.com> wrote: > Currently configuration of the CDCLK pad is being overwritten in > the i2s_shutdown() callback in order to gate the SoC output clock. > However if an ASoC machine driver doesn't restore that clock > settings each time after opening the sound device this results > in the CDCLK pin being permanently configured into input mode. > I.e. the output clock will always stay disabled. > Fix that by saving the CDCLKCON bit state in i2s_shutdown() and > and restoring it in the i2s_startup() callback. I'm still having trouble in this area on ODROID. Basically, if you start pulseaudio, all audio breaks. Even after you kill pulseaudio, audio is still broken. This happens because CDCLK gets disabled by i2s.c and never enabled again. pulseaudio does: 1. i2s_startup for playback channel 2. i2s_startup for capture channel 3. i2s_shutdown for capture channel 4. i2s_shutdown for playback channel In step 3 we disable CDCLK even though playback should still be active, oops. In step 4 we do this: u32 mod = readl(i2s->addr + I2SMOD); i2s->cdclk_out = !(mod & MOD_CDCLKCON); and now cdclk_out is always going to be 0, so we'll never turn it back on again. Regardless of what we do now, I think there is a bug in i2s_shutdown in that it should not do any real shutdown stuff if the other stream (playback/capture) is open. However I'm wondering if we should more thoroughly clean up CDCLK handling. Right now CDCLK is enabled during boot in odroidx2_late_probe() (even if you never play any audio), and then sometimes disabled by i2s.c after the sound device has been opened and then closed, with a half-broken attempt to sometimes enable it again next time it is opened. We're in this situation because that setup is pretty fragile and confusing... Is CDCLK something ODROID-specific? Perhaps we could have startup/shutdown hooks in odroidx2_max98090.c that start and stop the clock (with proper refcounting), and remove CDCLK interaction from i2s.c. Is CDCLK something more generic for Samsung i2s devices? In that case we could enable it in i2s_startup, disable it in i2s_shutdown, and avoid interacting with it at all from odroidx2_max98090.c. (again with proper refcounting) Thanks Daniel
diff --git a/sound/soc/samsung/i2s.c b/sound/soc/samsung/i2s.c index 2ac76fa..3cb3e95 100644 --- a/sound/soc/samsung/i2s.c +++ b/sound/soc/samsung/i2s.c @@ -68,6 +68,8 @@ struct i2s_dai { #define DAI_OPENED (1 << 0) /* Dai is opened */ #define DAI_MANAGER (1 << 1) /* Dai is the manager */ unsigned mode; + /* CDCLK pin direction: 0 - input, 1 - output */ + unsigned int cdclk_out:1; /* Driver for this DAI */ struct snd_soc_dai_driver i2s_dai_drv; /* DMA parameters */ @@ -737,6 +739,9 @@ static int i2s_startup(struct snd_pcm_substream *substream, spin_unlock_irqrestore(&lock, flags); + if (!is_opened(other) && i2s->cdclk_out) + i2s_set_sysclk(dai, SAMSUNG_I2S_CDCLK, + 0, SND_SOC_CLOCK_OUT); return 0; } @@ -752,9 +757,13 @@ static void i2s_shutdown(struct snd_pcm_substream *substream, i2s->mode &= ~DAI_OPENED; i2s->mode &= ~DAI_MANAGER; - if (is_opened(other)) + if (is_opened(other)) { other->mode |= DAI_MANAGER; - + } else { + u32 mod = readl(i2s->addr + I2SMOD); + i2s->cdclk_out = !(mod & MOD_CDCLKCON); + other->cdclk_out = i2s->cdclk_out; + } /* Reset any constraint on RFS and BFS */ i2s->rfs = 0; i2s->bfs = 0;