[2/2] CHROMIUM: ASoC: Tegra: Seaboard: Control power to digital mics

CHROMIUM: ASoC: Tegra: Seaboard: Control power to digital mics

The power supply to the digital microphones is controlled by a regulator.
Call the regulator APIs to enable this power rail when the digital mics
are being used.

Signed-off-by: Stephen Warren <swarren@nvidia.com>;

BUG=chrome-os-partner:3240
TEST=Tested recording from DMIC: 
  Run alsamixer to configure system for DMIC capture: 
  alsamixer 
    Left Capture Mux: Left 
    Left ADC Input: DMIC 
    Right ADC Input: Toggle to ADC then back to DMIC
  Record audio: 
    arecord -r 48000 -c 2 -f s16 -V stereo foo.wav 
  Play audio: 
    aplay foo.wav

[Stephen Warren, 2011-04-21 23:25:33]

Initial review request.

2 patches in this series:
http://codereview.chromium.org/6895005
http://codereview.chromium.org/6894006

Test instructions apply only after both patches applied.

[jiesun, 2011-04-25 18:21:04]

http://codereview.chromium.org/6894006/diff/1/sound/soc/tegra/seaboard.c
File sound/soc/tegra/seaboard.c (right):

http://codereview.chromium.org/6894006/diff/1/sound/soc/tegra/seaboard.c#newc...
sound/soc/tegra/seaboard.c:64: bool first_dmic_event;
I understand this is for preventing regulator_disable before enable, based on
the assumption that event on/off are interleaving. I do not know if that's
always true. wouldn't that keep the enable status of that regulator be more
reliable?

http://codereview.chromium.org/6894006/diff/1/sound/soc/tegra/seaboard.c#newc...
sound/soc/tegra/seaboard.c:477: card->dev = NULL;
missing a regulator_put here.

[Stephen Warren, 2011-04-25 22:27:41]

Issue updated to address review feedback.

http://codereview.chromium.org/6894006/diff/1/sound/soc/tegra/seaboard.c
File sound/soc/tegra/seaboard.c (right):

http://codereview.chromium.org/6894006/diff/1/sound/soc/tegra/seaboard.c#newc...
sound/soc/tegra/seaboard.c:64: bool first_dmic_event;
The issue isn't continual interleaving; it's a startup condition.

Initially, the new consumer entry for the DMICs is off. If we attempt to turn it
off again, the kernel will complain.

However, the ASoC core calls seaboard_event_dmic(off) at startup, which would
usually trigger this complaint. this variable is used to indicate if the call to
seaboard_event_dmic is the very first, and if so, ignore any request to turn off
the regulator, since we know it's already off. In any other case, we just do
exactly what the ASoC core tells us to; it doesn't give us any duplicate
events.

Note that we can't query the regulator API to determine if the power rail is
off; it reports the power rail status, not the individual consumer status.
Specifically, if the camera consumer is on, and hence the power rail is on, if
we query the regulator API to get the state, it'll say on. If we avoid turning
our consumer on in this case (since desired == current state), then when the
camera is turned off, it'll turn off the rail, since no other consumers are
marked active, which will turn off the DMICs too.


Should I describe this in a comment in seaboard_event_dmic?

[jiesun1, 2011-04-25 22:41:25]

if the ASoc core doesn't generate duplicate events, then I am okay with this.
What I had suggested is to replace a flag (that indicate whether this is first
dmic event) with a flag ( that indicate whether we had enabled regulator by this
driver. we do not need to query regulator API.) but I will let Olof to decide.
LGTM.

[Stephen Warren, 2011-04-26 15:30:56]

Oh right. I understand now. Yes, I suppose that'd be a little more obvious, and
protect the driver in case the ASoC core behaviour ever did change. I'll update
it to do that.

[Stephen Warren, 2011-04-26 16:58:13]

Uploaded patch set 3 to rework variable used to guard regulator enable/disable.

[jiesun1, 2011-04-26 17:17:39]

LGTM, thanks!

[Olof Johansson, 2011-04-27 17:12:09]

Thanks, applied.