1. Patchew
  2. linux
  3. [PATCH v3 0/6] bus: mhi: host: Enable SRIOV support in MHI driver
  4. View patch

[PATCH v3 2/6] bus: mhi: host: pci_generic: Read SUBSYSTEM_VENDOR_ID for VF's to check status

Vivek.Pernamitta@quicinc.com posted 6 patches 1 month, 1 week ago
There is a newer version of this series
[PATCH v3 2/6] bus: mhi: host: pci_generic: Read SUBSYSTEM_VENDOR_ID for VF's to check status
Posted by Vivek.Pernamitta@quicinc.com 1 month, 1 week ago 
From: Vivek Pernamitta <quic_vpernami@quicinc.com>

- In SR-IOV enabled devices, the VF DEVICE/VENDOR ID register returns
  FFFFh when read (as per PCIe SR-IOV spec section 3.4.1.1).
- To determine device activity, read the PCIe VENDOR_ID of the Physical
  Function (PF) instead.
- Disabled health check monitoring for Virtual Functions (VFs), as they
  are not physical functions and lack direct hardware control.
- This avoids unnecessary CPU cycles consumed by VF health checks, which
  are unintended and non-functional.

Signed-off-by: Vivek Pernamitta <quic_vpernami@quicinc.com>
Reviewed-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
---
 drivers/bus/mhi/host/pci_generic.c | 35 ++++++++++++++++++++++++-----------
 1 file changed, 24 insertions(+), 11 deletions(-)

diff --git a/drivers/bus/mhi/host/pci_generic.c b/drivers/bus/mhi/host/pci_generic.c
index 2967ed4cec29d1c1eeb581ed44cf9afcea90f533..df18627a9feb4f2d53ba1ca6a4e8087dc23c7873 100644
--- a/drivers/bus/mhi/host/pci_generic.c
+++ b/drivers/bus/mhi/host/pci_generic.c
@@ -1094,8 +1094,7 @@ static bool mhi_pci_is_alive(struct mhi_controller *mhi_cntrl)
 	struct pci_dev *pdev = to_pci_dev(mhi_cntrl->cntrl_dev);
 	u16 vendor = 0;
 
-	if (pci_read_config_word(pdev, PCI_VENDOR_ID, &vendor))
-		return false;
+	pci_read_config_word(pci_physfn(pdev), PCI_VENDOR_ID, &vendor);
 
 	if (vendor == (u16) ~0 || vendor == 0)
 		return false;
@@ -1205,7 +1204,9 @@ static void mhi_pci_recovery_work(struct work_struct *work)
 
 	dev_warn(&pdev->dev, "device recovery started\n");
 
-	timer_delete(&mhi_pdev->health_check_timer);
+	if (pdev->is_physfn)
+		timer_delete(&mhi_pdev->health_check_timer);
+
 	pm_runtime_forbid(&pdev->dev);
 
 	/* Clean up MHI state */
@@ -1232,7 +1233,10 @@ static void mhi_pci_recovery_work(struct work_struct *work)
 	dev_dbg(&pdev->dev, "Recovery completed\n");
 
 	set_bit(MHI_PCI_DEV_STARTED, &mhi_pdev->status);
-	mod_timer(&mhi_pdev->health_check_timer, jiffies + HEALTH_CHECK_PERIOD);
+
+	if (pdev->is_physfn)
+		mod_timer(&mhi_pdev->health_check_timer, jiffies + HEALTH_CHECK_PERIOD);
+
 	return;
 
 err_unprepare:
@@ -1319,7 +1323,9 @@ static int mhi_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 	else
 		mhi_cntrl_config = info->config;
 
-	timer_setup(&mhi_pdev->health_check_timer, health_check, 0);
+	/* Initialize health check monitor only for Physical functions */
+	if (pdev->is_physfn)
+		timer_setup(&mhi_pdev->health_check_timer, health_check, 0);
 
 	mhi_cntrl = &mhi_pdev->mhi_cntrl;
 
@@ -1383,7 +1389,8 @@ static int mhi_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 	set_bit(MHI_PCI_DEV_STARTED, &mhi_pdev->status);
 
 	/* start health check */
