Re: [PATCH 2/8] mhi: pci-generic: Perform hard reset on remove

[Loic Poulain <loic.poulain@linaro.org>]

Hi Jeffrey,

On Wed, 25 Nov 2020 at 18:41, Jeffrey Hugo <jhugo@codeaurora.org> 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