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=-2.6 required=3.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS, USER_AGENT_SANE_1 autolearn=no 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 CEC70C2BB1D for ; Tue, 17 Mar 2020 16:44:21 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id A6BDD20724 for ; Tue, 17 Mar 2020 16:44:21 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1584463461; bh=Um8AdnpibV8I7KSh87rkQ29Gpm/yufZOAQ2aQQvQ2U8=; h=Date:From:To:Cc:Subject:References:In-Reply-To:List-ID:From; b=VSiviYLivhwy2RoLmdn6v83AgBlW8v3B+cvWjnzllKPvdnBoEBbWDLnfdd92rCs8I kSBtUOuBrQnavXjsPnAm2VLQ9pUQ3dffyixm5kMg2PuVAsyyIhKD2hphv4QaZaWrAS AjqQPPGqVx0tyH8n08KyT/UMSR6M/JURPXRoo11I= Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726550AbgCQQoU (ORCPT ); Tue, 17 Mar 2020 12:44:20 -0400 Received: from foss.arm.com ([217.140.110.172]:40440 "EHLO foss.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726121AbgCQQoU (ORCPT ); Tue, 17 Mar 2020 12:44:20 -0400 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id AC80EFEC; Tue, 17 Mar 2020 09:44:19 -0700 (PDT) Received: from localhost (unknown [10.37.6.21]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 315193F52E; Tue, 17 Mar 2020 09:44:19 -0700 (PDT) Date: Tue, 17 Mar 2020 16:44:17 +0000 From: Mark Brown To: Doug Anderson Cc: Alok Chauhan , Dilip Kota , skakit@codeaurora.org, Girish Mahadevan , Andy Gross , Bjorn Andersson , Stephen Boyd , linux-arm-msm , LKML , linux-spi Subject: Re: [PATCH] spi: spi-geni-qcom: Speculative fix of "nobody cared" about interrupt Message-ID: <20200317164417.GJ3971@sirena.org.uk> References: <20200316151939.1.I752ebdcfd5e8bf0de06d66e767b8974932b3620e@changeid> <20200317121018.GB3971@sirena.org.uk> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="HSQ3hISbU3Um6hch" Content-Disposition: inline In-Reply-To: X-Cookie: There's only one everything. User-Agent: Mutt/1.10.1 (2018-07-13) Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org --HSQ3hISbU3Um6hch Content-Type: text/plain; charset=us-ascii Content-Disposition: inline On Tue, Mar 17, 2020 at 08:12:30AM -0700, Doug Anderson wrote: > On Tue, Mar 17, 2020 at 5:10 AM Mark Brown wrote: > > On Mon, Mar 16, 2020 at 03:20:01PM -0700, Douglas Anderson wrote: > > > > Does this mean that there was an actual concrete message of type > > CMD_NONE or does it mean that there was no message waiting? If there > > was no message then isn't the interrupt spurious? > There is no message of type "CMD_NONE". The "cur_mcmd" field is > basically where in the software state machine we're at: ... > ...so certainly if we see "cur_mcmd == CMD_NONE" in the interrupt > handler we're in an unexpected situation. We don't expect interrupts > while idle. I wouldn't necessarily say it was a spurious interrupt, > though. To say that I'd rather look at the result of this line in the > IRQ handler: > m_irq = readl(se->base + SE_GENI_M_IRQ_STATUS); > ...if that line returns 0 then I would be willing to say it is a > spurious interrupt. Right, that should return IRQ_NONE if there's nothing actually asserted. I think you're right about the state machine though. > C) Do we care to try to detect spurious interrupts (by checking > SE_GENI_M_IRQ_STATUS) and return IRQ_NONE? Right now a spurious > interrupt will be harmless because all of the logic in geni_spi_isr() > doesn't do anything if SE_GENI_M_IRQ_STATUS has no bits set. ...but > it will still return IRQ_HANDLED. I can't imagine anyone ever putting > this device on a shared interrupt, but if it's important I can detect > this and return IRQ_NONE in this case in a v2 of this patch. It's a good idea to return IRQ_NONE not just for the shared interrupt case but also because it lets the error handling code in the genirq core do it's thing and detect bugs - as seems to have happened here. This one was fairly benign but you can also see things like interrupts that are constantly asserted by the hardware where we end up spinning in interrupt handlers. --HSQ3hISbU3Um6hch Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQEzBAABCgAdFiEEreZoqmdXGLWf4p/qJNaLcl1Uh9AFAl5w/mAACgkQJNaLcl1U h9DXgwf/e1aqKwe3QdhJTo3ZnebAawyLn7xfkYE0amZAh7GQzsbx+MJv7Ar9hAm2 QF8ke5T67rDag/wSDpjuuyWEgRLmmpDg78DZSNoKVgNubIFJago000bwhqkFfJJR T4C5EPMvzEg9rDF7NvM4WI4pNsAeBHH3l0mxA1ZLHs8capjDYvjUpZQFBBwDjT6E blROzd29hLzWlVs44H/3rAVcCpT11ChwSjq4haJ/dBBHe/ObAVWUVjU5rhtP3Uod IHvnBRbWG+iY0g8CWsZSJb9mMklOHacgPMdlnumNiYpgvM4OffyhfXQ1EnNVT2YN 1qbOrLz1MqviNs0f4WL2+MyG57DoXA== =5ZtU -----END PGP SIGNATURE----- --HSQ3hISbU3Um6hch--