[U-Boot] [PATCH 2/3] bouncebuf: Add DMA validation check to addr_aligned().

[Christoph Müllner]

On 07.05.19 15:53, Marek Vasut wrote:
> On 5/7/19 3:52 PM, Christoph Müllner wrote:
>>
>>
>> On 5/7/19 3:48 PM, Christoph Müllner wrote:
>>>
>>>
>>> On 07.05.19 15:05, Marek Vasut wrote:
>>>> On 5/7/19 11:05 AM, Christoph Muellner wrote:
>>>>> Currently addr_aligned() performs an alignment and a length check
>>>>> to validate the DMA address. However, some machines have stricter
>>>>> restrictions of DMA-able addresses.
>>>>>
>>>>> This patch adds a call to mach_addr_is_dmaable() to honor this
>>>>> machine specific restrictions.
>>>>>
>>>>> Signed-off-by: Christoph Muellner <christoph.muellner at theobroma-systems.com>
>>>>> ---
>>>>>
>>>>>  common/bouncebuf.c | 6 ++++++
>>>>>  1 file changed, 6 insertions(+)
>>>>>
>>>>> diff --git a/common/bouncebuf.c b/common/bouncebuf.c
>>>>> index a7098e2caf..26ddf30ea2 100644
>>>>> --- a/common/bouncebuf.c
>>>>> +++ b/common/bouncebuf.c
>>>>> @@ -26,6 +26,12 @@ static int addr_aligned(struct bounce_buffer *state)
>>>>>  		return 0;
>>>>>  	}
>>>>>  
>>>>> +	/* Check if valid DMA address. */
>>>>> +	if (!mach_addr_is_dmaable((ulong)state->user_buffer)) {
>>>>
>>>> Is the cast necessary ?
>>>
>>> Of course not.
>>> Will be fixed in v2.
>>>
>>> Thanks!
>>
>> I take that back.
>> The cast is needed.
>> (And also done several times in this file)
>>
>> Otherwise you will get:
>>
>> common/bouncebuf.c: In function ‘addr_aligned’:
>> common/bouncebuf.c:35:33: warning: passing argument 1 of ‘mach_addr_is_dmaable’ makes integer from pointer without a cast [-Wint-conversion]
>>   if (!mach_addr_is_dmaable(state->user_buffer)) {
>>                             ~~~~~^~~~~~~~~~~~~
>> In file included from include/common.h:64,
>>                  from common/bouncebuf.c:8:
>> include/init.h:128:40: note: expected ‘long unsigned int’ but argument is of type ‘void *’
>>  int mach_addr_is_dmaable(unsigned long addr);
>>                           ~~~~~~~~~~~~~~^~~~
> 
> Shouldn't the function use void __iomem * in the first place ?

IMHO Matter of taste.
If you prefer it this way, then I can change.
Doing grep "unsigned long addr" gave me enough confidence to use that.
But I leave this up to you.