[PATCH 1/4] ALSA: emu10k1: simplify interrupt handler, part 1

[PATCH 1/4] ALSA: emu10k1: simplify interrupt handler, part 1

[Oswald Buddenhagen]

IPR_CHANNELNUMBERMASK cannot be non-zero when IPR_CHANNELLOOP is unset,
so join marking them as handled.
This logically reverts part of commit f453e20d8a0 ("ALSA update
0.9.3a"), which made the inverse change with no explanation.

Signed-off-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
---
 sound/pci/emu10k1/irq.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/sound/pci/emu10k1/irq.c b/sound/pci/emu10k1/irq.c
index dfb44e5e69a7..0cb89bd8c16b 100644
--- a/sound/pci/emu10k1/irq.c
+++ b/sound/pci/emu10k1/irq.c
@@ -79,9 +79,8 @@ irqreturn_t snd_emu10k1_interrupt(int irq, void *dev_id)
 				val >>= 1;
 				pvoice++;
 			}
-			status &= ~IPR_CHANNELLOOP;
+			status &= ~(IPR_CHANNELLOOP | IPR_CHANNELNUMBERMASK);
 		}
-		status &= ~IPR_CHANNELNUMBERMASK;
 		if (status & (IPR_ADCBUFFULL|IPR_ADCBUFHALFFULL)) {
 			if (emu->capture_interrupt)
 				emu->capture_interrupt(emu, status);
-- 
2.40.0.152.g15d061e6df

[PATCH 2/4] ALSA: emu10k1: simplify interrupt handler, part 2

[Oswald Buddenhagen]

Remove weird INTE_* clearing code. The bits were a subset of the
actually handled interrupts, which kind of contradicted the stated
purpose. I suppose it would make sense to complete the set and negate
it, but interrupts being enabled out of the blue is neither something
that happens a lot, nor should it result in just one error message, IMO.

Signed-off-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
---
 sound/pci/emu10k1/irq.c | 18 ------------------
 1 file changed, 18 deletions(-)

diff --git a/sound/pci/emu10k1/irq.c b/sound/pci/emu10k1/irq.c
index 0cb89bd8c16b..312511300053 100644
--- a/sound/pci/emu10k1/irq.c
+++ b/sound/pci/emu10k1/irq.c
@@ -146,26 +146,8 @@ irqreturn_t snd_emu10k1_interrupt(int irq, void *dev_id)
 		}
 
 		if (status) {
-			unsigned int bits;
 			dev_err(emu->card->dev,
 				"unhandled interrupt: 0x%08x\n", status);
-			//make sure any interrupts we don't handle are disabled:
-			bits = INTE_FXDSPENABLE |
-				INTE_PCIERRORENABLE |
-				INTE_VOLINCRENABLE |
-				INTE_VOLDECRENABLE |
-				INTE_MUTEENABLE |
-				INTE_MICBUFENABLE |
-				INTE_ADCBUFENABLE |
-				INTE_EFXBUFENABLE |
-				INTE_GPSPDIFENABLE |
-				INTE_CDSPDIFENABLE |
-				INTE_INTERVALTIMERENB |
-				INTE_MIDITXENABLE |
-				INTE_MIDIRXENABLE;
-			if (emu->audigy)
-				bits |= INTE_A_MIDITXENABLE2 | INTE_A_MIDIRXENABLE2;
-			snd_emu10k1_intr_disable(emu, bits);
 		}
 		outl(orig_status, emu->port + IPR); /* ack all */
 	}
-- 
2.40.0.152.g15d061e6df

[PATCH 3/4] ALSA: emu10k1: simplify interrupt handler, part 3

[Oswald Buddenhagen]

Handle the "timeout" (actually the retry counter) such that it's more
obvious and causes less cost in the normal case.

Signed-off-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
---
 sound/pci/emu10k1/irq.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/sound/pci/emu10k1/irq.c b/sound/pci/emu10k1/irq.c
