[GIT PULL] mtd: spi-nor: Changes for 4.11

[GIT PULL] mtd: spi-nor: Changes for 4.11

[Cyrille Pitchen]

Hi Brian,

this is the spi-nor PR for 4.11. There are few new controller drivers and
support to some memory parts. Otherwise about the spi-nor framework itself,
the main update is the support of the 4-byte address instruction set, which
is stateless, as opposed to the 4-byte address mode, which is stateful hence
creates issues with some bootloaders when spurious reboot occurs.

Best regards,

Cyrille

The following changes since commit 7ce7d89f48834cefece7804d38fc5d85382edf77:

  Linux 4.10-rc1 (2016-12-25 16:13:08 -0800)

are available in the git repository at:

  git://github.com/spi-nor/linux.git tags/spi-nor/for-4.11

for you to fetch changes up to dc12bcccadafb5441170e6b7c8a438c91d4f385b:

  mtd: spi-nor: cqspi: remove redundant dead code on error return check (2017-02-02 10:42:36 +0100)

----------------------------------------------------------------
This pull request contains the following notable changes:
- add support to the 4-byte address instruction set.
- add support to new memory parts.
- add support to S3AN memories.
- add support to the Intel SPI controller.
- add support to the Aspeed AST2400 and AST2550 controllers.
- fix max SPI transfer and message sizes in m25p80_read().
- fix the Candence QSPI driver.
- fix the Freescale QSPI driver.

----------------------------------------------------------------
Colin Ian King (1):
      mtd: spi-nor: cqspi: remove redundant dead code on error return check

Cyrille Pitchen (5):
      mtd: spi-nor: improve macronix_quad_enable()
      mtd: spi-nor: remove WARN_ONCE() message in spi_nor_write()
      mtd: spi-nor: rename SPINOR_OP_* macros of the 4-byte address op codes
      mtd: spi-nor: add a stateless method to support memory size above 128Mib
      Merge tag 'ib-mfd-mtd-v4.11' of git://git.kernel.org/.../lee/mfd

Cédric Le Goater (4):
      mtd: spi-nor: add memory controllers for the Aspeed AST2500 SoC
      mtd: aspeed: add memory controllers for the Aspeed AST2400 SoC
      mtd: spi-nor: bindings for the Aspeed memory controllers
      mtd: aspeed: fix compile warning in aspeed_smc_read_from_ahb()

Guochun Mao (1):
      Documentation: mtk-quadspi: update DT bindings

Heiner Kallweit (1):
      mtd: m25p80: consider max message size in m25p80_read

Kamal Dasu (1):
      mtd: spi-nor: Add support for gd25q16

Marek Vasut (1):
      mtd: spi-nor: cqspi: Fix build on arches missing readsl/writesl

Mika Westerberg (3):
      spi-nor: Add support for Intel SPI serial flash controller
      mfd: lpc_ich: Add support for SPI serial flash host controller
      mfd: lpc_ich: Add support for Intel Apollo Lake SoC

Ricardo Ribalda Delgado (2):
      mtd: spi-nor: Add support for S3AN spi-nor devices
      mtd: spi-nor: Fix S3AN addressing calculation

Uwe Kleine-König (1):
      mtd: spi-nor: add dt support for Everspin MRAMs

Victor Shyba (1):
      mtd: spi-nor: Add lock/unlock support for f25l32pa

Yunhui Cui (2):
      mtd:fsl-quadspi:use the property fields of SPI-NOR
      mtd: fsl-quadspi: Rename SEQID_QUAD_READ to SEQID_READ

 Documentation/devicetree/bindings/mtd/aspeed-smc.txt    |  51 +++++
 Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt |   2 +
 Documentation/devicetree/bindings/mtd/mtk-quadspi.txt   |   8 +-
 Documentation/mtd/intel-spi.txt                         |  88 +++++++++
 drivers/mfd/lpc_ich.c                                   | 131 +++++++++++++
 drivers/mtd/devices/m25p80.c                            |   9 +-
 drivers/mtd/devices/serial_flash_cmds.h                 |   7 -
 drivers/mtd/devices/st_spi_fsm.c                        |  28 +--
 drivers/mtd/spi-nor/Kconfig                             |  32 +++-
 drivers/mtd/spi-nor/Makefile                            |   3 +
 drivers/mtd/spi-nor/aspeed-smc.c                        | 758 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 drivers/mtd/spi-nor/cadence-quadspi.c                   |  10 +-
 drivers/mtd/spi-nor/fsl-quadspi.c                       |  48 ++---
 drivers/mtd/spi-nor/intel-spi-platform.c                |  57 ++++++
 drivers/mtd/spi-nor/intel-spi.c                         | 777 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 drivers/mtd/spi-nor/intel-spi.h                         |  24 +++
 drivers/mtd/spi-nor/spi-nor.c                           | 271 ++++++++++++++++++++++++---
 drivers/spi/spi-bcm-qspi.c                              |   6 +-
 include/linux/mfd/lpc_ich.h                             |   3 +
 include/linux/mtd/spi-nor.h                             |  34 +++-
 include/linux/platform_data/intel-spi.h                 |  31 +++
 21 files changed, 2278 insertions(+), 100 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/mtd/aspeed-smc.txt
 create mode 100644 Documentation/mtd/intel-spi.txt
 create mode 100644 drivers/mtd/spi-nor/aspeed-smc.c
 create mode 100644 drivers/mtd/spi-nor/intel-spi-platform.c
 create mode 100644 drivers/mtd/spi-nor/intel-spi.c
 create mode 100644 drivers/mtd/spi-nor/intel-spi.h
 create mode 100644 include/linux/platform_data/intel-spi.h

Re: [GIT PULL] mtd: spi-nor: Changes for 4.11

[Marek Vasut]