-	mod_timer(&mhi_pdev->health_check_timer, jiffies + HEALTH_CHECK_PERIOD);
+	if (pdev->is_physfn)
+		mod_timer(&mhi_pdev->health_check_timer, jiffies + HEALTH_CHECK_PERIOD);
 
 	/* Allow runtime suspend only if both PME from D3Hot and M3 are supported */
 	if (pci_pme_capable(pdev, PCI_D3hot) && !(info->no_m3)) {
@@ -1408,7 +1415,8 @@ static void mhi_pci_remove(struct pci_dev *pdev)
 	struct mhi_pci_device *mhi_pdev = pci_get_drvdata(pdev);
 	struct mhi_controller *mhi_cntrl = &mhi_pdev->mhi_cntrl;
 
-	timer_delete_sync(&mhi_pdev->health_check_timer);
+	if (pdev->is_physfn)
+		timer_delete_sync(&mhi_pdev->health_check_timer);
 	cancel_work_sync(&mhi_pdev->recovery_work);
 
 	if (test_and_clear_bit(MHI_PCI_DEV_STARTED, &mhi_pdev->status)) {
@@ -1436,7 +1444,8 @@ static void mhi_pci_reset_prepare(struct pci_dev *pdev)
 
 	dev_info(&pdev->dev, "reset\n");
 
-	timer_delete(&mhi_pdev->health_check_timer);
+	if (pdev->is_physfn)
+		timer_delete(&mhi_pdev->health_check_timer);
 
 	/* Clean up MHI state */
 	if (test_and_clear_bit(MHI_PCI_DEV_STARTED, &mhi_pdev->status)) {
@@ -1481,7 +1490,8 @@ static void mhi_pci_reset_done(struct pci_dev *pdev)
 	}
 
 	set_bit(MHI_PCI_DEV_STARTED, &mhi_pdev->status);
-	mod_timer(&mhi_pdev->health_check_timer, jiffies + HEALTH_CHECK_PERIOD);
+	if (pdev->is_physfn)
+		mod_timer(&mhi_pdev->health_check_timer, jiffies + HEALTH_CHECK_PERIOD);
 }
 
 static pci_ers_result_t mhi_pci_error_detected(struct pci_dev *pdev,
@@ -1546,7 +1556,9 @@ static int  __maybe_unused mhi_pci_runtime_suspend(struct device *dev)
 	if (test_and_set_bit(MHI_PCI_DEV_SUSPENDED, &mhi_pdev->status))
 		return 0;
 
-	timer_delete(&mhi_pdev->health_check_timer);
+	if (pdev->is_physfn)
+		timer_delete(&mhi_pdev->health_check_timer);
+
 	cancel_work_sync(&mhi_pdev->recovery_work);
 
 	if (!test_bit(MHI_PCI_DEV_STARTED, &mhi_pdev->status) ||
@@ -1597,7 +1609,8 @@ static int __maybe_unused mhi_pci_runtime_resume(struct device *dev)
 	}
 
 	/* Resume health check */
-	mod_timer(&mhi_pdev->health_check_timer, jiffies + HEALTH_CHECK_PERIOD);
+	if (pdev->is_physfn)
+		mod_timer(&mhi_pdev->health_check_timer, jiffies + HEALTH_CHECK_PERIOD);
 
 	/* It can be a remote wakeup (no mhi runtime_get), update access time */
 	pm_runtime_mark_last_busy(dev);

-- 
2.34.1
Re: [PATCH v3 2/6] bus: mhi: host: pci_generic: Read SUBSYSTEM_VENDOR_ID for VF's to check status
Posted by Manivannan Sadhasivam 4 weeks ago 
On Thu, Aug 21, 2025 at 06:25:34PM GMT, Vivek.Pernamitta@quicinc.com wrote:
> From: Vivek Pernamitta <quic_vpernami@quicinc.com>
> 
> - In SR-IOV enabled devices, the VF DEVICE/VENDOR ID register returns
>   FFFFh when read (as per PCIe SR-IOV spec section 3.4.1.1).
> - To determine device activity, read the PCIe VENDOR_ID of the Physical
>   Function (PF) instead.
> - Disabled health check monitoring for Virtual Functions (VFs), as they
>   are not physical functions and lack direct hardware control.
> - This avoids unnecessary CPU cycles consumed by VF health checks, which
>   are unintended and non-functional.
> 

Reword the description in imperative mood and try to come up with a paragraph
with continuity instead of points.

> Signed-off-by: Vivek Pernamitta <quic_vpernami@quicinc.com>
> Reviewed-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
> ---
>  drivers/bus/mhi/host/pci_generic.c | 35 ++++++++++++++++++++++++-----------
>  1 file changed, 24 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/bus/mhi/host/pci_generic.c b/drivers/bus/mhi/host/pci_generic.c
> index 2967ed4cec29d1c1eeb581ed44cf9afcea90f533..df18627a9feb4f2d53ba1ca6a4e8087dc23c7873 100644
> --- a/drivers/bus/mhi/host/pci_generic.c
> +++ b/drivers/bus/mhi/host/pci_generic.c
> @@ -1094,8 +1094,7 @@ static bool mhi_pci_is_alive(struct mhi_controller *mhi_cntrl)
>  	struct pci_dev *pdev = to_pci_dev(mhi_cntrl->cntrl_dev);
>  	u16 vendor = 0;
>  
> -	if (pci_read_config_word(pdev, PCI_VENDOR_ID, &vendor))
> -		return false;
> +	pci_read_config_word(pci_physfn(pdev), PCI_VENDOR_ID, &vendor);

You removed the return value check. Was that intentional?

>  
>  	if (vendor == (u16) ~0 || vendor == 0)
>  		return false;
> @@ -1205,7 +1204,9 @@ static void mhi_pci_recovery_work(struct work_struct *work)
>  
>  	dev_warn(&pdev->dev, "device recovery started\n");
>  
> -	timer_delete(&mhi_pdev->health_check_timer);
> +	if (pdev->is_physfn)
> +		timer_delete(&mhi_pdev->health_check_timer);

I really hate these if () checks everywhere. So I came up with the below macro:

	#define mhi_timer(pdev, timer_func, ...) \
		do { if (pdev->is_physfn) timer_func(__VA_ARGS__); } while(0)

	mhi_timer(pdev, timer_delete, &mhi_pdev->health_check_timer);

But mhi_timer() provides no information on what the macro is supposed to do and
I cannot think of a better name.

So maybe we have to live with the if () checks.

- Mani

-- 
மணிவண்ணன் சதாசிவம்

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.