Skip to content

Commit 2d3f481

Browse files
ossilatortiwai
ossilator
authored and
tiwai
committed
ALSA: emu10k1: use mutex for E-MU FPGA access locking
The FPGA access through the GPIO port does not interfere with other sound processor register access, so there is no need to subject it to emu_lock. And after moving all FPGA access out of the interrupt handler, it does not need to be IRQ-safe, either. What's more, attaching the dock causes a firmware upload, which takes several seconds. We really don't want to disable IRQs for this long, and even less also have someone else spin with IRQs disabled waiting for us. Therefore, use a mutex for FPGA access locking. This makes the code somewhat more noisy, as we need to wrap bigger sections into the mutex, as it needs to enclose the spinlocks. The latter has the "side effect" of fixing dock FPGA programming in a corner case: a really badly timed mixer access right between entering FPGA programming mode and uploading the netlist would mess up the protocol. Signed-off-by: Oswald Buddenhagen <[email protected]> Signed-off-by: Takashi Iwai <[email protected]> Message-ID: <[email protected]>
1 parent f848337 commit 2d3f481

File tree

5 files changed

+61
-37
lines changed

5 files changed

+61
-37
lines changed

‎include/sound/emu10k1.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1685,6 +1685,7 @@ struct snd_emu1010 {
16851685
unsigned int optical_in; /* 0:SPDIF, 1:ADAT */
16861686
unsigned int optical_out; /* 0:SPDIF, 1:ADAT */
16871687
struct work_struct work;
1688+
struct mutex lock;
16881689
};
16891690

16901691
struct snd_emu10k1 {
@@ -1833,6 +1834,9 @@ unsigned int snd_emu10k1_ptr20_read(struct snd_emu10k1 * emu, unsigned int reg,
18331834
void snd_emu10k1_ptr20_write(struct snd_emu10k1 *emu, unsigned int reg, unsigned int chn, unsigned int data);
18341835
int snd_emu10k1_spi_write(struct snd_emu10k1 * emu, unsigned int data);
18351836
int snd_emu10k1_i2c_write(struct snd_emu10k1 *emu, u32 reg, u32 value);
1837+
static inline void snd_emu1010_fpga_lock(struct snd_emu10k1 *emu) { mutex_lock(&emu->emu1010.lock); };
1838+
static inline void snd_emu1010_fpga_unlock(struct snd_emu10k1 *emu) { mutex_unlock(&emu->emu1010.lock); };
1839+
void snd_emu1010_fpga_write_lock(struct snd_emu10k1 *emu, u32 reg, u32 value);
18361840
void snd_emu1010_fpga_write(struct snd_emu10k1 *emu, u32 reg, u32 value);
18371841
void snd_emu1010_fpga_read(struct snd_emu10k1 *emu, u32 reg, u32 *value);
18381842
void snd_emu1010_fpga_link_dst_src_write(struct snd_emu10k1 *emu, u32 dst, u32 src);

‎sound/pci/emu10k1/emu10k1_main.c

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -810,6 +810,8 @@ static void emu1010_work(struct work_struct *work)
810810
return;
811811
#endif
812812

813+
snd_emu1010_fpga_lock(emu);
814+
813815
snd_emu1010_fpga_read(emu, EMU_HANA_IRQ_STATUS, &sts);
814816

815817
// The distinction of the IRQ status bits is unreliable,
@@ -819,6 +821,8 @@ static void emu1010_work(struct work_struct *work)
819821

820822
if (sts & EMU_HANA_IRQ_WCLK_CHANGED)
821823
emu1010_clock_event(emu);
824+
825+
snd_emu1010_fpga_unlock(emu);
822826
}
823827

824828
static void emu1010_interrupt(struct snd_emu10k1 *emu)
@@ -852,6 +856,8 @@ static int snd_emu10k1_emu1010_init(struct snd_emu10k1 *emu)
852856
* Proper init follows in snd_emu10k1_init(). */
853857
outl(HCFG_LOCKSOUNDCACHE | HCFG_LOCKTANKCACHE_MASK, emu->port + HCFG);
854858

859+
snd_emu1010_fpga_lock(emu);
860+
855861
/* Disable 48Volt power to Audio Dock */
856862
snd_emu1010_fpga_write(emu, EMU_HANA_DOCK_PWR, 0);
857863

@@ -877,7 +883,7 @@ static int snd_emu10k1_emu1010_init(struct snd_emu10k1 *emu)
877883
err = snd_emu1010_load_firmware(emu, 0, &emu->firmware);
878884
if (err < 0) {
879885
dev_info(emu->card->dev, "emu1010: Loading Firmware failed\n");
880-
return err;
886+
goto fail;
881887
}
882888

883889
/* ID, should read & 0x7f = 0x55 when FPGA programmed. */
@@ -887,7 +893,8 @@ static int snd_emu10k1_emu1010_init(struct snd_emu10k1 *emu)
887893
dev_info(emu->card->dev,
888894
"emu1010: Loading Hana Firmware file failed, reg = 0x%x\n",
889895
reg);
890-
return -ENODEV;
896+
err = -ENODEV;
897+
goto fail;
891898
}
892899

