[U-Boot] [PATCH v4 00/21] sf: Tunning spi-flash layer

[Jagan Teki]

Hi Simon,

On 29 October 2015 at 00:17, Simon Glass <sjg at chromium.org> wrote:
> Hi Jagan,
>
> On 19 October 2015 at 03:28, Jagan Teki <jteki at openedev.com> wrote:
>> Hi Simon,
>>
>> On 19 October 2015 at 01:57, Simon Glass <sjg at chromium.org> wrote:
>>> Hi Jagan,
>>>
>>> On 12 October 2015 at 09:00, Jagan Teki <jteki at openedev.com> wrote:
>>>> Previous version link:
>>>> http://permalink.gmane.org/gmane.comp.boot-loaders.u-boot/233262
>>>>
>>>> spi-flash layer need to tune a lot for better code handling and
>>>> to sync with Linux spi-nor. So below areas got updated in this series.
>>>> - BAR handling
>>>> - spi_flash_cmd_wait_ready updates.
>>>> - Separate core spi-flash handling and spi-flash interface
>>>>   (interface between spi drivers vs spi-flash layer)
>>>>
>>>> Currently I'm working on spi-nor framework for u-boot which
>>>> is slighly same as Linux spi-nor core with addition of
>>>> u-boot driver model to it.
>>>>
>>>> This series will be starting point to add spi-nor functionalities.
>>>>
>>>> TODO:
>>>> - MTD core addition to spi-flash layer.
>>>> - spi-nor core addition.
>>>>
>>>> Code sizes:
>>>> After:
>>>> dm:
>>>>    text    data     bss     dec     hex filename
>>>>  354820   12016  221112  587948   8f8ac u-boot
>>>> non-dm
>>>>    text    data     bss     dec     hex filename
>>>>  354317   11876  221124  587317   8f635 u-boot
>>>>
>>>> Before:
>>>> dm
>>>>    text    data     bss     dec     hex filename
>>>>  354878   12016  221096  587990   8f8d6 u-boot
>>>> non-dm
>>>>    text    data     bss     dec     hex filename
>>>>  354447   11876  221124  587447   8f6b7 u-boot
>>>
>>> I don't think you should be adding new features to the
>>> non-driver-model SPI flash code. We are supposed to be migrating
>>> everything to driver model, so it would be better to move your boards
>>> over, and then work to deprecate and remove the old code. Adding new
>>> features to it sends the wrong message.
>>
>> spi-flash core code doesn't require to add driver model, and cmd_sf to
>> spi-flash code is already supporting driver model.
>>
>> OK, let me explain in-detail.
>>
>> Code in sf_probe.c supports both dm and non dm-spi-flash and flash
>> initialization code using
>> spi_flash_validate_params. sf.c acts as interface between spi drivers
>> vs spi-flash code.
>> So the spi-flash initialization code(part of sf_probe) and code in
>> sf_ops are commonly categorized as spi-flash core code and this will
>> not require driver model, so-that the dm drivers will simply use this
>> common code for spi-flash core functionality.
>>
>> This patch series will separate all the necessary existing code into
>> core and spi-flash vs spi drivers interface code. So at ends
>> - sf_probe is simply the copy of sf.c and dm and non-dm spi-flash code
>> so this will acts a spi-flash vs spi drivers interface. (which has dm
>> and non-dm as same as before)
>> - sf_ops is core spi-flash functionality.
>>
>> On top of this I'm adding actual spi-nor core code, where sf_ops.c
>> will become spi-nor.c and sf_probe.c will become spi-nor-flash.c.
>> - spi-nor.c: Core SPI NOR
>> - spi-nor-flash: spi drivers vs spi-nor interface (which has dm and
>> non-dm as same as before)
>>
>> The reason for adding this spi-nor is to move flash code from
>> spi-drivers, example fsl_qspi and at the end this fsl_qspi will move
>> from spi drivers to spi-nor that will be in driver model.
>>
>> I'm simply adding new core functionality with adding new drivers as
>> dm-driven, I don't think this will not effect/change the driver model
>> growth.
>>
>>  View of spi-nor framework:
>>
>>     -----------------------------------------------------
>>                         cmd_sf
>>     -----------------------------------------------------
>>                         spi_flash
>>     -----------------------------------------------------
>>                         MTD Core
>>     -----------------------------------------------------
>>                         sf-uclass
>>     -----------------------------------------------------
>>                         SPI-NOR
>>     -----------------------------------------------------
>>         spi-nor-flash           drivers/mtd/spi/*
>>     -----------------------------------------------------
>>         spi-uclass
>>     -----------------------------------------------------
>>         drivers/spi/*
>>     -----------------------------------------------------
>>
>> drivers/mtd/spi/spi-nor.c: spi-nor core (not require to add dm)
>> drivers/mtd/spi/spi-flash-nor.c: spi-nor to spi drivers interface (dm-driven)
>> drivers/mtd/spi/fsl-quadspi.c: spi-nor controller driver (dm-driven)
>>
>> Please let me know for any more comments.
>
> Perhaps another way of asking this is, what is the plan to remove the
> non-DM code from SF or at least stop adding new features to it.

Sorry I didn't understand "remove non-dm code" or either I missed
something here.

The plan is not to remove any code intentionally it's about following
feature additions
1) Tuning up spi-flash framework: Separating Core spi-flash code and
interface code between spi-flash vs spi drivers
2) Adding MTD core support to spi-flash core (no spi_flash ops -
mtd_ops will use)
3) Introduce spi-nor core (spi-flash core becomes spi-nor) and new
spi-nor controller drivers are part of this like fsl_qspi or etc.
spi-nor controllers and interface code between spi-flash vs
spi-drivers become UCLASS_SPI_NOR

Agenda is to add SPI-NOR framework(almost similar to Linux) with
driver model(as UCLASS_SPI_NOR)

-----------------------------------------------------------------------------------------------
      cmd_spi                                      cmd_sf
-------|----------------------------------------------------------------------------------------
        |                                              spi_flash
-------|----------------------------------------------------------------------------------------
        |                                             MTD Core
-------|----------------------------------------------------------------------------------------
        |                                           spi-nor-uclass
-------|----------------------------------------------------------------------------------------
        |                                            SPI-NOR Core (spi-nor.c)
-------|-----------------------------------------------------------------------------------------
        |          |=========spi-nor-flash
drivers/mtd/spi/fsl_qspi
         |          |                   (m25p80.c)
              (fsl-quadspi.c)
--------|-------V-----------------------------------------------------------------------------------
         spi-uclass
--------------------------------------------------------------------------------------------------
         drivers/spi/*
 -----------------------------------------------------

Let me know for any more comments?

>
>>
>>>
>>> Sorry if I am misunderstanding your intent here.
>>>
>>>>
>>>> Testing:
>>>> $ git clone git://git.denx.de/u-boot-spi.git
>>>> $ cd u-boot-spi
>>>> $ git checkout -b spi-nor-tune origin/next-spi-nor-tune

-- 
Jagan | openedev.