On Wed, Jul 4, 2018 at 2:45 PM, Horia Geanta <horia.geanta@xxxxxxx> wrote: > On 7/4/2018 2:58 AM, Logan Gunthorpe wrote: >> >> >> On 03/07/18 04:21 PM, Andy Shevchenko wrote: >>> It is an explicit call to BUG(). >>> That's why we see wrong instruction trap. >> >> Ok, I think I see the problem... the code is rather confusing: >> >> Prior to the patch, IOs were BE depending on caam_little_end but if >> caam_imx was set, then it wrote two LE writes with the high one first. >> After the patch, it writes two BE writes with the high one first. >> > I think there are two separate issues here: > > 1. Semantics of operations in io-64-nonatomic-lo-hi.h, io-64-nonatomic-hi-lo.h > > Logan, you mentioned the following (which unfortunately I somehow missed): > https://lore.kernel.org/lkml/c3f2e061-5ed1-5c74-b955-3d2bfb0da074@xxxxxxxxxxxx > The lo_hi/hi_lo functions seem to always refer to the data being written > or read not to the address operated on. > > OTOH, initial commit that added asm-generic/io-64-nonatomic-lo-hi.h and > asm-generic/io-64-nonatomic-hi-lo.h mentions: > 797a796a13df6 ("asm-generic: architecture independent readq/writeq for 32bit > environment") > - <asm-generic/io-64-nonatomic-lo-hi.h> provides non-atomic readq/ writeq with > the order of lower address -> higher address > - <asm-generic/io-64-nonatomic-hi-lo.h> provides non-atomic readq/ writeq with > reversed order > > I think we should keep the initial semantics when adding support for > io{read|write}64, i.e. "lo" and "hi" to refer to address and not to value. There is a ambiguity with the commit message. Since it had implemented only LE IO accessors the address semantics is the same as data flow. As a driver writer intuitively I would expect data flow semantics. It means it would be invariant to LE BE accessors, right? lo-hi: LE (0x0 0x4) BE (0x4 0x0) hi-lo: LE (0x4 0x0) BE (0x0 0x4) -- With Best Regards, Andy Shevchenko
