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

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

[Grant Likely]

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);
+
 	/* 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)

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

[Jon Smirl]

On Mon, Jun 29, 2009 at 7:42 PM, Grant Likely<grant.likely@secretlab.ca> wr=
ote:
> 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.

Does your touchscreen driver use this mutex? Or was this mutex needed
just for the AC97 driver?


--=20
Jon Smirl
jonsmirl@gmail.com

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

[Wolfram Sang]

[-- 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 --]

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

[Mark Brown]

On Mon, Jun 29, 2009 at 08:26:12PM -0400, Jon Smirl wrote:

> Does your touchscreen driver use this mutex? Or was this mutex needed
> just for the AC97 driver?

It's in the register accesses so everything accessing the chip registers
will use it.

Re: [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@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@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 17 ++++++++++++++++-
>> =A03 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)
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 return -ENODEV;
>>
>> =A0 =A0 =A0 spin_lock_init(&psc_dma->lock);
>> + =A0 =A0 mutex_init(&psc_dma->mutex);
>> =A0 =A0 =A0 psc_dma->id =3D be32_to_cpu(*prop);
>> =A0 =A0 =A0 psc_dma->irq =3D irq;
>> =A0 =A0 =A0 psc_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 unsigned int irq;
>> =A0 =A0 =A0 struct device *dev;
>> =A0 =A0 =A0 spinlock_t lock;
>> + =A0 =A0 struct mutex mutex;
>> =A0 =A0 =A0 u32 sicr;
>> =A0 =A0 =A0 uint sysclk;
>> =A0 =A0 =A0 int imr;
>> diff --git a/sound/soc/fsl/mpc5200_psc_ac97.c b/sound/soc/fsl/mpc5200_ps=
c_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)
>> =A0 =A0 =A0 int status;
>> =A0 =A0 =A0 unsigned int val;
>>
>> + =A0 =A0 mutex_lock(&psc_dma->mutex);
>> +
>> =A0 =A0 =A0 /* Wait for command send status zero =3D ready */
>> =A0 =A0 =A0 status =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 MPC52xx_PSC_=
SR_CMDSEND), 100, 0);
>> =A0 =A0 =A0 if (status =3D=3D 0) {
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 pr_err("timeout on ac97 bus (rdy)\n");
>> + =A0 =A0 =A0 =A0 =A0 =A0 mutex_unlock(&psc_dma->mutex);
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 return -ENODEV;
>> =A0 =A0 =A0 }
>> +
>> + =A0 =A0 /* Force clear the data valid bit */
>> + =A0 =A0 in_be32(&psc_dma->psc_regs->ac97_data);
>> +
>
> No mutex involved here. I think this is either a seperate patch or it nee=
ds at
> least to be mentioned in the patch description.
>
>> =A0 =A0 =A0 /* Send the read */
>> =A0 =A0 =A0 out_be32(&psc_dma->psc_regs->ac97_cmd, (1<<31) | ((reg & 0x7=
f) << 24));
>>
>> @@ -50,16 +57,19 @@ static unsigned short psc_ac97_read(struct snd_ac97 =
*ac97, unsigned short reg)
>> =A0 =A0 =A0 if (status =3D=3D 0) {
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 pr_err("timeout on ac97 read (val) %x\n",
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 in_be16(&psc=
_dma->psc_regs->sr_csr.status));
>> + =A0 =A0 =A0 =A0 =A0 =A0 mutex_unlock(&psc_dma->mutex);
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 return -ENODEV;
>> =A0 =A0 =A0 }
>> =A0 =A0 =A0 /* Get the data */
>> =A0 =A0 =A0 val =3D in_be32(&psc_dma->psc_regs->ac97_data);
>> =A0 =A0 =A0 if (((val >> 24) & 0x7f) !=3D reg) {
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 pr_err("reg echo error on ac97 read\n");
>> + =A0 =A0 =A0 =A0 =A0 =A0 mutex_unlock(&psc_dma->mutex);
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 return -ENODEV;
>> =A0 =A0 =A0 }
>> =A0 =A0 =A0 val =3D (val >> 8) & 0xffff;
>>
>> + =A0 =A0 mutex_unlock(&psc_dma->mutex);
>> =A0 =A0 =A0 return (unsigned short) val;
>> =A0}
>>
>> @@ -68,16 +78,21 @@ static void psc_ac97_write(struct snd_ac97 *ac97,
>> =A0{
>> =A0 =A0 =A0 int status;
>>
>> + =A0 =A0 mutex_lock(&psc_dma->mutex);
>> +
>> =A0 =A0 =A0 /* Wait for command status zero =3D ready */
>> =A0 =A0 =A0 status =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 MPC52xx_PSC_=
SR_CMDSEND), 100, 0);
>> =A0 =A0 =A0 if (status =3D=3D 0) {
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 pr_err("timeout on ac97 bus (write)\n");
>> - =A0 =A0 =A0 =A0 =A0 =A0 return;
>> + =A0 =A0 =A0 =A0 =A0 =A0 goto out;
>> =A0 =A0 =A0 }
>> =A0 =A0 =A0 /* Write data */
>> =A0 =A0 =A0 out_be32(&psc_dma->psc_regs->ac97_cmd,
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 ((reg & 0x7f) << 24) | (val =
<< 8));
>> +
>> + out:
>> + =A0 =A0 mutex_unlock(&psc_dma->mutex);
>> =A0}
>>
>> =A0static 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. =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 | Wo=
lfram Sang =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0|
> Industrial Linux Solutions =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 | http://www.p=
engutronix.de/ =A0|
>
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.9 (GNU/Linux)
>
> iEYEARECAAYFAkpJrkcACgkQD27XaX1/VRsY/QCgyx8IMANjokbNnrOQlXINRLeW
> lT4AnAy3ES9r3wriGkRN7ivnLA3zyRHb
> =3DRUPr
> -----END PGP SIGNATURE-----
>
>



