* Re: [PATCH] nvme-pci: Save PCI state before putting drive into deepest state
[not found] <1568245353-13787-1-git-send-email-mario.limonciello@dell.com>
@ 2019-09-18 21:31 ` Rafael J. Wysocki
2019-09-18 21:42 ` Rafael J. Wysocki
2019-09-18 21:43 ` Mario.Limonciello
0 siblings, 2 replies; 6+ messages in thread
From: Rafael J. Wysocki @ 2019-09-18 21:31 UTC (permalink / raw)
To: Mario Limonciello
Cc: Keith Busch, Jens Axboe, Christoph Hellwig, Sagi Grimberg,
linux-nvme, LKML, Ryan Hong, Crag Wang, sjg, Jared Dominguez,
Linux PCI, Linux PM
On Thursday, September 12, 2019 1:42:33 AM CEST Mario Limonciello wrote:
> The action of saving the PCI state will cause numerous PCI configuration
> space reads which depending upon the vendor implementation may cause
> the drive to exit the deepest NVMe state.
>
> In these cases ASPM will typically resolve the PCIe link state and APST
> may resolve the NVMe power state. However it has also been observed
> that this register access after quiesced will cause PC10 failure
> on some device combinations.
>
> To resolve this, move the PCI state saving to before SetFeatures has been
> called. This has been proven to resolve the issue across a 5000 sample
> test on previously failing disk/system combinations.
This sounds reasonable to me, but it would be nice to CC that to linux-pm
and/or linux-pci too.
> Signed-off-by: Mario Limonciello <mario.limonciello@dell.com>
> ---
> drivers/nvme/host/pci.c | 13 +++++++------
> 1 file changed, 7 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
> index 732d5b6..9b3fed4 100644
> --- a/drivers/nvme/host/pci.c
> +++ b/drivers/nvme/host/pci.c
> @@ -2894,6 +2894,13 @@ static int nvme_suspend(struct device *dev)
> if (ret < 0)
> goto unfreeze;
>
> + /*
> + * A saved state prevents pci pm from generically controlling the
> + * device's power. If we're using protocol specific settings, we don't
> + * want pci interfering.
> + */
> + pci_save_state(pdev);
> +
> ret = nvme_set_power_state(ctrl, ctrl->npss);
> if (ret < 0)
> goto unfreeze;
> @@ -2908,12 +2915,6 @@ static int nvme_suspend(struct device *dev)
This is the case in which the PCI layer is expected to put the device into
D3, so you need
pdev->state_saved = 0;
at this point, because you have saved the config space already.
> ret = 0;
> goto unfreeze;
And here you don't need to jump to "unfreeze" any more.
> }
> - /*
> - * A saved state prevents pci pm from generically controlling the
> - * device's power. If we're using protocol specific settings, we don't
> - * want pci interfering.
> - */
> - pci_save_state(pdev);
> unfreeze:
> nvme_unfreeze(ctrl);
> return ret;
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] nvme-pci: Save PCI state before putting drive into deepest state
2019-09-18 21:31 ` [PATCH] nvme-pci: Save PCI state before putting drive into deepest state Rafael J. Wysocki
@ 2019-09-18 21:42 ` Rafael J. Wysocki
2019-09-18 21:43 ` Mario.Limonciello
1 sibling, 0 replies; 6+ messages in thread
From: Rafael J. Wysocki @ 2019-09-18 21:42 UTC (permalink / raw)
To: Mario Limonciello, Keith Busch
Cc: Jens Axboe, Christoph Hellwig, Sagi Grimberg, linux-nvme, LKML,
Ryan Hong, Crag Wang, sjg, Jared Dominguez, Linux PCI, Linux PM
On Wednesday, September 18, 2019 11:31:19 PM CEST Rafael J. Wysocki wrote:
> On Thursday, September 12, 2019 1:42:33 AM CEST Mario Limonciello wrote:
> > The action of saving the PCI state will cause numerous PCI configuration
> > space reads which depending upon the vendor implementation may cause
> > the drive to exit the deepest NVMe state.
> >
> > In these cases ASPM will typically resolve the PCIe link state and APST
> > may resolve the NVMe power state. However it has also been observed
> > that this register access after quiesced will cause PC10 failure
> > on some device combinations.
> >
> > To resolve this, move the PCI state saving to before SetFeatures has been
> > called. This has been proven to resolve the issue across a 5000 sample
> > test on previously failing disk/system combinations.
>
> This sounds reasonable to me, but it would be nice to CC that to linux-pm
> and/or linux-pci too.
>
> > Signed-off-by: Mario Limonciello <mario.limonciello@dell.com>
> > ---
> > drivers/nvme/host/pci.c | 13 +++++++------
> > 1 file changed, 7 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
> > index 732d5b6..9b3fed4 100644
> > --- a/drivers/nvme/host/pci.c
> > +++ b/drivers/nvme/host/pci.c
> > @@ -2894,6 +2894,13 @@ static int nvme_suspend(struct device *dev)
> > if (ret < 0)
> > goto unfreeze;
> >
> > + /*
> > + * A saved state prevents pci pm from generically controlling the
> > + * device's power. If we're using protocol specific settings, we don't
> > + * want pci interfering.
> > + */
> > + pci_save_state(pdev);
> > +
> > ret = nvme_set_power_state(ctrl, ctrl->npss);
> > if (ret < 0)
> > goto unfreeze;
> > @@ -2908,12 +2915,6 @@ static int nvme_suspend(struct device *dev)
>
> This is the case in which the PCI layer is expected to put the device into
> D3, so you need
>
> pdev->state_saved = 0;
>
> at this point, because you have saved the config space already.
>
> > ret = 0;
> > goto unfreeze;
>
> And here you don't need to jump to "unfreeze" any more.
BTW, doing nvme_dev_disable() before nvme_unfreeze() looks odd to me.
Maybe it would be better to do "unfreeze" and then "disable" in this case?
>
> > }
> > - /*
> > - * A saved state prevents pci pm from generically controlling the
> > - * device's power. If we're using protocol specific settings, we don't
> > - * want pci interfering.
> > - */
> > - pci_save_state(pdev);
> > unfreeze:
> > nvme_unfreeze(ctrl);
> > return ret;
> >
>
>
>
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [PATCH] nvme-pci: Save PCI state before putting drive into deepest state
2019-09-18 21:31 ` [PATCH] nvme-pci: Save PCI state before putting drive into deepest state Rafael J. Wysocki
2019-09-18 21:42 ` Rafael J. Wysocki
@ 2019-09-18 21:43 ` Mario.Limonciello
2019-09-18 21:57 ` Rafael J. Wysocki
1 sibling, 1 reply; 6+ messages in thread
From: Mario.Limonciello @ 2019-09-18 21:43 UTC (permalink / raw)
To: rjw
Cc: kbusch, axboe, hch, sagi, linux-nvme, linux-kernel, Ryan.Hong,
Crag.Wang, sjg, Jared.Dominguez, linux-pci, linux-pm
> -----Original Message-----
> From: Rafael J. Wysocki <rjw@rjwysocki.net>
> Sent: Wednesday, September 18, 2019 4:31 PM
> To: Limonciello, Mario
> Cc: Keith Busch; Jens Axboe; Christoph Hellwig; Sagi Grimberg; linux-
> nvme@lists.infradead.org; LKML; Hong, Ryan; Wang, Crag; sjg@google.com;
> Dominguez, Jared; Linux PCI; Linux PM
> Subject: Re: [PATCH] nvme-pci: Save PCI state before putting drive into deepest
> state
>
>
> [EXTERNAL EMAIL]
>
> On Thursday, September 12, 2019 1:42:33 AM CEST Mario Limonciello wrote:
> > The action of saving the PCI state will cause numerous PCI configuration
> > space reads which depending upon the vendor implementation may cause
> > the drive to exit the deepest NVMe state.
> >
> > In these cases ASPM will typically resolve the PCIe link state and APST
> > may resolve the NVMe power state. However it has also been observed
> > that this register access after quiesced will cause PC10 failure
> > on some device combinations.
> >
> > To resolve this, move the PCI state saving to before SetFeatures has been
> > called. This has been proven to resolve the issue across a 5000 sample
> > test on previously failing disk/system combinations.
>
> This sounds reasonable to me, but it would be nice to CC that to linux-pm
> and/or linux-pci too.
>
> > Signed-off-by: Mario Limonciello <mario.limonciello@dell.com>
> > ---
> > drivers/nvme/host/pci.c | 13 +++++++------
> > 1 file changed, 7 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
> > index 732d5b6..9b3fed4 100644
> > --- a/drivers/nvme/host/pci.c
> > +++ b/drivers/nvme/host/pci.c
> > @@ -2894,6 +2894,13 @@ static int nvme_suspend(struct device *dev)
> > if (ret < 0)
> > goto unfreeze;
> >
> > + /*
> > + * A saved state prevents pci pm from generically controlling the
> > + * device's power. If we're using protocol specific settings, we don't
> > + * want pci interfering.
> > + */
> > + pci_save_state(pdev);
> > +
> > ret = nvme_set_power_state(ctrl, ctrl->npss);
> > if (ret < 0)
> > goto unfreeze;
> > @@ -2908,12 +2915,6 @@ static int nvme_suspend(struct device *dev)
>
> This is the case in which the PCI layer is expected to put the device into
> D3, so you need
>
> pdev->state_saved = 0;
>
> at this point, because you have saved the config space already.
>
> > ret = 0;
> > goto unfreeze;
>
> And here you don't need to jump to "unfreeze" any more.
>
> > }
> > - /*
> > - * A saved state prevents pci pm from generically controlling the
> > - * device's power. If we're using protocol specific settings, we don't
> > - * want pci interfering.
> > - */
> > - pci_save_state(pdev);
> > unfreeze:
> > nvme_unfreeze(ctrl);
> > return ret;
> >
>
>
>
Thanks, I actually followed up with something along that line in a v2 sent out
today. My apology you weren't in CC, but here is a weblink to it.
http://lists.infradead.org/pipermail/linux-nvme/2019-September/027251.html
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] nvme-pci: Save PCI state before putting drive into deepest state
2019-09-18 21:43 ` Mario.Limonciello
@ 2019-09-18 21:57 ` Rafael J. Wysocki
2019-09-18 22:00 ` Mario.Limonciello
0 siblings, 1 reply; 6+ messages in thread
From: Rafael J. Wysocki @ 2019-09-18 21:57 UTC (permalink / raw)
To: Mario.Limonciello
Cc: kbusch, axboe, hch, sagi, linux-nvme, linux-kernel, Ryan.Hong,
Crag.Wang, sjg, Jared.Dominguez, linux-pci, linux-pm
On Wednesday, September 18, 2019 11:43:28 PM CEST Mario.Limonciello@dell.com wrote:
> > -----Original Message-----
> > From: Rafael J. Wysocki <rjw@rjwysocki.net>
> > Sent: Wednesday, September 18, 2019 4:31 PM
> > To: Limonciello, Mario
> > Cc: Keith Busch; Jens Axboe; Christoph Hellwig; Sagi Grimberg; linux-
> > nvme@lists.infradead.org; LKML; Hong, Ryan; Wang, Crag; sjg@google.com;
> > Dominguez, Jared; Linux PCI; Linux PM
> > Subject: Re: [PATCH] nvme-pci: Save PCI state before putting drive into deepest
> > state
> >
> >
> > [EXTERNAL EMAIL]
> >
> > On Thursday, September 12, 2019 1:42:33 AM CEST Mario Limonciello wrote:
> > > The action of saving the PCI state will cause numerous PCI configuration
> > > space reads which depending upon the vendor implementation may cause
> > > the drive to exit the deepest NVMe state.
> > >
> > > In these cases ASPM will typically resolve the PCIe link state and APST
> > > may resolve the NVMe power state. However it has also been observed
> > > that this register access after quiesced will cause PC10 failure
> > > on some device combinations.
> > >
> > > To resolve this, move the PCI state saving to before SetFeatures has been
> > > called. This has been proven to resolve the issue across a 5000 sample
> > > test on previously failing disk/system combinations.
> >
> > This sounds reasonable to me, but it would be nice to CC that to linux-pm
> > and/or linux-pci too.
> >
> > > Signed-off-by: Mario Limonciello <mario.limonciello@dell.com>
> > > ---
> > > drivers/nvme/host/pci.c | 13 +++++++------
> > > 1 file changed, 7 insertions(+), 6 deletions(-)
> > >
> > > diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
> > > index 732d5b6..9b3fed4 100644
> > > --- a/drivers/nvme/host/pci.c
> > > +++ b/drivers/nvme/host/pci.c
> > > @@ -2894,6 +2894,13 @@ static int nvme_suspend(struct device *dev)
> > > if (ret < 0)
> > > goto unfreeze;
> > >
> > > + /*
> > > + * A saved state prevents pci pm from generically controlling the
> > > + * device's power. If we're using protocol specific settings, we don't
> > > + * want pci interfering.
> > > + */
> > > + pci_save_state(pdev);
> > > +
> > > ret = nvme_set_power_state(ctrl, ctrl->npss);
> > > if (ret < 0)
> > > goto unfreeze;
> > > @@ -2908,12 +2915,6 @@ static int nvme_suspend(struct device *dev)
> >
> > This is the case in which the PCI layer is expected to put the device into
> > D3, so you need
> >
> > pdev->state_saved = 0;
> >
> > at this point, because you have saved the config space already.
> >
> > > ret = 0;
> > > goto unfreeze;
> >
> > And here you don't need to jump to "unfreeze" any more.
> >
> > > }
> > > - /*
> > > - * A saved state prevents pci pm from generically controlling the
> > > - * device's power. If we're using protocol specific settings, we don't
> > > - * want pci interfering.
> > > - */
> > > - pci_save_state(pdev);
> > > unfreeze:
> > > nvme_unfreeze(ctrl);
> > > return ret;
> > >
> >
> >
> >
>
> Thanks, I actually followed up with something along that line in a v2 sent out
> today. My apology you weren't in CC, but here is a weblink to it.
> http://lists.infradead.org/pipermail/linux-nvme/2019-September/027251.html
>
I don't think that pci_load_saved_state() will work, because it sets
state_saved at the end again (if all goes well). You simply only need to
clear state_saved here.
^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [PATCH] nvme-pci: Save PCI state before putting drive into deepest state
2019-09-18 21:57 ` Rafael J. Wysocki
@ 2019-09-18 22:00 ` Mario.Limonciello
2019-09-18 22:03 ` Rafael J. Wysocki
0 siblings, 1 reply; 6+ messages in thread
From: Mario.Limonciello @ 2019-09-18 22:00 UTC (permalink / raw)
To: rjw
Cc: kbusch, axboe, hch, sagi, linux-nvme, linux-kernel, Ryan.Hong,
Crag.Wang, sjg, Jared.Dominguez, linux-pci, linux-pm
> -----Original Message-----
> From: Rafael J. Wysocki <rjw@rjwysocki.net>
> Sent: Wednesday, September 18, 2019 4:57 PM
> To: Limonciello, Mario
> Cc: kbusch@kernel.org; axboe@fb.com; hch@lst.de; sagi@grimberg.me; linux-
> nvme@lists.infradead.org; linux-kernel@vger.kernel.org; Hong, Ryan; Wang,
> Crag; sjg@google.com; Dominguez, Jared; linux-pci@vger.kernel.org; linux-
> pm@vger.kernel.org
> Subject: Re: [PATCH] nvme-pci: Save PCI state before putting drive into deepest
> state
>
>
> [EXTERNAL EMAIL]
>
> On Wednesday, September 18, 2019 11:43:28 PM CEST
> Mario.Limonciello@dell.com wrote:
> > > -----Original Message-----
> > > From: Rafael J. Wysocki <rjw@rjwysocki.net>
> > > Sent: Wednesday, September 18, 2019 4:31 PM
> > > To: Limonciello, Mario
> > > Cc: Keith Busch; Jens Axboe; Christoph Hellwig; Sagi Grimberg; linux-
> > > nvme@lists.infradead.org; LKML; Hong, Ryan; Wang, Crag; sjg@google.com;
> > > Dominguez, Jared; Linux PCI; Linux PM
> > > Subject: Re: [PATCH] nvme-pci: Save PCI state before putting drive into
> deepest
> > > state
> > >
> > >
> > > [EXTERNAL EMAIL]
> > >
> > > On Thursday, September 12, 2019 1:42:33 AM CEST Mario Limonciello wrote:
> > > > The action of saving the PCI state will cause numerous PCI configuration
> > > > space reads which depending upon the vendor implementation may cause
> > > > the drive to exit the deepest NVMe state.
> > > >
> > > > In these cases ASPM will typically resolve the PCIe link state and APST
> > > > may resolve the NVMe power state. However it has also been observed
> > > > that this register access after quiesced will cause PC10 failure
> > > > on some device combinations.
> > > >
> > > > To resolve this, move the PCI state saving to before SetFeatures has been
> > > > called. This has been proven to resolve the issue across a 5000 sample
> > > > test on previously failing disk/system combinations.
> > >
> > > This sounds reasonable to me, but it would be nice to CC that to linux-pm
> > > and/or linux-pci too.
> > >
> > > > Signed-off-by: Mario Limonciello <mario.limonciello@dell.com>
> > > > ---
> > > > drivers/nvme/host/pci.c | 13 +++++++------
> > > > 1 file changed, 7 insertions(+), 6 deletions(-)
> > > >
> > > > diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
> > > > index 732d5b6..9b3fed4 100644
> > > > --- a/drivers/nvme/host/pci.c
> > > > +++ b/drivers/nvme/host/pci.c
> > > > @@ -2894,6 +2894,13 @@ static int nvme_suspend(struct device *dev)
> > > > if (ret < 0)
> > > > goto unfreeze;
> > > >
> > > > + /*
> > > > + * A saved state prevents pci pm from generically controlling the
> > > > + * device's power. If we're using protocol specific settings, we don't
> > > > + * want pci interfering.
> > > > + */
> > > > + pci_save_state(pdev);
> > > > +
> > > > ret = nvme_set_power_state(ctrl, ctrl->npss);
> > > > if (ret < 0)
> > > > goto unfreeze;
> > > > @@ -2908,12 +2915,6 @@ static int nvme_suspend(struct device *dev)
> > >
> > > This is the case in which the PCI layer is expected to put the device into
> > > D3, so you need
> > >
> > > pdev->state_saved = 0;
> > >
> > > at this point, because you have saved the config space already.
> > >
> > > > ret = 0;
> > > > goto unfreeze;
> > >
> > > And here you don't need to jump to "unfreeze" any more.
> > >
> > > > }
> > > > - /*
> > > > - * A saved state prevents pci pm from generically controlling the
> > > > - * device's power. If we're using protocol specific settings, we don't
> > > > - * want pci interfering.
> > > > - */
> > > > - pci_save_state(pdev);
> > > > unfreeze:
> > > > nvme_unfreeze(ctrl);
> > > > return ret;
> > > >
> > >
> > >
> > >
> >
> > Thanks, I actually followed up with something along that line in a v2 sent out
> > today. My apology you weren't in CC, but here is a weblink to it.
> > http://lists.infradead.org/pipermail/linux-nvme/2019-September/027251.html
> >
>
> I don't think that pci_load_saved_state() will work, because it sets
> state_saved at the end again (if all goes well). You simply only need to
> clear state_saved here.
Explicitly calling it with NULL as the saved state to restore seemed to have that effect
of clearing state (there is an explicit check in there if it's NULL to just return 0).
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] nvme-pci: Save PCI state before putting drive into deepest state
2019-09-18 22:00 ` Mario.Limonciello
@ 2019-09-18 22:03 ` Rafael J. Wysocki
0 siblings, 0 replies; 6+ messages in thread
From: Rafael J. Wysocki @ 2019-09-18 22:03 UTC (permalink / raw)
To: Mario Limonciello
Cc: Rafael J. Wysocki, Keith Busch, Jens Axboe, Christoph Hellwig,
Sagi Grimberg, linux-nvme, Linux Kernel Mailing List, Ryan.Hong,
Crag.Wang, sjg, Jared.Dominguez, Linux PCI, Linux PM
On Thu, Sep 19, 2019 at 12:00 AM <Mario.Limonciello@dell.com> wrote:
>
> > -----Original Message-----
> > From: Rafael J. Wysocki <rjw@rjwysocki.net>
> > Sent: Wednesday, September 18, 2019 4:57 PM
> > To: Limonciello, Mario
> > Cc: kbusch@kernel.org; axboe@fb.com; hch@lst.de; sagi@grimberg.me; linux-
> > nvme@lists.infradead.org; linux-kernel@vger.kernel.org; Hong, Ryan; Wang,
> > Crag; sjg@google.com; Dominguez, Jared; linux-pci@vger.kernel.org; linux-
> > pm@vger.kernel.org
> > Subject: Re: [PATCH] nvme-pci: Save PCI state before putting drive into deepest
> > state
> >
> >
> > [EXTERNAL EMAIL]
> >
> > On Wednesday, September 18, 2019 11:43:28 PM CEST
> > Mario.Limonciello@dell.com wrote:
> > > > -----Original Message-----
> > > > From: Rafael J. Wysocki <rjw@rjwysocki.net>
> > > > Sent: Wednesday, September 18, 2019 4:31 PM
> > > > To: Limonciello, Mario
> > > > Cc: Keith Busch; Jens Axboe; Christoph Hellwig; Sagi Grimberg; linux-
> > > > nvme@lists.infradead.org; LKML; Hong, Ryan; Wang, Crag; sjg@google.com;
> > > > Dominguez, Jared; Linux PCI; Linux PM
> > > > Subject: Re: [PATCH] nvme-pci: Save PCI state before putting drive into
> > deepest
> > > > state
> > > >
> > > >
> > > > [EXTERNAL EMAIL]
> > > >
> > > > On Thursday, September 12, 2019 1:42:33 AM CEST Mario Limonciello wrote:
> > > > > The action of saving the PCI state will cause numerous PCI configuration
> > > > > space reads which depending upon the vendor implementation may cause
> > > > > the drive to exit the deepest NVMe state.
> > > > >
> > > > > In these cases ASPM will typically resolve the PCIe link state and APST
> > > > > may resolve the NVMe power state. However it has also been observed
> > > > > that this register access after quiesced will cause PC10 failure
> > > > > on some device combinations.
> > > > >
> > > > > To resolve this, move the PCI state saving to before SetFeatures has been
> > > > > called. This has been proven to resolve the issue across a 5000 sample
> > > > > test on previously failing disk/system combinations.
> > > >
> > > > This sounds reasonable to me, but it would be nice to CC that to linux-pm
> > > > and/or linux-pci too.
> > > >
> > > > > Signed-off-by: Mario Limonciello <mario.limonciello@dell.com>
> > > > > ---
> > > > > drivers/nvme/host/pci.c | 13 +++++++------
> > > > > 1 file changed, 7 insertions(+), 6 deletions(-)
> > > > >
> > > > > diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
> > > > > index 732d5b6..9b3fed4 100644
> > > > > --- a/drivers/nvme/host/pci.c
> > > > > +++ b/drivers/nvme/host/pci.c
> > > > > @@ -2894,6 +2894,13 @@ static int nvme_suspend(struct device *dev)
> > > > > if (ret < 0)
> > > > > goto unfreeze;
> > > > >
> > > > > + /*
> > > > > + * A saved state prevents pci pm from generically controlling the
> > > > > + * device's power. If we're using protocol specific settings, we don't
> > > > > + * want pci interfering.
> > > > > + */
> > > > > + pci_save_state(pdev);
> > > > > +
> > > > > ret = nvme_set_power_state(ctrl, ctrl->npss);
> > > > > if (ret < 0)
> > > > > goto unfreeze;
> > > > > @@ -2908,12 +2915,6 @@ static int nvme_suspend(struct device *dev)
> > > >
> > > > This is the case in which the PCI layer is expected to put the device into
> > > > D3, so you need
> > > >
> > > > pdev->state_saved = 0;
> > > >
> > > > at this point, because you have saved the config space already.
> > > >
> > > > > ret = 0;
> > > > > goto unfreeze;
> > > >
> > > > And here you don't need to jump to "unfreeze" any more.
> > > >
> > > > > }
> > > > > - /*
> > > > > - * A saved state prevents pci pm from generically controlling the
> > > > > - * device's power. If we're using protocol specific settings, we don't
> > > > > - * want pci interfering.
> > > > > - */
> > > > > - pci_save_state(pdev);
> > > > > unfreeze:
> > > > > nvme_unfreeze(ctrl);
> > > > > return ret;
> > > > >
> > > >
> > > >
> > > >
> > >
> > > Thanks, I actually followed up with something along that line in a v2 sent out
> > > today. My apology you weren't in CC, but here is a weblink to it.
> > > http://lists.infradead.org/pipermail/linux-nvme/2019-September/027251.html
> > >
> >
> > I don't think that pci_load_saved_state() will work, because it sets
> > state_saved at the end again (if all goes well). You simply only need to
> > clear state_saved here.
>
> Explicitly calling it with NULL as the saved state to restore seemed to have that effect
> of clearing state (there is an explicit check in there if it's NULL to just return 0).
Ah, OK, right.
I still would rather clear the flag directly, though, as using
pci_load_saved_state() for that is just more convoluted. :-)
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2019-09-18 22:04 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <1568245353-13787-1-git-send-email-mario.limonciello@dell.com>
2019-09-18 21:31 ` [PATCH] nvme-pci: Save PCI state before putting drive into deepest state Rafael J. Wysocki
2019-09-18 21:42 ` Rafael J. Wysocki
2019-09-18 21:43 ` Mario.Limonciello
2019-09-18 21:57 ` Rafael J. Wysocki
2019-09-18 22:00 ` Mario.Limonciello
2019-09-18 22:03 ` Rafael J. Wysocki
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox