[U-Boot] [PATCH v2 4/7] usb: dwc2-otg: make regs_otg (in platdata) a uintptr_t

[Marek Vasut]

On 06/07/2017 02:53 PM, Simon Glass wrote:
> Hi Marek,
> 
> On 7 June 2017 at 06:41, Marek Vasut <marex at denx.de> wrote:
>> On 06/07/2017 02:38 PM, Simon Glass wrote:
>>> +Tom for comment
>>>
>>> Hi Marek,
>>>
>>> On 7 June 2017 at 00:27, Marek Vasut <marex at 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.tomsich at theobroma-systems.com> wrote:
>>>>>> Simon,
>>>>>>
>>>>>>> On 06 Jun 2017, at 23:09, Simon Glass <sjg at chromium.org> wrote:
>>>>>>>
>>>>>>> Hi Philipp,
>>>>>>>
>>>>>>> On 6 June 2017 at 07:42, Philipp Tomsich
>>>>>>> <philipp.tomsich at 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.tomsich at 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 at 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 .
> 
> Is there a distinction between a device address pointer and some other
> type of address?

ulong is not used only for addresses, which your metric doesn't account for.


-- 
Best regards,
Marek Vasut