[PATCH 1/4] spi: spi-nxp-fspi: enable runtime pm for fspi

[PATCH 1/4] spi: spi-nxp-fspi: enable runtime pm for fspi

[haibo.chen]

From: Haibo Chen <haibo.chen@nxp.com>

Enable the runtime PM in fspi driver. Reading the power mode from the
debug monitor, FSPI_0 was on and with the patch it is lp.

Signed-off-by: Han Xu <han.xu@nxp.com>
Signed-off-by: Haibo Chen <haibo.chen@nxp.com>
---
 drivers/spi/spi-nxp-fspi.c | 81 +++++++++++++++++++++++++++++++-------
 1 file changed, 67 insertions(+), 14 deletions(-)

diff --git a/drivers/spi/spi-nxp-fspi.c b/drivers/spi/spi-nxp-fspi.c
index a66fa97046ee..1eecf20f1dab 100644
--- a/drivers/spi/spi-nxp-fspi.c
+++ b/drivers/spi/spi-nxp-fspi.c
@@ -48,6 +48,7 @@
 #include <linux/of.h>
 #include <linux/of_device.h>
 #include <linux/platform_device.h>
+#include <linux/pm_runtime.h>
 #include <linux/pm_qos.h>
 #include <linux/regmap.h>
 #include <linux/sizes.h>
@@ -57,6 +58,8 @@
 #include <linux/spi/spi.h>
 #include <linux/spi/spi-mem.h>
 
+/* runtime pm timeout */
+#define FSPI_RPM_TIMEOUT 50	/* 50ms */
 /*
  * The driver only uses one single LUT entry, that is updated on
  * each call of exec_op(). Index 0 is preset at boot with a basic
@@ -373,6 +376,8 @@ struct nxp_fspi {
 	struct mutex lock;
 	struct pm_qos_request pm_qos_req;
 	int selected;
+#define FSPI_INITILIZED		(1 << 0)
+	int flags;
 };
 
 static inline int needs_ip_only(struct nxp_fspi *f)
@@ -864,6 +869,12 @@ static int nxp_fspi_exec_op(struct spi_mem *mem, const struct spi_mem_op *op)
 
 	mutex_lock(&f->lock);
 
+	err = pm_runtime_get_sync(f->dev);
+	if (err < 0) {
+		dev_err(f->dev, "Failed to enable clock %d\n", __LINE__);
+		goto err_mutex;
+	}
+
 	/* Wait for controller being ready. */
 	err = fspi_readl_poll_tout(f, f->iobase + FSPI_STS0,
 				   FSPI_STS0_ARB_IDLE, 1, POLL_TOUT, true);
@@ -892,8 +903,14 @@ static int nxp_fspi_exec_op(struct spi_mem *mem, const struct spi_mem_op *op)
 	/* Invalidate the data in the AHB buffer. */
 	nxp_fspi_invalid(f);
 
+	pm_runtime_mark_last_busy(f->dev);
+	pm_runtime_put_autosuspend(f->dev);
+
 	mutex_unlock(&f->lock);
+	return err;
 
+err_mutex:
+	mutex_unlock(&f->lock);
 	return err;
 }
 
@@ -1153,12 +1170,17 @@ static int nxp_fspi_probe(struct platform_device *pdev)
 			ret = PTR_ERR(f->clk);
 			goto err_put_ctrl;
 		}
+	}
 
-		ret = nxp_fspi_clk_prep_enable(f);
-		if (ret) {
-			dev_err(dev, "can not enable the clock\n");
-			goto err_put_ctrl;
-		}
+	pm_runtime_enable(dev);
+	pm_runtime_set_autosuspend_delay(dev, FSPI_RPM_TIMEOUT);
+	pm_runtime_use_autosuspend(dev);
+
+	/* enable clock */
+	ret = pm_runtime_get_sync(f->dev);
+	if (ret < 0) {
+		dev_err(f->dev, "Failed to enable clock %d\n", __LINE__);
+		goto err_put_ctrl;
 	}
 
 	/* Clear potential interrupts */
@@ -1192,13 +1214,19 @@ static int nxp_fspi_probe(struct platform_device *pdev)
 	if (ret)
 		goto err_destroy_mutex;
 
