Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
MIME-Version: 1.0
References: <20190718020745.8867-1-fred@fredlawl.com>
In-Reply-To: <20190718020745.8867-1-fred@fredlawl.com>
Reply-To: bjorn@helgaas.com
From:   Bjorn Helgaas <bjorn.helgaas@gmail.com>
Date:   Thu, 18 Jul 2019 08:50:50 -0500
Message-ID: <CABhMZUWX055ysYoy7CyFoKE1DCCrGnQXuh9cEqNHk4r5Se+5fA@mail.gmail.com>
Subject: Re: [PATCH] cxgb4: Prefer pcie_capability_read_word()
To:     Frederick Lawler <fred@fredlawl.com>
Cc:     vishal@chelsio.com, netdev@vger.kernel.org,
        linux-kernel@vger.kernel.org, Bjorn Helgaas <bhelgaas@google.com>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Wed, Jul 17, 2019 at 9:08 PM Frederick Lawler <fred@fredlawl.com> wrote:
>
> Commit 8c0d3a02c130 ("PCI: Add accessors for PCI Express Capability")
> added accessors for the PCI Express Capability so that drivers didn't
> need to be aware of differences between v1 and v2 of the PCI
> Express Capability.
>
> Replace pci_read_config_word() and pci_write_config_word() calls with
> pcie_capability_read_word() and pcie_capability_write_word().
>
> Signed-off-by: Frederick Lawler <fred@fredlawl.com>

Nice job on all these patches!  These all help avoid errors and
identify possibilities for refactoring.

If there were a cover letter for the series, I would have replied to
that, but for all of them:

Reviewed-by: Bjorn Helgaas <bhelgaas@google.com>

If you post the series again for any reason, you can add that.
Otherwise, whoever applies them can add my reviewed-by.

> ---
>  drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c | 6 ++----
>  drivers/net/ethernet/chelsio/cxgb4/t4_hw.c      | 9 +++------
>  2 files changed, 5 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c
> index 715e4edcf4a2..98ff71434673 100644
> --- a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c
> +++ b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c
> @@ -5441,7 +5441,6 @@ static int cxgb4_iov_configure(struct pci_dev *pdev, int num_vfs)
>                 char name[IFNAMSIZ];
>                 u32 devcap2;
>                 u16 flags;
> -               int pos;
>
>                 /* If we want to instantiate Virtual Functions, then our
>                  * parent bridge's PCI-E needs to support Alternative Routing
> @@ -5449,9 +5448,8 @@ static int cxgb4_iov_configure(struct pci_dev *pdev, int num_vfs)
>                  * and above.
>                  */
>                 pbridge = pdev->bus->self;
> -               pos = pci_find_capability(pbridge, PCI_CAP_ID_EXP);
> -               pci_read_config_word(pbridge, pos + PCI_EXP_FLAGS, &flags);
> -               pci_read_config_dword(pbridge, pos + PCI_EXP_DEVCAP2, &devcap2);
> +               pcie_capability_read_word(pbridge, PCI_EXP_FLAGS, &flags);
> +               pcie_capability_read_dword(pbridge, PCI_EXP_DEVCAP2, &devcap2);
>
>                 if ((flags & PCI_EXP_FLAGS_VERS) < 2 ||
>                     !(devcap2 & PCI_EXP_DEVCAP2_ARI)) {
> diff --git a/drivers/net/ethernet/chelsio/cxgb4/t4_hw.c b/drivers/net/ethernet/chelsio/cxgb4/t4_hw.c
> index f9b70be59792..346d7b59c50b 100644
> --- a/drivers/net/ethernet/chelsio/cxgb4/t4_hw.c
> +++ b/drivers/net/ethernet/chelsio/cxgb4/t4_hw.c
> @@ -7267,7 +7267,6 @@ int t4_fixup_host_params(struct adapter *adap, unsigned int page_size,
>         } else {
>                 unsigned int pack_align;
>                 unsigned int ingpad, ingpack;
> -               unsigned int pcie_cap;
>
>                 /* T5 introduced the separation of the Free List Padding and
>                  * Packing Boundaries.  Thus, we can select a smaller Padding
> @@ -7292,8 +7291,7 @@ int t4_fixup_host_params(struct adapter *adap, unsigned int page_size,
>                  * multiple of the Maximum Payload Size.
>                  */
>                 pack_align = fl_align;
> -               pcie_cap = pci_find_capability(adap->pdev, PCI_CAP_ID_EXP);
> -               if (pcie_cap) {
> +               if (pci_is_pcie(adap->pdev)) {
>                         unsigned int mps, mps_log;
>                         u16 devctl;
>
> @@ -7301,9 +7299,8 @@ int t4_fixup_host_params(struct adapter *adap, unsigned int page_size,
>                          * [bits 7:5] encodes sizes as powers of 2 starting at
>                          * 128 bytes.
>                          */
> -                       pci_read_config_word(adap->pdev,
> -                                            pcie_cap + PCI_EXP_DEVCTL,
> -                                            &devctl);
> +                       pcie_capability_read_word(adap->pdev, PCI_EXP_DEVCTL,
> +                                                 &devctl);
>                         mps_log = ((devctl & PCI_EXP_DEVCTL_PAYLOAD) >> 5) + 7;
>                         mps = 1 << mps_log;
>                         if (mps > pack_align)
> --
> 2.17.1
>