From mboxrd@z Thu Jan 1 00:00:00 1970 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=-15.0 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,INCLUDES_PATCH,MAILING_LIST_MULTI,MENTIONS_GIT_HOSTING, SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED 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 A5E29C433DF for ; Wed, 19 Aug 2020 09:07:48 +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 207CC2072D for ; Wed, 19 Aug 2020 09:07:48 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (1024-bit key) header.d=alsa-project.org header.i=@alsa-project.org header.b="e97drPic"; dkim=fail reason="signature verification failed" (1024-bit key) header.d=kernel.org header.i=@kernel.org header.b="lMW0n5iy" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 207CC2072D Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=kernel.org 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 8BF601800; Wed, 19 Aug 2020 11:06:56 +0200 (CEST) DKIM-Filter: OpenDKIM Filter v2.11.0 alsa0.perex.cz 8BF601800 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=alsa-project.org; s=default; t=1597828066; bh=+kIGh7xNBLq4Soui5QZSxxZW3BSSpRBV2ViZULaNly4=; h=Date:From:To:Subject:References:In-Reply-To:Cc:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From; b=e97drPickW/wiBtwGzDqOO5kQLZRuEOBwR8o/eCMb4k/Aeum1YudlsGpM3GbMXH+i W+cTHTxsteLQhqqrGmCP8tY+TtYweil7yTZ6CqJ3F0cafVbUy7NpRF5fNCCGaB1ObH Y+JmCJjg6KQYT7xGYS4ZtX+TPv3c42YB8mNprwaU= Received: from alsa1.perex.cz (localhost.localdomain [127.0.0.1]) by alsa1.perex.cz (Postfix) with ESMTP id 142B3F80216; Wed, 19 Aug 2020 11:06:56 +0200 (CEST) Received: by alsa1.perex.cz (Postfix, from userid 50401) id 60192F80218; Wed, 19 Aug 2020 11:06:55 +0200 (CEST) Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by alsa1.perex.cz (Postfix) with ESMTPS id 41FA0F80114 for ; Wed, 19 Aug 2020 11:06:44 +0200 (CEST) DKIM-Filter: OpenDKIM Filter v2.11.0 alsa1.perex.cz 41FA0F80114 Authentication-Results: alsa1.perex.cz; dkim=pass (1024-bit key) header.d=kernel.org header.i=@kernel.org header.b="lMW0n5iy" Received: from localhost (unknown [122.171.38.130]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id 402C02072D; Wed, 19 Aug 2020 09:06:40 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1597828002; bh=+kIGh7xNBLq4Soui5QZSxxZW3BSSpRBV2ViZULaNly4=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=lMW0n5iy8gqO/oeV3/AQHudp+ujLkQsnrje1maisElRLR1vFN2nGaPgkfP2/2S63f HsDwTO6PMmcB4QBY0dZXybGjOXPfdYarCusMqw/k9x+vlDkVjKDuXdk1fsy88Ax6pF UnKKYkXVmH2B72+FnyavT0pdHRvDq3gGDzI4SSYc= Date: Wed, 19 Aug 2020 14:36:37 +0530 From: Vinod Koul To: Bard Liao Subject: Re: [PATCH] soundwire: cadence: fix race condition between suspend and Slave device alerts Message-ID: <20200819090637.GE2639@vkoul-mobl> References: <20200817222340.18042-1-yung-chuan.liao@linux.intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20200817222340.18042-1-yung-chuan.liao@linux.intel.com> Cc: pierre-louis.bossart@linux.intel.com, alsa-devel@alsa-project.org, tiwai@suse.de, gregkh@linuxfoundation.org, linux-kernel@vger.kernel.org, ranjani.sridharan@linux.intel.com, hui.wang@canonical.com, broonie@kernel.org, srinivas.kandagatla@linaro.org, jank@cadence.com, mengdong.lin@intel.com, sanyog.r.kale@intel.com, rander.wang@linux.intel.com, bard.liao@intel.com 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" On 18-08-20, 06:23, Bard Liao wrote: > From: Pierre-Louis Bossart > > In system suspend stress cases, the SOF CI reports timeouts. The root > cause is that an alert is generated while the system suspends. The > interrupt handling generates transactions on the bus that will never > be handled because the interrupts are disabled in parallel. > > As a result, the transaction never completes and times out on resume. > This error doesn't seem too problematic since it happens in a work > queue, and the system recovers without issues. > > Nevertheless, this race condition should not happen. When doing a > system suspend, or when disabling interrupts, we should make sure the > current transaction can complete, and prevent new work from being > queued. > > BugLink: https://github.com/thesofproject/linux/issues/2344 > Signed-off-by: Pierre-Louis Bossart > Reviewed-by: Ranjani Sridharan > Reviewed-by: Rander Wang > Signed-off-by: Bard Liao > --- > drivers/soundwire/cadence_master.c | 24 +++++++++++++++++++++++- > drivers/soundwire/cadence_master.h | 1 + > 2 files changed, 24 insertions(+), 1 deletion(-) > > diff --git a/drivers/soundwire/cadence_master.c b/drivers/soundwire/cadence_master.c > index 24eafe0aa1c3..1330ffc47596 100644 > --- a/drivers/soundwire/cadence_master.c > +++ b/drivers/soundwire/cadence_master.c > @@ -791,7 +791,16 @@ irqreturn_t sdw_cdns_irq(int irq, void *dev_id) > CDNS_MCP_INT_SLAVE_MASK, 0); > > int_status &= ~CDNS_MCP_INT_SLAVE_MASK; > - schedule_work(&cdns->work); > + > + /* > + * Deal with possible race condition between interrupt > + * handling and disabling interrupts on suspend. > + * > + * If the master is in the process of disabling > + * interrupts, don't schedule a workqueue > + */ > + if (cdns->interrupt_enabled) > + schedule_work(&cdns->work); would it not make sense to mask the interrupts first and then cancel the work? that way you are guaranteed that after this call you dont have interrupts and work scheduled? > } > > cdns_writel(cdns, CDNS_MCP_INTSTAT, int_status); > @@ -924,6 +933,19 @@ int sdw_cdns_enable_interrupt(struct sdw_cdns *cdns, bool state) > slave_state = cdns_readl(cdns, CDNS_MCP_SLAVE_INTSTAT1); > cdns_writel(cdns, CDNS_MCP_SLAVE_INTSTAT1, slave_state); > } > + cdns->interrupt_enabled = state; > + > + /* > + * Complete any on-going status updates before updating masks, > + * and cancel queued status updates. > + * > + * There could be a race with a new interrupt thrown before > + * the 3 mask updates below are complete, so in the interrupt > + * we use the 'interrupt_enabled' status to prevent new work > + * from being queued. > + */ > + if (!state) > + cancel_work_sync(&cdns->work); > > cdns_writel(cdns, CDNS_MCP_SLAVE_INTMASK0, slave_intmask0); > cdns_writel(cdns, CDNS_MCP_SLAVE_INTMASK1, slave_intmask1); > diff --git a/drivers/soundwire/cadence_master.h b/drivers/soundwire/cadence_master.h > index fdec62b912d3..4d1aab5b5ec2 100644 > --- a/drivers/soundwire/cadence_master.h > +++ b/drivers/soundwire/cadence_master.h > @@ -133,6 +133,7 @@ struct sdw_cdns { > > bool link_up; > unsigned int msg_count; > + bool interrupt_enabled; > > struct work_struct work; > > -- > 2.17.1 -- ~Vinod