From 70ee853eec5693fefd8348a2b049d9cb83362e58 Mon Sep 17 00:00:00 2001 From: Richard Fitzgerald Date: Mon, 8 Apr 2024 11:18:00 +0100 Subject: regmap: Add regmap_read_bypassed() Add a regmap_read_bypassed() to allow reads from the hardware registers while the regmap is in cache-only mode. A typical use for this is to keep the cache in cache-only mode until the hardware has reached a valid state, but one or more status registers must be polled to determine when this state is reached. For example, firmware download on the cs35l56 can take several seconds if there are multiple amps sharing limited bus bandwidth. This is too long to block in probe() so it is done as a background task. The device must be soft-reset to reboot the firmware and during this time the registers are not accessible, so the cache should be in cache-only. But the driver must poll a register to detect when reboot has completed. Signed-off-by: Richard Fitzgerald Fixes: 8a731fd37f8b ("ASoC: cs35l56: Move utility functions to shared file") Link: https://msgid.link/r/20240408101803.43183-2-rf@opensource.cirrus.com Signed-off-by: Mark Brown --- include/linux/regmap.h | 8 ++++++++ 1 file changed, 8 insertions(+) (limited to 'include') diff --git a/include/linux/regmap.h b/include/linux/regmap.h index b743241cfb7c..d470303b1bbb 100644 --- a/include/linux/regmap.h +++ b/include/linux/regmap.h @@ -1230,6 +1230,7 @@ int regmap_multi_reg_write_bypassed(struct regmap *map, int regmap_raw_write_async(struct regmap *map, unsigned int reg, const void *val, size_t val_len); int regmap_read(struct regmap *map, unsigned int reg, unsigned int *val); +int regmap_read_bypassed(struct regmap *map, unsigned int reg, unsigned int *val); int regmap_raw_read(struct regmap *map, unsigned int reg, void *val, size_t val_len); int regmap_noinc_read(struct regmap *map, unsigned int reg, @@ -1739,6 +1740,13 @@ static inline int regmap_read(struct regmap *map, unsigned int reg, return -EINVAL; } +static inline int regmap_read_bypassed(struct regmap *map, unsigned int reg, + unsigned int *val) +{ + WARN_ONCE(1, "regmap API is disabled"); + return -EINVAL; +} + static inline int regmap_raw_read(struct regmap *map, unsigned int reg, void *val, size_t val_len) { -- cgit v1.2.3 From dfd2ffb373999630a14d7ff614440f1c2fcc704c Mon Sep 17 00:00:00 2001 From: Richard Fitzgerald Date: Mon, 8 Apr 2024 11:18:03 +0100 Subject: ASoC: cs35l56: Prevent overwriting firmware ASP config Only populate the ASP1 config registers in the regmap cache if the ASP DAI is used. This prevents regcache_sync() from overwriting these registers with their defaults when the firmware owns control of these registers. On a SoundWire system the ASP could be owned by the firmware to share reference audio with the firmware on other cs35l56. Or it can be used as a normal codec-codec interface owned by the driver. The driver must not overwrite the registers if the firmware has control of them. The original implementation for this in commit 07f7d6e7a124 ("ASoC: cs35l56: Fix for initializing ASP1 mixer registers") was to still provide defaults for these registers, assuming that if they were never reconfigured from defaults then regcache_sync() would not write them out because they are not dirty. Unfortunately regcache_sync() is not that smart. If the chip has not reset (so the driver has not called regcache_mark_dirty()) a regcache_sync() could write out registers that are not dirty. To avoid accidental overwriting of the ASP registers, they are removed from the table of defaults and instead are populated with defaults only if one of the ASP DAI configuration functions is called. So if the DAI has never been configured, the firmware is assumed to have ownership of these registers, and the regmap cache will not contain any entries for them. Signed-off-by: Richard Fitzgerald Fixes: 07f7d6e7a124 ("ASoC: cs35l56: Fix for initializing ASP1 mixer registers") Link: https://msgid.link/r/20240408101803.43183-5-rf@opensource.cirrus.com Signed-off-by: Mark Brown --- include/sound/cs35l56.h | 2 ++ sound/soc/codecs/cs35l56-shared.c | 63 ++++++++++++++++++++++++++------------- sound/soc/codecs/cs35l56.c | 24 ++++++++++++++- 3 files changed, 67 insertions(+), 22 deletions(-) (limited to 'include') diff --git a/include/sound/cs35l56.h b/include/sound/cs35l56.h index e0629699b563..1a3c6f66f620 100644 --- a/include/sound/cs35l56.h +++ b/include/sound/cs35l56.h @@ -267,6 +267,7 @@ struct cs35l56_base { bool fw_patched; bool secured; bool can_hibernate; + bool fw_owns_asp1; bool cal_data_valid; s8 cal_index; struct cirrus_amp_cal_data cal_data; @@ -283,6 +284,7 @@ extern const char * const cs35l56_tx_input_texts[CS35L56_NUM_INPUT_SRC]; extern const unsigned int cs35l56_tx_input_values[CS35L56_NUM_INPUT_SRC]; int cs35l56_set_patch(struct cs35l56_base *cs35l56_base); +int cs35l56_init_asp1_regs_for_driver_control(struct cs35l56_base *cs35l56_base); int cs35l56_force_sync_asp1_registers_from_cache(struct cs35l56_base *cs35l56_base); int cs35l56_mbox_send(struct cs35l56_base *cs35l56_base, unsigned int command); int cs35l56_firmware_shutdown(struct cs35l56_base *cs35l56_base); diff --git a/sound/soc/codecs/cs35l56-shared.c b/sound/soc/codecs/cs35l56-shared.c index a83317db75ed..ec1d95e57061 100644 --- a/sound/soc/codecs/cs35l56-shared.c +++ b/sound/soc/codecs/cs35l56-shared.c @@ -40,16 +40,11 @@ EXPORT_SYMBOL_NS_GPL(cs35l56_set_patch, SND_SOC_CS35L56_SHARED); static const struct reg_default cs35l56_reg_defaults[] = { /* no defaults for OTP_MEM - first read populates cache */ - { CS35L56_ASP1_ENABLES1, 0x00000000 }, - { CS35L56_ASP1_CONTROL1, 0x00000028 }, - { CS35L56_ASP1_CONTROL2, 0x18180200 }, - { CS35L56_ASP1_CONTROL3, 0x00000002 }, - { CS35L56_ASP1_FRAME_CONTROL1, 0x03020100 }, - { CS35L56_ASP1_FRAME_CONTROL5, 0x00020100 }, - { CS35L56_ASP1_DATA_CONTROL1, 0x00000018 }, - { CS35L56_ASP1_DATA_CONTROL5, 0x00000018 }, - - /* no defaults for ASP1TX mixer */ + /* + * No defaults for ASP1 control or ASP1TX mixer. See + * cs35l56_populate_asp1_register_defaults() and + * cs35l56_sync_asp1_mixer_widgets_with_firmware(). + */ { CS35L56_SWIRE_DP3_CH1_INPUT, 0x00000018 }, { CS35L56_SWIRE_DP3_CH2_INPUT, 0x00000019 }, @@ -210,6 +205,36 @@ static bool cs35l56_volatile_reg(struct device *dev, unsigned int reg) } } +static const struct reg_sequence cs35l56_asp1_defaults[] = { + REG_SEQ0(CS35L56_ASP1_ENABLES1, 0x00000000), + REG_SEQ0(CS35L56_ASP1_CONTROL1, 0x00000028), + REG_SEQ0(CS35L56_ASP1_CONTROL2, 0x18180200), + REG_SEQ0(CS35L56_ASP1_CONTROL3, 0x00000002), + REG_SEQ0(CS35L56_ASP1_FRAME_CONTROL1, 0x03020100), + REG_SEQ0(CS35L56_ASP1_FRAME_CONTROL5, 0x00020100), + REG_SEQ0(CS35L56_ASP1_DATA_CONTROL1, 0x00000018), + REG_SEQ0(CS35L56_ASP1_DATA_CONTROL5, 0x00000018), +}; + +/* + * The firmware can have control of the ASP so we don't provide regmap + * with defaults for these registers, to prevent a regcache_sync() from + * overwriting the firmware settings. But if the machine driver hooks up + * the ASP it means the driver is taking control of the ASP, so then the + * registers are populated with the defaults. + */ +int cs35l56_init_asp1_regs_for_driver_control(struct cs35l56_base *cs35l56_base) +{ + if (!cs35l56_base->fw_owns_asp1) + return 0; + + cs35l56_base->fw_owns_asp1 = false; + + return regmap_multi_reg_write(cs35l56_base->regmap, cs35l56_asp1_defaults, + ARRAY_SIZE(cs35l56_asp1_defaults)); +} +EXPORT_SYMBOL_NS_GPL(cs35l56_init_asp1_regs_for_driver_control, SND_SOC_CS35L56_SHARED); + /* * The firmware boot sequence can overwrite the ASP1 config registers so that * they don't match regmap's view of their values. Rewrite the values from the @@ -217,19 +242,15 @@ static bool cs35l56_volatile_reg(struct device *dev, unsigned int reg) */ int cs35l56_force_sync_asp1_registers_from_cache(struct cs35l56_base *cs35l56_base) { - struct reg_sequence asp1_regs[] = { - { .reg = CS35L56_ASP1_ENABLES1 }, - { .reg = CS35L56_ASP1_CONTROL1 }, - { .reg = CS35L56_ASP1_CONTROL2 }, - { .reg = CS35L56_ASP1_CONTROL3 }, - { .reg = CS35L56_ASP1_FRAME_CONTROL1 }, - { .reg = CS35L56_ASP1_FRAME_CONTROL5 }, - { .reg = CS35L56_ASP1_DATA_CONTROL1 }, - { .reg = CS35L56_ASP1_DATA_CONTROL5 }, - }; + struct reg_sequence asp1_regs[ARRAY_SIZE(cs35l56_asp1_defaults)]; int i, ret; - /* Read values from regmap cache into a write sequence */ + if (cs35l56_base->fw_owns_asp1) + return 0; + + memcpy(asp1_regs, cs35l56_asp1_defaults, sizeof(asp1_regs)); + + /* Read current values from regmap cache into the write sequence */ for (i = 0; i < ARRAY_SIZE(asp1_regs); ++i) { ret = regmap_read(cs35l56_base->regmap, asp1_regs[i].reg, &asp1_regs[i].def); if (ret) diff --git a/sound/soc/codecs/cs35l56.c b/sound/soc/codecs/cs35l56.c index 5a4e0e479414..6331b8c6136e 100644 --- a/sound/soc/codecs/cs35l56.c +++ b/sound/soc/codecs/cs35l56.c @@ -454,9 +454,14 @@ static int cs35l56_asp_dai_set_fmt(struct snd_soc_dai *codec_dai, unsigned int f { struct cs35l56_private *cs35l56 = snd_soc_component_get_drvdata(codec_dai->component); unsigned int val; + int ret; dev_dbg(cs35l56->base.dev, "%s: %#x\n", __func__, fmt); + ret = cs35l56_init_asp1_regs_for_driver_control(&cs35l56->base); + if (ret) + return ret; + switch (fmt & SND_SOC_DAIFMT_CLOCK_PROVIDER_MASK) { case SND_SOC_DAIFMT_CBC_CFC: break; @@ -530,6 +535,11 @@ static int cs35l56_asp_dai_set_tdm_slot(struct snd_soc_dai *dai, unsigned int tx unsigned int rx_mask, int slots, int slot_width) { struct cs35l56_private *cs35l56 = snd_soc_component_get_drvdata(dai->component); + int ret; + + ret = cs35l56_init_asp1_regs_for_driver_control(&cs35l56->base); + if (ret) + return ret; if ((slots == 0) || (slot_width == 0)) { dev_dbg(cs35l56->base.dev, "tdm config cleared\n"); @@ -578,6 +588,11 @@ static int cs35l56_asp_dai_hw_params(struct snd_pcm_substream *substream, struct cs35l56_private *cs35l56 = snd_soc_component_get_drvdata(dai->component); unsigned int rate = params_rate(params); u8 asp_width, asp_wl; + int ret; + + ret = cs35l56_init_asp1_regs_for_driver_control(&cs35l56->base); + if (ret) + return ret; asp_wl = params_width(params); if (cs35l56->asp_slot_width) @@ -634,7 +649,11 @@ static int cs35l56_asp_dai_set_sysclk(struct snd_soc_dai *dai, int clk_id, unsigned int freq, int dir) { struct cs35l56_private *cs35l56 = snd_soc_component_get_drvdata(dai->component); - int freq_id; + int freq_id, ret; + + ret = cs35l56_init_asp1_regs_for_driver_control(&cs35l56->base); + if (ret) + return ret; if (freq == 0) { cs35l56->sysclk_set = false; @@ -1403,6 +1422,9 @@ int cs35l56_common_probe(struct cs35l56_private *cs35l56) cs35l56->base.cal_index = -1; cs35l56->speaker_id = -ENOENT; + /* Assume that the firmware owns ASP1 until we know different */ + cs35l56->base.fw_owns_asp1 = true; + dev_set_drvdata(cs35l56->base.dev, cs35l56); cs35l56_fill_supply_names(cs35l56->supplies); -- cgit v1.2.3 From f848337cd801c7106a4ec0d61765771dab2a5909 Mon Sep 17 00:00:00 2001 From: Oswald Buddenhagen Date: Sun, 28 Apr 2024 11:37:13 +0200 Subject: 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 Signed-off-by: Takashi Iwai Message-ID: <20240428093716.3198666-4-oswald.buddenhagen@gmx.de> --- include/sound/emu10k1.h | 3 +-- sound/pci/emu10k1/emu10k1.c | 3 +-- sound/pci/emu10k1/emu10k1_main.c | 56 ++++++++++++++++++++-------------------- 3 files changed, 30 insertions(+), 32 deletions(-) (limited to 'include') diff --git a/include/sound/emu10k1.h b/include/sound/emu10k1.h index 1af9e6819392..9cc10fab01a8 100644 --- 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 fe72e7d77241..dadeda7758ce 100644 --- 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 6265fc9ae260..86eaf5963502 100644 --- 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, ®); /* 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); -- cgit v1.2.3 From 2d3f4810886eb7c319cec41b6d725d2953bfa88a Mon Sep 17 00:00:00 2001 From: Oswald Buddenhagen Date: Sun, 28 Apr 2024 11:37:14 +0200 Subject: 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 Signed-off-by: Takashi Iwai Message-ID: <20240428093716.3198666-5-oswald.buddenhagen@gmx.de> --- include/sound/emu10k1.h | 4 ++++ sound/pci/emu10k1/emu10k1_main.c | 19 ++++++++++++---- sound/pci/emu10k1/emumixer.c | 18 ++++++++++----- sound/pci/emu10k1/emuproc.c | 9 ++++++++ sound/pci/emu10k1/io.c | 48 +++++++++++++++++----------------------- 5 files changed, 61 insertions(+), 37 deletions(-) (limited to 'include') diff --git a/include/sound/emu10k1.h b/include/sound/emu10k1.h index 9cc10fab01a8..234b5baea69c 100644 --- a/include/sound/emu10k1.h +++ b/include/sound/emu10k1.h @@ -1685,6 +1685,7 @@ struct snd_emu1010 { unsigned int optical_in; /* 0:SPDIF, 1:ADAT */ unsigned int optical_out; /* 0:SPDIF, 1:ADAT */ struct work_struct work; + struct mutex lock; }; struct snd_emu10k1 { @@ -1833,6 +1834,9 @@ unsigned int snd_emu10k1_ptr20_read(struct snd_emu10k1 * emu, unsigned int reg, void snd_emu10k1_ptr20_write(struct snd_emu10k1 *emu, unsigned int reg, unsigned int chn, unsigned int data); int snd_emu10k1_spi_write(struct snd_emu10k1 * emu, unsigned int data); int snd_emu10k1_i2c_write(struct snd_emu10k1 *emu, u32 reg, u32 value); +static inline void snd_emu1010_fpga_lock(struct snd_emu10k1 *emu) { mutex_lock(&emu->emu1010.lock); }; +static inline void snd_emu1010_fpga_unlock(struct snd_emu10k1 *emu) { mutex_unlock(&emu->emu1010.lock); }; +void snd_emu1010_fpga_write_lock(struct snd_emu10k1 *emu, u32 reg, u32 value); void snd_emu1010_fpga_write(struct snd_emu10k1 *emu, u32 reg, u32 value); void snd_emu1010_fpga_read(struct snd_emu10k1 *emu, u32 reg, u32 *value); void snd_emu1010_fpga_link_dst_src_write(struct snd_emu10k1 *emu, u32 dst, u32 src); diff --git a/sound/pci/emu10k1/emu10k1_main.c b/sound/pci/emu10k1/emu10k1_main.c index 86eaf5963502..1a2905e8672b 100644 --- a/sound/pci/emu10k1/emu10k1_main.c +++ b/sound/pci/emu10k1/emu10k1_main.c @@ -810,6 +810,8 @@ static void emu1010_work(struct work_struct *work) return; #endif + snd_emu1010_fpga_lock(emu); + snd_emu1010_fpga_read(emu, EMU_HANA_IRQ_STATUS, &sts); // The distinction of the IRQ status bits is unreliable, @@ -819,6 +821,8 @@ static void emu1010_work(struct work_struct *work) if (sts & EMU_HANA_IRQ_WCLK_CHANGED) emu1010_clock_event(emu); + + snd_emu1010_fpga_unlock(emu); } static void emu1010_interrupt(struct snd_emu10k1 *emu) @@ -852,6 +856,8 @@ static int snd_emu10k1_emu1010_init(struct snd_emu10k1 *emu) * Proper init follows in snd_emu10k1_init(). */ outl(HCFG_LOCKSOUNDCACHE | HCFG_LOCKTANKCACHE_MASK, emu->port + HCFG); + snd_emu1010_fpga_lock(emu); + /* Disable 48Volt power to Audio Dock */ snd_emu1010_fpga_write(emu, EMU_HANA_DOCK_PWR, 0); @@ -877,7 +883,7 @@ static int snd_emu10k1_emu1010_init(struct snd_emu10k1 *emu) err = snd_emu1010_load_firmware(emu, 0, &emu->firmware); if (err < 0) { dev_info(emu->card->dev, "emu1010: Loading Firmware failed\n"); - return err; + goto fail; } /* ID, should read & 0x7f = 0x55 when FPGA programmed. */ @@ -887,7 +893,8 @@ static int snd_emu10k1_emu1010_init(struct snd_emu10k1 *emu) dev_info(emu->card->dev, "emu1010: Loading Hana Firmware file failed, reg = 0x%x\n", reg); - return -ENODEV; + err = -ENODEV; + goto fail; } dev_info(emu->card->dev, "emu1010: Hana Firmware loaded\n"); @@ -947,7 +954,9 @@ static int snd_emu10k1_emu1010_init(struct snd_emu10k1 *emu) // so it is safe to simply enable the outputs. snd_emu1010_fpga_write(emu, EMU_HANA_UNMUTE, EMU_UNMUTE); - return 0; +fail: + snd_emu1010_fpga_unlock(emu); + return err; } /* * Create the EMU10K1 instance @@ -969,9 +978,10 @@ static void snd_emu10k1_free(struct snd_card *card) } if (emu->card_capabilities->emu_model == EMU_MODEL_EMU1010) { /* Disable 48Volt power to Audio Dock */ - snd_emu1010_fpga_write(emu, EMU_HANA_DOCK_PWR, 0); + snd_emu1010_fpga_write_lock(emu, EMU_HANA_DOCK_PWR, 0); } cancel_work_sync(&emu->emu1010.work); + mutex_destroy(&emu->emu1010.lock); release_firmware(emu->firmware); release_firmware(emu->dock_fw); snd_util_memhdr_free(emu->memhdr); @@ -1551,6 +1561,7 @@ int snd_emu10k1_create(struct snd_card *card, emu->synth = NULL; emu->get_synth_voice = NULL; INIT_WORK(&emu->emu1010.work, emu1010_work); + mutex_init(&emu->emu1010.lock); /* read revision & serial */ emu->revision = pci->revision; pci_read_config_dword(pci, PCI_SUBSYSTEM_VENDOR_ID, &emu->serial); diff --git a/sound/pci/emu10k1/emumixer.c b/sound/pci/emu10k1/emumixer.c index 0a32ea53d8c6..05b98d9b547b 100644 --- a/sound/pci/emu10k1/emumixer.c +++ b/sound/pci/emu10k1/emumixer.c @@ -661,7 +661,9 @@ static int snd_emu1010_output_source_put(struct snd_kcontrol *kcontrol, change = (emu->emu1010.output_source[channel] != val); if (change) { emu->emu1010.output_source[channel] = val; + snd_emu1010_fpga_lock(emu); snd_emu1010_output_source_apply(emu, channel, val); + snd_emu1010_fpga_unlock(emu); } return change; } @@ -705,7 +707,9 @@ static int snd_emu1010_input_source_put(struct snd_kcontrol *kcontrol, change = (emu->emu1010.input_source[channel] != val); if (change) { emu->emu1010.input_source[channel] = val; + snd_emu1010_fpga_lock(emu); snd_emu1010_input_source_apply(emu, channel, val); + snd_emu1010_fpga_unlock(emu); } return change; } @@ -774,7 +778,7 @@ static int snd_emu1010_adc_pads_put(struct snd_kcontrol *kcontrol, struct snd_ct cache = cache & ~mask; change = (cache != emu->emu1010.adc_pads); if (change) { - snd_emu1010_fpga_write(emu, EMU_HANA_ADC_PADS, cache ); + snd_emu1010_fpga_write_lock(emu, EMU_HANA_ADC_PADS, cache ); emu->emu1010.adc_pads = cache; } @@ -832,7 +836,7 @@ static int snd_emu1010_dac_pads_put(struct snd_kcontrol *kcontrol, struct snd_ct cache = cache & ~mask; change = (cache != emu->emu1010.dac_pads); if (change) { - snd_emu1010_fpga_write(emu, EMU_HANA_DAC_PADS, cache ); + snd_emu1010_fpga_write_lock(emu, EMU_HANA_DAC_PADS, cache ); emu->emu1010.dac_pads = cache; } @@ -980,6 +984,7 @@ static int snd_emu1010_clock_source_put(struct snd_kcontrol *kcontrol, val = ucontrol->value.enumerated.item[0] ; if (val >= emu_ci->num) return -EINVAL; + snd_emu1010_fpga_lock(emu); spin_lock_irq(&emu->reg_lock); change = (emu->emu1010.clock_source != val); if (change) { @@ -996,6 +1001,7 @@ static int snd_emu1010_clock_source_put(struct snd_kcontrol *kcontrol, } else { spin_unlock_irq(&emu->reg_lock); } + snd_emu1010_fpga_unlock(emu); return change; } @@ -1041,7 +1047,7 @@ static int snd_emu1010_clock_fallback_put(struct snd_kcontrol *kcontrol, change = (emu->emu1010.clock_fallback != val); if (change) { emu->emu1010.clock_fallback = val; - snd_emu1010_fpga_write(emu, EMU_HANA_DEFCLOCK, 1 - val); + snd_emu1010_fpga_write_lock(emu, EMU_HANA_DEFCLOCK, 1 - val); } return change; } @@ -1093,7 +1099,7 @@ static int snd_emu1010_optical_out_put(struct snd_kcontrol *kcontrol, emu->emu1010.optical_out = val; tmp = (emu->emu1010.optical_in ? EMU_HANA_OPTICAL_IN_ADAT : EMU_HANA_OPTICAL_IN_SPDIF) | (emu->emu1010.optical_out ? EMU_HANA_OPTICAL_OUT_ADAT : EMU_HANA_OPTICAL_OUT_SPDIF); - snd_emu1010_fpga_write(emu, EMU_HANA_OPTICAL_TYPE, tmp); + snd_emu1010_fpga_write_lock(emu, EMU_HANA_OPTICAL_TYPE, tmp); } return change; } @@ -1144,7 +1150,7 @@ static int snd_emu1010_optical_in_put(struct snd_kcontrol *kcontrol, emu->emu1010.optical_in = val; tmp = (emu->emu1010.optical_in ? EMU_HANA_OPTICAL_IN_ADAT : EMU_HANA_OPTICAL_IN_SPDIF) | (emu->emu1010.optical_out ? EMU_HANA_OPTICAL_OUT_ADAT : EMU_HANA_OPTICAL_OUT_SPDIF); - snd_emu1010_fpga_write(emu, EMU_HANA_OPTICAL_TYPE, tmp); + snd_emu1010_fpga_write_lock(emu, EMU_HANA_OPTICAL_TYPE, tmp); } return change; } @@ -2323,7 +2329,9 @@ int snd_emu10k1_mixer(struct snd_emu10k1 *emu, for (i = 0; i < emu_ri->n_outs; i++) emu->emu1010.output_source[i] = emu1010_map_source(emu_ri, emu_ri->out_dflts[i]); + snd_emu1010_fpga_lock(emu); snd_emu1010_apply_sources(emu); + snd_emu1010_fpga_unlock(emu); kctl = emu->ctl_clock_source = snd_ctl_new1(&snd_emu1010_clock_source, emu); err = snd_ctl_add(card, kctl); diff --git a/sound/pci/emu10k1/emuproc.c b/sound/pci/emu10k1/emuproc.c index 2f80fd91017c..737c28d31b41 100644 --- a/sound/pci/emu10k1/emuproc.c +++ b/sound/pci/emu10k1/emuproc.c @@ -165,6 +165,8 @@ static void snd_emu10k1_proc_spdif_read(struct snd_info_entry *entry, u32 value2; if (emu->card_capabilities->emu_model) { + snd_emu1010_fpga_lock(emu); + // This represents the S/PDIF lock status on 0404b, which is // kinda weird and unhelpful, because monitoring it via IRQ is // 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, snd_iprintf(buffer, "\nS/PDIF mode: %s%s\n", value & EMU_HANA_SPDIF_MODE_RX_PRO ? "professional" : "consumer", value & EMU_HANA_SPDIF_MODE_RX_NOCOPY ? ", no copy" : ""); + + snd_emu1010_fpga_unlock(emu); } else { snd_emu10k1_proc_spdif_status(emu, buffer, "CD-ROM S/PDIF In", CDCS, CDSRCS); 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, struct snd_emu10k1 *emu = entry->private_data; u32 value; int i; + + snd_emu1010_fpga_lock(emu); + snd_iprintf(buffer, "EMU1010 Registers:\n\n"); for(i = 0; i < 0x40; i+=1) { @@ -496,6 +503,8 @@ static void snd_emu_proc_emu1010_reg_read(struct snd_info_entry *entry, snd_emu_proc_emu1010_link_read(emu, buffer, 0x701); } } + + snd_emu1010_fpga_unlock(emu); } static void snd_emu_proc_io_reg_read(struct snd_info_entry *entry, diff --git a/sound/pci/emu10k1/io.c b/sound/pci/emu10k1/io.c index 74df2330015f..f3260a81e47b 100644 --- a/sound/pci/emu10k1/io.c +++ b/sound/pci/emu10k1/io.c @@ -289,20 +289,28 @@ static void snd_emu1010_fpga_write_locked(struct snd_emu10k1 *emu, u32 reg, u32 void snd_emu1010_fpga_write(struct snd_emu10k1 *emu, u32 reg, u32 value) { - unsigned long flags; + if (snd_BUG_ON(!mutex_is_locked(&emu->emu1010.lock))) + return; + snd_emu1010_fpga_write_locked(emu, reg, value); +} - spin_lock_irqsave(&emu->emu_lock, flags); +void snd_emu1010_fpga_write_lock(struct snd_emu10k1 *emu, u32 reg, u32 value) +{ + snd_emu1010_fpga_lock(emu); snd_emu1010_fpga_write_locked(emu, reg, value); - spin_unlock_irqrestore(&emu->emu_lock, flags); + snd_emu1010_fpga_unlock(emu); } -static void snd_emu1010_fpga_read_locked(struct snd_emu10k1 *emu, u32 reg, u32 *value) +void snd_emu1010_fpga_read(struct snd_emu10k1 *emu, u32 reg, u32 *value) { // The higest input pin is used as the designated interrupt trigger, // so it needs to be masked out. // But note that any other input pin change will also cause an IRQ, // so using this function often causes an IRQ as a side effect. u32 mask = emu->card_capabilities->ca0108_chip ? 0x1f : 0x7f; + + if (snd_BUG_ON(!mutex_is_locked(&emu->emu1010.lock))) + return; if (snd_BUG_ON(reg > 0x3f)) return; reg += 0x40; /* 0x40 upwards are registers. */ @@ -313,47 +321,31 @@ static void snd_emu1010_fpga_read_locked(struct snd_emu10k1 *emu, u32 reg, u32 * *value = ((inw(emu->port + A_GPIO) >> 8) & mask); } -void snd_emu1010_fpga_read(struct snd_emu10k1 *emu, u32 reg, u32 *value) -{ - unsigned long flags; - - spin_lock_irqsave(&emu->emu_lock, flags); - snd_emu1010_fpga_read_locked(emu, reg, value); - spin_unlock_irqrestore(&emu->emu_lock, flags); -} - /* Each Destination has one and only one Source, * but one Source can feed any number of Destinations simultaneously. */ void snd_emu1010_fpga_link_dst_src_write(struct snd_emu10k1 *emu, u32 dst, u32 src) { - unsigned long flags; - if (snd_BUG_ON(dst & ~0x71f)) return; if (snd_BUG_ON(src & ~0x71f)) return; - spin_lock_irqsave(&emu->emu_lock, flags); - snd_emu1010_fpga_write_locked(emu, EMU_HANA_DESTHI, dst >> 8); - snd_emu1010_fpga_write_locked(emu, EMU_HANA_DESTLO, dst & 0x1f); - snd_emu1010_fpga_write_locked(emu, EMU_HANA_SRCHI, src >> 8); - snd_emu1010_fpga_write_locked(emu, EMU_HANA_SRCLO, src & 0x1f); - spin_unlock_irqrestore(&emu->emu_lock, flags); + snd_emu1010_fpga_write(emu, EMU_HANA_DESTHI, dst >> 8); + snd_emu1010_fpga_write(emu, EMU_HANA_DESTLO, dst & 0x1f); + snd_emu1010_fpga_write(emu, EMU_HANA_SRCHI, src >> 8); + snd_emu1010_fpga_write(emu, EMU_HANA_SRCLO, src & 0x1f); } u32 snd_emu1010_fpga_link_dst_src_read(struct snd_emu10k1 *emu, u32 dst) { - unsigned long flags; u32 hi, lo; if (snd_BUG_ON(dst & ~0x71f)) return 0; - spin_lock_irqsave(&emu->emu_lock, flags); - snd_emu1010_fpga_write_locked(emu, EMU_HANA_DESTHI, dst >> 8); - snd_emu1010_fpga_write_locked(emu, EMU_HANA_DESTLO, dst & 0x1f); - snd_emu1010_fpga_read_locked(emu, EMU_HANA_SRCHI, &hi); - snd_emu1010_fpga_read_locked(emu, EMU_HANA_SRCLO, &lo); - spin_unlock_irqrestore(&emu->emu_lock, flags); + snd_emu1010_fpga_write(emu, EMU_HANA_DESTHI, dst >> 8); + snd_emu1010_fpga_write(emu, EMU_HANA_DESTLO, dst & 0x1f); + snd_emu1010_fpga_read(emu, EMU_HANA_SRCHI, &hi); + snd_emu1010_fpga_read(emu, EMU_HANA_SRCLO, &lo); return (hi << 8) | lo; } -- cgit v1.2.3