linux-spi.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Steffen Trumtrar <s.trumtrar-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
To: Mark Brown
	<broonie-yzvPICuk2AATkU/dhu1WVueM+bqZidxxQQ4Iyu8u01E@public.gmane.org>
Cc: alsa-devel-K7yf7f+aM1XWsZ/bQMPhNw@public.gmane.org,
	Sascha Hauer <kernel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>,
	devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org,
	patches-yzvPICuk2AATkU/dhu1WVueM+bqZidxxQQ4Iyu8u01E@public.gmane.org,
	Rob Herring <robherring2-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>,
	Rob Landley <rob-VoJi6FS/r0vR7s880joybQ@public.gmane.org>,
	spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org,
	Liam Girdwood <lrg-l0cyMroinI0@public.gmane.org>,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
Subject: Re: [PATCH 6/6] ASoC: fsl: add imx-wm8974 machine driver
Date: Fri, 9 Nov 2012 19:54:01 +0100	[thread overview]
Message-ID: <20121109185401.GA3660@pengutronix.de> (raw)
In-Reply-To: <20121109153629.GK23807-yzvPICuk2AATkU/dhu1WVueM+bqZidxxQQ4Iyu8u01E@public.gmane.org>

On Fri, Nov 09, 2012 at 03:36:30PM +0000, Mark Brown wrote:
> On Fri, Nov 09, 2012 at 03:00:25PM +0100, Steffen Trumtrar wrote:
> 
> > +static int imx_wm8974_dai_init(struct snd_soc_pcm_runtime *rtd)
> > +{
> > +	struct imx_wm8974_data *data = container_of(rtd->card,
> > +					struct imx_wm8974_data, card);
> > +	struct device *dev = rtd->card->dev;
> > +	int ret;
> > +
> > +	/* the pll stability peaks at N=8 and around 90MHz.
> > +	 * This values are best reached with a 12.288MHz or
> > +	 * 11.289MHz clock. As the first is closer to N=8 in
> > +	 * more situations, chose 12.288MHz as the target clock
> > +	 * (ref: datasheet section "Master Clock and Phase Locked Loop")*/
> > +	ret = snd_soc_dai_set_pll(rtd->codec_dai, 0, 0, data->clk_frequency,
> > +		12288000);
> 
> This seems far from ideal - it's starting the PLL at system boot and
> just leaving it running.  This will be power inefficient when the system
> is not playing audio as the PLL will be consuming power but not used for
> anything.  You could also do the same thing with the MCLK.
> 
> Note also that a 12.288MHz clock is only suitable for 8kHz based sample
> rates, for CD audio 44.1kHz based rates like most media files 11.2896MHz
> should be chosen.  The driver may want to switch dynamically at runtime
> to support the widest possible range of rates.
> 

Maybe I had a weird test system, but I only got correct audio with the
12.288 MHz clock from the PLL. And this was for 48Khz and 44Khz audio.
So, as the datasheet was not really that clear about the clock as you
are, I chose the above clock freq.


> > +	data->codec_clk = clk_get(&codec_pdev->dev, NULL);
> > +	if (IS_ERR(data->codec_clk)) {
> 
> devm_clk_get() - this'd free the clock that's leaked on removal.

Okay

Regards,
Steffen

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

------------------------------------------------------------------------------
Everyone hates slow websites. So do we.
Make your web apps faster with AppDynamics
Download AppDynamics Lite for free today:
http://p.sf.net/sfu/appdyn_d2d_nov

      parent reply	other threads:[~2012-11-09 18:54 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-11-09 14:00 [PATCH 0/6] of: add support for imx-wm8974 Steffen Trumtrar
     [not found] ` <1352469625-32024-1-git-send-email-s.trumtrar-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
2012-11-09 14:00   ` [PATCH 1/6] spi: imx: specify spi base for device tree probe Steffen Trumtrar
2012-11-09 16:38     ` Mark Brown
     [not found]       ` <20121109163830.GP23807-yzvPICuk2AATkU/dhu1WVueM+bqZidxxQQ4Iyu8u01E@public.gmane.org>
2012-11-09 19:00         ` Steffen Trumtrar
2012-11-09 14:00   ` [PATCH 2/6] spi/devicetree: find spi_device via device_node Steffen Trumtrar
2012-11-09 14:00   ` [PATCH 3/6] ASoC: wm8974: include MCLKDIV in pll_factors Steffen Trumtrar
     [not found]     ` <1352469625-32024-4-git-send-email-s.trumtrar-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
2012-11-09 14:55       ` Mark Brown
2012-11-09 14:00   ` [PATCH 4/6] ASoC: wm8974: add SPI as a possible bus master Steffen Trumtrar
     [not found]     ` <1352469625-32024-5-git-send-email-s.trumtrar-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
2012-11-09 14:38       ` Mark Brown
     [not found]         ` <20121109143841.GG23807-yzvPICuk2AATkU/dhu1WVueM+bqZidxxQQ4Iyu8u01E@public.gmane.org>
2012-11-09 14:55           ` Steffen Trumtrar
     [not found]             ` <20121109145504.GC8598-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
2012-11-09 14:59               ` Mark Brown
2012-11-09 14:00   ` [PATCH 5/6] ARM i.MX: rename ssi1 clock for imx27 Steffen Trumtrar
     [not found]     ` <1352469625-32024-6-git-send-email-s.trumtrar-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
2012-11-09 14:58       ` Mark Brown
2012-11-09 14:00   ` [PATCH 6/6] ASoC: fsl: add imx-wm8974 machine driver Steffen Trumtrar
     [not found]     ` <1352469625-32024-7-git-send-email-s.trumtrar-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
2012-11-09 15:36       ` Mark Brown
     [not found]         ` <20121109153629.GK23807-yzvPICuk2AATkU/dhu1WVueM+bqZidxxQQ4Iyu8u01E@public.gmane.org>
2012-11-09 18:54           ` Steffen Trumtrar [this message]

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=20121109185401.GA3660@pengutronix.de \
    --to=s.trumtrar-bicnvbalz9megne8c9+irq@public.gmane.org \
    --cc=alsa-devel-K7yf7f+aM1XWsZ/bQMPhNw@public.gmane.org \
    --cc=broonie-yzvPICuk2AATkU/dhu1WVueM+bqZidxxQQ4Iyu8u01E@public.gmane.org \
    --cc=devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org \
    --cc=kernel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org \
    --cc=linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org \
    --cc=lrg-l0cyMroinI0@public.gmane.org \
    --cc=patches-yzvPICuk2AATkU/dhu1WVueM+bqZidxxQQ4Iyu8u01E@public.gmane.org \
    --cc=rob-VoJi6FS/r0vR7s880joybQ@public.gmane.org \
    --cc=robherring2-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org \
    --cc=spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).