[PATCH 1/2] spi/fsl-espi: Correct the maximum transaction length

[PATCH 1/2] spi/fsl-espi: Correct the maximum transaction length

[Zhiqiang Hou]

From: Hou Zhiqiang <B48286-KZfg59tc24xl57MIdRCFDg@public.gmane.org>

The maximum length during one transcation is 64KiB.

Signed-off-by: Hou Zhiqiang <B48286-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
---
 drivers/spi/spi-fsl-espi.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/spi/spi-fsl-espi.c b/drivers/spi/spi-fsl-espi.c
index 7fd6a4c..7cb0c19 100644
--- a/drivers/spi/spi-fsl-espi.c
+++ b/drivers/spi/spi-fsl-espi.c
@@ -84,7 +84,7 @@ struct fsl_espi_transfer {
 /* SPCOM register values */
 #define SPCOM_CS(x)		((x) << 30)
 #define SPCOM_TRANLEN(x)	((x) << 0)
-#define	SPCOM_TRANLEN_MAX	0xFFFF	/* Max transaction length */
+#define	SPCOM_TRANLEN_MAX	0x10000	/* Max transaction length */
 
 #define AUTOSUSPEND_TIMEOUT 2000
 
@@ -233,7 +233,7 @@ static int fsl_espi_bufs(struct spi_device *spi, struct spi_transfer *t)
 	reinit_completion(&mpc8xxx_spi->done);
 
 	/* Set SPCOM[CS] and SPCOM[TRANLEN] field */
-	if ((t->len - 1) > SPCOM_TRANLEN_MAX) {
+	if (t->len > SPCOM_TRANLEN_MAX) {
 		dev_err(mpc8xxx_spi->dev, "Transaction length (%d)"
 				" beyond the SPCOM[TRANLEN] field\n", t->len);
 		return -EINVAL;
-- 
2.1.0.27.g96db324

--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 2/2] spi/fsl-espi: Remove the address conversion operation

[Zhiqiang Hou]

From: Hou Zhiqiang <B48286-KZfg59tc24xl57MIdRCFDg@public.gmane.org>

The eSPI bus driver should not touch the protocol layer drivers'
operations. It makes the eSPI driver isn't compatiable with devices
except 3-Byte addressing SPI flash. So, remove the address
conversion operations to make it compatible for most SPI interface
devices.

The eSPI controller is able to transfer maxminum 64KiB each time.
If the transaction length exceed the limited length, the eSPI
controller driver will add the address by length of the last time
transferred with assuming that the SPI flash address width is 3
Bytes.

With this patch, if the transaction length exceed the limited
length, it will truncate the transaction to max-length and return
the actual length transferred.

Signed-off-by: Hou Zhiqiang <B48286-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
---
 Tested on T1042D4RDB.
 This patch depend on patchset:
 http://patchwork.ozlabs.org/patch/551304/
 http://patchwork.ozlabs.org/patch/551308/
 http://patchwork.ozlabs.org/patch/551305/
 http://patchwork.ozlabs.org/patch/551306/
 http://patchwork.ozlabs.org/patch/551309/
 http://patchwork.ozlabs.org/patch/551307/
 http://patchwork.ozlabs.org/patch/551310/
 http://patchwork.ozlabs.org/patch/551311/
 and patch:
 http://patchwork.ozlabs.org/patch/571581/

 drivers/spi/spi-fsl-espi.c | 84 ++++++++++++----------------------------------
 1 file changed, 21 insertions(+), 63 deletions(-)

diff --git a/drivers/spi/spi-fsl-espi.c b/drivers/spi/spi-fsl-espi.c
index 7cb0c19..66ebb02 100644
--- a/drivers/spi/spi-fsl-espi.c
+++ b/drivers/spi/spi-fsl-espi.c
@@ -253,23 +253,6 @@ static int fsl_espi_bufs(struct spi_device *spi, struct spi_transfer *t)
 	return mpc8xxx_spi->count;
 }
 
-static inline void fsl_espi_addr2cmd(unsigned int addr, u8 *cmd)
-{
-	if (cmd) {
-		cmd[1] = (u8)(addr >> 16);
-		cmd[2] = (u8)(addr >> 8);
-		cmd[3] = (u8)(addr >> 0);
-	}
-}
-
-static inline unsigned int fsl_espi_cmd2addr(u8 *cmd)
-{
-	if (cmd)
-		return cmd[1] << 16 | cmd[2] << 8 | cmd[3] << 0;
-
-	return 0;
-}
-
 static void fsl_espi_do_trans(struct spi_message *m,
 				struct fsl_espi_transfer *tr)
 {
@@ -366,12 +349,9 @@ static void fsl_espi_rw_trans(struct spi_message *m,
 	struct spi_transfer *t;
 	u8 *local_buf;
 	u8 *rx_buf = rx_buff;
-	unsigned int trans_len;
-	unsigned int addr;
-	unsigned int tx_only;
-	unsigned int rx_pos = 0;
-	unsigned int pos;
-	int i, loop;
+	unsigned int trans_len = total_len;
+	unsigned int tx_only = 0;
+	int i = 0;
 
 	local_buf = kzalloc(SPCOM_TRANLEN_MAX, GFP_KERNEL);
 	if (!local_buf) {
@@ -379,51 +359,29 @@ static void fsl_espi_rw_trans(struct spi_message *m,
 		return;
 	}
 
-	for (pos = 0, loop = 0; pos < total_len; pos += trans_len, loop++) {
-		trans_len = total_len - pos;
-
-		i = 0;
-		tx_only = 0;
-		list_for_each_entry(t, &m->transfers, transfer_list) {
-			if (t->tx_buf) {
-				memcpy(local_buf + i, t->tx_buf, t->len);
-				i += t->len;
-				if (!t->rx_buf)
-					tx_only += t->len;
-			}
-		}
-
-		/* Add additional TX bytes to compensate SPCOM_TRANLEN_MAX */
-		if (loop > 0)
-			trans_len += tx_only;
-
-		if (trans_len > SPCOM_TRANLEN_MAX)
-			trans_len = SPCOM_TRANLEN_MAX;
-
-		/* Update device offset */
-		if (pos > 0) {
-			addr = fsl_espi_cmd2addr(local_buf);
-			addr += rx_pos;
-			fsl_espi_addr2cmd(addr, local_buf);
+	list_for_each_entry(t, &m->transfers, transfer_list) {
+		if (t->tx_buf) {
+			memcpy(local_buf + i, t->tx_buf, t->len);
+			i += t->len;
+			if (!t->rx_buf)
+				tx_only += t->len;
 		}
+	}
 
-		espi_trans->len = trans_len;
-		espi_trans->tx_buf = local_buf;
-		espi_trans->rx_buf = local_buf;
-		fsl_espi_do_trans(m, espi_trans);
+	if (trans_len > SPCOM_TRANLEN_MAX)
+		trans_len = SPCOM_TRANLEN_MAX;
 
-		/* If there is at least one RX byte then copy it to rx_buf */
-		if (tx_only < SPCOM_TRANLEN_MAX)
-			memcpy(rx_buf + rx_pos, espi_trans->rx_buf + tx_only,
-					trans_len - tx_only);
+	espi_trans->len = trans_len;
+	espi_trans->tx_buf = local_buf;
+	espi_trans->rx_buf = local_buf;
+	fsl_espi_do_trans(m, espi_trans);
 
-		rx_pos += trans_len - tx_only;
+	/* If there is at least one RX byte then copy it to rx_buf */
+	if (tx_only < SPCOM_TRANLEN_MAX)
+		memcpy(rx_buf, espi_trans->rx_buf + tx_only,
+				trans_len - tx_only);
 
-		if (loop > 0)
-			espi_trans->actual_length += espi_trans->len - tx_only;
-		else
-			espi_trans->actual_length += espi_trans->len;
-	}
+	espi_trans->actual_length += espi_trans->len;
 
 	kfree(local_buf);
 }
-- 
2.1.0.27.g96db324

--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/2] spi/fsl-espi: Remove the address conversion operation

[Mark Brown]

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

On Fri, Jan 22, 2016 at 06:58:27PM +0800, Zhiqiang Hou wrote:
> From: Hou Zhiqiang <B48286-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
> 
> The eSPI bus driver should not touch the protocol layer drivers'
> operations. It makes the eSPI driver isn't compatiable with devices

I don't seem to have patch 1.

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

Re: [PATCH 2/2] spi/fsl-espi: Remove the address conversion operation

[Mark Brown]

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

On Fri, Jan 22, 2016 at 11:26:37AM +0000, Mark Brown wrote:
> On Fri, Jan 22, 2016 at 06:58:27PM +0800, Zhiqiang Hou wrote:
> > From: Hou Zhiqiang <B48286-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
> > 
> > The eSPI bus driver should not touch the protocol layer drivers'
> > operations. It makes the eSPI driver isn't compatiable with devices
> 
> I don't seem to have patch 1.

It turned up now.

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

Re: [PATCH 2/2] spi/fsl-espi: Remove the address conversion operation

[Mark Brown]

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

On Fri, Jan 22, 2016 at 06:58:27PM +0800, Zhiqiang Hou wrote:

>  Tested on T1042D4RDB.
>  This patch depend on patchset:
>  http://patchwork.ozlabs.org/patch/551304/
>  http://patchwork.ozlabs.org/patch/551308/
>  http://patchwork.ozlabs.org/patch/551305/
>  http://patchwork.ozlabs.org/patch/551306/
>  http://patchwork.ozlabs.org/patch/551309/
>  http://patchwork.ozlabs.org/patch/551307/
>  http://patchwork.ozlabs.org/patch/551310/
>  http://patchwork.ozlabs.org/patch/551311/
>  and patch:
>  http://patchwork.ozlabs.org/patch/571581/

This is a *very* large list of dependencies.  What exactly are these and
what is the relationship between them and this patch?

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.

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

RE: [PATCH 2/2] spi/fsl-espi: Remove the address conversion operation

[Zhiqiang Hou]

Hi Mark,

Thanks for your comments!

> -----Original Message-----
> From: Mark Brown [mailto:broonie@kernel.org]
> Sent: 2016年1月23日 1:03
> To: Zhiqiang Hou <Zhiqiang.Hou@freescale.com>
> Cc: linux-spi@vger.kernel.org; computersforpeace@gmail.com;
> hramrach@gmail.com; Mingkai.Hu@freescale.com; Shaohui.Xie@freescale.com;
> Hou Zhiqiang <B48286@freescale.com>
> Subject: Re: [PATCH 2/2] spi/fsl-espi: Remove the address conversion operation
> 
> On Fri, Jan 22, 2016 at 06:58:27PM +0800, Zhiqiang Hou wrote:
> 
> >  Tested on T1042D4RDB.
> >  This patch depend on patchset:
> >  http://patchwork.ozlabs.org/patch/551304/

patch title: [v6,01/10] mtd: spi-nor: change return value of read/write
Change the return value of spi-nor device read and write methods to
allow returning amount of data transferred and errors as
read(2)/write(2) does.

> >  http://patchwork.ozlabs.org/patch/551308/[] 

patch title: [v6,02/10] mtd: m25p80: return amount of data transferred or error in read/write
Add checking of SPI transfer errors and return them from read/write
functions. Also return the amount of data transferred.

> >  http://patchwork.ozlabs.org/patch/551305/

patch title: [v6,03/10] mtd: fsl-quadspi: return amount of data read/written or error
Return amount of data read/written or error as read(2)/write(2) does.

> >  http://patchwork.ozlabs.org/patch/551306/

patch title: [v6,04/10] mtd: spi-nor: check return value from read/write
SPI NOR hardware drivers now return useful value from their read/write
functions so check them.

> >  http://patchwork.ozlabs.org/patch/551309/

patch title: [v6,05/10] mtd: spi-nor: stop passing around retlen
Do not pass retlen to hardware driver read/write functions. Update it in
spi-nor generic driver instead.

> >  http://patchwork.ozlabs.org/patch/551307/

patch title: [v6,06/10] mtd: spi-nor: simplify write loop
The spi-nor write loop assumes that what is passed to the hardware
driver write() is what gets written.

When write() writes less than page size at once data is dropped on the
floor. Check the amount of data writen and exit if it does not match
requested amount.

> >  http://patchwork.ozlabs.org/patch/551310/

patch title: [v6,07/10] mtd: spi-nor: add read loop
mtdblock and ubi do not handle the situation when read returns less data
than requested. Loop in spi-nor until buffer is filled or an error is
returned.

> >  http://patchwork.ozlabs.org/patch/551311/[] 

patch title: [v6,10/10] mtd: m25p80: read in spi_max_transfer_size chunks
Take into account transfer size limitation of SPI master.
 
Those is a patchset that has 10 patches and 2 of them has been merged,
so I pasted the remnant as the dependence.

The patch [v6,03/10] mtd: fsl-quadspi: return amount of data read/written or error
and [v6,06/10] mtd: spi-nor: simplify write loop is not depended.
The others of this patchset is depended, as the removed protocol layer's operations
from eSPI controller driver were handled by this patchset.

> >  and patch:
> >  http://patchwork.ozlabs.org/patch/571581/

patch title: dts/fsl/powerpc: add "jedec, spi-nor" flash compatible binding
This patch is not a code dependency but test need, upon the latest code base the
m25p80 driver will failed to probe, I added this patch to make others can reproduce
the test. 

> 
> This is a *very* large list of dependencies.  What exactly are these and what is the
> relationship between them and this patch?
> 
> 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.

yes, thanks for your advice.

Thanks,
Zhiqiang

Re: [PATCH 2/2] spi/fsl-espi: Remove the address conversion operation

[Mark Brown]

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

On Mon, Jan 25, 2016 at 05:47:33AM +0000, Zhiqiang Hou wrote:

> > This is a *very* large list of dependencies.  What exactly are these and what is the
> > relationship between them and this patch?

> > 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.

> yes, thanks for your advice.

I still can't really tell what's going on here or where these
dependencies are at.  Probably the easiest thing here is to submit this
once the dependencies are in.

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

Applied "spi/fsl-espi: Correct the maximum transaction length" to the spi tree

[Mark Brown]

The patch

   spi/fsl-espi: Correct the maximum transaction length

has been applied to the spi tree at

   git://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git 

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 5cfa1e4e0deced0cccedb4b30facb8a8e68e209b Mon Sep 17 00:00:00 2001
From: Hou Zhiqiang <B48286-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
Date: Fri, 22 Jan 2016 18:58:26 +0800
Subject: [PATCH] spi/fsl-espi: Correct the maximum transaction length

The maximum length during one transcation is 64KiB.

Signed-off-by: Hou Zhiqiang <B48286-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
Signed-off-by: Mark Brown <broonie-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
 drivers/spi/spi-fsl-espi.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/spi/spi-fsl-espi.c b/drivers/spi/spi-fsl-espi.c
index 7fd6a4c009d2..7cb0c1921495 100644
--- a/drivers/spi/spi-fsl-espi.c
+++ b/drivers/spi/spi-fsl-espi.c
@@ -84,7 +84,7 @@ struct fsl_espi_transfer {
 /* SPCOM register values */
 #define SPCOM_CS(x)		((x) << 30)
 #define SPCOM_TRANLEN(x)	((x) << 0)
-#define	SPCOM_TRANLEN_MAX	0xFFFF	/* Max transaction length */
+#define	SPCOM_TRANLEN_MAX	0x10000	/* Max transaction length */
 
 #define AUTOSUSPEND_TIMEOUT 2000
 
@@ -233,7 +233,7 @@ static int fsl_espi_bufs(struct spi_device *spi, struct spi_transfer *t)
 	reinit_completion(&mpc8xxx_spi->done);
 
 	/* Set SPCOM[CS] and SPCOM[TRANLEN] field */
-	if ((t->len - 1) > SPCOM_TRANLEN_MAX) {
+	if (t->len > SPCOM_TRANLEN_MAX) {
 		dev_err(mpc8xxx_spi->dev, "Transaction length (%d)"
 				" beyond the SPCOM[TRANLEN] field\n", t->len);
 		return -EINVAL;
-- 
2.7.0.rc3

--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html