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]
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

Powered by blists - more mailing lists