[PATCH 01/11] spi: spi-nxp-fspi: enable runtime pm for fspi

[PATCH 01/11] 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.
Also for system PM, On some board like i.MX8ULP-EVK board,
after system suspend, IOMUX module will lost power, so all
the pinctrl setting will lost when system resume back, need
driver to save/restore the pinctrl setting.

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

diff --git a/drivers/spi/spi-nxp-fspi.c b/drivers/spi/spi-nxp-fspi.c
index 2b0301fc971c..b2cd8e06f374 100644
--- a/drivers/spi/spi-nxp-fspi.c
+++ b/drivers/spi/spi-nxp-fspi.c
@@ -49,6 +49,8 @@
 #include <linux/of.h>
 #include <linux/of_device.h>
 #include <linux/platform_device.h>
+#include <linux/pinctrl/consumer.h>
+#include <linux/pm_runtime.h>
 #include <linux/pm_qos.h>
 #include <linux/regmap.h>
 #include <linux/sizes.h>
@@ -58,6 +60,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
@@ -375,6 +379,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)
@@ -866,6 +872,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);
@@ -894,8 +906,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;
 }
 
@@ -1141,12 +1159,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 */
@@ -1180,13 +1203,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);
@@ -1212,20 +1241,79 @@ 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 int nxp_fspi_suspend(struct device *dev)
+{
+	int ret;
+
+	ret = pinctrl_pm_select_sleep_state(dev);
+	if (ret) {
+		dev_err(dev, "select flexspi sleep pinctrl failed!\n");
+		return ret;
+	}
+
+	return pm_runtime_force_suspend(dev);
+}
+
+static int nxp_fspi_resume(struct device *dev)
+{
+	int ret;
+
+	ret = pm_runtime_force_resume(dev);
+	if (ret)
+		return ret;
+
+	ret = pinctrl_pm_select_default_state(dev);
+	if (ret)
+		dev_err(dev, "select flexspi default pinctrl failed!\n");
+
+	return ret;
+}
+
+
+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(nxp_fspi_suspend, nxp_fspi_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, },
@@ -1244,11 +1332,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.25.1


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

[PATCH 02/11] 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 b2cd8e06f374..10e71e55d21a 100644
--- a/drivers/spi/spi-nxp-fspi.c
+++ b/drivers/spi/spi-nxp-fspi.c
@@ -65,9 +65,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.25.1


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

[PATCH 03/11] spi: spi-nxp-fspi: add DTR mode support

[haibo.chen]

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

For LUT, add DTR command support.

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

diff --git a/drivers/spi/spi-nxp-fspi.c b/drivers/spi/spi-nxp-fspi.c
index 10e71e55d21a..c32a4f53fa2a 100644
--- a/drivers/spi/spi-nxp-fspi.c
+++ b/drivers/spi/spi-nxp-fspi.c
@@ -537,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++;
@@ -550,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.
@@ -565,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++;
@@ -1090,6 +1102,10 @@ static const struct spi_controller_mem_ops nxp_fspi_mem_ops = {
 	.get_name = nxp_fspi_get_name,
 };
 
+static struct spi_controller_mem_caps nxp_fspi_mem_caps = {
+	.dtr = true,
+};
+
 static int nxp_fspi_probe(struct platform_device *pdev)
 {
 	struct spi_controller *ctlr;
@@ -1194,6 +1210,7 @@ static int nxp_fspi_probe(struct platform_device *pdev)
 	ctlr->bus_num = -1;
 	ctlr->num_chipselect = NXP_FSPI_MAX_CHIPSELECT;
 	ctlr->mem_ops = &nxp_fspi_mem_ops;
+	ctlr->mem_caps = &nxp_fspi_mem_caps;
 
 	nxp_fspi_default_setup(f);
 
-- 
2.25.1


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

[PATCH 04/11] 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 | 47 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 47 insertions(+)

diff --git a/drivers/spi/spi-nxp-fspi.c b/drivers/spi/spi-nxp-fspi.c
index c32a4f53fa2a..34679dc0e1ad 100644
--- a/drivers/spi/spi-nxp-fspi.c
+++ b/drivers/spi/spi-nxp-fspi.c
@@ -380,6 +380,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,50 @@ 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,
+						 const struct spi_mem_op *op)
+{
+	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 +942,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, op);
+
 	nxp_fspi_prepare_lut(f, op);
 	/*
 	 * If we have large chunks of data, we read them through the AHB bus by
-- 
2.25.1


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

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

[Michael Walle]

Am 2022-07-05 11:11, schrieb haibo.chen@nxp.com:
> 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 | 47 ++++++++++++++++++++++++++++++++++++++
>  1 file changed, 47 insertions(+)
> 
> diff --git a/drivers/spi/spi-nxp-fspi.c b/drivers/spi/spi-nxp-fspi.c
> index c32a4f53fa2a..34679dc0e1ad 100644
> --- a/drivers/spi/spi-nxp-fspi.c
> +++ b/drivers/spi/spi-nxp-fspi.c
> @@ -380,6 +380,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,50 @@ 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,
> +						 const struct spi_mem_op *op)
> +{
> +	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;
> +	}

How is this supposed to work? Are you unconditionally enable
flash provided read strobes if DTR is used? What if the
flash doesn't provide one or the board haven't DQS connected?

-michael

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

[PATCH 05/11] spi: spi-nxp-fspi: Add quirk to disable DTR support

[haibo.chen]

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

Not all platform currently supports octal DTR mode. lx2160a do not
implement DQS, this causes flash probe failure and therefore, provide
an option of quirk FSPI_QUIRK_DISABLE_DTR for platforms not support
DTR mode.

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

diff --git a/drivers/spi/spi-nxp-fspi.c b/drivers/spi/spi-nxp-fspi.c
index 34679dc0e1ad..61cf1b82c0d7 100644
--- a/drivers/spi/spi-nxp-fspi.c
+++ b/drivers/spi/spi-nxp-fspi.c
@@ -325,6 +325,9 @@
 /* Access flash memory using IP bus only */
 #define FSPI_QUIRK_USE_IP_ONLY	BIT(0)
 