On 02/07/2017 04:31 PM, Cyrille Pitchen wrote:
> Hi Brian,
> 
> this is the spi-nor PR for 4.11. There are few new controller drivers and
> support to some memory parts. Otherwise about the spi-nor framework itself,
> the main update is the support of the 4-byte address instruction set, which
> is stateless, as opposed to the 4-byte address mode, which is stateful hence
> creates issues with some bootloaders when spurious reboot occurs.
> 
> Best regards,
> 
> Cyrille
> 
> The following changes since commit 7ce7d89f48834cefece7804d38fc5d85382edf77:
> 
>   Linux 4.10-rc1 (2016-12-25 16:13:08 -0800)
> 
> are available in the git repository at:
> 
>   git://github.com/spi-nor/linux.git tags/spi-nor/for-4.11
> 
> for you to fetch changes up to dc12bcccadafb5441170e6b7c8a438c91d4f385b:
> 
>   mtd: spi-nor: cqspi: remove redundant dead code on error return check (2017-02-02 10:42:36 +0100)
> 
> ----------------------------------------------------------------
> This pull request contains the following notable changes:
> - add support to the 4-byte address instruction set.
> - add support to new memory parts.
> - add support to S3AN memories.
> - add support to the Intel SPI controller.
> - add support to the Aspeed AST2400 and AST2550 controllers.
> - fix max SPI transfer and message sizes in m25p80_read().
> - fix the Candence QSPI driver.
> - fix the Freescale QSPI driver.
> 
> ----------------------------------------------------------------
> Colin Ian King (1):
>       mtd: spi-nor: cqspi: remove redundant dead code on error return check
> 
> Cyrille Pitchen (5):
>       mtd: spi-nor: improve macronix_quad_enable()
>       mtd: spi-nor: remove WARN_ONCE() message in spi_nor_write()
>       mtd: spi-nor: rename SPINOR_OP_* macros of the 4-byte address op codes
>       mtd: spi-nor: add a stateless method to support memory size above 128Mib
>       Merge tag 'ib-mfd-mtd-v4.11' of git://git.kernel.org/.../lee/mfd
> 
> Cédric Le Goater (4):
>       mtd: spi-nor: add memory controllers for the Aspeed AST2500 SoC
>       mtd: aspeed: add memory controllers for the Aspeed AST2400 SoC
>       mtd: spi-nor: bindings for the Aspeed memory controllers
>       mtd: aspeed: fix compile warning in aspeed_smc_read_from_ahb()
> 
> Guochun Mao (1):
>       Documentation: mtk-quadspi: update DT bindings
> 
> Heiner Kallweit (1):
>       mtd: m25p80: consider max message size in m25p80_read

I cannot say I'm super-happy about this particular patch, still ...

> Kamal Dasu (1):
>       mtd: spi-nor: Add support for gd25q16
> 
> Marek Vasut (1):
>       mtd: spi-nor: cqspi: Fix build on arches missing readsl/writesl
> 
> Mika Westerberg (3):
>       spi-nor: Add support for Intel SPI serial flash controller
>       mfd: lpc_ich: Add support for SPI serial flash host controller
>       mfd: lpc_ich: Add support for Intel Apollo Lake SoC
> 
> Ricardo Ribalda Delgado (2):
>       mtd: spi-nor: Add support for S3AN spi-nor devices
>       mtd: spi-nor: Fix S3AN addressing calculation
> 
> Uwe Kleine-König (1):
>       mtd: spi-nor: add dt support for Everspin MRAMs
> 
> Victor Shyba (1):
>       mtd: spi-nor: Add lock/unlock support for f25l32pa
> 
> Yunhui Cui (2):
>       mtd:fsl-quadspi:use the property fields of SPI-NOR
>       mtd: fsl-quadspi: Rename SEQID_QUAD_READ to SEQID_READ
> 
>  Documentation/devicetree/bindings/mtd/aspeed-smc.txt    |  51 +++++
>  Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt |   2 +
>  Documentation/devicetree/bindings/mtd/mtk-quadspi.txt   |   8 +-
>  Documentation/mtd/intel-spi.txt                         |  88 +++++++++
>  drivers/mfd/lpc_ich.c                                   | 131 +++++++++++++
>  drivers/mtd/devices/m25p80.c                            |   9 +-
>  drivers/mtd/devices/serial_flash_cmds.h                 |   7 -
>  drivers/mtd/devices/st_spi_fsm.c                        |  28 +--
>  drivers/mtd/spi-nor/Kconfig                             |  32 +++-
>  drivers/mtd/spi-nor/Makefile                            |   3 +
>  drivers/mtd/spi-nor/aspeed-smc.c                        | 758 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  drivers/mtd/spi-nor/cadence-quadspi.c                   |  10 +-
>  drivers/mtd/spi-nor/fsl-quadspi.c                       |  48 ++---
>  drivers/mtd/spi-nor/intel-spi-platform.c                |  57 ++++++
>  drivers/mtd/spi-nor/intel-spi.c                         | 777 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  drivers/mtd/spi-nor/intel-spi.h                         |  24 +++
>  drivers/mtd/spi-nor/spi-nor.c                           | 271 ++++++++++++++++++++++++---
>  drivers/spi/spi-bcm-qspi.c                              |   6 +-
>  include/linux/mfd/lpc_ich.h                             |   3 +
>  include/linux/mtd/spi-nor.h                             |  34 +++-
>  include/linux/platform_data/intel-spi.h                 |  31 +++
>  21 files changed, 2278 insertions(+), 100 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/mtd/aspeed-smc.txt
>  create mode 100644 Documentation/mtd/intel-spi.txt
>  create mode 100644 drivers/mtd/spi-nor/aspeed-smc.c
>  create mode 100644 drivers/mtd/spi-nor/intel-spi-platform.c
>  create mode 100644 drivers/mtd/spi-nor/intel-spi.c
>  create mode 100644 drivers/mtd/spi-nor/intel-spi.h
>  create mode 100644 include/linux/platform_data/intel-spi.h
> 


-- 
Best regards,
Marek Vasut

Re: [GIT PULL] mtd: spi-nor: Changes for 4.11

[Heiner Kallweit]

