[PATCH 0/3] NXP Flexspi driver patches

[PATCH 0/3] NXP Flexspi driver patches

[Kuldeep Singh]

Hi Mark,
This patchset involves changes in NXP Flexspi driver.
Please note, though all patches might not be related to each other, I
have clubbed them so as to make easy application of series.

Patch2 is dependent on dt-bindings
https://patchwork.kernel.org/project/spi-devel-general/patch/1614593046-23832-1-git-send-email-kuldeep.singh@nxp.com/

Patch1: Enables IP read in flexspi
Patch2: Add imx8dxl support
Patch3: Errata workaround for LS1028A

Han Xu (1):
  spi: spi-nxp-fspi: Add driver support for imx8dxl

Kuldeep Singh (2):
  spi: spi-nxp-fspi: Add support for IP read only
  spi: spi-nxp-fspi: Implement errata workaround for LS1028A

 drivers/spi/spi-nxp-fspi.c | 114 +++++++++++++++++++++++++++++++++----
 1 file changed, 103 insertions(+), 11 deletions(-)

-- 
2.25.1

[PATCH 1/3] spi: spi-nxp-fspi: Add support for IP read only

[Kuldeep Singh]

Add support for disabling AHB bus and read entire flash contents via IP
only. Please note, this enables IP bus read using a quirk which can be
enabled directly in device-type data or in existence of an errata where
AHB bus may need to be disabled.

Signed-off-by: Kuldeep Singh <kuldeep.singh@nxp.com>
---
 drivers/spi/spi-nxp-fspi.c | 28 ++++++++++++++++++++++------
 1 file changed, 22 insertions(+), 6 deletions(-)

diff --git a/drivers/spi/spi-nxp-fspi.c b/drivers/spi/spi-nxp-fspi.c
index ab9035662717..829391e20e92 100644
--- a/drivers/spi/spi-nxp-fspi.c
+++ b/drivers/spi/spi-nxp-fspi.c
@@ -311,6 +311,9 @@
 #define NXP_FSPI_MAX_CHIPSELECT		4
 #define NXP_FSPI_MIN_IOMAP	SZ_4M
 
+/* Access flash memory using IP bus only */
+#define FSPI_QUIRK_USE_IP_ONLY	BIT(0)
+
 struct nxp_fspi_devtype_data {
 	unsigned int rxfifo;
 	unsigned int txfifo;
@@ -359,6 +362,11 @@ struct nxp_fspi {
 	int selected;
 };
 
+static inline int needs_ip_only(struct nxp_fspi *f)
+{
+	return f->devtype_data->quirks & FSPI_QUIRK_USE_IP_ONLY;
+}
+
 /*
  * R/W functions for big- or little-endian registers:
  * The FSPI controller's endianness is independent of
@@ -553,8 +561,8 @@ static void nxp_fspi_prepare_lut(struct nxp_fspi *f,
 	for (i = 0; i < ARRAY_SIZE(lutval); i++)
 		fspi_writel(f, lutval[i], base + FSPI_LUT_REG(i));
 
-	dev_dbg(f->dev, "CMD[%x] lutval[0:%x \t 1:%x \t 2:%x \t 3:%x]\n",
-		op->cmd.opcode, lutval[0], lutval[1], lutval[2], lutval[3]);
+	dev_dbg(f->dev, "CMD[%x] lutval[0:%x \t 1:%x \t 2:%x \t 3:%x], size: 0x%08x\n",
+		op->cmd.opcode, lutval[0], lutval[1], lutval[2], lutval[3], op->data.nbytes);
 
 	/* lock LUT */
 	fspi_writel(f, FSPI_LUTKEY_VALUE, f->iobase + FSPI_LUTKEY);
@@ -852,12 +860,14 @@ static int nxp_fspi_exec_op(struct spi_mem *mem, const struct spi_mem_op *op)
 
 	nxp_fspi_prepare_lut(f, op);
 	/*
-	 * If we have large chunks of data, we read them through the AHB bus
-	 * by accessing the mapped memory. In all other cases we use
-	 * IP commands to access the flash.
+	 * If we have large chunks of data, we read them through the AHB bus by
+	 * accessing the mapped memory. In all other cases we use IP commands
+	 * to access the flash. Read via AHB bus may be corrupted due to
+	 * existence of an errata and therefore discard AHB read in such cases.
 	 */
 	if (op->data.nbytes > (f->devtype_data->rxfifo - 4) &&
-	    op->data.dir == SPI_MEM_DATA_IN) {
+	    op->data.dir == SPI_MEM_DATA_IN &&
+	    !needs_ip_only(f)) {
 		err = nxp_fspi_read_ahb(f, op);
 	} else {
 		if (op->data.nbytes && op->data.dir == SPI_MEM_DATA_OUT)
@@ -888,6 +898,12 @@ static int nxp_fspi_adjust_op_size(struct spi_mem *mem, struct spi_mem_op *op)
 			op->data.nbytes = ALIGN_DOWN(op->data.nbytes, 8);
 	}
 
+	/* Limit data bytes to RX FIFO in case of IP read only */
+	if (op->data.dir == SPI_MEM_DATA_IN &&
+	    needs_ip_only(f) &&
+	    op->data.nbytes > f->devtype_data->rxfifo)
+		op->data.nbytes = f->devtype_data->rxfifo;
+
 	return 0;
 }
 
