Re: [PATCH v11 3/5] PCI/TPH: Add pcie_tph_enabled_mode() helper

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] `pcie_tph_enabled_mode()` conflates the "disabled" state with the valid `PCI_TPH_ST_NS_MODE` (No ST Mode), returning 0 for both.
--

commit 054f3de083c771d0143aa6293e77e165e4c44a41
Author: Chengwen Feng <fengchengwen@huawei.com>

PCI/TPH: Add pcie_tph_enabled_mode() helper

This commit adds a helper to query the enabled TPH mode on a PCI device,
useful for drivers like VFIO-PCI that need to validate TPH state.

> diff --git a/drivers/pci/tph.c b/drivers/pci/tph.c
> index ba31b010f67ad..91c1e83410a3e 100644
> --- a/drivers/pci/tph.c
> +++ b/drivers/pci/tph.c
[ ... ]
> @@ -451,6 +451,18 @@ int pcie_enable_tph(struct pci_dev *pdev, int mode)
>  }
>  EXPORT_SYMBOL(pcie_enable_tph);
>  
> +/**
> + * pcie_tph_enabled_mode - Get current enabled TPH mode
> + * @pdev: PCI device
> + *
> + * Return the enabled TPH mode (IV/DS) or 0 if disabled.
> + */
> +int pcie_tph_enabled_mode(struct pci_dev *pdev)
> +{
> +	return pdev->tph_enabled ? pdev->tph_mode : 0;

Will returning 0 when TPH is disabled cause issues for generic callers
trying to differentiate between a disabled state and an active "No ST Mode"?

Since PCI_TPH_ST_NS_MODE is defined as 0, 0 is a valid active TPH mode
supported and enabled via pcie_enable_tph(pdev, 0).

If a generic driver uses this helper as a boolean check, for example:

    if (pcie_tph_enabled_mode(pdev))
        pcie_disable_tph(pdev);

it will incorrectly evaluate to false for NS mode, failing to properly
disable TPH.

Could this return a negative error code (e.g., -EINVAL or -ENODATA) when TPH
is disabled to avoid aliasing an active state with the disabled state?

> +}
> +EXPORT_SYMBOL(pcie_tph_enabled_mode);
> +
[ ... ]
> diff --git a/include/linux/pci-tph.h b/include/linux/pci-tph.h
> index 5772d48ea4446..28d0fa7621465 100644
> --- a/include/linux/pci-tph.h
> +++ b/include/linux/pci-tph.h
[ ... ]
> @@ -42,6 +43,8 @@ static inline int pcie_tph_get_cpu_st(struct pci_dev *dev,
>  static inline void pcie_disable_tph(struct pci_dev *pdev) { }
>  static inline int pcie_enable_tph(struct pci_dev *pdev, int mode)
>  { return -EINVAL; }
> +static inline int pcie_tph_enabled_mode(struct pci_dev *pdev)
> +{ return 0; }

Does this stub need to be updated to match as well if a negative error code
is used for the disabled state?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260518071701.25177-1-fengchengwen@huawei.com?part=3