Am 08.02.2017 um 02:09 schrieb Marek Vasut:
> On 02/07/2017 04:31 PM, Cyrille Pitchen wrote:
>> Hi Brian,
>>
>> this is the spi-nor PR for 4.11. There are few new controller drivers and
>> support to some memory parts. Otherwise about the spi-nor framework itself,
>> the main update is the support of the 4-byte address instruction set, which
>> is stateless, as opposed to the 4-byte address mode, which is stateful hence
>> creates issues with some bootloaders when spurious reboot occurs.
>>
>> Best regards,
>>
>> Cyrille
>>
>> The following changes since commit 7ce7d89f48834cefece7804d38fc5d85382edf77:
>>
>>   Linux 4.10-rc1 (2016-12-25 16:13:08 -0800)
>>
>> are available in the git repository at:
>>
>>   git://github.com/spi-nor/linux.git tags/spi-nor/for-4.11
>>
>> for you to fetch changes up to dc12bcccadafb5441170e6b7c8a438c91d4f385b:
>>
>>   mtd: spi-nor: cqspi: remove redundant dead code on error return check (2017-02-02 10:42:36 +0100)
>>
>> ----------------------------------------------------------------
>> This pull request contains the following notable changes:
>> - add support to the 4-byte address instruction set.
>> - add support to new memory parts.
>> - add support to S3AN memories.
>> - add support to the Intel SPI controller.
>> - add support to the Aspeed AST2400 and AST2550 controllers.
>> - fix max SPI transfer and message sizes in m25p80_read().
>> - fix the Candence QSPI driver.
>> - fix the Freescale QSPI driver.
>>
>> ----------------------------------------------------------------
>> Colin Ian King (1):
>>       mtd: spi-nor: cqspi: remove redundant dead code on error return check
>>
>> Cyrille Pitchen (5):
>>       mtd: spi-nor: improve macronix_quad_enable()
>>       mtd: spi-nor: remove WARN_ONCE() message in spi_nor_write()
>>       mtd: spi-nor: rename SPINOR_OP_* macros of the 4-byte address op codes
>>       mtd: spi-nor: add a stateless method to support memory size above 128Mib
>>       Merge tag 'ib-mfd-mtd-v4.11' of git://git.kernel.org/.../lee/mfd
>>
>> Cédric Le Goater (4):
>>       mtd: spi-nor: add memory controllers for the Aspeed AST2500 SoC
>>       mtd: aspeed: add memory controllers for the Aspeed AST2400 SoC
>>       mtd: spi-nor: bindings for the Aspeed memory controllers
>>       mtd: aspeed: fix compile warning in aspeed_smc_read_from_ahb()
>>
>> Guochun Mao (1):
>>       Documentation: mtk-quadspi: update DT bindings
>>
>> Heiner Kallweit (1):
>>       mtd: m25p80: consider max message size in m25p80_read
> 
> I cannot say I'm super-happy about this particular patch, still ...
> 
W/o it at least the fsl-espi driver is broken for transfers >= 64kB.

>> Kamal Dasu (1):
>>       mtd: spi-nor: Add support for gd25q16
>>
>> Marek Vasut (1):
>>       mtd: spi-nor: cqspi: Fix build on arches missing readsl/writesl
>>
>> Mika Westerberg (3):
>>       spi-nor: Add support for Intel SPI serial flash controller
>>       mfd: lpc_ich: Add support for SPI serial flash host controller
>>       mfd: lpc_ich: Add support for Intel Apollo Lake SoC
>>
>> Ricardo Ribalda Delgado (2):
>>       mtd: spi-nor: Add support for S3AN spi-nor devices
>>       mtd: spi-nor: Fix S3AN addressing calculation
>>
>> Uwe Kleine-König (1):
>>       mtd: spi-nor: add dt support for Everspin MRAMs
>>
>> Victor Shyba (1):
>>       mtd: spi-nor: Add lock/unlock support for f25l32pa
>>
>> Yunhui Cui (2):
>>       mtd:fsl-quadspi:use the property fields of SPI-NOR
>>       mtd: fsl-quadspi: Rename SEQID_QUAD_READ to SEQID_READ
>>
>>  Documentation/devicetree/bindings/mtd/aspeed-smc.txt    |  51 +++++
>>  Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt |   2 +
>>  Documentation/devicetree/bindings/mtd/mtk-quadspi.txt   |   8 +-
>>  Documentation/mtd/intel-spi.txt                         |  88 +++++++++
>>  drivers/mfd/lpc_ich.c                                   | 131 +++++++++++++
>>  drivers/mtd/devices/m25p80.c                            |   9 +-
>>  drivers/mtd/devices/serial_flash_cmds.h                 |   7 -
>>  drivers/mtd/devices/st_spi_fsm.c                        |  28 +--
>>  drivers/mtd/spi-nor/Kconfig                             |  32 +++-
>>  drivers/mtd/spi-nor/Makefile                            |   3 +
>>  drivers/mtd/spi-nor/aspeed-smc.c                        | 758 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>  drivers/mtd/spi-nor/cadence-quadspi.c                   |  10 +-
>>  drivers/mtd/spi-nor/fsl-quadspi.c                       |  48 ++---
>>  drivers/mtd/spi-nor/intel-spi-platform.c                |  57 ++++++
>>  drivers/mtd/spi-nor/intel-spi.c                         | 777 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>  drivers/mtd/spi-nor/intel-spi.h                         |  24 +++
>>  drivers/mtd/spi-nor/spi-nor.c                           | 271 ++++++++++++++++++++++++---
>>  drivers/spi/spi-bcm-qspi.c                              |   6 +-
>>  include/linux/mfd/lpc_ich.h                             |   3 +
>>  include/linux/mtd/spi-nor.h                             |  34 +++-
>>  include/linux/platform_data/intel-spi.h                 |  31 +++
>>  21 files changed, 2278 insertions(+), 100 deletions(-)
>>  create mode 100644 Documentation/devicetree/bindings/mtd/aspeed-smc.txt
>>  create mode 100644 Documentation/mtd/intel-spi.txt
>>  create mode 100644 drivers/mtd/spi-nor/aspeed-smc.c
>>  create mode 100644 drivers/mtd/spi-nor/intel-spi-platform.c
>>  create mode 100644 drivers/mtd/spi-nor/intel-spi.c
>>  create mode 100644 drivers/mtd/spi-nor/intel-spi.h
>>  create mode 100644 include/linux/platform_data/intel-spi.h
>>
> 
> 

Re: [GIT PULL] mtd: spi-nor: Changes for 4.11

[Cyrille Pitchen]

Hi all,

Le 08/02/2017 à 06:52, Heiner Kallweit a écrit :
> Am 08.02.2017 um 02:09 schrieb Marek Vasut:
>> On 02/07/2017 04:31 PM, Cyrille Pitchen wrote:
[...]
>>> Heiner Kallweit (1):
>>>       mtd: m25p80: consider max message size in m25p80_read
>>
>> I cannot say I'm super-happy about this particular patch, still ...
>>
> W/o it at least the fsl-espi driver is broken for transfers >= 64kB.
> 

We asked Mark Brown about the need to check the limits for the size of SPI
transfers and messages and I've understood his answer correctly, he told us
we should check those limits:

http://patchwork.ozlabs.org/patch/688176/

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

Then the discussion ended there. I didn't see any comment after that so I
just thought the discussion was over and everybody agreed.


Besides, this patch doesn't introduce the check of SPI controller limits
but only extend this check. What I mean is that the m25p80 driver already
checks the maximum transfer size. This check was introduced by commit

95193796256c ("mtd: m25p80: read in spi_max_transfer_size chunks")

Then if we already check the SPI max transfer size, I guess it make sense
to also test other limits such as the SPI max message size.

