ALSA: emu10k1: move the whole GPIO event handling to the workqueue

The actual event processing was already done by workqueue items. We can
move the event dispatching there as well, rather than doing it already
in the interrupt handler callback.

This change has a rather profound "side effect" on the reliability of
the FPGA programming: once we enter programming mode, we must not issue
any snd_emu1010_fpga_{read,write}() calls until we're done, as these
would badly mess up the programming protocol. But exactly that would
happen when trying to program the dock, as that triggers GPIO interrupts
as a side effect. This is mitigated by deferring the actual interrupt
handling, as workqueue items are not re-entrant.

To avoid scheduling the dispatcher on non-events, we now explicitly
ignore GPIO IRQs triggered by "uninteresting" pins, which happens a lot
as a side effect of calling snd_emu1010_fpga_{read,write}().

Fixes: fbb64eedf5a3 ("ALSA: emu10k1: make E-MU dock monitoring interrupt-driven")
Link: https://bugzilla.kernel.org/show_bug.cgi?id=218584
Signed-off-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Message-ID: <20240428093716.3198666-4-oswald.buddenhagen@gmx.de>

diff --git a/include/sound/emu10k1.h b/include/sound/emu10k1.h
index 1af9e68193920d062f3c9cce40eba41dc8d74292..9cc10fab01a8ce25cea726a41bf6f4e4a6c222b0 100644 (file)
--- a/include/sound/emu10k1.h
+++ b/include/sound/emu10k1.h
@@ -1684,8 +1684,7 @@ struct snd_emu1010 {
        unsigned int clock_fallback;
        unsigned int optical_in; /* 0:SPDIF, 1:ADAT */
        unsigned int optical_out; /* 0:SPDIF, 1:ADAT */
-       struct work_struct firmware_work;
-       struct work_struct clock_work;
+       struct work_struct work;
 };
 
 struct snd_emu10k1 {

diff --git a/sound/pci/emu10k1/emu10k1.c b/sound/pci/emu10k1/emu10k1.c
index fe72e7d7724127418b0b5351e64f6dc0d8b2c831..dadeda7758ceebce94d6205d38b0807b1e6c5587 100644 (file)
--- a/sound/pci/emu10k1/emu10k1.c
+++ b/sound/pci/emu10k1/emu10k1.c
@@ -189,8 +189,7 @@ static int snd_emu10k1_suspend(struct device *dev)
 
        emu->suspend = 1;
 
-       cancel_work_sync(&emu->emu1010.firmware_work);
-       cancel_work_sync(&emu->emu1010.clock_work);
+       cancel_work_sync(&emu->emu1010.work);
 
        snd_ac97_suspend(emu->ac97);
 

diff --git a/sound/pci/emu10k1/emu10k1_main.c b/sound/pci/emu10k1/emu10k1_main.c
index 6265fc9ae26060351eff6501593c680e2b546209..86eaf5963502c46bad0c9ac982d14634f4e44b42 100644 (file)
--- a/sound/pci/emu10k1/emu10k1_main.c
+++ b/sound/pci/emu10k1/emu10k1_main.c
@@ -765,19 +765,10 @@ static void snd_emu1010_load_dock_firmware(struct snd_emu10k1 *emu)
        msleep(10);
 }
 
-static void emu1010_firmware_work(struct work_struct *work)
+static void emu1010_dock_event(struct snd_emu10k1 *emu)
 {
-       struct snd_emu10k1 *emu;
        u32 reg;
 
-       emu = container_of(work, struct snd_emu10k1,
-                          emu1010.firmware_work);
-       if (emu->card->shutdown)
-               return;
-#ifdef CONFIG_PM_SLEEP
-       if (emu->suspend)
-               return;
-#endif
        snd_emu1010_fpga_read(emu, EMU_HANA_OPTION_CARDS, &reg); /* OPTIONS: Which cards are attached to the EMU */
        if (reg & EMU_HANA_OPTION_DOCK_OFFLINE) {
                /* Audio Dock attached */
@@ -792,20 +783,10 @@ static void emu1010_firmware_work(struct work_struct *work)
        }
 }
 
