On Mon, Jun 17, 2019 at 7:50 AM Miquel Raynal <miquel.raynal@xxxxxxxxxxx> wrote: > > Hi Bjorn, > > Bjorn Helgaas <bhelgaas@xxxxxxxxxx> wrote on Tue, 4 Jun 2019 15:52:31 > -0500: > > > On Mon, May 27, 2019 at 8:46 AM Miquel Raynal <miquel.raynal@xxxxxxxxxxx> wrote: > > > > > > Hi Bjorn, > > > > > > Thanks for the feedback. > > > > > > Bjorn Helgaas <bhelgaas@xxxxxxxxxx> wrote on Tue, 21 May 2019 17:43:05 > > > -0500: > > > > > > > From: Miquel Raynal <miquel.raynal@xxxxxxxxxxx> > > > > Date: Tue, May 21, 2019 at 8:04 AM > > > > To: Michael Turquette, Stephen Boyd, Rob Herring, Mark Rutland > > > > Cc: <linux-clk@xxxxxxxxxxxxxxx>, <devicetree@xxxxxxxxxxxxxxx>, Thomas > > > > Petazzoni, Antoine Tenart, Gregory Clement, Maxime Chevallier, Nadav > > > > Haklai, Bjorn Helgaas, Rafael J . Wysocki, <linux-pm@xxxxxxxxxxxxxxx>, > > > > 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@xxxxxxxxxxxxxxxxxx> > > > 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@xxxxxxxxxxxxxxxxxx> Signed-off-by: Bjorn Helgaas > > > <bhelgaas@xxxxxxxxxx> Acked-by: Jason Cooper <jason@xxxxxxxxxxxxxx> > > > > > > [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? You're only touching drivers/clk, so you don't need my ack, and I don't really feel qualified to add a reviewed-by. Power management and suspend/resume is still a mystery to me :) Bjorn