Finally, I think it would be very difficult and dirty to try to fix this
bug only from the SPI controller driver. If m25p80_read() tries to read too
many data based on what the SPI controller can actually read in a single
operation, we would have to split the (Fast) Read command into two (Fast)
Read commands, incrementing the address bytes as needed before sending the
second command to the SPI bus.

Please note that this split is already handled by spi_nor_read() when
nor->read() returns a size lower than its len parameter.

However, if we try to fix the issue at the SPI controller driver side, once
we start to use the SPI API with spi_transfer and spi_message structures,
we loose protocol info. SPI transfers and messages only carry stream of
data/bytes, hence we loose the SPI flash protocol split: instruction |
address | mode | dummy | data.

When using the regular SPI API, the SPI controller driver should not try to
recover that protocol split. It should even not be aware that the SPI
device is indeed a SPI flash.
Also there is no mean to recover the protocol split out without duplicating
many parts of the spi-nor framework:
- 3-byte or 4-byte address ?
- how many dummy cycle bytes ? => this number depends on both the
instruction op code and the memory manufacturer.

Also I don't think it's a good think when SPI controller drivers by-pass
or duplicate the selection of the right flash parameters, op codes and so
on. When they do so, they do it so it works with very specific memory parts
and they just don't care about some other memory parts that work slightly
differently. Then it becomes a mess to maintain such drivers.


With Heiner's patch, the issue is fixed by changing only one line and the
bandwidth performance of read operations should not be impacted.

I don't think his solution is that dirty as compared to the only other
proposed solution: fix it in the SPI controller driver.

The SPI controller driver could also implement its own version of
spi_flash_read() mainly duplicating the code of m25p80_read() but it
doesn't sound like a cleaner fix, IMHO.


Best regards,

Cyrille

Re: [GIT PULL] mtd: spi-nor: Changes for 4.11

[Brian Norris]

+ Mark, as the disagreement touches his area

Hi,

On Wed, Feb 08, 2017 at 11:36:10AM +0100, Cyrille Pitchen wrote:
> Le 08/02/2017 à 06:52, Heiner Kallweit a écrit :
> > Am 08.02.2017 um 02:09 schrieb Marek Vasut:
> >> On 02/07/2017 04:31 PM, Cyrille Pitchen wrote:
> [...]
> >>> Heiner Kallweit (1):
> >>>       mtd: m25p80: consider max message size in m25p80_read
> >>
> >> I cannot say I'm super-happy about this particular patch, still ...
> >>
> > W/o it at least the fsl-espi driver is broken for transfers >= 64kB.
> > 
> 
> We asked Mark Brown about the need to check the limits for the size of SPI
> transfers and messages and I've understood his answer correctly, he told us
> we should check those limits:
> 
> http://patchwork.ozlabs.org/patch/688176/
> 
> "> 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"
> 
> Then the discussion ended there. I didn't see any comment after that so I
> just thought the discussion was over and everybody agreed.

I thought we more or less settled on this a while ago. But I could be
wrong.

> Besides, this patch doesn't introduce the check of SPI controller limits
> but only extend this check. What I mean is that the m25p80 driver already
> checks the maximum transfer size. This check was introduced by commit
> 
> 95193796256c ("mtd: m25p80: read in spi_max_transfer_size chunks")
> 
> Then if we already check the SPI max transfer size, I guess it make sense
> to also test other limits such as the SPI max message size.

Yes, if this patch is wrong, then I'd expect commit 95193796256c was
also wrong.

The central disagreement seems to be on the existence of upper limits
for transfer and/or message size. Marek seemed to suggest elsewhere that
there is usually no upper limit (or that "upper limits" are usually just
driver bugs that are being worked around). I don't disagree that often
these might just be driver bugs, but I *do* disagree that there are
never limits. I think Mark (? or somebody else, not reading too closely)
suggested that there are just few drivers that tickle the limits.

One example that shows both sides of this coin:

5185a81c02d4 "spi: rockchip: limit transfers to (64K - 1) bytes"

The hardware has a hard limit on transfers, at least, which can only be
64K. For some reason there's an off-by-one error (probably a driver
bug?), so I cheated and made it one smaller.

[...]

(snip the rest about trying to split spi_messages within the SPI driver;
I agree that would be awful.)

But one question: *why* exactly is the transfer or message limit so
fixed? If a SPI controller allows manual control over the CS line, then
why would anybody care if a sequence of data gets split up into multiple
HW sequences (e.g., for a long spi_transfer, why can't we split it into
2 sub-transfers, while keeping the CS asserted?).

For the above rockchip SPI controller, AIUI we could actually support
the "transfer limit" by doing this. And in fact, it could probably be
done within the SPI core code, instead either the SPI driver or the MTD
driver.

Now, in this case, *transfer* vs. *message* seems to have a difference,
in whether or not you have guarantees about the CS line. The CS must
stay asserted (barring the 'cs_change' flag) between transfers, but not
between messages.

So the one thing I can see that might support Marek's disagreement here:
does the FSL ESPI hardware support controlling the CS line? It seems
like currently, the driver just uses a few fixed settings for its CSMODE
register fields, whereas it might be able to support ->transfer_one()
(and flexible CS assertion, and splitting large transactions into
smaller chunks) if you handled that differently...

Heiner (or others), any thoughts on the above? I might be very mistaken,
especially since I'm not intimately familiar with a lot of SPI drivers.

Regardless, I think this patch is OK, given the established practice,
and we should take it. We might want to push back to handle this better
in the SPI layer, if at all possible.

One thing I do object to: why is this change marked for stable? It's not
our fault you broke your duct-taped-together SPI driver in 4.9. I don't
think backporting features is the way to satisfy the -stable
requirements. Maybe some reverts would be the right way...

Brian

Re: [GIT PULL] mtd: spi-nor: Changes for 4.11

[Mark Brown]

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

On Wed, Feb 08, 2017 at 11:09:39AM -0800, Brian Norris wrote:

> But one question: *why* exactly is the transfer or message limit so
> fixed? If a SPI controller allows manual control over the CS line, then
> why would anybody care if a sequence of data gets split up into multiple
> HW sequences (e.g., for a long spi_transfer, why can't we split it into
> 2 sub-transfers, while keeping the CS asserted?).

Not every device will provide software control of the chip select line
so it's possible you might hit limits there.  If a device does allow
software control then yes, we should be handling this transparently in
the core and splitting up longer transfers into chunks the device can
handle and in fact this is already done in spi_split_transfers_maxsize(). 

We're missing masking that out in spi_max_transfer_size() though so
clients won't know if we can handle things, it's overly conservative.

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

Re: [GIT PULL] mtd: spi-nor: Changes for 4.11

