Thread (27 messages) 27 messages, 6 authors, 2019-08-14

Re: [PATCH v4 6/6] dt-bindings: arm: coresight: Add support for coresight-needs-save-restore

From: Andrew Murray <hidden>
Date: 2019-08-14 10:02:06

On Sun, Aug 04, 2019 at 07:13:45AM -0600, Mathieu Poirier wrote:
On Fri, 2 Aug 2019 at 08:37, Andrew Murray [off-list ref] wrote:
quoted
On Fri, Aug 02, 2019 at 11:40:54AM +0100, Suzuki K Poulose wrote:
quoted
Hi Andrew,

On 30/07/2019 13:51, Andrew Murray wrote:
quoted
Some coresight components, because of choices made during hardware
integration, require their state to be saved and restored across CPU low
power states.

The software has no reliable method of detecting when save/restore is
required thus let's add a binding to inform the kernel.

Signed-off-by: Andrew Murray <redacted>
---
  Documentation/devicetree/bindings/arm/coresight.txt | 3 +++
  1 file changed, 3 insertions(+)
diff --git a/Documentation/devicetree/bindings/arm/coresight.txt b/Documentation/devicetree/bindings/arm/coresight.txt
index fcc3bacfd8bc..7cbdb7893af8 100644
--- a/Documentation/devicetree/bindings/arm/coresight.txt
+++ b/Documentation/devicetree/bindings/arm/coresight.txt
@@ -92,6 +92,9 @@ its hardware characteristcs.
    * arm,cp14: must be present if the system accesses ETM/PTM management
      registers via co-processor 14.
+   * arm,coresight-needs-save-restore: boolean. Indicates that software
+     should save/restore state across power down.
+
Do you think we could be a bit more descriptive here about when people could add
it to the DT ? Here we don't mention when someone should use this property and
it may be added to platforms where it may be absolutely unnecessary. How about :

"Indicates that the hardware implementation may not honor the Powerup request
from the software and thus might loose the register context on CPU power
down (e.g, during CPUIdle). Software must save/restore the context during a
CPU power transition cycle."
How about the following:

"Indicates that the hardware will loose register context on CPU power down (e.g.
CPUIdle), despite the TRCPDCR.PU bit being set."

I'm keen to avoid making suggestions about what the kernel will do when it sees
this flag and thus prefer to focus on describing what the hardware does. So I
dropped your last sentence. However the name of the flag still implies policy
which I don't like.

I also changed the 'may not honor' wording, I'm not sure if this is really the
case or if the spec is open to interpretation.

It would great for this wording to also apply to other CS components though I
haven't investigated if these have a PU bit or something different.
Exactly - the definition needs to be broad enough to apply to other CS
components.  Mike what do you think would be appropriate for CTIs?
How about we keep this short and simple:

* arm,coresight-loses-context-with-cpu : boolean. Indicates that the hardware
  will lose register context on CPU power down (e.g. CPUIdle).

I could have added something like "... despite TRCPDCR.PU being set", or to
apply more generically: "... despite available register controls being set to
prevent such context loss". However whilst these are more informative - they
elude to some of reasons as to why context is lost and as we cannot be
exhaustive I'd rather not give a limited example.

However if a longer explaination is required:

* arm,coresight-loses-context-with-cpu : boolean. Indicates that the hardware
  will lose register context on CPU power down (e.g. CPUIdle). An example of
  where this may be needed are systems which contain a coresight component and
  CPU in the same power domain. When the CPU powers down the coresight
  component also powers down and loses its context.

Any objections/preference? :)

Thanks,

Andrew Murray
quoted
Thanks,

Andrew Murray
quoted
Cheers
Suzuki
_______________________________________________
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