893900
dev_info(emu->card->dev, "emu1010: Hana Firmware loaded\n");
@@ -947,7 +954,9 @@ static int snd_emu10k1_emu1010_init(struct snd_emu10k1 *emu)
947954
// so it is safe to simply enable the outputs.
948955
snd_emu1010_fpga_write(emu, EMU_HANA_UNMUTE, EMU_UNMUTE);
949956

950-
return 0;
957+
fail:
958+
snd_emu1010_fpga_unlock(emu);
959+
return err;
951960
}
952961
/*
953962
* Create the EMU10K1 instance
@@ -969,9 +978,10 @@ static void snd_emu10k1_free(struct snd_card *card)
969978
}
970979
if (emu->card_capabilities->emu_model == EMU_MODEL_EMU1010) {
971980
/* Disable 48Volt power to Audio Dock */
972-
snd_emu1010_fpga_write(emu, EMU_HANA_DOCK_PWR, 0);
981+
snd_emu1010_fpga_write_lock(emu, EMU_HANA_DOCK_PWR, 0);
973982
}
974983
cancel_work_sync(&emu->emu1010.work);
984+
mutex_destroy(&emu->emu1010.lock);
975985
release_firmware(emu->firmware);
976986
release_firmware(emu->dock_fw);
977987
snd_util_memhdr_free(emu->memhdr);
@@ -1551,6 +1561,7 @@ int snd_emu10k1_create(struct snd_card *card,
15511561
emu->synth = NULL;
15521562
emu->get_synth_voice = NULL;
15531563
INIT_WORK(&emu->emu1010.work, emu1010_work);
1564+
mutex_init(&emu->emu1010.lock);
15541565
/* read revision & serial */
15551566
emu->revision = pci->revision;
15561567
pci_read_config_dword(pci, PCI_SUBSYSTEM_VENDOR_ID, &emu->serial);

‎sound/pci/emu10k1/emumixer.c

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -661,7 +661,9 @@ static int snd_emu1010_output_source_put(struct snd_kcontrol *kcontrol,
661661
change = (emu->emu1010.output_source[channel] != val);
662662
if (change) {
663663
emu->emu1010.output_source[channel] = val;
664+
snd_emu1010_fpga_lock(emu);
664665
snd_emu1010_output_source_apply(emu, channel, val);
666+
snd_emu1010_fpga_unlock(emu);
665667
}
666668
return change;
667669
}
@@ -705,7 +707,9 @@ static int snd_emu1010_input_source_put(struct snd_kcontrol *kcontrol,
705707
change = (emu->emu1010.input_source[channel] != val);
706708
if (change) {
707709
emu->emu1010.input_source[channel] = val;
710+
snd_emu1010_fpga_lock(emu);
708711
snd_emu1010_input_source_apply(emu, channel, val);
712+
snd_emu1010_fpga_unlock(emu);
709713
}
710714
return change;
711715
}
@@ -774,7 +778,7 @@ static int snd_emu1010_adc_pads_put(struct snd_kcontrol *kcontrol, struct snd_ct
774778
cache = cache & ~mask;
775779
change = (cache != emu->emu1010.adc_pads);
776780
if (change) {
777-
snd_emu1010_fpga_write(emu, EMU_HANA_ADC_PADS, cache );
781+
snd_emu1010_fpga_write_lock(emu, EMU_HANA_ADC_PADS, cache );
778782
emu->emu1010.adc_pads = cache;
779783
}
780784

@@ -832,7 +836,7 @@ static int snd_emu1010_dac_pads_put(struct snd_kcontrol *kcontrol, struct snd_ct
832836
cache = cache & ~mask;
833837
change = (cache != emu->emu1010.dac_pads);
834838
if (change) {
835-
snd_emu1010_fpga_write(emu, EMU_HANA_DAC_PADS, cache );
839+
snd_emu1010_fpga_write_lock(emu, EMU_HANA_DAC_PADS, cache );
836840
emu->emu1010.dac_pads = cache;
837841
}
838842

@@ -980,6 +984,7 @@ static int snd_emu1010_clock_source_put(struct snd_kcontrol *kcontrol,
980984
val = ucontrol->value.enumerated.item[0] ;
981985
if (val >= emu_ci->num)
982986
return -EINVAL;
987+
snd_emu1010_fpga_lock(emu);
983988
spin_lock_irq(&emu->reg_lock);
984989
change = (emu->emu1010.clock_source != val);
985990
if (change) {
@@ -996,6 +1001,7 @@ static int snd_emu1010_clock_source_put(struct snd_kcontrol *kcontrol,
9961001
} else {
9971002
spin_unlock_irq(&emu->reg_lock);
9981003
}
1004+
snd_emu1010_fpga_unlock(emu);
9991005
return change;
10001006
}
10011007

@@ -1041,7 +1047,7 @@ static int snd_emu1010_clock_fallback_put(struct snd_kcontrol *kcontrol,
10411047
change = (emu->emu1010.clock_fallback != val);
10421048
if (change) {
10431049
emu->emu1010.clock_fallback = val;
1044-
snd_emu1010_fpga_write(emu, EMU_HANA_DEFCLOCK, 1 - val);
1050+
snd_emu1010_fpga_write_lock(emu, EMU_HANA_DEFCLOCK, 1 - val);
10451051
}
10461052
return change;
10471053
}
@@ -1093,7 +1099,7 @@ static int snd_emu1010_optical_out_put(struct snd_kcontrol *kcontrol,
10931099
emu->emu1010.optical_out = val;
10941100
tmp = (emu->emu1010.optical_in ? EMU_HANA_OPTICAL_IN_ADAT : EMU_HANA_OPTICAL_IN_SPDIF) |
10951101
(emu->emu1010.optical_out ? EMU_HANA_OPTICAL_OUT_ADAT : EMU_HANA_OPTICAL_OUT_SPDIF);
1096-
snd_emu1010_fpga_write(emu, EMU_HANA_OPTICAL_TYPE, tmp);
1102+
snd_emu1010_fpga_write_lock(emu, EMU_HANA_OPTICAL_TYPE, tmp);
10971103
}
10981104
return change;
10991105
}
@@ -1144,7 +1150,7 @@ static int snd_emu1010_optical_in_put(struct snd_kcontrol *kcontrol,
11441150
emu->emu1010.optical_in = val;
11451151
tmp = (emu->emu1010.optical_in ? EMU_HANA_OPTICAL_IN_ADAT : EMU_HANA_OPTICAL_IN_SPDIF) |
11461152
(emu->emu1010.optical_out ? EMU_HANA_OPTICAL_OUT_ADAT : EMU_HANA_OPTICAL_OUT_SPDIF);
1147-
snd_emu1010_fpga_write(emu, EMU_HANA_OPTICAL_TYPE, tmp);
1153+
snd_emu1010_fpga_write_lock(emu, EMU_HANA_OPTICAL_TYPE, tmp);
11481154
}
11491155
return change;
11501156
}
@@ -2323,7 +2329,9 @@ int snd_emu10k1_mixer(struct snd_emu10k1 *emu,
23232329
for (i = 0; i < emu_ri->n_outs; i++)
23242330
emu->emu1010.output_source[i] =
23252331
emu1010_map_source(emu_ri, emu_ri->out_dflts[i]);
2332+
snd_emu1010_fpga_lock(emu);
23262333
snd_emu1010_apply_sources(emu);
2334+
snd_emu1010_fpga_unlock(emu);
23272335

23282336
kctl = emu->ctl_clock_source = snd_ctl_new1(&snd_emu1010_clock_source, emu);
23292337
err = snd_ctl_add(card, kctl);

‎sound/pci/emu10k1/emuproc.c

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -165,6 +165,8 @@ static void snd_emu10k1_proc_spdif_read(struct snd_info_entry *entry,
165165
u32 value2;
166166

167167
if (emu->card_capabilities->emu_model) {
168+
snd_emu1010_fpga_lock(emu);
169+
168170
// This represents the S/PDIF lock status on 0404b, which is
169171
// kinda weird and unhelpful, because monitoring it via IRQ is
170172
// impractical (one gets an IRQ flood as long as it is desynced).
@@ -197,6 +199,8 @@ static void snd_emu10k1_proc_spdif_read(struct snd_info_entry *entry,
197199
snd_iprintf(buffer, "\nS/PDIF mode: %s%s\n",
198200
value & EMU_HANA_SPDIF_MODE_RX_PRO ? "professional" : "consumer",
199201
value & EMU_HANA_SPDIF_MODE_RX_NOCOPY ? ", no copy" : "");
202+
203+
snd_emu1010_fpga_unlock(emu);
200204
} else {
201205
snd_emu10k1_proc_spdif_status(emu, buffer, "CD-ROM S/PDIF In", CDCS, CDSRCS);
202206
snd_emu10k1_proc_spdif_status(emu, buffer, "Optical or Coax S/PDIF In", GPSCS, GPSRCS);
@@ -458,6 +462,9 @@ static void snd_emu_proc_emu1010_reg_read(struct snd_info_entry *entry,
458462
struct snd_emu10k1 *emu = entry->private_data;
459463
u32 value;
460464
int i;
465+
466+
snd_emu1010_fpga_lock(emu);
467+
461468
snd_iprintf(buffer, "EMU1010 Registers:\n\n");
462469

463470
for(i = 0; i < 0x40; i+=1) {
@@ -496,6 +503,8 @@ static void snd_emu_proc_emu1010_reg_read(struct snd_info_entry *entry,
496503
snd_emu_proc_emu1010_link_read(emu, buffer, 0x701);
497504
}
498505
}
506+
507+
snd_emu1010_fpga_unlock(emu);
499508
}
500509

501510
static void snd_emu_proc_io_reg_read(struct snd_info_entry *entry,

‎sound/pci/emu10k1/io.c

Lines changed: 20 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -289,20 +289,28 @@ static void snd_emu1010_fpga_write_locked(struct snd_emu10k1 *emu, u32 reg, u32
289289

290290
void snd_emu1010_fpga_write(struct snd_emu10k1 *emu, u32 reg, u32 value)
291291
{
292-
unsigned long flags;
292+
if (snd_BUG_ON(!mutex_is_locked(&emu->emu1010.lock)))
293+
return;
294+
snd_emu1010_fpga_write_locked(emu, reg, value);
295+
}
293296

294-
spin_lock_irqsave(&emu->emu_lock, flags);
297+
void snd_emu1010_fpga_write_lock(struct snd_emu10k1 *emu, u32 reg, u32 value)
298+
{
299+
snd_emu1010_fpga_lock(emu);
295300
snd_emu1010_fpga_write_locked(emu, reg, value);
296-
spin_unlock_irqrestore(&emu->emu_lock, flags);
301+
snd_emu1010_fpga_unlock(emu);
297302
}
298303

299-
static void snd_emu1010_fpga_read_locked(struct snd_emu10k1 *emu, u32 reg, u32 *value)
304+
void snd_emu1010_fpga_read(struct snd_emu10k1 *emu, u32 reg, u32 *value)
300305
{
301306
// The higest input pin is used as the designated interrupt trigger,
302307
// so it needs to be masked out.
303308
// But note that any other input pin change will also cause an IRQ,
304309
// so using this function often causes an IRQ as a side effect.
305310
u32 mask = emu->card_capabilities->ca0108_chip ? 0x1f : 0x7f;
311+
312+
if (snd_BUG_ON(!mutex_is_locked(&emu->emu1010.lock)))
313+
return;
306314
if (snd_BUG_ON(reg > 0x3f))
307315
return;
308316
reg += 0x40; /* 0x40 upwards are registers. */
@@ -313,47 +321,31 @@ static void snd_emu1010_fpga_read_locked(struct snd_emu10k1 *emu, u32 reg, u32 *
313321
*value = ((inw(emu->port + A_GPIO) >> 8) & mask);
314322
}
315323

316-
void snd_emu1010_fpga_read(struct snd_emu10k1 *emu, u32 reg, u32 *value)
317-
{
318-
unsigned long flags;
319-
320-
spin_lock_irqsave(&emu->emu_lock, flags);
321-
snd_emu1010_fpga_read_locked(emu, reg, value);
322-
spin_unlock_irqrestore(&emu->emu_lock, flags);
323-
}
324-
325324
/* Each Destination has one and only one Source,
326325
* but one Source can feed any number of Destinations simultaneously.
327326
*/
328327
void snd_emu1010_fpga_link_dst_src_write(struct snd_emu10k1 *emu, u32 dst, u32 src)
329328
{
330-
unsigned long flags;
331-
332329
if (snd_BUG_ON(dst & ~0x71f))
333330
return;
334331
if (snd_BUG_ON(src & ~0x71f))
335332
return;
336-
spin_lock_irqsave(&emu->emu_lock, flags);
337-
snd_emu1010_fpga_write_locked(emu, EMU_HANA_DESTHI, dst >> 8);
338-
snd_emu1010_fpga_write_locked(emu, EMU_HANA_DESTLO, dst & 0x1f);
339-
snd_emu1010_fpga_write_locked(emu, EMU_HANA_SRCHI, src >> 8);
340-
snd_emu1010_fpga_write_locked(emu, EMU_HANA_SRCLO, src & 0x1f);
341-
spin_unlock_irqrestore(&emu->emu_lock, flags);
333+
snd_emu1010_fpga_write(emu, EMU_HANA_DESTHI, dst >> 8);
334+
snd_emu1010_fpga_write(emu, EMU_HANA_DESTLO, dst & 0x1f);
335+
snd_emu1010_fpga_write(emu, EMU_HANA_SRCHI, src >> 8);
336+
snd_emu1010_fpga_write(emu, EMU_HANA_SRCLO, src & 0x1f);
342337
}
343338

344339
u32 snd_emu1010_fpga_link_dst_src_read(struct snd_emu10k1 *emu, u32 dst)
345340
{
346-
unsigned long flags;
347341
u32 hi, lo;
348342

349343
if (snd_BUG_ON(dst & ~0x71f))
350344
return 0;
351-
spin_lock_irqsave(&emu->emu_lock, flags);
352-
snd_emu1010_fpga_write_locked(emu, EMU_HANA_DESTHI, dst >> 8);
353-
snd_emu1010_fpga_write_locked(emu, EMU_HANA_DESTLO, dst & 0x1f);
354-
snd_emu1010_fpga_read_locked(emu, EMU_HANA_SRCHI, &hi);
355-
snd_emu1010_fpga_read_locked(emu, EMU_HANA_SRCLO, &lo);
356-
spin_unlock_irqrestore(&emu->emu_lock, flags);
345+
snd_emu1010_fpga_write(emu, EMU_HANA_DESTHI, dst >> 8);
346+
snd_emu1010_fpga_write(emu, EMU_HANA_DESTLO, dst & 0x1f);
347+
snd_emu1010_fpga_read(emu, EMU_HANA_SRCHI, &hi);
348+
snd_emu1010_fpga_read(emu, EMU_HANA_SRCLO, &lo);
357349
return (hi << 8) | lo;
358350
}
359351

0 commit comments

Comments
 (0)