[U-Boot] [PATCH v4 1/3] net: designware: fix descriptor layout and warnings on 64-bit archs

[Joe Hershberger]

On Mon, Apr 18, 2016 at 5:57 AM, Beniamino Galvani <b.galvani at gmail.com> wrote:
> On Sun, Apr 17, 2016 at 10:59:11PM +0200, Marek Vasut wrote:
>> On 04/17/2016 01:14 PM, Beniamino Galvani wrote:
>> > On Sun, Apr 17, 2016 at 11:56:58AM +0200, Marek Vasut wrote:
>> >>> -         desc_p->dmamac_addr = &txbuffs[idx * CONFIG_ETH_BUFSIZE];
>> >>> -         desc_p->dmamac_next = &desc_table_p[idx + 1];
>> >>> +         desc_p->dmamac_addr = (ulong)&txbuffs[idx * CONFIG_ETH_BUFSIZE];
>> >>> +         desc_p->dmamac_next = (ulong)&desc_table_p[idx + 1];
>> >>
>> >> Why don't you use u32 instead of ulong ? The u32 is well defined.
>> >> DTTO all over the place.
>> >
>> > &txbuffs[idx * CONFIG_ETH_BUFSIZE] is a pointer (and hence has the
>> > size of a ulong) and casting it to u32 would give a warning on 64 bit
>> > archs ("cast from pointer to integer of different size").
>>
>> Will cast to uintptr_t and then to u32 help ?
>
> Note that uintptr_t is defined as ulong and the second cast to u32 is
> not needed because C does not require casts between arithmetic
> types. So I don't see much difference.
>
>> It's just a feeling, but casting to ulong just to circumvent compiler
>> warning does not sound right.
>
> It seems fine to me, the (ulong) is needed to cast the pointer to an
> arithmetic type of equivalent size which then can be assigned to an
> u32 variable.

Sorry, didn't notice that you already had this conversation in a
previous version.

>> >> btw just curious, but what will happen if the descriptors get allocated
>> >> in area above 4GiB ? Will the code silently corrupt memory by discarding
>> >> the top bits in the descriptor pointer?
>> >
>> > No, if the driver private structure (which contains buffers and
>> > descriptors) is above 4GiB, designware_initialize() will complain and
>> > return an error.
>>
>> Which code checks that ?
>
>  +       if ((unsigned long long)priv + sizeof(*priv) > (1ULL << 32)) {
>  +               printf("designware: buffers are outside DMA memory\n");
>  +               return -EINVAL;
>  +       }
>  +
>
> Beniamino
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot