[alsa-devel] [PATCH] alsa/soc: add locking to mpc5200-psc-ac97 driver

[Jon Smirl]

On Tue, Jun 30, 2009 at 2:18 AM, Wolfram Sang<w.sang at pengutronix.de> wrote:

Wolfram, do you have any way to test the on the Phytec hardware? The
WM9712 on the baseboard supports a touchscreen, is it brought out to a
header?


> Hi Grant,
>
> On Mon, Jun 29, 2009 at 05:42:21PM -0600, Grant Likely wrote:
>> From: Grant Likely <grant.likely at secretlab.ca>
>>
>> AC97 bus register read/write hooks need to provide locking, but the
>> mpc5200-psc-ac97 driver does not.  This patch adds a mutex around
>> the register access routines.
>>
>> Signed-off-by: Grant Likely <grant.likely at secretlab.ca>
>> ---
>>
>>  sound/soc/fsl/mpc5200_dma.c      |    1 +
>>  sound/soc/fsl/mpc5200_dma.h      |    1 +
>>  sound/soc/fsl/mpc5200_psc_ac97.c |   17 ++++++++++++++++-
>>  3 files changed, 18 insertions(+), 1 deletions(-)
>>
>>
>> diff --git a/sound/soc/fsl/mpc5200_dma.c b/sound/soc/fsl/mpc5200_dma.c
>> index efec33a..f0a2d40 100644
>> --- a/sound/soc/fsl/mpc5200_dma.c
>> +++ b/sound/soc/fsl/mpc5200_dma.c
>> @@ -456,6 +456,7 @@ int mpc5200_audio_dma_create(struct of_device *op)
>>               return -ENODEV;
>>
>>       spin_lock_init(&psc_dma->lock);
>> +     mutex_init(&psc_dma->mutex);
>>       psc_dma->id = be32_to_cpu(*prop);
>>       psc_dma->irq = irq;
>>       psc_dma->psc_regs = regs;
>> diff --git a/sound/soc/fsl/mpc5200_dma.h b/sound/soc/fsl/mpc5200_dma.h
>> index 2000803..8d396bb 100644
>> --- a/sound/soc/fsl/mpc5200_dma.h
>> +++ b/sound/soc/fsl/mpc5200_dma.h
>> @@ -55,6 +55,7 @@ struct psc_dma {
>>       unsigned int irq;
>>       struct device *dev;
>>       spinlock_t lock;
>> +     struct mutex mutex;
>>       u32 sicr;
>>       uint sysclk;
>>       int imr;
>> diff --git a/sound/soc/fsl/mpc5200_psc_ac97.c b/sound/soc/fsl/mpc5200_psc_ac97.c
>> index 794a247..7eb5499 100644
>> --- a/sound/soc/fsl/mpc5200_psc_ac97.c
>> +++ b/sound/soc/fsl/mpc5200_psc_ac97.c
>> @@ -34,13 +34,20 @@ static unsigned short psc_ac97_read(struct snd_ac97 *ac97, unsigned short reg)
>>       int status;
>>       unsigned int val;
>>
>> +     mutex_lock(&psc_dma->mutex);
>> +
>>       /* Wait for command send status zero = ready */
>>       status = spin_event_timeout(!(in_be16(&psc_dma->psc_regs->sr_csr.status) &
>>                               MPC52xx_PSC_SR_CMDSEND), 100, 0);
>>       if (status == 0) {
>>               pr_err("timeout on ac97 bus (rdy)\n");
>> +             mutex_unlock(&psc_dma->mutex);
>>               return -ENODEV;
>>       }
>> +
>> +     /* Force clear the data valid bit */
>> +     in_be32(&psc_dma->psc_regs->ac97_data);
>> +
>
> No mutex involved here. I think this is either a seperate patch or it needs at
> least to be mentioned in the patch description.
>
>>       /* Send the read */
>>       out_be32(&psc_dma->psc_regs->ac97_cmd, (1<<31) | ((reg & 0x7f) << 24));
>>
>> @@ -50,16 +57,19 @@ static unsigned short psc_ac97_read(struct snd_ac97 *ac97, unsigned short reg)
>>       if (status == 0) {
>>               pr_err("timeout on ac97 read (val) %x\n",
>>                               in_be16(&psc_dma->psc_regs->sr_csr.status));
>> +             mutex_unlock(&psc_dma->mutex);
>>               return -ENODEV;
>>       }
>>       /* Get the data */
>>       val = in_be32(&psc_dma->psc_regs->ac97_data);
>>       if (((val >> 24) & 0x7f) != reg) {
>>               pr_err("reg echo error on ac97 read\n");
>> +             mutex_unlock(&psc_dma->mutex);
>>               return -ENODEV;
>>       }
>>       val = (val >> 8) & 0xffff;
>>
>> +     mutex_unlock(&psc_dma->mutex);
>>       return (unsigned short) val;
>>  }
>>
>> @@ -68,16 +78,21 @@ static void psc_ac97_write(struct snd_ac97 *ac97,
>>  {
>>       int status;
>>
>> +     mutex_lock(&psc_dma->mutex);
>> +
>>       /* Wait for command status zero = ready */
>>       status = spin_event_timeout(!(in_be16(&psc_dma->psc_regs->sr_csr.status) &
>>                               MPC52xx_PSC_SR_CMDSEND), 100, 0);
>>       if (status == 0) {
>>               pr_err("timeout on ac97 bus (write)\n");
>> -             return;
>> +             goto out;
>>       }
>>       /* Write data */
>>       out_be32(&psc_dma->psc_regs->ac97_cmd,
>>                       ((reg & 0x7f) << 24) | (val << 8));
>> +
>> + out:
>> +     mutex_unlock(&psc_dma->mutex);
>>  }
>>
>>  static void psc_ac97_warm_reset(struct snd_ac97 *ac97)
>>
>> _______________________________________________
>> Linuxppc-dev mailing list
>> Linuxppc-dev at lists.ozlabs.org
>> https://lists.ozlabs.org/listinfo/linuxppc-dev
>
> --
> Pengutronix e.K.                           | Wolfram Sang                |
> Industrial Linux Solutions                 | http://www.pengutronix.de/  |
>
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.9 (GNU/Linux)
>
> iEYEARECAAYFAkpJrkcACgkQD27XaX1/VRsY/QCgyx8IMANjokbNnrOQlXINRLeW
> lT4AnAy3ES9r3wriGkRN7ivnLA3zyRHb
> =RUPr
> -----END PGP SIGNATURE-----
>
>



-- 
Jon Smirl
jonsmirl at gmail.com