On Sun, May 24, 2009 at 09:38:49PM -0400, Jon Smirl wrote:
I've implemented retries for when the AC97 hardware doesn't reset on first try. About 10% of the time both the Efika and pcm030 AC97 codecs don't reset on first try and need to be poked multiple times. Failure is indicated by not having the link clock start ticking. Every once in a while even five pokes won't get the link started and I have to power cycle.
This smells like either a very broken board or some issue with starting the master clock for the CODEC - if the CODEC is clocked by the AC97 controller you may need to do something to ensure that it has finished starting up before initiating the reset.
+static int psc_ac97_cold_reset_check(struct snd_ac97 *ac97) +{
- int max_reset, timeout;
- struct mpc52xx_psc __iomem *regs = psc_dma->psc_regs;
- /* AC97 clock is generated by the codec.
* Ensure that it starts ticking after codec reset.*/
The AC97 standard allows CODECs to come out of cold reset with the link disabled. With those CODECs this is going fail every time - they need a warm reset to come on-line.
If this really is a general issue with the AC97 controller here you'll need to do a warm reset in here. It's not ideal but extra warm resets will cause less harm than completely failing to come on-line.
+static int psc_ac97_trigger(struct snd_pcm_substream *substream, int cmd,
struct snd_soc_dai *dai)
I keep mentioning the indentation issues with your code without seeing any response from you. If you run checkpatch over your code you'll also see a bunch of complaints about using spaces instead of tabs for indentation. It looks for all the world like you're using 4 character tabs instead of the 8 character tabs which are the kernel standard.
- switch (cmd) {
- case SNDRV_PCM_TRIGGER_STOP:
if (substream->pstr->stream == SNDRV_PCM_STREAM_CAPTURE)psc_dma->slots &= 0xFFFF0000;elsepsc_dma->slots &= 0x0000FFFF;spin_lock(&psc_dma->lock);out_be32(&psc_dma->psc_regs->ac97_slots, psc_dma->slots);spin_unlock(&psc_dma->lock);break;
This locking looks wrong - I'd expect it to also cover the modification of psc_dma->slots? Otherwise it's hard to see what it buys you.
- /* AC97 clock is generated by the codec.
* Ensure that it starts ticking after codec reset.*/- rc = psc_ac97_cold_reset_check(&ac97);
- if (rc != 0) {
dev_err(&op->dev, "AC97 codec failed to reset\n");mpc5200_audio_dma_destroy(op);return rc;- }
Your AC97 driver should not be doing this - leave it to the card and CODEC driver to bring things on line.
- /* Go */
- out_8(®s->command, MPC52xx_PSC_TX_ENABLE | MPC52xx_PSC_RX_ENABLE);
As I said last time I'd expect this to be deferred to the ASoC device probe.