2009-03-24 16:27:30

by Luotao Fu

Subject: [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 <[email protected]>
---
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

2009-03-24 22:19:19

by Russell King - ARM Linux

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

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.

2009-03-25 10:34:27

by Mark Brown

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

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.

2009-03-26 12:18:22

by Luotao Fu

Subject: [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. 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 <[email protected]>
---
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

2009-03-26 14:28:18

by Mark Brown

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

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

Applied, thanks!