RE: [PATCH] PCI: rzg3s-host: Fix NVMe resume errors after s2idle

[Ovidiu Panait]

Hi Manivannan,

> 
> On 2026-10-05 11:02 +0000, Ovidiu Panait wrote:
> > On the Renesas RZ/V2H platform, when ASPM is enabled, resuming from
> s2idle
> > with an NVMe SSD connected fails with:
> >
> >   nvme nvme0: I/O tag 27 (001b) opcode 0x9 (Admin Cmd) QID 0 timeout,
> reset controller
> >   nvme nvme0: Disabling device after reset failure: -19
> >
> > This happens because during s2idle, when ASPM is enabled, nvme_suspend()
> > does not shut the drive down. It leaves it in D0, puts it in its lowest
> > NVMe power state, so that resume only has to restore the previous power
> > state instead of a full reinitialization. This assumes the drive keeps
> > its context over the suspend.
> >
> > However, rzg3s_pcie_suspend_noirq() doesn't take this into account and
> > it always powers down the controller and asserts the controller resets,
> > including PERST#. That resets the drive, and the command the NVMe driver
> > sends on resume never completes.
> >
> > To fix this, similar to the dwc driver, use
> > pci_host_common_d3cold_possible() to check if the controller should be
> > powered down and keep the controller running otherwise. Do this only for
> > s2idle, as deep sleep could power off the SoC and neither the power
> domain
> > nor the driver will restore the controller clocks and resets on resume.
> >
> > Fixes: 7ef502fb35b2 ("PCI: Add Renesas RZ/G3S host controller driver")
> > Signed-off-by: Ovidiu Panait <ovidiu.panait.rb@xxxxxxxxxxx>
> > ---
> > Note: this fix depends on the following series:
> > https://lore.kernel.org/all/20261002110935.2004349-1-
> claudiu.beznea@xxxxxxxxxx/
> >
> >  drivers/pci/controller/pcie-rzg3s-host.c | 8 ++++++++
> >  1 file changed, 8 insertions(+)
> >
> > diff --git a/drivers/pci/controller/pcie-rzg3s-host.c
> b/drivers/pci/controller/pcie-rzg3s-host.c
> > index eea215818633..9fc1a5a4bed8 100644
> > --- a/drivers/pci/controller/pcie-rzg3s-host.c
> > +++ b/drivers/pci/controller/pcie-rzg3s-host.c
> > @@ -2272,6 +2272,14 @@ static int rzg3s_pcie_probe(struct
> platform_device *pdev)
> >  static int rzg3s_pcie_suspend_noirq(struct device *dev)
> >  {
> >       struct rzg3s_pcie_host *host = dev_get_drvdata(dev);
> > +     struct pci_host_bridge *bridge = pci_host_bridge_from_priv(host);
> > +     bool pme_capable;
> > +
> > +     if (!pm_suspend_via_firmware() &&
> 
> For checking S2R, you need to use:
> 
>         if (pm_suspend_target_state != PM_SUSPEND_MEM)
> 

Will update in v2.

> > +         !pci_host_common_d3cold_possible(bridge, &pme_capable)) {
> > +             device_set_awake_path(dev);
> 
> Why? To keep power domain ON? This is not justified in the commit message.
> 

This was added so that the clocks are not turned off by the PM domain.

I'll add a note in the commit message for this in v2.

Thanks,
Ovidiu

> - Mani
>