linux-kernel - Re: [PATCH v2 08/36] coresight: tmc: Clean up device specific data

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] [day] [month] [year] [list]

Message-ID: <61ccfc44-c062-fa3e-6de9-825def4e9891@arm.com>
Date:   Fri, 3 May 2019 18:13:36 +0100
From:   Suzuki K Poulose <suzuki.poulose@....com>
To:     mathieu.poirier@...aro.org
Cc:     linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
        coresight@...ts.linaro.org, mike.leach@...aro.org,
        rjw@...ysocki.net, robert.walker@....com
Subject: Re: [PATCH v2 08/36] coresight: tmc: Clean up device specific data

Hi Mathieu

On 17/04/2019 22:23, Mathieu Poirier wrote:
> On Mon, Apr 15, 2019 at 05:03:51PM +0100, Suzuki K Poulose wrote:
>> In preparation to use a consistent device naming scheme,
>> clean up the device link tracking in replicator driver.
>> Use the "coresight" device instead of the "real" parent device
>> for all internal purposes. All other requests (e.g, power management,
>> DMA operations) must use the "real" device which is the parent device.
>>
>> Since the CATU driver also uses the TMC-SG infrastructure, update
>> the callers to ensure they pass the appropriate device argument
>> for the tables.
>>
>> Cc: Mathieu Poirier <mathieu.poirier@...aro.org>
>> Signed-off-by: Suzuki K Poulose <suzuki.poulose@....com>

>> diff --git a/drivers/hwtracing/coresight/coresight-tmc-etr.c b/drivers/hwtracing/coresight/coresight-tmc-etr.c
>> index f684283..0911f9c 100644
>> --- a/drivers/hwtracing/coresight/coresight-tmc-etr.c
>> +++ b/drivers/hwtracing/coresight/coresight-tmc-etr.c
>> @@ -153,10 +153,11 @@ static void tmc_pages_free(struct tmc_pages *tmc_pages,
>>   			   struct device *dev, enum dma_data_direction dir)
>>   {
>>   	int i;
>> +	struct device *real_dev = dev->parent;
> 
> I would have kept the 'dev' as it is quite obvious from the dev->parent that we
> are getting a reference on the parent.  That is just my opinion and it is
> entirely up to you.

"dev" is already an argument to the function. Hence the "real_dev" choice.

>> diff --git a/drivers/hwtracing/coresight/coresight-tmc.h b/drivers/hwtracing/coresight/coresight-tmc.h
>> index 487c537..3eeadcf 100644
>> --- a/drivers/hwtracing/coresight/coresight-tmc.h
>> +++ b/drivers/hwtracing/coresight/coresight-tmc.h
>> @@ -175,7 +175,6 @@ struct etr_buf {
>>    */
>>   struct tmc_drvdata {
>>   	void __iomem		*base;
>> -	struct device		*dev;
> 
> Please clean up the structure documentation.
> 
> With that and regardless of what you decide to do about the 'real_dev':

Done.

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

Cheers
Suzuki