[PATCH V3 05/14] coresight: ete: Add support for ETE tracing

[Suzuki K Poulose]

On 2/2/21 6:56 PM, Mathieu Poirier wrote:
> On Wed, Jan 27, 2021 at 02:25:29PM +0530, Anshuman Khandual wrote:
>> From: Suzuki K Poulose <suzuki.poulose at arm.com>
>>
>> Add ETE as one of the supported device types we support
>> with ETM4x driver. The devices are named following the
>> existing convention as ete<N>.
>>
>> ETE mandates that the trace resource status register is programmed
>> before the tracing is turned on. For the moment simply write to
>> it indicating TraceActive.
>>
>> Cc: Mathieu Poirier <mathieu.poirier at linaro.org>
>> Cc: Mike Leach <mike.leach at linaro.org>
>> Signed-off-by: Suzuki K Poulose <suzuki.poulose at arm.com>
>> Signed-off-by: Anshuman Khandual <anshuman.khandual at arm.com>
>> ---

...

>> @@ -1834,10 +1854,6 @@ static int etm4_probe(struct device *dev, void __iomem *base, u32 etm_pid)
>>   	if (drvdata->cpu < 0)
>>   		return drvdata->cpu;
>>   
>> -	desc.name = devm_kasprintf(dev, GFP_KERNEL, "etm%d", drvdata->cpu);
>> -	if (!desc.name)
>> -		return -ENOMEM;
>> -
>>   	init_arg.drvdata = drvdata;
>>   	init_arg.csa = &desc.access;
>>   	init_arg.pid = etm_pid;
>> @@ -1853,6 +1869,20 @@ static int etm4_probe(struct device *dev, void __iomem *base, u32 etm_pid)
>>   	if (!desc.access.io_mem ||
>>   	    fwnode_property_present(dev_fwnode(dev), "qcom,skip-power-up"))
>>   		drvdata->skip_power_up = true;
> 
> Add a space here...
> 
>> +	major = ETM_ARCH_MAJOR_VERSION(drvdata->arch);
>> +	minor = ETM_ARCH_MINOR_VERSION(drvdata->arch);
> 
> And here too.  Othersiwe it makes a big blob in the middle of the function.
> 
>> +	if (etm4x_is_ete(drvdata)) {
>> +		type_name = "ete";
>> +		/* ETE v1 has major version == 5. Adjust this for logging.*/
>> +		major -= 4;
> 
> I don't have the documentation for the ETE but I would not adjust @major.  I
> would simply leave it to what the HW gives us since regardless of the name, the
> major revision of the IP block is 5.
> 

At the moment only register definitions are public and can be found here :

https://developer.arm.com/docs/ddi0601/g/aarch64-system-registers/trcdevarch

The ETE is natural extension of the ETM architecture to support future
architecture changes and is designed in a way that the same software
can driver both ETM and ETE without much changes.

>> diff --git a/drivers/hwtracing/coresight/coresight-etm4x.h b/drivers/hwtracing/coresight/coresight-etm4x.h
>> index ca24ac5..8b90de5 100644
>> --- a/drivers/hwtracing/coresight/coresight-etm4x.h
>> +++ b/drivers/hwtracing/coresight/coresight-etm4x.h
>> @@ -128,6 +128,8 @@
>>   #define TRCCIDR2			0xFF8
>>   #define TRCCIDR3			0xFFC
>>   
>> +#define TRCRSR_TA			BIT(12)
>> +
>>   /*
>>    * System instructions to access ETM registers.
>>    * See ETMv4.4 spec ARM IHI0064F section 4.3.6 System instructions
>> @@ -390,6 +392,9 @@
>>   #define ETM_COMMON_SYSREG_LIST_CASES		\
>>   	ETM_COMMON_SYSREG_LIST(NOP, __unused)
>>   
>> +#define ETM4x_ONLY_SYSREG_LIST_CASES		\
>> +	ETM4x_ONLY_SYSREG_LIST(NOP, __unused)
>> +
>>   #define ETM4x_SYSREG_LIST_CASES			\
>>   	ETM_COMMON_SYSREG_LIST_CASES		\
>>   	ETM4x_ONLY_SYSREG_LIST(NOP, __unused)
>> @@ -406,7 +411,6 @@
>>   	ETE_ONLY_SYSREG_LIST(WRITE, (val))
>>   
>>   #define ETE_ONLY_SYSREG_LIST_CASES		\
>> -	ETM_COMMON_SYSREG_LIST_CASES		\
> 
> This goes in patch 04.
> 

Sure, will move it.

> With the above:
> 
> Reviewed-by: Mathieu Poirier <mathieu.poirier at linaro.org>

Thanks for the review

Suzuki