On 06/07/2017 02:38 PM, Simon Glass wrote:
> +Tom for comment
>
> Hi Marek,
>
> On 7 June 2017 at 00:27, Marek Vasut <ma...@denx.de> wrote:
>> On 06/07/2017 02:16 AM, Simon Glass wrote:
>>> Hi,
>>>
>>> On 6 June 2017 at 17:59, Dr. Philipp Tomsich
>>> <philipp.toms...@theobroma-systems.com> wrote:
>>>> Simon,
>>>>
>>>>> On 06 Jun 2017, at 23:09, Simon Glass <s...@chromium.org> wrote:
>>>>>
>>>>> Hi Philipp,
>>>>>
>>>>> On 6 June 2017 at 07:42, Philipp Tomsich
>>>>> <philipp.toms...@theobroma-systems.com> wrote:
>>>>>> The regs_otg field in uintptr_t of the platform data structure for
>>>>>> dwc2-otg has thus far been an unsigned int, but will eventually be
>>>>>> casted into a void*.
>>>>>>
>>>>>> This raises the following error with GCC 6.3 and buildman:
>>>>>> ../drivers/usb/gadget/dwc2_udc_otg.c: In function 'dwc2_udc_probe':
>>>>>> ../drivers/usb/gadget/dwc2_udc_otg.c:821:8: warning: cast to pointer
>>>>>> from integer of different size [-Wint-to-pointer-cast]
>>>>>> reg = (struct dwc2_usbotg_reg *)pdata->regs_otg;
>>>>>> ^
>>>>>>
>>>>>> This changes regs_otg to a uintptr_t to ensure that it is large enough
>>>>>> to hold any valid pointer (and fix the associated warning).
>>>>>>
>>>>>> Signed-off-by: Philipp Tomsich <philipp.toms...@theobroma-systems.com>
>>>>>>
>>>>>> ---
>>>>>>
>>>>>> Changes in v2:
>>>>>> - (new patch) fix a int-to-pointer cast warning for regs_otg in
>>>>>> dwc2-otg to fix a buildman failure for u-boot-rockchip/master@2b19b2f
>>>>>>
>>>>>> include/usb/dwc2_udc.h | 2 +-
>>>>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>>
>>>>>> diff --git a/include/usb/dwc2_udc.h b/include/usb/dwc2_udc.h
>>>>>> index 7324d8a..1a370e0 100644
>>>>>> --- a/include/usb/dwc2_udc.h
>>>>>> +++ b/include/usb/dwc2_udc.h
>>>>>> @@ -16,7 +16,7 @@ struct dwc2_plat_otg_data {
>>>>>> int phy_of_node;
>>>>>> int (*phy_control)(int on);
>>>>>> unsigned int regs_phy;
>>>>>> - unsigned int regs_otg;
>>>>>> + uintptr_t regs_otg;
>>>>>
>>>>> Can you use ulong instead?
>>>>
>>>> Sure, but can I first ask “why?”.
>>>> I may reopen an old discussion with this… if so, forgive my ignorance:
>>>>
>>>> uintptr_t makes the most sense for this use case in the C99 (or later)
>>>> type system,
>>>> as we want this field to hold an integer (i.e. the address from the
>>>> physical memory
>>>> map for one of the register blocks) that will be casted into a pointer.
>>>> The uintptr_t type will always the matching size in any and all
>>>> programming models;
>>>> in contrast, ulong would be wrong for LLP64 (and LLP64 probably “doesn’t
>>>> matter”
>>>> in the context of U-Boot anyway).
>>>>
>>>> What I always found odd, was that uintptr_t is optional according to
>>>> ISO9899.
>>>> I would thus not have been surprised if there’s a concern for portability
>>>> between
>>>> compilers behind this, but then again … U-Boot makes extensive use of GCC
>>>> extensions (such as inline assembly).
>>>>
>>>> So I am apparently missing something here.
>>>
>>> I don't know of any deep reason except that long is defined to be able
>>> to hold an address, and ulong makes sense since an address is
>>> generally considered unsigned.
>>>
>>> U-Boot by convention uses ulong for addresses.
>>
>> I was under the impression that u-boot by convention uses uintptr_t for
>> addresses.
>>
>>> You can see this all
>>> around the code base so I am really just arguing in favour of
>>> consistency (and I suppose ulong is easier to type!)
>>
>> Then I guess half of the codebase is inconsistent.
>
> Actually about 10%:
>
> git grep uintptr_t |wc -l
> 395
> git grep ulong |wc -l
> 4024
I don't think this is a valid way to count it at all, since uintptr_t is
only used for casting pointer to number, while ulong is used for
arbitrary numbers.
> Clearly we use ulong all over the place for addresses - see image.c,
> most commands, etc.
But none of those use ulong as device address pointers .
> If we are going to change then it should be a
> deliberate decision and a tree-wide update. I'm happy enough with
> ulong.
>
> Tom, what do you what to do here?
>
> Regards,
> Simon
>
--
Best regards,
Marek Vasut
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
