+Tom for comment Hi Marek,
On 7 June 2017 at 00:27, Marek Vasut <[email protected]> wrote: > On 06/07/2017 02:16 AM, Simon Glass wrote: >> Hi, >> >> On 6 June 2017 at 17:59, Dr. Philipp Tomsich >> <[email protected]> wrote: >>> Simon, >>> >>>> On 06 Jun 2017, at 23:09, Simon Glass <[email protected]> wrote: >>>> >>>> Hi Philipp, >>>> >>>> On 6 June 2017 at 07:42, Philipp Tomsich >>>> <[email protected]> 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 <[email protected]> >>>>> >>>>> --- >>>>> >>>>> 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 Clearly we use ulong all over the place for addresses - see image.c, most commands, etc. 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 _______________________________________________ U-Boot mailing list [email protected] https://lists.denx.de/listinfo/u-boot
