[PATCH 0/3] dynamically alloc AHB memory

[PATCH 0/3] dynamically alloc AHB memory

[han.xu]

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

i.MX platforms reserved 256M memory area for FSPI/QSPI AHB memory, it
may failed to alloc all of them at once on some platforms. These patches
allow the controller alloc AHB as needed.

i.MX7D RX FIFO should be 128B rather than 512B.

Han Xu (3):
  spi: spi-nxp-fspi: dynamically alloc AHB memory for FSPI
  spi: spi-fsl-qspi: change i.MX7D RX FIFO size
  spi: spi-fsl-qspi: dynamically alloc AHB memory for QSPI

 drivers/spi/spi-fsl-qspi.c | 60 +++++++++++++++++++++++++++-----------
 drivers/spi/spi-nxp-fspi.c | 39 ++++++++++++++++++++-----
 2 files changed, 74 insertions(+), 25 deletions(-)

-- 
2.17.1

[PATCH 1/3] spi: spi-nxp-fspi: dynamically alloc AHB memory for FSPI

[han.xu]

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

dynamically alloc AHB memory for FSPI controller

Signed-off-by: Han Xu <han.xu@nxp.com>
---
 drivers/spi/spi-nxp-fspi.c | 39 ++++++++++++++++++++++++++++++--------
 1 file changed, 31 insertions(+), 8 deletions(-)

diff --git a/drivers/spi/spi-nxp-fspi.c b/drivers/spi/spi-nxp-fspi.c
index 8894f98cc99c..84edaca28e01 100644
--- a/drivers/spi/spi-nxp-fspi.c
+++ b/drivers/spi/spi-nxp-fspi.c
@@ -307,6 +307,7 @@
 
 #define POLL_TOUT		5000
 #define NXP_FSPI_MAX_CHIPSELECT		4
