[PATCH 1/3] spi: introduce max message size flag in spi_master

[PATCH 1/3] spi: introduce max message size flag in spi_master

[Heiner Kallweit]

Recently a maximum transfer size was was introduced in struct spi_master.
However there are also spi controllers with a maximum message size, e.g.
fsl-espi has a max message size of 64KB.
As there are no (known) spi controller with differing size limits for
transfer and message, introduce a flag in spi_master to indicate that
the max transfer size is also the maximum message size.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 include/linux/spi/spi.h | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h
index 072cb2a..cadb1de 100644
--- a/include/linux/spi/spi.h
+++ b/include/linux/spi/spi.h
@@ -312,6 +312,8 @@ static inline void spi_unregister_driver(struct spi_driver *sdrv)
  * @flags: other constraints relevant to this driver
  * @max_transfer_size: function that returns the max transfer size for
  *	a &spi_device; may be %NULL, so the default %SIZE_MAX will be used.
+ * @max_message_size_flag: indicates that max transfer size is also
+ *	max message size
  * @io_mutex: mutex for physical bus access
  * @bus_lock_spinlock: spinlock for SPI bus locking
  * @bus_lock_mutex: mutex for exclusion of multiple callers
@@ -446,6 +448,8 @@ struct spi_master {
 	 * the limit may depend on device transfer settings
 	 */
 	size_t (*max_transfer_size)(struct spi_device *spi);
+	/* flag indicating that max transfer size is also max message size */
+	bool			max_message_size_flag;
 
 	/* I/O mutex */
 	struct mutex		io_mutex;
-- 
2.9.2

Re: [PATCH 1/3] spi: introduce max message size flag in spi_master

[Mark Brown]

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

On Sun, Aug 07, 2016 at 09:47:18PM +0200, Heiner Kallweit wrote:

> As there are no (known) spi controller with differing size limits for
> transfer and message, introduce a flag in spi_master to indicate that
> the max transfer size is also the maximum message size.

This is a *really* cumbersome interface which will doubtless be found to
be broken at some point by some hardware that does have separate
restrictons on both.  Let's just specify the limit directly unless
there's some pressing reason not to.

There's also something off with the way you're posting the patches, your
patch series isn't threaded together.  

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

Re: [PATCH 1/3] spi: introduce max message size flag in spi_master

[Heiner Kallweit]

Am 16.08.2016 um 18:42 schrieb Mark Brown:
> On Sun, Aug 07, 2016 at 09:47:18PM +0200, Heiner Kallweit wrote:
> 
>> As there are no (known) spi controller with differing size limits for
>> transfer and message, introduce a flag in spi_master to indicate that
>> the max transfer size is also the maximum message size.
> 
> This is a *really* cumbersome interface which will doubtless be found to
> be broken at some point by some hardware that does have separate
> restrictons on both.  Let's just specify the limit directly unless
> there's some pressing reason not to.
> 
> There's also something off with the way you're posting the patches, your
> patch series isn't threaded together.  
> 
Thanks for the feedback. I'll address both points and send a v2.

[PATCH v2 1/3] spi: introduce max_message_size hook in spi_master

[Heiner Kallweit]

Recently a maximum transfer size was was introduced in struct spi_master.
However there are also spi controllers with a maximum message size, e.g.
fsl-espi has a max message size of 64KB.
Introduce a hook max_message_size to deal with such limitations.

Also make sure that spi_max_transfer_size doesn't return greater values
than spi_max_message_size, even if hook max_transfer_size is not set.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
v2:
- replace the flag with a hook that allows to define a message size
  limit independednt of a potential transfer size limit
---
 include/linux/spi/spi.h | 25 +++++++++++++++++++++----
 1 file changed, 21 insertions(+), 4 deletions(-)

diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h
index 072cb2a..f2d3960 100644
--- a/include/linux/spi/spi.h
+++ b/include/linux/spi/spi.h
@@ -312,6 +312,8 @@ static inline void spi_unregister_driver(struct spi_driver *sdrv)
  * @flags: other constraints relevant to this driver
  * @max_transfer_size: function that returns the max transfer size for
  *	a &spi_device; may be %NULL, so the default %SIZE_MAX will be used.
+ * @max_message_size: function that returns the max message size for
+ *	a &spi_device; may be %NULL, so the default %SIZE_MAX will be used.
  * @io_mutex: mutex for physical bus access
  * @bus_lock_spinlock: spinlock for SPI bus locking
  * @bus_lock_mutex: mutex for exclusion of multiple callers
@@ -442,10 +444,11 @@ struct spi_master {
 #define SPI_MASTER_MUST_TX      BIT(4)		/* requires tx */
 
 	/*
-	 * on some hardware transfer size may be constrained
+	 * on some hardware transfer / message size may be constrained
 	 * the limit may depend on device transfer settings
 	 */
 	size_t (*max_transfer_size)(struct spi_device *spi);
+	size_t (*max_message_size)(struct spi_device *spi);
 
 	/* I/O mutex */
 	struct mutex		io_mutex;
@@ -905,12 +908,26 @@ extern int spi_async_locked(struct spi_device *spi,
 			    struct spi_message *message);
 
 static inline size_t
-spi_max_transfer_size(struct spi_device *spi)
+spi_max_message_size(struct spi_device *spi)
 {
 	struct spi_master *master = spi->master;
-	if (!master->max_transfer_size)
+	if (!master->max_message_size)
 		return SIZE_MAX;
-	return master->max_transfer_size(spi);
+	return master->max_message_size(spi);
+}
+
+static inline size_t
+spi_max_transfer_size(struct spi_device *spi)
+{
+	struct spi_master *master = spi->master;
+	size_t tr_max = SIZE_MAX;
+	size_t msg_max = spi_max_message_size(spi);
+
+	if (master->max_transfer_size)
+		tr_max = master->max_transfer_size(spi);
+
+	/* transfer size limit must not be greater than messsage size limit */
+	return min(tr_max, msg_max);
 }
 
 /*---------------------------------------------------------------------------*/
-- 
2.9.2

Re: [PATCH v2 1/3] spi: introduce max_message_size hook in spi_master

[Mark Brown]

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

On Wed, Aug 17, 2016 at 09:08:01PM +0200, Heiner Kallweit wrote:
> Recently a maximum transfer size was was introduced in struct spi_master.
> However there are also spi controllers with a maximum message size, e.g.
> fsl-espi has a max message size of 64KB.
> Introduce a hook max_message_size to deal with such limitations.

This can be pulled as:

The following changes since commit 29b4817d4018df78086157ea3a55c1d9424a7cfc:

  Linux 4.8-rc1 (2016-08-07 18:18:00 -0700)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git tags/spi-max-msg-size

for you to fetch changes up to 5090cc6ae2f79ee779e5faf7c8a28edf42b7d738:

  spi: introduce max_message_size hook in spi_master (2016-08-18 11:14:48 +0100)

----------------------------------------------------------------
spi: Add a maximum message size operation

Some controllers handle entire messages themselves and have limits on
the maximum message size, provide a way for clients to query this.

----------------------------------------------------------------
Heiner Kallweit (1):
      spi: introduce max_message_size hook in spi_master

 include/linux/spi/spi.h | 25 +++++++++++++++++++++----
 1 file changed, 21 insertions(+), 4 deletions(-)

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

Applied "spi: introduce max_message_size hook in spi_master" to the spi tree

[Mark Brown]

The patch

   spi: introduce max_message_size hook in spi_master

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 5090cc6ae2f79ee779e5faf7c8a28edf42b7d738 Mon Sep 17 00:00:00 2001
From: Heiner Kallweit <hkallweit1@gmail.com>
Date: Wed, 17 Aug 2016 21:08:01 +0200
Subject: [PATCH] spi: introduce max_message_size hook in spi_master

Recently a maximum transfer size was was introduced in struct spi_master.
However there are also spi controllers with a maximum message size, e.g.
fsl-espi has a max message size of 64KB.
Introduce a hook max_message_size to deal with such limitations.

Also make sure that spi_max_transfer_size doesn't return greater values
than spi_max_message_size, even if hook max_transfer_size is not set.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 include/linux/spi/spi.h | 25 +++++++++++++++++++++----
 1 file changed, 21 insertions(+), 4 deletions(-)

diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h
index 072cb2aa2413..f2d3960cc3c3 100644
--- a/include/linux/spi/spi.h
+++ b/include/linux/spi/spi.h
@@ -312,6 +312,8 @@ static inline void spi_unregister_driver(struct spi_driver *sdrv)
  * @flags: other constraints relevant to this driver
  * @max_transfer_size: function that returns the max transfer size for
  *	a &spi_device; may be %NULL, so the default %SIZE_MAX will be used.
+ * @max_message_size: function that returns the max message size for
+ *	a &spi_device; may be %NULL, so the default %SIZE_MAX will be used.
  * @io_mutex: mutex for physical bus access
  * @bus_lock_spinlock: spinlock for SPI bus locking
  * @bus_lock_mutex: mutex for exclusion of multiple callers
@@ -442,10 +444,11 @@ struct spi_master {
 #define SPI_MASTER_MUST_TX      BIT(4)		/* requires tx */
 
 	/*
-	 * on some hardware transfer size may be constrained
+	 * on some hardware transfer / message size may be constrained
 	 * the limit may depend on device transfer settings
 	 */
 	size_t (*max_transfer_size)(struct spi_device *spi);
+	size_t (*max_message_size)(struct spi_device *spi);
 
 	/* I/O mutex */
 	struct mutex		io_mutex;
@@ -905,12 +908,26 @@ extern int spi_async_locked(struct spi_device *spi,
 			    struct spi_message *message);
 
 static inline size_t
-spi_max_transfer_size(struct spi_device *spi)
+spi_max_message_size(struct spi_device *spi)
 {
 	struct spi_master *master = spi->master;
-	if (!master->max_transfer_size)
+	if (!master->max_message_size)
 		return SIZE_MAX;
-	return master->max_transfer_size(spi);
+	return master->max_message_size(spi);
+}
+
+static inline size_t
+spi_max_transfer_size(struct spi_device *spi)
+{
+	struct spi_master *master = spi->master;
+	size_t tr_max = SIZE_MAX;
+	size_t msg_max = spi_max_message_size(spi);
+
+	if (master->max_transfer_size)
+		tr_max = master->max_transfer_size(spi);
+
+	/* transfer size limit must not be greater than messsage size limit */
+	return min(tr_max, msg_max);
 }
 
 /*---------------------------------------------------------------------------*/
-- 
2.8.1

[PATCH v2 2/3] mtd: m25p80: consider max message size in m25p80_read

[Heiner Kallweit]

Consider a message size limit when calculating the maximum amount
of data that can be read.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
v2:
- use the new function spi_max_message_size
---
 drivers/mtd/devices/m25p80.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/mtd/devices/m25p80.c b/drivers/mtd/devices/m25p80.c
index 9cf7fcd..16a7df2 100644
--- a/drivers/mtd/devices/m25p80.c
+++ b/drivers/mtd/devices/m25p80.c
@@ -172,7 +172,8 @@ static ssize_t m25p80_read(struct spi_nor *nor, loff_t from, size_t len,
 
 	t[1].rx_buf = buf;
 	t[1].rx_nbits = m25p80_rx_nbits(nor);
-	t[1].len = min(len, spi_max_transfer_size(spi));
+	t[1].len = min3(len, spi_max_transfer_size(spi),
+			spi_max_message_size(spi) - t[0].len);
 	spi_message_add_tail(&t[1], &m);
 
 	ret = spi_sync(spi, &m);
-- 
2.9.2

Re: [PATCH v2 2/3] mtd: m25p80: consider max message size in m25p80_read

[Heiner Kallweit]

Am 17.08.2016 um 21:09 schrieb Heiner Kallweit:
> Consider a message size limit when calculating the maximum amount
> of data that can be read.
> 
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
> ---
> v2:
> - use the new function spi_max_message_size
> ---
>  drivers/mtd/devices/m25p80.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/mtd/devices/m25p80.c b/drivers/mtd/devices/m25p80.c
> index 9cf7fcd..16a7df2 100644
> --- a/drivers/mtd/devices/m25p80.c
> +++ b/drivers/mtd/devices/m25p80.c
> @@ -172,7 +172,8 @@ static ssize_t m25p80_read(struct spi_nor *nor, loff_t from, size_t len,
>  
>  	t[1].rx_buf = buf;
>  	t[1].rx_nbits = m25p80_rx_nbits(nor);
> -	t[1].len = min(len, spi_max_transfer_size(spi));
> +	t[1].len = min3(len, spi_max_transfer_size(spi),
> +			spi_max_message_size(spi) - t[0].len);
>  	spi_message_add_tail(&t[1], &m);
>  
>  	ret = spi_sync(spi, &m);
> 
Can we still get this into 4.9 ?
The other two patches of the series have been applied by Mark already
and there's a dependency for messages >64k on fsl-espi.

[PATCH RESEND] mtd: m25p80: consider max message size in m25p80_read

[Heiner Kallweit]

Consider a message size limit when calculating the maximum amount
of data that can be read.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
v2:
- use the new function spi_max_message_size

This patch was part of a series and is needed to correctly support
reads > 64k on fsl-espi (as first user of spi_max_message_size).
See commit 02a595d5d6e4 ("spi: fsl-espi: eliminate spi nor flash
read loop")
---
 drivers/mtd/devices/m25p80.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/mtd/devices/m25p80.c b/drivers/mtd/devices/m25p80.c
index 9cf7fcd..16a7df2 100644
--- a/drivers/mtd/devices/m25p80.c
+++ b/drivers/mtd/devices/m25p80.c
@@ -172,7 +172,8 @@ static ssize_t m25p80_read(struct spi_nor *nor, loff_t from, size_t len,
 
 	t[1].rx_buf = buf;
 	t[1].rx_nbits = m25p80_rx_nbits(nor);
-	t[1].len = min(len, spi_max_transfer_size(spi));
+	t[1].len = min3(len, spi_max_transfer_size(spi),
+			spi_max_message_size(spi) - t[0].len);
 	spi_message_add_tail(&t[1], &m);
 
 	ret = spi_sync(spi, &m);
-- 
2.9.2

Re: [PATCH RESEND] mtd: m25p80: consider max message size in m25p80_read

[Heiner Kallweit]

Am 27.10.2016 um 23:23 schrieb Heiner Kallweit:
> Consider a message size limit when calculating the maximum amount
> of data that can be read.
> 
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
> ---
> v2:
> - use the new function spi_max_message_size
> 
> This patch was part of a series and is needed to correctly support
> reads > 64k on fsl-espi (as first user of spi_max_message_size).
> See commit 02a595d5d6e4 ("spi: fsl-espi: eliminate spi nor flash
> read loop")
> ---
>  drivers/mtd/devices/m25p80.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/mtd/devices/m25p80.c b/drivers/mtd/devices/m25p80.c
> index 9cf7fcd..16a7df2 100644
> --- a/drivers/mtd/devices/m25p80.c
> +++ b/drivers/mtd/devices/m25p80.c
> @@ -172,7 +172,8 @@ static ssize_t m25p80_read(struct spi_nor *nor, loff_t from, size_t len,
>  
>  	t[1].rx_buf = buf;
>  	t[1].rx_nbits = m25p80_rx_nbits(nor);
> -	t[1].len = min(len, spi_max_transfer_size(spi));
> +	t[1].len = min3(len, spi_max_transfer_size(spi),
> +			spi_max_message_size(spi) - t[0].len);
>  	spi_message_add_tail(&t[1], &m);
>  
>  	ret = spi_sync(spi, &m);
> 
Sorry, forgot that this needs to go to stable as well. Will send a v2.

[PATCH v2] mtd: m25p80: consider max message size in m25p80_read

[Heiner Kallweit]

Consider a message size limit when calculating the maximum amount
of data that can be read.

The message size limit has been introduced with 4.9, so cc it
to stable.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
Cc: <stable@vger.kernel.org> # 4.9.x
---
v2:
- This patch was part of a series and the other patches of the
  series went into 4.9. It's needed to correctly support
  reads > 64k on fsl-espi as first user of spi_max_message_size.
  See commit 02a595d5d6e4 ("spi: fsl-espi: eliminate spi nor
  flash read loop"). CC it to stable.  
---
 drivers/mtd/devices/m25p80.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/mtd/devices/m25p80.c b/drivers/mtd/devices/m25p80.c
index 9cf7fcd..16a7df2 100644
--- a/drivers/mtd/devices/m25p80.c
+++ b/drivers/mtd/devices/m25p80.c
@@ -172,7 +172,8 @@ static ssize_t m25p80_read(struct spi_nor *nor, loff_t from, size_t len,
 
 	t[1].rx_buf = buf;
 	t[1].rx_nbits = m25p80_rx_nbits(nor);
-	t[1].len = min(len, spi_max_transfer_size(spi));
+	t[1].len = min3(len, spi_max_transfer_size(spi),
+			spi_max_message_size(spi) - t[0].len);
 	spi_message_add_tail(&t[1], &m);
 
 	ret = spi_sync(spi, &m);
-- 
2.9.2

Re: [PATCH v2] mtd: m25p80: consider max message size in m25p80_read

[Marek Vasut]

On 10/28/2016 07:58 AM, Heiner Kallweit wrote:
> Consider a message size limit when calculating the maximum amount
> of data that can be read.
> 
> The message size limit has been introduced with 4.9, so cc it
> to stable.
> 
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
> Cc: <stable@vger.kernel.org> # 4.9.x
> ---
> v2:
> - This patch was part of a series and the other patches of the
>   series went into 4.9. It's needed to correctly support
>   reads > 64k on fsl-espi as first user of spi_max_message_size.
>   See commit 02a595d5d6e4 ("spi: fsl-espi: eliminate spi nor
>   flash read loop"). CC it to stable.  

Does this imply that we have to fix every single driver this way ?

> ---
>  drivers/mtd/devices/m25p80.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/mtd/devices/m25p80.c b/drivers/mtd/devices/m25p80.c
> index 9cf7fcd..16a7df2 100644
> --- a/drivers/mtd/devices/m25p80.c
> +++ b/drivers/mtd/devices/m25p80.c
> @@ -172,7 +172,8 @@ static ssize_t m25p80_read(struct spi_nor *nor, loff_t from, size_t len,
>  
>  	t[1].rx_buf = buf;
>  	t[1].rx_nbits = m25p80_rx_nbits(nor);
> -	t[1].len = min(len, spi_max_transfer_size(spi));
> +	t[1].len = min3(len, spi_max_transfer_size(spi),
> +			spi_max_message_size(spi) - t[0].len);
>  	spi_message_add_tail(&t[1], &m);
>  
>  	ret = spi_sync(spi, &m);
> 


-- 
Best regards,
Marek Vasut

Re: [PATCH v2] mtd: m25p80: consider max message size in m25p80_read

[Mark Brown]

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

On Sat, Oct 29, 2016 at 07:59:09PM +0200, Marek Vasut wrote:
> On 10/28/2016 07:58 AM, Heiner Kallweit wrote:

> > - This patch was part of a series and the other patches of the
> >   series went into 4.9. It's needed to correctly support
> >   reads > 64k on fsl-espi as first user of spi_max_message_size.
> >   See commit 02a595d5d6e4 ("spi: fsl-espi: eliminate spi nor
> >   flash read loop"). CC it to stable.  

> Does this imply that we have to fix every single driver this way ?

Ideally.  Any SPI driver that has an upper limit really ought to export
it, and any driver that might bump into one of those limits should
check.  A lot of drivers are either unlikely to be deployed in a system
that has relevant limits or tend to only do short enough transfers
though - if systems are currently working it's just neatening things up
really.

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

Re: [PATCH v2] mtd: m25p80: consider max message size in m25p80_read

[Heiner Kallweit]

Am 28.10.2016 um 07:58 schrieb Heiner Kallweit:
> Consider a message size limit when calculating the maximum amount
> of data that can be read.
> 
> The message size limit has been introduced with 4.9, so cc it
> to stable.
> 
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
> Cc: <stable@vger.kernel.org> # 4.9.x
> ---
> v2:
> - This patch was part of a series and the other patches of the
>   series went into 4.9. It's needed to correctly support
>   reads > 64k on fsl-espi as first user of spi_max_message_size.
>   See commit 02a595d5d6e4 ("spi: fsl-espi: eliminate spi nor
>   flash read loop"). CC it to stable.  
> ---
>  drivers/mtd/devices/m25p80.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/mtd/devices/m25p80.c b/drivers/mtd/devices/m25p80.c
> index 9cf7fcd..16a7df2 100644
> --- a/drivers/mtd/devices/m25p80.c
> +++ b/drivers/mtd/devices/m25p80.c
> @@ -172,7 +172,8 @@ static ssize_t m25p80_read(struct spi_nor *nor, loff_t from, size_t len,
>  
>  	t[1].rx_buf = buf;
>  	t[1].rx_nbits = m25p80_rx_nbits(nor);
> -	t[1].len = min(len, spi_max_transfer_size(spi));
> +	t[1].len = min3(len, spi_max_transfer_size(spi),
> +			spi_max_message_size(spi) - t[0].len);
>  	spi_message_add_tail(&t[1], &m);
>  
>  	ret = spi_sync(spi, &m);
> 
This one is still open and there have been reports already from
people facing this issue. Can we still get it into 4.10 +
into 4.9 ?

Thanks

Re: [PATCH v2 2/3] mtd: m25p80: consider max message size in m25p80_read

[Cyrille Pitchen]

Le 17/08/2016 à 21:09, Heiner Kallweit a écrit :
> Consider a message size limit when calculating the maximum amount
> of data that can be read.
> 
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
Reviewed-by: Cyrille Pitchen <cyrille.pitchen@atmel.com>

> ---
> v2:
> - use the new function spi_max_message_size
> ---
>  drivers/mtd/devices/m25p80.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/mtd/devices/m25p80.c b/drivers/mtd/devices/m25p80.c
> index 9cf7fcd..16a7df2 100644
> --- a/drivers/mtd/devices/m25p80.c
> +++ b/drivers/mtd/devices/m25p80.c
> @@ -172,7 +172,8 @@ static ssize_t m25p80_read(struct spi_nor *nor, loff_t from, size_t len,
>  
>  	t[1].rx_buf = buf;
>  	t[1].rx_nbits = m25p80_rx_nbits(nor);
> -	t[1].len = min(len, spi_max_transfer_size(spi));
> +	t[1].len = min3(len, spi_max_transfer_size(spi),
> +			spi_max_message_size(spi) - t[0].len);
>  	spi_message_add_tail(&t[1], &m);
>  
>  	ret = spi_sync(spi, &m);
> 

Re: [PATCH v2 2/3] mtd: m25p80: consider max message size in m25p80_read

[Cyrille Pitchen]

Le 17/08/2016 à 21:09, Heiner Kallweit a écrit :
> Consider a message size limit when calculating the maximum amount
> of data that can be read.
> 
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>

Applied to git://github.com/spi-nor/linux.git

Thanks!

> ---
> v2:
> - use the new function spi_max_message_size
> ---
>  drivers/mtd/devices/m25p80.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/mtd/devices/m25p80.c b/drivers/mtd/devices/m25p80.c
> index 9cf7fcd..16a7df2 100644
> --- a/drivers/mtd/devices/m25p80.c
> +++ b/drivers/mtd/devices/m25p80.c
> @@ -172,7 +172,8 @@ static ssize_t m25p80_read(struct spi_nor *nor, loff_t from, size_t len,
>  
>  	t[1].rx_buf = buf;
>  	t[1].rx_nbits = m25p80_rx_nbits(nor);
> -	t[1].len = min(len, spi_max_transfer_size(spi));
> +	t[1].len = min3(len, spi_max_transfer_size(spi),
> +			spi_max_message_size(spi) - t[0].len);
>  	spi_message_add_tail(&t[1], &m);
>  
>  	ret = spi_sync(spi, &m);
> 

Re: [PATCH v2 2/3] mtd: m25p80: consider max message size in m25p80_read

[Marek Vasut]

On 12/23/2016 04:33 PM, Cyrille Pitchen wrote:
> Le 17/08/2016 à 21:09, Heiner Kallweit a écrit :
>> Consider a message size limit when calculating the maximum amount
>> of data that can be read.
>>
>> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
> 
> Applied to git://github.com/spi-nor/linux.git

Is that the concensus now that we should fix controller crappiness on
driver level ? Or did I miss the point of this patch ?

> Thanks!
> 
>> ---
>> v2:
>> - use the new function spi_max_message_size
>> ---
>>  drivers/mtd/devices/m25p80.c | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/mtd/devices/m25p80.c b/drivers/mtd/devices/m25p80.c
>> index 9cf7fcd..16a7df2 100644
>> --- a/drivers/mtd/devices/m25p80.c
>> +++ b/drivers/mtd/devices/m25p80.c
>> @@ -172,7 +172,8 @@ static ssize_t m25p80_read(struct spi_nor *nor, loff_t from, size_t len,
>>  
>>  	t[1].rx_buf = buf;
>>  	t[1].rx_nbits = m25p80_rx_nbits(nor);
>> -	t[1].len = min(len, spi_max_transfer_size(spi));
>> +	t[1].len = min3(len, spi_max_transfer_size(spi),
>> +			spi_max_message_size(spi) - t[0].len);
>>  	spi_message_add_tail(&t[1], &m);
>>  
>>  	ret = spi_sync(spi, &m);
>>
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-spi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 


-- 
Best regards,
Marek Vasut

Re: [PATCH v2 2/3] mtd: m25p80: consider max message size in m25p80_read

[Michal Suchanek]

Hello,

On 23 December 2016 at 19:53, Marek Vasut <marek.vasut@gmail.com> wrote:
> On 12/23/2016 04:33 PM, Cyrille Pitchen wrote:
>> Le 17/08/2016 à 21:09, Heiner Kallweit a écrit :
>>> Consider a message size limit when calculating the maximum amount
>>> of data that can be read.
>>>
>>> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
>>
>> Applied to git://github.com/spi-nor/linux.git
>
> Is that the concensus now that we should fix controller crappiness on
> driver level ? Or did I miss the point of this patch ?
>

In general most controllers do have some limit on the amount of data
they can transfer at once.

For some you can apply workaround at controller driver level and for
some it's not practical.

Also for some the limit is lower so it is more likely hit.

I guess you can call controllers that have both of these limitations
crappy but that's what's in the silicone out there.

If you have better solution to the problem please share it in detail.

Thanks

Michal

Re: [PATCH v2 2/3] mtd: m25p80: consider max message size in m25p80_read

[Marek Vasut]

On 12/23/2016 10:04 PM, Michal Suchanek wrote:
> Hello,

Hi,

> On 23 December 2016 at 19:53, Marek Vasut <marek.vasut@gmail.com> wrote:
>> On 12/23/2016 04:33 PM, Cyrille Pitchen wrote:
>>> Le 17/08/2016 à 21:09, Heiner Kallweit a écrit :
>>>> Consider a message size limit when calculating the maximum amount
>>>> of data that can be read.
>>>>
>>>> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
>>>
>>> Applied to git://github.com/spi-nor/linux.git
>>
>> Is that the concensus now that we should fix controller crappiness on
>> driver level ? Or did I miss the point of this patch ?
>>
> 
> In general most controllers do have some limit on the amount of data
> they can transfer at once.

My observations suggest otherwise.

> For some you can apply workaround at controller driver level and for
> some it's not practical.
> 
> Also for some the limit is lower so it is more likely hit.
> 
> I guess you can call controllers that have both of these limitations
> crappy but that's what's in the silicone out there.
> 
> If you have better solution to the problem please share it in detail.

I don't have a solution, but I am quite sure that if we now set a
precedent that each and every device driver should take care of each and
every controller's quirk, we have a lot of unpleasant work ahead
of us and it will be quite the maintanance burden. The separations
between device driver and controller driver has been weakened and I
don't think that's good.

-- 
Best regards,
Marek Vasut

Re: [PATCH v2 2/3] mtd: m25p80: consider max message size in m25p80_read

[Heiner Kallweit]

Am 24.12.2016 um 11:20 schrieb Marek Vasut:
> On 12/23/2016 10:04 PM, Michal Suchanek wrote:
>> Hello,
> 
> Hi,
> 
>> On 23 December 2016 at 19:53, Marek Vasut <marek.vasut@gmail.com> wrote:
>>> On 12/23/2016 04:33 PM, Cyrille Pitchen wrote:
>>>> Le 17/08/2016 à 21:09, Heiner Kallweit a écrit :
>>>>> Consider a message size limit when calculating the maximum amount
>>>>> of data that can be read.
>>>>>
>>>>> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
>>>>
>>>> Applied to git://github.com/spi-nor/linux.git
>>>
>>> Is that the concensus now that we should fix controller crappiness on
>>> driver level ? Or did I miss the point of this patch ?
>>>
>>
>> In general most controllers do have some limit on the amount of data
>> they can transfer at once.
> 
> My observations suggest otherwise.
> 
>> For some you can apply workaround at controller driver level and for
>> some it's not practical.
>>
>> Also for some the limit is lower so it is more likely hit.
>>
>> I guess you can call controllers that have both of these limitations
>> crappy but that's what's in the silicone out there.
>>
>> If you have better solution to the problem please share it in detail.
> 
> I don't have a solution, but I am quite sure that if we now set a
> precedent that each and every device driver should take care of each and
> every controller's quirk, we have a lot of unpleasant work ahead
> of us and it will be quite the maintanance burden. The separations
> between device driver and controller driver has been weakened and I
> don't think that's good.
> 
The controller driver API has been extended to allow exposing certain
types of controller limits (max tranfer size, max message size)
to upper layers. And controller drivers rely on upper layers
considering this information.
W/o this patch certain combinations of protocol and controller drivers
(e.g. m25p80 + fsl-espi) don't work for big transfer / message sizes.

The (most likely more ugly) alternative is controller drivers
implementing protocol-specific hacks to deal with messages exceeding
controller limits. One example was the fsl-espi drivers before the
recent refactoring. It assumed all big messages to be m25p80.

[PATCH v2 3/3] spi: fsl-espi: eliminate spi nor flash read loop

[Heiner Kallweit]

The fsl-espi driver contains a read loop that implicitely assumes that
the device to read from is a m25p80 SPI NOR flash (bytes 2 - 4 of the
first write transfer are interpreted as 3 byte flash address).

Now that we have such a read loop in the spi-nor driver and are able
to correctly indicate the message size limit of the controller,
the read loop can be removed from the fsl-espi driver.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
v2:
- set the new hook max_message_size in spi_master
---
 drivers/spi/spi-fsl-espi.c | 91 +++++++++++-----------------------------------
 1 file changed, 21 insertions(+), 70 deletions(-)

diff --git a/drivers/spi/spi-fsl-espi.c b/drivers/spi/spi-fsl-espi.c
index 8d85a3c..96a2442 100644
--- a/drivers/spi/spi-fsl-espi.c
+++ b/drivers/spi/spi-fsl-espi.c
@@ -258,23 +258,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,68 +349,36 @@ static void fsl_espi_cmd_trans(struct spi_message *m,
 static void fsl_espi_rw_trans(struct spi_message *m,
 				struct fsl_espi_transfer *trans, u8 *rx_buff)
 {
-	struct fsl_espi_transfer *espi_trans = trans;
-	unsigned int total_len = espi_trans->len;
 	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 tx_only = 0;
+	int i = 0;
 
 	local_buf = kzalloc(SPCOM_TRANLEN_MAX, GFP_KERNEL);
 	if (!local_buf) {
-		espi_trans->status = -ENOMEM;
+		trans->status = -ENOMEM;
 		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 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);
-
-		rx_pos += trans_len - tx_only;
+	trans->tx_buf = local_buf;
+	trans->rx_buf = local_buf;
+	fsl_espi_do_trans(m, trans);
 
-		if (loop > 0)
-			espi_trans->actual_length += espi_trans->len - tx_only;
-		else
-			espi_trans->actual_length += espi_trans->len;
+	if (!trans->status) {
+		/* If there is at least one RX byte then copy it to rx_buff */
+		if (trans->len > tx_only)
+			memcpy(rx_buff, trans->rx_buf + tx_only,
+			       trans->len - tx_only);
+		trans->actual_length += trans->len;
 	}
 
 	kfree(local_buf);
@@ -663,7 +614,7 @@ static int fsl_espi_runtime_resume(struct device *dev)
 }
 #endif
 
-static size_t fsl_espi_max_transfer_size(struct spi_device *spi)
+static size_t fsl_espi_max_message_size(struct spi_device *spi)
 {
 	return SPCOM_TRANLEN_MAX;
 }
@@ -695,7 +646,7 @@ static struct spi_master * fsl_espi_probe(struct device *dev,
 	master->cleanup = fsl_espi_cleanup;
 	master->transfer_one_message = fsl_espi_do_one_msg;
 	master->auto_runtime_pm = true;
-	master->max_transfer_size = fsl_espi_max_transfer_size;
+	master->max_message_size = fsl_espi_max_message_size;
 
 	mpc8xxx_spi = spi_master_get_devdata(master);
 
-- 
2.9.2

Applied "spi: fsl-espi: eliminate spi nor flash read loop" to the spi tree

[Mark Brown]

The patch

   spi: fsl-espi: eliminate spi nor flash read loop

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 02a595d5d6e4ecc0564ec31afb76202035a7591c Mon Sep 17 00:00:00 2001
From: Heiner Kallweit <hkallweit1@gmail.com>
Date: Wed, 17 Aug 2016 21:11:01 +0200
Subject: [PATCH] spi: fsl-espi: eliminate spi nor flash read loop

The fsl-espi driver contains a read loop that implicitely assumes that
the device to read from is a m25p80 SPI NOR flash (bytes 2 - 4 of the
first write transfer are interpreted as 3 byte flash address).

Now that we have such a read loop in the spi-nor driver and are able
to correctly indicate the message size limit of the controller,
the read loop can be removed from the fsl-espi driver.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 drivers/spi/spi-fsl-espi.c | 91 +++++++++++-----------------------------------
 1 file changed, 21 insertions(+), 70 deletions(-)

diff --git a/drivers/spi/spi-fsl-espi.c b/drivers/spi/spi-fsl-espi.c
index 8d85a3c343da..96a2442c1623 100644
--- a/drivers/spi/spi-fsl-espi.c
+++ b/drivers/spi/spi-fsl-espi.c
@@ -258,23 +258,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,68 +349,36 @@ static void fsl_espi_cmd_trans(struct spi_message *m,
 static void fsl_espi_rw_trans(struct spi_message *m,
 				struct fsl_espi_transfer *trans, u8 *rx_buff)
 {
-	struct fsl_espi_transfer *espi_trans = trans;
-	unsigned int total_len = espi_trans->len;
 	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 tx_only = 0;
+	int i = 0;
 
 	local_buf = kzalloc(SPCOM_TRANLEN_MAX, GFP_KERNEL);
 	if (!local_buf) {
-		espi_trans->status = -ENOMEM;
+		trans->status = -ENOMEM;
 		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 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);
-
-		rx_pos += trans_len - tx_only;
+	trans->tx_buf = local_buf;
+	trans->rx_buf = local_buf;
+	fsl_espi_do_trans(m, trans);
 
-		if (loop > 0)
-			espi_trans->actual_length += espi_trans->len - tx_only;
-		else
-			espi_trans->actual_length += espi_trans->len;
+	if (!trans->status) {
+		/* If there is at least one RX byte then copy it to rx_buff */
+		if (trans->len > tx_only)
+			memcpy(rx_buff, trans->rx_buf + tx_only,
+			       trans->len - tx_only);
+		trans->actual_length += trans->len;
 	}
 
 	kfree(local_buf);
@@ -663,7 +614,7 @@ static int fsl_espi_runtime_resume(struct device *dev)
 }
 #endif
 
-static size_t fsl_espi_max_transfer_size(struct spi_device *spi)
+static size_t fsl_espi_max_message_size(struct spi_device *spi)
 {
 	return SPCOM_TRANLEN_MAX;
 }
@@ -695,7 +646,7 @@ static struct spi_master * fsl_espi_probe(struct device *dev,
 	master->cleanup = fsl_espi_cleanup;
 	master->transfer_one_message = fsl_espi_do_one_msg;
 	master->auto_runtime_pm = true;
-	master->max_transfer_size = fsl_espi_max_transfer_size;
+	master->max_message_size = fsl_espi_max_message_size;
 
 	mpc8xxx_spi = spi_master_get_devdata(master);
 
-- 
2.8.1

[PATCH v2 RESEND 2/3] mtd: m25p80: consider max message size in m25p80_read

[Heiner Kallweit]

Consider a message size limit when calculating the maximum amount
of data that can be read.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
v2:
- use the new function spi_max_message_size
---
 drivers/mtd/devices/m25p80.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/mtd/devices/m25p80.c b/drivers/mtd/devices/m25p80.c
index 9cf7fcd..16a7df2 100644
--- a/drivers/mtd/devices/m25p80.c
+++ b/drivers/mtd/devices/m25p80.c
@@ -172,7 +172,8 @@ static ssize_t m25p80_read(struct spi_nor *nor, loff_t from, size_t len,
 
 	t[1].rx_buf = buf;
 	t[1].rx_nbits = m25p80_rx_nbits(nor);
-	t[1].len = min(len, spi_max_transfer_size(spi));
+	t[1].len = min3(len, spi_max_transfer_size(spi),
+			spi_max_message_size(spi) - t[0].len);
 	spi_message_add_tail(&t[1], &m);
 
 	ret = spi_sync(spi, &m);
-- 
2.9.2