[PATCH 4/5] bus: mhi: host: pci_generic: Remove MHI driver and ensure graceful device recovery

Vivek.Pernamitta@quicinc.com posted 5 patches 3 months ago
There is a newer version of this series
[PATCH 4/5] bus: mhi: host: pci_generic: Remove MHI driver and ensure graceful device recovery
Posted by Vivek.Pernamitta@quicinc.com 3 months ago
From: Vivek Pernamitta <quic_vpernami@quicinc.com>

When the MHI driver is removed from the host side, it is crucial to ensure
graceful recovery of the device. To achieve this, the host driver will
perform the following steps:

1. Disable SRIOV for any SRIOV-enabled devices on the Physical Function.
2. Perform a SOC_RESET on Physical Function (PF).

Disabling SRIOV ensures that all virtual functions are properly shut down,
preventing any potential issues during the reset process. Performing
SOC_RESET on each physical function guarantees that the device is fully
reset and ready for subsequent operations.

Signed-off-by: Vivek Pernamitta <quic_vpernami@quicinc.com>
---
 drivers/bus/mhi/host/pci_generic.c | 33 +++++++++++++++++++++++++++++++--
 1 file changed, 31 insertions(+), 2 deletions(-)

diff --git a/drivers/bus/mhi/host/pci_generic.c b/drivers/bus/mhi/host/pci_generic.c
index 938f37d306a18b9a47f302df85697f837c225f0d..ff9263d5dc4b54956c6ca4403e7b0b2429d0700e 100644
--- a/drivers/bus/mhi/host/pci_generic.c
+++ b/drivers/bus/mhi/host/pci_generic.c
@@ -45,6 +45,8 @@
  * @sideband_wake: Devices using dedicated sideband GPIO for wakeup instead
  *		   of inband wake support (such as sdx24)
  * @no_m3: M3 not supported
