[1/3] ALSA: pxa: slightly refactor reset handling

Commit Message

Dmitry Baryshkov Oct. 17, 2013, 10:01 a.m. UTC

PXA25x also shows some problems when using interrupts during reset
handling. Thus do not use interrupts on all pxa kinds (to detect codec
ready state). Instead use a common mdelay-loop on all platforms to
detect codecs becoming ready.

Signed-off-by: Dmitry Eremin-Solenikov <[e-mail]>
---
 sound/arm/pxa2xx-ac97-lib.c | 27 ++++++++++-----------------
 1 file changed, 10 insertions(+), 17 deletions(-)

Comments

Mark Brown Oct. 17, 2013, 11:36 p.m. UTC | #1

On Thu, Oct 17, 2013 at 02:01:35PM +0400, Dmitry Eremin-Solenikov wrote:
> PXA25x also shows some problems when using interrupts during reset
> handling. Thus do not use interrupts on all pxa kinds (to detect codec
> ready state). Instead use a common mdelay-loop on all platforms to
> detect codecs becoming ready.

Applied all, thanks.  I'm somewhat surprised nobody noticed the reset
issues on the pxa25x before - this stuff all got really heavily tested
back in the day.

Dmitry Baryshkov Oct. 17, 2013, 11:53 p.m. UTC | #2

Hello,

On Fri, Oct 18, 2013 at 3:36 AM, Mark Brown <[e-mail]> wrote:
> On Thu, Oct 17, 2013 at 02:01:35PM +0400, Dmitry Eremin-Solenikov wrote:
>> PXA25x also shows some problems when using interrupts during reset
>> handling. Thus do not use interrupts on all pxa kinds (to detect codec
>> ready state). Instead use a common mdelay-loop on all platforms to
>> detect codecs becoming ready.
>
> Applied all, thanks.  I'm somewhat surprised nobody noticed the reset
> issues on the pxa25x before - this stuff all got really heavily tested
> back in the day.

Maybe it was some coincidence/whatever. Maybe it is some hardware issue
of exact mask version. When I first started hacking around PXA, it was known
that tosa sometimes does not reset the AC97 codec for the first time.
However it looks like nobody bothered to debug that, because in most (~3/4)
cases reset went through. Now some kernel changes made kernel hang
looping in the interrupt, if the reset is not properly handled.

Mark Brown Oct. 18, 2013, 12:19 a.m. UTC | #3

On Fri, Oct 18, 2013 at 03:53:31AM +0400, Dmitry Eremin-Solenikov wrote:

> Maybe it was some coincidence/whatever. Maybe it is some hardware issue
> of exact mask version. When I first started hacking around PXA, it was known
> that tosa sometimes does not reset the AC97 codec for the first time.
> However it looks like nobody bothered to debug that, because in most (~3/4)
> cases reset went through. Now some kernel changes made kernel hang
> looping in the interrupt, if the reset is not properly handled.

Ah, now you mention that extra detail I do recall Wolfson CODEC drivers
with suspicious extra resets - probably this issue was why.

Patch

diff --git a/sound/arm/pxa2xx-ac97-lib.c b/sound/arm/pxa2xx-ac97-lib.c
index e6f4633..99a4668 100644
--- a/sound/arm/pxa2xx-ac97-lib.c
+++ b/sound/arm/pxa2xx-ac97-lib.c
@@ -117,8 +117,7 @@  static inline void pxa_ac97_warm_pxa25x(void)
 {
 	gsr_bits = 0;
 
-	GCR |= GCR_WARM_RST | GCR_PRIRDY_IEN | GCR_SECRDY_IEN;
-	wait_event_timeout(gsr_wq, gsr_bits & (GSR_PCR | GSR_SCR), 1);
+	GCR |= GCR_WARM_RST;
 }
 
 static inline void pxa_ac97_cold_pxa25x(void)