+/* Disable DTR */
+#define FSPI_QUIRK_DISABLE_DTR	BIT(1)
+
 struct nxp_fspi_devtype_data {
 	unsigned int rxfifo;
 	unsigned int txfifo;
@@ -337,7 +340,7 @@ static struct nxp_fspi_devtype_data lx2160a_data = {
 	.rxfifo = SZ_512,       /* (64  * 64 bits)  */
 	.txfifo = SZ_1K,        /* (128 * 64 bits)  */
 	.ahb_buf_size = SZ_2K,  /* (256 * 64 bits)  */
-	.quirks = 0,
+	.quirks = FSPI_QUIRK_DISABLE_DTR,
 	.little_endian = true,  /* little-endian    */
 };
 
@@ -1149,10 +1152,14 @@ static const struct spi_controller_mem_ops nxp_fspi_mem_ops = {
 	.get_name = nxp_fspi_get_name,
 };
 
-static struct spi_controller_mem_caps nxp_fspi_mem_caps = {
+static const struct spi_controller_mem_caps nxp_fspi_mem_caps = {
 	.dtr = true,
 };
 
+static const struct spi_controller_mem_caps nxp_fspi_mem_caps_quirks = {
+	.dtr = false,
+};
+
 static int nxp_fspi_probe(struct platform_device *pdev)
 {
 	struct spi_controller *ctlr;
@@ -1257,7 +1264,10 @@ static int nxp_fspi_probe(struct platform_device *pdev)
 	ctlr->bus_num = -1;
 	ctlr->num_chipselect = NXP_FSPI_MAX_CHIPSELECT;
 	ctlr->mem_ops = &nxp_fspi_mem_ops;
-	ctlr->mem_caps = &nxp_fspi_mem_caps;
+	if (f->devtype_data->quirks & FSPI_QUIRK_DISABLE_DTR)
+		ctlr->mem_caps = &nxp_fspi_mem_caps_quirks;
+	else
+		ctlr->mem_caps = &nxp_fspi_mem_caps;
 
 	nxp_fspi_default_setup(f);
 
-- 
2.25.1


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH 05/11] spi: spi-nxp-fspi: Add quirk to disable DTR support

[Michael Walle]

Am 2022-07-05 11:11, schrieb haibo.chen@nxp.com:
> From: Haibo Chen <haibo.chen@nxp.com>
> 
> Not all platform currently supports octal DTR mode. lx2160a do not
> implement DQS, this causes flash probe failure and therefore, provide
> an option of quirk FSPI_QUIRK_DISABLE_DTR for platforms not support
> DTR mode.

You write "DQS is not supported" but your quirk targets DTR. DTR works
without DQS. DQS is needed for faster frequencies, no? So the quirk
should be named accordingly.

Also, this compatible is (unfortunately!) also used on for the LS1028A
SoC and as far as I know DQS is supported there. I'm not sure what to
do here. Maybe add a new compatible "nxp,ls1028a-fspi" and change the
device tree to
compatible = "nxp,ls1028a-fspi", "nxp,lx2160a";

-michael

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

[PATCH 06/11] spi: spi-nxp-fspi: enable octal ddr for iMX8QM/QXP/DXL

[haibo.chen]

From: Han Xu <han.xu@nxp.com>

To enable the octal ddr for iMX8QM/QXP/DXL, need to set the proper
slave line delay to get the correct sample points. So add one entry
in dts to set it.

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

diff --git a/drivers/spi/spi-nxp-fspi.c b/drivers/spi/spi-nxp-fspi.c
index 61cf1b82c0d7..cfc2600e3b6d 100644
--- a/drivers/spi/spi-nxp-fspi.c
+++ b/drivers/spi/spi-nxp-fspi.c
@@ -218,9 +218,13 @@
 
 #define FSPI_DLLACR			0xC0
 #define FSPI_DLLACR_OVRDEN		BIT(8)
+#define FSPI_DLLACR_SLVDLY(x)          ((x) << 3)
+#define FSPI_DLLACR_DLLEN              BIT(0)
 
 #define FSPI_DLLBCR			0xC4
 #define FSPI_DLLBCR_OVRDEN		BIT(8)
+#define FSPI_DLLBCR_SLVDLY(x)          ((x) << 3)
+#define FSPI_DLLBCR_DLLEN              BIT(0)
 
 #define FSPI_STS0			0xE0
 #define FSPI_STS0_DLPHB(x)		((x) << 8)
@@ -375,6 +379,7 @@ struct nxp_fspi {
 	u32 memmap_phy_size;
 	u32 memmap_start;
 	u32 memmap_len;
+	u32 dll_slvdly;
 	struct clk *clk, *clk_en;
 	struct device *dev;
 	struct completion c;
@@ -1081,6 +1086,13 @@ static int nxp_fspi_default_setup(struct nxp_fspi *f)
 	fspi_writel(f, FSPI_DLLACR_OVRDEN, base + FSPI_DLLACR);
 	fspi_writel(f, FSPI_DLLBCR_OVRDEN, base + FSPI_DLLBCR);
 
+	if (f->dll_slvdly) {
+		fspi_writel(f, FSPI_DLLACR_DLLEN | FSPI_DLLACR_SLVDLY(f->dll_slvdly),
+			    base + FSPI_DLLACR);
+		fspi_writel(f, FSPI_DLLBCR_DLLEN | FSPI_DLLBCR_SLVDLY(f->dll_slvdly),
+			    base + FSPI_DLLBCR);
+	}
+
 	/* enable module */
 	fspi_writel(f, FSPI_MCR0_AHB_TIMEOUT(0xFF) |
 		    FSPI_MCR0_IP_TIMEOUT(0xFF) | (u32) FSPI_MCR0_OCTCOMB_EN,
@@ -1259,6 +1271,9 @@ static int nxp_fspi_probe(struct platform_device *pdev)
 		goto err_disable_clk;
 	}
 
+	/* check if need to set the slave delay line */
+	of_property_read_u32(np, "nxp,fspi-dll-slvdly", &f->dll_slvdly);
+
 	mutex_init(&f->lock);
 
 	ctlr->bus_num = -1;
-- 
2.25.1


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

[PATCH 07/11] dt-bindings: spi: spi-nxp-fspi: add a new property nxp,fspi-dll-slvdly

[haibo.chen]

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

Add one optional property nxp,fspi-dll-slvdly

Signed-off-by: Haibo Chen <haibo.chen@nxp.com>
---
 Documentation/devicetree/bindings/spi/spi-nxp-fspi.yaml | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/Documentation/devicetree/bindings/spi/spi-nxp-fspi.yaml b/Documentation/devicetree/bindings/spi/spi-nxp-fspi.yaml
index 1b552c298277..6bd61565686a 100644
--- a/Documentation/devicetree/bindings/spi/spi-nxp-fspi.yaml
+++ b/Documentation/devicetree/bindings/spi/spi-nxp-fspi.yaml
@@ -45,6 +45,12 @@ properties:
       - const: fspi_en
       - const: fspi
 
+  nxp,fspi-dll-slvdly:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description: |
+      Specify the DLL slave line delay value.
+    default: 0
+
 required:
   - compatible
   - reg
-- 
2.25.1


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH 07/11] dt-bindings: spi: spi-nxp-fspi: add a new property nxp,fspi-dll-slvdly

[Krzysztof Kozlowski]

On 05/07/2022 11:11, haibo.chen@nxp.com wrote:
> From: Haibo Chen <haibo.chen@nxp.com>
> 
> Add one optional property nxp,fspi-dll-slvdly
> 
> Signed-off-by: Haibo Chen <haibo.chen@nxp.com>
> ---
>  Documentation/devicetree/bindings/spi/spi-nxp-fspi.yaml | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/spi/spi-nxp-fspi.yaml b/Documentation/devicetree/bindings/spi/spi-nxp-fspi.yaml
> index 1b552c298277..6bd61565686a 100644
> --- a/Documentation/devicetree/bindings/spi/spi-nxp-fspi.yaml
> +++ b/Documentation/devicetree/bindings/spi/spi-nxp-fspi.yaml
> @@ -45,6 +45,12 @@ properties:
>        - const: fspi_en
>        - const: fspi
>  
> +  nxp,fspi-dll-slvdly:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description: |
> +      Specify the DLL slave line delay value.

What are the units?

Best regards,
Krzysztof

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

RE: [PATCH 07/11] dt-bindings: spi: spi-nxp-fspi: add a new property nxp,fspi-dll-slvdly

[Bough Chen]

> -----Original Message-----
> From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> Sent: 2022年7月5日 17:48
> To: Bough Chen <haibo.chen@nxp.com>; ashish.kumar@nxp.com;
> yogeshgaur.83@gmail.com; broonie@kernel.org; robh+dt@kernel.org;
> krzysztof.kozlowski+dt@linaro.org; Han Xu <han.xu@nxp.com>;
> singh.kuldeep87k@gmail.com; tudor.ambarus@microchip.com;
> p.yadav@ti.com; michael@walle.cc; miquel.raynal@bootlin.com;
> richard@nod.at; vigneshr@ti.com; shawnguo@kernel.org;
> s.hauer@pengutronix.de; kernel@pengutronix.de
> Cc: linux-spi@vger.kernel.org; linux-kernel@vger.kernel.org;
> devicetree@vger.kernel.org; linux-mtd@lists.infradead.org;
> festevam@gmail.com; dl-linux-imx <linux-imx@nxp.com>;
> linux-arm-kernel@lists.infradead.org; zhengxunli@mxic.com.tw
> Subject: Re: [PATCH 07/11] dt-bindings: spi: spi-nxp-fspi: add a new property
> nxp,fspi-dll-slvdly
> 
> On 05/07/2022 11:11, haibo.chen@nxp.com wrote:
> > From: Haibo Chen <haibo.chen@nxp.com>
> >
> > Add one optional property nxp,fspi-dll-slvdly
> >
> > Signed-off-by: Haibo Chen <haibo.chen@nxp.com>
> > ---
> >  Documentation/devicetree/bindings/spi/spi-nxp-fspi.yaml | 6 ++++++
> >  1 file changed, 6 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/spi/spi-nxp-fspi.yaml
> b/Documentation/devicetree/bindings/spi/spi-nxp-fspi.yaml
> > index 1b552c298277..6bd61565686a 100644
> > --- a/Documentation/devicetree/bindings/spi/spi-nxp-fspi.yaml
> > +++ b/Documentation/devicetree/bindings/spi/spi-nxp-fspi.yaml
> > @@ -45,6 +45,12 @@ properties:
> >        - const: fspi_en
> >        - const: fspi
> >
> > +  nxp,fspi-dll-slvdly:
> > +    $ref: /schemas/types.yaml#/definitions/uint32
> > +    description: |
> > +      Specify the DLL slave line delay value.
> 
> What are the units?

Do you mean here need to give more detail explain about this properity?

How about change like this?
   Specify the DLL slave line delay value. The delay target for slave delay line is: ((nxp,fspi-dll-slvdly+1) * 1/32 * clock cycle of reference clock (serial root clock). The range of this value is 0~16.


Best Regards
Haibo Chen

> 
> Best regards,
> Krzysztof
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH 07/11] dt-bindings: spi: spi-nxp-fspi: add a new property nxp,fspi-dll-slvdly

[Krzysztof Kozlowski]

On 05/07/2022 12:28, Bough Chen wrote:
>> -----Original Message-----
>> From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>> Sent: 2022年7月5日 17:48
>> To: Bough Chen <haibo.chen@nxp.com>; ashish.kumar@nxp.com;
>> yogeshgaur.83@gmail.com; broonie@kernel.org; robh+dt@kernel.org;
>> krzysztof.kozlowski+dt@linaro.org; Han Xu <han.xu@nxp.com>;
>> singh.kuldeep87k@gmail.com; tudor.ambarus@microchip.com;
>> p.yadav@ti.com; michael@walle.cc; miquel.raynal@bootlin.com;
>> richard@nod.at; vigneshr@ti.com; shawnguo@kernel.org;
>> s.hauer@pengutronix.de; kernel@pengutronix.de
>> Cc: linux-spi@vger.kernel.org; linux-kernel@vger.kernel.org;
>> devicetree@vger.kernel.org; linux-mtd@lists.infradead.org;
>> festevam@gmail.com; dl-linux-imx <linux-imx@nxp.com>;
>> linux-arm-kernel@lists.infradead.org; zhengxunli@mxic.com.tw
>> Subject: Re: [PATCH 07/11] dt-bindings: spi: spi-nxp-fspi: add a new property
>> nxp,fspi-dll-slvdly
>>
>> On 05/07/2022 11:11, haibo.chen@nxp.com wrote:
>>> From: Haibo Chen <haibo.chen@nxp.com>
>>>
>>> Add one optional property nxp,fspi-dll-slvdly
>>>
>>> Signed-off-by: Haibo Chen <haibo.chen@nxp.com>
>>> ---
>>>  Documentation/devicetree/bindings/spi/spi-nxp-fspi.yaml | 6 ++++++
>>>  1 file changed, 6 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/spi/spi-nxp-fspi.yaml
>> b/Documentation/devicetree/bindings/spi/spi-nxp-fspi.yaml
>>> index 1b552c298277..6bd61565686a 100644
>>> --- a/Documentation/devicetree/bindings/spi/spi-nxp-fspi.yaml
>>> +++ b/Documentation/devicetree/bindings/spi/spi-nxp-fspi.yaml
>>> @@ -45,6 +45,12 @@ properties:
>>>        - const: fspi_en
>>>        - const: fspi
>>>
>>> +  nxp,fspi-dll-slvdly:
>>> +    $ref: /schemas/types.yaml#/definitions/uint32
>>> +    description: |
>>> +      Specify the DLL slave line delay value.
>>
>> What are the units?
> 
> Do you mean here need to give more detail explain about this properity?
> 
> How about change like this?
>    Specify the DLL slave line delay value. The delay target for slave delay line is: ((nxp,fspi-dll-slvdly+1) * 1/32 * clock cycle of reference clock (serial root clock). 

This would be good.

> The range of this value is 0~16.

This needs to go to schema instead as "maximum: 16".

But still the question is - what are the units used in this "delay"? ms? us?

Best regards,
Krzysztof

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

RE: [PATCH 07/11] dt-bindings: spi: spi-nxp-fspi: add a new property nxp,fspi-dll-slvdly

[Han Xu]

>-----Original Message-----
>From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>Sent: Tuesday, July 5, 2022 5:37 AM
>To: Bough Chen <haibo.chen@nxp.com>; ashish.kumar@nxp.com;
>yogeshgaur.83@gmail.com; broonie@kernel.org; robh+dt@kernel.org;
>krzysztof.kozlowski+dt@linaro.org; Han Xu <han.xu@nxp.com>;
>singh.kuldeep87k@gmail.com; tudor.ambarus@microchip.com; p.yadav@ti.com;
>michael@walle.cc; miquel.raynal@bootlin.com; richard@nod.at; vigneshr@ti.com;
>shawnguo@kernel.org; s.hauer@pengutronix.de; kernel@pengutronix.de
>Cc: linux-spi@vger.kernel.org; linux-kernel@vger.kernel.org;
>devicetree@vger.kernel.org; linux-mtd@lists.infradead.org; festevam@gmail.com;
>dl-linux-imx <linux-imx@nxp.com>; linux-arm-kernel@lists.infradead.org;
>zhengxunli@mxic.com.tw
>Subject: Re: [PATCH 07/11] dt-bindings: spi: spi-nxp-fspi: add a new property
>nxp,fspi-dll-slvdly
>
>On 05/07/2022 12:28, Bough Chen wrote:
>>> -----Original Message-----
>>> From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>>> Sent: 2022年7月5日 17:48
>>> To: Bough Chen <haibo.chen@nxp.com>; ashish.kumar@nxp.com;
>>> yogeshgaur.83@gmail.com; broonie@kernel.org; robh+dt@kernel.org;
>>> krzysztof.kozlowski+dt@linaro.org; Han Xu <han.xu@nxp.com>;
>>> singh.kuldeep87k@gmail.com; tudor.ambarus@microchip.com;
>>> p.yadav@ti.com; michael@walle.cc; miquel.raynal@bootlin.com;
>>> richard@nod.at; vigneshr@ti.com; shawnguo@kernel.org;
>>> s.hauer@pengutronix.de; kernel@pengutronix.de
>>> Cc: linux-spi@vger.kernel.org; linux-kernel@vger.kernel.org;
>>> devicetree@vger.kernel.org; linux-mtd@lists.infradead.org;
>>> festevam@gmail.com; dl-linux-imx <linux-imx@nxp.com>;
>>> linux-arm-kernel@lists.infradead.org; zhengxunli@mxic.com.tw
>>> Subject: Re: [PATCH 07/11] dt-bindings: spi: spi-nxp-fspi: add a new
>>> property nxp,fspi-dll-slvdly
>>>
>>> On 05/07/2022 11:11, haibo.chen@nxp.com wrote:
>>>> From: Haibo Chen <haibo.chen@nxp.com>
>>>>
>>>> Add one optional property nxp,fspi-dll-slvdly
>>>>
>>>> Signed-off-by: Haibo Chen <haibo.chen@nxp.com>
>>>> ---
>>>>  Documentation/devicetree/bindings/spi/spi-nxp-fspi.yaml | 6 ++++++
>>>>  1 file changed, 6 insertions(+)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/spi/spi-nxp-fspi.yaml
>>> b/Documentation/devicetree/bindings/spi/spi-nxp-fspi.yaml
>>>> index 1b552c298277..6bd61565686a 100644
>>>> --- a/Documentation/devicetree/bindings/spi/spi-nxp-fspi.yaml
>>>> +++ b/Documentation/devicetree/bindings/spi/spi-nxp-fspi.yaml
>>>> @@ -45,6 +45,12 @@ properties:
>>>>        - const: fspi_en
>>>>        - const: fspi
>>>>
>>>> +  nxp,fspi-dll-slvdly:
>>>> +    $ref: /schemas/types.yaml#/definitions/uint32
>>>> +    description: |
>>>> +      Specify the DLL slave line delay value.
>>>
>>> What are the units?
>>
>> Do you mean here need to give more detail explain about this properity?
>>
>> How about change like this?
>>    Specify the DLL slave line delay value. The delay target for slave delay line is:
>((nxp,fspi-dll-slvdly+1) * 1/32 * clock cycle of reference clock (serial root clock).
>
>This would be good.
>
>> The range of this value is 0~16.
>
>This needs to go to schema instead as "maximum: 16".
>
>But still the question is - what are the units used in this "delay"? ms? us?

HI Krzysztof,

According to the formula, the range should be 0~15, 16 should do nothing or no delay.

The unit should be clock phase. In other words, the delay can be in range of 1/32~1/2 clock cycle.


>
>Best regards,
>Krzysztof
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH 07/11] dt-bindings: spi: spi-nxp-fspi: add a new property nxp,fspi-dll-slvdly

[Krzysztof Kozlowski]

On 05/07/2022 15:19, Han Xu wrote:
>>>>> +  nxp,fspi-dll-slvdly:
>>>>> +    $ref: /schemas/types.yaml#/definitions/uint32
>>>>> +    description: |
>>>>> +      Specify the DLL slave line delay value.
>>>>
>>>> What are the units?
>>>
>>> Do you mean here need to give more detail explain about this properity?
>>>
>>> How about change like this?
>>>    Specify the DLL slave line delay value. The delay target for slave delay line is:
>> ((nxp,fspi-dll-slvdly+1) * 1/32 * clock cycle of reference clock (serial root clock).
>>
>> This would be good.
>>
>>> The range of this value is 0~16.
>>
>> This needs to go to schema instead as "maximum: 16".
>>
>> But still the question is - what are the units used in this "delay"? ms? us?
> 
> HI Krzysztof,
> 
> According to the formula, the range should be 0~15, 16 should do nothing or no delay.

Sure, just add some constraint.

> 
> The unit should be clock phase. In other words, the delay can be in range of 1/32~1/2 clock cycle.

So we probably misunderstood each other... looking at the driver it also
explains the confusing. You encoded here register value which is pretty
often wrong approach.

This should be instead meaningful value for the user of the bindings, so
usually using one of property units:
https://github.com/devicetree-org/dt-schema/blob/main/dtschema/schemas/property-units.yaml

I think you could use here clock cycles or clock phase, but then it has
to be obvious it is that unit.

Best regards,
Krzysztof

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

RE: [PATCH 07/11] dt-bindings: spi: spi-nxp-fspi: add a new property nxp,fspi-dll-slvdly

[Han Xu]

>-----Original Message-----
>From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>Sent: Tuesday, July 5, 2022 8:29 AM
>To: Han Xu <han.xu@nxp.com>; Bough Chen <haibo.chen@nxp.com>;
>ashish.kumar@nxp.com; yogeshgaur.83@gmail.com; broonie@kernel.org;
>robh+dt@kernel.org; krzysztof.kozlowski+dt@linaro.org;
>singh.kuldeep87k@gmail.com; tudor.ambarus@microchip.com; p.yadav@ti.com;
>michael@walle.cc; miquel.raynal@bootlin.com; richard@nod.at; vigneshr@ti.com;
>shawnguo@kernel.org; s.hauer@pengutronix.de; kernel@pengutronix.de
>Cc: linux-spi@vger.kernel.org; linux-kernel@vger.kernel.org;
>devicetree@vger.kernel.org; linux-mtd@lists.infradead.org; festevam@gmail.com;
>dl-linux-imx <linux-imx@nxp.com>; linux-arm-kernel@lists.infradead.org;
>zhengxunli@mxic.com.tw
>Subject: Re: [PATCH 07/11] dt-bindings: spi: spi-nxp-fspi: add a new property
>nxp,fspi-dll-slvdly
>
>On 05/07/2022 15:19, Han Xu wrote:
>>>>>> +  nxp,fspi-dll-slvdly:
>>>>>> +    $ref: /schemas/types.yaml#/definitions/uint32
>>>>>> +    description: |
>>>>>> +      Specify the DLL slave line delay value.
>>>>>
>>>>> What are the units?
>>>>
>>>> Do you mean here need to give more detail explain about this properity?
>>>>
>>>> How about change like this?
>>>>    Specify the DLL slave line delay value. The delay target for slave delay line is:
>>> ((nxp,fspi-dll-slvdly+1) * 1/32 * clock cycle of reference clock (serial root clock).
>>>
>>> This would be good.
>>>
>>>> The range of this value is 0~16.
>>>
>>> This needs to go to schema instead as "maximum: 16".
>>>
>>> But still the question is - what are the units used in this "delay"? ms? us?
>>
>> HI Krzysztof,
>>
>> According to the formula, the range should be 0~15, 16 should do nothing or no
>delay.
>
>Sure, just add some constraint.
>
>>
>> The unit should be clock phase. In other words, the delay can be in range of
>1/32~1/2 clock cycle.
>
>So we probably misunderstood each other... looking at the driver it also explains
>the confusing. You encoded here register value which is pretty often wrong
>approach.
>
>This should be instead meaningful value for the user of the bindings, so usually
>using one of property units:
>https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com
>%2Fdevicetree-org%2Fdt-
>schema%2Fblob%2Fmain%2Fdtschema%2Fschemas%2Fproperty-
>units.yaml&amp;data=05%7C01%7Chan.xu%40nxp.com%7C0ffe3d706e064f14382
>108da5e8a5add%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C6379262
>45564450475%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV
>2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=Q4
>SfVnBN%2BQ0vYKJzRf%2FXZkCA1WGyPV9doFcb%2BLSKx4w%3D&amp;reserved=0
>
>I think you could use here clock cycles or clock phase, but then it has to be obvious
>it is that unit.

Hi Krzysztof, 

Let me clarify it, in the document a term "delay cell" was used to descript this register bit. Each delay cell equals "1/32 clock phase", so the unit of delay cell is clock phase. The value user need set in DT just number to define how many delay cells needed.

>
>Best regards,
>Krzysztof

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH 07/11] dt-bindings: spi: spi-nxp-fspi: add a new property nxp,fspi-dll-slvdly

[Krzysztof Kozlowski]

On 05/07/2022 16:00, Han Xu wrote:
>> So we probably misunderstood each other... looking at the driver it also explains
>> the confusing. You encoded here register value which is pretty often wrong
>> approach.
>>
>> This should be instead meaningful value for the user of the bindings, so usually
>> using one of property units:
>> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com
>> %2Fdevicetree-org%2Fdt-
>> schema%2Fblob%2Fmain%2Fdtschema%2Fschemas%2Fproperty-
>> units.yaml&data=05%7C01%7Chan.xu%40nxp.com%7C0ffe3d706e064f14382
>> 108da5e8a5add%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C6379262
>> 45564450475%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV
>> 2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=Q4
>> SfVnBN%2BQ0vYKJzRf%2FXZkCA1WGyPV9doFcb%2BLSKx4w%3D&reserved=0
>>
>> I think you could use here clock cycles or clock phase, but then it has to be obvious
>> it is that unit.
> 
> Hi Krzysztof, 
> 
> Let me clarify it, in the document a term "delay cell" was used to descript this register bit. 

Which document? The bindings (I cannot find it there)? Commit msg?

> Each delay cell equals "1/32 clock phase", so the unit of delay cell is clock phase. The value user need set in DT just number to define how many delay cells needed.

Your bindings did not say this at all.

Best regards,
Krzysztof

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH 07/11] dt-bindings: spi: spi-nxp-fspi: add a new property nxp,fspi-dll-slvdly

[Han Xu]

On 22/07/05 04:03PM, Krzysztof Kozlowski wrote:
> On 05/07/2022 16:00, Han Xu wrote:
> >> So we probably misunderstood each other... looking at the driver it also explains
> >> the confusing. You encoded here register value which is pretty often wrong
> >> approach.
> >>
> >> This should be instead meaningful value for the user of the bindings, so usually
> >> using one of property units:
> >> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2F&data=05%7C01%7Chan.xu%40nxp.com%7C8b8e3e6291c24579020308da5e8f2916%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637926266207468995%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=KHgjKLX7M8CYfJoWqpVhNdZc%2FlZhZxp6CuaPTUYgwE8%3D&reserved=0
> >> %2Fdevicetree-org%2Fdt-
> >> schema%2Fblob%2Fmain%2Fdtschema%2Fschemas%2Fproperty-
> >> units.yaml&data=05%7C01%7Chan.xu%40nxp.com%7C0ffe3d706e064f14382
> >> 108da5e8a5add%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C6379262
> >> 45564450475%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV
> >> 2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=Q4
> >> SfVnBN%2BQ0vYKJzRf%2FXZkCA1WGyPV9doFcb%2BLSKx4w%3D&reserved=0
> >>
> >> I think you could use here clock cycles or clock phase, but then it has to be obvious
> >> it is that unit.
> > 
> > Hi Krzysztof, 
> > 
> > Let me clarify it, in the document a term "delay cell" was used to descript this register bit. 
> 
> Which document? The bindings (I cannot find it there)? Commit msg?

The SoC Reference Manual.

> 
> > Each delay cell equals "1/32 clock phase", so the unit of delay cell is clock phase. The value user need set in DT just number to define how many delay cells needed.
> 
> Your bindings did not say this at all.

I will explain all details in v2 patch.

> 
> Best regards,
> Krzysztof

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH 07/11] dt-bindings: spi: spi-nxp-fspi: add a new property nxp,fspi-dll-slvdly

[Michael Walle]

Am 2022-07-05 16:00, schrieb Han Xu:
>> -----Original Message-----
>> From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>> Sent: Tuesday, July 5, 2022 8:29 AM
>> To: Han Xu <han.xu@nxp.com>; Bough Chen <haibo.chen@nxp.com>;
>> ashish.kumar@nxp.com; yogeshgaur.83@gmail.com; broonie@kernel.org;
>> robh+dt@kernel.org; krzysztof.kozlowski+dt@linaro.org;
>> singh.kuldeep87k@gmail.com; tudor.ambarus@microchip.com; 
>> p.yadav@ti.com;
>> michael@walle.cc; miquel.raynal@bootlin.com; richard@nod.at; 
>> vigneshr@ti.com;
>> shawnguo@kernel.org; s.hauer@pengutronix.de; kernel@pengutronix.de
>> Cc: linux-spi@vger.kernel.org; linux-kernel@vger.kernel.org;
>> devicetree@vger.kernel.org; linux-mtd@lists.infradead.org; 
>> festevam@gmail.com;
>> dl-linux-imx <linux-imx@nxp.com>; 
>> linux-arm-kernel@lists.infradead.org;
>> zhengxunli@mxic.com.tw
>> Subject: Re: [PATCH 07/11] dt-bindings: spi: spi-nxp-fspi: add a new 
>> property
>> nxp,fspi-dll-slvdly
>> 
>> On 05/07/2022 15:19, Han Xu wrote:
>>>>>>> +  nxp,fspi-dll-slvdly:
>>>>>>> +    $ref: /schemas/types.yaml#/definitions/uint32
>>>>>>> +    description: |
>>>>>>> +      Specify the DLL slave line delay value.
>>>>>> 
>>>>>> What are the units?
>>>>> 
>>>>> Do you mean here need to give more detail explain about this 
>>>>> properity?
>>>>> 
>>>>> How about change like this?
>>>>>    Specify the DLL slave line delay value. The delay target for 
>>>>> slave delay line is:
>>>> ((nxp,fspi-dll-slvdly+1) * 1/32 * clock cycle of reference clock 
>>>> (serial root clock).
>>>> 
>>>> This would be good.
>>>> 
>>>>> The range of this value is 0~16.
>>>> 
>>>> This needs to go to schema instead as "maximum: 16".
>>>> 
>>>> But still the question is - what are the units used in this "delay"? 
>>>> ms? us?
>>> 
>>> HI Krzysztof,
>>> 
>>> According to the formula, the range should be 0~15, 16 should do 
>>> nothing or no
>> delay.
>> 
>> Sure, just add some constraint.
>> 
>>> 
>>> The unit should be clock phase. In other words, the delay can be in 
>>> range of
>> 1/32~1/2 clock cycle.
>> 
>> So we probably misunderstood each other... looking at the driver it 
>> also explains
>> the confusing. You encoded here register value which is pretty often 
>> wrong
>> approach.
>> 
>> This should be instead meaningful value for the user of the bindings, 
>> so usually
>> using one of property units:
>> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com
>> %2Fdevicetree-org%2Fdt-
>> schema%2Fblob%2Fmain%2Fdtschema%2Fschemas%2Fproperty-
>> units.yaml&amp;data=05%7C01%7Chan.xu%40nxp.com%7C0ffe3d706e064f14382
>> 108da5e8a5add%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C6379262
>> 45564450475%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV
>> 2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=Q4
>> SfVnBN%2BQ0vYKJzRf%2FXZkCA1WGyPV9doFcb%2BLSKx4w%3D&amp;reserved=0

Hm, you should fix your mail server.

>> 
>> I think you could use here clock cycles or clock phase, but then it 
>> has to be obvious
>> it is that unit.
> 
> Hi Krzysztof,
> 
> Let me clarify it, in the document a term "delay cell" was used to
> descript this register bit. Each delay cell equals "1/32 clock phase",
> so the unit of delay cell is clock phase. The value user need set in
> DT just number to define how many delay cells needed.

Then should the unit be "-degrees" and the possible range 0-180?

-michael

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH 07/11] dt-bindings: spi: spi-nxp-fspi: add a new property nxp,fspi-dll-slvdly

[Krzysztof Kozlowski]

On 05/07/2022 16:06, Michael Walle wrote:
> 
>>>
>>> I think you could use here clock cycles or clock phase, but then it 
>>> has to be obvious
>>> it is that unit.
>>
>> Hi Krzysztof,
>>
>> Let me clarify it, in the document a term "delay cell" was used to
>> descript this register bit. Each delay cell equals "1/32 clock phase",
>> so the unit of delay cell is clock phase. The value user need set in
>> DT just number to define how many delay cells needed.
> 
> Then should the unit be "-degrees" and the possible range 0-180?

Thanks. We don't have it documented currently, but the unit seems
reasonable.

Best regards,
Krzysztof

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH 07/11] dt-bindings: spi: spi-nxp-fspi: add a new property nxp,fspi-dll-slvdly

[Han Xu]

On 22/07/05 04:12PM, Krzysztof Kozlowski wrote:
> On 05/07/2022 16:06, Michael Walle wrote:
> > 
> >>>
> >>> I think you could use here clock cycles or clock phase, but then it 
> >>> has to be obvious
> >>> it is that unit.
> >>
> >> Hi Krzysztof,
> >>
> >> Let me clarify it, in the document a term "delay cell" was used to
> >> descript this register bit. Each delay cell equals "1/32 clock phase",
> >> so the unit of delay cell is clock phase. The value user need set in
> >> DT just number to define how many delay cells needed.
> > 
> > Then should the unit be "-degrees" and the possible range 0-180?
> 
> Thanks. We don't have it documented currently, but the unit seems
> reasonable.

IMO, use the unit "-degrees" makes it more complicate. Personaly I would
calculate how many clock cycle delay needed, such as 1/4 clock cycle or half
clock cycle. Using degree brings extra calculation.

The granularity of the clock phase change is 1/32 of 180 degree, but the range
0-180 make people feel it can be set in any degree in range.

If I describe all details of the relation between "nxp,fspi-dll-slvdly" and
"delay cell" in patch v2, do you think it's clear for users?

> 
> Best regards,
> Krzysztof

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH 07/11] dt-bindings: spi: spi-nxp-fspi: add a new property nxp,fspi-dll-slvdly

