On 7/4/2018 6:06 PM, Fabio Estevam wrote: > Hi Horia, > > On Wed, Jul 4, 2018 at 8:45 AM, Horia Geanta <horia.geanta@xxxxxxx> wrote: > >> 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. >> >> Actually this is the semantics intended for the CAAM patch, see the note at the >> end of the commit message that refers to addresses, not values: >> To be consistent with CAAM engine HW spec: in case of 64-bit registers, >> irrespective of device endianness, the lower address should be read from >> / written to first, followed by the upper address. >> >> >> 2. CAAM driver I/O accessors for i.MX case >> >> CAAM block in some i.MX parts has broken endianness for 64b registers. >> For e.g. for i.MX6S/SL/D/Q even though CAAM is little endian, BARs for I/O rings >> have to be programmed as: >> I/O Ring BAR+0: unused >> I/O Ring BAR+4: IOVA (32-bit little endian) >> when the proper layout (for a 64b register) would have been to program IOVA at >> BAR+0. >> >> This explains why I/O accessors in CAAM driver handle things differently in case >> caam_imx=true. >> >> Since this quirk cannot be accommodated in generic fashion, code dealing with >> caam_imx has to stay. > > Should I sent a revert of patch 46e4bf08f6388 for the boot regression for now? > > Then the two issues you pointed out could be fixed later. > I guess it would be better this way. Thanks, Horia
