Hi Jeffrey, On Wed, 25 Nov 2020 at 18:41, Jeffrey Hugo <jhugo@xxxxxxxxxxxxxx> wrote: > >>> @@ -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); > >> > >> Referring to MHI spec: > >> Hosts writes this register to trigger a reset. This can be used when the > >> host detects a timeout in the MHI protocol and can use the reset as a > >> last resort to recover the device. Host should first attempt an MHI > >> Reset procedure before resetting the entire device. > >> > >> What issue are you facing which requires you to do full device reset ? > >> Based on the spec recommendation, looks like this should be a last resort. > > > > On module unload/reload, the device does not go through cold reset and > > can be in error state on further reload, causing mhi power up to fail. > > This patch simply resets the device in remove so that it is in the > > exact same condition as before probing (not only MHI layer, but all > > the device context), I think it makes sense and prevents any > > unexpected state on further reloading. Note also that unloading the > > module (or unbinding the device) is not something that usually > > happens, except when the user does it explicitly for any reason. > > This seems unnecessary to me. Qaic has the same usecase, and the MHI > state machine reset is sufficient. Perhaps you have a unique edge case > though. > > However, you are implementing the soc_reset functionality in your > driver, when its a common MHI functionality, and is something I would > like to use. If you dig back, I proposed a sysfs extension to expose > that to userspace, but I have a desire to use it from my driver, same as > you. > > Would you please make MHI core changes to expose the soc_reset > functionality out so that multiple drivers can use a common implementation? I overlooked this reply, going to move that into MHI core, as you suggested. Thanks, Loic
