On 07/31/2017 10:54 PM, Alexandru Gagniuc wrote: > 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. Look at the other drivers, AFAIR the io*_rep() issue was solved over and over again, maybe you can factor that code out. > 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); > } > } -- Best regards, Marek Vasut -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html