alsa-devel.alsa-project.org archive mirror
 help / color / mirror / Atom feed
From: "M R Swami Reddy" <MR.Swami.Reddy@ti.com>
To: Mark Brown <broonie@opensource.wolfsonmicro.com>
Cc: "alsa-devel@alsa-project.org" <alsa-devel@alsa-project.org>,
	"Girdwood, Liam" <lrg@ti.com>
Subject: Re: [PATCH v2] ASoC: Add support for TI LM49453 Audio codec
Date: Mon, 06 Feb 2012 16:00:23 +0530	[thread overview]
Message-ID: <4F2FABBF.7010604@ti.com> (raw)

Mark Brown Wrote:
 > > ---
 > > Changes made in V2
 > > o Removed fll and vco reference frequency settings from _set_dai_pll()
 > > o Reworked chip enable and disable in _STANDBY and _OFF modes in
 > > m49453_set_bias_level, as per the review comments.

 >You say this is version 2 but there's been rather more than two versions
 > posted...

Thats right. I started numbering the patch version with previous one, so 
mentioned it as v2. The version number will be continued.


 > > +static int lm49453_set_dai_pll(struct snd_soc_dai *codec_dai, int pll_id,
 > > +                              int source, unsigned int freq_in,
 > > +                              unsigned int freq_out) {

 >This is now a bit odd.

I will update it as per the other drivers formate.


 > > +       state->in = freq_in;
 > > +       state->out = freq_out;

 >It stores but otherwise ignores the configuration which was passed in (which
 > seems more than a little odd).

Will be removed in the next patch.


 > > +       /* Always disable the PLL - it is not safe to leave it running
 > > +        * while reprogramming it.
 > > +        */
 > > +       snd_soc_update_bits(codec, LM49453_P0_PMC_SETUP_REG,
 > > +                           LM49453_PMC_SETUP_PLL_EN, 0);
 > > +
 > > +       if (!freq_in || !freq_out)
 > > +               return 0;
 > > +
 > > +
 > > +       /* All done, turn it on */
 > > +       snd_soc_update_bits(codec, LM49453_P0_PMC_SETUP_REG, pwr_mask,
 > > +                           pwr_mask);

 >Then if the PLL was already enabled it bounces the power briefly.
 >Really it seems like this should just be merged in with set_sysclk() - you're
 >not actually configuring the PLL at all, just turning it on and off, at which 
 >point it's just another SYSCLK source.


Ok. The PLL disable code will be removed and the PLL enable code will be moved 
to _sysclk().

 >Otherwise this seems good, just this clock configuration stuff to sort out.

OK. Thank you. Do I need to resend the patch.

Thanks
Swami

             reply	other threads:[~2012-02-06 10:30 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-02-06 10:30 M R Swami Reddy [this message]
     [not found] <4F2C0DBC.50100@ti.com>
2012-02-04 17:00 ` [PATCH v2] ASoC: Add support for TI LM49453 Audio codec 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=4F2FABBF.7010604@ti.com \
    --to=mr.swami.reddy@ti.com \
    --cc=alsa-devel@alsa-project.org \
    --cc=broonie@opensource.wolfsonmicro.com \
    --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 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).