All of lore.kernel.org
 help / color / mirror / Atom feed
From: Mark Brown <broonie@opensource.wolfsonmicro.com>
To: Ashish Chavan <ashish.chavan@kpitcummins.com>
Cc: alsa-devel <alsa-devel@alsa-project.org>,
	David Dajun Chen <david.chen@diasemi.com>, lrg <lrg@ti.com>,
	linux-kernel <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] ASoC: codecs: Add DA9055 codec driver
Date: Wed, 12 Sep 2012 20:49:34 +0800	[thread overview]
Message-ID: <20120912124932.GC19055@opensource.wolfsonmicro.com> (raw)
In-Reply-To: <1347454578.23366.305.camel@matrix>

On Wed, Sep 12, 2012 at 06:26:18PM +0530, Ashish Chavan wrote:
> On Wed, 2012-09-12 at 10:57 +0800, Mark Brown wrote:

> > Why is any of this being exposed to userspace?  If this should be
> > configured I'd expect it to be static platform data, not something that
> > gets changed at runtime.

> These parameters are exposed considering the fact that DMIC itself is
> not part of the codec. Codec only provides DMIC interface using which an
> external DMIC can be attached. These parameters depend on the actual
> DMIC hardware and hence kept configurable to allow runtime plug in of
> any DMIC hardware. Doesn't it make sense to keep them runtime
> configurable?

No.  The only realistic way to attach a new DMIC to a board is to solder
it down, that's not something people are going to do while the system is
actve.  It's something that's fixed at PCB design time.

> > > +	/* In slave mode, there is only one set of divisors */
> > > +	if (!da9055->master)
> > > +		fout = 2822400;

> > Should check the user supplied this value

> Can you explain which value / user supplied value you are referring to?
> It is not quite clear to me.

The specified output frequency, you just totally ignore it.

> For other things like input mixers, Headphone and Lineouts, DAPM is
> already used to control power specific bits. The confusion is because
> there are two separate control bits for these blocks. One bit is for
> "Enabling" that block and other is for "Enabling Amplifier" of that
> block.
> e.g for headphone, one bit is for "output enable" while other is for
> "output amplifier enable".

So document this.

WARNING: multiple messages have this Message-ID (diff)
From: Mark Brown <broonie@opensource.wolfsonmicro.com>
To: Ashish Chavan <ashish.chavan@kpitcummins.com>
Cc: lrg <lrg@ti.com>, alsa-devel <alsa-devel@alsa-project.org>,
	linux-kernel <linux-kernel@vger.kernel.org>,
	David Dajun Chen <david.chen@diasemi.com>
Subject: Re: [alsa-devel] [PATCH] ASoC: codecs: Add DA9055 codec driver
Date: Wed, 12 Sep 2012 20:49:34 +0800	[thread overview]
Message-ID: <20120912124932.GC19055@opensource.wolfsonmicro.com> (raw)
In-Reply-To: <1347454578.23366.305.camel@matrix>

On Wed, Sep 12, 2012 at 06:26:18PM +0530, Ashish Chavan wrote:
> On Wed, 2012-09-12 at 10:57 +0800, Mark Brown wrote:

> > Why is any of this being exposed to userspace?  If this should be
> > configured I'd expect it to be static platform data, not something that
> > gets changed at runtime.

> These parameters are exposed considering the fact that DMIC itself is
> not part of the codec. Codec only provides DMIC interface using which an
> external DMIC can be attached. These parameters depend on the actual
> DMIC hardware and hence kept configurable to allow runtime plug in of
> any DMIC hardware. Doesn't it make sense to keep them runtime
> configurable?

No.  The only realistic way to attach a new DMIC to a board is to solder
it down, that's not something people are going to do while the system is
actve.  It's something that's fixed at PCB design time.

> > > +	/* In slave mode, there is only one set of divisors */
> > > +	if (!da9055->master)
> > > +		fout = 2822400;

> > Should check the user supplied this value

> Can you explain which value / user supplied value you are referring to?
> It is not quite clear to me.

The specified output frequency, you just totally ignore it.

> For other things like input mixers, Headphone and Lineouts, DAPM is
> already used to control power specific bits. The confusion is because
> there are two separate control bits for these blocks. One bit is for
> "Enabling" that block and other is for "Enabling Amplifier" of that
> block.
> e.g for headphone, one bit is for "output enable" while other is for
> "output amplifier enable".

So document this.

  reply	other threads:[~2012-09-12 12:49 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-09-11 15:03 [alsa-devel] [PATCH] ASoC: codecs: Add DA9055 codec driver Ashish Chavan
2012-09-12  2:57 ` Mark Brown
2012-09-12 12:56   ` Ashish Chavan
2012-09-12 12:56     ` [alsa-devel] " Ashish Chavan
2012-09-12 12:49     ` Mark Brown [this message]
2012-09-12 12:49       ` Mark Brown
2012-09-12 13:40       ` Ashish P. Chavan
2012-09-12 13:40         ` [alsa-devel] " Ashish P. Chavan
2012-09-13 12:08   ` Ashish Chavan
2012-09-13 12:08     ` [alsa-devel] " Ashish Chavan
2012-09-13 12:03     ` Mark Brown
2012-09-13 12:03       ` [alsa-devel] " 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=20120912124932.GC19055@opensource.wolfsonmicro.com \
    --to=broonie@opensource.wolfsonmicro.com \
    --cc=alsa-devel@alsa-project.org \
    --cc=ashish.chavan@kpitcummins.com \
    --cc=david.chen@diasemi.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lrg@ti.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.