From: Mark Brown <broonie@opensource•wolfsonmicro.com>
To: Jon Smirl <jonsmirl@gmail•com>
Cc: linuxppc-dev@ozlabs•org, alsa-devel@alsa-project•org
Subject: Re: [alsa-devel] [PATCH V2 6/9] Codec for STAC9766 used on the Efika
Date: Sun, 24 May 2009 12:46:35 +0100 [thread overview]
Message-ID: <20090524114635.GF4933@sirena.org.uk> (raw)
In-Reply-To: <20090523231307.17919.5277.stgit@terra>
On Sat, May 23, 2009 at 07:13:07PM -0400, Jon Smirl wrote:
> AC97 codec for STAC9766 used on the Efika.
> Datasheet: http://www.idt.com/products/getDoc.cfm?docID=13134007
>
> Signed-off-by: Jon Smirl <jonsmirl@gmail•com>
This looks mostly good, a couple of issues below. I'll apply the patch
but please submit a followup patch for resume as discussed below.
> +static int ac97_digital_trigger(struct snd_pcm_substream *substream,
> + int cmd, struct snd_soc_dai *dai)
Very strange indentation here...
> +static int stac9766_codec_resume(struct platform_device *pdev)
> +{
> + /* give the codec an AC97 warm reset to start the link */
> +reset:
> + if (reset > 5) {
> + printk(KERN_ERR "stac9766 failed to resume");
> + return -EIO;
> + }
> + codec->ac97->bus->ops->warm_reset(codec->ac97);
> + id = soc_ac97_ops.read(codec->ac97, AC97_VENDOR_ID2);
> + if (id != 0x4c13) {
> + stac9766_reset(codec, 0);
> + reset++;
> + goto reset;
> + }
As I said last time I'd this looks like it should be using the reset
function that you've provided?
The loop is new since last time and seems very suspicious - are you sure
that this is a CODEC problem that's being worked around and not an issue
with the AC97 controller or with the specific system starting up the
master clock for the CODEC after resume? If it's an issue with the
controler then it'd be better to fix it there so that all CODEC drivers
get the benefit. If it's a CODEC issue a comment explaining the problem
would be helpful since it's not the sort of issue I'd expect to see in a
CODEC. The fact that it's not needed on initial boot suggests something
controller or external clock related.
Either way, it'd be much clearer to rewrite it using a for or while loop
rather than a goto. Please submit a followup patch fixing at least this
issue.
> +static int __init stac9766_modinit(void)
> +{
> + return snd_soc_register_dais(stac9766_dai, ARRAY_SIZE(stac9766_dai));
> +}
> +module_init(stac9766_modinit);
> +
> +static void __exit stac9766_exit(void)
> +{
> + snd_soc_unregister_dais(stac9766_dai, ARRAY_SIZE(stac9766_dai));
> +}
> +module_exit(stac9766_exit);
These are not needed for AC97 CODECs, ASoC doesn't wait for them to
probe. I'll apply a patch removing these.
next prev parent reply other threads:[~2009-05-24 11:46 UTC|newest]
Thread overview: 35+ messages / expand[flat|nested] mbox.gz Atom feed top
2009-05-23 23:12 [PATCH V2 0/9] mpc5200 audio rework for AC97 Jon Smirl
2009-05-23 23:12 ` [PATCH V2 1/9] Register the wm9712 DAIs on module load Jon Smirl
2009-05-24 11:11 ` [alsa-devel] " Mark Brown
2009-05-24 15:28 ` Jon Smirl
2009-05-24 15:32 ` Jon Smirl
2009-05-24 19:14 ` Mark Brown
2009-05-23 23:12 ` [PATCH V2 2/9] Basic split of mpc5200 DMA code out from mpc5200_psc_i2s Jon Smirl
2009-05-24 14:11 ` Grant Likely
2009-05-23 23:13 ` [PATCH V2 3/9] Rename the PSC functions to DMA Jon Smirl
2009-05-24 11:15 ` [alsa-devel] " Mark Brown
2009-05-24 14:12 ` Grant Likely
2009-05-23 23:13 ` [PATCH V2 4/9] Add a few more mpc5200 PSC defines Jon Smirl
2009-05-24 14:13 ` Grant Likely
2009-05-24 18:00 ` Mark Brown
2009-05-24 18:19 ` Grant Likely
2009-05-24 18:32 ` [alsa-devel] " Mark Brown
2009-05-23 23:13 ` [PATCH V2 5/9] Main rewite of the mpc5200 audio DMA code Jon Smirl
2009-05-24 11:26 ` [alsa-devel] " Mark Brown
2009-05-24 18:55 ` Wolfram Sang
2009-05-24 20:10 ` Jon Smirl
2009-05-24 20:35 ` Wolfram Sang
2009-05-25 7:56 ` Juergen Beisert
2009-05-25 10:51 ` Mark Brown
2009-05-23 23:13 ` [PATCH V2 6/9] Codec for STAC9766 used on the Efika Jon Smirl
2009-05-24 11:46 ` Mark Brown [this message]
2009-05-23 23:13 ` [PATCH V2 7/9] AC97 driver for mpc5200 Jon Smirl
2009-05-24 12:10 ` [alsa-devel] " Mark Brown
2009-05-23 23:13 ` [PATCH V2 8/9] Fabric bindings for STAC9766 on the Efika Jon Smirl
2009-05-24 12:12 ` [alsa-devel] " Mark Brown
2009-05-23 23:13 ` [PATCH V2 9/9] Support for AC97 on Phytec pmc030 base board Jon Smirl
2009-05-24 12:13 ` [alsa-devel] " Mark Brown
2009-05-23 23:20 ` [PATCH V2 0/9] mpc5200 audio rework for AC97 Jon Smirl
2009-05-24 11:08 ` [alsa-devel] " Mark Brown
2009-05-24 15:21 ` Jon Smirl
2009-05-24 18:35 ` 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=20090524114635.GF4933@sirena.org.uk \
--to=broonie@opensource$(echo .)wolfsonmicro.com \
--cc=alsa-devel@alsa-project$(echo .)org \
--cc=jonsmirl@gmail$(echo .)com \
--cc=linuxppc-dev@ozlabs$(echo .)org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
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