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

[Jagan Teki]

Hi Bin,

On 27 November 2015 at 07:55, Bin Meng <bmeng.cn at gmail.com> wrote:
> Hi Jagan,
>
> On Fri, Nov 27, 2015 at 2:54 AM, Jagan Teki <jteki at openedev.com> wrote:
>> 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.
>
> My understanding is that when we introduce a new driver feature, it
> should target driver model by default. This is to encourage boards to
> take advantage of driver model, and all these *new* features. This is
> what Simon asked for. As Simon mentioned, probably we should make the
> SPI flash DM conversion complete now instead of adding any new feature
> on top of it.

I'm not introduced any new driver feature here, instead I re-used the
existing mtd core features and functionalities.

The current code looks as below:
--------------------------------------------
sf_probe.c:
1) having both non-dm and dm spi-flash and calls to
2) spi_flash_probe_slave for spi_flash detection and setting up hooks
(spi_flash ops)

sf_ops.c:
1) having all spi_flash ops definitions.

These are the changes I did
a) Moved spi_flash probing like definition of  spi_flash_probe_slave
from sf_probe.c to sf_ops.c so sf_ops.c having all core spi-flash code
like flash detection, setting up hooks (spi_flash ops) and spi_flash
ops definitions.
b) from above point a) sf_probe.c having both dm and non-dm interface
with a common call to sf_ops.c for spi-flash core functionalities
using spi_flash_scan.
c) Replaced existing spi_flash_mtd_register (sf_mtd.c) registration
with generic mtd core using add_mtd_device in sf_probe.c for both dm
and non-dm interfaces.
d) In sf_ops.c Updated all mtd_info structure filling and replaced
spi_flash ops hooks with mtd_info hooks - where dm_spi_flash_ops got
dropped.
e) Called mtd core operations from spi_flash.h like
mtd_erase|write|read instead of direct calls to sf_ops.c with
spi_flash operation as they are not needed, so mtd core calls will in
turn call mtd hooks on spi-flash like nand, cfi and spi-nor in Linux.

The code after these changes:
----------------------------------------
sf_probe.c:
1) having both non-dm and dm spi-flash and calls to
2) spi_flash_scan for spi-flash core functionalities and register with core mtd.

sf_ops.c:
1) core spi-flash operations => spi_flash detection + setting up hooks
(mtd ops) + having all mtd_info ops definitions.

I couldn't understand what's wrong with this approach, because I have
not added any new feature instead I reused it existing MTD core. I
agree that the dm_spi_flash_ops are removed as mtd_info ops are used
instead and sf_probe.c is still with dm.

Please let me know your inputs.

>
>>
>> cmd_sf
>> =======
>> MTD
>> =======
>> spi-nor or spi-flash
>> ===============
>> "spi-nor to spi drivers" and spi-nor controller driver
>> =======================================

thanks!
-- 
Jagan | openedev.