[Michael Walle]

Am 2022-07-05 16:52, schrieb Han Xu:
> On 22/07/05 04:12PM, Krzysztof Kozlowski wrote:
>> On 05/07/2022 16:06, Michael Walle wrote:
>> >
>> >>>
>> >>> I think you could use here clock cycles or clock phase, but then it
>> >>> has to be obvious
>> >>> it is that unit.
>> >>
>> >> Hi Krzysztof,
>> >>
>> >> Let me clarify it, in the document a term "delay cell" was used to
>> >> descript this register bit. Each delay cell equals "1/32 clock phase",
>> >> so the unit of delay cell is clock phase. The value user need set in
>> >> DT just number to define how many delay cells needed.
>> >
>> > Then should the unit be "-degrees" and the possible range 0-180?
>> 
>> Thanks. We don't have it documented currently, but the unit seems
>> reasonable.
> 
> IMO, use the unit "-degrees" makes it more complicate. Personaly I 
> would
> calculate how many clock cycle delay needed, such as 1/4 clock cycle or 
> half
> clock cycle. Using degree brings extra calculation.

What is the extra calculation here? For hardware engineer who has to
specify this, it is easier to give the delay in clock phase (in degrees)
rather than reading the documentation and transform that into a value
given in 1/32 part of a clock, that should be part of the driver.

