All of lore.kernel.org
 help / color / mirror / Atom feed
From: Mark Brown <broonie@opensource.wolfsonmicro.com>
To: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
Cc: Kuninori Morimoto <morimoto.kuninori@renesas.com>,
	alsa-devel@alsa-project.org, linux-sh@vger.kernel.org,
	Magnus Damm <damm@opensource.se>,
	Liam Girdwood <lrg@slimlogic.co.uk>
Subject: Re: [PATCH 1/4] ASoC: add a WM8978 codec driver
Date: Fri, 22 Jan 2010 10:35:04 +0000	[thread overview]
Message-ID: <20100122103504.GC10997@rakim.wolfsonmicro.main> (raw)
In-Reply-To: <Pine.LNX.4.64.1001220934230.4652@axis700.grange>

On Fri, Jan 22, 2010 at 09:35:44AM +0100, Guennadi Liakhovetski wrote:
> On Wed, 20 Jan 2010, Mark Brown wrote:

> > That seems wrong - if they're being managed in the bias level
> > configuration they ought to be being turned off at some point.  If they
> > should be set all the time then set them during chip init.

> Right, a couple of lines below:

> +	case SND_SOC_BIAS_OFF:
> +		snd_soc_write(codec, WM8978_POWER_MANAGEMENT_1, 0);

Yeah, I saw that.  This is one of those cases where numeric bitmask
based updates make things much harder to follow - without referring to
the datasheet and thinking about it it's not clear what's going on and
if it's intentional.

I'd suggest changing all this code to use snd_soc_update_bits() and
symbolic names for the bits that are getting twiddled - it really is a
lot easier to follow.

WARNING: multiple messages have this Message-ID (diff)
From: Mark Brown <broonie@opensource.wolfsonmicro.com>
To: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
Cc: Kuninori Morimoto <morimoto.kuninori@renesas.com>,
	alsa-devel@alsa-project.org, linux-sh@vger.kernel.org,
	Magnus Damm <damm@opensource.se>,
	Liam Girdwood <lrg@slimlogic.co.uk>
Subject: Re: [alsa-devel] [PATCH 1/4] ASoC: add a WM8978 codec driver
Date: Fri, 22 Jan 2010 10:35:04 +0000	[thread overview]
Message-ID: <20100122103504.GC10997@rakim.wolfsonmicro.main> (raw)
In-Reply-To: <Pine.LNX.4.64.1001220934230.4652@axis700.grange>

On Fri, Jan 22, 2010 at 09:35:44AM +0100, Guennadi Liakhovetski wrote:
> On Wed, 20 Jan 2010, Mark Brown wrote:

> > That seems wrong - if they're being managed in the bias level
> > configuration they ought to be being turned off at some point.  If they
> > should be set all the time then set them during chip init.

> Right, a couple of lines below:

> +	case SND_SOC_BIAS_OFF:
> +		snd_soc_write(codec, WM8978_POWER_MANAGEMENT_1, 0);

Yeah, I saw that.  This is one of those cases where numeric bitmask
based updates make things much harder to follow - without referring to
the datasheet and thinking about it it's not clear what's going on and
if it's intentional.

I'd suggest changing all this code to use snd_soc_update_bits() and
symbolic names for the bits that are getting twiddled - it really is a
lot easier to follow.

  reply	other threads:[~2010-01-22 10:35 UTC|newest]

Thread overview: 57+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-01-19  8:08 [PATCH 0/4] ALSA: SH: add ASoC driver for SIU audio engine, an audio codec and platform support Guennadi Liakhovetski
2010-01-19  8:08 ` [PATCH 0/4] ALSA: SH: add ASoC driver for SIU audio engine, an audio Guennadi Liakhovetski
2010-01-19  8:08 ` [PATCH 1/4] ASoC: add a WM8978 codec driver Guennadi Liakhovetski
2010-01-19  8:08   ` Guennadi Liakhovetski
2010-01-19 10:46   ` [alsa-devel] " Liam Girdwood
2010-01-19 10:46     ` Liam Girdwood
2010-01-20 20:01     ` Guennadi Liakhovetski
2010-01-20 20:01       ` [alsa-devel] " Guennadi Liakhovetski
2010-01-20 20:21       ` Mark Brown
2010-01-20 20:21         ` [alsa-devel] " Mark Brown
2010-01-20 20:25       ` Liam Girdwood
2010-01-19 10:57   ` Mark Brown
     [not found]   ` <20100119105729.GA32559@opensource.wolfsonmicro.com::587>
