Re: [PATCH] coresight: etm4x: Add support to disable trace unit power up

[Sai Prakash Ranjan <saiprakash.ranjan@codeaurora.org>]

Hi Mathieu,

On 2020-05-14 23:30, Mathieu Poirier wrote:
> Good morning Sai,
> 
> On Thu, May 14, 2020 at 04:29:15PM +0530, Sai Prakash Ranjan wrote:
>> From: Tingwei Zhang <tingwei@codeaurora.org>
>> 
>> On some Qualcomm Technologies Inc. SoCs like SC7180, there
>> exists a hardware errata where the APSS (Application Processor
>> SubSystem)/CPU watchdog counter is stopped when ETM register
>> TRCPDCR.PU=1.
> 
> Fun stuff...
> 

Yes :)

>> Since the ETMs share the same power domain as
>> that of respective CPU cores, they are powered on when the
>> CPU core is powered on. So we can disable powering up of the
>> trace unit after checking for this errata via new property
>> called "qcom,tupwr-disable".
>> 
>> Signed-off-by: Tingwei Zhang <tingwei@codeaurora.org>
>> Co-developed-by: Sai Prakash Ranjan <saiprakash.ranjan@codeaurora.org>
>> Signed-off-by: Sai Prakash Ranjan <saiprakash.ranjan@codeaurora.org>
> 
> Co-developed-by: Sai Prakash Ranjan <saiprakash.ranjan@codeaurora.org>
> Signed-off-by: Tingwei Zhang <tingwei@codeaurora.org>
> 

Tingwei is the author, so if I understand correctly, his signed-off-by
should appear first, am I wrong?

>> ---
>>  .../devicetree/bindings/arm/coresight.txt     |  6 ++++
>>  drivers/hwtracing/coresight/coresight-etm4x.c | 29 
>> ++++++++++++-------
> 
> Please split in two patches.
> 

Sure, I will split the dt-binding into separate patch, checkpatch did 
warn.

>>  2 files changed, 25 insertions(+), 10 deletions(-)
>> 
>> diff --git a/Documentation/devicetree/bindings/arm/coresight.txt 
>> b/Documentation/devicetree/bindings/arm/coresight.txt
>> index 846f6daae71b..d2030128fe46 100644
>> --- a/Documentation/devicetree/bindings/arm/coresight.txt
>> +++ b/Documentation/devicetree/bindings/arm/coresight.txt
>> @@ -108,6 +108,12 @@ its hardware characteristcs.
>>  	* arm,cp14: must be present if the system accesses ETM/PTM 
>> management
>>  	  registers via co-processor 14.
>> 
>> +	* qcom,tupwr-disable: boolean. Indicates that trace unit power up 
>> can
>> +	  be disabled on Qualcomm Technologies Inc. systems where ETMs are 
>> in
>> +	  the same power domain as their CPU cores. This property is 
>> required
>> +	  to identify such systems with hardware errata where the CPU 
>> watchdog
>> +	  counter is stopped when TRCPDCR.PU=1.
>> +
> 
> I think something like "qcom,skip-power-up" would be clearer.
> 
> Also, a better choice of words is that TRCPDCR.PU does not have to be 
> set on
> Qualcomm...
> 

Yes "qcom,skip-power-up" is a lot better, thanks. Also will use 
something as
you suggested for description.

>>  * Optional property for TMC:
>> 
>>  	* arm,buffer-size: size of contiguous buffer space for TMC ETR
>> diff --git a/drivers/hwtracing/coresight/coresight-etm4x.c 
>> b/drivers/hwtracing/coresight/coresight-etm4x.c
>> index fb0f5f4f3a91..6886b44f6947 100644
>> --- a/drivers/hwtracing/coresight/coresight-etm4x.c
>> +++ b/drivers/hwtracing/coresight/coresight-etm4x.c
>> @@ -104,6 +104,11 @@ struct etm4_enable_arg {
>>  	int rc;
>>  };
>> 
>> +static inline bool etm4_can_disable_tupwr(struct device *dev)
>> +{
>> +	return fwnode_property_present(dev_fwnode(dev), 
>> "qcom,tupwr-disable");
>> +}
>> +
> 
> Please call fwnode_property_present() at initialisation time to set a 
> new
> drvdata::skip_power_up variable.  From there just switch on that in
> etm4_enable/disable_hw().
> 

Will do, thanks.

Thanks,
Sai

-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a 
member
of Code Aurora Forum, hosted by The Linux Foundation

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