[U-Boot] [PATCH v2 5/7] usb: dwc2-otg: fix warnings for ep->dma_buf for LP64 builds

[Marek Vasut]

On 06/06/2017 05:11 PM, Dr. Philipp Tomsich wrote:
> Marek,
> 
>> On 06 Jun 2017, at 16:35, Marek Vasut <marex at denx.de> wrote:
>>
>> On 06/06/2017 03:42 PM, Philipp Tomsich wrote:
>>> For LP64 build w/ GCC 6.3, the handling of the ep->dma_buf variable
>>> when (truncating and) writing into the controller's registers is
>>> unsafe and triggers the following compiler warning (thus failing by
>>> buildman tests):
>>>  ../drivers/usb/gadget/dwc2_udc_otg_xfer_dma.c: In function 'setdma_rx':
>>>  ../drivers/usb/gadget/dwc2_udc_otg_xfer_dma.c:116:9: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
>>>    writel((unsigned int) ep->dma_buf, &reg->out_endp[ep_num].doepdma);
>>>           ^
>>>
>>> This change fixes the warning and makes the code a bit more robust by
>>> doing the following:
>>> - we check whether ep->dma_buf can be safely truncated to 32 bits
>>>  (i.e. whether dma_buf is the same value when masked to 32 bits) and
>>>  emit an error message if that is not the case
>>> - we cast to a uintptr_t and let the writel macro truncate the
>>>  uintptr_t (potentially a 64bit value) to 32bits
>>>
>>> Signed-off-by: Philipp Tomsich <philipp.tomsich at theobroma-systems.com>
>>>
>>> ---
>>>
>>> Changes in v2:
>>> - (new patch) fix warnings and add some robustness for the truncation
>>>  of ep->dma_buf in dwc2-otg for LP64 builds to fix a buildman failure
>>>  for u-boot-rockchip/master at 2b19b2f
>>>
>>> drivers/usb/gadget/dwc2_udc_otg_xfer_dma.c | 6 +++++-
>>> 1 file changed, 5 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/usb/gadget/dwc2_udc_otg_xfer_dma.c b/drivers/usb/gadget/dwc2_udc_otg_xfer_dma.c
>>> index 0d6d2fb..f5c926c 100644
>>> --- a/drivers/usb/gadget/dwc2_udc_otg_xfer_dma.c
>>> +++ b/drivers/usb/gadget/dwc2_udc_otg_xfer_dma.c
>>> @@ -113,7 +113,11 @@ static int setdma_rx(struct dwc2_ep *ep, struct dwc2_request *req)
>>> 	invalidate_dcache_range((unsigned long) ep->dma_buf,
>>> 				(unsigned long) ep->dma_buf + ep->len);
>>>
>>> -	writel((unsigned int) ep->dma_buf, &reg->out_endp[ep_num].doepdma);
>>> +	/* ensure that ep->dma_buf fits into 32 bits */
>>> +	if (((uintptr_t)ep->dma_buf & 0xffffffff) != (uintptr_t)ep->dma_buf)
>>> +		error("ep->dma_buf does not fit into a 32 bits");
>>
>> You print an error, but still continue with the function ? That'll
>> probably lead to a crash, you should rather abort. In fact, I suspect
>> you can use bounce-buffer to assure the dma buffer is in 32bit space.
> 
> This tries to stay close to the current behaviour (we have no hardware to test this,
> but I wanted to get it out of the way as it caused buildman-failures) and to provide
> diagnostics for whoever first encounters an issue here.
> 
> Note that I would only expect a DMA error, if someone really hit this issue ...

So this patch is just some hypothetical fix ? What triggered creation of
this patch ?

-- 
Best regards,
Marek Vasut