Re: [PATCH v3 0/3] mtd: spi-nor / spi / MFD: Convert intel-spi to SPI MEM

[Mauro Lima <mauro.lima@xxxxxxxxxxxxx>]

Hi all,

On Wed, Oct 13, 2021 at 8:44 AM Mika Westerberg
<mika.westerberg@xxxxxxxxxxxxxxx> wrote:
>
> Hi all,
>
> Based on discussion on the patch I sent some time ago here:
>
>   http://lists.infradead.org/pipermail/linux-mtd/2021-June/086867.html
>
> it turns out that the preferred way to deal with the SPI flash controller
> drivers is through SPI MEM which is part of Linux SPI subsystem.
>
> This series does that for the intel-spi driver. This also renames the
> driver to follow the convention used in the SPI subsystem. The first patch
> improves the write protection handling to be slightly more safer. The
> following two patches do the conversion itself. Note the Intel SPI flash
> controller only allows commands such as read, write and so on and it
> internally uses whatever addressing etc. it figured from the SFDP on the
> flash device.
>
> Previous versions of the patch series can be found here:
>
>   v2: https://lore.kernel.org/linux-mtd/20211007112132.30934-1-mika.westerberg@xxxxxxxxxxxxxxx/
>   v1: https://lore.kernel.org/linux-mtd/20210930100719.2176-1-mika.westerberg@xxxxxxxxxxxxxxx/
>
> Changes from v2:
>
>   * Added tag from Andy
>   * Check buswidth in intel_spi_supports_mem_op() and return false if octal
>     mode is asked. The Intel controllers support buswidths 1-4 but this is
>     not exposed to software. It figures this itself through SFDP tables.
>   * In case of software sequencer, support same opcodes than we support with
>     the hardware sequencer if found in the opcodes table.
>
> Changes from v1:
>
>   * Arrange dependencies in Kconfig entries the same way in both glue
>     drivers.
>   * Added empty lines between different subsystem includes.
>   * dev_err() to single line
>   * Return intel_spi_sw_cycle() directly in intel_spi_erase().
>   * Drop redundant elses.
>   * Fixed typo in the commit message of the patch 3/3.
>
> Mika Westerberg (3):
>   mtd: spi-nor: intel-spi: Disable write protection only if asked
>   mtd: spi-nor: intel-spi: Convert to SPI MEM
>   Documentation / MTD: Rename the intel-spi driver
>
>  Documentation/driver-api/mtd/index.rst        |   2 +-
>  .../mtd/{intel-spi.rst => spi-intel.rst}      |   8 +-
>  drivers/mfd/lpc_ich.c                         |  59 ++-
>  drivers/mtd/spi-nor/controllers/Kconfig       |  36 --
>  drivers/mtd/spi-nor/controllers/Makefile      |   3 -
>  drivers/mtd/spi-nor/controllers/intel-spi.h   |  21 --
>  drivers/spi/Kconfig                           |  39 ++
>  drivers/spi/Makefile                          |   3 +
>  .../intel-spi-pci.c => spi/spi-intel-pci.c}   |  49 ++-
>  .../spi-intel-platform.c}                     |  21 +-
>  .../intel-spi.c => spi/spi-intel.c}           | 357 +++++++++++-------
>  drivers/spi/spi-intel.h                       |  19 +
>  include/linux/mfd/lpc_ich.h                   |   2 +-
>  .../x86/{intel-spi.h => spi-intel.h}          |  12 +-
>  14 files changed, 367 insertions(+), 264 deletions(-)
>  rename Documentation/driver-api/mtd/{intel-spi.rst => spi-intel.rst} (94%)
>  delete mode 100644 drivers/mtd/spi-nor/controllers/intel-spi.h
>  rename drivers/{mtd/spi-nor/controllers/intel-spi-pci.c => spi/spi-intel-pci.c} (84%)
>  rename drivers/{mtd/spi-nor/controllers/intel-spi-platform.c => spi/spi-intel-platform.c} (65%)
>  rename drivers/{mtd/spi-nor/controllers/intel-spi.c => spi/spi-intel.c} (77%)
>  create mode 100644 drivers/spi/spi-intel.h
>  rename include/linux/platform_data/x86/{intel-spi.h => spi-intel.h} (64%)
>
> --
> 2.33.0
>
$ ./scripts/checkpatch.pl --file --strict drivers/spi/spi-intel.c
CHECK: Alignment should match open parenthesis
#215: FILE: drivers/spi/spi-intel.c:215:
+ dev_dbg(ispi->dev, " %02d base: 0x%08x limit: 0x%08x [%c%c]\n",
+ i, base << 12, (limit << 12) | 0xfff,

CHECK: Alignment should match open parenthesis
#232: FILE: drivers/spi/spi-intel.c:232:
+ dev_dbg(ispi->dev, " %02d base: 0x%08x limit: 0x%08x\n",
+ i, base << 12, (limit << 12) | 0xfff);

CHECK: Comparison to NULL could be written "!ispi->sregs"
#378: FILE: drivers/spi/spi-intel.c:378:
+ if (ispi->sregs == NULL && (ispi->swseq_reg || ispi->swseq_erase)) {

CHECK: Blank lines aren't necessary before a close brace '}'
#534: FILE: drivers/spi/spi-intel.c:534:
+
+ }

Beside these checks,
Reviewed-by: Mauro Lima <mauro.lima@xxxxxxxxxxxxx>
Thanks