+	pm_runtime_mark_last_busy(f->dev);
+	pm_runtime_put_autosuspend(f->dev);
+
+	/* indicate the controller has been initialized */
+	f->flags |= FSPI_INITILIZED;
+
 	return 0;
 
 err_destroy_mutex:
 	mutex_destroy(&f->lock);
 
 err_disable_clk:
-	nxp_fspi_clk_disable_unprep(f);
+	pm_runtime_disable(dev);
 
 err_put_ctrl:
 	spi_controller_put(ctlr);
@@ -1224,20 +1252,50 @@ static int nxp_fspi_remove(struct platform_device *pdev)
 	return 0;
 }
 
-static int nxp_fspi_suspend(struct device *dev)
+#ifdef CONFIG_PM
+static int nxp_fspi_initialized(struct nxp_fspi *f)
+{
+	return f->flags & FSPI_INITILIZED;
+}
+
+static int nxp_fspi_need_reinit(struct nxp_fspi *f)
+{
+	/*
+	 * we always use the controller in combination mode, so we check
+	 * this register bit to determine if the controller once lost power,
+	 * such as suspend/resume, and need to be re-init.
+	 */
+
+	return !(readl(f->iobase + FSPI_MCR0) & FSPI_MCR0_OCTCOMB_EN);
+}
+
+static int nxp_fspi_runtime_suspend(struct device *dev)
 {
+	struct nxp_fspi *f = dev_get_drvdata(dev);
+
+	nxp_fspi_clk_disable_unprep(f);
+
 	return 0;
 }
 
-static int nxp_fspi_resume(struct device *dev)
+static int nxp_fspi_runtime_resume(struct device *dev)
 {
 	struct nxp_fspi *f = dev_get_drvdata(dev);
 
-	nxp_fspi_default_setup(f);
+	nxp_fspi_clk_prep_enable(f);
+
+	if (nxp_fspi_initialized(f) && nxp_fspi_need_reinit(f))
+		nxp_fspi_default_setup(f);
 
 	return 0;
 }
 
