On Fri, 23 Jan 2015 02:38:37 +0000, John Youn <John.Youn@xxxxxxxxxxxx> wrote: > Having the __raw functions everywhere is not pretty and probably not a > good idea either. > > I would rather have a dwc2_writel/dwc2_readl (like in dwc3), and we > can figure out what needs to happen in there to support your platform. > > As for readl/writel or the __raw equivalents, I am not sure. I haven't > run the in-kernel driver on a big endian system so I don't know what > the issue is. But I suspect there may be consequences to other > platforms if we simply change this to __raw_readl/__raw_writel. (focussing on readl, but the following also applies to writel) readl is defined as __le32_to_cpu(__raw_readl(...)), so using readl on little endian is equivalent to __raw_readl, as __le32_to_cpu is a no-op. On big endian, it causes problems because __le32_to_cpu swaps bytes around, causing GSNPSID to be read as 0xnn2n544f instead of expected 0x4f542nnn - and likewise for all other registers. An earlier version (sent to linux-usb a week ago with not enough CC, but I cannot find it in archives now) defined dwc2_readl as le32_to_cpu(readl(...)) which is equivalent to le32_to_cpu(le32_to_cpu(__raw_readl(...))) and which, on big-endian, makes le32_to_cpu(le32_to_cpu()) a no-op and (ignoring possible compiler optimisation) swaps bytes twice per call. On little endian, it is still equivalent to __raw_readl(...). As for the prettiness of calling double-underscore functions, speaking for myself I'm not used to Linux development enough to tell. My python developer reflex tell me it's not how it should be. OTOH there is no "raw_readl" or so alias, and other USB drivers reference __raw_readl (example as of v3.14.28): linux/drivers/usb$ git grep -l __raw_readl gadget/at91_udc.c gadget/atmel_usba_udc.c gadget/atmel_usba_udc.h gadget/fsl_udc_core.c gadget/pxa27x_udc.h host/ehci-omap.c host/ehci-orion.c host/ehci-w90x900.c host/ehci.h host/isp1760-hcd.c host/ohci-da8xx.c host/ohci-nxp.c host/ohci-pxa27x.c musb/da8xx.c musb/davinci.c musb/musb_dsps.c musb/musb_io.h phy/phy-fsl-usb.c Would it be better to wrap __raw_readl in a macro and call that everywhere rather than calling __raw_readl itself ? Regards, -- Vincent Pelletier -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
