Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Subject: Re: [RFC PATCH 08/14] coresight: etm4x: Convert all register accesses
To:     mathieu.poirier@linaro.org
Cc:     linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org,
        mike.leach@linaro.org, coresight@lists.linaro.org
References: <20200722172040.1299289-1-suzuki.poulose@arm.com>
 <20200722172040.1299289-9-suzuki.poulose@arm.com>
 <20200730202021.GE3155687@xps15>
From:   Suzuki K Poulose <suzuki.poulose@arm.com>
Message-ID: <c3f6b2a2-67d6-44ed-9e03-c9f929bb469e@arm.com>
Date:   Fri, 31 Jul 2020 10:49:37 +0100
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101
 Thunderbird/52.7.0
MIME-Version: 1.0
In-Reply-To: <20200730202021.GE3155687@xps15>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

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