index 312511300053..7dc803aaa850 100644
--- a/sound/pci/emu10k1/irq.c
+++ b/sound/pci/emu10k1/irq.c
@@ -22,15 +22,18 @@ irqreturn_t snd_emu10k1_interrupt(int irq, void *dev_id)
 	int handled = 0;
 	int timeout = 0;
 
-	while (((status = inl(emu->port + IPR)) != 0) && (timeout < 1000)) {
-		timeout++;
-		orig_status = status;
+	while ((status = inl(emu->port + IPR)) != 0) {
 		handled = 1;
 		if ((status & 0xffffffff) == 0xffffffff) {
 			dev_info(emu->card->dev,
 				 "Suspected sound card removal\n");
 			break;
 		}
+		if (++timeout == 1000) {
+			dev_info(emu->card->dev, "emu10k1 irq routine failure\n");
+			break;
+		}
+		orig_status = status;
 		if (status & IPR_PCIERROR) {
 			dev_err(emu->card->dev, "interrupt: PCI error\n");
 			snd_emu10k1_intr_disable(emu, INTE_PCIERRORENABLE);
@@ -151,8 +154,6 @@ irqreturn_t snd_emu10k1_interrupt(int irq, void *dev_id)
 		}
 		outl(orig_status, emu->port + IPR); /* ack all */
 	}
-	if (timeout == 1000)
-		dev_info(emu->card->dev, "emu10k1 irq routine failure\n");
 
 	return IRQ_RETVAL(handled);
 }
-- 
2.40.0.152.g15d061e6df

[PATCH 4/4] ALSA: emu10k1: fix handling of half-loop interrupts

[Oswald Buddenhagen]

We'd try to iterate the voices twice without resetting the pointer.
This went unnoticed, because the code isn't actually in use.

Amends commit 27ae958cf6 ("emu10k1 driver - add multichannel device
hw:x,3 [2-8/8]").

Signed-off-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
---
 sound/pci/emu10k1/irq.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/sound/pci/emu10k1/irq.c b/sound/pci/emu10k1/irq.c
index 7dc803aaa850..a813ef8c2f8d 100644
--- a/sound/pci/emu10k1/irq.c
+++ b/sound/pci/emu10k1/irq.c
@@ -47,12 +47,13 @@ irqreturn_t snd_emu10k1_interrupt(int irq, void *dev_id)
 			status &= ~(IPR_VOLINCR|IPR_VOLDECR|IPR_MUTE);
 		}
 		if (status & IPR_CHANNELLOOP) {
+			struct snd_emu10k1_voice *pvoice;
 			int voice;
 			int voice_max = status & IPR_CHANNELNUMBERMASK;
 			u32 val;
-			struct snd_emu10k1_voice *pvoice = emu->voices;
 
 			val = snd_emu10k1_ptr_read(emu, CLIPL, 0);
+			pvoice = emu->voices;
 			for (voice = 0; voice <= voice_max; voice++) {
 				if (voice == 0x20)
 					val = snd_emu10k1_ptr_read(emu, CLIPH, 0);
@@ -68,6 +69,7 @@ irqreturn_t snd_emu10k1_interrupt(int irq, void *dev_id)
 				pvoice++;
 			}
 			val = snd_emu10k1_ptr_read(emu, HLIPL, 0);
+			pvoice = emu->voices;
 			for (voice = 0; voice <= voice_max; voice++) {
 				if (voice == 0x20)
 					val = snd_emu10k1_ptr_read(emu, HLIPH, 0);
-- 
2.40.0.152.g15d061e6df

Re: [PATCH 1/4] ALSA: emu10k1: simplify interrupt handler, part 1

[Takashi Iwai]

On Thu, 18 May 2023 11:30:44 +0200,
Oswald Buddenhagen wrote:
> 
> IPR_CHANNELNUMBERMASK cannot be non-zero when IPR_CHANNELLOOP is unset,
> so join marking them as handled.
> This logically reverts part of commit f453e20d8a0 ("ALSA update
> 0.9.3a"), which made the inverse change with no explanation.
> 
> Signed-off-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>

Applied all four patches now.  Thanks.


Takashi