> The granularity of the clock phase change is 1/32 of 180 degree, but 
> the range
> 0-180 make people feel it can be set in any degree in range.

I'm not sure if the DT bindings have a granularity feature but you
could just round to the next possible value. I guess that is
the case for any value which isn't given as the raw value.

-michael

> If I describe all details of the relation between "nxp,fspi-dll-slvdly" 
> and
> "delay cell" in patch v2, do you think it's clear for users?

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH 07/11] dt-bindings: spi: spi-nxp-fspi: add a new property nxp,fspi-dll-slvdly

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 787 bytes --]

On Tue, Jul 05, 2022 at 04:58:40PM +0200, Michael Walle wrote:
> Am 2022-07-05 16:52, schrieb Han Xu:

> > IMO, use the unit "-degrees" makes it more complicate. Personaly I would
> > calculate how many clock cycle delay needed, such as 1/4 clock cycle or
> > half
> > clock cycle. Using degree brings extra calculation.

> What is the extra calculation here? For hardware engineer who has to
> specify this, it is easier to give the delay in clock phase (in degrees)
> rather than reading the documentation and transform that into a value
> given in 1/32 part of a clock, that should be part of the driver.

IME if it's a hardware engineer specifying things by the time they get
as far as a software engineer they'll often have been turned into "write
these values to these registers".

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 144 bytes --]

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH 07/11] dt-bindings: spi: spi-nxp-fspi: add a new property nxp,fspi-dll-slvdly

[Krzysztof Kozlowski]

On 05/07/2022 16:52, Han Xu wrote:
> On 22/07/05 04:12PM, Krzysztof Kozlowski wrote:
>> On 05/07/2022 16:06, Michael Walle wrote:
>>>
>>>>>
>>>>> I think you could use here clock cycles or clock phase, but then it 
>>>>> has to be obvious
>>>>> it is that unit.
>>>>
>>>> Hi Krzysztof,
>>>>
>>>> Let me clarify it, in the document a term "delay cell" was used to
>>>> descript this register bit. Each delay cell equals "1/32 clock phase",
>>>> so the unit of delay cell is clock phase. The value user need set in
>>>> DT just number to define how many delay cells needed.
>>>
>>> Then should the unit be "-degrees" and the possible range 0-180?
>>
>> Thanks. We don't have it documented currently, but the unit seems
>> reasonable.
> 
> IMO, use the unit "-degrees" makes it more complicate. Personaly I would
> calculate how many clock cycle delay needed, such as 1/4 clock cycle or half
> clock cycle. Using degree brings extra calculation.

And what if the next device uses a bit different divider? Like 1/16?
This is why we have standard units so people won't push register values
into the bindings.

> 
> The granularity of the clock phase change is 1/32 of 180 degree, but the range
> 0-180 make people feel it can be set in any degree in range.

Yes, because that's how the bindings are being written - allowing any
reasonable value, not register-specific value, to be used because it is
the most flexible, hardware-independent and allows further customization
of bindings (e.g. new devices). Embedding device programming model into
the bindings contradicts it.

Second, nothing stops you from narrowing the acceptable values with an
enum. This still allows extension. Your 1/32 does not.

> 
> If I describe all details of the relation between "nxp,fspi-dll-slvdly" and
> "delay cell" in patch v2, do you think it's clear for users?

1/32 could be a nice unit, but degrees is better. Just like uV is better
than 1/32 of V. Like 1 us is better than 1/32 of ms.

Do you see  in the bindings many other values like time, potential,
current or power described in 1/32 units?

Best regards,
Krzysztof

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH 07/11] dt-bindings: spi: spi-nxp-fspi: add a new property nxp,fspi-dll-slvdly

[Han Xu]

On 22/07/05 05:38PM, Krzysztof Kozlowski wrote:
> On 05/07/2022 16:52, Han Xu wrote:
> > On 22/07/05 04:12PM, Krzysztof Kozlowski wrote:
> >> On 05/07/2022 16:06, Michael Walle wrote:
> >>>
> >>>>>
> >>>>> I think you could use here clock cycles or clock phase, but then it 
> >>>>> has to be obvious
> >>>>> it is that unit.
> >>>>
> >>>> Hi Krzysztof,
> >>>>
> >>>> Let me clarify it, in the document a term "delay cell" was used to
> >>>> descript this register bit. Each delay cell equals "1/32 clock phase",
> >>>> so the unit of delay cell is clock phase. The value user need set in
> >>>> DT just number to define how many delay cells needed.
> >>>
> >>> Then should the unit be "-degrees" and the possible range 0-180?
> >>
> >> Thanks. We don't have it documented currently, but the unit seems
> >> reasonable.
> > 
> > IMO, use the unit "-degrees" makes it more complicate. Personaly I would
> > calculate how many clock cycle delay needed, such as 1/4 clock cycle or half
> > clock cycle. Using degree brings extra calculation.
> 
> And what if the next device uses a bit different divider? Like 1/16?
> This is why we have standard units so people won't push register values
> into the bindings.
> 
> > 
> > The granularity of the clock phase change is 1/32 of 180 degree, but the range
> > 0-180 make people feel it can be set in any degree in range.
> 
> Yes, because that's how the bindings are being written - allowing any
> reasonable value, not register-specific value, to be used because it is
> the most flexible, hardware-independent and allows further customization
> of bindings (e.g. new devices). Embedding device programming model into
> the bindings contradicts it.
> 
> Second, nothing stops you from narrowing the acceptable values with an
> enum. This still allows extension. Your 1/32 does not.
> 
> > 
> > If I describe all details of the relation between "nxp,fspi-dll-slvdly" and
> > "delay cell" in patch v2, do you think it's clear for users?
> 
> 1/32 could be a nice unit, but degrees is better. Just like uV is better
> than 1/32 of V. Like 1 us is better than 1/32 of ms.
> 
> Do you see  in the bindings many other values like time, potential,
> current or power described in 1/32 units?

