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

[Wolfram Sang <w.sang@pengutronix.de>]

[-- Attachment #1: Type: text/plain, Size: 4049 bytes --]

Hi Grant,

On Mon, Jun 29, 2009 at 05:42:21PM -0600, 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.  This patch adds a mutex around
> the register access routines.
> 
> Signed-off-by: Grant Likely <grant.likely@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@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 197 bytes --]