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

[Marek Vasut]

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.

-- 
Best regards,
Marek Vasut