Hi Marek, On 7 June 2017 at 06:55, Marek Vasut <[email protected]> wrote: > On 06/07/2017 02:53 PM, Simon Glass wrote: >> Hi Marek, >> >> On 7 June 2017 at 06:41, Marek Vasut <[email protected]> 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 <[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 >>> >>> 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.
OK, but if you look around you can see my point. See for example cmd/mem.c which uses ulong everywhere. Image handling is the same - see e.g. image.h struct image_info and struct bootm_headers. Are you saying those are wrong, or that this case is different, or something else? Regards, Simon _______________________________________________ U-Boot mailing list [email protected] https://lists.denx.de/listinfo/u-boot
