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

[Marek Vasut]

On 5/7/19 4:01 PM, Christoph Müllner wrote:
> 
> 
> 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.

At least that's what Linux uses for mapped addresses.

-- 
Best regards,
Marek Vasut