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

[Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>]

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);
>   }
>   
>