-static void emu1010_clock_work(struct work_struct *work)
+static void emu1010_clock_event(struct snd_emu10k1 *emu)
 {
-       struct snd_emu10k1 *emu;
        struct snd_ctl_elem_id id;
 
-       emu = container_of(work, struct snd_emu10k1,
-                          emu1010.clock_work);
-       if (emu->card->shutdown)
-               return;
-#ifdef CONFIG_PM_SLEEP
-       if (emu->suspend)
-               return;
-#endif
-
        spin_lock_irq(&emu->reg_lock);
        // This is the only thing that can actually happen.
        emu->emu1010.clock_source = emu->emu1010.clock_fallback;
@@ -816,19 +797,40 @@ static void emu1010_clock_work(struct work_struct *work)
        snd_ctl_notify(emu->card, SNDRV_CTL_EVENT_MASK_VALUE, &id);
 }
 
-static void emu1010_interrupt(struct snd_emu10k1 *emu)
+static void emu1010_work(struct work_struct *work)
 {
+       struct snd_emu10k1 *emu;
        u32 sts;
 
+       emu = container_of(work, struct snd_emu10k1, emu1010.work);
+       if (emu->card->shutdown)
+               return;
+#ifdef CONFIG_PM_SLEEP
+       if (emu->suspend)
+               return;
+#endif
+
        snd_emu1010_fpga_read(emu, EMU_HANA_IRQ_STATUS, &sts);
 
        // The distinction of the IRQ status bits is unreliable,
        // so we dispatch later based on option card status.
        if (sts & (EMU_HANA_IRQ_DOCK | EMU_HANA_IRQ_DOCK_LOST))
-               schedule_work(&emu->emu1010.firmware_work);
+               emu1010_dock_event(emu);
 
        if (sts & EMU_HANA_IRQ_WCLK_CHANGED)
-               schedule_work(&emu->emu1010.clock_work);
+               emu1010_clock_event(emu);
+}
+
+static void emu1010_interrupt(struct snd_emu10k1 *emu)
+{
+       // We get an interrupt on each GPIO input pin change, but we
+       // care only about the ones triggered by the dedicated pin.
+       u16 sts = inw(emu->port + A_GPIO);
+       u16 bit = emu->card_capabilities->ca0108_chip ? 0x2000 : 0x8000;
+       if (!(sts & bit))
+               return;
+
+       schedule_work(&emu->emu1010.work);
 }
 
 /*
@@ -969,8 +971,7 @@ static void snd_emu10k1_free(struct snd_card *card)
                /* Disable 48Volt power to Audio Dock */
                snd_emu1010_fpga_write(emu, EMU_HANA_DOCK_PWR, 0);
        }
-       cancel_work_sync(&emu->emu1010.firmware_work);
-       cancel_work_sync(&emu->emu1010.clock_work);
+       cancel_work_sync(&emu->emu1010.work);
        release_firmware(emu->firmware);
        release_firmware(emu->dock_fw);
        snd_util_memhdr_free(emu->memhdr);
@@ -1549,8 +1550,7 @@ int snd_emu10k1_create(struct snd_card *card,
        emu->irq = -1;
        emu->synth = NULL;
        emu->get_synth_voice = NULL;
-       INIT_WORK(&emu->emu1010.firmware_work, emu1010_firmware_work);
-       INIT_WORK(&emu->emu1010.clock_work, emu1010_clock_work);
+       INIT_WORK(&emu->emu1010.work, emu1010_work);
        /* read revision & serial */
        emu->revision = pci->revision;
        pci_read_config_dword(pci, PCI_SUBSYSTEM_VENDOR_ID, &emu->serial);