2010-01-20 19:50     ` Guennadi Liakhovetski
2010-01-20 19:50       ` Guennadi Liakhovetski
2010-01-20 20:17       ` Mark Brown
2010-01-20 20:17         ` [alsa-devel] " Mark Brown
2010-01-22  8:35         ` Guennadi Liakhovetski
2010-01-22  8:35           ` Guennadi Liakhovetski
2010-01-22 10:35           ` Mark Brown [this message]
2010-01-22 10:35             ` Mark Brown
2010-01-22 16:27   ` [PATCH 1/4 v2] " Guennadi Liakhovetski
2010-01-22 16:27     ` Guennadi Liakhovetski
2010-01-22 17:39     ` Liam Girdwood
2010-01-23 20:47     ` [alsa-devel] " Mark Brown
2010-01-23 20:47       ` Mark Brown
2010-01-26 13:04       ` Guennadi Liakhovetski
2010-01-26 13:04         ` Guennadi Liakhovetski
2010-01-26 13:26         ` Mark Brown
2010-01-26 13:26           ` [alsa-devel] " Mark Brown
2010-01-26 14:08           ` Guennadi Liakhovetski
2010-01-26 14:08             ` [alsa-devel] " Guennadi Liakhovetski
2010-01-26 15:22             ` Mark Brown
2010-01-19  8:09 ` [PATCH 2/4] ASoC: add DAI and platform drivers for SH SIU and support for the Migo-R board Guennadi Liakhovetski
2010-01-19  8:09   ` [PATCH 2/4] ASoC: add DAI and platform drivers for SH SIU and support Guennadi Liakhovetski
2010-01-19 11:13   ` [alsa-devel] [PATCH 2/4] ASoC: add DAI and platform drivers for SH SIU and support for the Migo-R board Liam Girdwood
2010-01-19 11:13     ` [alsa-devel] [PATCH 2/4] ASoC: add DAI and platform drivers Liam Girdwood
2010-01-19 12:34   ` [PATCH 2/4] ASoC: add DAI and platform drivers for SH SIU and support for the Migo-R board Mark Brown
2010-01-19 12:34     ` [PATCH 2/4] ASoC: add DAI and platform drivers for SH SIU and Mark Brown
2010-01-22 18:09   ` [PATCH 2a/4 v2] ASoC: add DAI and platform / DMA drivers for SH SIU Guennadi Liakhovetski
2010-01-22 18:09     ` Guennadi Liakhovetski
2010-01-25 13:58     ` Liam Girdwood
2010-01-25 13:58       ` [PATCH 2a/4 v2] ASoC: add DAI and platform / DMA drivers for Liam Girdwood
2010-01-25 15:06     ` [PATCH 2a/4 v2] ASoC: add DAI and platform / DMA drivers for SH SIU Mark Brown
2010-01-25 15:06       ` [PATCH 2a/4 v2] ASoC: add DAI and platform / DMA drivers for Mark Brown
2010-01-22 18:17   ` [PATCH 2b/4 v2] ASoC: add support for the sh7722 Migo-R board Guennadi Liakhovetski
2010-01-22 18:17     ` Guennadi Liakhovetski
2010-01-25 13:21     ` Mark Brown
2010-01-25 13:47       ` [alsa-devel] " Liam Girdwood
2010-01-25 13:47         ` [alsa-devel] [PATCH 2b/4 v2] ASoC: add support for the sh7722 Liam Girdwood
2010-01-19  8:09 ` [PATCH 3/4] sh: add DMA slave definitions and SIU platform data to sh7722 setup Guennadi Liakhovetski
2010-01-19  8:09   ` [PATCH 3/4] sh: add DMA slave definitions and SIU platform data to Guennadi Liakhovetski
2010-01-19  8:09 ` [PATCH 4/4] sh: audio support for the sh7722 Migo-R board Guennadi Liakhovetski
2010-01-19  8:09   ` Guennadi Liakhovetski
  -- strict thread matches above, loose matches on Subject: below --
2010-01-27 11:15 [PATCH 2b/4 v2] ASoC: add " Guennadi Liakhovetski
2010-01-27 11:15 ` Guennadi Liakhovetski
2010-01-29 14:13 ` [alsa-devel] " Mark Brown
2010-01-29 14:13   ` [alsa-devel] [PATCH 2b/4 v2] ASoC: add support for the sh7722 Mark Brown

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20100122103504.GC10997@rakim.wolfsonmicro.main \
    --to=broonie@opensource.wolfsonmicro.com \
    --cc=alsa-devel@alsa-project.org \
    --cc=damm@opensource.se \
    --cc=g.liakhovetski@gmx.de \
    --cc=linux-sh@vger.kernel.org \
    --cc=lrg@slimlogic.co.uk \
    --cc=morimoto.kuninori@renesas.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.