On 5/7/19 5:22 PM, Christoph Müllner wrote:
>
>
> On 07.05.19 17:04, Marek Vasut wrote:
>> 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.muell...@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.
>
> In Linux two versions of that function are needed: one for phys_addr_t and
> one for void *.
> The linear mapping in U-Boot allows to combine both.
> Therefore I see it as a matter of taste.
>
> I will change this to "void __iomem *" and cast to uintptr_t in the rk3399
> implementation of that function in a v2 series.
Wait for more feedback please. Maybe someone has a more compelling
argument for one or the other.
--
Best regards,
Marek Vasut
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
