RE: [PATCH] powerpc/usb: use unsigned long to type cast an address of ioremap
> Below are codes for accessing usb sysif_regs in driver: > > usb_sys_regs = (struct usb_sys_interface *) > ((u32)dr_regs + USB_DR_SYS_OFFSET); > > these codes work in 32-bit, but in 64-bit, use u32 to type cast the address > of ioremap is not right, and accessing members of 'usb_sys_regs' will cause > call trace, so use unsigned long for both 32-bit and 64-bit. Wouldn't a (char *) cast be even better? David ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] powerpc/usb: use unsigned long to type cast an address of ioremap
On Thu, Nov 3, 2011 at 4:58 AM, Shaohui Xie wrote: > > usb_sys_regs = (struct usb_sys_interface *) > - ((u32)dr_regs + USB_DR_SYS_OFFSET); > + ((unsigned long)dr_regs + USB_DR_SYS_OFFSET); This makes more sense: usb_sys_regs = (void *)dr_regs + USB_DR_SYS_OFFSET; -- Timur Tabi Linux kernel developer at Freescale ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH] powerpc/usb: use unsigned long to type cast an address of ioremap
> On Thu, Nov 3, 2011 at 4:58 AM, Shaohui Xie wrote: > > > > usb_sys_regs = (struct usb_sys_interface *) > > - ((u32)dr_regs + USB_DR_SYS_OFFSET); > > + ((unsigned long)dr_regs + > > USB_DR_SYS_OFFSET); > > This makes more sense: > > usb_sys_regs = (void *)dr_regs + USB_DR_SYS_OFFSET; But that is invalid C. David ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] powerpc/usb: use unsigned long to type cast an address of ioremap
David Laight wrote: >> > usb_sys_regs = (void *)dr_regs + USB_DR_SYS_OFFSET; > But that is invalid C. What's invalid about it? I haven't tried compiling this specific line of code, but I've done stuff like it in the past many times. Are you talking about adding an integer to a void pointer? If so, then that's something that gcc supports and that the kernel uses all over the place. A char* is incorrect because a char could be more than one byte, in theory. -- Timur Tabi Linux kernel developer at Freescale ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH] powerpc/usb: use unsigned long to type cast an address of ioremap
> >> > usb_sys_regs = (void *)dr_regs + USB_DR_SYS_OFFSET; > > > But that is invalid C. > > What's invalid about it? I haven't tried compiling this > specific line of code, but I've done stuff like it in the past many times. > > Are you talking about adding an integer to a void pointer? > If so, then that's something that gcc supports and that the kernel uses > all over the place. Arithmetic on 'void *' should not be done. I know some versions of gcc allow it (provided some warning level/option is enabled) but that doesn't mean it is valid. My suspicions are that is was allowed due to the way 'void *' was originally bodged into gcc. > A char* is incorrect because a char could be more > than one byte, in theory. It is somewhat difficult to untangle the standard, but sizeof (char) is defined to be one. Of course, the C language doesn't actually require that you can converts between pointers to different types in any well-defined manner. But most of the low level device access assumes an adequately linear address space. David ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] powerpc/usb: use unsigned long to type cast an address of ioremap
On Nov 3, 2011, at 7:16 AM, David Laight wrote: > Arithmetic on 'void *' should not be done. I know some versions of > gcc allow it (provided some warning level/option is enabled) but > that doesn't mean it is valid. > My suspicions are that is was allowed due to the way 'void *' > was originally bodged into gcc. Well, I don't know what else to say. Arithmetic on void* is done all over the kernel. You're a little late to the party if you're going to advocate its avoidance. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] powerpc/usb: use unsigned long to type cast an address of ioremap
Arithmetic on 'void *' should not be done. I know some versions of gcc allow it (provided some warning level/option is enabled) but that doesn't mean it is valid. All relevant GCC versions support it (going back to 2.x at least). It is _always_ allowed, whatever compiler options you use; -pedantic or -Wpointer-arith will warn (which you can upgrade to an error). The Linux kernel is not built with these warnings enabled. A char* is incorrect because a char could be more than one byte, in theory. It is somewhat difficult to untangle the standard, but sizeof (char) is defined to be one. A char takes exactly one byte. A byte could be more than eight bits, of course. In GCC, sizeof(void) is 1 as well. Of course, the C language doesn't actually require that you can converts between pointers to different types in any well-defined manner. It does actually, see 6.3.2.3/7. You can convert any pointer to object to a pointer to a different object type, as long as it is properly aligned. You cannot in general access the object as that new type of course, but it is perfectly well-defined; in particular, you can convert it back to the original type and get the same value again, and you can walk consecutive bytes of the object by using a pointer to character type. Segher ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