[Brian Norris]

On Wed, Feb 08, 2017 at 07:22:36PM +0000, Mark Brown wrote:
> On Wed, Feb 08, 2017 at 11:09:39AM -0800, Brian Norris wrote:
> 
> > But one question: *why* exactly is the transfer or message limit so
> > fixed? If a SPI controller allows manual control over the CS line, then
> > why would anybody care if a sequence of data gets split up into multiple
> > HW sequences (e.g., for a long spi_transfer, why can't we split it into
> > 2 sub-transfers, while keeping the CS asserted?).
> 
> Not every device will provide software control of the chip select line
> so it's possible you might hit limits there.  If a device does allow
> software control then yes, we should be handling this transparently in
> the core and splitting up longer transfers into chunks the device can
> handle

Thanks for the info. Then it sounds like that really should be the
dividing line; driver-visible limitations ideally should only happen
when CS is not software-controllable. (As Heiner points out, it seems
FSL ESPI is one such case.) All other cases can be handled by some form
of splitting, in the SPI core.

So that actually sounds like drivers like m25p80 should never have to
account for spi_max_transfer_size(), but instead should have to account
for only spi_max_message_size().

But that's all only a "should" -- nobody has done the work to do this
properly, and so right now, we need to check for both.

> and in fact this is already done in spi_split_transfers_maxsize(). 
> 
> We're missing masking that out in spi_max_transfer_size() though so
> clients won't know if we can handle things, it's overly conservative.

Huh, I've never noticed spi_split_transfers_maxsize(). That looks like
work for it got dropped on the floor somewhere though; nobody is using
it yet, and it's over a year since its introduction.

Brian

Re: [GIT PULL] mtd: spi-nor: Changes for 4.11

[Mark Brown]

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

On Wed, Feb 08, 2017 at 12:39:22PM -0800, Brian Norris wrote:

> So that actually sounds like drivers like m25p80 should never have to
> account for spi_max_transfer_size(), but instead should have to account
> for only spi_max_message_size().

You might have both depending on how the hardware is structured, some
things support multiple transfers while controlling the chip select
internally.

> > and in fact this is already done in spi_split_transfers_maxsize(). 

> > We're missing masking that out in spi_max_transfer_size() though so
> > clients won't know if we can handle things, it's overly conservative.

> Huh, I've never noticed spi_split_transfers_maxsize(). That looks like
> work for it got dropped on the floor somewhere though; nobody is using
> it yet, and it's over a year since its introduction.

Yeah, the optimisation for rpi ground to a halt.  We should probably
just unconditionally do that in the core if something declares a maximum
transfer size.

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

Re: [GIT PULL] mtd: spi-nor: Changes for 4.11

[Michal Suchanek]

On 8 February 2017 at 20:09, Brian Norris <computersforpeace@gmail.com> wrote:
> + Mark, as the disagreement touches his area
>
> Hi,
>
> On Wed, Feb 08, 2017 at 11:36:10AM +0100, Cyrille Pitchen wrote:
>> Le 08/02/2017 à 06:52, Heiner Kallweit a écrit :
>> > Am 08.02.2017 um 02:09 schrieb Marek Vasut:
>> >> On 02/07/2017 04:31 PM, Cyrille Pitchen wrote:
>> [...]
>> >>> Heiner Kallweit (1):
>> >>>       mtd: m25p80: consider max message size in m25p80_read
[...]
> One example that shows both sides of this coin:
>
> 5185a81c02d4 "spi: rockchip: limit transfers to (64K - 1) bytes"
>
> The hardware has a hard limit on transfers, at least, which can only be
> 64K. For some reason there's an off-by-one error (probably a driver
> bug?), so I cheated and made it one smaller.
>
> [...]
>
> (snip the rest about trying to split spi_messages within the SPI driver;
> I agree that would be awful.)
>
> But one question: *why* exactly is the transfer or message limit so
> fixed? If a SPI controller allows manual control over the CS line, then
> why would anybody care if a sequence of data gets split up into multiple
> HW sequences (e.g., for a long spi_transfer, why can't we split it into
> 2 sub-transfers, while keeping the CS asserted?).
>
> For the above rockchip SPI controller, AIUI we could actually support
> the "transfer limit" by doing this. And in fact, it could probably be
> done within the SPI core code, instead either the SPI driver or the MTD
> driver.

Yes, this is something repeatedly open-coded in SPI drivers, unfortunately.

In this case limiting the transfer size to 64k rather than doing
another clone of the manual CS handling actually looks reasonable.
It's not like anything but mtd_debug normally triggers that limit.

>
> Now, in this case, *transfer* vs. *message* seems to have a difference,
> in whether or not you have guarantees about the CS line. The CS must
> stay asserted (barring the 'cs_change' flag) between transfers, but not
> between messages.
>
> So the one thing I can see that might support Marek's disagreement here:
> does the FSL ESPI hardware support controlling the CS line? It seems
> like currently, the driver just uses a few fixed settings for its CSMODE
> register fields, whereas it might be able to support ->transfer_one()
> (and flexible CS assertion, and splitting large transactions into
> smaller chunks) if you handled that differently...

It was argued that while this is the case with most SPI controllers it
is not the case with FSL ESPI. I do not recall anybody producing any
datasheet/manual supporting this, though. It's not like this is a
confidential piece of hardware, right?

Thanks

Michal

Re: [GIT PULL] mtd: spi-nor: Changes for 4.11

[Heiner Kallweit]