+ * @bool reset_on_driver_unbind: Set true for devices support SOC reset and
+ *				 perform it when unbinding driver
  */
 struct mhi_pci_dev_info {
 	const struct mhi_controller_config *config;
@@ -58,6 +60,7 @@ struct mhi_pci_dev_info {
 	unsigned int mru_default;
 	bool sideband_wake;
 	bool no_m3;
+	bool reset_on_driver_unbind;
 };
 
 #define MHI_CHANNEL_CONFIG_UL(ch_num, ch_name, el_count, ev_ring) \
@@ -300,6 +303,7 @@ static const struct mhi_pci_dev_info mhi_qcom_qdu100_info = {
 	.dma_data_width = 32,
 	.sideband_wake = false,
 	.no_m3 = true,
+	.reset_on_driver_unbind = true,
 };
 
 static const struct mhi_channel_config mhi_qcom_sa8775p_channels[] = {
@@ -970,6 +974,7 @@ struct mhi_pci_device {
 	struct work_struct recovery_work;
 	struct timer_list health_check_timer;
 	unsigned long status;
+	bool reset_on_driver_unbind;
 };
 
 static int mhi_pci_read_reg(struct mhi_controller *mhi_cntrl,
@@ -1270,6 +1275,11 @@ static int mhi_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 	mhi_cntrl->mru = info->mru_default;
 	mhi_cntrl->name = info->name;
 
+	/* Assign reset functionalities only for PF */
+	if (pdev->is_physfn)
+		mhi_pdev->reset_on_driver_unbind = info->reset_on_driver_unbind;
+
+
 	if (info->edl_trigger)
 		mhi_cntrl->edl_trigger = mhi_pci_generic_edl_trigger;
 
@@ -1336,7 +1346,7 @@ static int mhi_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 	return err;
 }
 
-static void mhi_pci_remove(struct pci_dev *pdev)
+static void mhi_pci_resource_deinit(struct pci_dev *pdev)
 {
 	struct mhi_pci_device *mhi_pdev = pci_get_drvdata(pdev);
 	struct mhi_controller *mhi_cntrl = &mhi_pdev->mhi_cntrl;
@@ -1352,13 +1362,32 @@ static void mhi_pci_remove(struct pci_dev *pdev)
 	/* balancing probe put_noidle */
 	if (pci_pme_capable(pdev, PCI_D3hot))
 		pm_runtime_get_noresume(&pdev->dev);
+}
 
+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;
+
+	/* Disable SRIOV */
+	pci_disable_sriov(pdev);
+	mhi_pci_resource_deinit(pdev);
+	if (mhi_pdev->reset_on_driver_unbind) {
+		dev_info(&pdev->dev, "perform SOC reset\n");
+		mhi_soc_reset(mhi_cntrl);
+	}
+
+	/* Perform FLR if supported*/
 	mhi_unregister_controller(mhi_cntrl);
 }
 
 static void mhi_pci_shutdown(struct pci_dev *pdev)
 {
-	mhi_pci_remove(pdev);
+	struct mhi_pci_device *mhi_pdev = pci_get_drvdata(pdev);
+	struct mhi_controller *mhi_cntrl = &mhi_pdev->mhi_cntrl;
+
+	mhi_pci_resource_deinit(pdev);
+	mhi_unregister_controller(mhi_cntrl);
 	pci_set_power_state(pdev, PCI_D3hot);
 }
 

-- 
2.34.1
Re: [PATCH 4/5] bus: mhi: host: pci_generic: Remove MHI driver and ensure graceful device recovery
Posted by Krishna Chaitanya Chundru 3 months ago

On 7/3/2025 8:39 PM, Vivek.Pernamitta@quicinc.com wrote:
> From: Vivek Pernamitta <quic_vpernami@quicinc.com>
> 
> When the MHI driver is removed from the host side, it is crucial to ensure
> graceful recovery of the device. To achieve this, the host driver will
> perform the following steps:
> 
> 1. Disable SRIOV for any SRIOV-enabled devices on the Physical Function.
> 2. Perform a SOC_RESET on Physical Function (PF).
> 
> Disabling SRIOV ensures that all virtual functions are properly shut down,
> preventing any potential issues during the reset process. Performing
> SOC_RESET on each physical function guarantees that the device is fully
> reset and ready for subsequent operations.
> 
> Signed-off-by: Vivek Pernamitta <quic_vpernami@quicinc.com>
> ---
>   drivers/bus/mhi/host/pci_generic.c | 33 +++++++++++++++++++++++++++++++--
>   1 file changed, 31 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/bus/mhi/host/pci_generic.c b/drivers/bus/mhi/host/pci_generic.c
> index 938f37d306a18b9a47f302df85697f837c225f0d..ff9263d5dc4b54956c6ca4403e7b0b2429d0700e 100644
> --- a/drivers/bus/mhi/host/pci_generic.c
> +++ b/drivers/bus/mhi/host/pci_generic.c
> @@ -45,6 +45,8 @@
>    * @sideband_wake: Devices using dedicated sideband GPIO for wakeup instead
>    *		   of inband wake support (such as sdx24)
>    * @no_m3: M3 not supported
> + * @bool reset_on_driver_unbind: Set true for devices support SOC reset and
> + *				 perform it when unbinding driver
>    */
>   struct mhi_pci_dev_info {
>   	const struct mhi_controller_config *config;
> @@ -58,6 +60,7 @@ struct mhi_pci_dev_info {
>   	unsigned int mru_default;
>   	bool sideband_wake;
>   	bool no_m3;
> +	bool reset_on_driver_unbind;
>   };
>   
>   #define MHI_CHANNEL_CONFIG_UL(ch_num, ch_name, el_count, ev_ring) \
> @@ -300,6 +303,7 @@ static const struct mhi_pci_dev_info mhi_qcom_qdu100_info = {
>   	.dma_data_width = 32,
>   	.sideband_wake = false,
>   	.no_m3 = true,
> +	.reset_on_driver_unbind = true,
>   };
>   
>   static const struct mhi_channel_config mhi_qcom_sa8775p_channels[] = {
> @@ -970,6 +974,7 @@ struct mhi_pci_device {
>   	struct work_struct recovery_work;
>   	struct timer_list health_check_timer;
>   	unsigned long status;
> +	bool reset_on_driver_unbind;
>   };
>   
>   static int mhi_pci_read_reg(struct mhi_controller *mhi_cntrl,
> @@ -1270,6 +1275,11 @@ static int mhi_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
>   	mhi_cntrl->mru = info->mru_default;
>   	mhi_cntrl->name = info->name;
>   
> +	/* Assign reset functionalities only for PF */
> +	if (pdev->is_physfn)
> +		mhi_pdev->reset_on_driver_unbind = info->reset_on_driver_unbind;
> +
> +
>   	if (info->edl_trigger)
>   		mhi_cntrl->edl_trigger = mhi_pci_generic_edl_trigger;
>   
> @@ -1336,7 +1346,7 @@ static int mhi_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
>   	return err;
>   }
>   
> -static void mhi_pci_remove(struct pci_dev *pdev)
> +static void mhi_pci_resource_deinit(struct pci_dev *pdev)
>   {
>   	struct mhi_pci_device *mhi_pdev = pci_get_drvdata(pdev);
>   	struct mhi_controller *mhi_cntrl = &mhi_pdev->mhi_cntrl;
> @@ -1352,13 +1362,32 @@ static void mhi_pci_remove(struct pci_dev *pdev)
>   	/* balancing probe put_noidle */
>   	if (pci_pme_capable(pdev, PCI_D3hot))
>   		pm_runtime_get_noresume(&pdev->dev);
> +}
>   
> +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;
> +
> +	/* Disable SRIOV */
> +	pci_disable_sriov(pdev);
> +	mhi_pci_resource_deinit(pdev);
> +	if (mhi_pdev->reset_on_driver_unbind) {
> +		dev_info(&pdev->dev, "perform SOC reset\n");
> +		mhi_soc_reset(mhi_cntrl);
> +	}
> +
> +	/* Perform FLR if supported*/
Misleading comment.
>   	mhi_unregister_controller(mhi_cntrl);
>   }
>   
>   static void mhi_pci_shutdown(struct pci_dev *pdev)
>   {
> -	mhi_pci_remove(pdev);
why can't we use this same as mhi_pci_remove.

- Krishna Chaitanya.
> +	struct mhi_pci_device *mhi_pdev = pci_get_drvdata(pdev);
> +	struct mhi_controller *mhi_cntrl = &mhi_pdev->mhi_cntrl;
> +
> +	mhi_pci_resource_deinit(pdev);
> +	mhi_unregister_controller(mhi_cntrl);
>   	pci_set_power_state(pdev, PCI_D3hot);
>   }
>   
>
Re: [PATCH 4/5] bus: mhi: host: pci_generic: Remove MHI driver and ensure graceful device recovery
Posted by kernel test robot 3 months ago
Hi,

kernel test robot noticed the following build warnings:

[auto build test WARNING on 1343433ed38923a21425c602e92120a1f1db5f7a]

url:    https://github.com/intel-lab-lkp/linux/commits/Vivek-Pernamitta-quicinc-com/bus-mhi-host-pci_generic-Add-SRIOV-support-for-PCIe-device/20250703-231724
base:   1343433ed38923a21425c602e92120a1f1db5f7a
patch link:    https://lore.kernel.org/r/20250703-sriov_vdev_next-20250630-v1-4-87071d1047e3%40quicinc.com
patch subject: [PATCH 4/5] bus: mhi: host: pci_generic: Remove MHI driver and ensure graceful device recovery
config: i386-buildonly-randconfig-001-20250704 (https://download.01.org/0day-ci/archive/20250704/202507041755.evB3U2Ju-lkp@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14+deb12u1) 12.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250704/202507041755.evB3U2Ju-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202507041755.evB3U2Ju-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> Warning: drivers/bus/mhi/host/pci_generic.c:63 struct member 'reset_on_driver_unbind' not described in 'mhi_pci_dev_info'

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki