From patchwork Mon Jun 14 18:08:11 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Pierre-Louis Bossart X-Patchwork-Id: 459936 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-16.8 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID, HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_CR_TRAILER, INCLUDES_PATCH, MAILING_LIST_MULTI, SPF_HELO_NONE, SPF_PASS, URIBL_BLOCKED, USER_AGENT_GIT autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 0C9FBC2B9F4 for ; Mon, 14 Jun 2021 18:12:08 +0000 (UTC) Received: from alsa0.perex.cz (alsa0.perex.cz [77.48.224.243]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 7431C610A3 for ; Mon, 14 Jun 2021 18:12:07 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 7431C610A3 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=linux.intel.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=alsa-devel-bounces@alsa-project.org Received: from alsa1.perex.cz (alsa1.perex.cz [207.180.221.201]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by alsa0.perex.cz (Postfix) with ESMTPS id F0C031699; Mon, 14 Jun 2021 20:11:15 +0200 (CEST) DKIM-Filter: OpenDKIM Filter v2.11.0 alsa0.perex.cz F0C031699 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=alsa-project.org; s=default; t=1623694326; bh=UrfyM19Gzr1IPLHE8IrRlP8ldlC+LoFnsjbE7B5yO6Q=; h=From:To:Subject:Date:In-Reply-To:References:Cc:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From; b=tZpV+jZQm5Dbdx3aMOtUgjD1qsOcbrmb2Qi7YPq05AtePwtwhq5CdIqTAhaPJnOrd TRSrgIqWgz6ahNsAobFoWrKoD3EV27qjAMlamPIfJtDKWdde3E0CJsAVfbDavPu2Il m1OQRm7XekaPWq7M+YF4DpANcvv4aK7UajecGhok= Received: from alsa1.perex.cz (localhost.localdomain [127.0.0.1]) by alsa1.perex.cz (Postfix) with ESMTP id 1A0B4F804E2; Mon, 14 Jun 2021 20:09:06 +0200 (CEST) Received: by alsa1.perex.cz (Postfix, from userid 50401) id 3F847F8028B; Mon, 14 Jun 2021 20:09:00 +0200 (CEST) Received: from mga02.intel.com (mga02.intel.com [134.134.136.20]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by alsa1.perex.cz (Postfix) with ESMTPS id B8A30F804B0 for ; Mon, 14 Jun 2021 20:08:51 +0200 (CEST) DKIM-Filter: OpenDKIM Filter v2.11.0 alsa1.perex.cz B8A30F804B0 IronPort-SDR: DE187v1OTQOl6HsdfP81MwD6G1wsUov0V9KOsZZL7yOk22IqvM53vDrMqq/rULMFcXj3yf7Q2C 4QyYRpFw71aw== X-IronPort-AV: E=McAfee;i="6200,9189,10015"; a="192969572" X-IronPort-AV: E=Sophos;i="5.83,273,1616482800"; d="scan'208";a="192969572" Received: from orsmga001.jf.intel.com ([10.7.209.18]) by orsmga101.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 14 Jun 2021 11:08:43 -0700 IronPort-SDR: epK+xTlGYHENtncTLXGdA6WyMiw/vl1OD/KZoeuxdbIk3sesVcf4S5kDKrTVL6uzmlq3N5E6HR 5nxDiOhEVZ4A== X-IronPort-AV: E=Sophos;i="5.83,273,1616482800"; d="scan'208";a="484166413" Received: from jepowell-mobl1.amr.corp.intel.com (HELO pbossart-mobl3.intel.com) ([10.212.114.106]) by orsmga001-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 14 Jun 2021 11:08:42 -0700 From: Pierre-Louis Bossart To: alsa-devel@alsa-project.org, srinivas.kandagatla@linaro.org Subject: [PATCH 1/5] soundwire: export sdw_update() and sdw_update_no_pm() Date: Mon, 14 Jun 2021 13:08:11 -0500 Message-Id: <20210614180815.153711-2-pierre-louis.bossart@linux.intel.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20210614180815.153711-1-pierre-louis.bossart@linux.intel.com> References: <20210614180815.153711-1-pierre-louis.bossart@linux.intel.com> MIME-Version: 1.0 Cc: Oder Chiou , Jack Yu , open list , tiwai@suse.de, gregkh@linuxfoundation.org, =?utf-8?q?P=C3=A9ter_Ujfalusi?= , Pierre-Louis Bossart , Hui Wang , vkoul@kernel.org, broonie@kernel.org, Shuming Fan , Sanyog Kale , Bard liao , Rander Wang , Bard Liao X-BeenThere: alsa-devel@alsa-project.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: "Alsa-devel mailing list for ALSA developers - http://www.alsa-project.org" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: alsa-devel-bounces@alsa-project.org Sender: "Alsa-devel" We currently export sdw_read() and sdw_write() but the sdw_update() and sdw_update_no_pm() are currently available only to the bus code. This was missed in an earlier contribution. Export both functions so that codec drivers can perform read-modify-write operations without duplicating the code. Fixes: b04c975e654c ('soundwire: bus: use sdw_update_no_pm when initializing a device') Signed-off-by: Pierre-Louis Bossart Reviewed-by: Bard Liao Reviewed-by: Péter Ujfalusi --- drivers/soundwire/bus.c | 17 ++++++++++++++++- drivers/soundwire/bus.h | 13 ------------- include/linux/soundwire/sdw.h | 3 +++ 3 files changed, 19 insertions(+), 14 deletions(-) diff --git a/drivers/soundwire/bus.c b/drivers/soundwire/bus.c index a9e0aa72654d..5d5b0bd59ae3 100644 --- a/drivers/soundwire/bus.c +++ b/drivers/soundwire/bus.c @@ -492,7 +492,7 @@ int sdw_read_no_pm(struct sdw_slave *slave, u32 addr) } EXPORT_SYMBOL(sdw_read_no_pm); -static int sdw_update_no_pm(struct sdw_slave *slave, u32 addr, u8 mask, u8 val) +int sdw_update_no_pm(struct sdw_slave *slave, u32 addr, u8 mask, u8 val) { int tmp; @@ -503,6 +503,21 @@ static int sdw_update_no_pm(struct sdw_slave *slave, u32 addr, u8 mask, u8 val) tmp = (tmp & ~mask) | val; return sdw_write_no_pm(slave, addr, tmp); } +EXPORT_SYMBOL(sdw_update_no_pm); + +/* Read-Modify-Write Slave register */ +int sdw_update(struct sdw_slave *slave, u32 addr, u8 mask, u8 val) +{ + int tmp; + + tmp = sdw_read(slave, addr); + if (tmp < 0) + return tmp; + + tmp = (tmp & ~mask) | val; + return sdw_write(slave, addr, tmp); +} +EXPORT_SYMBOL(sdw_update); /** * sdw_nread() - Read "n" contiguous SDW Slave registers diff --git a/drivers/soundwire/bus.h b/drivers/soundwire/bus.h index 40354469860a..7631ef5e71fb 100644 --- a/drivers/soundwire/bus.h +++ b/drivers/soundwire/bus.h @@ -201,19 +201,6 @@ static inline void sdw_fill_port_params(struct sdw_port_params *params, params->data_mode = data_mode; } -/* Read-Modify-Write Slave register */ -static inline int sdw_update(struct sdw_slave *slave, u32 addr, u8 mask, u8 val) -{ - int tmp; - - tmp = sdw_read(slave, addr); - if (tmp < 0) - return tmp; - - tmp = (tmp & ~mask) | val; - return sdw_write(slave, addr, tmp); -} - /* broadcast read/write for tests */ int sdw_bread_no_pm_unlocked(struct sdw_bus *bus, u16 dev_num, u32 addr); int sdw_bwrite_no_pm_unlocked(struct sdw_bus *bus, u16 dev_num, u32 addr, u8 value); diff --git a/include/linux/soundwire/sdw.h b/include/linux/soundwire/sdw.h index ced07f8fde87..de9802a24e7e 100644 --- a/include/linux/soundwire/sdw.h +++ b/include/linux/soundwire/sdw.h @@ -1041,6 +1041,9 @@ int sdw_write_no_pm(struct sdw_slave *slave, u32 addr, u8 value); int sdw_read_no_pm(struct sdw_slave *slave, u32 addr); int sdw_nread(struct sdw_slave *slave, u32 addr, size_t count, u8 *val); int sdw_nwrite(struct sdw_slave *slave, u32 addr, size_t count, u8 *val); +int sdw_update(struct sdw_slave *slave, u32 addr, u8 mask, u8 val); +int sdw_update_no_pm(struct sdw_slave *slave, u32 addr, u8 mask, u8 val); + int sdw_compare_devid(struct sdw_slave *slave, struct sdw_slave_id id); void sdw_extract_slave_id(struct sdw_bus *bus, u64 addr, struct sdw_slave_id *id); From patchwork Mon Jun 14 18:08:12 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Pierre-Louis Bossart X-Patchwork-Id: 459938 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-21.8 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID, HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_CR_TRAILER, INCLUDES_PATCH, MAILING_LIST_MULTI, MENTIONS_GIT_HOSTING, SPF_HELO_NONE, SPF_PASS, URIBL_BLOCKED, USER_AGENT_GIT autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 78BF8C2B9F4 for ; Mon, 14 Jun 2021 18:10:41 +0000 (UTC) Received: from alsa0.perex.cz (alsa0.perex.cz [77.48.224.243]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id AF7C5601FA for ; Mon, 14 Jun 2021 18:10:40 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org AF7C5601FA Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=linux.intel.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=alsa-devel-bounces@alsa-project.org Received: from alsa1.perex.cz (alsa1.perex.cz [207.180.221.201]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by alsa0.perex.cz (Postfix) with ESMTPS id 1A7CB168B; Mon, 14 Jun 2021 20:09:49 +0200 (CEST) DKIM-Filter: OpenDKIM Filter v2.11.0 alsa0.perex.cz 1A7CB168B DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=alsa-project.org; s=default; t=1623694239; bh=ng/L5vEWSUxMmHGYfl9Pc9AQ1U7DhYLZq24p8A4yI5k=; h=From:To:Subject:Date:In-Reply-To:References:Cc:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From; b=CLk3HK88NpjO54m2mLFxM/GHM+GD/32jCUNQPRdUs9OSFBhvTkZV0uUTBEAllxUXQ E7SBJE2YQ/kasrfE032NrabS4RPRQYtKX5wHs2Ikgqdyag1m51+uKd3r5t3mZwmUfu GVL5qn0nv6pYo/i1tVUZ4W9Iw9sf2qcioQOOqCSE= Received: from alsa1.perex.cz (localhost.localdomain [127.0.0.1]) by alsa1.perex.cz (Postfix) with ESMTP id CF8A1F800F7; Mon, 14 Jun 2021 20:09:00 +0200 (CEST) Received: by alsa1.perex.cz (Postfix, from userid 50401) id 7DAA1F804C2; Mon, 14 Jun 2021 20:08:56 +0200 (CEST) Received: from mga02.intel.com (mga02.intel.com [134.134.136.20]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by alsa1.perex.cz (Postfix) with ESMTPS id D06B2F800F7 for ; Mon, 14 Jun 2021 20:08:52 +0200 (CEST) DKIM-Filter: OpenDKIM Filter v2.11.0 alsa1.perex.cz D06B2F800F7 IronPort-SDR: TAZvkYvx5jQZlD+6KJ6i6roF7uFQ+vtuX8X9ix9iBSiEsvEgeMzU+kQunymzOyd/DlIfrg1JQj cCZeOoq6fuew== X-IronPort-AV: E=McAfee;i="6200,9189,10015"; a="192969575" X-IronPort-AV: E=Sophos;i="5.83,273,1616482800"; d="scan'208";a="192969575" Received: from orsmga001.jf.intel.com ([10.7.209.18]) by orsmga101.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 14 Jun 2021 11:08:45 -0700 IronPort-SDR: lhAyVb42Y4tFIWQd17H9o9r/4UwWb+jlKm45saZS4Yc4VKAeIesxvfM2d9LCK+n4h6z4CwcFxu uefeOaX+rBbQ== X-IronPort-AV: E=Sophos;i="5.83,273,1616482800"; d="scan'208";a="484166421" Received: from jepowell-mobl1.amr.corp.intel.com (HELO pbossart-mobl3.intel.com) ([10.212.114.106]) by orsmga001-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 14 Jun 2021 11:08:44 -0700 From: Pierre-Louis Bossart To: alsa-devel@alsa-project.org, srinivas.kandagatla@linaro.org Subject: [PATCH 2/5] ASoC: rt700-sdw: fix race condition on system suspend Date: Mon, 14 Jun 2021 13:08:12 -0500 Message-Id: <20210614180815.153711-3-pierre-louis.bossart@linux.intel.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20210614180815.153711-1-pierre-louis.bossart@linux.intel.com> References: <20210614180815.153711-1-pierre-louis.bossart@linux.intel.com> MIME-Version: 1.0 Cc: Oder Chiou , Jack Yu , open list , Liam Girdwood , tiwai@suse.de, gregkh@linuxfoundation.org, Takashi Iwai , =?utf-8?q?P=C3=A9ter_Ujfalusi?= , Pierre-Louis Bossart , Hui Wang , vkoul@kernel.org, broonie@kernel.org, Shuming Fan , Bard liao , Rander Wang , Bard Liao X-BeenThere: alsa-devel@alsa-project.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: "Alsa-devel mailing list for ALSA developers - http://www.alsa-project.org" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: alsa-devel-bounces@alsa-project.org Sender: "Alsa-devel" In previous commits we cancelled deferred work, but there is still a window of time where a new interrupt could result in new deferred work executed after the link is disabled, leading to an IO error. This patch uses an 'disable_irq_lock' mutex to prevent new interrupts from happening after the start of the system suspend. The choice of a mutex v. a spinlock is mainly due to the time required to clear interrupts, which requires a command to be transmitted by the SoundWire host IP and acknowledged with an interrupt. The 'interrupt_callback' routine is also not meant to be called from an interrupt context. An additional 'disable_irq' flag prevents race conditions where the status changes before the interrupts are disabled, but the workqueue handling status changes is scheduled after the completion of the system suspend. On resume the interrupts are re-enabled already by the io_init routine so we only clear the flag. BugLink: https://github.com/thesofproject/linux/issues/2943 Fixes: 5f2df2a4583b ('ASoC: rt700: wait for the delayed work to finish when the system suspends') Signed-off-by: Pierre-Louis Bossart Reviewed-by: Bard Liao Reviewed-by: Péter Ujfalusi --- sound/soc/codecs/rt700-sdw.c | 34 ++++++++++++++++++++++++++++++++-- sound/soc/codecs/rt700.c | 4 ++++ sound/soc/codecs/rt700.h | 2 ++ 3 files changed, 38 insertions(+), 2 deletions(-) diff --git a/sound/soc/codecs/rt700-sdw.c b/sound/soc/codecs/rt700-sdw.c index d1d9c0f455b4..bda594899664 100644 --- a/sound/soc/codecs/rt700-sdw.c +++ b/sound/soc/codecs/rt700-sdw.c @@ -418,10 +418,12 @@ static int rt700_interrupt_callback(struct sdw_slave *slave, dev_dbg(&slave->dev, "%s control_port_stat=%x", __func__, status->control_port); - if (status->control_port & 0x4) { + mutex_lock(&rt700->disable_irq_lock); + if (status->control_port & 0x4 && !rt700->disable_irq) { mod_delayed_work(system_power_efficient_wq, &rt700->jack_detect_work, msecs_to_jiffies(250)); } + mutex_unlock(&rt700->disable_irq_lock); return 0; } @@ -490,6 +492,34 @@ static int __maybe_unused rt700_dev_suspend(struct device *dev) return 0; } +static int __maybe_unused rt700_dev_system_suspend(struct device *dev) +{ + struct sdw_slave *slave = dev_to_sdw_dev(dev); + struct rt700_priv *rt700 = dev_get_drvdata(dev); + int ret; + + if (!rt700->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(&rt700->disable_irq_lock); + rt700->disable_irq = true; + ret = sdw_update_no_pm(slave, SDW_SCP_INTMASK1, + SDW_SCP_INT1_IMPL_DEF, 0); + mutex_unlock(&rt700->disable_irq_lock); + + if (ret < 0) { + /* log but don't prevent suspend from happening */ + dev_dbg(&slave->dev, "%s: could not disable imp-def interrupts\n:", __func__); + } + + return rt700_dev_suspend(dev); +} + #define RT700_PROBE_TIMEOUT 5000 static int __maybe_unused rt700_dev_resume(struct device *dev) @@ -521,7 +551,7 @@ static int __maybe_unused rt700_dev_resume(struct device *dev) } static const struct dev_pm_ops rt700_pm = { - SET_SYSTEM_SLEEP_PM_OPS(rt700_dev_suspend, rt700_dev_resume) + SET_SYSTEM_SLEEP_PM_OPS(rt700_dev_system_suspend, rt700_dev_resume) SET_RUNTIME_PM_OPS(rt700_dev_suspend, rt700_dev_resume, NULL) }; diff --git a/sound/soc/codecs/rt700.c b/sound/soc/codecs/rt700.c index 01af9d9dd3ca..921382724f9c 100644 --- a/sound/soc/codecs/rt700.c +++ b/sound/soc/codecs/rt700.c @@ -1112,6 +1112,8 @@ int rt700_init(struct device *dev, struct regmap *sdw_regmap, rt700->sdw_regmap = sdw_regmap; rt700->regmap = regmap; + mutex_init(&rt700->disable_irq_lock); + /* * Mark hw_init to false * HW init will be performed when device reports present @@ -1133,6 +1135,8 @@ int rt700_io_init(struct device *dev, struct sdw_slave *slave) { struct rt700_priv *rt700 = dev_get_drvdata(dev); + rt700->disable_irq = false; + if (rt700->hw_init) return 0; diff --git a/sound/soc/codecs/rt700.h b/sound/soc/codecs/rt700.h index 794ee2e29051..bed9d1de6d5b 100644 --- a/sound/soc/codecs/rt700.h +++ b/sound/soc/codecs/rt700.h @@ -23,6 +23,8 @@ struct rt700_priv { struct delayed_work jack_detect_work; struct delayed_work jack_btn_check_work; int jack_type; + struct mutex disable_irq_lock; /* imp-def irq lock protection */ + bool disable_irq; }; struct sdw_stream_data { From patchwork Mon Jun 14 18:08:13 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Pierre-Louis Bossart X-Patchwork-Id: 460581 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-21.8 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID, HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_CR_TRAILER, INCLUDES_PATCH, MAILING_LIST_MULTI, MENTIONS_GIT_HOSTING, SPF_HELO_NONE, SPF_PASS, URIBL_BLOCKED, USER_AGENT_GIT autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id E1168C2B9F4 for ; Mon, 14 Jun 2021 18:10:45 +0000 (UTC) Received: from alsa0.perex.cz (alsa0.perex.cz [77.48.224.243]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 3F3C6601FA for ; Mon, 14 Jun 2021 18:10:45 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 3F3C6601FA Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=linux.intel.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=alsa-devel-bounces@alsa-project.org Received: from alsa1.perex.cz (alsa1.perex.cz [207.180.221.201]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by alsa0.perex.cz (Postfix) with ESMTPS id 8538F168C; Mon, 14 Jun 2021 20:09:53 +0200 (CEST) DKIM-Filter: OpenDKIM Filter v2.11.0 alsa0.perex.cz 8538F168C DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=alsa-project.org; s=default; t=1623694243; bh=YPXqwNTxjE0+LNSpeMRuo0UJcBU0hFwePIHSOveT3d4=; h=From:To:Subject:Date:In-Reply-To:References:Cc:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From; b=UI9q1+vHI++C/xMQpZRJyvqdVbjdyyNFCMxK98+if0pLu//3d71DCeImLBqrOt9PH I0tGdu7p+/De/fZ1Zx77marFkizyMy6L+D23MTpi2r3Llc2+B3/GQuR+7w77UCxtuL bdF2pVTDEHevzeLHzKdbToTfEITVg47PQrBP5ISI= Received: from alsa1.perex.cz (localhost.localdomain [127.0.0.1]) by alsa1.perex.cz (Postfix) with ESMTP id D5461F804CC; Mon, 14 Jun 2021 20:09:02 +0200 (CEST) Received: by alsa1.perex.cz (Postfix, from userid 50401) id 1789AF804C2; Mon, 14 Jun 2021 20:08:57 +0200 (CEST) Received: from mga02.intel.com (mga02.intel.com [134.134.136.20]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by alsa1.perex.cz (Postfix) with ESMTPS id B806EF80424 for ; Mon, 14 Jun 2021 20:08:53 +0200 (CEST) DKIM-Filter: OpenDKIM Filter v2.11.0 alsa1.perex.cz B806EF80424 IronPort-SDR: qh9ij7WTq0HXOnyvqwgC8P4mrZamit+AzUs9xe3b1BNKK6dAaoly7URhl7LqU0ivaqw4RXG5Oz aHH5iReNPIOg== X-IronPort-AV: E=McAfee;i="6200,9189,10015"; a="192969578" X-IronPort-AV: E=Sophos;i="5.83,273,1616482800"; d="scan'208";a="192969578" Received: from orsmga001.jf.intel.com ([10.7.209.18]) by orsmga101.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 14 Jun 2021 11:08:48 -0700 IronPort-SDR: JxpUUiVAyyQrQSij/i+O8Y6scFMUKJkkR9iq+9yJPSvqCHeEbd61JTP9ADc21TCL0AH5ZSiezV eSdbNa+wmyUQ== X-IronPort-AV: E=Sophos;i="5.83,273,1616482800"; d="scan'208";a="484166431" Received: from jepowell-mobl1.amr.corp.intel.com (HELO pbossart-mobl3.intel.com) ([10.212.114.106]) by orsmga001-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 14 Jun 2021 11:08:46 -0700 From: Pierre-Louis Bossart To: alsa-devel@alsa-project.org, srinivas.kandagatla@linaro.org Subject: [PATCH 3/5] ASoC: rt711-sdw: fix race condition on system suspend Date: Mon, 14 Jun 2021 13:08:13 -0500 Message-Id: <20210614180815.153711-4-pierre-louis.bossart@linux.intel.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20210614180815.153711-1-pierre-louis.bossart@linux.intel.com> References: <20210614180815.153711-1-pierre-louis.bossart@linux.intel.com> MIME-Version: 1.0 Cc: Oder Chiou , Jack Yu , open list , Liam Girdwood , tiwai@suse.de, gregkh@linuxfoundation.org, Takashi Iwai , =?utf-8?q?P=C3=A9ter_Ujfalusi?= , Pierre-Louis Bossart , Hui Wang , vkoul@kernel.org, broonie@kernel.org, Shuming Fan , Bard liao , Rander Wang , Bard Liao X-BeenThere: alsa-devel@alsa-project.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: "Alsa-devel mailing list for ALSA developers - http://www.alsa-project.org" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: alsa-devel-bounces@alsa-project.org Sender: "Alsa-devel" In previous commits we cancelled deferred work, but there is still a window of time where a new interrupt could result in new deferred work executed after the link is disabled, leading to an IO error. While we did not see this IO error on RT711-based platforms, the code pattern is similar to the RT700 case where the IO error was noted, so the fix is added for consistency. This patch uses an 'disable_irq_lock' mutex to prevent new interrupts from happening after the start of the system suspend. The choice of a mutex v. a spinlock is mainly due to the time required to clear interrupts, which requires a command to be transmitted by the SoundWire host IP and acknowledged with an interrupt. The 'interrupt_callback' routine is also not meant to be called from an interrupt context. An additional 'disable_irq' flag prevents race conditions where the status changes before the interrupts are disabled, but the workqueue handling status changes is scheduled after the completion of the system suspend. On resume the interrupts are re-enabled already by the io_init routine so we only clear the flag. BugLink: https://github.com/thesofproject/linux/issues/2943 Fixes: 501ef013390b ('ASoC: rt711: wait for the delayed work to finish when the system suspends') Signed-off-by: Pierre-Louis Bossart Reviewed-by: Bard Liao Reviewed-by: Péter Ujfalusi --- sound/soc/codecs/rt711-sdw.c | 34 ++++++++++++++++++++++++++++++++-- sound/soc/codecs/rt711.c | 4 ++++ sound/soc/codecs/rt711.h | 2 ++ 3 files changed, 38 insertions(+), 2 deletions(-) diff --git a/sound/soc/codecs/rt711-sdw.c b/sound/soc/codecs/rt711-sdw.c index 15299084429f..bda2cc9439c9 100644 --- a/sound/soc/codecs/rt711-sdw.c +++ b/sound/soc/codecs/rt711-sdw.c @@ -423,10 +423,12 @@ static int rt711_interrupt_callback(struct sdw_slave *slave, dev_dbg(&slave->dev, "%s control_port_stat=%x", __func__, status->control_port); - if (status->control_port & 0x4) { + mutex_lock(&rt711->disable_irq_lock); + if (status->control_port & 0x4 && !rt711->disable_irq) { mod_delayed_work(system_power_efficient_wq, &rt711->jack_detect_work, msecs_to_jiffies(250)); } + mutex_unlock(&rt711->disable_irq_lock); return 0; } @@ -493,6 +495,34 @@ static int __maybe_unused rt711_dev_suspend(struct device *dev) return 0; } +static int __maybe_unused rt711_dev_system_suspend(struct device *dev) +{ + struct rt711_priv *rt711 = dev_get_drvdata(dev); + struct sdw_slave *slave = dev_to_sdw_dev(dev); + int ret; + + if (!rt711->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->disable_irq_lock); + rt711->disable_irq = true; + ret = sdw_update_no_pm(slave, SDW_SCP_INTMASK1, + SDW_SCP_INT1_IMPL_DEF, 0); + mutex_unlock(&rt711->disable_irq_lock); + + if (ret < 0) { + /* log but don't prevent suspend from happening */ + dev_dbg(&slave->dev, "%s: could not disable imp-def interrupts\n:", __func__); + } + + return rt711_dev_suspend(dev); +} + #define RT711_PROBE_TIMEOUT 5000 static int __maybe_unused rt711_dev_resume(struct device *dev) @@ -524,7 +554,7 @@ static int __maybe_unused rt711_dev_resume(struct device *dev) } static const struct dev_pm_ops rt711_pm = { - SET_SYSTEM_SLEEP_PM_OPS(rt711_dev_suspend, rt711_dev_resume) + SET_SYSTEM_SLEEP_PM_OPS(rt711_dev_system_suspend, rt711_dev_resume) SET_RUNTIME_PM_OPS(rt711_dev_suspend, rt711_dev_resume, NULL) }; diff --git a/sound/soc/codecs/rt711.c b/sound/soc/codecs/rt711.c index 9f5b2dc16c54..4dbfa7b8680e 100644 --- a/sound/soc/codecs/rt711.c +++ b/sound/soc/codecs/rt711.c @@ -1166,6 +1166,8 @@ int rt711_init(struct device *dev, struct regmap *sdw_regmap, rt711->sdw_regmap = sdw_regmap; rt711->regmap = regmap; + mutex_init(&rt711->disable_irq_lock); + /* * Mark hw_init to false * HW init will be performed when device reports present @@ -1190,6 +1192,8 @@ int rt711_io_init(struct device *dev, struct sdw_slave *slave) { struct rt711_priv *rt711 = dev_get_drvdata(dev); + rt711->disable_irq = false; + if (rt711->hw_init) return 0; diff --git a/sound/soc/codecs/rt711.h b/sound/soc/codecs/rt711.h index ca0f581feec7..2af467631435 100644 --- a/sound/soc/codecs/rt711.h +++ b/sound/soc/codecs/rt711.h @@ -25,6 +25,8 @@ struct rt711_priv { struct work_struct calibration_work; struct mutex calibrate_mutex; /* for headset calibration */ int jack_type, jd_src; + struct mutex disable_irq_lock; /* imp-def irq lock protection */ + bool disable_irq; }; struct sdw_stream_data { From patchwork Mon Jun 14 18:08:14 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Pierre-Louis Bossart X-Patchwork-Id: 459937 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-21.8 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID, HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_CR_TRAILER, INCLUDES_PATCH, MAILING_LIST_MULTI, MENTIONS_GIT_HOSTING, SPF_HELO_NONE, SPF_PASS, URIBL_BLOCKED, USER_AGENT_GIT autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 5C0B2C48BE6 for ; Mon, 14 Jun 2021 18:11:21 +0000 (UTC) Received: from alsa0.perex.cz (alsa0.perex.cz [77.48.224.243]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 8CC1F610A3 for ; Mon, 14 Jun 2021 18:11:20 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 8CC1F610A3 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=linux.intel.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=alsa-devel-bounces@alsa-project.org Received: from alsa1.perex.cz (alsa1.perex.cz [207.180.221.201]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by alsa0.perex.cz (Postfix) with ESMTPS id 326C8169B; Mon, 14 Jun 2021 20:10:29 +0200 (CEST) DKIM-Filter: OpenDKIM Filter v2.11.0 alsa0.perex.cz 326C8169B DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=alsa-project.org; s=default; t=1623694279; bh=RfIy6QgMfuFTeJWqKXklAL6IUdgA0Uqncz81RAnUveQ=; h=From:To:Subject:Date:In-Reply-To:References:Cc:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From; b=IzSyKd3GhoUoOKrj1Bz+hhunXg0Jp7U1kNFrCOkca07sfjEZs7t9lG+vEGjd4NlCy CHUG8YrfCv5TSAyQj20x5vsJMNp2SopVUOk937kxd3Rmg9xFPXBJqkwid8Pw0/Aw2w EdCXcsc1qWzTBFWGth5sMPH/rG1oVopGdG6yDOkA= Received: from alsa1.perex.cz (localhost.localdomain [127.0.0.1]) by alsa1.perex.cz (Postfix) with ESMTP id 8B19FF804CF; Mon, 14 Jun 2021 20:09:03 +0200 (CEST) Received: by alsa1.perex.cz (Postfix, from userid 50401) id 76A4DF804CA; Mon, 14 Jun 2021 20:08:58 +0200 (CEST) Received: from mga02.intel.com (mga02.intel.com [134.134.136.20]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by alsa1.perex.cz (Postfix) with ESMTPS id D11CBF8028B for ; Mon, 14 Jun 2021 20:08:54 +0200 (CEST) DKIM-Filter: OpenDKIM Filter v2.11.0 alsa1.perex.cz D11CBF8028B IronPort-SDR: moKwY6mGwqKGAZPP4CMLKPhT8gRuR82iCHosVVCc6J8HualZnZGfoJ9qAK/9juEQCvLRX7M5s5 F1ZxkUwTfjEQ== X-IronPort-AV: E=McAfee;i="6200,9189,10015"; a="192969581" X-IronPort-AV: E=Sophos;i="5.83,273,1616482800"; d="scan'208";a="192969581" Received: from orsmga001.jf.intel.com ([10.7.209.18]) by orsmga101.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 14 Jun 2021 11:08:51 -0700 IronPort-SDR: QMz9h5D4R+M5rWNcbW4LAYtZHDOgB1PVLVWdGUQw1RLWI5jzomHB4u9seA9GkvgLsLDvLqTfmQ DB095xgsO89A== X-IronPort-AV: E=Sophos;i="5.83,273,1616482800"; d="scan'208";a="484166440" Received: from jepowell-mobl1.amr.corp.intel.com (HELO pbossart-mobl3.intel.com) ([10.212.114.106]) by orsmga001-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 14 Jun 2021 11:08:48 -0700 From: Pierre-Louis Bossart To: alsa-devel@alsa-project.org, srinivas.kandagatla@linaro.org Subject: [PATCH 4/5] ASoC: rt5682-sdw: fix race condition on system suspend Date: Mon, 14 Jun 2021 13:08:14 -0500 Message-Id: <20210614180815.153711-5-pierre-louis.bossart@linux.intel.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20210614180815.153711-1-pierre-louis.bossart@linux.intel.com> References: <20210614180815.153711-1-pierre-louis.bossart@linux.intel.com> MIME-Version: 1.0 Cc: Oder Chiou , Jack Yu , open list , Kai Vehmanen , Liam Girdwood , tiwai@suse.de, gregkh@linuxfoundation.org, Takashi Iwai , =?utf-8?q?P=C3=A9ter_Ujfalusi?= , Pierre-Louis Bossart , Hui Wang , vkoul@kernel.org, broonie@kernel.org, Shuming Fan , Bard liao , Rander Wang , Bard Liao X-BeenThere: alsa-devel@alsa-project.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: "Alsa-devel mailing list for ALSA developers - http://www.alsa-project.org" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: alsa-devel-bounces@alsa-project.org Sender: "Alsa-devel" In the initial driver we cancelled deferred work, but there is still a window of time where a new interrupt could result in new deferred work executed after the link is disabled, leading to an IO error. While we did not see this IO error on RT5682-based platforms, the code pattern is similar to the RT700 case where the IO error was noted, so the fix is added for consistency. This patch uses an 'disable_irq_lock' mutex to prevent new interrupts from happening after the start of the system suspend. The choice of a mutex v. a spinlock is mainly due to the time required to clear interrupts, which requires a command to be transmitted by the SoundWire host IP and acknowledged with an interrupt. The 'interrupt_callback' routine is also not meant to be called from an interrupt context. An additional 'disable_irq' flag prevents race conditions where the status changes before the interrupts are disabled, but the workqueue handling status changes is scheduled after the completion of the system suspend. On resume the interrupts are re-enabled already by the io_init routine so we only clear the flag. The Fixes tag points to a 5.10 commit, there's no need to propagate this change to earlier upstream versions. BugLink: https://github.com/thesofproject/linux/issues/2943 Fixes: 4a55000722d7 ('ASoC: codecs: rt*.c: remove useless pointer cast') Signed-off-by: Pierre-Louis Bossart Reviewed-by: Bard Liao Reviewed-by: Péter Ujfalusi --- sound/soc/codecs/rt5682-sdw.c | 38 +++++++++++++++++++++++++++++++++-- sound/soc/codecs/rt5682.h | 2 ++ 2 files changed, 38 insertions(+), 2 deletions(-) diff --git a/sound/soc/codecs/rt5682-sdw.c b/sound/soc/codecs/rt5682-sdw.c index 54873730bec5..31a4f286043e 100644 --- a/sound/soc/codecs/rt5682-sdw.c +++ b/sound/soc/codecs/rt5682-sdw.c @@ -344,6 +344,8 @@ static int rt5682_sdw_init(struct device *dev, struct regmap *regmap, rt5682->sdw_regmap = regmap; rt5682->is_sdw = true; + mutex_init(&rt5682->disable_irq_lock); + rt5682->regmap = devm_regmap_init(dev, NULL, dev, &rt5682_sdw_indirect_regmap); if (IS_ERR(rt5682->regmap)) { @@ -378,6 +380,8 @@ static int rt5682_io_init(struct device *dev, struct sdw_slave *slave) int ret = 0, loop = 10; unsigned int val; + rt5682->disable_irq = false; + if (rt5682->hw_init) return 0; @@ -679,10 +683,12 @@ static int rt5682_interrupt_callback(struct sdw_slave *slave, dev_dbg(&slave->dev, "%s control_port_stat=%x", __func__, status->control_port); - if (status->control_port & 0x4) { + mutex_lock(&rt5682->disable_irq_lock); + if (status->control_port & 0x4 && !rt5682->disable_irq) { mod_delayed_work(system_power_efficient_wq, &rt5682->jack_detect_work, msecs_to_jiffies(rt5682->irq_work_delay_time)); } + mutex_unlock(&rt5682->disable_irq_lock); return 0; } @@ -740,6 +746,34 @@ static int __maybe_unused rt5682_dev_suspend(struct device *dev) return 0; } +static int __maybe_unused rt5682_dev_system_suspend(struct device *dev) +{ + struct rt5682_priv *rt5682 = dev_get_drvdata(dev); + struct sdw_slave *slave = dev_to_sdw_dev(dev); + int ret; + + if (!rt5682->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(&rt5682->disable_irq_lock); + rt5682->disable_irq = true; + ret = sdw_update_no_pm(slave, SDW_SCP_INTMASK1, + SDW_SCP_INT1_IMPL_DEF, 0); + mutex_unlock(&rt5682->disable_irq_lock); + + if (ret < 0) { + /* log but don't prevent suspend from happening */ + dev_dbg(&slave->dev, "%s: could not disable imp-def interrupts\n:", __func__); + } + + return rt5682_dev_suspend(dev); +} + static int __maybe_unused rt5682_dev_resume(struct device *dev) { struct sdw_slave *slave = dev_to_sdw_dev(dev); @@ -768,7 +802,7 @@ static int __maybe_unused rt5682_dev_resume(struct device *dev) } static const struct dev_pm_ops rt5682_pm = { - SET_SYSTEM_SLEEP_PM_OPS(rt5682_dev_suspend, rt5682_dev_resume) + SET_SYSTEM_SLEEP_PM_OPS(rt5682_dev_system_suspend, rt5682_dev_resume) SET_RUNTIME_PM_OPS(rt5682_dev_suspend, rt5682_dev_resume, NULL) }; diff --git a/sound/soc/codecs/rt5682.h b/sound/soc/codecs/rt5682.h index 74ff66767016..b59221048ebf 100644 --- a/sound/soc/codecs/rt5682.h +++ b/sound/soc/codecs/rt5682.h @@ -1415,6 +1415,8 @@ struct rt5682_priv { struct regulator_bulk_data supplies[RT5682_NUM_SUPPLIES]; struct delayed_work jack_detect_work; struct delayed_work jd_check_work; + struct mutex disable_irq_lock; /* imp-def irq lock protection */ + bool disable_irq; struct mutex calibrate_mutex; struct sdw_slave *slave; enum sdw_slave_status status; From patchwork Mon Jun 14 18:08:15 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Pierre-Louis Bossart X-Patchwork-Id: 460580 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-21.8 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID, HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_CR_TRAILER, INCLUDES_PATCH, MAILING_LIST_MULTI, MENTIONS_GIT_HOSTING, SPF_HELO_NONE, SPF_PASS, URIBL_BLOCKED, USER_AGENT_GIT autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id E4CA6C2B9F4 for ; Mon, 14 Jun 2021 18:11:33 +0000 (UTC) Received: from alsa0.perex.cz (alsa0.perex.cz [77.48.224.243]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 2FC8E601FA for ; Mon, 14 Jun 2021 18:11:33 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 2FC8E601FA Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=linux.intel.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=alsa-devel-bounces@alsa-project.org Received: from alsa1.perex.cz (alsa1.perex.cz [207.180.221.201]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by alsa0.perex.cz (Postfix) with ESMTPS id B92CC169F; Mon, 14 Jun 2021 20:10:41 +0200 (CEST) DKIM-Filter: OpenDKIM Filter v2.11.0 alsa0.perex.cz B92CC169F DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=alsa-project.org; s=default; t=1623694291; bh=iYGxxyJB55rn+lQoJh0OYRVBT6C8JemB9bKK+SSuP/Y=; h=From:To:Subject:Date:In-Reply-To:References:Cc:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From; b=tsW1B53sCmtoq94OQOE+hTa/wqm5N1Tm1eitr4Bb97p1agCPWM2mLYVmL3NIDlAd5 BLB39fk2MXZ5jSXdw1WfCt8TfQNidKfMhREAlqi9Xp/qrL1gO+Zo/lU0FoON2fvh15 07kZaNTQTtZ1g6nOQIrzLjM951cMTRh6S22s/yKY= Received: from alsa1.perex.cz (localhost.localdomain [127.0.0.1]) by alsa1.perex.cz (Postfix) with ESMTP id 6621AF804DA; Mon, 14 Jun 2021 20:09:05 +0200 (CEST) Received: by alsa1.perex.cz (Postfix, from userid 50401) id 2767AF800F7; Mon, 14 Jun 2021 20:09:00 +0200 (CEST) Received: from mga02.intel.com (mga02.intel.com [134.134.136.20]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by alsa1.perex.cz (Postfix) with ESMTPS id B35E0F802E8 for ; Mon, 14 Jun 2021 20:08:55 +0200 (CEST) DKIM-Filter: OpenDKIM Filter v2.11.0 alsa1.perex.cz B35E0F802E8 IronPort-SDR: paBzcVBJg7en1eDsEEkZ0KSo1THwq05nB/zu0CkOmxCxTfT5D1C0SlfJ/USXJsJ5Ik6qn3T/Z2 5wBBsIeioBhQ== X-IronPort-AV: E=McAfee;i="6200,9189,10015"; a="192969583" X-IronPort-AV: E=Sophos;i="5.83,273,1616482800"; d="scan'208";a="192969583" Received: from orsmga001.jf.intel.com ([10.7.209.18]) by orsmga101.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 14 Jun 2021 11:08:52 -0700 IronPort-SDR: RJRfPtptmK6kRzPKR/K8i/PT2mO9BtG8B0QE/TjD1OcAsiNysNYNTgSpppUZyVmQO0WWcnCpkF Cnd43xGSP3Gg== X-IronPort-AV: E=Sophos;i="5.83,273,1616482800"; d="scan'208";a="484166446" Received: from jepowell-mobl1.amr.corp.intel.com (HELO pbossart-mobl3.intel.com) ([10.212.114.106]) by orsmga001-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 14 Jun 2021 11:08:50 -0700 From: Pierre-Louis Bossart To: alsa-devel@alsa-project.org, srinivas.kandagatla@linaro.org Subject: [PATCH 5/5] ASoC: rt711-sdca-sdw: fix race condition on system suspend Date: Mon, 14 Jun 2021 13:08:15 -0500 Message-Id: <20210614180815.153711-6-pierre-louis.bossart@linux.intel.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20210614180815.153711-1-pierre-louis.bossart@linux.intel.com> References: <20210614180815.153711-1-pierre-louis.bossart@linux.intel.com> MIME-Version: 1.0 Cc: Oder Chiou , Jack Yu , open list , Liam Girdwood , tiwai@suse.de, gregkh@linuxfoundation.org, Takashi Iwai , =?utf-8?q?P=C3=A9ter_Ujfalusi?= , Pierre-Louis Bossart , Hui Wang , vkoul@kernel.org, broonie@kernel.org, Shuming Fan , Bard liao , Rander Wang , Bard Liao X-BeenThere: alsa-devel@alsa-project.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: "Alsa-devel mailing list for ALSA developers - http://www.alsa-project.org" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: alsa-devel-bounces@alsa-project.org Sender: "Alsa-devel" In the initial driver we cancelled deferred work, but there is still a window of time where a new interrupt could result in new deferred work executed after the link is disabled, leading to an IO error. While we did not see this IO error on RT711-sdca-based platforms, the code pattern is similar to the RT700 case where the IO error was noted, so the fix is added for consistency. This patch uses an 'disable_irq_lock' mutex to prevent new interrupts from happening after the start of the system suspend. The choice of a mutex v. a spinlock is mainly due to the time required to clear interrupts, which requires a command to be transmitted by the SoundWire host IP and acknowledged with an interrupt. The 'interrupt_callback' routine is also not meant to be called from an interrupt context. An additional 'disable_irq' flag prevents race conditions where the status changes before the interrupts are disabled, but the workqueue handling status changes is scheduled after the completion of the system suspend. On resume the interrupts are re-enabled already by the io_init routine so we only clear the flag. The code is slightly different from the other codecs since the interrupt callback deals with the SDCA interrupts, leading to a much larger section that's protected by the mutex. The SoundWire interrupt scheme requires a read after clearing a status, it's not clear from the specifications what would happen if SDCA interrupts are disabled in the middle of the sequence, so the entire interrupt status read/write is kept as is, even if in the end we discard the information. BugLink: https://github.com/thesofproject/linux/issues/2943 Fixes: 7ad4d237e7c4 ('ASoC: rt711-sdca: Add RT711 SDCA vendor-specific driver') Signed-off-by: Pierre-Louis Bossart Reviewed-by: Bard Liao Reviewed-by: Péter Ujfalusi --- sound/soc/codecs/rt711-sdca-sdw.c | 46 +++++++++++++++++++++++++++++-- sound/soc/codecs/rt711-sdca.c | 4 +++ sound/soc/codecs/rt711-sdca.h | 2 ++ 3 files changed, 50 insertions(+), 2 deletions(-) diff --git a/sound/soc/codecs/rt711-sdca-sdw.c b/sound/soc/codecs/rt711-sdca-sdw.c index 03cd3e0142f9..aaf5af153d3f 100644 --- a/sound/soc/codecs/rt711-sdca-sdw.c +++ b/sound/soc/codecs/rt711-sdca-sdw.c @@ -256,6 +256,15 @@ static int rt711_sdca_interrupt_callback(struct sdw_slave *slave, scp_sdca_stat2 = rt711->scp_sdca_stat2; } + /* + * 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; @@ -314,13 +323,16 @@ static int rt711_sdca_interrupt_callback(struct sdw_slave *slave, "%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) + 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; } @@ -382,6 +394,36 @@ static int __maybe_unused rt711_sdca_dev_suspend(struct device *dev) return 0; } +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; + + 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) { + /* log but don't prevent suspend from happening */ + dev_dbg(&slave->dev, "%s: could not disable SDCA interrupts\n:", __func__); + } + + return rt711_sdca_dev_suspend(dev); +} + #define RT711_PROBE_TIMEOUT 5000 static int __maybe_unused rt711_sdca_dev_resume(struct device *dev) @@ -413,7 +455,7 @@ static int __maybe_unused rt711_sdca_dev_resume(struct device *dev) } static const struct dev_pm_ops rt711_sdca_pm = { - SET_SYSTEM_SLEEP_PM_OPS(rt711_sdca_dev_suspend, rt711_sdca_dev_resume) + SET_SYSTEM_SLEEP_PM_OPS(rt711_sdca_dev_system_suspend, rt711_sdca_dev_resume) SET_RUNTIME_PM_OPS(rt711_sdca_dev_suspend, rt711_sdca_dev_resume, NULL) }; diff --git a/sound/soc/codecs/rt711-sdca.c b/sound/soc/codecs/rt711-sdca.c index 0b0c230dcf71..2e992589f1e4 100644 --- a/sound/soc/codecs/rt711-sdca.c +++ b/sound/soc/codecs/rt711-sdca.c @@ -1411,6 +1411,8 @@ int rt711_sdca_init(struct device *dev, struct regmap *regmap, rt711->regmap = regmap; rt711->mbq_regmap = mbq_regmap; + mutex_init(&rt711->disable_irq_lock); + /* * Mark hw_init to false * HW init will be performed when device reports present @@ -1494,6 +1496,8 @@ 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; diff --git a/sound/soc/codecs/rt711-sdca.h b/sound/soc/codecs/rt711-sdca.h index 43ae82b7fdb3..498ca687c47b 100644 --- a/sound/soc/codecs/rt711-sdca.h +++ b/sound/soc/codecs/rt711-sdca.h @@ -27,6 +27,8 @@ 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;