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

[Grant Likely <grant.likely@secretlab.ca>]

On Thu, Jul 2, 2009 at 1:08 PM, michael<michael@evidence.eu.com> wrote:
> Hi,
>
> Grant Likely wrote:
>>
>> From: Grant Likely <grant.likely@secretlab.ca>
>>
>> AC97 bus register read/write hooks need to provide locking, but the
>> mpc5200-psc-ac97 driver does not. =A0This patch adds a mutex around
>> the register access routines.
>>
>> Signed-off-by: Grant Likely <grant.likely@secretlab.ca>
>> ---
>>
>> =A0sound/soc/fsl/mpc5200_dma.c =A0 =A0 =A0| =A0 =A01 +
>> =A0sound/soc/fsl/mpc5200_dma.h =A0 =A0 =A0| =A0 =A01 +
>> =A0sound/soc/fsl/mpc5200_psc_ac97.c | =A0 13 ++++++++++++-
>> =A03 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)
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0return -ENODEV;
>> =A0 =A0 =A0 =A0 =A0spin_lock_init(&psc_dma->lock);
>> + =A0 =A0 =A0 mutex_init(&psc_dma->mutex);
>> =A0 =A0 =A0 =A0psc_dma->id =3D be32_to_cpu(*prop);
>> =A0 =A0 =A0 =A0psc_dma->irq =3D irq;
>> =A0 =A0 =A0 =A0psc_dma->psc_regs =3D 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 {
>> =A0 =A0 =A0 =A0unsigned int irq;
>> =A0 =A0 =A0 =A0struct device *dev;
>> =A0 =A0 =A0 =A0spinlock_t lock;
>> + =A0 =A0 =A0 struct mutex mutex;
>> =A0 =A0 =A0 =A0u32 sicr;
>> =A0 =A0 =A0 =A0uint sysclk;
>> =A0 =A0 =A0 =A0int 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)
>> =A0 =A0 =A0 =A0int status;
>> =A0 =A0 =A0 =A0unsigned int val;
>> =A0+ =A0 =A0 =A0 mutex_lock(&psc_dma->mutex);
>> +
>> =A0 =A0 =A0 =A0/* Wait for command send status zero =3D ready */
>> =A0 =A0 =A0 =A0status =3D
>> spin_event_timeout(!(in_be16(&psc_dma->psc_regs->sr_csr.status) &
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0MPC52xx_P=
SC_SR_CMDSEND), 100, 0);
>> =A0 =A0 =A0 =A0if (status =3D=3D 0) {
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0pr_err("timeout on ac97 bus (rdy)\n");
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 mutex_unlock(&psc_dma->mutex);
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0return -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.

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