Hi Yogesh,

On 10.12.18 10:41, Yogesh Narayan Gaur wrote:
[...]>>> +

quoted

quoted

+static void nxp_fspi_prepare_lut(struct nxp_fspi *f,
+				 const struct spi_mem_op *op)
+{
+	void __iomem *base = f->iobase;
+	u32 lutval[4] = {};
+	int lutidx = 1, i;
+
+	/* cmd */
+	lutval[0] |= LUT_DEF(0, LUT_CMD, LUT_PAD(op->cmd.buswidth),
+			     op->cmd.opcode);
+
+	/* addr bus width */

This comment should match the code below. So maybe only "addr" should be 
enough.

quoted

quoted

+	if (op->addr.nbytes) {
+		u32 addrlen = 0;
+
+		switch (op->addr.nbytes) {
+		case 1:
+			addrlen = ADDR8BIT;
+			break;
+		case 2:
+			addrlen = ADDR16BIT;
+			break;
+		case 3:
+			addrlen = ADDR24BIT;
+			break;
+		case 4:
+			addrlen = ADDR32BIT;
+			break;
+		default:
+			dev_err(f->dev, "In-correct address length\n");
+			return;
+		}

You don't need to validate op->addr.nbytes here, this is already done in
nxp_fspi_supports_op().

Yes, I need to validate op->addr.nbytes else LUT would going to be programmed for 0 addrlen.
I have checked this on the target.

quoted

quoted

+
+		lutval[lutidx / 2] |= LUT_DEF(lutidx, LUT_ADDR,
+					      LUT_PAD(op->addr.buswidth),
+					      addrlen);

You can also just remove the whole switch statement above and use this:

lutval[lutidx / 2] |= LUT_DEF(lutidx, LUT_ADDR,
			      LUT_PAD(op->addr.buswidth),
			      op->addr.nbytes * 8);

Ok, would include in next version.

quoted

quoted

+		lutidx++;
+	}
+
+	/* dummy bytes, if needed */
+	if (op->dummy.nbytes) {
+		lutval[lutidx / 2] |= LUT_DEF(lutidx, LUT_DUMMY,
+		/*
+		 * Due to FlexSPI controller limitation number of PAD for

dummy

quoted

+		 * buswidth needs to be programmed as equal to data buswidth.
+		 */
+					      LUT_PAD(op->data.buswidth),
+					      op->dummy.nbytes * 8 /
+					      op->dummy.buswidth);
+		lutidx++;
+	}
+
+	/* read/write data bytes */
+	if (op->data.nbytes) {
+		lutval[lutidx / 2] |= LUT_DEF(lutidx,
+					      op->data.dir ==

SPI_MEM_DATA_IN ?

quoted

+					      LUT_NXP_READ : LUT_NXP_WRITE,
+					      LUT_PAD(op->data.buswidth),
+					      0);
+		lutidx++;
+	}
+
+	/* stop condition. */
+	lutval[lutidx / 2] |= LUT_DEF(lutidx, LUT_STOP, 0, 0);
+
+	/* unlock LUT */
+	fspi_writel(f, FSPI_LUTKEY_VALUE, f->iobase + FSPI_LUTKEY);
+	fspi_writel(f, FSPI_LCKER_UNLOCK, f->iobase + FSPI_LCKCR);
+
+	/* fill LUT */
+	for (i = 0; i < ARRAY_SIZE(lutval); i++)
+		fspi_writel(f, lutval[i], base + FSPI_LUT_REG(i));
+
+	dev_dbg(f->dev, "CMD[%x] lutval[0:%x \t 1:%x \t 2:%x \t 3:%x]\n",
+		op->cmd.opcode, lutval[0], lutval[1], lutval[2], lutval[3]);
+
+	/* lock LUT */
+	fspi_writel(f, FSPI_LUTKEY_VALUE, f->iobase + FSPI_LUTKEY);
+	fspi_writel(f, FSPI_LCKER_LOCK, f->iobase + FSPI_LCKCR); }

[...]

quoted

quoted

+
+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.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel