Re: [PATCH 01/10] PCI: Use FIELD_GET()

From: Ilpo Järvinen
Date: Wed Oct 11 2023 - 07:24:11 EST

On Tue, 10 Oct 2023, Bjorn Helgaas wrote:

> From: Bjorn Helgaas <bhelgaas@xxxxxxxxxx>
>
> Use FIELD_GET() to remove dependences on the field position, i.e., the
> shift value. No functional change intended.

FIELD_PREP() is also used below.

> Signed-off-by: Bjorn Helgaas <bhelgaas@xxxxxxxxxx>
> ---
> drivers/pci/pci.c | 45 ++++++++++++++++++++++-----------------------
> drivers/pci/probe.c | 8 ++++----
> 2 files changed, 26 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index a8adc34dc86f..848c9ee65d7f 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -1776,8 +1776,7 @@ static void pci_restore_rebar_state(struct pci_dev *pdev)
> return;
>
> pci_read_config_dword(pdev, pos + PCI_REBAR_CTRL, &ctrl);
> - nbars = (ctrl & PCI_REBAR_CTRL_NBAR_MASK) >>
> - PCI_REBAR_CTRL_NBAR_SHIFT;
> + nbars = FIELD_GET(PCI_REBAR_CTRL_NBAR_MASK, ctrl);
>
> for (i = 0; i < nbars; i++, pos += 8) {
> struct resource *res;
> @@ -1788,7 +1787,7 @@ static void pci_restore_rebar_state(struct pci_dev *pdev)
> res = pdev->resource + bar_idx;
> size = pci_rebar_bytes_to_size(resource_size(res));
> ctrl &= ~PCI_REBAR_CTRL_BAR_SIZE;
> - ctrl |= size << PCI_REBAR_CTRL_BAR_SHIFT;
> + ctrl |= FIELD_PREP(PCI_REBAR_CTRL_BAR_SIZE, size);
> pci_write_config_dword(pdev, pos + PCI_REBAR_CTRL, ctrl);
> }
> }
> @@ -3229,7 +3228,7 @@ void pci_pm_init(struct pci_dev *dev)
> (pmc & PCI_PM_CAP_PME_D2) ? " D2" : "",
> (pmc & PCI_PM_CAP_PME_D3hot) ? " D3hot" : "",
> (pmc & PCI_PM_CAP_PME_D3cold) ? " D3cold" : "");
> - dev->pme_support = pmc >> PCI_PM_CAP_PME_SHIFT;
> + dev->pme_support = FIELD_GET(PCI_PM_CAP_PME_MASK, pmc);
> dev->pme_poll = true;
> /*
> * Make device's PM flags reflect the wake-up capability, but
> @@ -3300,20 +3299,20 @@ static int pci_ea_read(struct pci_dev *dev, int offset)
> ent_offset += 4;
>
> /* Entry size field indicates DWORDs after 1st */
> - ent_size = ((dw0 & PCI_EA_ES) + 1) << 2;
> + ent_size = (FIELD_GET(PCI_EA_ES, dw0) + 1) << 2;
>
> if (!(dw0 & PCI_EA_ENABLE)) /* Entry not enabled */
> goto out;
>
> - bei = (dw0 & PCI_EA_BEI) >> 4;
> - prop = (dw0 & PCI_EA_PP) >> 8;
> + bei = FIELD_GET(PCI_EA_BEI, dw0);
> + prop = FIELD_GET(PCI_EA_PP, dw0);
>
> /*
> * If the Property is in the reserved range, try the Secondary
> * Property instead.
> */
> if (prop > PCI_EA_P_BRIDGE_IO && prop < PCI_EA_P_MEM_RESERVED)
> - prop = (dw0 & PCI_EA_SP) >> 16;
> + prop = FIELD_GET(PCI_EA_SP, dw0);
> if (prop > PCI_EA_P_BRIDGE_IO)
> goto out;
>
> @@ -3720,14 +3719,13 @@ static int pci_rebar_find_pos(struct pci_dev *pdev, int bar)
> return -ENOTSUPP;
>
> pci_read_config_dword(pdev, pos + PCI_REBAR_CTRL, &ctrl);
> - nbars = (ctrl & PCI_REBAR_CTRL_NBAR_MASK) >>
> - PCI_REBAR_CTRL_NBAR_SHIFT;
> + nbars = FIELD_GET(PCI_REBAR_CTRL_NBAR_MASK, ctrl);
>
> for (i = 0; i < nbars; i++, pos += 8) {
> int bar_idx;
>
> pci_read_config_dword(pdev, pos + PCI_REBAR_CTRL, &ctrl);
> - bar_idx = ctrl & PCI_REBAR_CTRL_BAR_IDX;
> + bar_idx = FIELD_GET(PCI_REBAR_CTRL_BAR_IDX, ctrl);
> if (bar_idx == bar)
> return pos;
> }
> @@ -3782,7 +3780,7 @@ int pci_rebar_get_current_size(struct pci_dev *pdev, int bar)
> return pos;
>
> pci_read_config_dword(pdev, pos + PCI_REBAR_CTRL, &ctrl);
> - return (ctrl & PCI_REBAR_CTRL_BAR_SIZE) >> PCI_REBAR_CTRL_BAR_SHIFT;
> + return FIELD_GET(PCI_REBAR_CTRL_BAR_SIZE, ctrl);
> }
>
> /**
> @@ -3805,7 +3803,7 @@ int pci_rebar_set_size(struct pci_dev *pdev, int bar, int size)
>
> pci_read_config_dword(pdev, pos + PCI_REBAR_CTRL, &ctrl);
> ctrl &= ~PCI_REBAR_CTRL_BAR_SIZE;
> - ctrl |= size << PCI_REBAR_CTRL_BAR_SHIFT;
> + ctrl |= FIELD_PREP(PCI_REBAR_CTRL_BAR_SIZE, size);
> pci_write_config_dword(pdev, pos + PCI_REBAR_CTRL, ctrl);
> return 0;
> }
> @@ -6043,7 +6041,7 @@ int pcix_get_max_mmrbc(struct pci_dev *dev)
> if (pci_read_config_dword(dev, cap + PCI_X_STATUS, &stat))
> return -EINVAL;
>
> - return 512 << ((stat & PCI_X_STATUS_MAX_READ) >> 21);
> + return 512 << FIELD_GET(PCI_X_STATUS_MAX_READ, stat);
> }
> EXPORT_SYMBOL(pcix_get_max_mmrbc);
>
> @@ -6066,7 +6064,7 @@ int pcix_get_mmrbc(struct pci_dev *dev)
> if (pci_read_config_word(dev, cap + PCI_X_CMD, &cmd))
> return -EINVAL;
>
> - return 512 << ((cmd & PCI_X_CMD_MAX_READ) >> 2);
> + return 512 << FIELD_GET(PCI_X_CMD_MAX_READ, cmd);
> }
> EXPORT_SYMBOL(pcix_get_mmrbc);
>
> @@ -6097,19 +6095,19 @@ int pcix_set_mmrbc(struct pci_dev *dev, int mmrbc)
> if (pci_read_config_dword(dev, cap + PCI_X_STATUS, &stat))
> return -EINVAL;
>
> - if (v > (stat & PCI_X_STATUS_MAX_READ) >> 21)
> + if (v > FIELD_GET(PCI_X_STATUS_MAX_READ, stat))
> return -E2BIG;
>
> if (pci_read_config_word(dev, cap + PCI_X_CMD, &cmd))
> return -EINVAL;
>
> - o = (cmd & PCI_X_CMD_MAX_READ) >> 2;
> + o = FIELD_GET(PCI_X_CMD_MAX_READ, cmd);
> if (o != v) {
> if (v > o && (dev->bus->bus_flags & PCI_BUS_FLAGS_NO_MMRBC))
> return -EIO;
>
> cmd &= ~PCI_X_CMD_MAX_READ;
> - cmd |= v << 2;
> + cmd |= FIELD_PREP(PCI_X_CMD_MAX_READ, v);
> if (pci_write_config_word(dev, cap + PCI_X_CMD, cmd))
> return -EIO;
> }
> @@ -6129,7 +6127,7 @@ int pcie_get_readrq(struct pci_dev *dev)
>
> pcie_capability_read_word(dev, PCI_EXP_DEVCTL, &ctl);
>
> - return 128 << ((ctl & PCI_EXP_DEVCTL_READRQ) >> 12);
> + return 128 << FIELD_GET(PCI_EXP_DEVCTL_READRQ, ctl);
> }
> EXPORT_SYMBOL(pcie_get_readrq);
>
> @@ -6162,7 +6160,7 @@ int pcie_set_readrq(struct pci_dev *dev, int rq)
> rq = mps;
> }
>
> - v = (ffs(rq) - 8) << 12;
> + v = FIELD_PREP(PCI_EXP_DEVCTL_READRQ, ffs(rq) - 8);
>
> if (bridge->no_inc_mrrs) {
> int max_mrrs = pcie_get_readrq(dev);
> @@ -6192,7 +6190,7 @@ int pcie_get_mps(struct pci_dev *dev)
>
> pcie_capability_read_word(dev, PCI_EXP_DEVCTL, &ctl);
>
> - return 128 << ((ctl & PCI_EXP_DEVCTL_PAYLOAD) >> 5);
> + return 128 << FIELD_GET(PCI_EXP_DEVCTL_PAYLOAD, ctl);
> }
> EXPORT_SYMBOL(pcie_get_mps);
>
> @@ -6215,7 +6213,7 @@ int pcie_set_mps(struct pci_dev *dev, int mps)
> v = ffs(mps) - 8;
> if (v > dev->pcie_mpss)
> return -EINVAL;
> - v <<= 5;
> + v = FIELD_PREP(PCI_EXP_DEVCTL_PAYLOAD, v);


--
i.