Received: by 2002:a25:1506:0:0:0:0:0 with SMTP id 6csp2774136ybv; Sun, 9 Feb 2020 07:07:50 -0800 (PST) X-Google-Smtp-Source: APXvYqyZK08F/d3Y+BO7gUvW+SnlFErxsFUoFwKWIhl2/GBOTCOyVE9+/qkVfZNTQP1n0ZzRSk66 X-Received: by 2002:aca:1204:: with SMTP id 4mr8135875ois.143.1581260870857; Sun, 09 Feb 2020 07:07:50 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1581260870; cv=none; d=google.com; s=arc-20160816; b=OkHKIUT4ktupX01+2fvo1VERtO6zpQps9v+qMfc6kofGTNjAOSi4GZLlfS/rlLVpqi Vaz6vpZpZwn+nYJoYusBpTJ2TUqIzCUHNxRyIG/NxtXBVq+IvZnmbH8kUrlKkuxMNfCm h2F7CpODgi8vSOrdKQxBcQJBpicOVvAdCE4cDwDoj2V0MvOdtE+7RvQclojoDqg8suAZ 0ALoAtIJqJJrYZeIYKdOljosg2PaACrwBqteKBh2fa0FD+MAW1Ws9xiITecDCNdMmPTG juHMIqHEfhoaghO58TaiDTqLQFvVgnsXMD21cZuGnEIND8ia8O+ZlWyRjd+z/O/rzz3i 9ymg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:user-agent:in-reply-to :content-disposition:mime-version:references:message-id:subject:cc :to:from:date; bh=o48PBhnrGXYR/x6nQ2TLJjOxH7i/jRRSSirCVAVYp6Y=; b=PjlgIa3c2jpCDwJmsV2xiza174wEJXdLZ6WWwZDfF6OZ1/ZP0BVpt7yA7cZLjhINEw d52fuNDjy4GbvKFfa5Zt24Sk07icr0T6xpXtU8PRJRZVcVKMRhzDEY1iFMb+LvYGlGk8 82MqqDidSlHyV0cLkTjvWHWg15F+Paq9keRI0bzyY98PZIKaa7ZpA6jaSaGnXIOUn4zu rzLwRjcZnGNfRLh14tPm6YmKeOfzfoC4KYYhJLW+RbYnJNPXGCnzmvYouuTQqB7oxvLE JLTzMpAnUmbd8henU/Ey338KAarogQs4JJKGV8ipbz3axbs2LRJgB2JM9vhWt++5Fy4P SVxg== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id h9si3149607otb.49.2020.02.09.07.07.38; Sun, 09 Feb 2020 07:07:50 -0800 (PST) Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67; Authentication-Results: mx.google.com; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727795AbgBIPDb (ORCPT + 99 others); Sun, 9 Feb 2020 10:03:31 -0500 Received: from bmailout1.hostsharing.net ([83.223.95.100]:51627 "EHLO bmailout1.hostsharing.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727473AbgBIPDb (ORCPT ); Sun, 9 Feb 2020 10:03:31 -0500 Received: from h08.hostsharing.net (h08.hostsharing.net [IPv6:2a01:37:1000::53df:5f1c:0]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client CN "*.hostsharing.net", Issuer "COMODO RSA Domain Validation Secure Server CA" (not verified)) by bmailout1.hostsharing.net (Postfix) with ESMTPS id 1BABC3000469E; Sun, 9 Feb 2020 16:03:29 +0100 (CET) Received: by h08.hostsharing.net (Postfix, from userid 100393) id E5810129957; Sun, 9 Feb 2020 16:03:28 +0100 (CET) Date: Sun, 9 Feb 2020 16:03:28 +0100 From: Lukas Wunner To: Stuart Hayes Cc: Bjorn Helgaas , Austin Bolen , Keith Busch , Alexandru Gagniuc , "Rafael J . Wysocki" , Mika Westerberg , Andy Shevchenko , "Gustavo A . R . Silva" , Sinan Kaya , Oza Pawandeep , linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org, narendra_k@dell.com Subject: Re: [PATCH v3] PCI: pciehp: Make sure pciehp_isr clears interrupt events Message-ID: <20200209150328.2x2zumhqbs6fihmc@wunner.de> References: <20200207195450.52026-1-stuart.w.hayes@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20200207195450.52026-1-stuart.w.hayes@gmail.com> User-Agent: NeoMutt/20170113 (1.7.2) Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, Feb 07, 2020 at 02:54:50PM -0500, Stuart Hayes wrote: > +/* > + * Set a limit to how many times the ISR will loop reading and writing the > + * slot status register trying to clear the event bits. These bits should > + * not toggle rapidly, and there are only six possible events that could > + * generate this interrupt. If we still see events after this many reads, > + * there is likely a bit stuck. > + */ > +#define MAX_ISR_STATUS_READS 6 Actually only *three* possible events could generate this interrupt because pcie_enable_notification() only enables DLLSC, CCIE and either of ABP or PDC. > - pcie_capability_write_word(pdev, PCI_EXP_SLTSTA, events); > + if (status) { > + pcie_capability_write_word(pdev, PCI_EXP_SLTSTA, status); Writing "events" instead of "status" would seem to be more advantageous because it reduces the number of loops. Say you read PDC in the first loop iteration, then DLLSC in the second loop iteration and shortly before writing the register, PDC transitions to 1. If you write "events", you can make do with 2 loop iterations, if you write "status" you'll need 3. > + > + /* > + * Unless the MSI happens to be masked, all of the event > + * bits must be zero before the port will send a new > + * interrupt (see PCI Express Base Specification Rev 5.0 > + * Version 1.0, section 6.7.3.4, "Software Notification of > + * Hot-Plug Events"). So, if an event bit gets set between > + * the read and the write of PCI_EXP_SLTSTA, we need to > + * loop back and try again. > + */ > + if (status_reads++ < MAX_ISR_STATUS_READS) > + goto read_status; Please use "pci_dev_msi_enabled(pdev)" as conditional for the if-clause, we don't need this with INTx. Using a for (;;) or do/while loop that you jump out of if (!status || !pci_dev_msi_enabled(pdev)) might be more readable than a goto, but I'm not sure. Thanks, Lukas