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

[Grant Likely]

On Thu, Jul 2, 2009 at 1:08 PM, michael<michael at evidence.eu.com> wrote:
> Hi,
>
> 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 |   13 ++++++++++++-
>>  3 files changed, 14 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 9b8503f..7eb5499 100644
>> --- a/sound/soc/fsl/mpc5200_psc_ac97.c
>> +++ b/sound/soc/fsl/mpc5200_psc_ac97.c
>> @@ -34,11 +34,14 @@ 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;
>>
>
> maybe define an err variable and and a goto out.

My first iteration did that, but after looking at it I decided I like
this better and it adds (slightly) fewer lines of code.

g.

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