Re: [PATCH v9 1/8] PCI: Add pcie_reset_flr to follow calling convention of other reset methods

[Alex Williamson]

On Mon, 5 Jul 2021 20:07:39 +0530
Amey Narkhede <ameynarkhede03@xxxxxxxxx> 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@xxxxxxxxx>
> > ---
> >  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@xxxxxxxxxx>