All of lore.kernel.org
 help / color / mirror / Atom feed
From: Suzuki K Poulose <suzuki.poulose@arm.com>
To: mike.leach@linaro.org, andrew.murray@arm.com
Cc: Al.Grant@arm.com, mathieu.poirier@linaro.org,
	alexander.shishkin@linux.intel.com, coresight@lists.linaro.org,
	Sudeep.Holla@arm.com, leo.yan@linaro.org,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH v4 6/6] dt-bindings: arm: coresight: Add support for coresight-needs-save-restore
Date: Wed, 14 Aug 2019 13:35:27 +0100	[thread overview]
Message-ID: <718b5bdf-9828-f215-9a81-637308be3b49@arm.com> (raw)
In-Reply-To: <CAJ9a7Vj+bo2PMnh2fbMJnaHRwJm9jU689P+iZ4q8_Vg7-3SnDg@mail.gmail.com>

Hi Mike,

On 14/08/2019 12:06, Mike Leach wrote:
> Hi,
> 
> On Wed, 14 Aug 2019 at 11:01, Andrew Murray <andrew.murray@arm.com> wrote:
>>
>> On Sun, Aug 04, 2019 at 07:13:45AM -0600, Mathieu Poirier wrote:
>>> On Fri, 2 Aug 2019 at 08:37, Andrew Murray <andrew.murray@arm.com> wrote:
>>>>
>>>> On Fri, Aug 02, 2019 at 11:40:54AM +0100, Suzuki K Poulose wrote:
>>>>> Hi Andrew,
>>>>>
>>>>> On 30/07/2019 13:51, Andrew Murray wrote:
>>>>>> Some coresight components, because of choices made during hardware
>>>>>> integration, require their state to be saved and restored across CPU low
>>>>>> power states.

...

>>>>>> --- 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?
>>
> CTIs have no power control at all - i.e. no PU bit to request we stay
> up - and reside in the debug power domain. So they are coupled to the
> CS/CPU/ETM/ power domains and reliant on outside forces to request
> power.
> The expectation is that for a PE bound CTI, if debug is powered then
> it will be fully powered - so an ETM with PU respected, or the
> external debug logic with DBGNOPWRDWN respected should be sufficient
> for CTI to stay alive.

I am trying to understand why we need this property for CTI.
Don't we always need to save-restore the CTI controls on a CPU_DOWN for the
associated CTI ? Since it may not be really tied to an ETM (e.g, if the CTI is
purely used to handle CPU triggers, PMU etc,). If that is the case, do we need
this property for CTI at all ?

> 
>> How about we keep this short and simple:
>>
>> * arm,coresight-loses-context-with-cpu : boolean. Indicates that the hardware

nit: s/loses/looses ?

>>    will lose register context on CPU power down (e.g. CPUIdle).
>>
> 
> So the above name is generic enough to encompass the CTI as well.
> 
>> 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.

This looks fine for me. But I am trying to understand the rationale behind
using this for CTI

Suzuki

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

  reply	other threads:[~2019-08-14 12:35 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-07-30 12:51 [PATCH v4 0/6] coresight: etm4x: save/restore ETMv4 context across CPU low power states Andrew Murray
2019-07-30 12:51 ` [PATCH v4 1/6] coresight: etm4x: remove superfluous setting of os_unlock Andrew Murray
2019-07-30 12:51 ` [PATCH v4 2/6] coresight: etm4x: use explicit barriers on enable/disable Andrew Murray
2019-07-30 12:51   ` Andrew Murray
2019-08-01 13:31   ` Sasha Levin
2019-08-01 14:48     ` Mathieu Poirier
2019-08-01 14:48       ` Mathieu Poirier
2019-08-02 18:08   ` Sasha Levin
2019-07-30 12:51 ` [PATCH v4 3/6] coresight: etm4x: use module_param instead of module_param_named Andrew Murray
2019-08-02 10:23   ` Suzuki K Poulose
2019-07-30 12:51 ` [PATCH v4 4/6] coresight: etm4x: improve clarity of etm4_os_unlock comment Andrew Murray
2019-07-30 12:51 ` [PATCH v4 5/6] coresight: etm4x: save/restore state across CPU low power states Andrew Murray
2019-07-30 21:16   ` Mathieu Poirier
2019-07-30 21:45     ` Andrew Murray
2019-07-31  8:16       ` Mike Leach
2019-07-31  9:45         ` Andrew Murray
2019-08-02 10:54   ` Suzuki K Poulose
2019-08-14  9:12     ` Andrew Murray
2019-07-30 12:51 ` [PATCH v4 6/6] dt-bindings: arm: coresight: Add support for coresight-needs-save-restore Andrew Murray
2019-08-02 10:40   ` Suzuki K Poulose
2019-08-02 14:37     ` Andrew Murray
2019-08-04 13:13       ` Mathieu Poirier
2019-08-14 10:01         ` Andrew Murray
2019-08-14 11:06           ` Mike Leach
2019-08-14 12:35             ` Suzuki K Poulose [this message]
2019-08-14 12:49               ` Andrew Murray
2019-08-14 14:20               ` Mike Leach
2019-07-30 20:12 ` [PATCH v4 0/6] coresight: etm4x: save/restore ETMv4 context across CPU low power states Mathieu Poirier
2019-07-30 21:46   ` Andrew Murray

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=718b5bdf-9828-f215-9a81-637308be3b49@arm.com \
    --to=suzuki.poulose@arm.com \
    --cc=Al.Grant@arm.com \
    --cc=Sudeep.Holla@arm.com \
    --cc=alexander.shishkin@linux.intel.com \
    --cc=andrew.murray@arm.com \
    --cc=coresight@lists.linaro.org \
    --cc=leo.yan@linaro.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=mathieu.poirier@linaro.org \
    --cc=mike.leach@linaro.org \
    /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 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.