[alsa-devel] [RESEND] [PATCH] pxa2xx-ac97: fix displaying GSR after reset timeout
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; }
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.
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.
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; }
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!
participants (3)
-
Luotao Fu -
Mark Brown -
Russell King - ARM Linux