-- 
2.25.1

[PATCH 2/3] spi: spi-nxp-fspi: Add driver support for imx8dxl

[Kuldeep Singh]

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

Add driver support for imx8dxl which support read through IP bus only
and disable AHB bus due to an IC errata. Use the pre-defined quirk
FSPI_QUIRK_USE_IP_ONLY directly in device-type data to disable AHB read.

Signed-off-by: Han Xu <han.xu@nxp.com>
Signed-off-by: Kuldeep Singh <kuldeep.singh@nxp.com>
---
Hi Mark,
This patch is dependent on
https://patchwork.kernel.org/project/spi-devel-general/patch/1614593046-23832-1-git-send-email-kuldeep.singh@nxp.com/

 drivers/spi/spi-nxp-fspi.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/spi/spi-nxp-fspi.c b/drivers/spi/spi-nxp-fspi.c
index 829391e20e92..80a9278d91ed 100644
--- a/drivers/spi/spi-nxp-fspi.c
+++ b/drivers/spi/spi-nxp-fspi.c
@@ -346,6 +346,14 @@ static const struct nxp_fspi_devtype_data imx8qxp_data = {
 	.little_endian = true,  /* little-endian    */
 };
 
+static const struct nxp_fspi_devtype_data imx8dxl_data = {
+	.rxfifo = SZ_512,       /* (64  * 64 bits)  */
+	.txfifo = SZ_1K,        /* (128 * 64 bits)  */
+	.ahb_buf_size = SZ_2K,  /* (256 * 64 bits)  */
+	.quirks = FSPI_QUIRK_USE_IP_ONLY,
+	.little_endian = true,  /* little-endian    */
+};
+
 struct nxp_fspi {
 	void __iomem *iobase;
 	void __iomem *ahb_addr;
@@ -1168,6 +1176,7 @@ 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, },
 	{ .compatible = "nxp,imx8qxp-fspi", .data = (void *)&imx8qxp_data, },
+	{ .compatible = "nxp,imx8dxl-fspi", .data = (void *)&imx8dxl_data, },
 	{ /* sentinel */ }
 };
 MODULE_DEVICE_TABLE(of, nxp_fspi_dt_ids);
-- 
2.25.1

Re: [PATCH 2/3] spi: spi-nxp-fspi: Add driver support for imx8dxl

[Mark Brown]

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

On Mon, Mar 01, 2021 at 04:02:29PM +0530, Kuldeep Singh wrote:

> This patch is dependent on
> https://patchwork.kernel.org/project/spi-devel-general/patch/1614593046-23832-1-git-send-email-kuldeep.singh@nxp.com/

Please include human readable descriptions of things like commits and
issues being discussed in e-mail in your mails, this makes them much
easier for humans to read especially when they have no internet access.
I do frequently catch up on my mail on flights or while otherwise
travelling so this is even more pressing for me than just being about
making things a bit easier to read.

