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 mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 34DEAC433EF for ; Wed, 17 Nov 2021 23:58:19 +0000 (UTC) Received: from smtp4.osuosl.org (smtp4.osuosl.org [140.211.166.137]) (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 BB31E61BC1 for ; Wed, 17 Nov 2021 23:58:18 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org BB31E61BC1 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=kernel.org Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=lists.linuxfoundation.org Received: from localhost (localhost [127.0.0.1]) by smtp4.osuosl.org (Postfix) with ESMTP id 8578340178; Wed, 17 Nov 2021 23:58:18 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from smtp4.osuosl.org ([127.0.0.1]) by localhost (smtp4.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id GqskZ_3MekSP; Wed, 17 Nov 2021 23:58:17 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by smtp4.osuosl.org (Postfix) with ESMTPS id 92A1A40119; Wed, 17 Nov 2021 23:58:17 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id 69744C002E; Wed, 17 Nov 2021 23:58:17 +0000 (UTC) Received: from smtp1.osuosl.org (smtp1.osuosl.org [140.211.166.138]) by lists.linuxfoundation.org (Postfix) with ESMTP id 98064C0012 for ; Wed, 17 Nov 2021 23:58:15 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by smtp1.osuosl.org (Postfix) with ESMTP id 717A1826E5 for ; Wed, 17 Nov 2021 23:58:15 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Authentication-Results: smtp1.osuosl.org (amavisd-new); dkim=pass (2048-bit key) header.d=kernel.org Received: from smtp1.osuosl.org ([127.0.0.1]) by localhost (smtp1.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id Ffw85ugK4mhL for ; Wed, 17 Nov 2021 23:58:14 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.8.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp1.osuosl.org (Postfix) with ESMTPS id 973A080EFA for ; Wed, 17 Nov 2021 23:58:14 +0000 (UTC) Received: by mail.kernel.org (Postfix) with ESMTPSA id C8D5A61AD2; Wed, 17 Nov 2021 23:58:13 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1637193494; bh=jt1bptChpIJAfJm7hhcvnL0z6T39eDJjHsyLzG8zD1Y=; h=Date:From:To:Cc:Subject:In-Reply-To:From; b=lyKjNjZh0qOCqIMVIpeRoNDwhdUJ/tL20BWpiwNwGttOe0RfGC7KJ4PhUovOify8k CPIwK9uSUQMg7VMxx+ystTmi5eBYFIkTtVD4sE8ulLOvbgkfTXGOpyI5/5ntrQgvl0 CfZ1WfNlECodqxIGcyQGBzURqYPwL89BjB4irtfrZDGz99G0bGiOvyoqAHp04d3fBv aBxC568xp2R7kzM9biPqbt5SZcYkGOgkMTsagR6oWZfjlJq8OfZ4tIYBwT1Dm+Bjb+ xfWKIqeKqVbpwaZ3tBQdETriNpArC0Ads/tlo1ukjZbgXjypi2+h68mtdIpt5Dhoxd 4UKc+KstDfPQQ== Date: Wed, 17 Nov 2021 17:58:12 -0600 From: Bjorn Helgaas To: Naveen Naidu Subject: Re: [PATCH v3 01/25] PCI: Add PCI_ERROR_RESPONSE and it's related definitions Message-ID: <20211117235812.GA1786428@bhelgaas> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: Cc: Krzysztof =?utf-8?Q?Wilczy=C5=84ski?= , linux-hyperv@vger.kernel.org, Heiko Stuebner , linux-pci@vger.kernel.org, Shawn Lin , Binghui Wang , Kuppuswamy Sathyanarayanan , Ryder Lee , Oliver O'Halloran , Thomas Petazzoni , Russell Currey , Toan Le , "K. Y. Srinivasan" , Nirmal Patel , Marek Vasut , Rob Herring , Wei Liu , Lorenzo Pieralisi , Stephen Hemminger , Krzysztof Kozlowski , Marc Zyngier , Dexuan Cui , Jianjun Wang , linux-rockchip@lists.infradead.org, "maintainer:BROADCOM IPROC ARM ARCHITECTURE" , Jonathan Derrick , Xiaowei Song , linux-kernel-mentees@lists.linuxfoundation.org, Matthias Brugger , Robert Richter , Sean V Kelley , Ray Jui , Haiyang Zhang , linux-samsung-soc@vger.kernel.org, linux-mediatek@lists.infradead.org, bhelgaas@google.com, linux-arm-kernel@lists.infradead.org, Qiuxu Zhuo , Pali =?iso-8859-1?Q?Roh=E1r?= , Scott Branden , Jingoo Han , Yoshihiro Shimoda , linux-kernel@vger.kernel.org, linux-renesas-soc@vger.kernel.org, Lukas Wunner , Joyce Ooi , Shawn Guo , linuxppc-dev@lists.ozlabs.org X-BeenThere: linux-kernel-mentees@lists.linuxfoundation.org X-Mailman-Version: 2.1.15 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 Errors-To: linux-kernel-mentees-bounces@lists.linuxfoundation.org Sender: "Linux-kernel-mentees" On Thu, Oct 21, 2021 at 08:37:26PM +0530, Naveen Naidu wrote: > An MMIO read from a PCI device that doesn't exist or doesn't respond > causes a PCI error. There's no real data to return to satisfy the > CPU read, so most hardware fabricates ~0 data. > > Add a PCI_ERROR_RESPONSE definition for that and use it where > appropriate to make these checks consistent and easier to find. > > Also add helper definitions SET_PCI_ERROR_RESPONSE and > RESPONSE_IS_PCI_ERROR to make the code more readable. > > Suggested-by: Bjorn Helgaas > Signed-off-by: Naveen Naidu > --- > include/linux/pci.h | 9 +++++++++ > 1 file changed, 9 insertions(+) > > diff --git a/include/linux/pci.h b/include/linux/pci.h > index cd8aa6fce204..689c8277c584 100644 > --- a/include/linux/pci.h > +++ b/include/linux/pci.h > @@ -154,6 +154,15 @@ enum pci_interrupt_pin { > /* The number of legacy PCI INTx interrupts */ > #define PCI_NUM_INTX 4 > > +/* > + * Reading from a device that doesn't respond typically returns ~0. A > + * successful read from a device may also return ~0, so you need additional > + * information to reliably identify errors. > + */ > +#define PCI_ERROR_RESPONSE (~0ULL) > +#define SET_PCI_ERROR_RESPONSE(val) (*(val) = ((typeof(*(val))) PCI_ERROR_RESPONSE)) > +#define RESPONSE_IS_PCI_ERROR(val) ((val) == ((typeof(val)) PCI_ERROR_RESPONSE)) Beautiful! I really like this. I would prefer the macros to start with "PCI_", e.g., PCI_SET_ERROR_RESPONSE(). I think "RESPONSE_IS_PCI_ERROR()" is too strong because (as the comment says), ~0 *may* indicate an error. Or it may be a successful read of a register that happens to contain ~0. Possibilities to convey the idea that this isn't definitive: PCI_POSSIBLE_ERROR_RESPONSE(val) # a little long PCI_LIKELY_ERROR(val) # we really have no idea whether PCI_PROBABLE_ERROR(val) # likely or probable PCI_POSSIBLE_ERROR(val) # promising? Can you rebase to my "main" branch (v5.16-rc1), tweak the above, and collect up the acks/reviews? We should also browse drivers outside drivers/pci for places we could use these. Not necessarily as part of this series, although if authors there object, it would be good to learn that earlier than later. Drivers that implement pci_error_handlers might be a fruitful place to start. But you've done a great job finding users of ~0 and 0xffff... in drivers/pci/, too. > + > /* > * pci_power_t values must match the bits in the Capabilities PME_Support > * and Control/Status PowerState fields in the Power Management capability. > -- > 2.25.1 > > _______________________________________________ > Linux-kernel-mentees mailing list > Linux-kernel-mentees@lists.linuxfoundation.org > https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees _______________________________________________ Linux-kernel-mentees mailing list Linux-kernel-mentees@lists.linuxfoundation.org https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees 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 mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 22C6FC433FE for ; Wed, 17 Nov 2021 23:58:36 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 0B43561AD2 for ; Wed, 17 Nov 2021 23:58:36 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S241772AbhKRABc (ORCPT ); Wed, 17 Nov 2021 19:01:32 -0500 Received: from mail.kernel.org ([198.145.29.99]:59276 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S241730AbhKRABN (ORCPT ); Wed, 17 Nov 2021 19:01:13 -0500 Received: by mail.kernel.org (Postfix) with ESMTPSA id C8D5A61AD2; Wed, 17 Nov 2021 23:58:13 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1637193494; bh=jt1bptChpIJAfJm7hhcvnL0z6T39eDJjHsyLzG8zD1Y=; h=Date:From:To:Cc:Subject:In-Reply-To:From; b=lyKjNjZh0qOCqIMVIpeRoNDwhdUJ/tL20BWpiwNwGttOe0RfGC7KJ4PhUovOify8k CPIwK9uSUQMg7VMxx+ystTmi5eBYFIkTtVD4sE8ulLOvbgkfTXGOpyI5/5ntrQgvl0 CfZ1WfNlECodqxIGcyQGBzURqYPwL89BjB4irtfrZDGz99G0bGiOvyoqAHp04d3fBv aBxC568xp2R7kzM9biPqbt5SZcYkGOgkMTsagR6oWZfjlJq8OfZ4tIYBwT1Dm+Bjb+ xfWKIqeKqVbpwaZ3tBQdETriNpArC0Ads/tlo1ukjZbgXjypi2+h68mtdIpt5Dhoxd 4UKc+KstDfPQQ== Date: Wed, 17 Nov 2021 17:58:12 -0600 From: Bjorn Helgaas To: Naveen Naidu Cc: bhelgaas@google.com, Krzysztof =?utf-8?Q?Wilczy=C5=84ski?= , linux-hyperv@vger.kernel.org, Heiko Stuebner , linux-pci@vger.kernel.org, Shawn Lin , Binghui Wang , Kuppuswamy Sathyanarayanan , Oliver O'Halloran , Thomas Petazzoni , Russell Currey , Lorenzo Pieralisi , Toan Le , "K. Y. Srinivasan" , Nirmal Patel , Marek Vasut , Rob Herring , Wei Liu , linux-samsung-soc@vger.kernel.org, Marc Zyngier , Joyce Ooi , Dexuan Cui , Jianjun Wang , linux-rockchip@lists.infradead.org, "maintainer:BROADCOM IPROC ARM ARCHITECTURE" , Jonathan Derrick , Xiaowei Song , linux-kernel-mentees@lists.linuxfoundation.org, Robert Richter , Sean V Kelley , Ray Jui , Haiyang Zhang , Ryder Lee , linux-mediatek@lists.infradead.org, Matthias Brugger , Stephen Hemminger , linux-arm-kernel@lists.infradead.org, Qiuxu Zhuo , Scott Branden , linuxppc-dev@lists.ozlabs.org, Yoshihiro Shimoda , Krzysztof Kozlowski , linux-kernel@vger.kernel.org, linux-renesas-soc@vger.kernel.org, Lukas Wunner , Jingoo Han , Shawn Guo , Pali =?iso-8859-1?Q?Roh=E1r?= Subject: Re: [PATCH v3 01/25] PCI: Add PCI_ERROR_RESPONSE and it's related definitions Message-ID: <20211117235812.GA1786428@bhelgaas> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, Oct 21, 2021 at 08:37:26PM +0530, Naveen Naidu wrote: > An MMIO read from a PCI device that doesn't exist or doesn't respond > causes a PCI error. There's no real data to return to satisfy the > CPU read, so most hardware fabricates ~0 data. > > Add a PCI_ERROR_RESPONSE definition for that and use it where > appropriate to make these checks consistent and easier to find. > > Also add helper definitions SET_PCI_ERROR_RESPONSE and > RESPONSE_IS_PCI_ERROR to make the code more readable. > > Suggested-by: Bjorn Helgaas > Signed-off-by: Naveen Naidu > --- > include/linux/pci.h | 9 +++++++++ > 1 file changed, 9 insertions(+) > > diff --git a/include/linux/pci.h b/include/linux/pci.h > index cd8aa6fce204..689c8277c584 100644 > --- a/include/linux/pci.h > +++ b/include/linux/pci.h > @@ -154,6 +154,15 @@ enum pci_interrupt_pin { > /* The number of legacy PCI INTx interrupts */ > #define PCI_NUM_INTX 4 > > +/* > + * Reading from a device that doesn't respond typically returns ~0. A > + * successful read from a device may also return ~0, so you need additional > + * information to reliably identify errors. > + */ > +#define PCI_ERROR_RESPONSE (~0ULL) > +#define SET_PCI_ERROR_RESPONSE(val) (*(val) = ((typeof(*(val))) PCI_ERROR_RESPONSE)) > +#define RESPONSE_IS_PCI_ERROR(val) ((val) == ((typeof(val)) PCI_ERROR_RESPONSE)) Beautiful! I really like this. I would prefer the macros to start with "PCI_", e.g., PCI_SET_ERROR_RESPONSE(). I think "RESPONSE_IS_PCI_ERROR()" is too strong because (as the comment says), ~0 *may* indicate an error. Or it may be a successful read of a register that happens to contain ~0. Possibilities to convey the idea that this isn't definitive: PCI_POSSIBLE_ERROR_RESPONSE(val) # a little long PCI_LIKELY_ERROR(val) # we really have no idea whether PCI_PROBABLE_ERROR(val) # likely or probable PCI_POSSIBLE_ERROR(val) # promising? Can you rebase to my "main" branch (v5.16-rc1), tweak the above, and collect up the acks/reviews? We should also browse drivers outside drivers/pci for places we could use these. Not necessarily as part of this series, although if authors there object, it would be good to learn that earlier than later. Drivers that implement pci_error_handlers might be a fruitful place to start. But you've done a great job finding users of ~0 and 0xffff... in drivers/pci/, too. > + > /* > * pci_power_t values must match the bits in the Capabilities PME_Support > * and Control/Status PowerState fields in the Power Management capability. > -- > 2.25.1 > > _______________________________________________ > Linux-kernel-mentees mailing list > Linux-kernel-mentees@lists.linuxfoundation.org > https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees 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 mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 4BB07C433F5 for ; Wed, 17 Nov 2021 23:58:55 +0000 (UTC) Received: from lists.ozlabs.org (lists.ozlabs.org [112.213.38.117]) (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 B88BA6124B for ; Wed, 17 Nov 2021 23:58:54 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org B88BA6124B Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=kernel.org Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=lists.ozlabs.org Received: from boromir.ozlabs.org (localhost [IPv6:::1]) by lists.ozlabs.org (Postfix) with ESMTP id 4Hvfz12y18z3c5F for ; Thu, 18 Nov 2021 10:58:53 +1100 (AEDT) Authentication-Results: lists.ozlabs.org; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=kernel.org header.i=@kernel.org header.a=rsa-sha256 header.s=k20201202 header.b=lyKjNjZh; dkim-atps=neutral Authentication-Results: lists.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=kernel.org (client-ip=198.145.29.99; helo=mail.kernel.org; envelope-from=helgaas@kernel.org; receiver=) Authentication-Results: lists.ozlabs.org; dkim=pass (2048-bit key; unprotected) header.d=kernel.org header.i=@kernel.org header.a=rsa-sha256 header.s=k20201202 header.b=lyKjNjZh; dkim-atps=neutral Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by lists.ozlabs.org (Postfix) with ESMTPS id 4HvfyK2QQfz2yJM for ; Thu, 18 Nov 2021 10:58:17 +1100 (AEDT) Received: by mail.kernel.org (Postfix) with ESMTPSA id C8D5A61AD2; Wed, 17 Nov 2021 23:58:13 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1637193494; bh=jt1bptChpIJAfJm7hhcvnL0z6T39eDJjHsyLzG8zD1Y=; h=Date:From:To:Cc:Subject:In-Reply-To:From; b=lyKjNjZh0qOCqIMVIpeRoNDwhdUJ/tL20BWpiwNwGttOe0RfGC7KJ4PhUovOify8k CPIwK9uSUQMg7VMxx+ystTmi5eBYFIkTtVD4sE8ulLOvbgkfTXGOpyI5/5ntrQgvl0 CfZ1WfNlECodqxIGcyQGBzURqYPwL89BjB4irtfrZDGz99G0bGiOvyoqAHp04d3fBv aBxC568xp2R7kzM9biPqbt5SZcYkGOgkMTsagR6oWZfjlJq8OfZ4tIYBwT1Dm+Bjb+ xfWKIqeKqVbpwaZ3tBQdETriNpArC0Ads/tlo1ukjZbgXjypi2+h68mtdIpt5Dhoxd 4UKc+KstDfPQQ== Date: Wed, 17 Nov 2021 17:58:12 -0600 From: Bjorn Helgaas To: Naveen Naidu Subject: Re: [PATCH v3 01/25] PCI: Add PCI_ERROR_RESPONSE and it's related definitions Message-ID: <20211117235812.GA1786428@bhelgaas> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-BeenThere: linuxppc-dev@lists.ozlabs.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Linux on PowerPC Developers Mail List List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Krzysztof =?utf-8?Q?Wilczy=C5=84ski?= , linux-hyperv@vger.kernel.org, Heiko Stuebner , linux-pci@vger.kernel.org, Shawn Lin , Binghui Wang , Kuppuswamy Sathyanarayanan , Ryder Lee , Oliver O'Halloran , Thomas Petazzoni , Toan Le , "K. Y. Srinivasan" , Nirmal Patel , Marek Vasut , Rob Herring , Wei Liu , Lorenzo Pieralisi , Stephen Hemminger , Krzysztof Kozlowski , Marc Zyngier , Dexuan Cui , Jianjun Wang , linux-rockchip@lists.infradead.org, "maintainer:BROADCOM IPROC ARM ARCHITECTURE" , Jonathan Derrick , Xiaowei Song , linux-kernel-mentees@lists.linuxfoundation.org, Matthias Brugger , Robert Richter , Sean V Kelley , Ray Jui , Haiyang Zhang , linux-samsung-soc@vger.kernel.org, linux-mediatek@lists.infradead.org, bhelgaas@google.com, linux-arm-kernel@lists.infradead.org, Qiuxu Zhuo , Pali =?iso-8859-1?Q?Roh=E1r?= , Scott Branden , Jingoo Han , Yoshihiro Shimoda , linux-kernel@vger.kernel.org, linux-renesas-soc@vger.kernel.org, Lukas Wunner , Joyce Ooi , Shawn Guo , linuxppc-dev@lists.ozlabs.org Errors-To: linuxppc-dev-bounces+linuxppc-dev=archiver.kernel.org@lists.ozlabs.org Sender: "Linuxppc-dev" On Thu, Oct 21, 2021 at 08:37:26PM +0530, Naveen Naidu wrote: > An MMIO read from a PCI device that doesn't exist or doesn't respond > causes a PCI error. There's no real data to return to satisfy the > CPU read, so most hardware fabricates ~0 data. > > Add a PCI_ERROR_RESPONSE definition for that and use it where > appropriate to make these checks consistent and easier to find. > > Also add helper definitions SET_PCI_ERROR_RESPONSE and > RESPONSE_IS_PCI_ERROR to make the code more readable. > > Suggested-by: Bjorn Helgaas > Signed-off-by: Naveen Naidu > --- > include/linux/pci.h | 9 +++++++++ > 1 file changed, 9 insertions(+) > > diff --git a/include/linux/pci.h b/include/linux/pci.h > index cd8aa6fce204..689c8277c584 100644 > --- a/include/linux/pci.h > +++ b/include/linux/pci.h > @@ -154,6 +154,15 @@ enum pci_interrupt_pin { > /* The number of legacy PCI INTx interrupts */ > #define PCI_NUM_INTX 4 > > +/* > + * Reading from a device that doesn't respond typically returns ~0. A > + * successful read from a device may also return ~0, so you need additional > + * information to reliably identify errors. > + */ > +#define PCI_ERROR_RESPONSE (~0ULL) > +#define SET_PCI_ERROR_RESPONSE(val) (*(val) = ((typeof(*(val))) PCI_ERROR_RESPONSE)) > +#define RESPONSE_IS_PCI_ERROR(val) ((val) == ((typeof(val)) PCI_ERROR_RESPONSE)) Beautiful! I really like this. I would prefer the macros to start with "PCI_", e.g., PCI_SET_ERROR_RESPONSE(). I think "RESPONSE_IS_PCI_ERROR()" is too strong because (as the comment says), ~0 *may* indicate an error. Or it may be a successful read of a register that happens to contain ~0. Possibilities to convey the idea that this isn't definitive: PCI_POSSIBLE_ERROR_RESPONSE(val) # a little long PCI_LIKELY_ERROR(val) # we really have no idea whether PCI_PROBABLE_ERROR(val) # likely or probable PCI_POSSIBLE_ERROR(val) # promising? Can you rebase to my "main" branch (v5.16-rc1), tweak the above, and collect up the acks/reviews? We should also browse drivers outside drivers/pci for places we could use these. Not necessarily as part of this series, although if authors there object, it would be good to learn that earlier than later. Drivers that implement pci_error_handlers might be a fruitful place to start. But you've done a great job finding users of ~0 and 0xffff... in drivers/pci/, too. > + > /* > * pci_power_t values must match the bits in the Capabilities PME_Support > * and Control/Status PowerState fields in the Power Management capability. > -- > 2.25.1 > > _______________________________________________ > Linux-kernel-mentees mailing list > Linux-kernel-mentees@lists.linuxfoundation.org > https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees 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 mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id A57CBC433EF for ; Wed, 17 Nov 2021 23:58:33 +0000 (UTC) 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 mail.kernel.org (Postfix) with ESMTPS id 704DA61AD2 for ; Wed, 17 Nov 2021 23:58:33 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org 704DA61AD2 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=kernel.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=lists.infradead.org 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: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:References: List-Owner; bh=nKoT6yw9cxDLoFwEBOR3fDLpVPs2iRfpkfSJaHAPDtQ=; b=dkOKF2G7IoFsVd JGOAzDkOFe2ZEsRh/SQc6YsiuiIq3oOHjFnkKItYtTYwvZ4LM6AL0YCXVHHfPNYKMDFDFIokTTosn P/2CvEHH3G5kB327ES8iAIOo/kWeFitnMYHNEeNHo6LmaSWfMLc2ioYam+/lhka/z1042YAKSY8bq d60czFXLO7V9XdUWGHHg513DQwj4HUUd9K8QSeZ2L+LLAfS5doTIsc9PXgjO7Fn/cqXgGgGZZ1EDe JGe/x+ylco1mH4ZkQMHMoL3UGOtx21uD1gzNY7w8OK4cj2rKSpnpEbUdxUVbWQBLSNUAUCK1zdPoS y31OykujGtjY/phtMpTA==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1mnUov-006cBJ-Gz; Wed, 17 Nov 2021 23:58:29 +0000 Received: from mail.kernel.org ([198.145.29.99]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1mnUog-006c86-UK; Wed, 17 Nov 2021 23:58:16 +0000 Received: by mail.kernel.org (Postfix) with ESMTPSA id C8D5A61AD2; Wed, 17 Nov 2021 23:58:13 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1637193494; bh=jt1bptChpIJAfJm7hhcvnL0z6T39eDJjHsyLzG8zD1Y=; h=Date:From:To:Cc:Subject:In-Reply-To:From; b=lyKjNjZh0qOCqIMVIpeRoNDwhdUJ/tL20BWpiwNwGttOe0RfGC7KJ4PhUovOify8k CPIwK9uSUQMg7VMxx+ystTmi5eBYFIkTtVD4sE8ulLOvbgkfTXGOpyI5/5ntrQgvl0 CfZ1WfNlECodqxIGcyQGBzURqYPwL89BjB4irtfrZDGz99G0bGiOvyoqAHp04d3fBv aBxC568xp2R7kzM9biPqbt5SZcYkGOgkMTsagR6oWZfjlJq8OfZ4tIYBwT1Dm+Bjb+ xfWKIqeKqVbpwaZ3tBQdETriNpArC0Ads/tlo1ukjZbgXjypi2+h68mtdIpt5Dhoxd 4UKc+KstDfPQQ== Date: Wed, 17 Nov 2021 17:58:12 -0600 From: Bjorn Helgaas To: Naveen Naidu Cc: bhelgaas@google.com, Krzysztof =?utf-8?Q?Wilczy=C5=84ski?= , linux-hyperv@vger.kernel.org, Heiko Stuebner , linux-pci@vger.kernel.org, Shawn Lin , Binghui Wang , Kuppuswamy Sathyanarayanan , Oliver O'Halloran , Thomas Petazzoni , Russell Currey , Lorenzo Pieralisi , Toan Le , "K. Y. Srinivasan" , Nirmal Patel , Marek Vasut , Rob Herring , Wei Liu , linux-samsung-soc@vger.kernel.org, Marc Zyngier , Joyce Ooi , Dexuan Cui , Jianjun Wang , linux-rockchip@lists.infradead.org, "maintainer:BROADCOM IPROC ARM ARCHITECTURE" , Jonathan Derrick , Xiaowei Song , linux-kernel-mentees@lists.linuxfoundation.org, Robert Richter , Sean V Kelley , Ray Jui , Haiyang Zhang , Ryder Lee , linux-mediatek@lists.infradead.org, Matthias Brugger , Stephen Hemminger , linux-arm-kernel@lists.infradead.org, Qiuxu Zhuo , Scott Branden , linuxppc-dev@lists.ozlabs.org, Yoshihiro Shimoda , Krzysztof Kozlowski , linux-kernel@vger.kernel.org, linux-renesas-soc@vger.kernel.org, Lukas Wunner , Jingoo Han , Shawn Guo , Pali =?iso-8859-1?Q?Roh=E1r?= Subject: Re: [PATCH v3 01/25] PCI: Add PCI_ERROR_RESPONSE and it's related definitions Message-ID: <20211117235812.GA1786428@bhelgaas> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20211117_155815_060913_BCBCC514 X-CRM114-Status: GOOD ( 25.31 ) X-BeenThere: linux-rockchip@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Upstream kernel work for Rockchip platforms List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "Linux-rockchip" Errors-To: linux-rockchip-bounces+linux-rockchip=archiver.kernel.org@lists.infradead.org On Thu, Oct 21, 2021 at 08:37:26PM +0530, Naveen Naidu wrote: > An MMIO read from a PCI device that doesn't exist or doesn't respond > causes a PCI error. There's no real data to return to satisfy the > CPU read, so most hardware fabricates ~0 data. > > Add a PCI_ERROR_RESPONSE definition for that and use it where > appropriate to make these checks consistent and easier to find. > > Also add helper definitions SET_PCI_ERROR_RESPONSE and > RESPONSE_IS_PCI_ERROR to make the code more readable. > > Suggested-by: Bjorn Helgaas > Signed-off-by: Naveen Naidu > --- > include/linux/pci.h | 9 +++++++++ > 1 file changed, 9 insertions(+) > > diff --git a/include/linux/pci.h b/include/linux/pci.h > index cd8aa6fce204..689c8277c584 100644 > --- a/include/linux/pci.h > +++ b/include/linux/pci.h > @@ -154,6 +154,15 @@ enum pci_interrupt_pin { > /* The number of legacy PCI INTx interrupts */ > #define PCI_NUM_INTX 4 > > +/* > + * Reading from a device that doesn't respond typically returns ~0. A > + * successful read from a device may also return ~0, so you need additional > + * information to reliably identify errors. > + */ > +#define PCI_ERROR_RESPONSE (~0ULL) > +#define SET_PCI_ERROR_RESPONSE(val) (*(val) = ((typeof(*(val))) PCI_ERROR_RESPONSE)) > +#define RESPONSE_IS_PCI_ERROR(val) ((val) == ((typeof(val)) PCI_ERROR_RESPONSE)) Beautiful! I really like this. I would prefer the macros to start with "PCI_", e.g., PCI_SET_ERROR_RESPONSE(). I think "RESPONSE_IS_PCI_ERROR()" is too strong because (as the comment says), ~0 *may* indicate an error. Or it may be a successful read of a register that happens to contain ~0. Possibilities to convey the idea that this isn't definitive: PCI_POSSIBLE_ERROR_RESPONSE(val) # a little long PCI_LIKELY_ERROR(val) # we really have no idea whether PCI_PROBABLE_ERROR(val) # likely or probable PCI_POSSIBLE_ERROR(val) # promising? Can you rebase to my "main" branch (v5.16-rc1), tweak the above, and collect up the acks/reviews? We should also browse drivers outside drivers/pci for places we could use these. Not necessarily as part of this series, although if authors there object, it would be good to learn that earlier than later. Drivers that implement pci_error_handlers might be a fruitful place to start. But you've done a great job finding users of ~0 and 0xffff... in drivers/pci/, too. > + > /* > * pci_power_t values must match the bits in the Capabilities PME_Support > * and Control/Status PowerState fields in the Power Management capability. > -- > 2.25.1 > > _______________________________________________ > Linux-kernel-mentees mailing list > Linux-kernel-mentees@lists.linuxfoundation.org > https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees _______________________________________________ Linux-rockchip mailing list Linux-rockchip@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-rockchip 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 mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id BFD89C433EF for ; Wed, 17 Nov 2021 23:58:35 +0000 (UTC) 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 mail.kernel.org (Postfix) with ESMTPS id 7B59661ABF for ; Wed, 17 Nov 2021 23:58:35 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org 7B59661ABF Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=kernel.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=lists.infradead.org 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: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:References: List-Owner; bh=vqk0iOrHOR5Fz8Jt8GId5QasELHM10oU+sFYbV5p834=; b=y6bvnCdYZNCT5f 0Ct6dgEiiGzZdJ8kyqFk8CoUOZcEA3b3ZBFqLFJXYPv6SAqP4bzOVqLWSU+2Da0RPuQctCauqEY2P yD4IslUM4gGqU2PiajkpgPijnbFa2MUS7GDf0s7GT/TwtnWOqTz5YbD1v7AFAV3Ixdt9ZGQEXczFW Bawhjih11x4JZ9rtKpm9IEHpyYXEbNntAy+TFRPhLVDY1nSlx4uvTBBG64nsG3c4u3PkGLXMLFxfD AetALjylaCAwd9ONI9OOhFgc00XOLQOPmlcN9FPPaqTUbECpkMMkVeW7O/Ogt7Ol2rvKn5L4ZN41E GYP+h2d634ga0YlTtwmw==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1mnUou-006cAz-K9; Wed, 17 Nov 2021 23:58:28 +0000 Received: from mail.kernel.org ([198.145.29.99]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1mnUog-006c86-UK; Wed, 17 Nov 2021 23:58:16 +0000 Received: by mail.kernel.org (Postfix) with ESMTPSA id C8D5A61AD2; Wed, 17 Nov 2021 23:58:13 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1637193494; bh=jt1bptChpIJAfJm7hhcvnL0z6T39eDJjHsyLzG8zD1Y=; h=Date:From:To:Cc:Subject:In-Reply-To:From; b=lyKjNjZh0qOCqIMVIpeRoNDwhdUJ/tL20BWpiwNwGttOe0RfGC7KJ4PhUovOify8k CPIwK9uSUQMg7VMxx+ystTmi5eBYFIkTtVD4sE8ulLOvbgkfTXGOpyI5/5ntrQgvl0 CfZ1WfNlECodqxIGcyQGBzURqYPwL89BjB4irtfrZDGz99G0bGiOvyoqAHp04d3fBv aBxC568xp2R7kzM9biPqbt5SZcYkGOgkMTsagR6oWZfjlJq8OfZ4tIYBwT1Dm+Bjb+ xfWKIqeKqVbpwaZ3tBQdETriNpArC0Ads/tlo1ukjZbgXjypi2+h68mtdIpt5Dhoxd 4UKc+KstDfPQQ== Date: Wed, 17 Nov 2021 17:58:12 -0600 From: Bjorn Helgaas To: Naveen Naidu Cc: bhelgaas@google.com, Krzysztof =?utf-8?Q?Wilczy=C5=84ski?= , linux-hyperv@vger.kernel.org, Heiko Stuebner , linux-pci@vger.kernel.org, Shawn Lin , Binghui Wang , Kuppuswamy Sathyanarayanan , Oliver O'Halloran , Thomas Petazzoni , Russell Currey , Lorenzo Pieralisi , Toan Le , "K. Y. Srinivasan" , Nirmal Patel , Marek Vasut , Rob Herring , Wei Liu , linux-samsung-soc@vger.kernel.org, Marc Zyngier , Joyce Ooi , Dexuan Cui , Jianjun Wang , linux-rockchip@lists.infradead.org, "maintainer:BROADCOM IPROC ARM ARCHITECTURE" , Jonathan Derrick , Xiaowei Song , linux-kernel-mentees@lists.linuxfoundation.org, Robert Richter , Sean V Kelley , Ray Jui , Haiyang Zhang , Ryder Lee , linux-mediatek@lists.infradead.org, Matthias Brugger , Stephen Hemminger , linux-arm-kernel@lists.infradead.org, Qiuxu Zhuo , Scott Branden , linuxppc-dev@lists.ozlabs.org, Yoshihiro Shimoda , Krzysztof Kozlowski , linux-kernel@vger.kernel.org, linux-renesas-soc@vger.kernel.org, Lukas Wunner , Jingoo Han , Shawn Guo , Pali =?iso-8859-1?Q?Roh=E1r?= Subject: Re: [PATCH v3 01/25] PCI: Add PCI_ERROR_RESPONSE and it's related definitions Message-ID: <20211117235812.GA1786428@bhelgaas> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20211117_155815_060913_BCBCC514 X-CRM114-Status: GOOD ( 25.31 ) X-BeenThere: linux-mediatek@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-mediatek" Errors-To: linux-mediatek-bounces+linux-mediatek=archiver.kernel.org@lists.infradead.org On Thu, Oct 21, 2021 at 08:37:26PM +0530, Naveen Naidu wrote: > An MMIO read from a PCI device that doesn't exist or doesn't respond > causes a PCI error. There's no real data to return to satisfy the > CPU read, so most hardware fabricates ~0 data. > > Add a PCI_ERROR_RESPONSE definition for that and use it where > appropriate to make these checks consistent and easier to find. > > Also add helper definitions SET_PCI_ERROR_RESPONSE and > RESPONSE_IS_PCI_ERROR to make the code more readable. > > Suggested-by: Bjorn Helgaas > Signed-off-by: Naveen Naidu > --- > include/linux/pci.h | 9 +++++++++ > 1 file changed, 9 insertions(+) > > diff --git a/include/linux/pci.h b/include/linux/pci.h > index cd8aa6fce204..689c8277c584 100644 > --- a/include/linux/pci.h > +++ b/include/linux/pci.h > @@ -154,6 +154,15 @@ enum pci_interrupt_pin { > /* The number of legacy PCI INTx interrupts */ > #define PCI_NUM_INTX 4 > > +/* > + * Reading from a device that doesn't respond typically returns ~0. A > + * successful read from a device may also return ~0, so you need additional > + * information to reliably identify errors. > + */ > +#define PCI_ERROR_RESPONSE (~0ULL) > +#define SET_PCI_ERROR_RESPONSE(val) (*(val) = ((typeof(*(val))) PCI_ERROR_RESPONSE)) > +#define RESPONSE_IS_PCI_ERROR(val) ((val) == ((typeof(val)) PCI_ERROR_RESPONSE)) Beautiful! I really like this. I would prefer the macros to start with "PCI_", e.g., PCI_SET_ERROR_RESPONSE(). I think "RESPONSE_IS_PCI_ERROR()" is too strong because (as the comment says), ~0 *may* indicate an error. Or it may be a successful read of a register that happens to contain ~0. Possibilities to convey the idea that this isn't definitive: PCI_POSSIBLE_ERROR_RESPONSE(val) # a little long PCI_LIKELY_ERROR(val) # we really have no idea whether PCI_PROBABLE_ERROR(val) # likely or probable PCI_POSSIBLE_ERROR(val) # promising? Can you rebase to my "main" branch (v5.16-rc1), tweak the above, and collect up the acks/reviews? We should also browse drivers outside drivers/pci for places we could use these. Not necessarily as part of this series, although if authors there object, it would be good to learn that earlier than later. Drivers that implement pci_error_handlers might be a fruitful place to start. But you've done a great job finding users of ~0 and 0xffff... in drivers/pci/, too. > + > /* > * pci_power_t values must match the bits in the Capabilities PME_Support > * and Control/Status PowerState fields in the Power Management capability. > -- > 2.25.1 > > _______________________________________________ > Linux-kernel-mentees mailing list > Linux-kernel-mentees@lists.linuxfoundation.org > https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees _______________________________________________ Linux-mediatek mailing list Linux-mediatek@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-mediatek 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 mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id C804FC433F5 for ; Wed, 17 Nov 2021 23:59:56 +0000 (UTC) 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 mail.kernel.org (Postfix) with ESMTPS id 8FD2B61B72 for ; Wed, 17 Nov 2021 23:59:56 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org 8FD2B61B72 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=kernel.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=lists.infradead.org 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: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:References: List-Owner; bh=YFnz6EfIvaigAbjuZFQBfDO40Hdnfg5269BBqSkpEG0=; b=fgsBl63J3Aw9O7 +GN5jKa6qy7sE0bDBa2e6zyvUwZhkduY8BdPFUAZQF4c0vMQl8fwVhTtY5Wfq2sGWNN03Kbq1umZq h3L5tXDawObu4+zO/i80Qt/fLa8KRpmP/v6FcI5BUJtA03vvtywmnU0OdLy5XGWfxV/RXMS8fD8h0 C3zghqIVJT7qqwmBOFJg1Ibh866+lLMIraFocxyGe6W13iTP+GH1xfSQMTpb3FgImvppMYOtoUSpE xUOULU9uMLm0sq3flXy+CwXvYVttR9XiLmishmhRvcusLbaGqOGMbjmgB5m01enplNd8CCuOFv5yY lmnWyC+lKrcSsDuVeRGQ==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1mnUol-006c8v-5H; Wed, 17 Nov 2021 23:58:19 +0000 Received: from mail.kernel.org ([198.145.29.99]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1mnUog-006c86-UK; Wed, 17 Nov 2021 23:58:16 +0000 Received: by mail.kernel.org (Postfix) with ESMTPSA id C8D5A61AD2; Wed, 17 Nov 2021 23:58:13 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1637193494; bh=jt1bptChpIJAfJm7hhcvnL0z6T39eDJjHsyLzG8zD1Y=; h=Date:From:To:Cc:Subject:In-Reply-To:From; b=lyKjNjZh0qOCqIMVIpeRoNDwhdUJ/tL20BWpiwNwGttOe0RfGC7KJ4PhUovOify8k CPIwK9uSUQMg7VMxx+ystTmi5eBYFIkTtVD4sE8ulLOvbgkfTXGOpyI5/5ntrQgvl0 CfZ1WfNlECodqxIGcyQGBzURqYPwL89BjB4irtfrZDGz99G0bGiOvyoqAHp04d3fBv aBxC568xp2R7kzM9biPqbt5SZcYkGOgkMTsagR6oWZfjlJq8OfZ4tIYBwT1Dm+Bjb+ xfWKIqeKqVbpwaZ3tBQdETriNpArC0Ads/tlo1ukjZbgXjypi2+h68mtdIpt5Dhoxd 4UKc+KstDfPQQ== Date: Wed, 17 Nov 2021 17:58:12 -0600 From: Bjorn Helgaas To: Naveen Naidu Cc: bhelgaas@google.com, Krzysztof =?utf-8?Q?Wilczy=C5=84ski?= , linux-hyperv@vger.kernel.org, Heiko Stuebner , linux-pci@vger.kernel.org, Shawn Lin , Binghui Wang , Kuppuswamy Sathyanarayanan , Oliver O'Halloran , Thomas Petazzoni , Russell Currey , Lorenzo Pieralisi , Toan Le , "K. Y. Srinivasan" , Nirmal Patel , Marek Vasut , Rob Herring , Wei Liu , linux-samsung-soc@vger.kernel.org, Marc Zyngier , Joyce Ooi , Dexuan Cui , Jianjun Wang , linux-rockchip@lists.infradead.org, "maintainer:BROADCOM IPROC ARM ARCHITECTURE" , Jonathan Derrick , Xiaowei Song , linux-kernel-mentees@lists.linuxfoundation.org, Robert Richter , Sean V Kelley , Ray Jui , Haiyang Zhang , Ryder Lee , linux-mediatek@lists.infradead.org, Matthias Brugger , Stephen Hemminger , linux-arm-kernel@lists.infradead.org, Qiuxu Zhuo , Scott Branden , linuxppc-dev@lists.ozlabs.org, Yoshihiro Shimoda , Krzysztof Kozlowski , linux-kernel@vger.kernel.org, linux-renesas-soc@vger.kernel.org, Lukas Wunner , Jingoo Han , Shawn Guo , Pali =?iso-8859-1?Q?Roh=E1r?= Subject: Re: [PATCH v3 01/25] PCI: Add PCI_ERROR_RESPONSE and it's related definitions Message-ID: <20211117235812.GA1786428@bhelgaas> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20211117_155815_060913_BCBCC514 X-CRM114-Status: GOOD ( 25.31 ) 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 On Thu, Oct 21, 2021 at 08:37:26PM +0530, Naveen Naidu wrote: > An MMIO read from a PCI device that doesn't exist or doesn't respond > causes a PCI error. There's no real data to return to satisfy the > CPU read, so most hardware fabricates ~0 data. > > Add a PCI_ERROR_RESPONSE definition for that and use it where > appropriate to make these checks consistent and easier to find. > > Also add helper definitions SET_PCI_ERROR_RESPONSE and > RESPONSE_IS_PCI_ERROR to make the code more readable. > > Suggested-by: Bjorn Helgaas > Signed-off-by: Naveen Naidu > --- > include/linux/pci.h | 9 +++++++++ > 1 file changed, 9 insertions(+) > > diff --git a/include/linux/pci.h b/include/linux/pci.h > index cd8aa6fce204..689c8277c584 100644 > --- a/include/linux/pci.h > +++ b/include/linux/pci.h > @@ -154,6 +154,15 @@ enum pci_interrupt_pin { > /* The number of legacy PCI INTx interrupts */ > #define PCI_NUM_INTX 4 > > +/* > + * Reading from a device that doesn't respond typically returns ~0. A > + * successful read from a device may also return ~0, so you need additional > + * information to reliably identify errors. > + */ > +#define PCI_ERROR_RESPONSE (~0ULL) > +#define SET_PCI_ERROR_RESPONSE(val) (*(val) = ((typeof(*(val))) PCI_ERROR_RESPONSE)) > +#define RESPONSE_IS_PCI_ERROR(val) ((val) == ((typeof(val)) PCI_ERROR_RESPONSE)) Beautiful! I really like this. I would prefer the macros to start with "PCI_", e.g., PCI_SET_ERROR_RESPONSE(). I think "RESPONSE_IS_PCI_ERROR()" is too strong because (as the comment says), ~0 *may* indicate an error. Or it may be a successful read of a register that happens to contain ~0. Possibilities to convey the idea that this isn't definitive: PCI_POSSIBLE_ERROR_RESPONSE(val) # a little long PCI_LIKELY_ERROR(val) # we really have no idea whether PCI_PROBABLE_ERROR(val) # likely or probable PCI_POSSIBLE_ERROR(val) # promising? Can you rebase to my "main" branch (v5.16-rc1), tweak the above, and collect up the acks/reviews? We should also browse drivers outside drivers/pci for places we could use these. Not necessarily as part of this series, although if authors there object, it would be good to learn that earlier than later. Drivers that implement pci_error_handlers might be a fruitful place to start. But you've done a great job finding users of ~0 and 0xffff... in drivers/pci/, too. > + > /* > * pci_power_t values must match the bits in the Capabilities PME_Support > * and Control/Status PowerState fields in the Power Management capability. > -- > 2.25.1 > > _______________________________________________ > Linux-kernel-mentees mailing list > Linux-kernel-mentees@lists.linuxfoundation.org > https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel