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 Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id E7C85C77B73 for ; Tue, 6 Jun 2023 19:25:24 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References: Message-ID:Subject:Cc:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=ITCde/xpAP+DZ9cF4bEkA+VM2lc7QFcv9ec7yDaTVmY=; b=Dz6LSzEMh3A5fL B6vonHW11YJp1dQ5sAYhw2OBHUMURpkRudoEPw3bFqgYKej0IX/sLdetSzbwP01B36XIklvd5KHfM 2BosO+nvNPQy3Gz63mEHjvWPW7dZyez+ZSOOLwzi8JDP3XOIxIJzE+OKMeB2hhdQeAX0tglEzvLfF XaQ54l2mJK7CCyT3QQeD/SDzmz/762wpH4QEPxpnc0iI9sdnR4o9tcgebCs+5029ccSII34BfMChI Sqz1nD+3YHDGgDy8usayQsc2IglO+PAGUQRLwx8fX6/XPxf29ftAIELE3tqa0KF5XHXHEkOsQGvej zQeSAn3WXc27x6PgjGtA==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.96 #2 (Red Hat Linux)) id 1q6cIf-002wox-2U; Tue, 06 Jun 2023 19:25:01 +0000 Received: from dfw.source.kernel.org ([2604:1380:4641:c500::1]) by bombadil.infradead.org with esmtps (Exim 4.96 #2 (Red Hat Linux)) id 1q6cIc-002woB-3A for linux-arm-kernel@lists.infradead.org; Tue, 06 Jun 2023 19:25:00 +0000 Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dfw.source.kernel.org (Postfix) with ESMTPS id 189776375E; Tue, 6 Jun 2023 19:24:57 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id CD4FFC433D2; Tue, 6 Jun 2023 19:24:55 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1686079496; bh=7IVqUea4CeAvBJ0YJiBaoz5mK2aLV5XBqtStm4tszvU=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=SebT49CAW+GApBH75WCqelppL8SJDA+b9FdjSRbsLAr1MGNCsf83PKrNzprrXB2ag ErGKWPOhc478ioO+28QAKv8MzYfq0OPmAzaVAByZzdA58E5zbO1Nu9XJRZ+vhk4Vbp byQamSnPhawNZczIhIFX6tF1+tfKWBFm8WBYI0sOfnaLQfzrkUBCsqafBm67zbf5G2 K7caYi/olKBzb53/uRCBOmCoGCLf9AuLZbU1cFWLEZIYMJIFVdeKnoo9Rg99r9btjg HTPNlnkSaZWXXY/sGmTEWImLmEaIX0SSSsEo+ISrDnC5PIYKhMTHUcGt0ndkbV5hWT 94oj4Z2y74JnA== Date: Tue, 6 Jun 2023 21:24:53 +0200 From: Andi Shyti To: Robert Hancock Cc: Michal Simek , Shubhrajyoti Datta , Wolfram Sang , Marek Vasut , linux-arm-kernel@lists.infradead.org, linux-i2c@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH] i2c: xiic: Don't try to handle more interrupt events after error Message-ID: <20230606192453.zjzz4kt76kus5hr5@intel.intel> References: <20230606182558.1301413-1-robert.hancock@calian.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20230606182558.1301413-1-robert.hancock@calian.com> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20230606_122459_084743_02C699BB X-CRM114-Status: GOOD ( 28.53 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org Hi Robert, On Tue, Jun 06, 2023 at 12:25:58PM -0600, Robert Hancock wrote: > In xiic_process, it is possible that error events such as arbitration > lost or TX error can be raised in conjunction with other interrupt flags > such as TX FIFO empty or bus not busy. Error events result in the > controller being reset and the error returned to the calling request, > but the function could potentially try to keep handling the other > events, such as by writing more messages into the TX FIFO. Since the > transaction has already failed, this is not helpful and will just cause > issues. what kind of issues? > This problem has been present ever since: > > commit 7f9906bd7f72 ("i2c: xiic: Service all interrupts in isr") > > which allowed non-error events to be handled after errors, but became > more obvious after: > > commit 743e227a8959 ("i2c: xiic: Defer xiic_wakeup() and > __xiic_start_xfer() in xiic_process()") > > which reworked the code to add a WARN_ON which triggers if both the > xfer_more and wakeup_req flags were set, since this combination is > not supposed to happen, but was occurring in this scenario. > > Skip further interrupt handling after error flags are detected to avoid > this problem. > > Fixes: 7f9906bd7f72 ("i2c: xiic: Service all interrupts in isr") > Signed-off-by: Robert Hancock please add: Cc: # v4.3+ > --- > drivers/i2c/busses/i2c-xiic.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/i2c/busses/i2c-xiic.c b/drivers/i2c/busses/i2c-xiic.c > index 8a3d9817cb41..ee6edc963dea 100644 > --- a/drivers/i2c/busses/i2c-xiic.c > +++ b/drivers/i2c/busses/i2c-xiic.c > @@ -721,6 +721,8 @@ static irqreturn_t xiic_process(int irq, void *dev_id) > wakeup_req = 1; > wakeup_code = STATE_ERROR; > } > + /* don't try to handle other events */ > + goto out; why don't we have goto's after every irq evaluation but only here? Do the issues you mentioned happen olny in this particular error case? Thanks, Andi > } > if (pend & XIIC_INTR_RX_FULL_MASK) { > /* Receive register/FIFO is full */ > -- > 2.40.1 > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel