Hi Marek, Me again! On 07/29/2017 02:34 AM, Marek Vasut wrote: > On 07/29/2017 12:07 AM, Alexandru Gagniuc wrote: >> +static void aspi_drain_fifo(struct anarion_qspi *aspi, uint8_t *buf, size_t len) >> +{ >> + uint32_t data; > > Is this stuff below something like ioread32_rep() ? > [snip] >> + aspi_write_reg(spi, ASPI_REG_BYTE_COUNT, sizeof(uint32_t)); >> + while (len >= 4) { >> + memcpy(&data, buf, sizeof(data)); >> + aspi_write_reg(spi, ASPI_REG_DATA1, data); > > iowrite32_rep ? > >> + buf += 4; >> + len -= 4; >> + } I looked at using io(read|write)32_rep in these two places, and I've run into some issues. First, I'm seeing unaligned MMIO accesses, which are not supported on ARC. Note that 'buf' has an alignment of 1, while the register requires an alignment of 4. The memcpy() in-between takes care of that, which was the original intent. Other than that, we still need to break off the tail because we need to update ASPI_REG_BYTE_COUNT before writing/reading any more data from the FIFO. We have to keep track of the remainder, so we're not really saving any SLOC. I'd like to keep the original version as I find it to be much more symmetrical and readable. Thanks, Alex >> + >> + if (len) { >> + aspi_write_reg(spi, ASPI_REG_BYTE_COUNT, len); >> + memcpy(&data, buf, len); >> + aspi_write_reg(spi, ASPI_REG_DATA1, data); >> + } >> +} Exhibit A: aspi_seed_fifo with writesl() static void aspi_seed_fifo(struct anarion_qspi *spi, const uint8_t *buf, size_t len) { uint32_t data; void __iomem *data_reg = (void *)(spi->regbase + ASPI_REG_DATA1); aspi_write_reg(spi, ASPI_REG_BYTE_COUNT, sizeof(uint32_t)); writesl(data_reg, buf, len / 4); buf += len & ~0x03; len &= 0x03; if (len) { aspi_write_reg(spi, ASPI_REG_BYTE_COUNT, len); memcpy(&data, buf, len); aspi_write_reg(spi, ASPI_REG_DATA1, data); } }