+static const struct dev_pm_ops nxp_fspi_pm_ops = {
+	SET_RUNTIME_PM_OPS(nxp_fspi_runtime_suspend, nxp_fspi_runtime_resume, NULL)
+	SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend, pm_runtime_force_resume)
+};
+#endif	/* CONFIG_PM */
+
 static const struct of_device_id nxp_fspi_dt_ids[] = {
 	{ .compatible = "nxp,lx2160a-fspi", .data = (void *)&lx2160a_data, },
 	{ .compatible = "nxp,imx8mm-fspi", .data = (void *)&imx8mm_data, },
@@ -1256,11 +1314,6 @@ static const struct acpi_device_id nxp_fspi_acpi_ids[] = {
 MODULE_DEVICE_TABLE(acpi, nxp_fspi_acpi_ids);
 #endif
 
-static const struct dev_pm_ops nxp_fspi_pm_ops = {
-	.suspend	= nxp_fspi_suspend,
-	.resume		= nxp_fspi_resume,
-};
-
 static struct platform_driver nxp_fspi_driver = {
 	.driver = {
 		.name	= "nxp-fspi",
-- 
2.17.1

[PATCH 2/4] spi: spi-nxp-fspi: change the default lut index

[haibo.chen]

From: Haibo Chen <haibo.chen@nxp.com>

The fspi dynamic lut use the last lut for all IPS operations, the
imx8ulp only supports 15 luts, so change the last lut index from
31 to 15.

Signed-off-by: Han Xu <han.xu@nxp.com>
Signed-off-by: Haibo Chen <haibo.chen@nxp.com>
---
 drivers/spi/spi-nxp-fspi.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/spi/spi-nxp-fspi.c b/drivers/spi/spi-nxp-fspi.c
index 1eecf20f1dab..a764eb475aed 100644
--- a/drivers/spi/spi-nxp-fspi.c
+++ b/drivers/spi/spi-nxp-fspi.c
@@ -63,9 +63,9 @@
 /*
  * The driver only uses one single LUT entry, that is updated on
  * each call of exec_op(). Index 0 is preset at boot with a basic
- * read operation, so let's use the last entry (31).
+ * read operation, so let's use the last entry (15).
  */
-#define	SEQID_LUT			31
+#define	SEQID_LUT			15
 
 /* Registers used by the driver */
 #define FSPI_MCR0			0x00
-- 
2.17.1

[PATCH 3/4] spi: spi-nxp-fspi: add DDR mode support

[haibo.chen]

From: Haibo Chen <haibo.chen@nxp.com>

For LUT, add DDR command support.
Also use new API spi_mem_dtr_supports_op() to check the DTR mode.

Signed-off-by: Haibo Chen <haibo.chen@nxp.com>
---
 drivers/spi/spi-nxp-fspi.c | 25 ++++++++++++++++++++-----
 1 file changed, 20 insertions(+), 5 deletions(-)

diff --git a/drivers/spi/spi-nxp-fspi.c b/drivers/spi/spi-nxp-fspi.c
index a764eb475aed..f7acad2cbe64 100644
--- a/drivers/spi/spi-nxp-fspi.c
+++ b/drivers/spi/spi-nxp-fspi.c
@@ -486,6 +486,9 @@ static bool nxp_fspi_supports_op(struct spi_mem *mem,
 	    op->data.nbytes > f->devtype_data->txfifo)
 		return false;
 
+	if (op->cmd.dtr && op->addr.dtr && op->dummy.dtr && op->data.dtr)
+		return spi_mem_dtr_supports_op(mem, op);
+
 	return spi_mem_default_supports_op(mem, op);
 }
 
@@ -534,12 +537,22 @@ static void nxp_fspi_prepare_lut(struct nxp_fspi *f,
 	int lutidx = 1, i;
 
 	/* cmd */
-	lutval[0] |= LUT_DEF(0, LUT_CMD, LUT_PAD(op->cmd.buswidth),
-			     op->cmd.opcode);
+	if (op->cmd.dtr) {
+		lutval[0] |= LUT_DEF(0, LUT_CMD_DDR, LUT_PAD(op->cmd.buswidth),
+				     op->cmd.opcode >> 8);
+		lutval[lutidx / 2] |= LUT_DEF(lutidx, LUT_CMD_DDR,
+					      LUT_PAD(op->cmd.buswidth),
+					      op->cmd.opcode & 0x00ff);
+		lutidx++;
+	} else {
+		lutval[0] |= LUT_DEF(0, LUT_CMD, LUT_PAD(op->cmd.buswidth),
+				     op->cmd.opcode);
+	}
 
 	/* addr bytes */
 	if (op->addr.nbytes) {
-		lutval[lutidx / 2] |= LUT_DEF(lutidx, LUT_ADDR,
+		lutval[lutidx / 2] |= LUT_DEF(lutidx, op->addr.dtr ?
+					      LUT_ADDR_DDR : LUT_ADDR,
 					      LUT_PAD(op->addr.buswidth),
 					      op->addr.nbytes * 8);
 		lutidx++;
@@ -547,7 +560,8 @@ static void nxp_fspi_prepare_lut(struct nxp_fspi *f,
 
 	/* dummy bytes, if needed */
 	if (op->dummy.nbytes) {
-		lutval[lutidx / 2] |= LUT_DEF(lutidx, LUT_DUMMY,
+		lutval[lutidx / 2] |= LUT_DEF(lutidx, op->dummy.dtr ?
+					      LUT_DUMMY_DDR : LUT_DUMMY,
 		/*
 		 * Due to FlexSPI controller limitation number of PAD for dummy
 		 * buswidth needs to be programmed as equal to data buswidth.
@@ -562,7 +576,8 @@ static void nxp_fspi_prepare_lut(struct nxp_fspi *f,
 	if (op->data.nbytes) {
 		lutval[lutidx / 2] |= LUT_DEF(lutidx,
 					      op->data.dir == SPI_MEM_DATA_IN ?
-					      LUT_NXP_READ : LUT_NXP_WRITE,
+					      (op->data.dtr ? LUT_READ_DDR : LUT_NXP_READ) :
+					      (op->data.dtr ? LUT_WRITE_DDR : LUT_NXP_WRITE),
 					      LUT_PAD(op->data.buswidth),
 					      0);
 		lutidx++;
-- 
2.17.1

[PATCH 4/4] spi: spi-nxp-fspi: add function to select sample clock source for flash reading

[haibo.chen]

From: Haibo Chen <haibo.chen@nxp.com>

fspi define four mode for sample clock source selection.

Here is the list of modes:
mode 0: Dummy Read strobe generated by FlexSPI Controller and loopback internally
mode 1: Dummy Read strobe generated by FlexSPI Controller and loopback from DQS pad
mode 2: Reserved
mode 3: Flash provided Read strobe and input from DQS pad

In default, fspi use mode 0 after reset.
For 8-8-8-DTR mode, need to use mode 3, otherwise 8-8-8-DTR read always
get incorrect data.

Signed-off-by: Haibo Chen <haibo.chen@nxp.com>
---
 drivers/spi/spi-nxp-fspi.c | 46 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 46 insertions(+)

diff --git a/drivers/spi/spi-nxp-fspi.c b/drivers/spi/spi-nxp-fspi.c
index f7acad2cbe64..309685a6d493 100644
--- a/drivers/spi/spi-nxp-fspi.c
+++ b/drivers/spi/spi-nxp-fspi.c
@@ -377,6 +377,7 @@ struct nxp_fspi {
 	struct pm_qos_request pm_qos_req;
 	int selected;
 #define FSPI_INITILIZED		(1 << 0)
+#define FSPI_RXCLKSRC_3		(1 << 1)
 	int flags;
 };
 
@@ -877,6 +878,49 @@ static int nxp_fspi_do_op(struct nxp_fspi *f, const struct spi_mem_op *op)
 	return err;
 }
 
+/*
+ * Sample Clock source selection for Flash Reading
+ * Four modes defined by fspi:
+ * mode 0: Dummy Read strobe generated by FlexSPI Controller
+ *         and loopback internally
+ * mode 1: Dummy Read strobe generated by FlexSPI Controller
+ *         and loopback from DQS pad
+ * mode 2: Reserved
+ * mode 3: Flash provided Read strobe and input from DQS pad
+ *
+ * fspi default use mode 0 after reset
+ */
+static void nxp_fspi_select_rx_sample_clk_source(struct nxp_fspi *f)
+{
+	u32 reg;
+
+	/*
+	 * For 8-8-8-DTR mode, need to use mode 3 (Flash provided Read
+	 * strobe and input from DQS pad), otherwise read operaton may
+	 * meet issue.
+	 * This mode require flash device connect the DQS pad on board.
+	 * For other modes, still use mode 0, keep align with before.
+	 * spi_nor_suspend will disable 8-8-8-DTR mode, also need to
+	 * change the mode back to mode 0.
+	 */
+	if (!(f->flags & FSPI_RXCLKSRC_3) &&
+			op->cmd.dtr && op->addr.dtr &&
+			op->dummy.dtr && op->data.dtr) {
+		reg = fspi_readl(f, f->iobase + FSPI_MCR0);
+		reg |= FSPI_MCR0_RXCLKSRC(3);
+		fspi_writel(f, reg, f->iobase + FSPI_MCR0);
+		f->flags |= FSPI_RXCLKSRC_3;
+	} else if ((f->flags & FSPI_RXCLKSRC_3) &&
+			!op->cmd.dtr && !op->addr.dtr &&
+			!op->dummy.dtr && !op->data.dtr) {
+		reg = fspi_readl(f, f->iobase + FSPI_MCR0);
+		reg &= ~FSPI_MCR0_RXCLKSRC(3);	/* select mode 0 */
+		fspi_writel(f, reg, f->iobase + FSPI_MCR0);
+		f->flags &= ~FSPI_RXCLKSRC_3;
+	}
+
+}
+
 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);
@@ -897,6 +941,8 @@ static int nxp_fspi_exec_op(struct spi_mem *mem, const struct spi_mem_op *op)
 
 	nxp_fspi_select_mem(f, mem->spi);
 
+	nxp_fspi_select_rx_sample_clk_source(f);
+
 	nxp_fspi_prepare_lut(f, op);
 	/*
 	 * If we have large chunks of data, we read them through the AHB bus by
-- 
2.17.1

RE: [EXT] [PATCH 2/4] spi: spi-nxp-fspi: change the default lut index

[Kuldeep Singh]

Hi Haibo,

> -----Original Message-----
> From: haibo.chen@nxp.com <haibo.chen@nxp.com>
> Sent: Friday, August 20, 2021 1:14 PM
> To: Ashish Kumar <ashish.kumar@nxp.com>; yogeshgaur.83@gmail.com;
> broonie@kernel.org
> Cc: linux-spi@vger.kernel.org; dl-linux-imx <linux-imx@nxp.com>; Bough Chen
> <haibo.chen@nxp.com>
> Subject: [EXT] [PATCH 2/4] spi: spi-nxp-fspi: change the default lut index
> 
> Caution: EXT Email
> 
> From: Haibo Chen <haibo.chen@nxp.com>
> 
> The fspi dynamic lut use the last lut for all IPS operations, the imx8ulp only
> supports 15 luts, so change the last lut index from
> 31 to 15.
> 
> Signed-off-by: Han Xu <han.xu@nxp.com>
> Signed-off-by: Haibo Chen <haibo.chen@nxp.com>
> ---
>  drivers/spi/spi-nxp-fspi.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/spi/spi-nxp-fspi.c b/drivers/spi/spi-nxp-fspi.c index
> 1eecf20f1dab..a764eb475aed 100644
> --- a/drivers/spi/spi-nxp-fspi.c
> +++ b/drivers/spi/spi-nxp-fspi.c
> @@ -63,9 +63,9 @@
>  /*
>   * The driver only uses one single LUT entry, that is updated on
>   * each call of exec_op(). Index 0 is preset at boot with a basic
> - * read operation, so let's use the last entry (31).
> + * read operation, so let's use the last entry (15).

One nitpick:
Last LUT entry is still 31 for all platforms except imx8ulp which has 15.
QSPI driver uses last lut entry 15 whereas 15 will be middle entry for flexspi(in general).

The word 'last' in current comment does not seem appropriate for all cases.
Maybe you can update comment for ex:
"Index 0 is present at boot with a basic read operation and last lut entry is 31.
Some platforms like imx8ulp supports 16 lut entries and therefore, use lut index entry 15."

Other than that,
Reviewed-by: Kuldeep Singh <kuldeep.singh@nxp.com>

Regards
Kuldeep

RE: [EXT] [PATCH 2/4] spi: spi-nxp-fspi: change the default lut index

[Bough Chen]

[-- Attachment #1: Type: text/plain, Size: 2463 bytes --]

> -----Original Message-----
> From: Kuldeep Singh
> Sent: 2021年8月20日 16:49
> To: Bough Chen <haibo.chen@nxp.com>; Ashish Kumar
> <ashish.kumar@nxp.com>; yogeshgaur.83@gmail.com; broonie@kernel.org
> Cc: linux-spi@vger.kernel.org; dl-linux-imx <linux-imx@nxp.com>; Bough
Chen
> <haibo.chen@nxp.com>
> Subject: RE: [EXT] [PATCH 2/4] spi: spi-nxp-fspi: change the default lut
index
> 
> Hi Haibo,
> 
> > -----Original Message-----
> > From: haibo.chen@nxp.com <haibo.chen@nxp.com>
> > Sent: Friday, August 20, 2021 1:14 PM
> > To: Ashish Kumar <ashish.kumar@nxp.com>; yogeshgaur.83@gmail.com;
> > broonie@kernel.org
> > Cc: linux-spi@vger.kernel.org; dl-linux-imx <linux-imx@nxp.com>; Bough
> > Chen <haibo.chen@nxp.com>
> > Subject: [EXT] [PATCH 2/4] spi: spi-nxp-fspi: change the default lut
> > index
> >
> > Caution: EXT Email
> >
> > From: Haibo Chen <haibo.chen@nxp.com>
> >
> > The fspi dynamic lut use the last lut for all IPS operations, the
> > imx8ulp only supports 15 luts, so change the last lut index from
> > 31 to 15.
> >
> > Signed-off-by: Han Xu <han.xu@nxp.com>
> > Signed-off-by: Haibo Chen <haibo.chen@nxp.com>
> > ---
> >  drivers/spi/spi-nxp-fspi.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/spi/spi-nxp-fspi.c b/drivers/spi/spi-nxp-fspi.c
> > index 1eecf20f1dab..a764eb475aed 100644
> > --- a/drivers/spi/spi-nxp-fspi.c
> > +++ b/drivers/spi/spi-nxp-fspi.c
> > @@ -63,9 +63,9 @@
> >  /*
> >   * The driver only uses one single LUT entry, that is updated on
> >   * each call of exec_op(). Index 0 is preset at boot with a basic
> > - * read operation, so let's use the last entry (31).
> > + * read operation, so let's use the last entry (15).
> 
> One nitpick:
> Last LUT entry is still 31 for all platforms except imx8ulp which has 15.
> QSPI driver uses last lut entry 15 whereas 15 will be middle entry for
flexspi(in
> general).
> 
> The word 'last' in current comment does not seem appropriate for all
cases.
> Maybe you can update comment for ex:
> "Index 0 is present at boot with a basic read operation and last lut entry
is 31.
> Some platforms like imx8ulp supports 16 lut entries and therefore, use lut
index
> entry 15."
> 
> Other than that,
> Reviewed-by: Kuldeep Singh <kuldeep.singh@nxp.com>

Hi Kuldeep,

Thanks for your suggestion. I will do that.

Best Regards
Haibo Chen
> 
> Regards
> Kuldeep

[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 9571 bytes --]

RE: [EXT] [PATCH 3/4] spi: spi-nxp-fspi: add DDR mode support

[Kuldeep Singh]

Hi Haibo,

> -----Original Message-----
> From: haibo.chen@nxp.com <haibo.chen@nxp.com>
> Sent: Friday, August 20, 2021 1:14 PM
> To: Ashish Kumar <ashish.kumar@nxp.com>; yogeshgaur.83@gmail.com;
> broonie@kernel.org
> Cc: linux-spi@vger.kernel.org; dl-linux-imx <linux-imx@nxp.com>; Bough Chen
> <haibo.chen@nxp.com>
> Subject: [EXT] [PATCH 3/4] spi: spi-nxp-fspi: add DDR mode support
> 
> Caution: EXT Email
> 
> From: Haibo Chen <haibo.chen@nxp.com>
> 
> For LUT, add DDR command support.
> Also use new API spi_mem_dtr_supports_op() to check the DTR mode.
> 
> Signed-off-by: Haibo Chen <haibo.chen@nxp.com>

Thanks for submitting patch to support octal DTR.

> ---
>  drivers/spi/spi-nxp-fspi.c | 25 ++++++++++++++++++++-----
>  1 file changed, 20 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/spi/spi-nxp-fspi.c b/drivers/spi/spi-nxp-fspi.c index
> a764eb475aed..f7acad2cbe64 100644
> --- a/drivers/spi/spi-nxp-fspi.c
> +++ b/drivers/spi/spi-nxp-fspi.c
> @@ -486,6 +486,9 @@ static bool nxp_fspi_supports_op(struct spi_mem
> *mem,
>             op->data.nbytes > f->devtype_data->txfifo)
>                 return false;
> 
> +       if (op->cmd.dtr && op->addr.dtr && op->dummy.dtr && op->data.dtr)
> +               return spi_mem_dtr_supports_op(mem, op);
> +
[snip]

Now that spi-nor framework and flexspi driver supports octal DTR, if device-tree specifies RX and TX bus-width as <8,8> ,
then above change will always make sure to select DTR mode and SDR will never get selected.
Please note, Layerscape LX2160A and LX2162A are two platforms which have micron mt35xu512aba (support octal dtr) 
and are causing probe failure with these patches as required dependencies for enabling DTR are not met.

Since framework selects maximum supported capability, I think there should be a mechanism to choose SDR or DTR mode in driver itself.
This will also help for a platform to fallback from DTR to SDR in case DTR doesn't work.
We can enable this feature as a quirk or by reading a property from device-tree.
Please let me know your views.

Regards
Kuldeep

RE: [EXT] [PATCH 3/4] spi: spi-nxp-fspi: add DDR mode support

[Bough Chen]

[-- Attachment #1: Type: text/plain, Size: 3022 bytes --]

> -----Original Message-----
> From: Kuldeep Singh
> Sent: 2021年8月31日 18:42
> To: Bough Chen <haibo.chen@nxp.com>
> Cc: linux-spi@vger.kernel.org; dl-linux-imx <linux-imx@nxp.com>; Bough
Chen
> <haibo.chen@nxp.com>; Rajesh Bhagat <rajesh.bhagat@nxp.com>;
> broonie@kernel.org
> Subject: RE: [EXT] [PATCH 3/4] spi: spi-nxp-fspi: add DDR mode support
> 
> Hi Haibo,
> 
> > -----Original Message-----
> > From: haibo.chen@nxp.com <haibo.chen@nxp.com>
> > Sent: Friday, August 20, 2021 1:14 PM
> > To: Ashish Kumar <ashish.kumar@nxp.com>; yogeshgaur.83@gmail.com;
> > broonie@kernel.org
> > Cc: linux-spi@vger.kernel.org; dl-linux-imx <linux-imx@nxp.com>; Bough
> > Chen <haibo.chen@nxp.com>
> > Subject: [EXT] [PATCH 3/4] spi: spi-nxp-fspi: add DDR mode support
> >
> > Caution: EXT Email
> >
> > From: Haibo Chen <haibo.chen@nxp.com>
> >
> > For LUT, add DDR command support.
> > Also use new API spi_mem_dtr_supports_op() to check the DTR mode.
> >
> > Signed-off-by: Haibo Chen <haibo.chen@nxp.com>
> 
> Thanks for submitting patch to support octal DTR.
> 
> > ---
> >  drivers/spi/spi-nxp-fspi.c | 25 ++++++++++++++++++++-----
> >  1 file changed, 20 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/spi/spi-nxp-fspi.c b/drivers/spi/spi-nxp-fspi.c
> > index
> > a764eb475aed..f7acad2cbe64 100644
> > --- a/drivers/spi/spi-nxp-fspi.c
> > +++ b/drivers/spi/spi-nxp-fspi.c
> > @@ -486,6 +486,9 @@ static bool nxp_fspi_supports_op(struct spi_mem
> > *mem,
> >             op->data.nbytes > f->devtype_data->txfifo)
> >                 return false;
> >
> > +       if (op->cmd.dtr && op->addr.dtr && op->dummy.dtr &&
> op->data.dtr)
> > +               return spi_mem_dtr_supports_op(mem, op);
> > +
> [snip]
> 
> Now that spi-nor framework and flexspi driver supports octal DTR, if
> device-tree specifies RX and TX bus-width as <8,8> , then above change
will
> always make sure to select DTR mode and SDR will never get selected.
> Please note, Layerscape LX2160A and LX2162A are two platforms which have
> micron mt35xu512aba (support octal dtr) and are causing probe failure with
> these patches as required dependencies for enabling DTR are not met.
> 
> Since framework selects maximum supported capability, I think there should
be
> a mechanism to choose SDR or DTR mode in driver itself.
> This will also help for a platform to fallback from DTR to SDR in case DTR
> doesn't work.
> We can enable this feature as a quirk or by reading a property from
device-tree.
> Please let me know your views.

Thanks for your comments!
The flexspi controller in Layerscape support 8bit DTR, but for flexspi, if
support 8bit DTR, need to use mode3, which means we must connect the DQS pad
on board. 
Seems LX216x board do not connect DQS line.
We should take this into consideration, this is board limitation, better to
involve a property in device-tree.
I will send a V2 patch soon.

Best Regards
Haibo Chen

> 
> Regards
> Kuldeep

[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 9571 bytes --]

RE: [EXT] [PATCH 3/4] spi: spi-nxp-fspi: add DDR mode support

[Kuldeep Singh]

> -----Original Message-----
> From: Bough Chen <haibo.chen@nxp.com>
> Sent: Wednesday, September 1, 2021 1:35 PM
> To: Kuldeep Singh <kuldeep.singh@nxp.com>
> Cc: linux-spi@vger.kernel.org; dl-linux-imx <linux-imx@nxp.com>; Rajesh
> Bhagat <rajesh.bhagat@nxp.com>; broonie@kernel.org
> Subject: RE: [EXT] [PATCH 3/4] spi: spi-nxp-fspi: add DDR mode support
> 
> > -----Original Message-----
> > From: Kuldeep Singh
> > Sent: 2021年8月31日 18:42
> > To: Bough Chen <haibo.chen@nxp.com>
> > Cc: linux-spi@vger.kernel.org; dl-linux-imx <linux-imx@nxp.com>; Bough
> Chen
> > <haibo.chen@nxp.com>; Rajesh Bhagat <rajesh.bhagat@nxp.com>;
> > broonie@kernel.org
> > Subject: RE: [EXT] [PATCH 3/4] spi: spi-nxp-fspi: add DDR mode support
> >
> > Hi Haibo,
> >
> > > -----Original Message-----
> > > From: haibo.chen@nxp.com <haibo.chen@nxp.com>
> > > Sent: Friday, August 20, 2021 1:14 PM
> > > To: Ashish Kumar <ashish.kumar@nxp.com>; yogeshgaur.83@gmail.com;
> > > broonie@kernel.org
> > > Cc: linux-spi@vger.kernel.org; dl-linux-imx <linux-imx@nxp.com>; Bough
> > > Chen <haibo.chen@nxp.com>
> > > Subject: [EXT] [PATCH 3/4] spi: spi-nxp-fspi: add DDR mode support
> > >
> > > Caution: EXT Email
> > >
> > > From: Haibo Chen <haibo.chen@nxp.com>
> > >
> > > For LUT, add DDR command support.
> > > Also use new API spi_mem_dtr_supports_op() to check the DTR mode.
> > >
> > > Signed-off-by: Haibo Chen <haibo.chen@nxp.com>
> >
> > Thanks for submitting patch to support octal DTR.
> >
> > > ---
> > >  drivers/spi/spi-nxp-fspi.c | 25 ++++++++++++++++++++-----
> > >  1 file changed, 20 insertions(+), 5 deletions(-)
> > >
> > > diff --git a/drivers/spi/spi-nxp-fspi.c b/drivers/spi/spi-nxp-fspi.c
> > > index
> > > a764eb475aed..f7acad2cbe64 100644
> > > --- a/drivers/spi/spi-nxp-fspi.c
> > > +++ b/drivers/spi/spi-nxp-fspi.c
> > > @@ -486,6 +486,9 @@ static bool nxp_fspi_supports_op(struct spi_mem
> > > *mem,
> > >             op->data.nbytes > f->devtype_data->txfifo)
> > >                 return false;
> > >
> > > +       if (op->cmd.dtr && op->addr.dtr && op->dummy.dtr &&
> > op->data.dtr)
> > > +               return spi_mem_dtr_supports_op(mem, op);
> > > +
> > [snip]
> >
> > Now that spi-nor framework and flexspi driver supports octal DTR, if
> > device-tree specifies RX and TX bus-width as <8,8> , then above change
> will
> > always make sure to select DTR mode and SDR will never get selected.
> > Please note, Layerscape LX2160A and LX2162A are two platforms which have
> > micron mt35xu512aba (support octal dtr) and are causing probe failure with
> > these patches as required dependencies for enabling DTR are not met.
> >
> > Since framework selects maximum supported capability, I think there should
> be
> > a mechanism to choose SDR or DTR mode in driver itself.
> > This will also help for a platform to fallback from DTR to SDR in case DTR
> > doesn't work.
> > We can enable this feature as a quirk or by reading a property from
> device-tree.
> > Please let me know your views.
> 
> Thanks for your comments!
> The flexspi controller in Layerscape support 8bit DTR, but for flexspi, if
> support 8bit DTR, need to use mode3, which means we must connect the DQS
> pad
> on board.
> Seems LX216x board do not connect DQS line.
> We should take this into consideration, this is board limitation, better to
> involve a property in device-tree.
> I will send a V2 patch soon.

Thanks for your understanding.
I will make sure to confirm NXP layerscape devices for next version of series.

Regards
Kuldeep