Re: [PATCH v5 1/5] spi: spi-mem: Add driver for NXP FlexSPI controller

From: Boris Brezillon
Date: Mon Dec 10 2018 - 05:41:31 EST


On Mon, 10 Dec 2018 10:35:35 +0000
Schrempf Frieder <frieder.schrempf@xxxxxxxxxx> wrote:

> >>> +
> >>> +static int nxp_fspi_exec_op(struct spi_mem *mem, const struct
> >>> +spi_mem_op *op) {
> >>> + struct nxp_fspi *f = spi_controller_get_devdata(mem->spi->master);
> >>> + int err = 0;
> >>> +
> >>> + mutex_lock(&f->lock);
> >>> +
> >>> + /* Wait for controller being ready. */
> >>> + err = fspi_readl_poll_tout(f, f->iobase + FSPI_STS0,
> >>> + FSPI_STS0_ARB_IDLE, 1, POLL_TOUT, true);
> >>> + WARN_ON(err);
> >>> +
> >>> + nxp_fspi_select_mem(f, mem->spi);
> >>> +
> >>> + nxp_fspi_prepare_lut(f, op);
> >>> + /*
> >>> + * If we have large chunks of data, we read them through the AHB bus
> >>> + * by accessing the mapped memory. In all other cases we use
> >>> + * IP commands to access the flash.
> >>> + */
> >>> + if (op->data.nbytes > (f->devtype_data->rxfifo - 4) &&
> >>> + op->data.dir == SPI_MEM_DATA_IN) {
> >>> + nxp_fspi_read_ahb(f, op);
> >>> + } else {
> >>> + if (op->data.nbytes && op->data.dir == SPI_MEM_DATA_OUT)
> >>> + nxp_fspi_fill_txfifo(f, op);
> >>> +
> >>> + err = nxp_fspi_do_op(f, op);
> >>> +
> >>> + /* Invalidate the data in the AHB buffer. */
> >>> + if (op->data.nbytes && op->data.dir == SPI_MEM_DATA_OUT)
> >>> + nxp_fspi_invalid(f);
> >>
> >> E.g. in case of an erase operation or a NAND load page operation, the
> >> invalidation is not triggered, but flash/buffer contents have changed.
> >> So I'm not sure if this is enough...
> > Ok, would change this and have invalidate for all operations.
>
> Maybe you can find out the correct way through testing with NOR and NAND.

Or just invalidate the buffer every time you're doing a read through the
AHB. This should always work.

I also think we should quickly move to a model where AHB accesses are
reserved for dirmap, and regular spi-mem op are limited to non-ahb
reads.