Re: [PATCH v3 3/9] mhi: pci-generic: Perform hard reset on remove

[Jeffrey Hugo <jhugo@codeaurora.org>]

On 11/27/2020 10:34 AM, Jeffrey Hugo wrote:
> On 11/26/2020 8:29 AM, Loic Poulain wrote:
>> Ensure that the device is hard-reset on remove to restore its initial
>> state and avoid further issues on subsequent probe.
>>
>> This has been tested with Telit FN980m module.
>>
>> Signed-off-by: Loic Poulain <loic.poulain@linaro.org>
>> ---
>>   drivers/bus/mhi/pci_generic.c | 11 +++++++++++
>>   1 file changed, 11 insertions(+)
>>
>> diff --git a/drivers/bus/mhi/pci_generic.c 
>> b/drivers/bus/mhi/pci_generic.c
>> index d3896ef..4363676 100644
>> --- a/drivers/bus/mhi/pci_generic.c
>> +++ b/drivers/bus/mhi/pci_generic.c
>> @@ -15,6 +15,8 @@
>>   #define MHI_PCI_DEFAULT_BAR_NUM 0
>> +#define DEV_RESET_REG (0xB0)
>> +
>>   /**
>>    * struct mhi_pci_dev_info - MHI PCI device specific information
>>    * @config: MHI controller configuration
>> @@ -166,6 +168,11 @@ static void mhi_pci_status_cb(struct 
>> mhi_controller *mhi_cntrl,
>>       /* Nothing to do for now */
>>   }
>> +static inline void mhi_pci_reset(struct mhi_controller *mhi_cntrl)
>> +{
>> +    writel(1, mhi_cntrl->regs + DEV_RESET_REG);
>> +}
>> +
>>   static int mhi_pci_claim(struct mhi_controller *mhi_cntrl,
>>                unsigned int bar_num, u64 dma_mask)
>>   {
>> @@ -329,6 +336,10 @@ static void mhi_pci_remove(struct pci_dev *pdev)
>>       mhi_power_down(mhi_cntrl, true);
>>       mhi_unprepare_after_power_down(mhi_cntrl);
>>       mhi_unregister_controller(mhi_cntrl);
>> +
>> +    /* MHI-layer reset could not be enough, always hard-reset the 
>> device */
>> +    mhi_pci_reset(mhi_cntrl);
>> +
>>       mhi_free_controller(mhi_cntrl);
>>   }
>>
> 
> Did you miss my eariler comment asking to make this functionality common?

Sorry, I missed your reply to my reply.

> 
> Also, I caution you to think carefully about this.  Its possible doing 
> this type of reset can take down the PCI link, and the host would need 
> to have PCI hotplug to recover.  If not, the device will become 
> inaccessable until the host reboots.
> 


-- 
Jeffrey Hugo
Qualcomm Technologies, Inc. is a member of the
Code Aurora Forum, a Linux Foundation Collaborative Project.