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
next 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).