Am 08.02.2017 um 20:40 schrieb Michal Suchanek:
> On 8 February 2017 at 20:09, Brian Norris <computersforpeace@gmail.com> wrote:
>> + Mark, as the disagreement touches his area
>>
>> Hi,
>>
>> On Wed, Feb 08, 2017 at 11:36:10AM +0100, Cyrille Pitchen wrote:
>>> Le 08/02/2017 à 06:52, Heiner Kallweit a écrit :
>>>> Am 08.02.2017 um 02:09 schrieb Marek Vasut:
>>>>> On 02/07/2017 04:31 PM, Cyrille Pitchen wrote:
>>> [...]
>>>>>> Heiner Kallweit (1):
>>>>>>       mtd: m25p80: consider max message size in m25p80_read
> [...]
>> One example that shows both sides of this coin:
>>
>> 5185a81c02d4 "spi: rockchip: limit transfers to (64K - 1) bytes"
>>
>> The hardware has a hard limit on transfers, at least, which can only be
>> 64K. For some reason there's an off-by-one error (probably a driver
>> bug?), so I cheated and made it one smaller.
>>
>> [...]
>>
>> (snip the rest about trying to split spi_messages within the SPI driver;
>> I agree that would be awful.)
>>
>> But one question: *why* exactly is the transfer or message limit so
>> fixed? If a SPI controller allows manual control over the CS line, then
>> why would anybody care if a sequence of data gets split up into multiple
>> HW sequences (e.g., for a long spi_transfer, why can't we split it into
>> 2 sub-transfers, while keeping the CS asserted?).
>>
>> For the above rockchip SPI controller, AIUI we could actually support
>> the "transfer limit" by doing this. And in fact, it could probably be
>> done within the SPI core code, instead either the SPI driver or the MTD
>> driver.
> 
> Yes, this is something repeatedly open-coded in SPI drivers, unfortunately.
> 
> In this case limiting the transfer size to 64k rather than doing
> another clone of the manual CS handling actually looks reasonable.
> It's not like anything but mtd_debug normally triggers that limit.
> 
>>
>> Now, in this case, *transfer* vs. *message* seems to have a difference,
>> in whether or not you have guarantees about the CS line. The CS must
>> stay asserted (barring the 'cs_change' flag) between transfers, but not
>> between messages.
>>
>> So the one thing I can see that might support Marek's disagreement here:
>> does the FSL ESPI hardware support controlling the CS line? It seems
>> like currently, the driver just uses a few fixed settings for its CSMODE
>> register fields, whereas it might be able to support ->transfer_one()
>> (and flexible CS assertion, and splitting large transactions into
>> smaller chunks) if you handled that differently...
> 
> It was argued that while this is the case with most SPI controllers it
> is not the case with FSL ESPI. I do not recall anybody producing any
> datasheet/manual supporting this, though. It's not like this is a
> confidential piece of hardware, right?
> 

The datasheet (here called Reference Manual) is available on the NXP
(ex Freescale) website, you just have to register.

http://www.nxp.com/products/microcontrollers-and-processors/power-architecture-processors/qoriq-platforms/p-series/qoriq-p1010-and-p1014-low-power-communications-processors-with-trust-architecture:P1010?tab=Documentation_Tab

> Thanks
> 
> Michal
> 

Re: [GIT PULL] mtd: spi-nor: Changes for 4.11

[Mark Brown]

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

On Wed, Feb 08, 2017 at 08:40:17PM +0100, Michal Suchanek wrote:

> Yes, this is something repeatedly open-coded in SPI drivers, unfortunately.

> In this case limiting the transfer size to 64k rather than doing
> another clone of the manual CS handling actually looks reasonable.
> It's not like anything but mtd_debug normally triggers that limit.

Any driver that's open coding this is buggy and should be fixed, it
should mainly be legacy drivers - some of the older drivers have a lot
of problems.

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

Re: [GIT PULL] mtd: spi-nor: Changes for 4.11

[Heiner Kallweit]

Am 08.02.2017 um 20:09 schrieb Brian Norris:
> + Mark, as the disagreement touches his area
> 
> Hi,
> 
> On Wed, Feb 08, 2017 at 11:36:10AM +0100, Cyrille Pitchen wrote:
>> Le 08/02/2017 à 06:52, Heiner Kallweit a écrit :
>>> Am 08.02.2017 um 02:09 schrieb Marek Vasut:
>>>> On 02/07/2017 04:31 PM, Cyrille Pitchen wrote:
>> [...]
>>>>> Heiner Kallweit (1):
>>>>>       mtd: m25p80: consider max message size in m25p80_read
>>>>
>>>> I cannot say I'm super-happy about this particular patch, still ...
>>>>
>>> W/o it at least the fsl-espi driver is broken for transfers >= 64kB.
>>>
>>
>> We asked Mark Brown about the need to check the limits for the size of SPI
>> transfers and messages and I've understood his answer correctly, he told us
>> we should check those limits:
>>
>> http://patchwork.ozlabs.org/patch/688176/
>>
>> "> 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"
>>
>> Then the discussion ended there. I didn't see any comment after that so I
>> just thought the discussion was over and everybody agreed.
> 
> I thought we more or less settled on this a while ago. But I could be
> wrong.
> 
>> Besides, this patch doesn't introduce the check of SPI controller limits
>> but only extend this check. What I mean is that the m25p80 driver already
>> checks the maximum transfer size. This check was introduced by commit
>>
>> 95193796256c ("mtd: m25p80: read in spi_max_transfer_size chunks")
>>
>> Then if we already check the SPI max transfer size, I guess it make sense
>> to also test other limits such as the SPI max message size.
> 
> Yes, if this patch is wrong, then I'd expect commit 95193796256c was
> also wrong.
> 
> The central disagreement seems to be on the existence of upper limits
> for transfer and/or message size. Marek seemed to suggest elsewhere that
> there is usually no upper limit (or that "upper limits" are usually just
> driver bugs that are being worked around). I don't disagree that often
> these might just be driver bugs, but I *do* disagree that there are
> never limits. I think Mark (? or somebody else, not reading too closely)
> suggested that there are just few drivers that tickle the limits.
> 
> One example that shows both sides of this coin:
> 
> 5185a81c02d4 "spi: rockchip: limit transfers to (64K - 1) bytes"
> 
> The hardware has a hard limit on transfers, at least, which can only be
> 64K. For some reason there's an off-by-one error (probably a driver
> bug?), so I cheated and made it one smaller.
> 
> [...]
> 
> (snip the rest about trying to split spi_messages within the SPI driver;
> I agree that would be awful.)
> 
> But one question: *why* exactly is the transfer or message limit so
> fixed? If a SPI controller allows manual control over the CS line, then
> why would anybody care if a sequence of data gets split up into multiple
> HW sequences (e.g., for a long spi_transfer, why can't we split it into
> 2 sub-transfers, while keeping the CS asserted?).
> 
> For the above rockchip SPI controller, AIUI we could actually support
> the "transfer limit" by doing this. And in fact, it could probably be
> done within the SPI core code, instead either the SPI driver or the MTD
> driver.
> 
> Now, in this case, *transfer* vs. *message* seems to have a difference,
> in whether or not you have guarantees about the CS line. The CS must
> stay asserted (barring the 'cs_change' flag) between transfers, but not
> between messages.
> 
> So the one thing I can see that might support Marek's disagreement here:
> does the FSL ESPI hardware support controlling the CS line? It seems
> like currently, the driver just uses a few fixed settings for its CSMODE
> register fields, whereas it might be able to support ->transfer_one()
> (and flexible CS assertion, and splitting large transactions into
> smaller chunks) if you handled that differently...
> 
> Heiner (or others), any thoughts on the above? I might be very mistaken,
> especially since I'm not intimately familiar with a lot of SPI drivers.
> 
FSL ESPI has hardware-controlled CS. You program the length of transfer
(max. 64K) and when starting the transfer HW will assert CS and after
having transferred the programmed number of bytes CS is deasserted
internally.

