[alsa-devel] [PATCH V3 2/4] AC97 driver for mpc5200

Grant Likely grant.likely at secretlab.ca
Mon May 25 08:16:51 CEST 2009


On Sun, May 24, 2009 at 7:38 PM, Jon Smirl <jonsmirl at gmail.com> wrote:
> AC97 driver for mpc5200
>
> 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.
>
> Signed-off-by: Jon Smirl <jonsmirl at gmail.com>
> ---
>  sound/soc/fsl/Kconfig            |   11 +
>  sound/soc/fsl/Makefile           |    1
>  sound/soc/fsl/mpc5200_psc_ac97.c |  392 ++++++++++++++++++++++++++++++++++++++
>  sound/soc/fsl/mpc5200_psc_ac97.h |   15 +
>  4 files changed, 419 insertions(+), 0 deletions(-)
>  create mode 100644 sound/soc/fsl/mpc5200_psc_ac97.c
>  create mode 100644 sound/soc/fsl/mpc5200_psc_ac97.h
>
> diff --git a/sound/soc/fsl/Kconfig b/sound/soc/fsl/Kconfig
> index 1918c78..3bce952 100644
> --- a/sound/soc/fsl/Kconfig
> +++ b/sound/soc/fsl/Kconfig
> @@ -29,3 +29,14 @@ config SND_SOC_MPC5200_I2S
>        select PPC_BESTCOMM_GEN_BD
>        help
>          Say Y here to support the MPC5200 PSCs in I2S mode.
> +
> +config SND_SOC_MPC5200_AC97
> +       tristate "Freescale MPC5200 PSC in AC97 mode driver"
> +       depends on PPC_MPC52xx && PPC_BESTCOMM
> +       select AC97_BUS
> +       select SND_MPC52xx_DMA
> +       select PPC_BESTCOMM_GEN_BD
> +       help
> +         Say Y here to support the MPC5200 PSCs in AC97 mode.
> +
> +
> diff --git a/sound/soc/fsl/Makefile b/sound/soc/fsl/Makefile
> index 7731ef2..14631a1 100644
> --- a/sound/soc/fsl/Makefile
> +++ b/sound/soc/fsl/Makefile
> @@ -13,4 +13,5 @@ obj-$(CONFIG_SND_SOC_MPC8610) += snd-soc-fsl-ssi.o snd-soc-fsl-dma.o
>  # MPC5200 Platform Support
>  obj-$(CONFIG_SND_MPC52xx_DMA) += mpc5200_dma.o
>  obj-$(CONFIG_SND_SOC_MPC5200_I2S) += mpc5200_psc_i2s.o
> +obj-$(CONFIG_SND_SOC_MPC5200_AC97) += mpc5200_psc_ac97.o
>
> diff --git a/sound/soc/fsl/mpc5200_psc_ac97.c b/sound/soc/fsl/mpc5200_psc_ac97.c
> new file mode 100644
> index 0000000..480b677
> --- /dev/null
> +++ b/sound/soc/fsl/mpc5200_psc_ac97.c
> @@ -0,0 +1,392 @@
> +/*
> + * linux/sound/mpc5200-ac97.c -- AC97 support for the Freescale MPC52xx chip.
> + *
> + * Copyright (C) 2009 Jon Smirl, Digispeaker
> + * Author: Jon Smirl <jonsmirl at gmail.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#include <linux/module.h>
> +#include <linux/of_device.h>
> +#include <linux/of_platform.h>
> +
> +#include <sound/pcm.h>
> +#include <sound/pcm_params.h>
> +#include <sound/soc.h>
> +
> +#include <asm/mpc52xx_psc.h>
> +
> +#include "mpc5200_dma.h"
> +#include "mpc5200_psc_ac97.h"
> +
> +#define DRV_NAME "mpc5200-psc-ac97"
> +
> +/* ALSA only supports a single AC97 device so static is recommend here */
> +static struct psc_dma *psc_dma;
> +
> +static unsigned short psc_ac97_read(struct snd_ac97 *ac97, unsigned short reg)
> +{
> +       int timeout;
> +       unsigned int val;
> +
> +       spin_lock(&psc_dma->lock);
> +
> +       /* Wait for it to be ready */
> +       timeout = 1000;
> +       while ((--timeout) && (in_be16(&psc_dma->psc_regs->sr_csr.status) &
> +                                               MPC52xx_PSC_SR_CMDSEND))
> +               udelay(10);

Holy unbounded latency Batman!  This code waits up to 10ms for a register read!

I hate spinning, but if it must be done; I'd like to see it small.
What is the worst case latency? 125us for 8000Hz bus speed?  If you
must spin; can a cpu_relax() be used instead of the udelay() while
watch the timebase?  Timur recently posted a patch which makes this
easier.

http://patchwork.ozlabs.org/patch/27414/

They *should* be appearing in Ben's -next branch soon.

> +
> +       if (!timeout) {
> +               pr_err("timeout on ac97 bus (rdy)\n");
> +               return 0xffff;
> +       }
> +
> +       /* Do the read */
> +       out_be32(&psc_dma->psc_regs->ac97_cmd, (1<<31) | ((reg & 0x7f) << 24));
> +
> +       /* Wait for the answer */
> +       timeout = 1000;
> +       while ((--timeout) && !(in_be16(&psc_dma->psc_regs->sr_csr.status) &
> +                                               MPC52xx_PSC_SR_DATA_VAL))
> +               udelay(10);

ditto.

> +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.
> +        */
> +       for (max_reset = 0; max_reset < 5; max_reset++) {
> +
> +               /* Do a cold reset */
> +               out_8(&regs->op1, MPC52xx_PSC_OP_RES);
> +               udelay(10);
> +               out_8(&regs->op0, MPC52xx_PSC_OP_RES);
> +               udelay(50);

:-/  Don't like, but don't know if there is an alternative.

> +
> +               /* PSC recover from cold reset
> +                * (cfr user manual, not sure if useful)
> +                */
> +               out_be32(&regs->sicr, in_be32(&regs->sicr));
> +
> +               psc_ac97_warm_reset(ac97);
> +
> +               /* first make sure AC97 clock is low */
> +               for (timeout = 0; ((in_8(&regs->ipcr_acr.ipcr) & 0x80) != 0) &&
> +                               (timeout < 100); timeout++)
> +                       udelay(10);
> +               if (timeout == 100)
> +                       continue;
> +
> +               /* then wait for the transition to high */
> +               for (timeout = 0; ((in_8(&regs->ipcr_acr.ipcr) & 0x80) == 0) &&
> +                               (timeout < 100); timeout++)
> +                       udelay(10);
> +               if (timeout == 100)
> +                       continue;

Using udelay makes this less accurate.  Only possible reason to use a
udelay is if the register cannot be polled at full speed (which is
possibly the case if it adds bus contention; but I don't think it is
an issue here).

g.


-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.


More information about the Alsa-devel mailing list