[U-Boot] [PATCH v7 00/34] sf: MTD support

[Simon Glass]

+Fabio

Hi Jagan,

On 26 November 2015 at 04:03, Jagan Teki <jteki at openedev.com> wrote:
> This series is combination of mtd and sf tunning stuff in previous
> version patches.[1][2]
>
> This is whole patch series for add mtd support to spi-flash
> framework and related stuff.
>
> The idea is to introduce the spi-nor flash framework which
> similar to Linux with driver-model support.
>
> Detail changes:
> - drivers/mtd/spi/sf_probe.c: spi-flash to spi drivers interface(dm and non-dm)
> - drivers/mtd/spi/sf_ops.c: Core spi-flash functionalities.
> - spi_flash ops and dm_spi_ops are not needed as flash opertaion are
>   common for dm and non-dm via MTD
>
> Changes in v7:
> - Rebase to master
> - Added MTD core support to dataflash
> - Few patch bisectable separations
>
> Changes in v6, v5, v4, v3, v2:
> - One patch bisectable separation
> - Rebase to master
> - added newly mtd stuff patches.
>
> Testing:
> $ git clone git://git.denx.de/u-boot-spi.git
> $ cd u-boot-spi
> $ git checkout -b spi-nor-mtd origin/next-spi-nor-mtd
>
> [1] http://u-boot.10912.n7.nabble.com/PATCH-v6-00-23-sf-MTD-support-td233769.html
> [2] http://lists.denx.de/pipermail/u-boot/2015-October/229857.html
>
> thanks!
> Jagan.

This seems to build on the patch which adds the flash locking stuff
for non-DM SPI flash - commit c3c016cf. When I asked about this a week
ago Tom mentioned that Fabio was going to fix this up so that it
supports driver model.

As I tried to explain last time I reviewed this, the problem with this
series is that it adds new functionality to non-DM code. In fact this
series takes a backwards step, since it plumbs in DM SPI flash to
non-DM SPI MTD, since the latter does not support driver model.

I am obviously failing to explain what I mean here - let me try again...

Function methods should be implemented as 'ops' structs in driver
model and use the 'ops' member of the driver to provide the
implementation. There should be no other function pointers or structs
of function pointers.

Drivers should be declared as DM drivers, and devices should be
created by driver model when it finds a definition in the device tree,
or dynamically if needed for probing buses (e.g. PCI, USB, I2C in some
cases). There should not be calls to 'register' and memory
allocations. That is a sign that you are bypassing driver model for
these things.

I do understand that the SPI flash DM conversion is incomplete - there
are still many boards which have yet to be converted for SPI or SPI
flash. Until that is done I think the efforts should be directed
towards that goal, and things like SPI MTD should be introduced for DM
only. At present it seems that we have created yet another migration
task (SPI MTD) that would be better avoided.

I know you have sent detailed emails about the software layers, etc. I
do understand that (or at least I think I do), but in the DM world,
each layer should be a uclass with its own API and its own devices.

How can we move forward on this and get everything moved over to DM in
short order?

>
> Jagan Teki (34):
>   sf: spi_flash_validate_params => spi_flash_scan
>   sf: Move spi_flash_scan code to sf_ops
>   sf: Move read_id code to sf_ops
>   sf: probe: Code cleanup
>   sf: Use static for file-scope functions
>   sf: Fix Makefile
>   sf: Use simple name for register access functions
>   sf: Use flash function pointers in dm_spi_flash_ops
>   sf: Flash power up read-only based on idcode0
>   sf: Use static for file-scope functions
>   sf: Remove unneeded header includes
>   sf: probe: Use spi_flash_scan in dm-spi-flash
>   sf: Re-factorize spi_flash_probe_tail code
>   dm-sf: Re-factorize spi_flash_std_probe code
>   zynq: Enable CONFIG_SPL_MTD_SUPPORT
>   sf: Add MTD support to spi_flash
>   sf: Use mtd_info ops instead of spi_flash ops
>   cmd_sf: Use mtd->size instead of flash->size
>   sf: Use mtd->erasesize instead of flash->erase_size
>   dm-sf: use mtd_ops, drop dm_spi_flash_ops
>   sf: Use MTD lock operations
>   sf: Add MTD support for non-dm spi_flash interface
>   sf: probe: Minor cleanup
>   sf: Drop SNOR_F_SST_WR flash->flags
>   sf: Remove unneeded SST_BP and SST_WP
>   sf: ops: Fix missing break on spansion read_bar
>   sf: Drop SPI_FLASH_MTD driver
>   configs: Remove CONFIG_SPI_FLASH_MTD
>   sf: dataflash: Remove unneeded spi data
>   sf: dataflash: Move flash id detection into jedec_probe
>   sf: dataflash: Fix add_dataflash return logic
>   sf: dataflash: Add MTD core support
>   sf: dataflash: Rename sf_dataflash.c to mtd_dataflash.c
>   mtd: dataflash: Minor cleanups
>
>  common/cmd_sf.c                                    |  16 +-
>  drivers/mtd/spi/Kconfig                            |  14 +-
>  drivers/mtd/spi/Makefile                           |   9 +-
>  .../mtd/spi/{sf_dataflash.c => mtd_dataflash.c}    | 402 ++++++-----
>  drivers/mtd/spi/sf-uclass.c                        |  39 --
>  drivers/mtd/spi/sf_internal.h                      |  74 +-
>  drivers/mtd/spi/sf_mtd.c                           | 104 ---
>  drivers/mtd/spi/sf_ops.c                           | 769 ++++++++++++++++-----
>  drivers/mtd/spi/sf_probe.c                         | 508 ++------------
>  include/configs/aristainetos-common.h              |   1 -
>  include/configs/gw_ventana.h                       |   1 -
>  include/configs/ls1021aqds.h                       |   2 +-
>  include/configs/socfpga_common.h                   |   1 -
>  include/configs/zynq-common.h                      |   1 +
>  include/spi_flash.h                                | 163 ++---
>  15 files changed, 926 insertions(+), 1178 deletions(-)
>  rename drivers/mtd/spi/{sf_dataflash.c => mtd_dataflash.c} (64%)
>  delete mode 100644 drivers/mtd/spi/sf_mtd.c
>
> --
> 1.9.1
>

Regards,
Simon