And because of course this question came up:
No, you can't use pinmux to turn the CS line into a GPIO. It's only
possible to change the complete block of SPI signal to GPIOs.
But then we would end up with bitbanging, loose the HW FIFO etc.

Heiner

> Regardless, I think this patch is OK, given the established practice,
> and we should take it. We might want to push back to handle this better
> in the SPI layer, if at all possible.
> 
> One thing I do object to: why is this change marked for stable? It's not
> our fault you broke your duct-taped-together SPI driver in 4.9. I don't
> think backporting features is the way to satisfy the -stable
> requirements. Maybe some reverts would be the right way...
> 
> Brian
> 

Re: [GIT PULL] mtd: spi-nor: Changes for 4.11

[Brian Norris]

On Wed, Feb 08, 2017 at 11:09:39AM -0800, Brian Norris wrote:
> On Wed, Feb 08, 2017 at 11:36:10AM +0100, Cyrille Pitchen wrote:
> > Le 08/02/2017 à 06:52, Heiner Kallweit a écrit :
> > > Am 08.02.2017 um 02:09 schrieb Marek Vasut:
> > >> On 02/07/2017 04:31 PM, Cyrille Pitchen wrote:
> > [...]
> > >>> Heiner Kallweit (1):
> > >>>       mtd: m25p80: consider max message size in m25p80_read
> > >>
> > >> I cannot say I'm super-happy about this particular patch, still ...
> > >>
> > > W/o it at least the fsl-espi driver is broken for transfers >= 64kB.
> > > 
[...]

> One thing I do object to: why is this change marked for stable? It's not
> our fault you broke your duct-taped-together SPI driver in 4.9. I don't
> think backporting features is the way to satisfy the -stable
> requirements. Maybe some reverts would be the right way...

OK, I've reviewed the rest of the pull request as well as I care for.
I still have this one objection: I don't think this belongs in -stable.

Am I crazy? Or else Cyrille, can you remove the stable tag and resend?
Or I can go rewrite your history for you and do this...

Brian

P.S. Also, this stuck out at me in the diffs, and checkpatch complained
too. Not a real big deal:

WARNING:BLOCK_COMMENT_STYLE: Block comments should align the * on each
line
#42: FILE: drivers/mtd/spi-nor/spi-nor.c:83:
+					 * ATMEL flashes)
+					*/

total: 0 errors, 1 warnings, 264 lines checked

Re: [GIT PULL] mtd: spi-nor: Changes for 4.11

[Heiner Kallweit]

Am 10.02.2017 um 03:46 schrieb Brian Norris:
> On Wed, Feb 08, 2017 at 11:09:39AM -0800, Brian Norris wrote:
>> On Wed, Feb 08, 2017 at 11:36:10AM +0100, Cyrille Pitchen wrote:
>>> Le 08/02/2017 à 06:52, Heiner Kallweit a écrit :
>>>> Am 08.02.2017 um 02:09 schrieb Marek Vasut:
>>>>> On 02/07/2017 04:31 PM, Cyrille Pitchen wrote:
>>> [...]
>>>>>> Heiner Kallweit (1):
>>>>>>       mtd: m25p80: consider max message size in m25p80_read
>>>>>
>>>>> I cannot say I'm super-happy about this particular patch, still ...
>>>>>
>>>> W/o it at least the fsl-espi driver is broken for transfers >= 64kB.
>>>>
> [...]
> 
>> One thing I do object to: why is this change marked for stable? It's not
>> our fault you broke your duct-taped-together SPI driver in 4.9. I don't
>> think backporting features is the way to satisfy the -stable
>> requirements. Maybe some reverts would be the right way...
> 
The patch was submitted in Aug 2016 already and there was also an inquiry
to get it into 4.9:
http://lists.infradead.org/pipermail/linux-mtd/2016-September/069363.html

But due to whatever circumstances nothing happened and there was no
response.

> OK, I've reviewed the rest of the pull request as well as I care for.
> I still have this one objection: I don't think this belongs in -stable.
> 
> Am I crazy? Or else Cyrille, can you remove the stable tag and resend?
> Or I can go rewrite your history for you and do this...
> 
> Brian
> 
> P.S. Also, this stuck out at me in the diffs, and checkpatch complained
> too. Not a real big deal:
> 
> WARNING:BLOCK_COMMENT_STYLE: Block comments should align the * on each
> line
> #42: FILE: drivers/mtd/spi-nor/spi-nor.c:83:
> +					 * ATMEL flashes)
> +					*/
> 
> total: 0 errors, 1 warnings, 264 lines checked
> 
> 

Re: [GIT PULL] mtd: spi-nor: Changes for 4.11

[Marek Vasut]

On 02/10/2017 07:39 AM, Heiner Kallweit wrote:
> Am 10.02.2017 um 03:46 schrieb Brian Norris:
>> On Wed, Feb 08, 2017 at 11:09:39AM -0800, Brian Norris wrote:
>>> On Wed, Feb 08, 2017 at 11:36:10AM +0100, Cyrille Pitchen wrote:
>>>> Le 08/02/2017 à 06:52, Heiner Kallweit a écrit :
>>>>> Am 08.02.2017 um 02:09 schrieb Marek Vasut:
>>>>>> On 02/07/2017 04:31 PM, Cyrille Pitchen wrote:
>>>> [...]
>>>>>>> Heiner Kallweit (1):
>>>>>>>       mtd: m25p80: consider max message size in m25p80_read
>>>>>>
>>>>>> I cannot say I'm super-happy about this particular patch, still ...
>>>>>>
>>>>> W/o it at least the fsl-espi driver is broken for transfers >= 64kB.
>>>>>
>> [...]
>>
>>> One thing I do object to: why is this change marked for stable? It's not
>>> our fault you broke your duct-taped-together SPI driver in 4.9. I don't
>>> think backporting features is the way to satisfy the -stable
>>> requirements. Maybe some reverts would be the right way...
>>
> The patch was submitted in Aug 2016 already and there was also an inquiry
> to get it into 4.9:
> http://lists.infradead.org/pipermail/linux-mtd/2016-September/069363.html
> 
> But due to whatever circumstances nothing happened and there was no
> response.

Is that a bugfix ?

-- 
Best regards,
Marek Vasut

Re: [GIT PULL] mtd: spi-nor: Changes for 4.11

