Re: [PATCH] bus: mhi: pci_generic: Fix erroneous system suspend abort

[Len Brown]

Hello Loic,

I agree that the device and the driver should be debugged, and perhaps
re-implemented.

Indeed, I'm happy to share the dmesg from 28,000 suspend/resume cycles
on this machine
over the last few months to support that effort, should somebody else
undertake it.

But today, this device is allowing its internal contagion to spread
into Linux and interfere with
system wide suspend due to an obvious 1-line programming error.

This 1-line fix should be applied NOW, without waiting for internal
mhi internal or development.

thanks,
-Len

ps.  I filed this over a year ago:
https://bugzilla.kernel.org/show_bug.cgi?id=220020

On Thu, Nov 12, 2026 at 5:23 AM Loic Poulain
<loic.poulain@xxxxxxxxxxxxxxxx> wrote:
>
> Hi Len,
>
> On Wed, Nov 11, 2026 at 6:23 PM Len Brown <lenb@xxxxxxxxxx> wrote:
> >
> > From: Len Brown <len.brown@xxxxxxxxx>
> >
> > mhi sometimes aborts system suspend on the thinkpad T16 gen3:
> >
> > mhi-pci-generic 0000:08:00.0: failed to suspend device: -16
> > mhi-pci-generic 0000:08:00.0: PM: pci_pm_suspend(): mhi_pci_suspend [mhi_pci_generic] returns -16
> > mhi-pci-generic 0000:08:00.0: PM: dpm_run_callback(): pci_pm_suspend returns -16
> > mhi-pci-generic 0000:08:00.0: PM: pci_pm_suspend returned -16 after 15145 usecs
> > mhi-pci-generic 0000:08:00.0: PM: failed to suspend async: error -16
> >
> > When a driver returns -EBUSY for its runtime suspend method,
> > that means "try again later".
> >
> > But when a driver returns anything but 0 from its system-suspend
> > method, that means "abort system suspend".
> >
> > Always return 0 from mhi_pci_suspend().
>
> Two things:
> - I think we should simply use pm_runtime_force_suspend() /
> pm_runtime_force_resume() for the system sleep callbacks instead of
> implementing separate suspend/resume handling.
> - Rather than silently ignoring the error, I think it would be worth
> looking at the mhi_pm_suspend() implementation to understand what can
> be returned here and under which conditions the MHI device fails to
> suspend.
>
> Regards,
> Loic
>
> >
> > Fixes: d3800c1dce24 ("bus: mhi: pci_generic: Add support for runtime PM")
> > Signed-off-by: Len Brown <len.brown@xxxxxxxxx>
> > ---
> >  drivers/bus/mhi/host/pci_generic.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/bus/mhi/host/pci_generic.c b/drivers/bus/mhi/host/pci_generic.c
> > index 7193a384cca4..b036fa6e06c4 100644
> > --- a/drivers/bus/mhi/host/pci_generic.c
> > +++ b/drivers/bus/mhi/host/pci_generic.c
> > @@ -1727,7 +1727,8 @@ static int __maybe_unused mhi_pci_runtime_resume(struct device *dev)
> >  static int  __maybe_unused mhi_pci_suspend(struct device *dev)
> >  {
> >         pm_runtime_disable(dev);
> > -       return mhi_pci_runtime_suspend(dev);
> > +       mhi_pci_runtime_suspend(dev);
> > +       return 0;
> >  }
> >
> >  static int __maybe_unused mhi_pci_resume(struct device *dev)
> > --
> > 2.45.2
> >



-- 
Len Brown, Intel Open Source Technology Center