From 43c3910c7527e40c5c8e7c52290fafc8f5536cd4 Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Thu, 30 May 2024 18:27:48 -0500 Subject: [PATCH] ASoC: rt711-sdca: use generic SDCA interrupt handler Rather than open-code, use the generic SDCA interrupt handler after registering the interrupt sources and callbacks. FIXME: Need to figure out what actions need to be taken for INT0 and INT8. Signed-off-by: Pierre-Louis Bossart --- sound/soc/codecs/Kconfig | 1 + sound/soc/codecs/rt711-sdca-sdw.c | 152 ++++++------------------------ sound/soc/codecs/rt711-sdca.c | 100 +++++++++++++++++--- sound/soc/codecs/rt711-sdca.h | 3 - 4 files changed, 117 insertions(+), 139 deletions(-) diff --git a/sound/soc/codecs/Kconfig b/sound/soc/codecs/Kconfig index b5e6d0a986c8e9..afa2620b55d809 100644 --- a/sound/soc/codecs/Kconfig +++ b/sound/soc/codecs/Kconfig @@ -1727,6 +1727,7 @@ config SND_SOC_RT711_SDW config SND_SOC_RT711_SDCA_SDW tristate "Realtek RT711 SDCA Codec - SDW" depends on SOUNDWIRE + select SND_SOC_SDCA_IRQ_HANDLER select REGMAP_SOUNDWIRE select REGMAP_SOUNDWIRE_MBQ diff --git a/sound/soc/codecs/rt711-sdca-sdw.c b/sound/soc/codecs/rt711-sdca-sdw.c index f5933d2e085e95..712a0a4ada0b95 100644 --- a/sound/soc/codecs/rt711-sdca-sdw.c +++ b/sound/soc/codecs/rt711-sdca-sdw.c @@ -146,21 +146,6 @@ static int rt711_sdca_update_status(struct sdw_slave *slave, if (status == SDW_SLAVE_UNATTACHED) rt711->hw_init = false; - if (status == SDW_SLAVE_ATTACHED) { - if (rt711->hs_jack) { - /* - * Due to the SCP_SDCA_INTMASK will be cleared by any reset, and then - * if the device attached again, we will need to set the setting back. - * It could avoid losing the jack detection interrupt. - * This also could sync with the cache value as the rt711_sdca_jack_init set. - */ - sdw_write_no_pm(rt711->slave, SDW_SCP_SDCA_INTMASK1, - SDW_SCP_SDCA_INTMASK_SDCA_0); - sdw_write_no_pm(rt711->slave, SDW_SCP_SDCA_INTMASK2, - SDW_SCP_SDCA_INTMASK_SDCA_8); - } - } - /* * Perform initialization only if slave status is present and * hw_init flag is false @@ -238,9 +223,7 @@ static int rt711_sdca_interrupt_callback(struct sdw_slave *slave, struct sdw_slave_intr_status *status) { struct rt711_sdca_priv *rt711 = dev_get_drvdata(&slave->dev); - int ret, stat; - int count = 0, retry = 3; - unsigned int sdca_cascade, scp_sdca_stat1, scp_sdca_stat2 = 0; + int ret; dev_dbg(&slave->dev, "%s control_port_stat=%x, sdca_cascade=%x", __func__, @@ -248,90 +231,24 @@ static int rt711_sdca_interrupt_callback(struct sdw_slave *slave, if (cancel_delayed_work_sync(&rt711->jack_detect_work)) { dev_warn(&slave->dev, "%s the pending delayed_work was cancelled", __func__); - /* avoid the HID owner doesn't change to device */ - if (rt711->scp_sdca_stat2) - scp_sdca_stat2 = rt711->scp_sdca_stat2; + /* + * Preserve status for HID, to make sure the UMP interrupt is handled + * and the ownership can be changed in the workqueue back to DEVICE + */ + sdca_interrupt_clear_history(slave, BIT(8)); + } else { + /* clear all history */ + sdca_interrupt_clear_history(slave, 0); } - /* - * The critical section below intentionally protects a rather large piece of code. - * We don't want to allow the system suspend to disable an interrupt while we are - * processing it, which could be problematic given the quirky SoundWire interrupt - * scheme. We do want however to prevent new workqueues from being scheduled if - * the disable_irq flag was set during system suspend. - */ - mutex_lock(&rt711->disable_irq_lock); - - ret = sdw_read_no_pm(rt711->slave, SDW_SCP_SDCA_INT1); - if (ret < 0) - goto io_error; - rt711->scp_sdca_stat1 = ret; - ret = sdw_read_no_pm(rt711->slave, SDW_SCP_SDCA_INT2); + ret = sdca_interrupt_handler(slave); if (ret < 0) - goto io_error; - rt711->scp_sdca_stat2 = ret; - if (scp_sdca_stat2) - rt711->scp_sdca_stat2 |= scp_sdca_stat2; - - do { - /* clear flag */ - ret = sdw_read_no_pm(rt711->slave, SDW_SCP_SDCA_INT1); - if (ret < 0) - goto io_error; - if (ret & SDW_SCP_SDCA_INTMASK_SDCA_0) { - ret = sdw_write_no_pm(rt711->slave, SDW_SCP_SDCA_INT1, - SDW_SCP_SDCA_INTMASK_SDCA_0); - if (ret < 0) - goto io_error; - } - ret = sdw_read_no_pm(rt711->slave, SDW_SCP_SDCA_INT2); - if (ret < 0) - goto io_error; - if (ret & SDW_SCP_SDCA_INTMASK_SDCA_8) { - ret = sdw_write_no_pm(rt711->slave, SDW_SCP_SDCA_INT2, - SDW_SCP_SDCA_INTMASK_SDCA_8); - if (ret < 0) - goto io_error; - } - - /* check if flag clear or not */ - ret = sdw_read_no_pm(rt711->slave, SDW_DP0_INT); - if (ret < 0) - goto io_error; - sdca_cascade = ret & SDW_DP0_SDCA_CASCADE; - - ret = sdw_read_no_pm(rt711->slave, SDW_SCP_SDCA_INT1); - if (ret < 0) - goto io_error; - scp_sdca_stat1 = ret & SDW_SCP_SDCA_INTMASK_SDCA_0; - - ret = sdw_read_no_pm(rt711->slave, SDW_SCP_SDCA_INT2); - if (ret < 0) - goto io_error; - scp_sdca_stat2 = ret & SDW_SCP_SDCA_INTMASK_SDCA_8; - - stat = scp_sdca_stat1 || scp_sdca_stat2 || sdca_cascade; + dev_err(&slave->dev, + "%s: error %d in sdca_interrupt_handler\n", + __func__, ret); - count++; - } while (stat != 0 && count < retry); - - if (stat) - dev_warn(&slave->dev, - "%s scp_sdca_stat1=0x%x, scp_sdca_stat2=0x%x\n", __func__, - rt711->scp_sdca_stat1, rt711->scp_sdca_stat2); - - if (status->sdca_cascade && !rt711->disable_irq) - mod_delayed_work(system_power_efficient_wq, - &rt711->jack_detect_work, msecs_to_jiffies(30)); - - mutex_unlock(&rt711->disable_irq_lock); - - return 0; - -io_error: - mutex_unlock(&rt711->disable_irq_lock); - pr_err_ratelimited("IO error in %s, ret %d\n", __func__, ret); return ret; + } static const struct sdw_slave_ops rt711_sdca_slave_ops = { @@ -369,7 +286,8 @@ static int rt711_sdca_sdw_remove(struct sdw_slave *slave) pm_runtime_disable(&slave->dev); mutex_destroy(&rt711->calibrate_mutex); - mutex_destroy(&rt711->disable_irq_lock); + + sdca_interrupt_info_release(slave); return 0; } @@ -400,28 +318,17 @@ static int __maybe_unused rt711_sdca_dev_system_suspend(struct device *dev) { struct rt711_sdca_priv *rt711_sdca = dev_get_drvdata(dev); struct sdw_slave *slave = dev_to_sdw_dev(dev); - int ret1, ret2; + int ret; if (!rt711_sdca->hw_init) return 0; - /* - * prevent new interrupts from being handled after the - * deferred work completes and before the parent disables - * interrupts on the link - */ - mutex_lock(&rt711_sdca->disable_irq_lock); - rt711_sdca->disable_irq = true; - ret1 = sdw_update_no_pm(slave, SDW_SCP_SDCA_INTMASK1, - SDW_SCP_SDCA_INTMASK_SDCA_0, 0); - ret2 = sdw_update_no_pm(slave, SDW_SCP_SDCA_INTMASK2, - SDW_SCP_SDCA_INTMASK_SDCA_8, 0); - mutex_unlock(&rt711_sdca->disable_irq_lock); - - if (ret1 < 0 || ret2 < 0) { + ret = sdca_interrupt_enable(rt711_sdca->slave, + BIT(8) | BIT(0), + false); + if (ret < 0) /* log but don't prevent suspend from happening */ - dev_dbg(&slave->dev, "%s: could not disable SDCA interrupts\n:", __func__); - } + dev_dbg(&slave->dev, "could not disable SDCA interrupts\n:"); return rt711_sdca_dev_suspend(dev); } @@ -433,18 +340,18 @@ static int __maybe_unused rt711_sdca_dev_resume(struct device *dev) struct sdw_slave *slave = dev_to_sdw_dev(dev); struct rt711_sdca_priv *rt711 = dev_get_drvdata(dev); unsigned long time; + int ret; if (!rt711->first_hw_init) return 0; if (!slave->unattach_request) { - mutex_lock(&rt711->disable_irq_lock); - if (rt711->disable_irq == true) { - sdw_write_no_pm(slave, SDW_SCP_SDCA_INTMASK1, SDW_SCP_SDCA_INTMASK_SDCA_0); - sdw_write_no_pm(slave, SDW_SCP_SDCA_INTMASK2, SDW_SCP_SDCA_INTMASK_SDCA_8); - rt711->disable_irq = false; - } - mutex_unlock(&rt711->disable_irq_lock); + ret = sdca_interrupt_enable(rt711->slave, + BIT(8) | BIT(0), + true); + if (ret < 0) + /* log but don't prevent resume from happening */ + dev_dbg(&slave->dev, "could not enable SDCA interrupts\n:"); goto regmap_sync; } @@ -486,3 +393,4 @@ module_sdw_driver(rt711_sdca_sdw_driver); MODULE_DESCRIPTION("ASoC RT711 SDCA SDW driver"); MODULE_AUTHOR("Shuming Fan "); MODULE_LICENSE("GPL"); +MODULE_IMPORT_NS(SND_SOC_SDCA_IRQ_HANDLER); diff --git a/sound/soc/codecs/rt711-sdca.c b/sound/soc/codecs/rt711-sdca.c index dd6ccf17afd439..c6fb510b891994 100644 --- a/sound/soc/codecs/rt711-sdca.c +++ b/sound/soc/codecs/rt711-sdca.c @@ -23,6 +23,7 @@ #include #include #include +#include #include "rt711-sdca.h" @@ -310,8 +311,13 @@ static void rt711_sdca_jack_detect_handler(struct work_struct *work) { struct rt711_sdca_priv *rt711 = container_of(work, struct rt711_sdca_priv, jack_detect_work.work); + struct sdca_interrupt_info *interrupt_info; int btn_type = 0, ret; + interrupt_info = rt711->slave->sdca_data.interrupt_info; + if (!interrupt_info) + return; + if (!rt711->hs_jack) return; @@ -319,14 +325,14 @@ static void rt711_sdca_jack_detect_handler(struct work_struct *work) return; /* SDW_SCP_SDCA_INT_SDCA_0 is used for jack detection */ - if (rt711->scp_sdca_stat1 & SDW_SCP_SDCA_INT_SDCA_0) { + if (interrupt_info->detected_interrupt_mask & BIT(0)) { ret = rt711_sdca_headset_detect(rt711); if (ret < 0) return; } /* SDW_SCP_SDCA_INT_SDCA_8 is used for button detection */ - if (rt711->scp_sdca_stat2 & SDW_SCP_SDCA_INT_SDCA_8) + if (interrupt_info->detected_interrupt_mask & BIT(8)) btn_type = rt711_sdca_button_detect(rt711); if (rt711->jack_type == 0) @@ -337,8 +343,7 @@ static void rt711_sdca_jack_detect_handler(struct work_struct *work) dev_dbg(&rt711->slave->dev, "in %s, btn_type=0x%x\n", __func__, btn_type); dev_dbg(&rt711->slave->dev, - "in %s, scp_sdca_stat1=0x%x, scp_sdca_stat2=0x%x\n", __func__, - rt711->scp_sdca_stat1, rt711->scp_sdca_stat2); + "in %s, detected_interrupt_mask=0x%x\n", __func__, interrupt_info->detected_interrupt_mask); snd_soc_jack_report(rt711->hs_jack, rt711->jack_type | btn_type, SND_JACK_HEADSET | @@ -450,6 +455,8 @@ static void rt711_sdca_btn_check_handler(struct work_struct *work) static void rt711_sdca_jack_init(struct rt711_sdca_priv *rt711) { + int ret; + mutex_lock(&rt711->calibrate_mutex); if (rt711->hs_jack) { @@ -503,11 +510,15 @@ static void rt711_sdca_jack_init(struct rt711_sdca_priv *rt711) break; } - /* set SCP_SDCA_IntMask1[0]=1 */ - sdw_write_no_pm(rt711->slave, SDW_SCP_SDCA_INTMASK1, SDW_SCP_SDCA_INTMASK_SDCA_0); - /* set SCP_SDCA_IntMask2[0]=1 */ - sdw_write_no_pm(rt711->slave, SDW_SCP_SDCA_INTMASK2, SDW_SCP_SDCA_INTMASK_SDCA_8); - dev_dbg(&rt711->slave->dev, "in %s enable\n", __func__); + ret = sdca_interrupt_enable(rt711->slave, + BIT(0) | BIT(8), + true); + if (ret < 0) + dev_err(&rt711->slave->dev, + "%s: sdca_interrupt_enable failed: %d\n", + __func__, ret); + else + dev_dbg(&rt711->slave->dev, "in %s enable\n", __func__); } else { /* disable HID 1/2 event */ rt711_sdca_index_update_bits(rt711, RT711_VENDOR_HDA_CTL, @@ -1470,9 +1481,37 @@ static struct snd_soc_dai_driver rt711_sdca_dai[] = { } }; +/* this is called with the irqs_lock mutex held */ +static void jack_detection_callback(void *context) +{ + struct rt711_sdca_priv *rt711 = context; + + /* + * Invoke the delayed work unconditionally, the test on + * the SDCA_0 being enabled was done in the common handler + */ + mod_delayed_work(system_power_efficient_wq, + &rt711->jack_detect_work, msecs_to_jiffies(30)); +} + +/* this is called with the irqs_lock mutex held */ +static void button_detection_callback(void *context) +{ + struct rt711_sdca_priv *rt711 = context; + + /* + * Invoke the delayed work unconditionally, the test on + * the SDCA_8 being enabled was done in the common handler + */ + mod_delayed_work(system_power_efficient_wq, + &rt711->jack_detect_work, msecs_to_jiffies(30)); +} + int rt711_sdca_init(struct device *dev, struct regmap *regmap, struct regmap *mbq_regmap, struct sdw_slave *slave) { + struct sdca_interrupt_source *sdca_int_0; + struct sdca_interrupt_source *sdca_int_8; struct rt711_sdca_priv *rt711; int ret; @@ -1480,6 +1519,41 @@ int rt711_sdca_init(struct device *dev, struct regmap *regmap, if (!rt711) return -ENOMEM; + ret = sdca_interrupt_info_alloc(slave); + if (ret < 0) + return ret; + + /* SDCA_INT1 and SDCA_INT2 are supported */ + ret = sdca_interrupt_initialize(slave, GENMASK(1, 0)); + if (ret < 0) + return ret; + + /* alloc and configure SDCA interrupt sources */ + sdca_int_0 = devm_kzalloc(dev, sizeof(*sdca_int_0), GFP_KERNEL); + if (!sdca_int_0) + return -ENOMEM; + + sdca_int_8 = devm_kzalloc(dev, sizeof(*sdca_int_8), GFP_KERNEL); + if (!sdca_int_8) + return -ENOMEM; + + sdca_int_0->index = 0; + sdca_int_0->context = rt711; + sdca_int_0->callback = jack_detection_callback; + + sdca_int_8->index = 8; + sdca_int_8->context = rt711; + sdca_int_8->callback = button_detection_callback; + + /* now register sources */ + ret = sdca_interrupt_register_source(slave, sdca_int_0); + if (ret < 0) + return ret; + + ret = sdca_interrupt_register_source(slave, sdca_int_8); + if (ret < 0) + return ret; + dev_set_drvdata(dev, rt711); rt711->slave = slave; rt711->regmap = regmap; @@ -1489,7 +1563,6 @@ int rt711_sdca_init(struct device *dev, struct regmap *regmap, regcache_cache_only(rt711->mbq_regmap, true); mutex_init(&rt711->calibrate_mutex); - mutex_init(&rt711->disable_irq_lock); INIT_DELAYED_WORK(&rt711->jack_detect_work, rt711_sdca_jack_detect_handler); INIT_DELAYED_WORK(&rt711->jack_btn_check_work, rt711_sdca_btn_check_handler); @@ -1595,10 +1668,9 @@ int rt711_sdca_io_init(struct device *dev, struct sdw_slave *slave) int ret = 0; unsigned int val; - rt711->disable_irq = false; - - if (rt711->hw_init) - return 0; + ret = sdca_interrupt_info_reset(slave); + if (ret < 0) + return ret; regcache_cache_only(rt711->regmap, false); regcache_cache_only(rt711->mbq_regmap, false); diff --git a/sound/soc/codecs/rt711-sdca.h b/sound/soc/codecs/rt711-sdca.h index 15263dcb0314ac..91ea0d61ce7aaa 100644 --- a/sound/soc/codecs/rt711-sdca.h +++ b/sound/soc/codecs/rt711-sdca.h @@ -26,10 +26,7 @@ struct rt711_sdca_priv { struct delayed_work jack_detect_work; struct delayed_work jack_btn_check_work; struct mutex calibrate_mutex; /* for headset calibration */ - struct mutex disable_irq_lock; /* SDCA irq lock protection */ - bool disable_irq; int jack_type, jd_src; - unsigned int scp_sdca_stat1, scp_sdca_stat2; int hw_ver; bool fu0f_dapm_mute, fu0f_mixer_l_mute, fu0f_mixer_r_mute; bool fu1e_dapm_mute, fu1e_mixer_l_mute, fu1e_mixer_r_mute;