> On 07 Jun 2017, at 08:28, Marek Vasut <ma...@denx.de> wrote:
>
> On 06/06/2017 05:11 PM, Dr. Philipp Tomsich wrote:
>>
>>> On 06 Jun 2017, at 16:35, Marek Vasut <ma...@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, ®->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.toms...@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@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, ®->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 ?
Running buildman (w/ a GCC 6.3 configured as aarch-unknown-elf) against
unrelated
patch-series (for a Rockchip device that has a different USB controller) fails
due to the
warnings raised from this. Just casting those warnings away (without having an
“assertion-like” diagnostic printed) doesn’t sound like the best plan either,
though.
Details of the buildman-failure are in the above commit-message.
Regards,
Philipp.
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
