[U-Boot] dm: Introduce SPI-NOR framework

[Simon Glass]

Hi Jagan,

On 3 December 2015 at 03:10, Jagan Teki <jteki at openedev.com> wrote:
> Hi Simon,
>
> I re-phrase all the question from previous thread and continue in this for
> more discussion on spi-nor development.
>
>> Is it intended that SPI flash should be a driver for the MTD uclass?
>> Others would be NAND and CFI. From what I can tell MTD has the same
>> operations as SPI flash (erase, read, write) and adds a lot more.
>>
>> Or is SPI flash really a separate uclass from MTD, with SPI flash
>> being at a higher level?
>
> Based on my "sf: MTD support" series SPI flash is a separate uclass from MTD
> and it uses mtd_info operations.
>
>>> cmd_sf
>>> =======
>>> MTD
>>> =======
>>> spi-nor or spi-flash
>>> ===============
>>> "spi-nor to spi drivers" and spi-nor controller driver
>>> =======================================
>> Your diagram above suggests that MTD calls into SPI NOR or SPI flash,
>> but when I look at (for exampe) spi_flash_erase(), it is calling
>> mtd_erase(), suggesting that it is above MTD in the software stack,
>> the opposite of your diagram above.
>>
>
> Will explain this clearly in below description.
>
>> Conceptually this seems problematic.
>>
>> SPI flash is a uclass and supports driver model. It has operations,
>> etc. Your patches remove the operations in favour of calling MTD. But
>> MTD does not support driver model. This is getting really messy.
>>
>
> Will explain this clearly in below description.
>
>
> Introducing SPI-NOR:
> ********************
>
> Some of the spi drivers or spi controllers at drivers/spi/*
> not a real spi controllers, unlike normal spi controllers
> those operates varienty of spi devices among spi-nor flash is
> one of them but instead these were specially designed for
> to operate spi-nor flash devices - these are spi-nor controllers.
> example: drivers/spi/fsl_qspi.c
>
> The problem with these were sitting at drivers/spi is entire
> spi layer becomes spi-nor flash oriented which is absolutely
> wrong indication where spi layer gets effected more with
> flash operations - So this SPI-NOR will resolve this issue
> by separating all spi-nor flash operations from spi layer
> and by creating a generic layer called SPI-NOR core where it
> deals with all SPI-NOR flash activities. The basic idea is
> taken from Linux spi-nor framework.
>
> SPI-NOR Block diagram:
> *********************
>
> =========
> cmd_sf.c
> ===========
> spi_flash.h
> ===========
> mtdcore.c
> ===========
> spi-nor.c
> ========================
> m25p80.c     fsl_qspi.c
> ==========   ===========
> spi-uclass   spi-nor hw
> ==========   ===========
> spi_drivers
> ===========
> spi-bus hw
> ==========
>
> Note:
> - spi-nor.c is a common core for m25p80.c which is a spi-nor to spi driver
> interface layer and for fsl_qspi.c which is a typical spi-nor
> controller driver.
> - Challenging task is about probe.
>
> SPI-NOR Development plan:
> *************************
>
> a) Adding MTD core support:
>    From command point of view the spi-flash is like a mtd device having
> erase/write/read ops with offset, addr and size, but from lower layers the
> spi-flash becomes either spi-nor or spi-nand so they have their own specific
> features like struct spi_nor {}.
>
>    This is the reason for calling MTD from command layer and the lower layer
> as SPI_NOR uclass.
>
> b) Drop spi_flash uclass:
>    Since spi_flash is a generic term for serial flash devices among
>    these spi-nor and spi-nand are the real device categories so add
>    driver model to these categories.
>
> I sent the series [1] for above a) and b)

It doesn't look like that series drops the SPI flash uclass.

>
> c) Add spi-nor support (mean Adding SPI-NOR dm drivers) the next step.

I think this is what I am missing. If you are moving to SPI NOR, what
is the API? Is it the roughly the same as the current SPI flash API?
(read, write, erase)

>
> Detailed SPI-NOR with examples:
> ******************************
>
> include/spi_flash.h
> -------------------
>
> struct spi_flash {
>    struct spi_nor  *nor;
> };
>
> include/linux/mtd/spi-nor.h
> ---------------------------
>
> struct spi_nor {
>    struct udevice   *dev;
>    struct mtd_info  *info;
>
>    // spi-nor hooks
>    int (*read_reg)(struct udevice *dev, u8 cmd, u8 *val, int len);
>    int (*write_reg)(struct udevice *dev, u8 cmd, u8 *data, int len);
>
>    int (*read_mmap)(struct udevice *dev, void *data, void *offset,
>                 size_t len);
>    int (*read)(struct udevice *dev, const u8 *opcode, size_t cmd_len,
>                 void *data, size_t data_len);
>    int (*write)(struct udevice *dev, const u8 *cmd, size_t cmd_len,
>                 const void *data, size_t data_len);

But here you show SPI nor with its own operations. So does SPI nor
call into MTD? What is your plan to move MTD to driver model and
create a MTD uclass? Also, does this mean that each SPI nor device
will have an MTD sub-device?

> };
>
> drivers/mtd/spi-nor/spi-nor.c
> -----------------------------
>
> static int spi_nor_erase(struct mtd_info *mtd, struct erase_info *instr)
> {
>         // call to spi-nor erase nor->erase()
> }
>
> static int spi_nor_read(struct mtd_info *mtd, loff_t from, size_t len,
>                         size_t *retlen, u_char *buf)
> {
>         // call to spi-nor erase nor->read()
> }
>
> static int spi_nor_write(struct mtd_info *mtd, loff_t to, size_t len,
>         size_t *retlen, const u_char *buf)
> {
>         // call to spi-nor erase nor->erase()
> }
>
> int spi_nor_scan(struct spi_nor *nor)
> {
>     struct mtd_info *mtd = nor->mtd;
>
>     // read_jedec
>
>     // assign mtd hooks
>
>     // other spi-nor stuff
>
> }
>
> drivers/mtd/spi-nor/m25p80.c
> ----------------------------
>
> struct m25p80 {
>         struct spi_slave *spi;
>         struct mtd_info   mtd;
> };
>
> static int m25p80_probe(struct udevice *dev)
> {
>     struct spi_slave *spi   = dev_get_parentdata(dev);
>     struct spi_nor   *nor   = dev_get_uclass_priv(dev);
>     struct m25p80    *flash = dev_get_priv(dev);
>     struct spi_flash *sflash;
>
>     spi_nor_scan(nor);
>
>     add_mtd_device(&flash->mtd);
>
> }
>
> // spi-nor hooks
> static const struct dm_spi_flash_ops m25p80_ops = {
>         .read  = m25p80_read,
>         .write = m25p80_write,
>         .erase = m25p80_erase,
> };
>
> static const struct udevice_id m25p80_ids[] = {
>         { .compatible = "jedec,spi-nor" },
>         { }
> };
>
> U_BOOT_DRIVER(m25p80) = {
>         .name           = "m25p80",
>         .id             = UCLASS_SPI_NOR,
>         .of_match       = m25p80_ids,
>         .probe          = m25p80_probe,
>         .priv_auto_alloc_size = sizeof(struct m25p80),
>         .ops            = &m25p80_ops,
> };
>
> drivers/mtd/spi-nor/fsl-qspi.c
> ------------------------------
>
> struct fsl_qspi {
>         struct mtd_info   mtd;
> };
>
> static int fsl_probe(struct udevice *dev)
> {
>     struct spi_nor   *nor = dev_get_uclass_priv(dev);
>     struct fsl_qspi   *q  = dev_get_priv(dev);
>     struct spi_flash *sflash;
>
>     spi_nor_scan(nor);
>
>     add_mtd_device(&q->mtd);
>
> }
>
> // spi-nor hooks
> static const struct dm_spi_nor_ops fsl_qspi_ops = {
>         .read  = fsl_qspi_read,
>         .write = fsl_qspi_write,
>         .erase = fsl_qspi_erase,
> };
>
> static const struct udevice_id fsl_qspi_ids[] = {
>         { .compatible = "fsl,vf610-qspi" },
>         { }
> };
>
> U_BOOT_DRIVER(fsl_qspi) = {
>         .name           = "fsl_qspi",
>         .id             = UCLASS_SPI_NOR,
>         .of_match       = fsl_qspi_ids,
>         .probe          = fsl_qspi_probe,
>         .priv_auto_alloc_size = sizeof(struct fsl_qspi),
>         ops             = &fsl_qspi_ops,
> };
>
> About non-dm handling
> *********************
>
> We still maintain the driver similar to m25p80.c for non-dm interface till
> all spi-drivers converted into dm.
>
> [1] https://www.mail-archive.com/u-boot@lists.denx.de/msg193802.html

Regards,
Simon