On 07.05.19 17:56, Marek Vasut wrote: > 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.
Too late, but I have no fear from a v3. _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot