Thread (36 messages) 36 messages, 6 authors, 2018-12-18

Re: [PATCH 05/12] PCI: aardvark: add suspend to RAM support

From: Miquel Raynal <miquel.raynal@bootlin.com>
Date: 2018-12-17 14:55:03
Also in: linux-devicetree, linux-pci, lkml

Hi Rafael,

"Rafael J. Wysocki" [off-list ref] wrote on Thu, 13 Dec 2018
22:50:51 +0100:
On Thursday, December 13, 2018 3:30:00 PM CET Miquel Raynal wrote:
quoted
Hi Lorenzo,
  
quoted
quoted
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.
* 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)

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


Thanks,
Miquèl

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help