That make sense. I will use degree as the unit and round to register proper
value in driver as Michael suggested. Thanks for all comments.

> 
> Best regards,
> Krzysztof

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH 07/11] dt-bindings: spi: spi-nxp-fspi: add a new property nxp,fspi-dll-slvdly

[Rob Herring]

On Tue, Jul 05, 2022 at 10:50:31AM -0500, Han Xu wrote:
> On 22/07/05 05:38PM, Krzysztof Kozlowski wrote:
> > On 05/07/2022 16:52, Han Xu wrote:
> > > On 22/07/05 04:12PM, Krzysztof Kozlowski wrote:
> > >> On 05/07/2022 16:06, Michael Walle wrote:
> > >>>
> > >>>>>
> > >>>>> I think you could use here clock cycles or clock phase, but then it 
> > >>>>> has to be obvious
> > >>>>> it is that unit.
> > >>>>
> > >>>> Hi Krzysztof,
> > >>>>
> > >>>> Let me clarify it, in the document a term "delay cell" was used to
> > >>>> descript this register bit. Each delay cell equals "1/32 clock phase",
> > >>>> so the unit of delay cell is clock phase. The value user need set in
> > >>>> DT just number to define how many delay cells needed.
> > >>>
> > >>> Then should the unit be "-degrees" and the possible range 0-180?
> > >>
> > >> Thanks. We don't have it documented currently, but the unit seems
> > >> reasonable.
> > > 
> > > IMO, use the unit "-degrees" makes it more complicate. Personaly I would
> > > calculate how many clock cycle delay needed, such as 1/4 clock cycle or half
> > > clock cycle. Using degree brings extra calculation.
> > 
> > And what if the next device uses a bit different divider? Like 1/16?
> > This is why we have standard units so people won't push register values
> > into the bindings.
> > 
> > > 
> > > The granularity of the clock phase change is 1/32 of 180 degree, but the range
> > > 0-180 make people feel it can be set in any degree in range.
> > 
> > Yes, because that's how the bindings are being written - allowing any
> > reasonable value, not register-specific value, to be used because it is
> > the most flexible, hardware-independent and allows further customization
> > of bindings (e.g. new devices). Embedding device programming model into
> > the bindings contradicts it.
> > 
> > Second, nothing stops you from narrowing the acceptable values with an
> > enum. This still allows extension. Your 1/32 does not.
> > 
> > > 
> > > If I describe all details of the relation between "nxp,fspi-dll-slvdly" and
> > > "delay cell" in patch v2, do you think it's clear for users?

What's a cell?

> > 
> > 1/32 could be a nice unit, but degrees is better. Just like uV is better
> > than 1/32 of V. Like 1 us is better than 1/32 of ms.
> > 
> > Do you see  in the bindings many other values like time, potential,
> > current or power described in 1/32 units?
> 
> That make sense. I will use degree as the unit and round to register proper
> value in driver as Michael suggested. Thanks for all comments.

I'm still wondering what this is delaying? From what to what? We already 
have numerous common delay properties for SPI. If one of those doesn't 
work, then should this be a new common property? And if common, I think 
time units is better to use than degrees.

If this is vendor specific, then I'd just use the register value. 
There's not much point in using common units unless it is a common 
property.

Rob

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH 07/11] dt-bindings: spi: spi-nxp-fspi: add a new property nxp,fspi-dll-slvdly

[Michael Walle]

Am 2022-07-06 18:11, schrieb Rob Herring:
> On Tue, Jul 05, 2022 at 10:50:31AM -0500, Han Xu wrote:
>> On 22/07/05 05:38PM, Krzysztof Kozlowski wrote:
>> > On 05/07/2022 16:52, Han Xu wrote:
>> > > On 22/07/05 04:12PM, Krzysztof Kozlowski wrote:
>> > >> On 05/07/2022 16:06, Michael Walle wrote:
>> > >>>
>> > >>>>>
>> > >>>>> I think you could use here clock cycles or clock phase, but then it
>> > >>>>> has to be obvious
>> > >>>>> it is that unit.
>> > >>>>
>> > >>>> Hi Krzysztof,
>> > >>>>
>> > >>>> Let me clarify it, in the document a term "delay cell" was used to
>> > >>>> descript this register bit. Each delay cell equals "1/32 clock phase",
>> > >>>> so the unit of delay cell is clock phase. The value user need set in
>> > >>>> DT just number to define how many delay cells needed.
>> > >>>
>> > >>> Then should the unit be "-degrees" and the possible range 0-180?
>> > >>
>> > >> Thanks. We don't have it documented currently, but the unit seems
>> > >> reasonable.
>> > >
>> > > IMO, use the unit "-degrees" makes it more complicate. Personaly I would
>> > > calculate how many clock cycle delay needed, such as 1/4 clock cycle or half
>> > > clock cycle. Using degree brings extra calculation.
>> >
>> > And what if the next device uses a bit different divider? Like 1/16?
>> > This is why we have standard units so people won't push register values
>> > into the bindings.
>> >
>> > >
>> > > The granularity of the clock phase change is 1/32 of 180 degree, but the range
>> > > 0-180 make people feel it can be set in any degree in range.
>> >
>> > Yes, because that's how the bindings are being written - allowing any
>> > reasonable value, not register-specific value, to be used because it is
>> > the most flexible, hardware-independent and allows further customization
>> > of bindings (e.g. new devices). Embedding device programming model into
>> > the bindings contradicts it.
>> >
>> > Second, nothing stops you from narrowing the acceptable values with an
>> > enum. This still allows extension. Your 1/32 does not.
>> >
>> > >
>> > > If I describe all details of the relation between "nxp,fspi-dll-slvdly" and
>> > > "delay cell" in patch v2, do you think it's clear for users?
> 
> What's a cell?

A delay cell I presume. Which if I read the datasheet correctly is
somewhere between 75ps and 225ps per cell *in an unlocked state*.

I don't understand what the intention of this setting in the device
tree is. I does *not* specify the delay of the DLL, rather it specifies
the target value to be achieved by the DLL. And the RM tells the
recommended value is 0xf. Which makes sense, because it's half a clock
cycle and you'd want to sample in the middle of the clock (note this is
double data rate and you are sampling on falling and rising edge). But
and this is the catch I think, the DLL will only lock if the frequency
is >100MHz. Now if the frequency is lower than 100MHz you can actually
set the value manually (see above), but this is not what the driver
does. You'd need to write the manual value into OVRDVAL, not into
SLVDLYTARGET.

So I'm confused. Why can't you just set SLVDLYTARGET to 0xf if the
frequency is larger than 100MHz? How is this supposed to be used?
Do I need to set the value to 0xf if the frequency is higher than
100MHz? I see you used 4 in your device tree, why don't you use
the recommended value?

>> > 1/32 could be a nice unit, but degrees is better. Just like uV is better
>> > than 1/32 of V. Like 1 us is better than 1/32 of ms.
>> >
>> > Do you see  in the bindings many other values like time, potential,
>> > current or power described in 1/32 units?
>> 
>> That make sense. I will use degree as the unit and round to register 
>> proper
>> value in driver as Michael suggested. Thanks for all comments.
> 
> I'm still wondering what this is delaying? From what to what? We 
> already
> have numerous common delay properties for SPI. If one of those doesn't
> work, then should this be a new common property? And if common, I think
> time units is better to use than degrees.

It's delaying the internal sampling clock with respect to the
internal clock (which also drivers SCK then). With that clock
the data written by the flash device is then sampled. Now there
is also the read strobe; I'm unsure if that can also be delayed.

Time units depend on the frequency (or changes with the frequency),
whereas the phase angle doesn't.

> If this is vendor specific, then I'd just use the register value.
> There's not much point in using common units unless it is a common
> property.

-michael

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

[PATCH 08/11] mtd: spi-nor: macronix: add support for Macronix octaflash

[haibo.chen]

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

The octaflash is an xSPI compliant octal DTR flash. Add support
for using it in octal DTR mode.

Try to verify the flash ID to check whether the flash memory in octal
DTR mode and SPI mode are correct. When reading ID in OCTAL DTR mode,
ID will appear in a repeated manner. ex: ID[0] = 0xc2, ID[1] = 0xc2,
ID[2] = 0x94, ID[3] = 0x94... Rearrange the order so that the ID can pass.

The octaflash series can be divided into the following types:

MX25 series : Serial NOR Flash.
MX66 series : Serial NOR Flash with stacked die.(Size larger than 1Gb)
LM/UM series : Up to 250MHz clock frequency with both DTR/STR operation.
LW/UW series : Support simultaneous Read-while-Write operation in multiple
	       bank architecture. Read-while-write feature which means read
	       data one bank while another bank is programing or erasing.

MX25LM : 3.0V Octal I/O
 -https://www.mxic.com.tw/Lists/Datasheet/Attachments/7841/MX25LM51245G,%203V,%20512Mb,%20v1.1.pdf

MX25UM : 1.8V Octal I/O
 -https://www.mxic.com.tw/Lists/Datasheet/Attachments/7525/MX25UM51245G%20Extreme%20Speed,%201.8V,%20512Mb,%20v1.0.pdf

MX66LM : 3.0V Octal I/O with stacked die
 -https://www.mxic.com.tw/Lists/Datasheet/Attachments/7929/MX66LM1G45G,%203V,%201Gb,%20v1.1.pdf

MX66UM : 1.8V Octal I/O with stacked die
 -https://www.mxic.com.tw/Lists/Datasheet/Attachments/7721/MX66UM1G45G,%201.8V,%201Gb,%20v1.1.pdf

MX25LW : 3.0V Octal I/O with Read-while-Write
MX25UW : 1.8V Octal I/O with Read-while-Write
MX66LW : 3.0V Octal I/O with Read-while-Write and stack die
MX66UW : 1.8V Octal I/O with Read-while-Write and stack die

About LW/UW series, please contact us freely if you have any
questions. For adding Octal NOR Flash IDs, we have validated
each Flash on plateform zynq-picozed.

Get this patch from
https://patchwork.ozlabs.org/project/linux-mtd/patch/1621232088-12567-2-git-send-email-zhengxunli@mxic.com.tw/
And add some simple change to match current kernel version.

Signed-off-by: Zhengxun Li <zhengxunli@mxic.com.tw>
Reviewed-by: Pratyush Yadav <p.yadav@ti.com>
Signed-off-by: Haibo Chen <haibo.chen@nxp.com>
---
 drivers/mtd/spi-nor/macronix.c | 210 +++++++++++++++++++++++++++++++++
 1 file changed, 210 insertions(+)

diff --git a/drivers/mtd/spi-nor/macronix.c b/drivers/mtd/spi-nor/macronix.c
index d81a4cb2812b..b4f4d8185cce 100644
--- a/drivers/mtd/spi-nor/macronix.c
+++ b/drivers/mtd/spi-nor/macronix.c
@@ -8,6 +8,12 @@
 
 #include "core.h"
 
