I2C controller vs. WM8903 suspend ordering

I2C controller vs. WM8903 suspend ordering

[Stephen Warren]

Mark, Liam,

We're facing a suspend ordering issue on Tegra, when playing audio when
suspend is initiated. Specifically, the I2C controller to which the WM8903
codec is attached is suspended before snd_soc_suspend is executed. This
then prevents any I2C register accesses during snd_soc_suspend, so various
analog paths are left active within the WM8903 during suspend, and this
causes a pop noise during resume.

(For reference, this is in the chromeos-2.6.38 kernel, with ASoC back-
ported from a point prior to 2.6.39)

We've found that switching the Tegra I2C controller from struct
platform_driver.{suspend,resume} to .{suspend,resume}_noirq solves this.
However, this seems like it's more of an accident than registering an
explicit dependency, especially since most I2C controllers just use
suspend rather than suspend_noirq.

Is there a way to explicitly indicate that sound should be suspended before
the I2C controller? I assume the issue is more that snd_soc_suspend shuts
down various DAPM paths, which can't actually program the HW for this when
this happens after I2C is shut down, rather than just the I2C controller
being suspended before the WM8903 driver itself (which I assume doesn't
happen, but didn't check).

Thanks for any pointers!

-- 
nvpublic

Re: I2C controller vs. WM8903 suspend ordering

[Liam Girdwood]

On 02/08/11 19:34, Stephen Warren wrote:
> Mark, Liam,
> 
> We're facing a suspend ordering issue on Tegra, when playing audio when
> suspend is initiated. Specifically, the I2C controller to which the WM8903
> codec is attached is suspended before snd_soc_suspend is executed. This
> then prevents any I2C register accesses during snd_soc_suspend, so various
> analog paths are left active within the WM8903 during suspend, and this
> causes a pop noise during resume.
> 
> (For reference, this is in the chromeos-2.6.38 kernel, with ASoC back-
> ported from a point prior to 2.6.39)
> 
> We've found that switching the Tegra I2C controller from struct
> platform_driver.{suspend,resume} to .{suspend,resume}_noirq solves this.
> However, this seems like it's more of an accident than registering an
> explicit dependency, especially since most I2C controllers just use
> suspend rather than suspend_noirq.
> 
> Is there a way to explicitly indicate that sound should be suspended before
> the I2C controller? I assume the issue is more that snd_soc_suspend shuts
> down various DAPM paths, which can't actually program the HW for this when
> this happens after I2C is shut down, rather than just the I2C controller
> being suspended before the WM8903 driver itself (which I assume doesn't
> happen, but didn't check).
> 
> Thanks for any pointers!
> 

IIRC, with stand alone I2C devices the child will suspend before it's parent I2C controller (due to registration order).

Now I think in this case we may not have such a direct relationship between the I2C controller and the soc audio device and thus we dont know how to order the suspend correctly.

Your fix just re-orders the calls wrt the I2C controller (PM has a call sequence described in it's kernel docs) so at least we have something that now "works" but is maybe not ideal. It maybe better if we change the soc PM DAPM shutdown to be called before PM suspend (e.g. move to PM prepare()) thus guaranteeing the I2C controller is alive when we do the DAPM calls. 

Could you take a look into this.

Thanks

Liam

Re: I2C controller vs. WM8903 suspend ordering

[Mark Brown]

On Tue, Aug 02, 2011 at 11:34:18AM -0700, Stephen Warren wrote:

> We've found that switching the Tegra I2C controller from struct
> platform_driver.{suspend,resume} to .{suspend,resume}_noirq solves this.
> However, this seems like it's more of an accident than registering an
> explicit dependency, especially since most I2C controllers just use
> suspend rather than suspend_noirq.

This is about as good as it gets, actually - I2C controllers should
generally suspend very late and resume early as they're needed for core
system components like the RTC and PMIC.

> Is there a way to explicitly indicate that sound should be suspended before
> the I2C controller? I assume the issue is more that snd_soc_suspend shuts

No, this is a general limitation in the Linux PM model.  Everything goes
by the order that devices got registered in.  There's nothing doing at
the minute without core work other than bodges like _noirq.