mbox series

[v10,00/13] External ECC engines & Macronix support

Message ID 20220127091808.1043392-1-miquel.raynal@bootlin.com (mailing list archive)
Headers show
Series External ECC engines & Macronix support | expand

Message

Miquel Raynal Jan. 27, 2022, 9:17 a.m. UTC
Hello all,

I've applied the beginning of the series (bindings + ECC part) in a
branch named spi-mem-ecc on the MTD korg repository. I already applied
this second halve on top of the spi-mem-ecc branch but there was a
misunderstanding and they ended up not being fully reviewed. Hence here
there are once again, there are no changes since v9 beside the rebase.

If needed I will drop the applied patches and send a v11.

Cheers,
Miquèl

Changes in v10:
* Rebased on top of the spi-mem-ecc branch (itself on top of v5.17-rc1).
* Collected Boris and Pratyush's Acks.

Changes in v9:
* Dropped the patch from Pratyush, keeping the DTR check in
  spi_mem_dtr_supports_op() as it was before the series. Pratyush will
  later reroll its entire series on top of this.
* Changed the location of the spi_controller_mem_caps capabilities to be
  in the spi-controller structure, as initially advised by Mark, then
  repeated by Boris and Pratyush.

Changes in v8:
* Applied this patch from Pratyush at the beginning of my series:
  https://lore.kernel.org/all/20210531181757.19458-5-p.yadav@ti.com/
  Made the necessary changes in the following commits.
* Changed the spi-mem-op ecc_en parameter to become ecc and match the
  dtr parameter. Changed its type to "u8 : 1" as well for the same
  reason. Moved it to the data sub-structure as advised by Pratyush.
* Added the received Acks/R-by.

Changes in v7:
* Added a macro to check if the caps are present or not before accessing
  them. This allows for optional caps.
* Dropped the 'no-caps' instance created in v6.
* Reworked a bit all the patches using these caps to have a nice and
  bisectable series, like adding missing static keywords.

Changes in v6:
* Re-include the first patches because a few things have changed in the
  bindings. These are only style changes as Rob asked to group every
  property above or below the description field, which I applied to all
  the binding commits, but without any further update.
* Created a spi-mem capabilities structure. Put that one in the spi-mem
  ops strucure and ensured that all the controllers provided one.
* Created a default "no-caps" empty instance that controller drivers can
  point to by default.
* Dropped the spi_mem_generic_defaults_op() intermediate helper entirely
  (not needed anymore).

Changes in v5:
* Moved a helper in the core as it seems that it will be useful for
  other ECC engines as well (Xiangsheng Hou for Mediatek will need it).
* Changed the parameters of the spi_mem_generic_supports_op() function
  in order to take a structure as input instead of a list of arguments,
  which will be much easier to complement in the future if ever needed.

Changes in v4:
* The first half of the series has been left aside (all the binding
  changes + the external mode in the Macronix driver), now let's focus
  on the pipelined mode.
* Added the ecc_en spi_mem_op structure parameter in a dedicated commit.
* Introduced a new helper for supporting generically the supported ops.
* Used this new helper in the macronix driver.
* By default all the other drivers would refuse a spi_mem_op with ecc_en
  enabled.

Changes in v3:
* Added Mark's R-by.
* Added a commit changing the initialization order between the dirmaps
  and the ECC engine so that the core might now if we are using a
  pipelined engine or not.
* Stopped creating additional dirmaps with ECC if the engine is not a
  pipelined engine.
* Solved the kernel test robot reports. In particular, I added a
  dependency on MTD_NAND_ECC to Macronix SPI controller driver.
* Added a patch to clean the NAND controller yaml file before moving
  some bits to nand-chip.yaml. This addresses the comments made by Rob
  about the useless allOf's.
* Used platform_get_irq_byname_optional() in order to avoid useless
  warnings when there is no IRQ.

Changes in v2:
* Fixed the bindings and added Rob's acks when relevant.
* Added locking in the ECC engine driver.
* Brought more changes in the core in order to bring the ECC information
  into the spi_mem_op structure with the idea of avoiding any races
  between parallel calls on the same engine.
* Reorganized the ECC driver entirely in order to have a per-engine mxic
  structure plus a per-NAND context. This lead to a number of changes
  internally which cannot all be listed.

Changes since the RFC:
* Rebased on top of v5.15-rc1.
* Fixed the dirmap configuration.
* Added the various tags received.
* Fixed the bindings as reported by the robots.
* Fixed the return value of the helper counting bitflips.
* Included a fix from Jaime Liao in the external pattern logic.
* Added the yaml conversion of Macronix SPI controller description.
* Added the yaml conversion of the SPI-NAND description.
* Created a nand-chip.yaml file to share properties between SPI-NAND and
  raw NAND.

