Re: [RFC PATCH 08/14] coresight: etm4x: Convert all register accesses

[Suzuki K Poulose <suzuki.poulose@arm.com>]

On 07/30/2020 09:20 PM, Mathieu Poirier wrote:
> On Wed, Jul 22, 2020 at 06:20:34PM +0100, Suzuki K Poulose wrote:
>> Convert all register accesses from etm4x driver to use a wrapper
>> to allow switching the access at runtime with little overhead.
>>
>> co-developed by sed tool ;-), mostly equivalent to :
>>
>> s/readl\(_relaxed\)\?(drvdata->base + \(.*\))/etm4x_\1_read32(csdev, \2)
>> s/writel\(_relaxed\)\?(\(.*\), drvdata->base + \(.*\))/etm4x_\1_write32(csdev, \2, \3)
>>
>> We don't want to replace them with the csdev_access_* to
>> avoid a function call for every register access for system
>> register access.
>>
>> Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
>> Cc: Mike Leach <mike.leach@linaro.org>
>> Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
>> ---
>>   .../coresight/coresight-etm4x-sysfs.c         |   9 +-
>>   drivers/hwtracing/coresight/coresight-etm4x.c | 334 +++++++++---------
>>   drivers/hwtracing/coresight/coresight-etm4x.h |  24 ++
>>   3 files changed, 189 insertions(+), 178 deletions(-)
>>


Agreed to all the comments.

>> diff --git a/drivers/hwtracing/coresight/coresight-etm4x.h b/drivers/hwtracing/coresight/coresight-etm4x.h
>> index b8283e1d6d88..2b51d03ab6d7 100644
>> --- a/drivers/hwtracing/coresight/coresight-etm4x.h
>> +++ b/drivers/hwtracing/coresight/coresight-etm4x.h
>> @@ -120,6 +120,30 @@
>>   #define TRCCIDR2			0xFF8
>>   #define TRCCIDR3			0xFFC
>>   
>> +#define etm4x_relaxed_read32(csa, offset)		\
>> +	readl_relaxed((csa)->base + (offset))
>> +
>> +#define etm4x_read32(csa, offset)			\
>> +	readl((csa)->base + (offset))
>> +
>> +#define etm4x_relaxed_write32(csa, val, offset)		\
>> +	writel_relaxed((val), (csa)->base + (offset))
>> +
>> +#define etm4x_write32(csa, val, offset)			\
>> +	writel((val), (csa)->base + (offset))
>> +
>> +#define etm4x_relaxed_read64(csa, offset)		\
>> +	readq_relaxed((csa)->base + (offset))
>> +
>> +#define etm4x_read64(csa, offset)			\
>> +	readq((csa)->base + (offset))
>> +
>> +#define etm4x_relaxed_write64(csa, val, offset)		\
>> +	writeq_relaxed((val), (csa)->base + (offset))
>> +
>> +#define etm4x_write64(csa, val, offset)			\
>> +	writeq((val), (csa)->base + (offset))
>> +
> 
> Since I haven't gone through the rest of the patches I'll assume you want to
> enhance the above to pick an access type at some point in the future.
> 

Yes, they are plumbed in with the introduction of system instruction
support. We don't want to the overhead of a function call for
each register access. Also, we would like to avoid jumping
through the large switch..cases for a compile time constant
offset. Hence this macro.

Cheers
Suzuki