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

From: Alex Williamson
Date: Thu Jul 08 2021 - 16:25:03 EST


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>