That's a DT binding YAML conversion patch, why would there be a
dependency on it?

> 
>  drivers/spi/spi-nxp-fspi.c | 9 +++++++++
>  1 file changed, 9 insertions(+)

This needs a DT binding update adding the new compatible.

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

RE: [EXT] Re: [PATCH 2/3] spi: spi-nxp-fspi: Add driver support for imx8dxl

[Kuldeep Singh]

Hi Mark,

> -----Original Message-----
> From: Mark Brown <broonie@kernel.org>
> Sent: Monday, March 1, 2021 6:56 PM
> To: Kuldeep Singh <kuldeep.singh@nxp.com>
> Cc: linux-spi@vger.kernel.org; linux-kernel@vger.kernel.org; Ashish Kumar
> <ashish.kumar@nxp.com>; Han Xu <han.xu@nxp.com>
> Subject: [EXT] Re: [PATCH 2/3] spi: spi-nxp-fspi: Add driver support for imx8dxl
> 
> On Mon, Mar 01, 2021 at 04:02:29PM +0530, Kuldeep Singh wrote:
> 
> > This patch is dependent on
> > https://patchwork.kernel.org/project/spi-devel-general/patch/161459304
> > 6-23832-1-git-send-email-kuldeep.singh@nxp.com/
> 
> Please include human readable descriptions of things like commits and issues
> being discussed in e-mail in your mails, this makes them much easier for
> humans to read especially when they have no internet access.
> I do frequently catch up on my mail on flights or while otherwise travelling so
> this is even more pressing for me than just being about making things a bit
> easier to read.
> 
> That's a DT binding YAML conversion patch, why would there be a
> dependency on it?

I have converted bindings to yaml version in the patch and also added imx8dxl compatible along-with the conversion. Please see the difference in compatible entries from txt to yaml conversion[1].
Kindly let me know do I need to submit different patch for adding new compatible or ok to include in the conversion patch itself?

[1]
Documentation/devicetree/bindings/spi/nxp,spi-nxp-fspi.yaml
+properties:
+  compatible:
+    enum:
+      - nxp,lx2160a-fspi
+      - nxp,imx8qxp-fspi
+      - nxp,imx8mm-fspi
+      - nxp,imx8dxl-fspi

Documentation/devicetree/bindings/spi/spi-nxp-fspi.txt
-Required properties:
-  - compatible : Should be "nxp,lx2160a-fspi"
-			    "nxp,imx8qxp-fspi"
-			    "nxp,imx8mm-fspi"

> 
> >
> >  drivers/spi/spi-nxp-fspi.c | 9 +++++++++
> >  1 file changed, 9 insertions(+)
> 
> This needs a DT binding update adding the new compatible.

Please see above for comments.

Thanks
Kuldeep

Re: [EXT] Re: [PATCH 2/3] spi: spi-nxp-fspi: Add driver support for imx8dxl

[Mark Brown]

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

On Mon, Mar 01, 2021 at 04:39:18PM +0000, Kuldeep Singh wrote:

> > That's a DT binding YAML conversion patch, why would there be a
> > dependency on it?

> I have converted bindings to yaml version in the patch and also added imx8dxl compatible along-with the conversion. Please see the difference in compatible entries from txt to yaml conversion[1].
> Kindly let me know do I need to submit different patch for adding new compatible or ok to include in the conversion patch itself?

Your YAML binding conversion should have been a standalone patch only
doing that conversion, the new compatible should have been split out and
gone first - each patch should only do one thing as covered in
submitting-patches.rst.  As things stand the changelog for the
conversion is misleading since it doesn't mention the new compatible.

Please fix your mail client to word wrap within paragraphs at something
substantially less than 80 columns.  Doing this makes your messages much
easier to read and reply to.

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

RE: [EXT] Re: [PATCH 2/3] spi: spi-nxp-fspi: Add driver support for imx8dxl

[Kuldeep Singh]