[Heiner Kallweit]

Am 10.02.2017 um 14:07 schrieb Marek Vasut:
> On 02/10/2017 07:39 AM, Heiner Kallweit wrote:
>> Am 10.02.2017 um 03:46 schrieb Brian Norris:
>>> On Wed, Feb 08, 2017 at 11:09:39AM -0800, Brian Norris wrote:
>>>> On Wed, Feb 08, 2017 at 11:36:10AM +0100, Cyrille Pitchen wrote:
>>>>> Le 08/02/2017 à 06:52, Heiner Kallweit a écrit :
>>>>>> Am 08.02.2017 um 02:09 schrieb Marek Vasut:
>>>>>>> On 02/07/2017 04:31 PM, Cyrille Pitchen wrote:
>>>>> [...]
>>>>>>>> Heiner Kallweit (1):
>>>>>>>>       mtd: m25p80: consider max message size in m25p80_read
>>>>>>>
>>>>>>> I cannot say I'm super-happy about this particular patch, still ...
>>>>>>>
>>>>>> W/o it at least the fsl-espi driver is broken for transfers >= 64kB.
>>>>>>
>>> [...]
>>>
>>>> One thing I do object to: why is this change marked for stable? It's not
>>>> our fault you broke your duct-taped-together SPI driver in 4.9. I don't
>>>> think backporting features is the way to satisfy the -stable
>>>> requirements. Maybe some reverts would be the right way...
>>>
>> The patch was submitted in Aug 2016 already and there was also an inquiry
>> to get it into 4.9:
>> http://lists.infradead.org/pipermail/linux-mtd/2016-September/069363.html
>>
>> But due to whatever circumstances nothing happened and there was no
>> response.
> 
> Is that a bugfix ?
> 
Certain changes to the fsl-espi driver and the patch we're talking about
here were supposed to go into the same kernel release.
The driver changes made it to mainline months ago, the m25p80 patch not.
So yes, we have a bug in 4.9, transfers >= 64k fail on fsl-espi.

Re: [GIT PULL] mtd: spi-nor: Changes for 4.11

[Marek Vasut]

On 02/10/2017 06:56 PM, Heiner Kallweit wrote:
> Am 10.02.2017 um 14:07 schrieb Marek Vasut:
>> On 02/10/2017 07:39 AM, Heiner Kallweit wrote:
>>> Am 10.02.2017 um 03:46 schrieb Brian Norris:
>>>> On Wed, Feb 08, 2017 at 11:09:39AM -0800, Brian Norris wrote:
>>>>> On Wed, Feb 08, 2017 at 11:36:10AM +0100, Cyrille Pitchen wrote:
>>>>>> Le 08/02/2017 à 06:52, Heiner Kallweit a écrit :
>>>>>>> Am 08.02.2017 um 02:09 schrieb Marek Vasut:
>>>>>>>> On 02/07/2017 04:31 PM, Cyrille Pitchen wrote:
>>>>>> [...]
>>>>>>>>> Heiner Kallweit (1):
>>>>>>>>>       mtd: m25p80: consider max message size in m25p80_read
>>>>>>>>
>>>>>>>> I cannot say I'm super-happy about this particular patch, still ...
>>>>>>>>
>>>>>>> W/o it at least the fsl-espi driver is broken for transfers >= 64kB.
>>>>>>>
>>>> [...]
>>>>
>>>>> One thing I do object to: why is this change marked for stable? It's not
>>>>> our fault you broke your duct-taped-together SPI driver in 4.9. I don't
>>>>> think backporting features is the way to satisfy the -stable
>>>>> requirements. Maybe some reverts would be the right way...
>>>>
>>> The patch was submitted in Aug 2016 already and there was also an inquiry
>>> to get it into 4.9:
>>> http://lists.infradead.org/pipermail/linux-mtd/2016-September/069363.html
>>>
>>> But due to whatever circumstances nothing happened and there was no
>>> response.
>>
>> Is that a bugfix ?
>>
> Certain changes to the fsl-espi driver and the patch we're talking about
> here were supposed to go into the same kernel release.
> The driver changes made it to mainline months ago, the m25p80 patch not.
> So yes, we have a bug in 4.9, transfers >= 64k fail on fsl-espi.

Maybe the fsl-espi change should be reverted for 4.9 ...

-- 
Best regards,
Marek Vasut

Re: [GIT PULL] mtd: spi-nor: Changes for 4.11

[Cyrille Pitchen]

Le 10/02/2017 à 03:46, Brian Norris a écrit :
> On Wed, Feb 08, 2017 at 11:09:39AM -0800, Brian Norris wrote:
>> On Wed, Feb 08, 2017 at 11:36:10AM +0100, Cyrille Pitchen wrote:
>>> Le 08/02/2017 à 06:52, Heiner Kallweit a écrit :
>>>> Am 08.02.2017 um 02:09 schrieb Marek Vasut:
>>>>> On 02/07/2017 04:31 PM, Cyrille Pitchen wrote:
>>> [...]
>>>>>> Heiner Kallweit (1):
>>>>>>       mtd: m25p80: consider max message size in m25p80_read
>>>>>
>>>>> I cannot say I'm super-happy about this particular patch, still ...
>>>>>
>>>> W/o it at least the fsl-espi driver is broken for transfers >= 64kB.
>>>>
> [...]
> 
>> One thing I do object to: why is this change marked for stable? It's not
>> our fault you broke your duct-taped-together SPI driver in 4.9. I don't
>> think backporting features is the way to satisfy the -stable
>> requirements. Maybe some reverts would be the right way...
> 
> OK, I've reviewed the rest of the pull request as well as I care for.
> I still have this one objection: I don't think this belongs in -stable.
> 
> Am I crazy? Or else Cyrille, can you remove the stable tag and resend?
> Or I can go rewrite your history for you and do this...
>

No problem, I'm rewriting the history to prepare a new PR :)

So 2 modifications:
1 - remove the stable tag from "mtd: spi-nor: rename SPINOR_OP_* macros of
the 4-byte address op codes"
2 - fix * alignment in comment in "mtd: spi-nor: Add support for S3AN
spi-nor devices"

Best regards,

Cyrille


> Brian
> 
> P.S. Also, this stuck out at me in the diffs, and checkpatch complained
> too. Not a real big deal:
> 
> WARNING:BLOCK_COMMENT_STYLE: Block comments should align the * on each
> line
> #42: FILE: drivers/mtd/spi-nor/spi-nor.c:83:
> +					 * ATMEL flashes)
> +					*/
> 
> total: 0 errors, 1 warnings, 264 lines checked
> 
>