linux-kernel - Re: [PATCH v3 09/26] coresight: Convert coresight

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <b1323f88-d6b8-5525-4d2f-d001236f8860@arm.com>
Date:   Wed, 4 Nov 2020 10:42:56 +0000
From:   Suzuki K Poulose <suzuki.poulose@....com>
To:     Mathieu Poirier <mathieu.poirier@...aro.org>
Cc:     linux-arm-kernel@...ts.infradead.org, mike.leach@...aro.org,
        coresight@...ts.linaro.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3 09/26] coresight: Convert coresight_timeout to use
 access abstraction

On 11/3/20 6:03 PM, Mathieu Poirier wrote:
> On Wed, Oct 28, 2020 at 10:09:28PM +0000, Suzuki K Poulose wrote:
>> Convert the generic routines to use the new access abstraction layer
>> gradually, starting with coresigth_timeout.
>>
>> Cc: Mike Leach <mike.leach@...aro.org>
>> Reviewed-by: Mathieu Poirier <mathieu.poirier@...aro.org>
>> Signed-off-by: Suzuki K Poulose <suzuki.poulose@....com>
>> ---



>> diff --git a/drivers/hwtracing/coresight/coresight-catu.c b/drivers/hwtracing/coresight/coresight-catu.c
>> index 5baf29510f1b..34c74b05c542 100644
>> --- a/drivers/hwtracing/coresight/coresight-catu.c
>> +++ b/drivers/hwtracing/coresight/coresight-catu.c
>> @@ -401,8 +401,9 @@ static const struct attribute_group *catu_groups[] = {
>>   
>>   static inline int catu_wait_for_ready(struct catu_drvdata *drvdata)
>>   {
>> -	return coresight_timeout(drvdata->base,
>> -				 CATU_STATUS, CATU_STATUS_READY, 1);
>> +	struct csdev_access *csa = &drvdata->csdev->access;
>> +
>> +	return coresight_timeout(csa, CATU_STATUS, CATU_STATUS_READY, 1);
>>   }
>>   
>>   static int catu_enable_hw(struct catu_drvdata *drvdata, void *data)
>> diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c
>> index e96deaca8cab..42ba989a6b5e 100644
>> --- a/drivers/hwtracing/coresight/coresight-core.c
>> +++ b/drivers/hwtracing/coresight/coresight-core.c
>> @@ -1412,23 +1412,26 @@ static void coresight_remove_conns(struct coresight_device *csdev)
>>   }
>>   
>>   /**
>> - * coresight_timeout - loop until a bit has changed to a specific state.
>> - * @addr: base address of the area of interest.
>> - * @offset: address of a register, starting from @addr.
>> + * coresight_timeout - loop until a bit has changed to a specific register
>> + *			state.
>> + * @csa: coresight device access for the device
>> + * @offset: Offset of the register from the base of the device.
>>    * @position: the position of the bit of interest.
>>    * @value: the value the bit should have.
>>    *
>>    * Return: 0 as soon as the bit has taken the desired state or -EAGAIN if
>>    * TIMEOUT_US has elapsed, which ever happens first.
>>    */
>> -
>> -int coresight_timeout(void __iomem *addr, u32 offset, int position, int value)
>> +int coresight_timeout(struct csdev_access *csa,
>> +		      u32 offset,
>> +		      int position,
>> +		      int value)
> 
> There is no need for stacking, please maximise the 80 characters.  The function
> stubs in coresight.h should also be revised.
> 
>>   {
>>   	int i;
>>   	u32 val;
>>   
>>   	for (i = TIMEOUT_US; i > 0; i--) {
>> -		val = __raw_readl(addr + offset);
>> +		val = csdev_access_read32(csa, offset);
> 
> I vaguely remember commenting on this, or perhaps it was on some othe patch you
> wrote...  Anyways, I think it is a good thing to go from an unordered access to
> an ordered access for the timeout function.

Yes, you did mention it in the v1.

> 
> With the above:
> 
> Reviewed-by: Mathieu Poirier <mathieu.poirier@...aro.org>

Thanks, will address the comments.

Suzuki