[RESEND] [PATCH] pxa2xx-ac97: fix displaying GSR after reset timeout

[RESEND] [PATCH] pxa2xx-ac97: fix displaying GSR after reset timeout

[Luotao Fu]

the variable gsr_bit is set in isr. It is however set to 0 and interrupts are
disabled prior to reset on pxa27x and pxa3xx. Hence it doesn't make a lot of
sense to show the content of gsr_bit in case of a reset timeout.

Signed-off-by: Luotao Fu <l.fu@pengutronix.de>
---
 sound/arm/pxa2xx-ac97-lib.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/sound/arm/pxa2xx-ac97-lib.c b/sound/arm/pxa2xx-ac97-lib.c
index 35afd0c..25933ba 100644
--- a/sound/arm/pxa2xx-ac97-lib.c
+++ b/sound/arm/pxa2xx-ac97-lib.c
@@ -218,7 +218,7 @@ bool pxa2xx_ac97_try_warm_reset(struct snd_ac97 *ac97)
 
 	if (!((GSR | gsr_bits) & (GSR_PCR | GSR_SCR))) {
 		printk(KERN_INFO "%s: warm reset timeout (GSR=%#lx)\n",
-				 __func__, gsr_bits);
+				 __func__, GSR | gsr_bits);
 
 		return false;
 	}
@@ -248,7 +248,7 @@ bool pxa2xx_ac97_try_cold_reset(struct snd_ac97 *ac97)
 
 	if (!((GSR | gsr_bits) & (GSR_PCR | GSR_SCR))) {
 		printk(KERN_INFO "%s: cold reset timeout (GSR=%#lx)\n",
-				 __func__, gsr_bits);
+				 __func__, GSR | gsr_bits);
 
 		return false;
 	}
-- 
1.5.6.5

Re: [RESEND] [PATCH] pxa2xx-ac97: fix displaying GSR after reset timeout

[Russell King - ARM Linux]

On Tue, Mar 24, 2009 at 05:25:28PM +0100, Luotao Fu wrote:
> the variable gsr_bit is set in isr. It is however set to 0 and interrupts are
> disabled prior to reset on pxa27x and pxa3xx. Hence it doesn't make a lot of
> sense to show the content of gsr_bit in case of a reset timeout.

It would make more sense to put "GSR | gsr_bits" into a temporary variable
for testing, and then printing so that the value printed is the exact
value which was tested, not somehting that may have changed.

Also, you've resent your patch but omitted the ALSA mailing list.

Re: [alsa-devel] [RESEND] [PATCH] pxa2xx-ac97: fix displaying GSR after reset timeout

[Mark Brown]

On Tue, Mar 24, 2009 at 10:18:41PM +0000, Russell King - ARM Linux wrote:

> It would make more sense to put "GSR | gsr_bits" into a temporary variable
> for testing, and then printing so that the value printed is the exact
> value which was tested, not somehting that may have changed.

Indeed - Luotao, please make this change.

> Also, you've resent your patch but omitted the ALSA mailing list.

Looks like it's there on the copies that reached here.

[PATCH] pxa2xx-ac97: fix displaying GSR after reset timeout

[Luotao Fu]

the variable gsr_bit is set in isr. It is however set to 0 and interrupts are
disabled prior to reset. Hence it doesn't make a lot of sense to show the
content of gsr_bit in case of a reset timeout.

Signed-off-by: Luotao Fu <l.fu@pengutronix.de>
---
 sound/arm/pxa2xx-ac97-lib.c |   15 ++++++++++-----
 1 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/sound/arm/pxa2xx-ac97-lib.c b/sound/arm/pxa2xx-ac97-lib.c
index 35afd0c..bbb71ae 100644
--- a/sound/arm/pxa2xx-ac97-lib.c
+++ b/sound/arm/pxa2xx-ac97-lib.c
@@ -199,6 +199,8 @@ static inline void pxa_ac97_cold_pxa3xx(void)
 
 bool pxa2xx_ac97_try_warm_reset(struct snd_ac97 *ac97)
 {
+	unsigned long gsr;
+
 #ifdef CONFIG_PXA25x
 	if (cpu_is_pxa25x())
 		pxa_ac97_warm_pxa25x();
@@ -215,10 +217,10 @@ bool pxa2xx_ac97_try_warm_reset(struct snd_ac97 *ac97)
 	else
 #endif
 		BUG();
-
-	if (!((GSR | gsr_bits) & (GSR_PCR | GSR_SCR))) {
+	gsr = GSR | gsr_bits;
+	if (!(gsr & (GSR_PCR | GSR_SCR))) {
 		printk(KERN_INFO "%s: warm reset timeout (GSR=%#lx)\n",
-				 __func__, gsr_bits);
+				 __func__, gsr);
 
 		return false;
 	}
@@ -229,6 +231,8 @@ EXPORT_SYMBOL_GPL(pxa2xx_ac97_try_warm_reset);
 
 bool pxa2xx_ac97_try_cold_reset(struct snd_ac97 *ac97)
 {
+	unsigned long gsr;
+
 #ifdef CONFIG_PXA25x
 	if (cpu_is_pxa25x())
 		pxa_ac97_cold_pxa25x();
@@ -246,9 +250,10 @@ bool pxa2xx_ac97_try_cold_reset(struct snd_ac97 *ac97)
 #endif
 		BUG();
 
-	if (!((GSR | gsr_bits) & (GSR_PCR | GSR_SCR))) {
+	gsr = GSR | gsr_bits;
+	if (!(gsr & (GSR_PCR | GSR_SCR))) {
 		printk(KERN_INFO "%s: cold reset timeout (GSR=%#lx)\n",
-				 __func__, gsr_bits);
+				 __func__, gsr);
 
 		return false;
 	}
-- 
1.6.2

Re: [PATCH] pxa2xx-ac97: fix displaying GSR after reset timeout

[Mark Brown]

On Thu, Mar 26, 2009 at 01:18:03PM +0100, Luotao Fu wrote:
> the variable gsr_bit is set in isr. It is however set to 0 and interrupts are
> disabled prior to reset. Hence it doesn't make a lot of sense to show the
> content of gsr_bit in case of a reset timeout.

> Signed-off-by: Luotao Fu <l.fu@pengutronix.de>

Applied, thanks!