> From: Vincent Pelletier [mailto:plr.vincent@xxxxxxxxx] > Sent: Friday, January 23, 2015 12:19 AM > > 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 ? I was thinking more along the lines of what dwc3 does. See drivers/usb/dwc3/io.h. One solution would be to do as above, and put in a compile-time check for your platform and call the appropriate access function. I believe writel/readl also preserves memory ordering so it might affect other platforms to just use the __raw equivalent in its place. John ��.n��������+%������w��{.n�����{���)�� jg��� �����ݢj����G���� ���j:+v���w�m������w�������h�����٥
