Mostly minor grammer fixes: * Will Deacon <will.deacon@xxxxxxx> wrote: > + (*) readX(), writeX(): > > + The readX() and writeX() MMIO accessors take a pointer to the peripheral > + being accessed as an __iomem * parameter. For pointers mapped with the > + default I/O attributes (e.g. those returned by ioremap()), then the > + ordering guarantees are as follows: s/then the /the > + 1. All readX() and writeX() accesses to the same peripheral are ordered > + with respect to each other. For example, this ensures that MMIO register > + writes by the CPU to a particular device will arrive in program order. Vertical alignment whitespace damage: some indentations are done via spaces, one via tabs. Please standardize to tabs. I'd also suggest: s/For example, this ensures /For example this ensures for the rest of the text too. The comma after the 'For example,' introductory phrase is grammatically correct but stylistically confusing, because in reality there's a *second* introductory phrase via "this ensures". > > + 2. A writeX() by the CPU to the peripheral will first wait for the > + completion of all prior CPU writes to memory. For example, this ensures > + that writes by the CPU to an outbound DMA buffer allocated by > + dma_alloc_coherent() will be visible to a DMA engine when the CPU writes > + to its MMIO control register to trigger the transfer. > > + 3. A readX() by the CPU from the peripheral will complete before any > + subsequent CPU reads from memory can begin. For example, this ensures > + that reads by the CPU from an incoming DMA buffer allocated by > + dma_alloc_coherent() will not see stale data after reading from the DMA > + engine's MMIO status register to establish that the DMA transfer has > + completed. > > + 4. A readX() by the CPU from the peripheral will complete before any > + subsequent delay() loop can begin execution. For example, this ensures > + that two MMIO register writes by the CPU to a peripheral will arrive at > + least 1us apart if the first write is immediately read back with readX() > + and udelay(1) is called prior to the second writeX(). This might be more readable via some short code sequence instead? > > + __iomem pointers obtained with non-default attributes (e.g. those returned > + by ioremap_wc()) are unlikely to provide many of these guarantees. This part is a bit confusing I think, because it's so cryptic. "Unlikely" as in probabilistic? ;-) So I think we should at least give some scope of the exceptions and expected trouble, or at least direct people to those APIs to see what the semantics are? > > + (*) readX_relaxed(), writeX_relaxed(): > > + These are similar to readX() and writeX(), but provide weaker memory > + ordering guarantees. Specifically, they do not guarantee ordering with > + respect to normal memory accesses or delay() loops (i.e bullets 2-4 above) > + but they are still guaranteed to be ordered with respect to other accesses > + to the same peripheral when operating on __iomem pointers mapped with the > + default I/O attributes. > > + (*) readsX(), writesX(): > > + The readsX() and writesX() MMIO accessors are designed for accessing > + register-based, memory-mapped FIFOs residing on peripherals that are not > + capable of performing DMA. Consequently, they provide only the ordering > + guarantees of readX_relaxed() and writeX_relaxed(), as documented above. So is there any difference between 'X_relaxed' and 'sX' variants? What is the 's' about? Thanks, Ingo