lkml.org 
[lkml]   [2021]   [Jul]   [28]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    SubjectRe: [PATCH v10 1/8] PCI: Add pcie_reset_flr to follow calling convention of other reset methods
    From
    Date
    Hi Bjorn,

    On 7/27/21 5:12 PM, Bjorn Helgaas wrote:
    > External email: Use caution opening links or attachments
    >
    >
    > On Fri, Jul 09, 2021 at 06:08:06PM +0530, 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/crypto/cavium/nitrox/nitrox_main.c b/drivers/crypto/cavium/nitrox/nitrox_main.c
    >> index facc8e6bc..15d6c8452 100644
    >> --- a/drivers/crypto/cavium/nitrox/nitrox_main.c
    >> +++ b/drivers/crypto/cavium/nitrox/nitrox_main.c
    >> @@ -306,9 +306,7 @@ static int nitrox_device_flr(struct pci_dev *pdev)
    >> return -ENOMEM;
    >> }
    >>
    >> - /* check flr support */
    >> - if (pcie_has_flr(pdev))
    >> - pcie_flr(pdev);
    >> + pcie_reset_flr(pdev, 0);
    > I'm not really a fan of exposing the "probe" argument outside
    > drivers/pci/. I think this would be the only occurrence. Is there a
    > way to avoid that?
    >
    > Can we just make pcie_flr() do the equivalent of pcie_has_flr()
    > internally?
    >
    I like your suggestion don't change the existing definition of pcie_has_flr()/pcie_flr()
    and define a new function pcie_reset_flr() to fit into the reset framework. This way
    no need to modify logic/drivers outside of driver/pci/xxx.

    int pcie_reset_flr(struct pci_dev *dev, int probe)
    {
            if (!pcie_has_flr(dev))
                    return -ENOTTY;

            if (probe)
                    return 0;

            return pcie_flr(dev);
    }


    And add a new patch to begging of the series for caching 'devcap' in pci_dev structure.

    --- a/include/linux/pci.h
    +++ b/include/linux/pci.h
    @@ -333,6 +333,7 @@ struct pci_dev {
            struct rcec_ea  *rcec_ea;       /* RCEC cached endpoint association */
            struct pci_dev  *rcec;          /* Associated RCEC device */
     #endif
    +       u32             devcap;         /* Cached PCIe device capabilities */
            u8              pcie_cap;       /* PCIe capability offset */
            u8              msi_cap;        /* MSI capability offset */
            u8              msix_cap;       /* MSI-X capability offset */


    --- a/drivers/pci/pci.c
    +++ b/drivers/pci/pci.c
    @@ -31,6 +31,7 @@
     #include <linux/vmalloc.h>
     #include <asm/dma.h>
     #include <linux/aer.h>
    +#include <linux/bitfield.h>
     #include "pci.h"

     DEFINE_MUTEX(pci_slot_mutex);
    @@ -4630,13 +4631,10 @@ EXPORT_SYMBOL(pci_wait_for_pending_transaction);
      */
     bool pcie_has_flr(struct pci_dev *dev)
     {
    -       u32 cap;
    -
            if (dev->dev_flags & PCI_DEV_FLAGS_NO_FLR_RESET)
                    return false;

    -       pcie_capability_read_dword(dev, PCI_EXP_DEVCAP, &cap);
    -       return cap & PCI_EXP_DEVCAP_FLR;
    +       return !!FIELD_GET(PCI_EXP_DEVCAP_FLR, dev->devcap);
     }

    --- a/drivers/pci/probe.c
    +++ b/drivers/pci/probe.c
    @@ -19,6 +19,7 @@
     #include <linux/hypervisor.h>
     #include <linux/irqdomain.h>
     #include <linux/pm_runtime.h>
    +#include <linux/bitfield.h>
     #include "pci.h"

     #define CARDBUS_LATENCY_TIMER  176     /* secondary latency timer */
    @@ -1498,8 +1499,8 @@ 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, &pdev->devcap);
    +       pdev->pcie_mpss = FIELD_GET(PCI_EXP_DEVCAP_PAYLOAD, pdev->devcap);

    \
     
     \ /
      Last update: 2021-07-28 20:54    [W:3.189 / U:0.032 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site