> > I have converted bindings to yaml version in the patch and also added
> imx8dxl compatible along-with the conversion. Please see the difference in
> compatible entries from txt to yaml conversion[1].
> > Kindly let me know do I need to submit different patch for adding new
> compatible or ok to include in the conversion patch itself?
> 
> Your YAML binding conversion should have been a standalone patch only
> doing that conversion, the new compatible should have been split out and
> gone first - each patch should only do one thing as covered in submitting-
> patches.rst.  As things stand the changelog for the conversion is misleading
> since it doesn't mention the new compatible.

Thanks Mark for mentioning.
I will send compatible adding patch with this series and post yaml conversion later.

Regards
Kuldeep

[PATCH 3/3] spi: spi-nxp-fspi: Implement errata workaround for LS1028A

[Kuldeep Singh]

Errata ERR050568 description says that "Flash access by FlexSPI AHB
command may not work with platform frequency equal to 300 MHz" on
LS1028A.

By default, smaller length reads(equal to RX FIFO size) are done by IP
bus and larger length reads using AHB bus. For adding errata workaround,
use IP bus to read entire flash contents and disable AHB path when
platform frequency is 300Mhz.

Signed-off-by: Kuldeep Singh <kuldeep.singh@nxp.com>
---
 drivers/spi/spi-nxp-fspi.c | 79 +++++++++++++++++++++++++++++++++++---
 1 file changed, 73 insertions(+), 6 deletions(-)

diff --git a/drivers/spi/spi-nxp-fspi.c b/drivers/spi/spi-nxp-fspi.c
index 80a9278d91ed..ed1e888c7ece 100644
--- a/drivers/spi/spi-nxp-fspi.c
+++ b/drivers/spi/spi-nxp-fspi.c
@@ -49,8 +49,11 @@
 #include <linux/of_device.h>
 #include <linux/platform_device.h>
 #include <linux/pm_qos.h>
+#include <linux/regmap.h>
 #include <linux/sizes.h>
+#include <linux/sys_soc.h>
 
+#include <linux/mfd/syscon.h>
 #include <linux/spi/spi.h>
 #include <linux/spi/spi-mem.h>
 
@@ -311,6 +314,8 @@
 #define NXP_FSPI_MAX_CHIPSELECT		4
 #define NXP_FSPI_MIN_IOMAP	SZ_4M
 
+#define DCFG_RCWSR1		0x100
+
 /* Access flash memory using IP bus only */
 #define FSPI_QUIRK_USE_IP_ONLY	BIT(0)
 
@@ -322,7 +327,7 @@ struct nxp_fspi_devtype_data {
 	bool little_endian;
 };
 
-static const struct nxp_fspi_devtype_data lx2160a_data = {
+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)  */
@@ -330,7 +335,7 @@ static const struct nxp_fspi_devtype_data lx2160a_data = {
 	.little_endian = true,  /* little-endian    */
 };
 
