On Fri, May 31, 2024 at 04:38:37PM -0500, Wei Huang wrote:
> This patch introduces three API functions, pcie_tph_intr_vec_supported(),
> pcie_tph_get_st() and pcie_tph_set_st(), for a driver to query, retrieve
> or configure device's steering tags. There are two possible locations for
> steering tag table and the code automatically figure out the right
> location to set the tags if pcie_tph_set_st() is called. Note the tag
> value is always zero currently and will be extended in the follow-up
> patches.
> +static int tph_get_reg_field_u32(struct pci_dev *dev, u8 offset, u32 mask,
> + u8 shift, u32 *field)
> +{
> + u32 reg_val;
> + int ret;
> +
> + if (!dev->tph_cap)
> + return -EINVAL;
> +
> + ret = pci_read_config_dword(dev, dev->tph_cap + offset, ®_val);
> + if (ret)
> + return ret;
> +
> + *field = (reg_val & mask) >> shift;
> +
> + return 0;
> +}
> +
> +static int tph_get_table_size(struct pci_dev *dev, u16 *size_out)
> +{
> + int ret;
> + u32 tmp;
> +
> + ret = tph_get_reg_field_u32(dev, PCI_TPH_CAP,
> + PCI_TPH_CAP_ST_MASK,
> + PCI_TPH_CAP_ST_SHIFT, &tmp);
Just use FIELD_GET() instead.
> + if (ret)
> + return ret;
> +
> + *size_out = (u16)tmp;
> +
> + return 0;
> +}
> +
> +/*
> + * For a given device, return a pointer to the MSI table entry at msi_index.
s/MSI/MSI-X/ to avoid any possible confusion.
> +static void __iomem *tph_msix_table_entry(struct pci_dev *dev,
> + u16 msi_index)
> + ret = pcie_capability_read_dword(rp, PCI_EXP_DEVCAP2, &val);
> + if (ret) {
> + pr_err("cannot read device capabilities 2 of %s\n",
> + dev_name(&dev->dev));
Never use pr_err() when you can use pci_err() instead. Obviously no
dev_name() needed with pci_err(). Other instances below.
> + val &= PCI_EXP_DEVCAP2_TPH_COMP;
> +
> + return val >> PCI_EXP_DEVCAP2_TPH_COMP_SHIFT;
FIELD_GET()
> + * The PCI Specification version 5.0 requires the "No ST Mode" mode
> + * be supported by any compatible device.
Cite r6.0 or newer and include section number.
> + /* clear the mode select and enable fields and set new values*/
Space before closing */
> + ctrl_reg &= ~(PCI_TPH_CTRL_REQ_EN_MASK);
> + ctrl_reg |= (((u32)req_type << PCI_TPH_CTRL_REQ_EN_SHIFT) &
> + PCI_TPH_CTRL_REQ_EN_MASK);
FIELD_GET()/FIELD_PREP()
> +static bool pcie_tph_write_st(struct pci_dev *dev, unsigned int msix_nr,
> + u8 req_type, u16 tag)
This function is not a predicate and testing for true/false gives no
indication of the sense.
For typical functions that do read/write/etc, returning 0 means
success and -errno means failure. This is the opposite.
> + /*
> + * disable TPH before updating the tag to avoid potential instability
> + * as cautioned about in the "ST Table Programming" of PCI-E spec
s/disable/Disable/
"PCIe r6.0, sec ..."
> +bool pcie_tph_set_st(struct pci_dev *dev, unsigned int msix_nr,
> + unsigned int cpu, enum tph_mem_type mem_type,
> + u8 req_type)
Should return 0 or -errno.