+#define SPINOR_OP_RD_CR2		0x71		/* Read configuration register 2 */
+#define SPINOR_OP_WR_CR2		0x72		/* Write configuration register 2 */
+#define SPINOR_REG_MXIC_CR2_MODE	0x00000000	/* For setting octal DTR mode */
+#define SPINOR_REG_MXIC_OPI_DTR_EN	0x2		/* Enable Octal DTR */
+#define SPINOR_REG_MXIC_SPI_EN		0x0		/* Enable SPI */
+
 static int
 mx25l25635_post_bfpt_fixups(struct spi_nor *nor,
 			    const struct sfdp_parameter_header *bfpt_header,
@@ -32,6 +38,85 @@ static const struct spi_nor_fixups mx25l25635_fixups = {
 	.post_bfpt = mx25l25635_post_bfpt_fixups,
 };
 
+/**
+ * spi_nor_macronix_octal_dtr_enable() - Enable octal DTR on Macronix flashes.
+ * @nor:		pointer to a 'struct spi_nor'
+ * @enable:		whether to enable Octal DTR or switch back to SPI
+ *
+ * Return: 0 on success, -errno otherwise.
+ */
+static int spi_nor_macronix_octal_dtr_enable(struct spi_nor *nor, bool enable)
+{
+	struct spi_mem_op op;
+	u8 *buf = nor->bouncebuf, i;
+	int ret;
+
+	/* Set/unset the octal and DTR enable bits. */
+	ret = spi_nor_write_enable(nor);
+	if (ret)
+		return ret;
+
+	if (enable) {
+		buf[0] = SPINOR_REG_MXIC_OPI_DTR_EN;
+	} else {
+		/*
+		 * The register is 1-byte wide, but 1-byte transactions are not
+		 * allowed in 8D-8D-8D mode. Since there is no register at the
+		 * next location, just initialize the value to 0 and let the
+		 * transaction go on.
+		 */
+		buf[0] = SPINOR_REG_MXIC_SPI_EN;
+		buf[1] = 0x0;
+	}
+
+	op = (struct spi_mem_op)
+		SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_WR_CR2, 1),
+			   SPI_MEM_OP_ADDR(4, SPINOR_REG_MXIC_CR2_MODE, 1),
+			   SPI_MEM_OP_NO_DUMMY,
+			   SPI_MEM_OP_DATA_OUT(enable ? 1 : 2, buf, 1));
+
+	if (!enable)
+		spi_nor_spimem_setup_op(nor, &op, SNOR_PROTO_8_8_8_DTR);
+
+	ret = spi_mem_exec_op(nor->spimem, &op);
+	if (ret)
+		return ret;
+
+	/* Read flash ID to make sure the switch was successful. */
+	op = (struct spi_mem_op)
+		SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_RDID, 1),
+			   SPI_MEM_OP_ADDR(enable ? 4 : 0, 0, 1),
+			   SPI_MEM_OP_DUMMY(enable ? 4 : 0, 1),
+			   SPI_MEM_OP_DATA_IN(SPI_NOR_MAX_ID_LEN, buf, 1));
+
+	if (enable)
+		spi_nor_spimem_setup_op(nor, &op, SNOR_PROTO_8_8_8_DTR);
+
+	ret = spi_mem_exec_op(nor->spimem, &op);
+	if (ret)
+		return ret;
+
+	if (enable) {
+		for (i = 0; i < nor->info->id_len; i++)
+			if (buf[i * 2] != nor->info->id[i])
+				return -EINVAL;
+	} else {
+		if (memcmp(buf, nor->info->id, nor->info->id_len))
+			return -EINVAL;
+	}
+
+	return 0;
+}
+
+static void octaflash_default_init(struct spi_nor *nor)
+{
+	nor->params->octal_dtr_enable = spi_nor_macronix_octal_dtr_enable;
+}
+
+static struct spi_nor_fixups octaflash_fixups = {
+	.default_init = octaflash_default_init,
+};
+
 static const struct flash_info macronix_nor_parts[] = {
 	/* Macronix */
 	{ "mx25l512e",   INFO(0xc22010, 0, 64 * 1024,   1)
@@ -100,6 +185,131 @@ static const struct flash_info macronix_nor_parts[] = {
 	{ "mx66u2g45g",	 INFO(0xc2253c, 0, 64 * 1024, 4096)
 		NO_SFDP_FLAGS(SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ)
 		FIXUP_FLAGS(SPI_NOR_4B_OPCODES) },
+	{ "mx66lm2g45g", INFO(0xc2853c, 0, 64 * 1024, 4096)
+		NO_SFDP_FLAGS(SECT_4K | SPI_NOR_OCTAL_DTR_READ |
+			      SPI_NOR_OCTAL_DTR_PP)
+		FIXUP_FLAGS(SPI_NOR_4B_OPCODES)
+		.fixups = &octaflash_fixups },
+	{ "mx66lm1g45g", INFO(0xc2853b, 0, 32 * 1024, 4096)
+		NO_SFDP_FLAGS(SECT_4K | SPI_NOR_OCTAL_DTR_READ |
+			      SPI_NOR_OCTAL_DTR_PP)
+		FIXUP_FLAGS(SPI_NOR_4B_OPCODES)
+		.fixups = &octaflash_fixups },
+	{ "mx66lw1g45g", INFO(0xc2863b, 0, 32 * 1024, 4096)
+		NO_SFDP_FLAGS(SECT_4K | SPI_NOR_OCTAL_DTR_READ |
+			      SPI_NOR_OCTAL_DTR_PP)
+		FIXUP_FLAGS(SPI_NOR_4B_OPCODES)
+		.fixups = &octaflash_fixups },
+	{ "mx25lm51245g", INFO(0xc2853a, 0, 16 * 1024, 4096)
+		NO_SFDP_FLAGS(SECT_4K | SPI_NOR_OCTAL_DTR_READ |
+			      SPI_NOR_OCTAL_DTR_PP)
+		FIXUP_FLAGS(SPI_NOR_4B_OPCODES)
+		.fixups = &octaflash_fixups },
+	{ "mx25lw51245g", INFO(0xc2863a, 0, 16 * 1024, 4096)
+		NO_SFDP_FLAGS(SECT_4K | SPI_NOR_OCTAL_DTR_READ |
+			      SPI_NOR_OCTAL_DTR_PP)
+		FIXUP_FLAGS(SPI_NOR_4B_OPCODES)
+		.fixups = &octaflash_fixups },
+	{ "mx25lm25645g", INFO(0xc28539, 0, 8 * 1024, 4096)
+		NO_SFDP_FLAGS(SECT_4K | SPI_NOR_OCTAL_DTR_READ |
+			      SPI_NOR_OCTAL_DTR_PP)
+		FIXUP_FLAGS(SPI_NOR_4B_OPCODES)
+		.fixups = &octaflash_fixups },
+	{ "mx25lw25645g", INFO(0xc28639, 0, 8 * 1024, 4096)
+		NO_SFDP_FLAGS(SECT_4K | SPI_NOR_OCTAL_DTR_READ |
+			      SPI_NOR_OCTAL_DTR_PP)
+		FIXUP_FLAGS(SPI_NOR_4B_OPCODES)
+		.fixups = &octaflash_fixups },
+	{ "mx66um2g45g", INFO(0xc2803c, 0, 64 * 1024, 4096)
+		NO_SFDP_FLAGS(SECT_4K | SPI_NOR_OCTAL_DTR_READ |
+			      SPI_NOR_OCTAL_DTR_PP)
+		FIXUP_FLAGS(SPI_NOR_4B_OPCODES)
+		.fixups = &octaflash_fixups },
+	{ "mx66uw2g345g", INFO(0xc2843c, 0, 64 * 1024, 4096)
+		NO_SFDP_FLAGS(SECT_4K | SPI_NOR_OCTAL_DTR_READ |
+			      SPI_NOR_OCTAL_DTR_PP)
+		FIXUP_FLAGS(SPI_NOR_4B_OPCODES)
+		.fixups = &octaflash_fixups },
+	{ "mx66uw2g345gx0", INFO(0xc2943c, 0, 64 * 1024, 4096)
+		NO_SFDP_FLAGS(SECT_4K | SPI_NOR_OCTAL_DTR_READ |
+			      SPI_NOR_OCTAL_DTR_PP)
+		FIXUP_FLAGS(SPI_NOR_4B_OPCODES)
+		.fixups = &octaflash_fixups },
+	{ "mx66um1g45g", INFO(0xc2803b, 0, 32 * 1024, 4096)
+		NO_SFDP_FLAGS(SECT_4K | SPI_NOR_OCTAL_DTR_READ |
+			      SPI_NOR_OCTAL_DTR_PP)
+		FIXUP_FLAGS(SPI_NOR_4B_OPCODES)
+		.fixups = &octaflash_fixups },
+	{ "mx66um1g45g40", INFO(0xc2808b, 0, 32 * 1024, 4096)
+		NO_SFDP_FLAGS(SECT_4K | SPI_NOR_OCTAL_DTR_READ |
+			      SPI_NOR_OCTAL_DTR_PP)
+		FIXUP_FLAGS(SPI_NOR_4B_OPCODES)
+		.fixups = &octaflash_fixups },
+	{ "mx66uw1g45g", INFO(0xc2813b, 0, 32 * 1024, 4096)
+		NO_SFDP_FLAGS(SECT_4K | SPI_NOR_OCTAL_DTR_READ |
+			      SPI_NOR_OCTAL_DTR_PP)
+		FIXUP_FLAGS(SPI_NOR_4B_OPCODES)
+		.fixups = &octaflash_fixups },
+	{ "mx25um51245g", INFO(0xc2803a, 0, 16 * 1024, 4096)
+		NO_SFDP_FLAGS(SECT_4K | SPI_NOR_OCTAL_DTR_READ |
+			      SPI_NOR_OCTAL_DTR_PP)
+		FIXUP_FLAGS(SPI_NOR_4B_OPCODES)
+		.fixups = &octaflash_fixups },
+	{ "mx25uw51245g", INFO(0xc2813a, 0, 16 * 1024, 4096)
+		NO_SFDP_FLAGS(SECT_4K | SPI_NOR_OCTAL_DTR_READ |
+			      SPI_NOR_OCTAL_DTR_PP)
+		FIXUP_FLAGS(SPI_NOR_4B_OPCODES)
+		.fixups = &octaflash_fixups },
+	{ "mx25uw51345g", INFO(0xc2843a, 0, 16 * 1024, 4096)
+		NO_SFDP_FLAGS(SECT_4K | SPI_NOR_OCTAL_DTR_READ |
+			      SPI_NOR_OCTAL_DTR_PP)
+		FIXUP_FLAGS(SPI_NOR_4B_OPCODES)
+		.fixups = &octaflash_fixups },
+	{ "mx25um25645g", INFO(0xc28039, 0, 8 * 1024, 4096)
+		NO_SFDP_FLAGS(SECT_4K | SPI_NOR_OCTAL_DTR_READ |
+			      SPI_NOR_OCTAL_DTR_PP)
+		FIXUP_FLAGS(SPI_NOR_4B_OPCODES)
+		.fixups = &octaflash_fixups },
+	{ "mx25uw25645g", INFO(0xc28139, 0, 8 * 1024, 4096)
+		NO_SFDP_FLAGS(SECT_4K | SPI_NOR_OCTAL_DTR_READ |
+			      SPI_NOR_OCTAL_DTR_PP)
+		FIXUP_FLAGS(SPI_NOR_4B_OPCODES)
+		.fixups = &octaflash_fixups },
+	{ "mx25um25345g", INFO(0xc28339, 0, 8 * 1024, 4096)
+		NO_SFDP_FLAGS(SECT_4K | SPI_NOR_OCTAL_DTR_READ |
+			      SPI_NOR_OCTAL_DTR_PP)
+		FIXUP_FLAGS(SPI_NOR_4B_OPCODES)
+		.fixups = &octaflash_fixups },
+	{ "mx25uw25345g", INFO(0xc28439, 0, 8 * 1024, 4096)
+		NO_SFDP_FLAGS(SECT_4K | SPI_NOR_OCTAL_DTR_READ |
+			      SPI_NOR_OCTAL_DTR_PP)
+		FIXUP_FLAGS(SPI_NOR_4B_OPCODES)
+		.fixups = &octaflash_fixups },
+	{ "mx25uw12845g", INFO(0xc28138, 0, 4 * 1024, 4096)
+		NO_SFDP_FLAGS(SECT_4K | SPI_NOR_OCTAL_DTR_READ |
+			      SPI_NOR_OCTAL_DTR_PP)
+		FIXUP_FLAGS(SPI_NOR_4B_OPCODES)
+		.fixups = &octaflash_fixups },
+	{ "mx25uw12a45g", INFO(0xc28938, 0, 4 * 1024, 4096)
+		NO_SFDP_FLAGS(SECT_4K | SPI_NOR_OCTAL_DTR_READ |
+			      SPI_NOR_OCTAL_DTR_PP)
+		FIXUP_FLAGS(SPI_NOR_4B_OPCODES)
+		.fixups = &octaflash_fixups },
+	{ "mx25uw12345g", INFO(0xc28438, 0, 4 * 1024, 4096)
+		NO_SFDP_FLAGS(SECT_4K | SPI_NOR_OCTAL_DTR_READ |
+			      SPI_NOR_OCTAL_DTR_PP)
+		FIXUP_FLAGS(SPI_NOR_4B_OPCODES)
+		.fixups = &octaflash_fixups },
+	{ "mx25uw6445g", INFO(0xc28137, 0, 2 * 1024, 4096)
+		NO_SFDP_FLAGS(SECT_4K | SPI_NOR_OCTAL_DTR_READ |
+			      SPI_NOR_OCTAL_DTR_PP)
+		FIXUP_FLAGS(SPI_NOR_4B_OPCODES)
+		.fixups = &octaflash_fixups },
+	{ "mx25uw6345g", INFO(0xc28437, 0, 2 * 1024, 4096)
+		NO_SFDP_FLAGS(SECT_4K | SPI_NOR_OCTAL_DTR_READ |
+			      SPI_NOR_OCTAL_DTR_PP)
+		FIXUP_FLAGS(SPI_NOR_4B_OPCODES)
+		.fixups = &octaflash_fixups },
 };
 
 static void macronix_nor_default_init(struct spi_nor *nor)
-- 
2.25.1


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

[PATCH 09/11] mtd: spi-nor: macronix: add mx25uw51345g OPI mode support

[haibo.chen]

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

mx25uw51345g has a special OPI DTR read command id, so add this
special fixup.
For RDID under OPI DTR mode, the dummy need to enlarge to 20 cycles,
otherwise can't get correct ID value.

Acked-by: Han Xu <han.xu@nxp.com>
Signed-off-by: Haibo Chen <haibo.chen@nxp.com>
---
 drivers/mtd/spi-nor/macronix.c | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/drivers/mtd/spi-nor/macronix.c b/drivers/mtd/spi-nor/macronix.c
index b4f4d8185cce..99140fc9305b 100644
--- a/drivers/mtd/spi-nor/macronix.c
+++ b/drivers/mtd/spi-nor/macronix.c
@@ -13,6 +13,7 @@
 #define SPINOR_REG_MXIC_CR2_MODE	0x00000000	/* For setting octal DTR mode */
 #define SPINOR_REG_MXIC_OPI_DTR_EN	0x2		/* Enable Octal DTR */
 #define SPINOR_REG_MXIC_SPI_EN		0x0		/* Enable SPI */
+#define SPINOR_OP_OPI_DTR_RD		0xEE		/* OPI DTR first read opcode */
 
 static int
 mx25l25635_post_bfpt_fixups(struct spi_nor *nor,
@@ -117,6 +118,19 @@ static struct spi_nor_fixups octaflash_fixups = {
 	.default_init = octaflash_default_init,
 };
 
+static void mx25uw51345g_post_sfdp_fixup(struct spi_nor *nor)
+{
+	nor->params->hwcaps.mask |= SNOR_HWCAPS_READ_8_8_8_DTR;
+	spi_nor_set_read_settings(&nor->params->reads[SNOR_CMD_READ_8_8_8_DTR],
+				  0, 20, SPINOR_OP_OPI_DTR_RD,
+				  SNOR_PROTO_8_8_8_DTR);
+}
+
+static struct spi_nor_fixups mx25uw51345g_fixups = {
+	.default_init = octaflash_default_init,
+	.post_sfdp = mx25uw51345g_post_sfdp_fixup,
+};
+
 static const struct flash_info macronix_nor_parts[] = {
 	/* Macronix */
 	{ "mx25l512e",   INFO(0xc22010, 0, 64 * 1024,   1)
@@ -264,7 +278,7 @@ static const struct flash_info macronix_nor_parts[] = {
 		NO_SFDP_FLAGS(SECT_4K | SPI_NOR_OCTAL_DTR_READ |
 			      SPI_NOR_OCTAL_DTR_PP)
 		FIXUP_FLAGS(SPI_NOR_4B_OPCODES)
-		.fixups = &octaflash_fixups },
+		.fixups = &mx25uw51345g_fixups },
 	{ "mx25um25645g", INFO(0xc28039, 0, 8 * 1024, 4096)
 		NO_SFDP_FLAGS(SECT_4K | SPI_NOR_OCTAL_DTR_READ |
 			      SPI_NOR_OCTAL_DTR_PP)
-- 
2.25.1


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH 09/11] mtd: spi-nor: macronix: add mx25uw51345g OPI mode support

[Michael Walle]

Am 2022-07-05 11:11, schrieb haibo.chen@nxp.com:
> From: Haibo Chen <haibo.chen@nxp.com>
> 
> mx25uw51345g has a special OPI DTR read command id, so add this
> special fixup.
> For RDID under OPI DTR mode, the dummy need to enlarge to 20 cycles,
> otherwise can't get correct ID value.

Could you please dump the SFDP data of this flash, see [1]. I wonder
if this command isn't described in the SFDP.

-michael

[1] 
https://lore.kernel.org/linux-mtd/4304e19f3399a0a6e856119d01ccabe0@walle.cc/

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

[PATCH 10/11] arm64: dts: imx8ulp: add flexspi support

[haibo.chen]

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

Add flexspi support, and enable the 8 bit TX/RX for the
mx25uw51345gxdi00 spi-nor flash.

Signed-off-by: Haibo Chen <haibo.chen@nxp.com>
---
 arch/arm64/boot/dts/freescale/imx8ulp-evk.dts | 34 +++++++++++++++++++
 arch/arm64/boot/dts/freescale/imx8ulp.dtsi    | 20 ++++++++++-
 2 files changed, 53 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/freescale/imx8ulp-evk.dts b/arch/arm64/boot/dts/freescale/imx8ulp-evk.dts
index 33e84c4e9ed8..69209b87d432 100644
--- a/arch/arm64/boot/dts/freescale/imx8ulp-evk.dts
+++ b/arch/arm64/boot/dts/freescale/imx8ulp-evk.dts
@@ -21,6 +21,23 @@ memory@80000000 {
 	};
 };
 
+&flexspi2 {
+	pinctrl-names = "default", "sleep";
+	pinctrl-0 = <&pinctrl_flexspi2_ptd>;
+	pinctrl-1 = <&pinctrl_flexspi2_ptd>;
+	status = "okay";
+
+	mx25uw51345gxdi00: flash@0 {
+		reg = <0>;
+		#address-cells = <1>;
+		#size-cells = <1>;
+		compatible = "jedec,spi-nor";
+		spi-max-frequency = <200000000>;
+		spi-tx-bus-width = <8>;
+		spi-rx-bus-width = <8>;
+	};
+};
+
 &lpuart5 {
 	/* console */
 	pinctrl-names = "default", "sleep";
@@ -39,6 +56,23 @@ &usdhc0 {
 };
 
 &iomuxc1 {
+	pinctrl_flexspi2_ptd: flexspi2ptdgrp {
+		fsl,pins = <
+
+			MX8ULP_PAD_PTD12__FLEXSPI2_A_SS0_B	0x42
+			MX8ULP_PAD_PTD13__FLEXSPI2_A_SCLK	0x42
+			MX8ULP_PAD_PTD14__FLEXSPI2_A_DATA3	0x42
+			MX8ULP_PAD_PTD15__FLEXSPI2_A_DATA2	0x42
+			MX8ULP_PAD_PTD16__FLEXSPI2_A_DATA1	0x42
+			MX8ULP_PAD_PTD17__FLEXSPI2_A_DATA0	0x42
+			MX8ULP_PAD_PTD18__FLEXSPI2_A_DQS	0x42
+			MX8ULP_PAD_PTD19__FLEXSPI2_A_DATA7	0x42
+			MX8ULP_PAD_PTD20__FLEXSPI2_A_DATA6	0x42
+			MX8ULP_PAD_PTD21__FLEXSPI2_A_DATA5	0x42
+			MX8ULP_PAD_PTD22__FLEXSPI2_A_DATA4	0x42
+		>;
+	};
+
 	pinctrl_lpuart5: lpuart5grp {
 		fsl,pins = <
 			MX8ULP_PAD_PTF14__LPUART5_TX	0x3
diff --git a/arch/arm64/boot/dts/freescale/imx8ulp.dtsi b/arch/arm64/boot/dts/freescale/imx8ulp.dtsi
index 09f7364dd1d0..4f92c4d6e8c0 100644
--- a/arch/arm64/boot/dts/freescale/imx8ulp.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8ulp.dtsi
@@ -141,7 +141,8 @@ soc@0 {
 		compatible = "simple-bus";
 		#address-cells = <1>;
 		#size-cells = <1>;
-		ranges = <0x0 0x0 0x0 0x40000000>;
+		ranges = <0x0 0x0 0x0 0x40000000>,
+			 <0x60000000 0x0 0x60000000 0x1000000>;
 
 		per_bridge3: bus@29000000 {
 			compatible = "simple-bus";
@@ -272,6 +273,23 @@ pcc4: clock-controller@29800000 {
 				#clock-cells = <1>;
 			};
 
+			flexspi2: spi@29810000 {
+				#address-cells = <1>;
+				#size-cells = <0>;
+				compatible = "nxp,imx8mm-fspi";
+				reg = <0x29810000 0x10000>, <0x60000000 0x10000000>;
+				reg-names = "fspi_base", "fspi_mmap";
+				interrupts = <GIC_SPI 141 IRQ_TYPE_LEVEL_HIGH>;
+				clocks = <&pcc4 IMX8ULP_CLK_FLEXSPI2>,
+					 <&pcc4 IMX8ULP_CLK_FLEXSPI2>;
+				clock-names = "fspi", "fspi_en";
+				assigned-clocks = <&pcc4 IMX8ULP_CLK_FLEXSPI2>;
+				assigned-clock-parents = <&cgc1 IMX8ULP_CLK_FROSC_DIV1>;
+				status = "disabled";
+			};
+
+
+
 			lpi2c6: i2c@29840000 {
 				compatible = "fsl,imx8ulp-lpi2c", "fsl,imx7ulp-lpi2c";
 				reg = <0x29840000 0x10000>;
-- 
2.25.1


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

[PATCH 11/11] arm64: dts: imx8qm/imx8qxp: add flexspi support

[haibo.chen]

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

Add flexspi support, and enable the 8 bit TX/RX for the
spi-nor device.

Signed-off-by: Haibo Chen <haibo.chen@nxp.com>
---
 .../boot/dts/freescale/imx8-ss-lsio.dtsi      | 17 ++++++++-
 arch/arm64/boot/dts/freescale/imx8qm-mek.dts  | 38 +++++++++++++++++++
 arch/arm64/boot/dts/freescale/imx8qxp-mek.dts | 38 +++++++++++++++++++
 3 files changed, 92 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/freescale/imx8-ss-lsio.dtsi b/arch/arm64/boot/dts/freescale/imx8-ss-lsio.dtsi
index 6446e6df7a9a..1f3d225e64ec 100644
--- a/arch/arm64/boot/dts/freescale/imx8-ss-lsio.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8-ss-lsio.dtsi
@@ -11,7 +11,8 @@ lsio_subsys: bus@5d000000 {
 	compatible = "simple-bus";
 	#address-cells = <1>;
 	#size-cells = <1>;
-	ranges = <0x5d000000 0x0 0x5d000000 0x1000000>;
+	ranges = <0x5d000000 0x0 0x5d000000 0x1000000>,
+		 <0x08000000 0x0 0x08000000 0x10000000>;
 
 	lsio_mem_clk: clock-lsio-mem {
 		compatible = "fixed-clock";
@@ -107,6 +108,20 @@ lsio_gpio7: gpio@5d0f0000 {
 		power-domains = <&pd IMX_SC_R_GPIO_7>;
 	};
 
+	flexspi0: spi@5d120000 {
+		#address-cells = <1>;
+		#size-cells = <0>;
+		compatible = "nxp,imx8qxp-fspi";
+		reg = <0x5d120000 0x10000>, <0x08000000 0x10000000>;
+		reg-names = "fspi_base", "fspi_mmap";
+		interrupts = <GIC_SPI 92 IRQ_TYPE_LEVEL_HIGH>;
+		clocks = <&clk IMX_SC_R_FSPI_0 IMX_SC_PM_CLK_PER>,
+			 <&clk IMX_SC_R_FSPI_0 IMX_SC_PM_CLK_PER>;
+		clock-names = "fspi", "fspi_en";
+		power-domains = <&pd IMX_SC_R_FSPI_0>;
+		status = "disabled";
+	};
+
 	lsio_mu0: mailbox@5d1b0000 {
 		reg = <0x5d1b0000 0x10000>;
 		interrupts = <GIC_SPI 176 IRQ_TYPE_LEVEL_HIGH>;
diff --git a/arch/arm64/boot/dts/freescale/imx8qm-mek.dts b/arch/arm64/boot/dts/freescale/imx8qm-mek.dts
index ce9d3f0b98fc..ce78f494c7d8 100644
--- a/arch/arm64/boot/dts/freescale/imx8qm-mek.dts
+++ b/arch/arm64/boot/dts/freescale/imx8qm-mek.dts
@@ -67,6 +67,23 @@ ethphy1: ethernet-phy@1 {
 	};
 };
 
+&flexspi0 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&pinctrl_flexspi0>;
+	nxp,fspi-dll-slvdly = <4>;
+	status = "okay";
+
+	flash0: mt35xu512aba@0 {
+		reg = <0>;
+		#address-cells = <1>;
+		#size-cells = <1>;
+		compatible = "jedec,spi-nor";
+		spi-max-frequency = <133000000>;
+		spi-tx-bus-width = <8>;
+		spi-rx-bus-width = <8>;
+	};
+};
+
 &usdhc1 {
 	pinctrl-names = "default";
 	pinctrl-0 = <&pinctrl_usdhc1>;
@@ -107,6 +124,27 @@ IMX8QM_ENET0_RGMII_RXD3_CONN_ENET0_RGMII_RXD3		0x06000020
 		>;
 	};
 
+	pinctrl_flexspi0: flexspi0grp {
+		fsl,pins = <
+			IMX8QM_QSPI0A_DATA0_LSIO_QSPI0A_DATA0     0x06000021
+			IMX8QM_QSPI0A_DATA1_LSIO_QSPI0A_DATA1     0x06000021
+			IMX8QM_QSPI0A_DATA2_LSIO_QSPI0A_DATA2     0x06000021
+			IMX8QM_QSPI0A_DATA3_LSIO_QSPI0A_DATA3     0x06000021
+			IMX8QM_QSPI0A_DQS_LSIO_QSPI0A_DQS         0x06000021
+			IMX8QM_QSPI0A_SS0_B_LSIO_QSPI0A_SS0_B     0x06000021
+			IMX8QM_QSPI0A_SS1_B_LSIO_QSPI0A_SS1_B     0x06000021
+			IMX8QM_QSPI0A_SCLK_LSIO_QSPI0A_SCLK       0x06000021
+			IMX8QM_QSPI0B_SCLK_LSIO_QSPI0B_SCLK       0x06000021
+			IMX8QM_QSPI0B_DATA0_LSIO_QSPI0B_DATA0     0x06000021
+			IMX8QM_QSPI0B_DATA1_LSIO_QSPI0B_DATA1     0x06000021
+			IMX8QM_QSPI0B_DATA2_LSIO_QSPI0B_DATA2     0x06000021
+			IMX8QM_QSPI0B_DATA3_LSIO_QSPI0B_DATA3     0x06000021
+			IMX8QM_QSPI0B_DQS_LSIO_QSPI0B_DQS         0x06000021
+			IMX8QM_QSPI0B_SS0_B_LSIO_QSPI0B_SS0_B     0x06000021
+			IMX8QM_QSPI0B_SS1_B_LSIO_QSPI0B_SS1_B     0x06000021
+		>;
+	};
+
 	pinctrl_lpuart0: lpuart0grp {
 		fsl,pins = <
 			IMX8QM_UART0_RX_DMA_UART0_RX				0x06000020
diff --git a/arch/arm64/boot/dts/freescale/imx8qxp-mek.dts b/arch/arm64/boot/dts/freescale/imx8qxp-mek.dts
index 07d8dd8160f6..15d13a9d545f 100644
--- a/arch/arm64/boot/dts/freescale/imx8qxp-mek.dts
+++ b/arch/arm64/boot/dts/freescale/imx8qxp-mek.dts
@@ -53,6 +53,23 @@ ethphy0: ethernet-phy@0 {
 	};
 };
 
+&flexspi0 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&pinctrl_flexspi0>;
+	nxp,fspi-dll-slvdly = <4>;
+	status = "okay";
+
+	flash0: mt35xu512aba@0 {
+		reg = <0>;
+		#address-cells = <1>;
+		#size-cells = <1>;
+		compatible = "jedec,spi-nor";
+		spi-max-frequency = <133000000>;
+		spi-tx-bus-width = <8>;
+		spi-rx-bus-width = <8>;
+	};
+};
+
 &i2c1 {
 	#address-cells = <1>;
 	#size-cells = <0>;
@@ -241,6 +258,27 @@ IMX8QXP_ENET0_RGMII_RXD3_CONN_ENET0_RGMII_RXD3		0x06000020
 		>;
 	};
 
+	pinctrl_flexspi0: flexspi0grp {
+		fsl,pins = <
+			IMX8QXP_QSPI0A_DATA0_LSIO_QSPI0A_DATA0     0x06000021
+			IMX8QXP_QSPI0A_DATA1_LSIO_QSPI0A_DATA1     0x06000021
+			IMX8QXP_QSPI0A_DATA2_LSIO_QSPI0A_DATA2     0x06000021
+			IMX8QXP_QSPI0A_DATA3_LSIO_QSPI0A_DATA3     0x06000021
+			IMX8QXP_QSPI0A_DQS_LSIO_QSPI0A_DQS         0x06000021
+			IMX8QXP_QSPI0A_SS0_B_LSIO_QSPI0A_SS0_B     0x06000021
+			IMX8QXP_QSPI0A_SS1_B_LSIO_QSPI0A_SS1_B     0x06000021
+			IMX8QXP_QSPI0A_SCLK_LSIO_QSPI0A_SCLK       0x06000021
+			IMX8QXP_QSPI0B_SCLK_LSIO_QSPI0B_SCLK       0x06000021
+			IMX8QXP_QSPI0B_DATA0_LSIO_QSPI0B_DATA0     0x06000021
+			IMX8QXP_QSPI0B_DATA1_LSIO_QSPI0B_DATA1     0x06000021
+			IMX8QXP_QSPI0B_DATA2_LSIO_QSPI0B_DATA2     0x06000021
+			IMX8QXP_QSPI0B_DATA3_LSIO_QSPI0B_DATA3     0x06000021
+			IMX8QXP_QSPI0B_DQS_LSIO_QSPI0B_DQS         0x06000021
+			IMX8QXP_QSPI0B_SS0_B_LSIO_QSPI0B_SS0_B     0x06000021
+			IMX8QXP_QSPI0B_SS1_B_LSIO_QSPI0B_SS1_B     0x06000021
+		>;
+	};
+
 	pinctrl_ioexp_rst: ioexprstgrp {
 		fsl,pins = <
 			IMX8QXP_SPI2_SDO_LSIO_GPIO1_IO01			0x06000021
-- 
2.25.1


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH 01/11] spi: spi-nxp-fspi: enable runtime pm for fspi

[Michael Walle]

Am 2022-07-05 11:11, schrieb haibo.chen@nxp.com:
> From: Haibo Chen <haibo.chen@nxp.com>
> 
> Enable the runtime PM in fspi driver.
> Also for system PM, On some board like i.MX8ULP-EVK board,
> after system suspend, IOMUX module will lost power, so all
> the pinctrl setting will lost when system resume back, need
> driver to save/restore the pinctrl setting.

On a side note: The mails to Ashish Kumar bounces. He is currently
listed as the maintainer for the FlexSPI driver. Will someone from
NXP take over?

-michael

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

RE: [PATCH 01/11] spi: spi-nxp-fspi: enable runtime pm for fspi

[Han Xu]

>-----Original Message-----
>From: Michael Walle <michael@walle.cc>
>Sent: Tuesday, July 5, 2022 9:01 AM
>To: Bough Chen <haibo.chen@nxp.com>
>Cc: ashish.kumar@nxp.com; yogeshgaur.83@gmail.com; broonie@kernel.org;
>robh+dt@kernel.org; krzysztof.kozlowski+dt@linaro.org; Han Xu
><han.xu@nxp.com>; singh.kuldeep87k@gmail.com;
>tudor.ambarus@microchip.com; p.yadav@ti.com; miquel.raynal@bootlin.com;
>richard@nod.at; vigneshr@ti.com; shawnguo@kernel.org;
>s.hauer@pengutronix.de; kernel@pengutronix.de; linux-spi@vger.kernel.org;
>linux-kernel@vger.kernel.org; devicetree@vger.kernel.org; linux-
>mtd@lists.infradead.org; festevam@gmail.com; dl-linux-imx <linux-
>imx@nxp.com>; linux-arm-kernel@lists.infradead.org; zhengxunli@mxic.com.tw
>Subject: Re: [PATCH 01/11] spi: spi-nxp-fspi: enable runtime pm for fspi
>
>Am 2022-07-05 11:11, schrieb haibo.chen@nxp.com:
>> From: Haibo Chen <haibo.chen@nxp.com>
>>
>> Enable the runtime PM in fspi driver.
>> Also for system PM, On some board like i.MX8ULP-EVK board, after
>> system suspend, IOMUX module will lost power, so all the pinctrl
>> setting will lost when system resume back, need driver to save/restore
>> the pinctrl setting.
>
>On a side note: The mails to Ashish Kumar bounces. He is currently listed as the
>maintainer for the FlexSPI driver. Will someone from NXP take over?

I will take it over.

>
>-michael

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/