From: Miquel Raynal <miquel.raynal@bootlin.com>
To: Bjorn Helgaas <bhelgaas@google.com>
Cc: Michael Turquette <mturquette@baylibre.com>,
Stephen Boyd <sboyd@kernel.org>, Rob Herring <robh+dt@kernel.org>,
Mark Rutland <mark.rutland@arm.com>,
linux-clk@vger.kernel.org, devicetree@vger.kernel.org,
Thomas Petazzoni <thomas.petazzoni@bootlin.com>,
Antoine Tenart <antoine.tenart@bootlin.com>,
Gregory Clement <gregory.clement@bootlin.com>,
Maxime Chevallier <maxime.chevallier@bootlin.com>,
Nadav Haklai <nadavh@marvell.com>,
"Rafael J . Wysocki" <rjw@rjwysocki.net>,
Linux PM list <linux-pm@vger.kernel.org>
Subject: Re: [PATCH v2 2/4] clk: mvebu: armada-37xx-periph: change suspend/resume time
Date: Mon, 17 Jun 2019 14:50:04 +0200 [thread overview]
Message-ID: <20190617145004.7b11988f@xps13> (raw)
In-Reply-To: <CAErSpo7fimH5QhHTLsF2ASyPqstkw7Zibe3CYB=KXTYBOh-4GQ@mail.gmail.com>
Hi Bjorn,
Bjorn Helgaas <bhelgaas@google.com> wrote on Tue, 4 Jun 2019 15:52:31
-0500:
> On Mon, May 27, 2019 at 8:46 AM Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> >
> > Hi Bjorn,
> >
> > Thanks for the feedback.
> >
> > Bjorn Helgaas <bhelgaas@google.com> wrote on Tue, 21 May 2019 17:43:05
> > -0500:
> >
> > > From: Miquel Raynal <miquel.raynal@bootlin.com>
> > > Date: Tue, May 21, 2019 at 8:04 AM
> > > To: Michael Turquette, Stephen Boyd, Rob Herring, Mark Rutland
> > > Cc: <linux-clk@vger.kernel.org>, <devicetree@vger.kernel.org>, Thomas
> > > Petazzoni, Antoine Tenart, Gregory Clement, Maxime Chevallier, Nadav
> > > Haklai, Bjorn Helgaas, Rafael J . Wysocki, <linux-pm@vger.kernel.org>,
> > > Miquel Raynal
> > >
> > > > Armada 3700 PCIe IP relies on the PCIe clock managed by this
> > > > driver. For reasons related to the PCI core's organization when
> > > > suspending/resuming, PCI host controller drivers must reconfigure
> > > > their register at suspend_noirq()/resume_noirq() which happens after
> > > > suspend()/suspend_late() and before resume_early()/resume().
> > >
> > > "For reasons related to the PCI core's organization" manages to
> > > suggest that this change wouldn't be needed if only the PCI core did
> > > something differently, without actually being specific about what it
> > > would need to do differently.
> > >
> > > Is there something the PCI core could do better to make this easier?
> > > Or is it just something like "the PCI core needs to access registers
> > > after suspend_late()"? You mention the host controller, but of course
> > > that's not itself a PCI device, so the PCI core doesn't have much to
> > > do with it directly.
> >
> > Actually, if I understand correctly the below commit [1] and the core
> > [2] & [3], PCI device fixups can happen at any time, including at the
> > _noirq phase where, obviously, the PCI controller must be already
> > setup.
> >
> > I don't think changing this behavior is a viable solution and I would
> > not see it as a "PCI core could do better" alternative.
> >
> > ---8<---
> >
> > [1]
> > commit ab14d45ea58eae67c739e4ba01871cae7b6c4586
> > Author: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> > Date: Tue Mar 17 15:55:45 2015 +0100
> >
> > PCI: mvebu: Add suspend/resume support
> >
> > Add suspend/resume support for the mvebu PCIe host driver. Without
> > this commit, the system will panic at resume time when PCIe devices
> > are connected.
> >
> > Note that we have to use the ->suspend_noirq() and ->resume_noirq()
> > hooks, because at resume time, the PCI fixups are done at
> > ->resume_noirq() time, so the PCIe controller has to be ready at
> > this point.
> >
> > Signed-off-by: Thomas Petazzoni
> > <thomas.petazzoni@free-electrons.com> Signed-off-by: Bjorn Helgaas
> > <bhelgaas@google.com> Acked-by: Jason Cooper <jason@lakedaemon.net>
> >
> > [2] https://elixir.bootlin.com/linux/v5.2-rc1/source/drivers/pci/pci-driver.c#L1181
> > [3] https://elixir.bootlin.com/linux/v5.2-rc1/source/drivers/pci/pci-driver.c#L522
> >
> > --->8---
> >
> > >
> > > s/register/registers/ ?
> >
> > Indeed. I would like to sort out the above technical point before
> > sending a v3 with this typo corrected.
>
> I don't have anything more to contribute here; just wanted to make
> sure this wasn't working around a fixable problem in PCI.
Great! Would you mind adding a A-b/R-b tag then?
Thanks,
Miquèl
next prev parent reply other threads:[~2019-06-17 12:50 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-05-21 13:03 [PATCH v2 0/4] Prepare Armada 3700 PCIe suspend to RAM support Miquel Raynal
2019-05-21 13:03 ` [PATCH v2 1/4] clk: mvebu: armada-37xx-periph: add PCIe gated clock Miquel Raynal
2019-05-21 13:03 ` [PATCH v2 2/4] clk: mvebu: armada-37xx-periph: change suspend/resume time Miquel Raynal
2019-05-21 22:43 ` Bjorn Helgaas
2019-05-27 13:46 ` Miquel Raynal
2019-06-04 20:52 ` Bjorn Helgaas
2019-06-17 12:50 ` Miquel Raynal [this message]
2019-06-17 20:07 ` Bjorn Helgaas
2019-05-21 13:03 ` [PATCH v2 3/4] dt-bindings: clk: armada3700: fix typo in SoC name Miquel Raynal
2019-05-21 13:03 ` [PATCH v2 4/4] dt-bindings: clk: armada3700: document the PCIe clock Miquel Raynal
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20190617145004.7b11988f@xps13 \
--to=miquel.raynal@bootlin.com \
--cc=antoine.tenart@bootlin.com \
--cc=bhelgaas@google.com \
--cc=devicetree@vger.kernel.org \
--cc=gregory.clement@bootlin.com \
--cc=linux-clk@vger.kernel.org \
--cc=linux-pm@vger.kernel.org \
--cc=mark.rutland@arm.com \
--cc=maxime.chevallier@bootlin.com \
--cc=mturquette@baylibre.com \
--cc=nadavh@marvell.com \
--cc=rjw@rjwysocki.net \
--cc=robh+dt@kernel.org \
--cc=sboyd@kernel.org \
--cc=thomas.petazzoni@bootlin.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).