-static const struct nxp_fspi_devtype_data imx8mm_data = {
+static struct nxp_fspi_devtype_data imx8mm_data = {
 	.rxfifo = SZ_512,       /* (64  * 64 bits)  */
 	.txfifo = SZ_1K,        /* (128 * 64 bits)  */
 	.ahb_buf_size = SZ_2K,  /* (256 * 64 bits)  */
@@ -338,7 +343,7 @@ static const struct nxp_fspi_devtype_data imx8mm_data = {
 	.little_endian = true,  /* little-endian    */
 };
 
-static const struct nxp_fspi_devtype_data imx8qxp_data = {
+static struct nxp_fspi_devtype_data imx8qxp_data = {
 	.rxfifo = SZ_512,       /* (64  * 64 bits)  */
 	.txfifo = SZ_1K,        /* (128 * 64 bits)  */
 	.ahb_buf_size = SZ_2K,  /* (256 * 64 bits)  */
@@ -346,7 +351,7 @@ static const struct nxp_fspi_devtype_data imx8qxp_data = {
 	.little_endian = true,  /* little-endian    */
 };
 
-static const struct nxp_fspi_devtype_data imx8dxl_data = {
+static struct nxp_fspi_devtype_data imx8dxl_data = {
 	.rxfifo = SZ_512,       /* (64  * 64 bits)  */
 	.txfifo = SZ_1K,        /* (128 * 64 bits)  */
 	.ahb_buf_size = SZ_2K,  /* (256 * 64 bits)  */
@@ -364,7 +369,7 @@ struct nxp_fspi {
 	struct clk *clk, *clk_en;
 	struct device *dev;
 	struct completion c;
-	const struct nxp_fspi_devtype_data *devtype_data;
+	struct nxp_fspi_devtype_data *devtype_data;
 	struct mutex lock;
 	struct pm_qos_request pm_qos_req;
 	int selected;
@@ -915,6 +920,59 @@ static int nxp_fspi_adjust_op_size(struct spi_mem *mem, struct spi_mem_op *op)
 	return 0;
 }
 
+static void erratum_err050568(struct nxp_fspi *f)
+{
+	const struct soc_device_attribute ls1028a_soc_attr[] = {
+		{ .family = "QorIQ LS1028A" },
+		{ /* sentinel */ }
+	};
+	struct device_node *np;
+	struct regmap *map;
+	u32 val = 0, sysclk = 0;
+	int ret;
+
+	/* Check for LS1028A family */
+	if (!soc_device_match(ls1028a_soc_attr)) {
+		dev_dbg(f->dev, "Errata applicable only for LS1028A\n");
+		return;
+	}
+
+	/* Compute system clock frequency multiplier ratio */
+	map = syscon_regmap_lookup_by_compatible("fsl,ls1028a-dcfg");
+	if (IS_ERR(map)) {
+		dev_err(f->dev, "No syscon regmap\n");
+		goto err;
+	}
+
+	ret = regmap_read(map, DCFG_RCWSR1, &val);
+	if (ret < 0)
+		goto err;
+
+	/* Strap bits 6:2 define SYS_PLL_RAT i.e frequency multiplier ratio */
+	val = (val >> 2) & 0x1F;
+	WARN(val == 0, "Strapping is zero: Cannot determine ratio");
+
+	/* Compute system clock frequency */
+	np = of_find_node_by_name(NULL, "clock-sysclk");
+	if (!np)
+		goto err;
+
+	if (of_property_read_u32(np, "clock-frequency", &sysclk))
+		goto err;
+
+	sysclk = (sysclk * val) / 1000000; /* Convert sysclk to Mhz */
+	dev_dbg(f->dev, "val: 0x%08x, sysclk: %dMhz\n", val, sysclk);
+
+	/* Use IP bus only if PLL is 300MHz */
+	if (sysclk == 300)
+		f->devtype_data->quirks |= FSPI_QUIRK_USE_IP_ONLY;
+
+	return;
+
+err:
+	dev_err(f->dev, "Errata cannot be executed. Read via IP bus may not work\n");
+}
+
 static int nxp_fspi_default_setup(struct nxp_fspi *f)
 {
 	void __iomem *base = f->iobase;
@@ -933,6 +991,15 @@ static int nxp_fspi_default_setup(struct nxp_fspi *f)
 	if (ret)
 		return ret;
 
+	/*
+	 * ERR050568: Flash access by FlexSPI AHB command may not work with
+	 * platform frequency equal to 300 MHz on LS1028A.
+	 * LS1028A reuses LX2160A compatible entry. Make errata applicable for
+	 * Layerscape LS1028A platform.
+	 */
+	if (of_device_is_compatible(f->dev->of_node, "nxp,lx2160a-fspi"))
+		erratum_err050568(f);
+
 	/* Reset the module */
 	/* w1c register, wait unit clear */
 	ret = fspi_readl_poll_tout(f, f->iobase + FSPI_MCR0,
@@ -1036,7 +1103,7 @@ static int nxp_fspi_probe(struct platform_device *pdev)
 
 	f = spi_controller_get_devdata(ctlr);
 	f->dev = dev;
-	f->devtype_data = device_get_match_data(dev);
+	f->devtype_data = (struct nxp_fspi_devtype_data *)device_get_match_data(dev);
 	if (!f->devtype_data) {
 		ret = -ENODEV;
 		goto err_put_ctrl;
-- 
2.25.1