Hi Simon,
Some how I'm unclear about your comments in previous series probably I my misunderstanding or something. let me explain about my plan on spi-nor development.
The entire spi-flash code is generic for all means there is no separate code for any platform or device. So I call it as spi-flash core. spi-flash core having functionaries like read_jedec, flash read/write/erase all these were calling from cmd_sf using dm and non-dm version, here I'm replacing this MTD since it is core interface it doesn't handle any specific device or uclass and mtd operations are being used directly without any dm except the probe.
So spi-flash shouldn't need dm and the respective probe sf_probe will follow dm as it allocates spi_flash and by taking spi_slave{} setup spi-uclass. spi-flash operation are common and there is no different ops for different drivers or devices or something and mtd_ops are manged directly from cmd_sf like nand and spi-nor in Linux.
And once core sits stable spi-flash will implemented as spi-nor core and the bellowed drivers becomes spi-nor drivers like spi-nor to spi driver interface and spi-nor controller drivers these are dm-driven it's having dm ops similar to spi-flash ops and spi-flash no where required.
cmd_sf ======= MTD ======= spi-nor or spi-flash =============== "spi-nor to spi drivers" and spi-nor controller driver =======================================
On 26 November 2015 at 23:20, Simon Glass sjg@chromium.org wrote:
+Fabio
Hi Jagan,
On 26 November 2015 at 04:03, Jagan Teki jteki@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.htm... [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.
"adds new functionality to non-DM code." means code with #ifndef CONFIG_DM_SPI_FLASH sf_probe.c? ie because for mtd should available for non-dm drivers as well. Once all move to dm this will drop anyway.
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
thanks!