Re: [PATCH] cxgb4: Prefer pcie_capability_read_word()

From: Bjorn Helgaas
Date: Thu Jul 18 2019 - 09:51:07 EST


On Wed, Jul 17, 2019 at 9:08 PM Frederick Lawler <fred@xxxxxxxxxxxx> 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@xxxxxxxxxxxx>

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@xxxxxxxxxx>

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
>