Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Thu, 8 Jul 2021 14:24:54 -0600
From:   Alex Williamson <alex.williamson@redhat.com>
To:     Amey Narkhede <ameynarkhede03@gmail.com>
Cc:     Bjorn Helgaas <bhelgaas@google.com>,
        Raphael Norwitz <raphael.norwitz@nutanix.com>,
        linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org,
        kw@linux.com, Shanker Donthineni <sdonthineni@nvidia.com>,
        Sinan Kaya <okaya@kernel.org>, Len Brown <lenb@kernel.org>,
        "Rafael J . Wysocki" <rjw@rjwysocki.net>
Subject: Re: [PATCH v9 1/8] PCI: Add pcie_reset_flr to follow calling
 convention of other reset methods
Message-ID: <20210708142454.566608c1.alex.williamson@redhat.com>
In-Reply-To: <20210705143739.nghgqghnskp7emai@archlinux>
References: <20210705142138.2651-1-ameynarkhede03@gmail.com>
        <20210705142138.2651-2-ameynarkhede03@gmail.com>
        <20210705143739.nghgqghnskp7emai@archlinux>
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
Precedence: bulk

On Mon, 5 Jul 2021 20:07:39 +0530
Amey Narkhede <ameynarkhede03@gmail.com> wrote:

> On 21/07/05 07:51PM, Amey Narkhede wrote:
> > Add has_pcie_flr bitfield in struct pci_dev to indicate support for PCIe
> > FLR to avoid reading PCI_EXP_DEVCAP multiple times.
> >
> > Currently there is separate function pcie_has_flr() to probe if PCIe FLR
> > is supported by the device which does not match the calling convention
> > followed by reset methods which use second function argument to decide
> > whether to probe or not. Add new function pcie_reset_flr() that follows
> > the calling convention of reset methods.
> >
> > Signed-off-by: Amey Narkhede <ameynarkhede03@gmail.com>
> > ---
> >  drivers/crypto/cavium/nitrox/nitrox_main.c |  4 +-
> >  drivers/pci/pci.c                          | 59 +++++++++++-----------
> >  drivers/pci/pcie/aer.c                     | 12 ++---
> >  drivers/pci/probe.c                        |  6 ++-
> >  drivers/pci/quirks.c                       |  9 ++--
> >  include/linux/pci.h                        |  3 +-
> >  6 files changed, 45 insertions(+), 48 deletions(-)
> >
> > diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c  
> [...]
> > index 3a62d09b8..a95252113 100644
> > --- a/drivers/pci/probe.c
> > +++ b/drivers/pci/probe.c
> > @@ -1487,6 +1487,7 @@ void set_pcie_port_type(struct pci_dev *pdev)
> >  {
> >  	int pos;
> >  	u16 reg16;
> > +	u32 reg32;
> >  	int type;
> >  	struct pci_dev *parent;
> >
> > @@ -1497,8 +1498,9 @@ void set_pcie_port_type(struct pci_dev *pdev)
> >  	pdev->pcie_cap = pos;
> >  	pci_read_config_word(pdev, pos + PCI_EXP_FLAGS, &reg16);
> >  	pdev->pcie_flags_reg = reg16;
> > -	pci_read_config_word(pdev, pos + PCI_EXP_DEVCAP, &reg16);
> > -	pdev->pcie_mpss = reg16 & PCI_EXP_DEVCAP_PAYLOAD;
> > +	pci_read_config_dword(pdev, pos + PCI_EXP_DEVCAP, &reg32);
> > +	pdev->pcie_mpss = reg32 & PCI_EXP_DEVCAP_PAYLOAD;
> > +	pdev->has_pcie_flr = reg32 & PCI_EXP_DEVCAP_FLR ? 1 : 0;  
> On the side note, removing ternary here as Alex suggested doesn't work
> for some reason.

Probably I'm just incorrectly extrapolating boolean behavior to
bitfields, but indeed it doesn't work that way.  The other option is to
use the !! trick, ie. has_pcie_flr = !!(reg32 & PCI_EXP_DEVCAP_FLR),
but both solutions are used elsewhere in this file.

Reviewed-by: Alex Williamson <alex.williamson@redhat.com>