[U-Boot] [PATCH v10 00/27] dm: Generic MTD Subsystem, with SPI-NOR interface

[Jagan Teki]

On Tue, Jan 23, 2018 at 3:20 PM, Siva Durga Prasad Paladugu
<sivadur at xilinx.com> wrote:
>
> Hi Jagan,
>
>> -----Original Message-----
>> From: U-Boot [mailto:u-boot-bounces at lists.denx.de] On Behalf Of Jagan
>> Teki
>> Sent: Tuesday, January 02, 2018 3:39 PM
>> To: Lukasz Majewski <lukma at denx.de>
>> Cc: U-Boot Mailing List <u-boot at lists.denx.de>; Tom Rini
>> <trini at konsulko.com>
>> Subject: Re: [U-Boot] [PATCH v10 00/27] dm: Generic MTD Subsystem, with
>> SPI-NOR interface
>>
>> On Thu, Dec 28, 2017 at 8:14 PM, Lukasz Majewski <lukma at denx.de>
>> wrote:
>> > Hi Jagan,
>> >
>> >> Compared to previous series’s [1], [2], [3] and [4] this patch set
>> >> redefined most of the implementation suitable to fit into existing
>> >> driver-model.
>> >>
>> >> MTD is generic subsystem for underlying flash devices like nand,
>> >> parallel nor, spinor, dataflash etc. So to drive this theory with
>> >> driver model(with an example of block layer) mtd is common device
>> >> interaction for most of  memory technology flashes like nand,
>> >> parallel nor, spinor, dataflash etc, these are treated as interface
>> >> types wrt u-boot driver model.
>> >>
>> >> Once the respective interface driver bind happen, the uclass driver
>> >> will pass an 'interface type' to mtd layer to create device for it,
>> >> for example once spinor ULASS_SPI_NOR driver bind happen, the uclass
>> >> driver of spinor will pass MTD_IF_TYPE_SPI_NOR interface type to
>> >> create mtd device for spinor devices.
>> >>
>> >> SPI-NOR:
>> >> =======
>> >> Some of the SPI device drivers at drivers/spi not a real spi
>> >> controllers, Unlike normal/generic SPI controllers they operates only
>> >> with SPI-NOR flash devices. these were technically termed as SPI-NOR
>> >> controllers, Ex: drivers/spi/fsl_qspi.c
>> >>
>> >> The problem with these were resides at drivers/spi is entire SPI
>> >> layer becomes SPI-NOR flash oriented which is absolutely a wrong
>> >> indication where SPI layer getting effected more with flash
>> >> operations - So this SPI-NOR core will resolve this issue by
>> >> separating all SPI-NOR flash operations from spi layer and creats a
>> >> generic layer called SPI-NOR core which can be used to interact
>> >> SPI-NOR to SPI driver interface layer and the SPI-NOR controller
>> >> driver.
>> >
>> > I must admit that I'm a bit confused....
>> >
>> > If you don't mind I would like to ask for clarification of a few things:
>> >
>> >
>> >
>> >>
>> >> =======================================
>> >>              cmd/spinor.c
>> >
>> >                 ^^^^^ - this would be a new set of commands to comply
>> >                 with DM?
>>
>> with this series yes, and we're working on supporting the same with non-
>> dm.
>>
>> >
>> >                 What about "sf" command which we use now to get access
>> >                 to SPI-NOR memory? A lot of boards already use "sf"
>> >                 command... which may be tricky to replace.
>>
>> end-goal will be replace sf with spinor command and removal of 'sf'
>> will be done when the new spi-nor framework stable enough to handle all
>> scenarios which are spi-flash supporting as of now.
>>
>> >
>> >> =======================================
>> >>              mtd-uclass.c
>> >                 ^^^^^ - here we will have a generic MTD layer (as it is
>> >                 in Linux)
>>
>> it is not like in Linux, leaving ops names apart all the code is for creating
>> mtd device for underlying flash devices in the form of interface type during
>> bind. the interface types as of now is spi-nor it can be nand, nor, dataflash
>> etc in future if could be.
>>
>> >
>> >> =======================================
>> >>            spi-nor-uclass.c
>> >> =======================================
>> >>               spi-nor.c
>> >                 ^^^^^^ - why do we need to have spi-nor.c ? Cannot we
>> >                 have its functionality in the spi-nor-uclass.c ?
>> >                 (I'm just curious)
>>
>> spi-nor-uclass.c is an uclass driver for underlying UCLASS_SPI_NOR drivers
>> like m25p80, zynq_qspinor etc but the spi-nor.c is the actual spi-nor core
>> code which handle actual flash specific stuff. and also syncing Linux stuff(if
>> require) becomes easy with spi-nor.c as a separate file.
>>
>> >
>> >> =======================================
>> >> m25p80.c                zynq_qspinor.c
>> >   ^^^^^ - this is the     ^^^^^^^^ - [*]
>> > "generic" spi-nor
>> > driver used in Linux.
>> > I suppose that it will
>> > be reused here?
>>
>> yes - name reused because, I do think the Linux and U-Boot development on
>> spi-nor will follow in-line at some point but the most of code is following U-
>> Boot driver-model.
>>
>> >
>> >> =======================================
>> >> spi-uclass.c
>> >   ^^^^^^^ - why do we
>> > need this uclass?
>>
>> This is an existing uclass spi driver for underlying UCLASS_SPI drivers like
>> drivers/spi/zynq_qspi.c etc
>>
>> >
>> >> =======================================
>> >> zynq_qspi.c
>> >  ^^^^^ - This is probably
>> > the low-level driver for
>> > zynq QSPI IP block - why
>> > do we have zynq_qspinor.c file above [*]?
>> > What is the difference/need of such division?
>>
>> Both the drivers are same, the reason for zynq_qspinor is for testing
>> framework work for two directions like
>> - spi driver interface side through m25p80 cmd/spinor->mtd->spi-nor-
>> uclass->spi-nor->m25p80->zynq_qspi.c
>> - direct spi-nor controller side
>> spinor->mtd->spi-nor-uclass->spi-nor->zynq_qspinor
>>
>> Will remove drivers/spi/zynq_qspi.c once all set.
>
> As mentioned in Zynq TRM, Zynq qspi controller can work in legacy spi mode as well which is same as regular spi controller.
> As you mentioned if you remove from drivers/spi/zynq_qspi and moving it to spi-nor, none can use it in spi legacy mode. Right?

AFAIK, none used till know for generic slaves except for spi-nor. and
main motive here to have support for controller features like BAR,
dual flash however writing this on drivers/spi make difficult have
these.