linux-kernel - Re: [PATCH v1] dma: imx-sdma: add support for sdma memory copy

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: <5357F909.7070205@infradead.org>
Date:	Wed, 23 Apr 2014 10:31:53 -0700
From:	Randy Dunlap <rdunlap@...radead.org>
To:	Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
	Robin Gong <b38343@...escale.com>
CC:	"Koul, Vinod" <vinod.koul@...el.com>,
	"Williams, Dan J" <dan.j.williams@...el.com>,
	"dmaengine@...r.kernel.org" <dmaengine@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"linux-arm-kernel@...ts.infradead.org" 
	<linux-arm-kernel@...ts.infradead.org>
Subject: Re: [PATCH v1] dma: imx-sdma: add support for sdma memory copy

On 04/22/14 04:24, Andy Shevchenko wrote:
> On Tue, 2014-04-22 at 18:51 +0800, Robin Gong wrote:
>> On Tue, Apr 22, 2014 at 10:28:05AM +0000, Shevchenko, Andriy wrote:
>>> On Fri, 2014-04-18 at 17:41 +0800, Robin Gong wrote:
>>>> On Thu, Apr 17, 2014 at 10:24:50AM +0000, Shevchenko, Andriy wrote:
>>>>> On Thu, 2014-04-17 at 18:01 +0800, Robin Gong wrote:
>>>
>>> []
>>>
>>>>>> +	dev_dbg(sdma->dev, "memcpy: %x->%x, len=%d, channel=%d.\n",
>>>>>
>>>>> %pad for dma_addr_t variables.
>>>>>
>>>> Yes, %x here is not proper, will be %#llx here to align with others similar
>>>> code in this file.
>>>
>>> Why %#llx? You don't need the specific casting since kernel has special
>>> specifiers for phys_addr_t and dma_addr_t and their derivatives (see
>>> Documentation/printk-formats.txt)
>>>
>> I think both are ok, why I choose %llx is only for align the code style,
> 
> Yes, but it was started being developed earlier than kernel gains the
> feature.
> 
>>  you
>> can find the same code in sdma_prep_dma_cyclic function. below description also
>> copy from Documentation/printk-formats.txt:
>>
>>
>> If <type> is dependent on a config option for its size (e.g., sector_t,
>> blkcnt_t, phys_addr_t, resource_size_t) or is architecture-dependent
>> for its size (e.g., tcflag_t), use a format specifier of its largest
>> possible type and explicitly cast to it.  Example:
>>
>> 	printk("test: sector number/total blocks: %llu/%llu\n",
>> 		(unsigned long long)sector, (unsigned long long)blockcount);
> 
> Yeah, I think it requires to be updated accordingly to last changes
> regarding to new specifiers.
> 
> +Cc Randy. Randy, do you think is a matter of fact that we have to
> update that paragraph somehow and recommend to prefer special specifiers
> over explicit casting to longest possible type?

Yes, I agree, if there is a printk format specifier that supports a certain type
to be printed, we should prefer to use that instead of the generic casting method.

IMO, the cast to (long long) or (unsigned long long) or (u64) or (s64) should be a last
resort safe method.


>>>>>> +		dev_dbg(sdma->dev, "entry %d: count: %d dma: 0x%08x %s%s\n",
>>>>>> +				i, count, sg_src->dma_address,
>>>>>
>>>>> %pad for dma_addr_t.
>>>>>
>>>> Accept the idea, same as the above.
>>>
>>> Same as above.
> 
> 


-- 
~Randy
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/