Hi, On Wed, Mar 11, 2015 at 12:28:41AM +0100, Vincent Pelletier wrote: > On Tue, 10 Mar 2015 16:05:36 -0500, Felipe Balbi <balbi@xxxxxx> wrote: > > I don't get it, why is it so that only mips needs this ? What's special > > about mips' writel/readl implementation that it can't be used here ? > > > > This works for everybody else. > > Would it be a design oddity of this SoC that device registers do match > CPU endianness, instead of matching implementations used in other SoCs ? > > This is the first time I ever get to debug some endianness issue in > a driver, let alone on a SoC. I have extremely little knowledge of how > this is expected to happen in general. Please pardon my naivety. > > AFAIK the Raspberry Pi is another SoC where this device is present, and > the RPi is little endian. As this driver works there, it suggests the > registers are little-endian too. I think proposed change will not break > this, as discussed with John Youn on previous submission. I couldn't > get a reasonable RPi kernel build environment (because of the lack of > working kernel .config for a recent kernel) to verify this though. > > > Is it so that dwc2 and cpu endianness don't match here ? > > They do match actually. It's readl/writel internal byte swapping > happening on big-endian archs which breaks this. Once straightened as > per Antti's patch, the driver detects the HCD and USB devices work fine. yeah, but isn't this a bug with MIPS itself ? If device and cpu endianness match, why is it swapping anything ? > Well, actually there is one out-of-driver (platform-specific) register > to set so DMA transfers match CPU endianness (I have no idea why it > doesn't default to the working setting). > > > weren't readl() and writel() already supposed to handle endianness for > > us? > > If asm-generic implementation is representative of the intent, then the > intent is clearly to read little-endian values from memory and convert > to CPU endianness. > > http://lxr.free-electrons.com/source/include/asm-generic/io.h#L127 > http://lxr.free-electrons.com/source/include/asm-generic/io.h#L161 You're right, I was under the impression that readl() would only swap bytes if it didn't match cpu endianness so it would. Interesting... I guess your patch is really the only way to go :-s > Which brings me back to my original questions: Is SoC hardware expected > to expose registers in cpu-consistent endianness (or maybe even > "default-cpu-endianness") ? I don't think there is such expectation although it's what usually happens. > > wrong comment style. > > Expected style is to prefix lines with * and to end block below these, > right ? right :-) > Nice catch, thanks. np. -- balbi
Attachment:
signature.asc
Description: Digital signature
