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

[Beniamino Galvani]

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.

> >> 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