--=20
Jon Smirl
jonsmirl@gmail.com

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

[Mark Brown]

On Tue, Jun 30, 2009 at 09:42:06AM -0400, Jon Smirl 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?

You can probably test adequately by writing a dummy AC97 driver that
sits in a thread and does reads (which should be all that's required to
test the interaction).

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

[Grant Likely]

On Tue, Jun 30, 2009 at 12:18 AM, Wolfram Sang<w.sang@pengutronix.de> wrote=
:
>> +
>> + =A0 =A0 /* Force clear the data valid bit */
>> + =A0 =A0 in_be32(&psc_dma->psc_regs->ac97_data);
>> +
>
> No mutex involved here. I think this is either a seperate patch or it nee=
ds at
> least to be mentioned in the patch description.

Oops, that was sloppy.  Yes, I'll put this into a separate patch.  Thanks.

g.

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

[Grant Likely]

On Tue, Jun 30, 2009 at 2:59 AM, Mark
Brown<broonie@opensource.wolfsonmicro.com> wrote:
> On Mon, Jun 29, 2009 at 08:26:12PM -0400, Jon Smirl wrote:
>
>> Does your touchscreen driver use this mutex? Or was this mutex needed
>> just for the AC97 driver?
>
> It's in the register accesses so everything accessing the chip registers
> will use it.

The mutexes are needed.  The ac97 bus doesn't have any knowledge about
what codec(s) will be wired up to it so it must protect against
multiple access.  In my case both the wm9712 audio codec driver and
the wm9712 touchscreen driver perform register accesses.

g.

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

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

[Jon Smirl]

On Tue, Jun 30, 2009 at 12:50 PM, Grant Likely<grant.likely@secretlab.ca> w=
rote:
> On Tue, Jun 30, 2009 at 12:18 AM, Wolfram Sang<w.sang@pengutronix.de> wro=
te:
>>> +
>>> + =A0 =A0 /* Force clear the data valid bit */
>>> + =A0 =A0 in_be32(&psc_dma->psc_regs->ac97_data);
>>> +
>>
>> No mutex involved here. I think this is either a separate patch or it ne=
eds at
>> least to be mentioned in the patch description.
>
> Oops, that was sloppy. =A0Yes, I'll put this into a separate patch. =A0Th=
anks.

Now that you have added the mutexes, do you ever need to force clear
the valid bit?
Maybe log an error if this happens so that we can track down why.

--=20
Jon Smirl
jonsmirl@gmail.com

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

[Grant Likely]

On Tue, Jun 30, 2009 at 12:33 PM, Jon Smirl<jonsmirl@gmail.com> wrote:
> On Tue, Jun 30, 2009 at 12:50 PM, Grant Likely<grant.likely@secretlab.ca>=
 wrote:
>> On Tue, Jun 30, 2009 at 12:18 AM, Wolfram Sang<w.sang@pengutronix.de> wr=
ote:
>>>> +
>>>> + =A0 =A0 /* Force clear the data valid bit */
>>>> + =A0 =A0 in_be32(&psc_dma->psc_regs->ac97_data);
>>>> +
>>>
>>> No mutex involved here. I think this is either a separate patch or it n=
eeds at
>>> least to be mentioned in the patch description.
>>
>> Oops, that was sloppy. =A0Yes, I'll put this into a separate patch. =A0T=
hanks.
>
> Now that you have added the mutexes, do you ever need to force clear
> the valid bit?
> Maybe log an error if this happens so that we can track down why.

I know exactly why it happened.  spin_event_timeout() had a bug that
would cause it to report a false positive timeout.  The bug is fixed
and queued for merging.

However, I still think this explicit clear should be applied.  It is
just a cheap register read and it prevents the driver from getting
wedged after a timeout does event, for whatever reason.

g.

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

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

[Wolfram Sang]

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

Hi Jon,

> 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?

Sorry, I didn't get it: Shall I test something specific?

The touchscreen connector is X19 BTW (assuming a PCM-973 baseboard).

