Messages in this thread | | | Date | Wed, 21 Sep 2022 22:01:43 +0200 | From | "Arnd Bergmann" <> | Subject | Re: [mtd:nand/next 11/31] drivers/mtd/nand/raw/cadence-nand-controller.c:1893:4: error: implicit declaration of function 'ioread64_rep' is invalid in C99 |
| |
On Wed, Sep 21, 2022, at 6:38 PM, Miquel Raynal wrote: > arnd@arndb.de wrote on Wed, 21 Sep 2022 17:49:11 +0200: >> On Wed, Sep 21, 2022, at 4:47 PM, Miquel Raynal wrote: > >> - every architecture should provide readsq()/readsl()/readsw()/readsb() >> these days, regardless of CONFIG_GENERIC_IOMAP. If x86 does >> not have that, we should fix asm-generic/io.h. > > ARM does not seem to define readsq/writesq. Should it be fixed?
64-bit Arm should get it from include/asm-generic/io.h. If it does not, this should be fixed. 32-bit Arm obviously cannot define them in a generic way.
>> - CONFIG_GENERIC_IOMAP just means an architecture uses the generic >> ioread32_rep() style wrapper around readsl()/insl(). On most >> architectures (not x86), insl() is implemented as a wrapper around >> readsl() itself, so readsl() and ioread32_rep() should be identical. > > Ok. But if CONFIG_GENERIC_IOMAP=n (ARM, aarch64, x86_64),
x86_64 has GENERIC_IOMAP=y
> ioread64_rep is then only defined if CONFIG_64BIT. As it is based > on readsq/writesq() and those must be defined (as you said), I don't get > why the *64_rep() helpers are not defined in all cases. Maybe because no > 32-bit system _should_ need them? But then compile testing gets more > difficult.
Both readsq/writesq and ioread64_rep/iowrite64_rep must be defined for 64-bit architectures and cannot be defined for 32-bit ones.
>> - For a FIFO, you cannot use readq() but have to use __raw_readq() >> to get the correct endianness. You cannot use this for an >> MMIO register with side-effects though, as this needs the byteswap >> and the barrier in readsl(). > > I'm not sure about the true definition of "FIFO" as you say. I guess > you just mean reading from a BE device? > > In this case I guess we need the barrier+byteswap helpers.
The difference is that a register has a fixed length, and gets accessed with a device specific endianness, which may have to be swapped if the device and the CPU disagree.
A FIFO register is what you use for transferring a stream of bytes, such as reading a file system block from disk. The first byte in the register corresponds to the first byte in memory later, so there must not be any byteswap while copying to/from memory. If the data itself is structured (i.e. an on-disk inode or a network packet), then the byteswap will happen if necessary while interpreting the data.
> I don't think this is actually what we want. My understanding is > (Valentin, please correct me if I'm wrong): > - on ARM: we will always use 32-bit accesses > - on aarch64: we may use either 32-bit or 64-bit accesses > - on other architectures: we only want to compile test > > I believe what Valentin wanted to achieve in the first place, was to > use 64-bit accesses when relevant (otherwise it does not work).
The width is read from a device specific register at runtime, it is not related to the architecture you are running on, presumably this is hardwired during the design of an SoC, based on the capabilities of the DMA engine:
reg = readl_relaxed(cdns_ctrl->reg + CTRL_FEATURES); if (FIELD_GET(CTRL_FEATURES_DMA_DWITH64, reg)) cdns_ctrl->caps2.data_dma_width = 8; else cdns_ctrl->caps2.data_dma_width = 4;
This usually means the largest access that is valid for reading from the FIFO, but usually smaller accesses work as well, just slower.
Arnd
| |