Re: [PATCH V7 2/5] PCI/TPH: Add Steering Tag support

[Robin Murphy <robin.murphy@arm.com>]

On 2024-10-02 5:59 pm, Wei Huang wrote:
[...]
> +/**
> + * pcie_tph_set_st_entry() - Set Steering Tag in the ST table entry
> + * @pdev: PCI device
> + * @index: ST table entry index
> + * @tag: Steering Tag to be written
> + *
> + * This function will figure out the proper location of ST table, either in the
> + * MSI-X table or in the TPH Extended Capability space, and write the Steering
> + * Tag into the ST entry pointed by index.
> + *
> + * Returns: 0 if success, otherwise negative value (-errno)
> + */
> +int pcie_tph_set_st_entry(struct pci_dev *pdev, unsigned int index, u16 tag)
> +{
> +	u32 loc;
> +	int err = 0;
> +
> +	if (!pdev->tph_cap)
> +		return -EINVAL;
> +
> +	if (!pdev->tph_enabled)
> +		return -EINVAL;
> +
> +	/* No need to write tag if device is in "No ST Mode" */
> +	if (pdev->tph_mode == PCI_TPH_ST_NS_MODE)
> +		return 0;
> +
> +	/* Disable TPH before updating ST to avoid potential instability as
> +	 * cautioned in PCIe r6.2, sec 6.17.3, "ST Modes of Operation"
> +	 */
> +	set_ctrl_reg_req_en(pdev, PCI_TPH_REQ_DISABLE);
> +
> +	loc = get_st_table_loc(pdev);
> +	/* Convert loc to match with PCI_TPH_LOC_* defined in pci_regs.h */
> +	loc = FIELD_PREP(PCI_TPH_CAP_LOC_MASK, loc);
> +
> +	switch (loc) {
> +	case PCI_TPH_LOC_MSIX:
> +		err = write_tag_to_msix(pdev, index, tag);
> +		break;
> +	case PCI_TPH_LOC_CAP:
> +		err = write_tag_to_st_table(pdev, index, tag);
> +		break;
> +	default:
> +		err = -EINVAL;
> +	}
> +
> +	if (err) {
> +		pcie_disable_tph(pdev);
> +		return err;
> +	}
> +
> +	set_ctrl_reg_req_en(pdev, pdev->tph_mode);

Just looking at this code in mainline, and I don't trust my 
understanding quite enough to send a patch myself, but doesn't this want 
to be pdev->tph_req_type, rather than tph_mode?

Thanks,
Robin.

> +
> +	pci_dbg(pdev, "set steering tag: %s table, index=%d, tag=%#04x\n",
> +		(loc == PCI_TPH_LOC_MSIX) ? "MSI-X" : "ST", index, tag);
> +
> +	return 0;
> +}