+#define NXP_FSPI_MIN_IOMAP	SZ_4M
 
 struct nxp_fspi_devtype_data {
 	unsigned int rxfifo;
@@ -329,6 +330,8 @@ struct nxp_fspi {
 	void __iomem *ahb_addr;
 	u32 memmap_phy;
 	u32 memmap_phy_size;
+	u32 memmap_start;
+	u32 memmap_len;
 	struct clk *clk, *clk_en;
 	struct device *dev;
 	struct completion c;
@@ -641,12 +644,34 @@ static void nxp_fspi_select_mem(struct nxp_fspi *f, struct spi_device *spi)
 	f->selected = spi->chip_select;
 }
 
-static void nxp_fspi_read_ahb(struct nxp_fspi *f, const struct spi_mem_op *op)
+static int nxp_fspi_read_ahb(struct nxp_fspi *f, const struct spi_mem_op *op)
 {
+	u32 start = op->addr.val;
 	u32 len = op->data.nbytes;
 
+	/* if necessary, ioremap before AHB read */
+	if ((!f->ahb_addr) || start < f->memmap_start ||
+	    start + len > f->memmap_start + f->memmap_len) {
+		if (f->ahb_addr)
+			iounmap(f->ahb_addr);
+
+		f->memmap_start = start;
+		f->memmap_len = len > NXP_FSPI_MIN_IOMAP ?
+				len : NXP_FSPI_MIN_IOMAP;
+
+		f->ahb_addr = ioremap_wc(f->memmap_phy + f->memmap_start,
+					 f->memmap_len);
+		if (!f->ahb_addr) {
+			dev_err(f->dev, "failed to alloc memory\n");
+			return -ENOMEM;
+		}
+	}
+
 	/* Read out the data directly from the AHB buffer. */
-	memcpy_fromio(op->data.buf.in, (f->ahb_addr + op->addr.val), len);
+	memcpy_fromio(op->data.buf.in,
+		      (f->ahb_addr + start - f->memmap_start), len);
+
+	return 0;
 }
 
 static void nxp_fspi_fill_txfifo(struct nxp_fspi *f,
@@ -806,7 +831,7 @@ static int nxp_fspi_exec_op(struct spi_mem *mem, const struct spi_mem_op *op)
 	 */
 	if (op->data.nbytes > (f->devtype_data->rxfifo - 4) &&
 	    op->data.dir == SPI_MEM_DATA_IN) {
-		nxp_fspi_read_ahb(f, op);
+		err = nxp_fspi_read_ahb(f, op);
 	} else {
 		if (op->data.nbytes && op->data.dir == SPI_MEM_DATA_OUT)
 			nxp_fspi_fill_txfifo(f, op);
@@ -976,11 +1001,6 @@ static int nxp_fspi_probe(struct platform_device *pdev)
 
 	/* find the resources - controller memory mapped space */
 	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "fspi_mmap");
-	f->ahb_addr = devm_ioremap_resource(dev, res);
-	if (IS_ERR(f->ahb_addr)) {
-		ret = PTR_ERR(f->ahb_addr);
-		goto err_put_ctrl;
-	}
 
 	/* assign memory mapped starting address and mapped size. */
 	f->memmap_phy = res->start;
@@ -1059,6 +1079,9 @@ static int nxp_fspi_remove(struct platform_device *pdev)
 
 	mutex_destroy(&f->lock);
 
+	if (f->ahb_addr)
+		iounmap(f->ahb_addr);
+
 	return 0;
 }
 
-- 
2.17.1

Re: [PATCH 1/3] spi: spi-nxp-fspi: dynamically alloc AHB memory for FSPI

[Mark Brown]

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

On Wed, Jul 10, 2019 at 10:31:26AM +0800, han.xu@nxp.com wrote:
> From: Han Xu <han.xu@nxp.com>
> 
> dynamically alloc AHB memory for FSPI controller

Why?  This is currently done at probe which is what you'd expect
to happen here, there's no explanation as to why this change is
being made.

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

RE: [EXT] Re: [PATCH 1/3] spi: spi-nxp-fspi: dynamically alloc AHB memory for FSPI

[Han Xu]

> -----Original Message-----
> From: Mark Brown <broonie@kernel.org>
> Sent: Wednesday, July 10, 2019 10:16 AM
> To: Han Xu <han.xu@nxp.com>
> Cc: Ashish Kumar <ashish.kumar@nxp.com>; linux-spi@vger.kernel.org; linux-
> kernel@vger.kernel.org; dl-linux-imx <linux-imx@nxp.com>
> Subject: [EXT] Re: [PATCH 1/3] spi: spi-nxp-fspi: dynamically alloc AHB memory
> for FSPI
> 
> On Wed, Jul 10, 2019 at 10:31:26AM +0800, han.xu@nxp.com wrote:
> > From: Han Xu <han.xu@nxp.com>
> >
> > dynamically alloc AHB memory for FSPI controller
> 
> Why?  This is currently done at probe which is what you'd expect to happen
> here, there's no explanation as to why this change is being made.

Explained in cover letter, It failed to alloc the whole memory mapping area during
probe on some platforms, since the AHB memory area could be pretty large. The
error may look like:

[    1.129404] fsl-quadspi 1550000.spi: ioremap failed for resource [mem 0x40000000-0x7fffffff]

Re: [EXT] Re: [PATCH 1/3] spi: spi-nxp-fspi: dynamically alloc AHB memory for FSPI

[Mark Brown]

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

On Wed, Jul 10, 2019 at 03:35:46PM +0000, Han Xu wrote:

> > > dynamically alloc AHB memory for FSPI controller

> > Why?  This is currently done at probe which is what you'd expect to happen
> > here, there's no explanation as to why this change is being made.

> Explained in cover letter, It failed to alloc the whole memory mapping area during
> probe on some platforms, since the AHB memory area could be pretty large. The
> error may look like:

> [    1.129404] fsl-quadspi 1550000.spi: ioremap failed for resource [mem 0x40000000-0x7fffffff]

The commit itself needs to have some explanation of what it's
doing so it's in the git log, particularly for something odd like
this.  More generally this just doesn't feel like it's solving
the problem - essentially we're just deferring the mapping and
then keep on failing operations until the allocation succeeds for
some reason.  That's going to be disruptive for users of the
device and it doesn't seem like it's going to be a robust
solution.  Why does the allocation not work initially and why is
it more likely to work later on?

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

RE: [EXT] Re: [PATCH 1/3] spi: spi-nxp-fspi: dynamically alloc AHB memory for FSPI

[Han Xu]

> -----Original Message-----
> From: Mark Brown <broonie@kernel.org>
> Sent: Thursday, July 11, 2019 7:41 AM
> To: Han Xu <han.xu@nxp.com>
> Cc: Ashish Kumar <ashish.kumar@nxp.com>; linux-spi@vger.kernel.org; linux-
> kernel@vger.kernel.org; dl-linux-imx <linux-imx@nxp.com>
> Subject: Re: [EXT] Re: [PATCH 1/3] spi: spi-nxp-fspi: dynamically alloc AHB memory
> for FSPI
> 
> On Wed, Jul 10, 2019 at 03:35:46PM +0000, Han Xu wrote:
> 
> > > > dynamically alloc AHB memory for FSPI controller
> 
> > > Why?  This is currently done at probe which is what you'd expect to
> > > happen here, there's no explanation as to why this change is being made.
> 
> > Explained in cover letter, It failed to alloc the whole memory mapping
> > area during probe on some platforms, since the AHB memory area could
> > be pretty large. The error may look like:
> 
> > [    1.129404] fsl-quadspi 1550000.spi: ioremap failed for resource [mem
> 0x40000000-0x7fffffff]
> 
> The commit itself needs to have some explanation of what it's doing so it's in the
> git log, particularly for something odd like this.  More generally this just doesn't feel
> like it's solving the problem - essentially we're just deferring the mapping and then
> keep on failing operations until the allocation succeeds for some reason.  That's
> going to be disruptive for users of the device and it doesn't seem like it's going to
> be a robust solution.  Why does the allocation not work initially and why is it more
> likely to work later on?

Yes, I will explain the reason in next version. To allocate the whole 256MB memory at one time exceed the vmalloc limit, so we dynamically allocate small amount of memory just as needed. There is no failing operation, just deferring and re-allocate if new access area beyond the previous allocate area.

[PATCH 2/3] spi: spi-fsl-qspi: change i.MX7D RX FIFO size

[han.xu]

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

The RX FIFO should be 128 byte rather than 512 byte. It's a typo on
reference manual.

Signed-off-by: Han Xu <han.xu@nxp.com>
---
 drivers/spi/spi-fsl-qspi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/spi/spi-fsl-qspi.c b/drivers/spi/spi-fsl-qspi.c
index 6a713f78a62e..1d08c60e5ae2 100644
--- a/drivers/spi/spi-fsl-qspi.c
+++ b/drivers/spi/spi-fsl-qspi.c
@@ -206,7 +206,7 @@ static const struct fsl_qspi_devtype_data imx6sx_data = {
 };
 
 static const struct fsl_qspi_devtype_data imx7d_data = {
-	.rxfifo = SZ_512,
+	.rxfifo = SZ_128,
 	.txfifo = SZ_512,
 	.ahb_buf_size = SZ_1K,
 	.quirks = QUADSPI_QUIRK_TKT253890 | QUADSPI_QUIRK_4X_INT_CLK,
-- 
2.17.1

Applied "spi: spi-fsl-qspi: change i.MX7D RX FIFO size" to the spi tree

[Mark Brown]

The patch

   spi: spi-fsl-qspi: change i.MX7D RX FIFO size

has been applied to the spi tree at

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-5.3

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.  

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

>From d6b197a14863818a7ed7890e91f043fab49e8c60 Mon Sep 17 00:00:00 2001
From: Han Xu <han.xu@nxp.com>
Date: Wed, 10 Jul 2019 10:31:27 +0800
Subject: [PATCH] spi: spi-fsl-qspi: change i.MX7D RX FIFO size

The RX FIFO should be 128 byte rather than 512 byte. It's a typo on
reference manual.

Signed-off-by: Han Xu <han.xu@nxp.com>
Link: https://lore.kernel.org/r/20190710023128.13115-3-han.xu@nxp.com
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 drivers/spi/spi-fsl-qspi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/spi/spi-fsl-qspi.c b/drivers/spi/spi-fsl-qspi.c
index 41a49b93ca60..448c00e4065b 100644
--- a/drivers/spi/spi-fsl-qspi.c
+++ b/drivers/spi/spi-fsl-qspi.c
@@ -206,7 +206,7 @@ static const struct fsl_qspi_devtype_data imx6sx_data = {
 };
 
 static const struct fsl_qspi_devtype_data imx7d_data = {
-	.rxfifo = SZ_512,
+	.rxfifo = SZ_128,
 	.txfifo = SZ_512,
 	.ahb_buf_size = SZ_1K,
 	.quirks = QUADSPI_QUIRK_TKT253890 | QUADSPI_QUIRK_4X_INT_CLK,
-- 
2.20.1

[PATCH 3/3] spi: spi-fsl-qspi: dynamically alloc AHB memory for QSPI

[han.xu]

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

dynamically alloc AHB memory for QSPI controller.

Signed-off-by: Han Xu <han.xu@nxp.com>
---
 drivers/spi/spi-fsl-qspi.c | 58 +++++++++++++++++++++++++++-----------
 1 file changed, 42 insertions(+), 16 deletions(-)

diff --git a/drivers/spi/spi-fsl-qspi.c b/drivers/spi/spi-fsl-qspi.c
index 1d08c60e5ae2..f7795e071eb6 100644
--- a/drivers/spi/spi-fsl-qspi.c
+++ b/drivers/spi/spi-fsl-qspi.c
@@ -181,6 +181,8 @@
  */
 #define QUADSPI_QUIRK_BASE_INTERNAL	BIT(4)
 
+#define QUADSPI_MIN_IOMAP		SZ_4M
+
 struct fsl_qspi_devtype_data {
 	unsigned int rxfifo;
 	unsigned int txfifo;
@@ -241,6 +243,9 @@ struct fsl_qspi {
 	void __iomem *iobase;
 	void __iomem *ahb_addr;
 	u32 memmap_phy;
+	u32 memmap_phy_size;
+	u32 memmap_start;
+	u32 memmap_len;
 	struct clk *clk, *clk_en;
 	struct device *dev;
 	struct completion c;
@@ -519,11 +524,34 @@ static void fsl_qspi_select_mem(struct fsl_qspi *q, struct spi_device *spi)
 	fsl_qspi_invalidate(q);
 }
 
-static void fsl_qspi_read_ahb(struct fsl_qspi *q, const struct spi_mem_op *op)
+static int fsl_qspi_read_ahb(struct fsl_qspi *q, const struct spi_mem_op *op)
 {
+	u32 start = op->addr.val + q->selected * q->memmap_phy_size;
+	u32 len = op->data.nbytes;
+
+	/* if necessary, ioremap before AHB read */
+	if ((!q->ahb_addr) || start < q->memmap_start ||
+	    start + len > q->memmap_start + q->memmap_len) {
+		if (q->ahb_addr) {
+			iounmap(q->ahb_addr);
+		}
+
+		q->memmap_start = start;
+		q->memmap_len = len > QUADSPI_MIN_IOMAP ?
+				len : QUADSPI_MIN_IOMAP;
+
+		q->ahb_addr = ioremap_wc(q->memmap_phy + q->memmap_start,
+					 q->memmap_len);
+		if (!q->ahb_addr) {
+			dev_err(q->dev, "failed to alloc memory\n");
+			return -ENOMEM;
+		}
+	}
+
 	memcpy_fromio(op->data.buf.in,
-		      q->ahb_addr + q->selected * q->devtype_data->ahb_buf_size,
-		      op->data.nbytes);
+		      q->ahb_addr + start - q->memmap_start, len);
+
+	return 0;
 }
 
 static void fsl_qspi_fill_txfifo(struct fsl_qspi *q,
@@ -647,7 +675,7 @@ static int fsl_qspi_exec_op(struct spi_mem *mem, const struct spi_mem_op *op)
 	 */
 	if (op->data.nbytes > (q->devtype_data->rxfifo - 4) &&
 	    op->data.dir == SPI_MEM_DATA_IN) {
-		fsl_qspi_read_ahb(q, op);
+		err = fsl_qspi_read_ahb(q, op);
 	} else {
 		qspi_writel(q, QUADSPI_RBCT_WMRK_MASK |
 			    QUADSPI_RBCT_RXBRD_USEIPS, base + QUADSPI_RBCT);
@@ -735,16 +763,16 @@ static int fsl_qspi_default_setup(struct fsl_qspi *q)
 	 * In HW there can be a maximum of four chips on two buses with
 	 * two chip selects on each bus. We use four chip selects in SW
 	 * to differentiate between the four chips.
-	 * We use ahb_buf_size for each chip and set SFA1AD, SFA2AD, SFB1AD,
-	 * SFB2AD accordingly.
+	 * We divide the total memory region size equally for each chip
+	 * and set SFA1AD, SFA2AD, SFB1AD, SFB2AD accordingly.
 	 */
-	qspi_writel(q, q->devtype_data->ahb_buf_size + addr_offset,
+	qspi_writel(q, q->memmap_phy_size / 4 + addr_offset,
 		    base + QUADSPI_SFA1AD);
-	qspi_writel(q, q->devtype_data->ahb_buf_size * 2 + addr_offset,
+	qspi_writel(q, q->memmap_phy_size / 4 * 2 + addr_offset,
 		    base + QUADSPI_SFA2AD);
-	qspi_writel(q, q->devtype_data->ahb_buf_size * 3 + addr_offset,
+	qspi_writel(q, q->memmap_phy_size / 4 * 3 + addr_offset,
 		    base + QUADSPI_SFB1AD);
-	qspi_writel(q, q->devtype_data->ahb_buf_size * 4 + addr_offset,
+	qspi_writel(q, q->memmap_phy_size / 4 * 4 + addr_offset,
 		    base + QUADSPI_SFB2AD);
 
 	q->selected = -1;
@@ -831,13 +859,8 @@ static int fsl_qspi_probe(struct platform_device *pdev)
 
 	res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
 					"QuadSPI-memory");
-	q->ahb_addr = devm_ioremap_resource(dev, res);
-	if (IS_ERR(q->ahb_addr)) {
-		ret = PTR_ERR(q->ahb_addr);
-		goto err_put_ctrl;
-	}
-
 	q->memmap_phy = res->start;
+	q->memmap_phy_size = resource_size(res);
 
 	/* find the clocks */
 	q->clk_en = devm_clk_get(dev, "qspi_en");
@@ -913,6 +936,9 @@ static int fsl_qspi_remove(struct platform_device *pdev)
 
 	mutex_destroy(&q->lock);
 
+	if (q->ahb_addr)
+		iounmap(q->ahb_addr);
+
 	return 0;
 }
 
-- 
2.17.1