Regards,

   Wolfram

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

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

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

[Jon Smirl]

On Wed, Jul 1, 2009 at 4:56 AM, Wolfram Sang<w.sang@pengutronix.de> wrote:
> Hi Jon,
>
>> 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?
>
> Sorry, I didn't get it: Shall I test something specific?

I don't own a touchscreen.

AFAIK no one has ever plugged a touchscreen into the PCM-973 to see if
works since there hasn't been a driver previously. Just do a general
test so that you can tell customers that it works.

>
> The touchscreen connector is X19 BTW (assuming a PCM-973 baseboard).
>
> Regards,
>
> =A0 Wolfram
>
> --
> Pengutronix e.K. =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 | Wo=
lfram Sang =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0|
> Industrial Linux Solutions =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 | http://www.p=
engutronix.de/ =A0|
>
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.9 (GNU/Linux)
>
> iEYEARECAAYFAkpLJMEACgkQD27XaX1/VRs/cwCgn7p7ffxr8WGoW7MwALkBMKtD
> VtYAoIa5/viinyvFF5cQP3BvH9WP3a5I
> =3DZrq3
> -----END PGP SIGNATURE-----
>
>



--=20
Jon Smirl
jonsmirl@gmail.com

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

[Wolfram Sang]

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


> > Sorry, I didn't get it: Shall I test something specific?
> 
> I don't own a touchscreen.
> 
> AFAIK no one has ever plugged a touchscreen into the PCM-973 to see if
> works since there hasn't been a driver previously. Just do a general
> test so that you can tell customers that it works.

I am not sure, we have a suitable touchscreen. Also, I am afraid this is a bit
too much to do besides my regular day work right now. Sorry.

Regards,

   Wolfram

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

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

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

[Grant Likely]

On Wed, Jul 1, 2009 at 7:55 AM, Wolfram Sang<w.sang@pengutronix.de> wrote:
>
>> > Sorry, I didn't get it: Shall I test something specific?
>>
>> I don't own a touchscreen.
>>
>> AFAIK no one has ever plugged a touchscreen into the PCM-973 to see if
>> works since there hasn't been a driver previously.
>> Just do a general
>> test so that you can tell customers that it works.
>
> I am not sure, we have a suitable touchscreen. Also, I am afraid this is a bit
> too much to do besides my regular day work right now. Sorry.

I've now tested this on a client's board which uses the pcm030 with a
custom base board (based on the development board) which uses the same
codec.  It works there.  I don't have a discrete touchscreen to wire
up to the PCM-973 though.

g.

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

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

[Eric Millbrandt]

-----Original Message-----
From: =
linuxppc-dev-bounces+emillbrandt=3Ddekaresearch.com@lists.ozlabs.org
[mailto:linuxppc-dev-bounces+emillbrandt=3Ddekaresearch.com@lists.ozlabs.=
o
rg] On Behalf Of Grant Likely
Sent: Wednesday, July 01, 2009 10:45
To: Wolfram Sang
Cc: linuxppc-dev@ozlabs.org; alsa-devel@alsa-project.org;
broonie@sirena.org.uk
Subject: Re: [PATCH] alsa/soc: add locking to mpc5200-psc-ac97 driver

On Wed, Jul 1, 2009 at 7:55 AM, Wolfram Sang<w.sang@pengutronix.de>
wrote:
>
>> > Sorry, I didn't get it: Shall I test something specific?
>>
>> I don't own a touchscreen.
>>
>> AFAIK no one has ever plugged a touchscreen into the PCM-973 to see
if
>> works since there hasn't been a driver previously.
>> Just do a general
>> test so that you can tell customers that it works.
>
> I am not sure, we have a suitable touchscreen. Also, I am afraid this
is a bit
> too much to do besides my regular day work right now. Sorry.

I've now tested this on a client's board which uses the pcm030 with a
custom base board (based on the development board) which uses the same
codec.  It works there.  I don't have a discrete touchscreen to wire
up to the PCM-973 though.

g.

--=20

There is a header on the phytec PCM-973 base board (X20) that brings out
aux1, aux2, aux3, aux4 (wiper) ADCs from the wm9712.  The ADCs use chip
registers the same way the touchscreen does.  There is even support for
doing this through the wm97xx-core driver (wm97xx_read_aux_adc).  If you
wanted to test it out I would put a pot or some fixed voltage divider on
one of those inputs and give it a try.  I don't know what the internal
ADC ref is, I think its 1.65V so whatever signal you put in should be
less than that.

Eric


_________________________________________________________________________=
________________

This e-mail and the information, including any attachments, it contains =
are intended to be a confidential communication only to the person or =
entity to whom it is addressed and may contain information that is =
privileged. If the reader of this message is not the intended recipient, =
you are hereby notified that any dissemination, distribution or copying =
of this communication is strictly prohibited. If you have received this =
communication in error, please immediately notify the sender and destroy =
the original message.

Thank you.

Please consider the environment before printing this email.