Miquel Raynal (13):
  spi: spi-mem: Introduce a capability structure
  spi: spi-mem: Check the controller extra capabilities
  spi: cadence-quadspi: Provide a capability structure
  spi: mxic: Provide a capability structure
  spi: spi-mem: Kill the spi_mem_dtr_supports_op() helper
  spi: spi-mem: Add an ecc parameter to the spi_mem_op structure
  mtd: spinand: Delay a little bit the dirmap creation
  mtd: spinand: Create direct mapping descriptors for ECC operations
  spi: mxic: Fix the transmit path
  spi: mxic: Create a helper to configure the controller before an
    operation
  spi: mxic: Create a helper to ease the start of an operation
  spi: mxic: Add support for direct mapping
  spi: mxic: Add support for pipelined ECC operations

 drivers/mtd/nand/spi/core.c       |  51 ++++-
 drivers/spi/Kconfig               |   2 +-
 drivers/spi/spi-cadence-quadspi.c |  10 +-
 drivers/spi/spi-mem.c             |  32 +--
 drivers/spi/spi-mxic.c            | 340 ++++++++++++++++++++++++------
 include/linux/mtd/spinand.h       |   2 +
 include/linux/spi/spi-mem.h       |  27 ++-
 include/linux/spi/spi.h           |   3 +
 8 files changed, 367 insertions(+), 100 deletions(-)

Comments

Mark Brown Jan. 27, 2022, 7:56 p.m. UTC | #1
On Thu, Jan 27, 2022 at 10:17:55AM +0100, Miquel Raynal wrote:
> Hello all,
> 
> I've applied the beginning of the series (bindings + ECC part) in a
> branch named spi-mem-ecc on the MTD korg repository. I already applied
> this second halve on top of the spi-mem-ecc branch but there was a
> misunderstanding and they ended up not being fully reviewed. Hence here
> there are once again, there are no changes since v9 beside the rebase.

This looks good, please send a pull request.

In general for future reference if there's patches for one of my
subsystems that I've not explicitly commented on I'm probably expecting
to apply them.
Miquel Raynal Jan. 28, 2022, 8:08 a.m. UTC | #2
Hi Mark,

broonie@kernel.org wrote on Thu, 27 Jan 2022 19:56:50 +0000:

> On Thu, Jan 27, 2022 at 10:17:55AM +0100, Miquel Raynal wrote:
> > Hello all,
> > 
> > I've applied the beginning of the series (bindings + ECC part) in a
> > branch named spi-mem-ecc on the MTD korg repository. I already applied
> > this second halve on top of the spi-mem-ecc branch but there was a
> > misunderstanding and they ended up not being fully reviewed. Hence here
> > there are once again, there are no changes since v9 beside the rebase.  
> 
> This looks good, please send a pull request.

Great thanks!

> In general for future reference if there's patches for one of my
> subsystems that I've not explicitly commented on I'm probably expecting
> to apply them.

Oh yeah this is very clear, but as this series heavily depends on some
core MTD changes as well, it was decided that everything would be merged
in one operation and that's why I started constructing the spi-mem-ecc
branch, which touches four different 'cores': nand, spi-nand, spi,
spi-mem.

Because of the last minute reviews right before the beginning
of the merge window I decided not to apply the second halve of the
series too quickly and delayed the whole patchset including the
previous patches which had no interest on their own.

> On Fri, Nov 26, 2021 at 03:10:59PM +0100, Miquel Raynal wrote:
> 
> > If you acknowledge the SPI bits I believe I can carry the entire series
> > through the MTD tree. If you fear conflicts and need an immutable tag I
> > can also do that.  
> 
> It'd be good to have the tag just in case, there's generally a lot of
> work in this area.
> 
> Reviewed-by: Mark Brown <broonie@kernel.org>

As three patches in this series were new/reworked I dropped your tag
there. I should have asked you again. Shall I add them (back) to the
following patches?

	spi: cadence-quadspi: Provide a capability structure
	spi: mxic: Provide a capability structure
	spi: spi-mem: Introduce a capability structure

Thanks,
Miquèl
Mark Brown Jan. 28, 2022, 10:34 p.m. UTC | #3
On Fri, Jan 28, 2022 at 09:08:12AM +0100, Miquel Raynal wrote:

> As three patches in this series were new/reworked I dropped your tag
> there. I should have asked you again. Shall I add them (back) to the
> following patches?

> 	spi: cadence-quadspi: Provide a capability structure
> 	spi: mxic: Provide a capability structure
> 	spi: spi-mem: Introduce a capability structure

Yes, please.
Miquel Raynal Jan. 31, 2022, 1:49 p.m. UTC | #4
Hi Mark,

broonie@kernel.org wrote on Fri, 28 Jan 2022 22:34:52 +0000:

> On Fri, Jan 28, 2022 at 09:08:12AM +0100, Miquel Raynal wrote:
> 
> > As three patches in this series were new/reworked I dropped your tag
> > there. I should have asked you again. Shall I add them (back) to the
> > following patches?  
> 
> > 	spi: cadence-quadspi: Provide a capability structure
> > 	spi: mxic: Provide a capability structure
> > 	spi: spi-mem: Introduce a capability structure  
> 
> Yes, please.

Great, thanks. I'll apply v10 soon, and will share an immutable tag.

Thanks,
Miquèl