@@ -129,8 +128,6 @@  static inline void pxa_ac97_cold_pxa25x(void)
 	gsr_bits = 0;
 
 	GCR = GCR_COLD_RST;
-	GCR |= GCR_CDONE_IE|GCR_SDONE_IE;
-	wait_event_timeout(gsr_wq, gsr_bits & (GSR_PCR | GSR_SCR), 1);
 }
 #endif
 
@@ -149,8 +146,6 @@  static inline void pxa_ac97_warm_pxa27x(void)
 
 static inline void pxa_ac97_cold_pxa27x(void)
 {
-	unsigned int timeout;
-
 	GCR &=  GCR_COLD_RST;  /* clear everything but nCRST */
 	GCR &= ~GCR_COLD_RST;  /* then assert nCRST */
 
@@ -161,29 +156,20 @@  static inline void pxa_ac97_cold_pxa27x(void)
 	udelay(5);
 	clk_disable(ac97conf_clk);
 	GCR = GCR_COLD_RST | GCR_WARM_RST;
-	timeout = 100;     /* wait for the codec-ready bit to be set */
-	while (!((GSR | gsr_bits) & (GSR_PCR | GSR_SCR)) && timeout--)
-		mdelay(1);
 }
 #endif
 
 #ifdef CONFIG_PXA3xx
 static inline void pxa_ac97_warm_pxa3xx(void)
 {
-	int timeout = 100;
-
 	gsr_bits = 0;
 
 	/* Can't use interrupts */
 	GCR |= GCR_WARM_RST;
-	while (!((GSR | gsr_bits) & (GSR_PCR | GSR_SCR)) && timeout--)
-		mdelay(1);
 }
 
 static inline void pxa_ac97_cold_pxa3xx(void)
 {
-	int timeout = 1000;
-
 	/* Hold CLKBPB for 100us */
 	GCR = 0;
 	GCR = GCR_CLKBPB;
@@ -199,14 +185,13 @@  static inline void pxa_ac97_cold_pxa3xx(void)
 	GCR &= ~(GCR_PRIRDY_IEN|GCR_SECRDY_IEN);
 
 	GCR = GCR_WARM_RST | GCR_COLD_RST;
-	while (!(GSR & (GSR_PCR | GSR_SCR)) && timeout--)
-		mdelay(10);
 }
 #endif
 
 bool pxa2xx_ac97_try_warm_reset(struct snd_ac97 *ac97)
 {
 	unsigned long gsr;
+	unsigned int timeout = 100;
 
 #ifdef CONFIG_PXA25x
 	if (cpu_is_pxa25x())
@@ -224,6 +209,10 @@  bool pxa2xx_ac97_try_warm_reset(struct snd_ac97 *ac97)
 	else
 #endif
 		BUG();
+
+	while (!((GSR | gsr_bits) & (GSR_PCR | GSR_SCR)) && timeout--)
+		mdelay(1);
+
 	gsr = GSR | gsr_bits;
 	if (!(gsr & (GSR_PCR | GSR_SCR))) {
 		printk(KERN_INFO "%s: warm reset timeout (GSR=%#lx)\n",
@@ -239,6 +228,7 @@  EXPORT_SYMBOL_GPL(pxa2xx_ac97_try_warm_reset);
 bool pxa2xx_ac97_try_cold_reset(struct snd_ac97 *ac97)
 {
 	unsigned long gsr;
+	unsigned int timeout = 1000;
 
 #ifdef CONFIG_PXA25x
 	if (cpu_is_pxa25x())
@@ -257,6 +247,9 @@  bool pxa2xx_ac97_try_cold_reset(struct snd_ac97 *ac97)
 #endif
 		BUG();
 
+	while (!((GSR | gsr_bits) & (GSR_PCR | GSR_SCR)) && timeout--)
+		mdelay(1);
+
 	gsr = GSR | gsr_bits;
 	if (!(gsr & (GSR_PCR | GSR_SCR))) {
 		printk(KERN_INFO "%s: cold reset timeout (GSR=%#lx)\n",