From: "Rafael J. Wysocki" <rjw@rjwysocki.net> To: Miquel Raynal <miquel.raynal@bootlin.com> Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>, Stephen Boyd <sboyd@kernel.org>, sudeep.holla@arm.com, Gregory Clement <gregory.clement@bootlin.com>, Jason Cooper <jason@lakedaemon.net>, Andrew Lunn <andrew@lunn.ch>, Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>, Thomas Petazzoni <thomas.petazzoni@bootlin.com>, Bjorn Helgaas <bhelgaas@google.com>, devicetree@vger.kernel.org, Rob Herring <robh+dt@kernel.org>, Mark Rutland <mark.rutland@arm.com>, linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, Antoine Tenart <antoine.tenart@bootlin.com>, Maxime Chevallier <maxime.chevallier@bootlin.com>, Nadav Haklai <nadavh@marvell.com> Subject: Re: [PATCH 05/12] PCI: aardvark: add suspend to RAM support Date: Tue, 18 Dec 2018 11:54:43 +0100 [thread overview] Message-ID: <28619896.152T6JUIHS@aspire.rjw.lan> (raw) In-Reply-To: <20181217155426.71058a03@xps13> On Monday, December 17, 2018 3:54:26 PM CET Miquel Raynal wrote: > Hi Rafael, > > "Rafael J. Wysocki" <rjw@rjwysocki.net> wrote on Thu, 13 Dec 2018 > 22:50:51 +0100: > > > On Thursday, December 13, 2018 3:30:00 PM CET Miquel Raynal wrote: > > > Hi Lorenzo, > > > > > > > > If that's really the case, then I can see how one device and it's > > > > > children are suspended and the irq for it is disabled but the providing > > > > > devices (clk, regulator, bus controller, etc.) are still fully active > > > > > and not suspended but in fact completely usable and able to service > > > > > interrupts. If that all makes sense, then I would answer the question > > > > > with a definitive "yes it's all fine" because the clk consumer could be > > > > > in the NOIRQ phase of its suspend but the clk provider wouldn't have > > > > > even started suspending yet when clk_disable_unprepare() is called. > > > > > > > > That's a very good summary and address my concern, I still question this > > > > patch correctness (and many others that carry out clk operations in S2R > > > > NOIRQ phase), they may work but do not tell me they are rock solid given > > > > your accurate summary above. > > > > > > I understand your concern but I don't see any alternative right now > > > and a deep rework of the PM core to respect such dependency is not > > > something that can be done in a reasonable amount of time. > > > > Maybe you don't need to rework anything. :-) > > > > Have you considered using device links? > > Absolutely, yes :) I am actively working on it in parallel, you can > check the third version there [1]. Stephen Boyd has a slightly > different idea of how it should be done, I will propose a v4 this week, > I can add you in copy if you are interested! > > Anyway, there is one thing that is still missing: > * Let's have device A that requests clock B > * With the device link series, A is linked (as a child) to B. > * A suspend/resume hooks handle things in the NOIRQ phase. Why do you need them to run in the "noirq" phase in the first place? > * B suspend/resume hooks handle things in the default phase. > > What I expected during a suspend: > 1/ ->suspend_noirq(device A) > 2/ ->suspend(clock B) This expectation is not in agreement with the documented suspend code flow, however. Each phase of it is carried out for *all* devices completely before getting to the next phase, "prepare" first, then "suspend", "suspend_late" and "suspend_noirq", in this order. > Unfortunately, device links do not seem to enforce any priority between > phases (default/late/noirq) and what happens is: > 1/ ->suspend(B) > 2/ ->suspend_noirq(A) > Which has no sense in my case. Hence, I had to request the clock > suspend/resume callbacks to be upgraded to the NOIRQ phase as well (I > don't have a better solution for now). This is still under discussion > in a thread you have been recently added to by Bjorn, see [2]. > > So when I told you I was not confident in "reworking the PM core to > respect such dependency", this is what I was referring to. I am > definitely ready to help, but I don't feel I can do it alone. > > [1] https://www.spinics.net/lists/linux-clk/msg32824.html > [2] https://marc.info/?l=linux-pm&m=154465198510735&w=2 The rework you seem to be talking about is not possible, I'm afraid.
WARNING: multiple messages have this Message-ID (diff)
From: "Rafael J. Wysocki" <rjw@rjwysocki.net> To: Miquel Raynal <miquel.raynal@bootlin.com> Cc: Mark Rutland <mark.rutland@arm.com>, Andrew Lunn <andrew@lunn.ch>, Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>, Jason Cooper <jason@lakedaemon.net>, devicetree@vger.kernel.org, Stephen Boyd <sboyd@kernel.org>, linux-pci@vger.kernel.org, Gregory Clement <gregory.clement@bootlin.com>, linux-kernel@vger.kernel.org, Maxime Chevallier <maxime.chevallier@bootlin.com>, Nadav Haklai <nadavh@marvell.com>, Antoine Tenart <antoine.tenart@bootlin.com>, Rob Herring <robh+dt@kernel.org>, Thomas Petazzoni <thomas.petazzoni@bootlin.com>, sudeep.holla@arm.com, Bjorn Helgaas <bhelgaas@google.com>, linux-arm-kernel@lists.infradead.org, Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com> Subject: Re: [PATCH 05/12] PCI: aardvark: add suspend to RAM support Date: Tue, 18 Dec 2018 11:54:43 +0100 [thread overview] Message-ID: <28619896.152T6JUIHS@aspire.rjw.lan> (raw) In-Reply-To: <20181217155426.71058a03@xps13> On Monday, December 17, 2018 3:54:26 PM CET Miquel Raynal wrote: > Hi Rafael, > > "Rafael J. Wysocki" <rjw@rjwysocki.net> wrote on Thu, 13 Dec 2018 > 22:50:51 +0100: > > > On Thursday, December 13, 2018 3:30:00 PM CET Miquel Raynal wrote: > > > Hi Lorenzo, > > > > > > > > If that's really the case, then I can see how one device and it's > > > > > children are suspended and the irq for it is disabled but the providing > > > > > devices (clk, regulator, bus controller, etc.) are still fully active > > > > > and not suspended but in fact completely usable and able to service > > > > > interrupts. If that all makes sense, then I would answer the question > > > > > with a definitive "yes it's all fine" because the clk consumer could be > > > > > in the NOIRQ phase of its suspend but the clk provider wouldn't have > > > > > even started suspending yet when clk_disable_unprepare() is called. > > > > > > > > That's a very good summary and address my concern, I still question this > > > > patch correctness (and many others that carry out clk operations in S2R > > > > NOIRQ phase), they may work but do not tell me they are rock solid given > > > > your accurate summary above. > > > > > > I understand your concern but I don't see any alternative right now > > > and a deep rework of the PM core to respect such dependency is not > > > something that can be done in a reasonable amount of time. > > > > Maybe you don't need to rework anything. :-) > > > > Have you considered using device links? > > Absolutely, yes :) I am actively working on it in parallel, you can > check the third version there [1]. Stephen Boyd has a slightly > different idea of how it should be done, I will propose a v4 this week, > I can add you in copy if you are interested! > > Anyway, there is one thing that is still missing: > * Let's have device A that requests clock B > * With the device link series, A is linked (as a child) to B. > * A suspend/resume hooks handle things in the NOIRQ phase. Why do you need them to run in the "noirq" phase in the first place? > * B suspend/resume hooks handle things in the default phase. > > What I expected during a suspend: > 1/ ->suspend_noirq(device A) > 2/ ->suspend(clock B) This expectation is not in agreement with the documented suspend code flow, however. Each phase of it is carried out for *all* devices completely before getting to the next phase, "prepare" first, then "suspend", "suspend_late" and "suspend_noirq", in this order. > Unfortunately, device links do not seem to enforce any priority between > phases (default/late/noirq) and what happens is: > 1/ ->suspend(B) > 2/ ->suspend_noirq(A) > Which has no sense in my case. Hence, I had to request the clock > suspend/resume callbacks to be upgraded to the NOIRQ phase as well (I > don't have a better solution for now). This is still under discussion > in a thread you have been recently added to by Bjorn, see [2]. > > So when I told you I was not confident in "reworking the PM core to > respect such dependency", this is what I was referring to. I am > definitely ready to help, but I don't feel I can do it alone. > > [1] https://www.spinics.net/lists/linux-clk/msg32824.html > [2] https://marc.info/?l=linux-pm&m=154465198510735&w=2 The rework you seem to be talking about is not possible, I'm afraid. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2018-12-18 10:55 UTC|newest] Thread overview: 88+ messages / expand[flat|nested] mbox.gz Atom feed top 2018-11-23 14:18 [PATCH 00/12] Bring suspend to RAM support to PCIe Aardvark driver Miquel Raynal 2018-11-23 14:18 ` Miquel Raynal 2018-11-23 14:18 ` Miquel Raynal 2018-11-23 14:18 ` [PATCH 01/12] PCI: aardvark: configure more registers in the configuration helper Miquel Raynal 2018-11-23 14:18 ` Miquel Raynal 2018-11-23 14:18 ` Miquel Raynal 2018-11-23 14:18 ` [PATCH 02/12] PCI: aardvark: add reset GPIO support Miquel Raynal 2018-11-23 14:18 ` Miquel Raynal 2018-11-23 14:18 ` Miquel Raynal 2018-11-23 14:18 ` [PATCH 03/12] PCI: aardvark: add PHY support Miquel Raynal 2018-11-23 14:18 ` Miquel Raynal 2018-11-23 14:18 ` Miquel Raynal 2018-11-23 14:18 ` [PATCH 04/12] PCI: aardvark: add clock support Miquel Raynal 2018-11-23 14:18 ` Miquel Raynal 2018-11-23 14:18 ` Miquel Raynal 2018-11-23 14:18 ` [PATCH 05/12] PCI: aardvark: add suspend to RAM support Miquel Raynal 2018-11-23 14:18 ` Miquel Raynal 2018-11-23 14:18 ` Miquel Raynal 2018-12-03 10:27 ` Lorenzo Pieralisi 2018-12-03 10:27 ` Lorenzo Pieralisi 2018-12-03 15:38 ` Miquel Raynal 2018-12-03 15:38 ` Miquel Raynal 2018-12-03 17:18 ` Lorenzo Pieralisi 2018-12-03 17:18 ` Lorenzo Pieralisi 2018-12-03 19:19 ` Stephen Boyd 2018-12-03 19:19 ` Stephen Boyd 2018-12-03 22:00 ` Rafael J. Wysocki 2018-12-03 22:00 ` Rafael J. Wysocki 2018-12-03 22:18 ` Miquel Raynal 2018-12-03 22:18 ` Miquel Raynal 2018-12-04 9:45 ` Lorenzo Pieralisi 2018-12-04 9:45 ` Lorenzo Pieralisi 2018-12-04 21:42 ` Rafael J. Wysocki 2018-12-04 21:42 ` Rafael J. Wysocki 2018-12-05 11:00 ` Lorenzo Pieralisi 2018-12-05 11:00 ` Lorenzo Pieralisi 2018-12-11 14:16 ` Lorenzo Pieralisi 2018-12-11 14:16 ` Lorenzo Pieralisi 2018-12-13 9:00 ` Stephen Boyd 2018-12-13 9:00 ` Stephen Boyd 2018-12-13 10:53 ` Lorenzo Pieralisi 2018-12-13 10:53 ` Lorenzo Pieralisi 2018-12-13 14:30 ` Miquel Raynal 2018-12-13 14:30 ` Miquel Raynal 2018-12-13 14:52 ` Lorenzo Pieralisi 2018-12-13 14:52 ` Lorenzo Pieralisi 2018-12-13 21:50 ` Rafael J. Wysocki 2018-12-13 21:50 ` Rafael J. Wysocki 2018-12-17 14:54 ` Miquel Raynal 2018-12-17 14:54 ` Miquel Raynal 2018-12-18 10:54 ` Rafael J. Wysocki [this message] 2018-12-18 10:54 ` Rafael J. Wysocki 2018-12-18 14:14 ` Miquel Raynal 2018-12-18 14:14 ` Miquel Raynal 2018-11-23 14:18 ` [PATCH 06/12] dt-bindings: PCI: aardvark: describe the reset-gpios property Miquel Raynal 2018-11-23 14:18 ` Miquel Raynal 2018-11-23 14:18 ` Miquel Raynal 2018-12-11 21:44 ` Rob Herring 2018-12-11 21:44 ` Rob Herring 2018-12-11 21:44 ` Rob Herring 2018-11-23 14:18 ` [PATCH 07/12] dt-bindings: PCI: aardvark: describe the clocks property Miquel Raynal 2018-11-23 14:18 ` Miquel Raynal 2018-11-23 14:18 ` Miquel Raynal 2018-12-11 21:44 ` Rob Herring 2018-12-11 21:44 ` Rob Herring 2018-12-11 21:44 ` Rob Herring 2018-11-23 14:18 ` [PATCH 08/12] dt-bindings: PCI: aardvark: describe the PHY property Miquel Raynal 2018-11-23 14:18 ` Miquel Raynal 2018-11-23 14:18 ` Miquel Raynal 2018-12-11 21:44 ` Rob Herring 2018-12-11 21:44 ` Rob Herring 2018-12-11 21:44 ` Rob Herring 2018-11-23 14:18 ` [PATCH 09/12] ARM64: dts: marvell: armada-37xx: declare PCIe reset pin Miquel Raynal 2018-11-23 14:18 ` Miquel Raynal 2018-11-23 14:18 ` Miquel Raynal 2018-11-23 14:18 ` [PATCH 10/12] ARM64: dts: marvell: armada-3720-espressobin: declare PCIe reset GPIO Miquel Raynal 2018-11-23 14:18 ` Miquel Raynal 2018-11-23 14:18 ` Miquel Raynal 2018-11-23 14:18 ` [PATCH 11/12] ARM64: dts: marvell: armada-37xx: declare PCIe clock Miquel Raynal 2018-11-23 14:18 ` Miquel Raynal 2018-11-23 14:18 ` Miquel Raynal 2018-11-23 14:18 ` [PATCH 12/12] ARM64: dts: marvell: armada-3720-espressobin: declare PCIe PHY Miquel Raynal 2018-11-23 14:18 ` Miquel Raynal 2018-11-23 14:18 ` Miquel Raynal 2018-11-26 14:50 ` [PATCH 00/12] Bring suspend to RAM support to PCIe Aardvark driver Bjorn Helgaas 2018-11-26 14:50 ` Bjorn Helgaas 2018-11-30 13:12 ` Miquel Raynal 2018-11-30 13:12 ` 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=28619896.152T6JUIHS@aspire.rjw.lan \ --to=rjw@rjwysocki.net \ --cc=andrew@lunn.ch \ --cc=antoine.tenart@bootlin.com \ --cc=bhelgaas@google.com \ --cc=devicetree@vger.kernel.org \ --cc=gregory.clement@bootlin.com \ --cc=jason@lakedaemon.net \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-pci@vger.kernel.org \ --cc=lorenzo.pieralisi@arm.com \ --cc=mark.rutland@arm.com \ --cc=maxime.chevallier@bootlin.com \ --cc=miquel.raynal@bootlin.com \ --cc=nadavh@marvell.com \ --cc=robh+dt@kernel.org \ --cc=sboyd@kernel.org \ --cc=sebastian.hesselbarth@gmail.com \ --cc=sudeep.holla@arm.com \ --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: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.