dm: Introduce SPI-NOR framework

U-Boot Jagan Teki 2 participants 10 messages Dec 3, 2015 – Jan 6, 2016

lore

Share & tools

lore

Jagan Teki

author

2015-12-03 10:10 UTC #0 lore raw

Hi Simon,

I re-phrase all the question from previous thread and continue in this for more discussion on spi-nor development.

Quoted 6 lines

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.

Quoted 8 lines

Quoted 8 lines
cmd_sf
=======
MTD
=======
spi-nor or spi-flash
===============
"spi-nor to spi drivers" and spi-nor controller driver
=======================================

Quoted 5 lines

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.

Quoted 6 lines

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)

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

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);
};

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 at lists.denx.de/msg193802.html

Jagan Teki

author

2015-12-06 15:01 UTC #1 lore raw

+ Bin

Quoted 51 lines · Jagan Teki

Hi Simon,
I re-phrase all the question from previous thread and continue in this for more discussion on spi-nor development.
Quoted 6 lines
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.
Quoted 8 lines
Quoted 8 lines
cmd_sf
=======
MTD
=======
spi-nor or spi-flash
===============
"spi-nor to spi drivers" and spi-nor controller driver
=======================================
Quoted 5 lines
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.
Quoted 6 lines
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

Quoted 8 lines

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.

Comments please?

Quoted 192 lines

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)
c) Add spi-nor support (mean Adding SPI-NOR dm drivers) the next step.
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);
};
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,
};

Quoted 7 lines

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 at lists.denx.de/msg193802.html

thanks!

Signature · 1 line

Jagan.

Simon Glass

2015-12-08 02:52 UTC #2 lore raw

In reply to Jagan Teki

Hi Jagan,

Quoted 99 lines · Jagan Teki

Hi Simon,
I re-phrase all the question from previous thread and continue in this for more discussion on spi-nor development.
Quoted 6 lines
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.
Quoted 8 lines
Quoted 8 lines
cmd_sf
=======
MTD
=======
spi-nor or spi-flash
===============
"spi-nor to spi drivers" and spi-nor controller driver
=======================================
Quoted 5 lines
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.
Quoted 6 lines
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.

Quoted 6 lines

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.

Quoted 2 lines

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)

Quoted 14 lines

Detailed SPI-NOR with examples:
******************************
include/spi_flash.h
-------------------
struct spi_flash {
   struct spi_nor  *nor;
};
include/linux/mtd/spi-nor.h
---------------------------

Quoted 14 lines

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?

Quoted 116 lines

};
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,
};

Quoted 7 lines

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 at lists.denx.de/msg193802.html

Regards,
Simon

Jagan Teki

author

2015-12-08 15:36 UTC #3 lore raw

Hi Simon,

Quoted 102 lines · Simon Glass

Hi Jagan,
Quoted 99 lines · Jagan Teki
Hi Simon,
I re-phrase all the question from previous thread and continue in this for more discussion on spi-nor development.
Quoted 6 lines
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.
Quoted 8 lines
Quoted 8 lines
cmd_sf
=======
MTD
=======
spi-nor or spi-flash
===============
"spi-nor to spi drivers" and spi-nor controller driver
=======================================
Quoted 5 lines
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.
Quoted 6 lines
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.

Quoted 8 lines

Quoted 6 lines
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.

Yes, I have not dropped SPI flash uclass fully on the series only ops.

Quoted 7 lines

Quoted 3 lines
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)

For now SPI NOR api's are same as SPI flash in terms of structure members but spi-nor hooks, definition and the calling context are different. Usually SPI flash ops are getting called from cmd_sf ==> spi_flash.h ==> sf-uclass.c and then finally goes into sf_ops.c for actual function definitions, but in case of SPI NOR spi-nor ops (defined in spi-nor controller drivers - sf_probe.c, fsl_qspi.c) are getting called from mtd_info ops definitions which are defined at spi-nor core (which is sf_ops.c as of now), and from cmd_sf => spi_flash.h => mtdcore.c and the finally goes into spi-nor for actual mtd ops definitions.

Unlike SPI flash, SPI nor framework will separate the flow from generic spi controller which one of the connected slave as spi nor flash (through sf_probe.c) vs spi-nor controller drivers like fsl_qspi.c

Quoted 30 lines

Quoted 15 lines

Detailed SPI-NOR with examples:
******************************
include/spi_flash.h
-------------------
struct spi_flash {
   struct spi_nor  *nor;
};
include/linux/mtd/spi-nor.h
---------------------------

Quoted 14 lines

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);

Quoted 4 lines

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?

Yes, SPI NOR is different from MTD and MTD operations which are defined at spi-nor will call the respective spi-nor controller driver spi-nor operations. which I explained above.

I think using MTD uclass is another approach where we can remove the udevice from SPI NOR and making all spi-nor controller driver as MTD_UCLASS - do you prefer this?

Quoted 117 lines

Quoted 117 lines
};
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,
};

Quoted 7 lines

Quoted 7 lines
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 at lists.denx.de/msg193802.html

thanks!

Signature · 1 line

Jagan.

Simon Glass

2015-12-11 02:05 UTC #4 lore raw

Hi Jagan,

Quoted 175 lines · Jagan Teki

Hi Simon,
Quoted 102 lines · Simon Glass
Hi Jagan,
Quoted 99 lines · Jagan Teki
Hi Simon,
I re-phrase all the question from previous thread and continue in this for more discussion on spi-nor development.
Quoted 6 lines
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.
Quoted 8 lines
Quoted 8 lines
cmd_sf
=======
MTD
=======
spi-nor or spi-flash
===============
"spi-nor to spi drivers" and spi-nor controller driver
=======================================
Quoted 5 lines
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.
Quoted 6 lines
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.
Quoted 8 lines
Quoted 6 lines
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.
Yes, I have not dropped SPI flash uclass fully on the series only ops.
Quoted 7 lines
Quoted 3 lines
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)
For now SPI NOR api's are same as SPI flash in terms of structure members but spi-nor hooks, definition and the calling context are different. Usually SPI flash ops are getting called from cmd_sf ==> spi_flash.h ==> sf-uclass.c and then finally goes into sf_ops.c for actual function definitions, but in case of SPI NOR spi-nor ops (defined in spi-nor controller drivers - sf_probe.c, fsl_qspi.c) are getting called from mtd_info ops definitions which are defined at spi-nor core (which is sf_ops.c as of now), and from cmd_sf => spi_flash.h => mtdcore.c and the finally goes into spi-nor for actual mtd ops definitions.
Unlike SPI flash, SPI nor framework will separate the flow from generic spi controller which one of the connected slave as spi nor flash (through sf_probe.c) vs spi-nor controller drivers like fsl_qspi.c
Quoted 30 lines
Quoted 15 lines
Detailed SPI-NOR with examples:
******************************
include/spi_flash.h
-------------------
struct spi_flash {
   struct spi_nor  *nor;
};
include/linux/mtd/spi-nor.h
---------------------------
Quoted 14 lines
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);
Quoted 4 lines
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?

Quoted 7 lines

Yes, SPI NOR is different from MTD and MTD operations which are defined at spi-nor will call the respective spi-nor controller driver spi-nor operations. which I explained above.
I think using MTD uclass is another approach where we can remove the udevice from SPI NOR and making all spi-nor controller driver as MTD_UCLASS - do you prefer this?

I think so, so far as I understand it. My initial concern with your approach was that you had a driver model uclass making calls into an API that was not driver-model-compatible. So creating an MTD uclass definitely seems cleaner to me. Also I've found that driver model conversion is best done starting from the bottom of the stack.

Quoted 121 lines

Quoted 121 lines
Quoted 121 lines
};
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
*********************

Quoted 8 lines

Quoted 4 lines
Quoted 4 lines
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 at lists.denx.de/msg193802.html
thanks!
--
Jagan.

Regards,
Simon

Jagan Teki

author

2015-12-11 16:57 UTC #5 lore raw

Hi Simon,

Quoted 186 lines · Simon Glass

Hi Jagan,
Quoted 175 lines · Jagan Teki
Hi Simon,
Quoted 102 lines · Simon Glass
Hi Jagan,
Quoted 99 lines · Jagan Teki
Hi Simon,
I re-phrase all the question from previous thread and continue in this for more discussion on spi-nor development.
Quoted 6 lines
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.
Quoted 8 lines
Quoted 8 lines
cmd_sf
=======
MTD
=======
spi-nor or spi-flash
===============
"spi-nor to spi drivers" and spi-nor controller driver
=======================================
Quoted 5 lines
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.
Quoted 6 lines
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.
Quoted 8 lines
Quoted 6 lines
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.
Yes, I have not dropped SPI flash uclass fully on the series only ops.
Quoted 7 lines
Quoted 3 lines
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)
For now SPI NOR api's are same as SPI flash in terms of structure members but spi-nor hooks, definition and the calling context are different. Usually SPI flash ops are getting called from cmd_sf ==> spi_flash.h ==> sf-uclass.c and then finally goes into sf_ops.c for actual function definitions, but in case of SPI NOR spi-nor ops (defined in spi-nor controller drivers - sf_probe.c, fsl_qspi.c) are getting called from mtd_info ops definitions which are defined at spi-nor core (which is sf_ops.c as of now), and from cmd_sf => spi_flash.h => mtdcore.c and the finally goes into spi-nor for actual mtd ops definitions.
Unlike SPI flash, SPI nor framework will separate the flow from generic spi controller which one of the connected slave as spi nor flash (through sf_probe.c) vs spi-nor controller drivers like fsl_qspi.c
Quoted 30 lines
Quoted 15 lines
Detailed SPI-NOR with examples:
******************************
include/spi_flash.h
-------------------
struct spi_flash {
   struct spi_nor  *nor;
};
include/linux/mtd/spi-nor.h
---------------------------
Quoted 14 lines
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);
Quoted 4 lines
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?
Quoted 7 lines
Yes, SPI NOR is different from MTD and MTD operations which are defined at spi-nor will call the respective spi-nor controller driver spi-nor operations. which I explained above.
I think using MTD uclass is another approach where we can remove the udevice from SPI NOR and making all spi-nor controller driver as MTD_UCLASS - do you prefer this?

Quoted 5 lines

I think so, so far as I understand it. My initial concern with your approach was that you had a driver model uclass making calls into an API that was not driver-model-compatible. So creating an MTD uclass definitely seems cleaner to me. Also I've found that driver model conversion is best done starting from the bottom of the stack.

MTD uclass looks ok to me as well and with this I may drop spi_flash {} and will use mtd_info since from cmd_sf and below is sample code snippet, just written for understanding things will clear when patches submitted. Let me know your inputs on this.

cmd_sf.c
---------

spi_flash_t *flash;

spi_flash.h
-----------

typedef struct mtd_info spi_flash_t;

static inline int spi_flash_read(spi_flash_t *flash, u32 offset,
size_t len, void *buf)

{
    // mtd_read
}

static inline int spi_flash_write(spi_flash_t *flash, u32 offset,
size_t len, const void *buf)
{
// mtd_write
}

static inline int spi_flash_erase(spi_flash_t *flash, u32 offset,
size_t len)
{
// mtd_erase
}

static inline int spi_flash_protect(spi_flash_t *flash, u32 ofs, u32 len,
bool prot)
{
// mtd_lock
// mtd_unlock
}

include/linux/mtd/spi-nor.h
---------------------------

struct spi_nor {
struct mtd_info *mtd;

   // spi-nor hooks
   int (*read_reg)(struct spi_nor *nor, u8 cmd, u8 *val, int len);
   int (*write_reg)(struct spi_nor *nor, u8 cmd, u8 *data, int len);

int (*read_mmap)(struct spi_nor *nor, void *data, void *offset,

                size_t len);
   int (*read)(struct spi_nor *nor, const u8 *opcode, size_t cmd_len,
        void *data, size_t data_len);
   int (*write)(struct spi_nor *nor, const u8 *cmd, size_t cmd_len,
                const void *data, size_t data_len);
};

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_nor nor;
};

static int m25p80_probe(struct udevice *dev)

{
    struct spi_slave *spi   = dev_get_parentdata(dev);
    struct mtd_info  *mtd   = dev_get_uclass_priv(dev);
    struct m25p80    *flash = dev_get_priv(dev);
    struct spi_nor   *nor;

    nor = &flash->nor;

    nor->mtd = mtd;
    flash->spi = spi;

    // spi-nor hooks
    nor->read =
    nor->write =
    nor->read_reg =
    nor->write_reg =

    spi_nor_scan(nor);

    add_mtd_device(mtd);

}

static const struct udevice_id m25p80_ids[] = {
    { .compatible = "jedec,spi-nor" },
    { }
};

U_BOOT_DRIVER(m25p80) = {
    .name        = "m25p80",
    .id        = UCLASS_MTD,
    .of_match    = m25p80_ids,
    .probe        = m25p80_probe,
    .priv_auto_alloc_size = sizeof(struct m25p80),
};

drivers/mtd/spi-nor/fsl-qspi.c
------------------------------

struct fsl_qspi {
        struct spi_nor  nor;
};

static int fsl_probe(struct udevice *dev)

{
    struct mtd_info   *mtd = dev_get_uclass_priv(dev);
    struct fsl_qspi   *q  = dev_get_priv(dev);
    struct spi_nor    *nor;

    nor = &q->nor;
    nor->mtd = mtd;

    // spi-nor hooks
    nor->read =
    nor->write =
    nor->read_reg =
    nor->write_reg =

    spi_nor_scan(nor);

    add_mtd_device(&q->mtd);

}

static const struct udevice_id fsl_qspi_ids[] = {
    { .compatible = "fsl,vf610-qspi" },
    { }
};

U_BOOT_DRIVER(fsl_qspi) = {
.name = "fsl_qspi",
.id = UCLASS_MTD,
.of_match = fsl_qspi_ids,
.probe = fsl_qspi_probe,
.priv_auto_alloc_size = sizeof(struct fsl_qspi),
};

Quoted 119 lines

Quoted 119 lines
Quoted 119 lines
Quoted 119 lines
};
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,
};

Quoted 7 lines

Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
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 at lists.denx.de/msg193802.html

thanks!

Signature · 5 lines

Jagan.



Sent with MailTrack

Simon Glass

2015-12-14 22:14 UTC #6 lore raw

Hi Jagan,

Quoted 208 lines · Jagan Teki

Hi Simon,
Quoted 186 lines · Simon Glass
Hi Jagan,
Quoted 175 lines · Jagan Teki
Hi Simon,
Quoted 102 lines · Simon Glass
Hi Jagan,
Quoted 99 lines · Jagan Teki
Hi Simon,
I re-phrase all the question from previous thread and continue in this for more discussion on spi-nor development.
Quoted 6 lines
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.
Quoted 8 lines
Quoted 8 lines
cmd_sf
=======
MTD
=======
spi-nor or spi-flash
===============
"spi-nor to spi drivers" and spi-nor controller driver
=======================================
Quoted 5 lines
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.
Quoted 6 lines
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.
Quoted 8 lines
Quoted 6 lines
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.
Yes, I have not dropped SPI flash uclass fully on the series only ops.
Quoted 7 lines
Quoted 3 lines
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)
For now SPI NOR api's are same as SPI flash in terms of structure members but spi-nor hooks, definition and the calling context are different. Usually SPI flash ops are getting called from cmd_sf ==> spi_flash.h ==> sf-uclass.c and then finally goes into sf_ops.c for actual function definitions, but in case of SPI NOR spi-nor ops (defined in spi-nor controller drivers - sf_probe.c, fsl_qspi.c) are getting called from mtd_info ops definitions which are defined at spi-nor core (which is sf_ops.c as of now), and from cmd_sf => spi_flash.h => mtdcore.c and the finally goes into spi-nor for actual mtd ops definitions.
Unlike SPI flash, SPI nor framework will separate the flow from generic spi controller which one of the connected slave as spi nor flash (through sf_probe.c) vs spi-nor controller drivers like fsl_qspi.c
Quoted 30 lines
Quoted 15 lines
Detailed SPI-NOR with examples:
******************************
include/spi_flash.h
-------------------
struct spi_flash {
   struct spi_nor  *nor;
};
include/linux/mtd/spi-nor.h
---------------------------
Quoted 14 lines
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);
Quoted 4 lines
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?
Quoted 7 lines
Yes, SPI NOR is different from MTD and MTD operations which are defined at spi-nor will call the respective spi-nor controller driver spi-nor operations. which I explained above.
I think using MTD uclass is another approach where we can remove the udevice from SPI NOR and making all spi-nor controller driver as MTD_UCLASS - do you prefer this?
Quoted 5 lines
I think so, so far as I understand it. My initial concern with your approach was that you had a driver model uclass making calls into an API that was not driver-model-compatible. So creating an MTD uclass definitely seems cleaner to me. Also I've found that driver model conversion is best done starting from the bottom of the stack.
MTD uclass looks ok to me as well and with this I may drop spi_flash {} and will use mtd_info since from cmd_sf and below is sample code snippet, just written for understanding things will clear when patches submitted. Let me know your inputs on this.
cmd_sf.c
---------
spi_flash_t *flash;
spi_flash.h
-----------

Quoted 6 lines

typedef struct mtd_info spi_flash_t;
static inline int spi_flash_read(spi_flash_t *flash, u32 offset,
size_t len, void *buf)
{
    // mtd_read

Looks good, but you will need an MTD uclass before doing this.

Quoted 25 lines

}
static inline int spi_flash_write(spi_flash_t *flash, u32 offset,
size_t len, const void *buf)
{
// mtd_write
}
static inline int spi_flash_erase(spi_flash_t *flash, u32 offset,
size_t len)
{
// mtd_erase
}
static inline int spi_flash_protect(spi_flash_t *flash, u32 ofs, u32 len,
bool prot)
{
// mtd_lock
// mtd_unlock
}
include/linux/mtd/spi-nor.h
---------------------------

Quoted 13 lines

struct spi_nor {
   struct mtd_info  *mtd;

   // spi-nor hooks
   int (*read_reg)(struct spi_nor *nor, u8 cmd, u8 *val, int len);
   int (*write_reg)(struct spi_nor *nor, u8 cmd, u8 *data, int len);

   int (*read_mmap)(struct spi_nor *nor, void *data, void *offset,
                size_t len);
   int (*read)(struct spi_nor *nor, const u8 *opcode, size_t cmd_len,
        void *data, size_t data_len);
   int (*write)(struct spi_nor *nor, const u8 *cmd, size_t cmd_len,
                const void *data, size_t data_len);

Is this intended to me a new uclass that has a different API from MTD?

Quoted 41 lines

};
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_nor nor;
};

Quoted 8 lines

static int m25p80_probe(struct udevice *dev)

{
    struct spi_slave *spi   = dev_get_parentdata(dev);
    struct mtd_info  *mtd   = dev_get_uclass_priv(dev);
    struct m25p80    *flash = dev_get_priv(dev);
    struct spi_nor   *nor;

    nor = &flash->nor;

Is this a SPI nor device? See above question. It seems perhaps not, as dev_get_uclass_priv() returns an mtd_info, which suggests that it is an MTD uclass? This needs to be very clear...

Quoted 62 lines

    nor->mtd = mtd;
    flash->spi = spi;

    // spi-nor hooks
    nor->read =
    nor->write =
    nor->read_reg =
    nor->write_reg =

    spi_nor_scan(nor);

    add_mtd_device(mtd);

}

static const struct udevice_id m25p80_ids[] = {
    { .compatible = "jedec,spi-nor" },
    { }
};

U_BOOT_DRIVER(m25p80) = {
    .name        = "m25p80",
    .id        = UCLASS_MTD,
    .of_match    = m25p80_ids,
    .probe        = m25p80_probe,
    .priv_auto_alloc_size = sizeof(struct m25p80),
};

drivers/mtd/spi-nor/fsl-qspi.c
------------------------------

struct fsl_qspi {
        struct spi_nor  nor;
};

static int fsl_probe(struct udevice *dev)

{
    struct mtd_info   *mtd = dev_get_uclass_priv(dev);
    struct fsl_qspi   *q  = dev_get_priv(dev);
    struct spi_nor    *nor;

    nor = &q->nor;
    nor->mtd = mtd;

    // spi-nor hooks
    nor->read =
    nor->write =
    nor->read_reg =
    nor->write_reg =

    spi_nor_scan(nor);

    add_mtd_device(&q->mtd);

}

static const struct udevice_id fsl_qspi_ids[] = {
    { .compatible = "fsl,vf610-qspi" },
    { }
};

Quoted 10 lines

U_BOOT_DRIVER(fsl_qspi) = {
.name = "fsl_qspi",
.id = UCLASS_MTD,
.of_match = fsl_qspi_ids,
.probe = fsl_qspi_probe,
.priv_auto_alloc_size = sizeof(struct fsl_qspi),
};
Quoted 1 line

[snip]

Regards,
Simon

Jagan Teki

author

2015-12-17 17:06 UTC #7 lore raw

Hi Simon,

Quoted 211 lines · Simon Glass

Hi Jagan,
Quoted 208 lines · Jagan Teki
Hi Simon,
Quoted 186 lines · Simon Glass
Hi Jagan,
Quoted 175 lines · Jagan Teki
Hi Simon,
Quoted 102 lines · Simon Glass
Hi Jagan,
Quoted 99 lines · Jagan Teki
Hi Simon,
I re-phrase all the question from previous thread and continue in this for more discussion on spi-nor development.
Quoted 6 lines
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.
Quoted 8 lines
Quoted 8 lines
cmd_sf
=======
MTD
=======
spi-nor or spi-flash
===============
"spi-nor to spi drivers" and spi-nor controller driver
=======================================
Quoted 5 lines
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.
Quoted 6 lines
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.
Quoted 8 lines
Quoted 6 lines
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.
Yes, I have not dropped SPI flash uclass fully on the series only ops.
Quoted 7 lines
Quoted 3 lines
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)
For now SPI NOR api's are same as SPI flash in terms of structure members but spi-nor hooks, definition and the calling context are different. Usually SPI flash ops are getting called from cmd_sf ==> spi_flash.h ==> sf-uclass.c and then finally goes into sf_ops.c for actual function definitions, but in case of SPI NOR spi-nor ops (defined in spi-nor controller drivers - sf_probe.c, fsl_qspi.c) are getting called from mtd_info ops definitions which are defined at spi-nor core (which is sf_ops.c as of now), and from cmd_sf => spi_flash.h => mtdcore.c and the finally goes into spi-nor for actual mtd ops definitions.
Unlike SPI flash, SPI nor framework will separate the flow from generic spi controller which one of the connected slave as spi nor flash (through sf_probe.c) vs spi-nor controller drivers like fsl_qspi.c
Quoted 30 lines
Quoted 15 lines
Detailed SPI-NOR with examples:
******************************
include/spi_flash.h
-------------------
struct spi_flash {
   struct spi_nor  *nor;
};
include/linux/mtd/spi-nor.h
---------------------------
Quoted 14 lines
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);
Quoted 4 lines
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?
Quoted 7 lines
Yes, SPI NOR is different from MTD and MTD operations which are defined at spi-nor will call the respective spi-nor controller driver spi-nor operations. which I explained above.
I think using MTD uclass is another approach where we can remove the udevice from SPI NOR and making all spi-nor controller driver as MTD_UCLASS - do you prefer this?
Quoted 5 lines
I think so, so far as I understand it. My initial concern with your approach was that you had a driver model uclass making calls into an API that was not driver-model-compatible. So creating an MTD uclass definitely seems cleaner to me. Also I've found that driver model conversion is best done starting from the bottom of the stack.
MTD uclass looks ok to me as well and with this I may drop spi_flash {} and will use mtd_info since from cmd_sf and below is sample code snippet, just written for understanding things will clear when patches submitted. Let me know your inputs on this.
cmd_sf.c
---------
spi_flash_t *flash;
spi_flash.h
-----------

Quoted 8 lines

Quoted 6 lines
typedef struct mtd_info spi_flash_t;
static inline int spi_flash_read(spi_flash_t *flash, u32 offset,
size_t len, void *buf)
{
    // mtd_read
Looks good, but you will need an MTD uclass before doing this.

Yes.

Quoted 90 lines

Quoted 26 lines
}
static inline int spi_flash_write(spi_flash_t *flash, u32 offset,
size_t len, const void *buf)
{
// mtd_write
}
static inline int spi_flash_erase(spi_flash_t *flash, u32 offset,
size_t len)
{
// mtd_erase
}
static inline int spi_flash_protect(spi_flash_t *flash, u32 ofs, u32 len,
bool prot)
{
// mtd_lock
// mtd_unlock
}
include/linux/mtd/spi-nor.h
---------------------------
Quoted 13 lines
struct spi_nor {
   struct mtd_info  *mtd;

   // spi-nor hooks
   int (*read_reg)(struct spi_nor *nor, u8 cmd, u8 *val, int len);
   int (*write_reg)(struct spi_nor *nor, u8 cmd, u8 *data, int len);

   int (*read_mmap)(struct spi_nor *nor, void *data, void *offset,
                size_t len);
   int (*read)(struct spi_nor *nor, const u8 *opcode, size_t cmd_len,
        void *data, size_t data_len);
   int (*write)(struct spi_nor *nor, const u8 *cmd, size_t cmd_len,
                const void *data, size_t data_len);
Is this intended to me a new uclass that has a different API from MTD?
Quoted 48 lines
};
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_nor nor;
};
static int m25p80_probe(struct udevice *dev)
{
    struct spi_slave *spi   = dev_get_parentdata(dev);
    struct mtd_info  *mtd   = dev_get_uclass_priv(dev);
    struct m25p80    *flash = dev_get_priv(dev);
    struct spi_nor   *nor;

Quoted 5 lines

Quoted 1 line
    nor = &flash->nor;
Is this a SPI nor device? See above question. It seems perhaps not, as dev_get_uclass_priv() returns an mtd_info, which suggests that it is an MTD uclass? This needs to be very clear...

struct spi_nor is a low level device for mtd other wards from user point-of-view, user doing an mtd operation on mtd flash device which is SPI nor in this case. Since the driver is MTD uclass, driver probe must assign hooks to spi_nor ops but struct spi_nor is not a uclass.

Quoted 63 lines


    nor->mtd = mtd;
    flash->spi = spi;

    // spi-nor hooks
    nor->read =
    nor->write =
    nor->read_reg =
    nor->write_reg =

    spi_nor_scan(nor);

    add_mtd_device(mtd);

}

static const struct udevice_id m25p80_ids[] = {
    { .compatible = "jedec,spi-nor" },
    { }
};

U_BOOT_DRIVER(m25p80) = {
    .name        = "m25p80",
    .id        = UCLASS_MTD,
    .of_match    = m25p80_ids,
    .probe        = m25p80_probe,
    .priv_auto_alloc_size = sizeof(struct m25p80),
};

drivers/mtd/spi-nor/fsl-qspi.c
------------------------------

struct fsl_qspi {
        struct spi_nor  nor;
};

static int fsl_probe(struct udevice *dev)

{
    struct mtd_info   *mtd = dev_get_uclass_priv(dev);
    struct fsl_qspi   *q  = dev_get_priv(dev);
    struct spi_nor    *nor;

    nor = &q->nor;
    nor->mtd = mtd;

    // spi-nor hooks
    nor->read =
    nor->write =
    nor->read_reg =
    nor->write_reg =

    spi_nor_scan(nor);

    add_mtd_device(&q->mtd);

}

static const struct udevice_id fsl_qspi_ids[] = {
    { .compatible = "fsl,vf610-qspi" },
    { }
};

Quoted 8 lines

Quoted 8 lines
U_BOOT_DRIVER(fsl_qspi) = {
.name = "fsl_qspi",
.id = UCLASS_MTD,
.of_match = fsl_qspi_ids,
.probe = fsl_qspi_probe,
.priv_auto_alloc_size = sizeof(struct fsl_qspi),
};

thanks!

Signature · 1 line

Jagan.

Simon Glass

2016-01-06 00:24 UTC #8 lore raw

Hi Jagan,

Quoted 317 lines · Jagan Teki

Hi Simon,
Quoted 211 lines · Simon Glass
Hi Jagan,
Quoted 208 lines · Jagan Teki
Hi Simon,
Quoted 186 lines · Simon Glass
Hi Jagan,
Quoted 175 lines · Jagan Teki
Hi Simon,
Quoted 102 lines · Simon Glass
Hi Jagan,
Quoted 99 lines · Jagan Teki
Hi Simon,
I re-phrase all the question from previous thread and continue in this for more discussion on spi-nor development.
Quoted 6 lines
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.
Quoted 8 lines
Quoted 8 lines
cmd_sf
=======
MTD
=======
spi-nor or spi-flash
===============
"spi-nor to spi drivers" and spi-nor controller driver
=======================================
Quoted 5 lines
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.
Quoted 6 lines
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.
Quoted 8 lines
Quoted 6 lines
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.
Yes, I have not dropped SPI flash uclass fully on the series only ops.
Quoted 7 lines
Quoted 3 lines
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)
For now SPI NOR api's are same as SPI flash in terms of structure members but spi-nor hooks, definition and the calling context are different. Usually SPI flash ops are getting called from cmd_sf ==> spi_flash.h ==> sf-uclass.c and then finally goes into sf_ops.c for actual function definitions, but in case of SPI NOR spi-nor ops (defined in spi-nor controller drivers - sf_probe.c, fsl_qspi.c) are getting called from mtd_info ops definitions which are defined at spi-nor core (which is sf_ops.c as of now), and from cmd_sf => spi_flash.h => mtdcore.c and the finally goes into spi-nor for actual mtd ops definitions.
Unlike SPI flash, SPI nor framework will separate the flow from generic spi controller which one of the connected slave as spi nor flash (through sf_probe.c) vs spi-nor controller drivers like fsl_qspi.c
Quoted 30 lines
Quoted 15 lines
Detailed SPI-NOR with examples:
******************************
include/spi_flash.h
-------------------
struct spi_flash {
   struct spi_nor  *nor;
};
include/linux/mtd/spi-nor.h
---------------------------
Quoted 14 lines
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);
Quoted 4 lines
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?
Quoted 7 lines
Yes, SPI NOR is different from MTD and MTD operations which are defined at spi-nor will call the respective spi-nor controller driver spi-nor operations. which I explained above.
I think using MTD uclass is another approach where we can remove the udevice from SPI NOR and making all spi-nor controller driver as MTD_UCLASS - do you prefer this?
Quoted 5 lines
I think so, so far as I understand it. My initial concern with your approach was that you had a driver model uclass making calls into an API that was not driver-model-compatible. So creating an MTD uclass definitely seems cleaner to me. Also I've found that driver model conversion is best done starting from the bottom of the stack.
MTD uclass looks ok to me as well and with this I may drop spi_flash {} and will use mtd_info since from cmd_sf and below is sample code snippet, just written for understanding things will clear when patches submitted. Let me know your inputs on this.
cmd_sf.c
---------
spi_flash_t *flash;
spi_flash.h
-----------
Quoted 8 lines
Quoted 6 lines
typedef struct mtd_info spi_flash_t;
static inline int spi_flash_read(spi_flash_t *flash, u32 offset,
size_t len, void *buf)
{
    // mtd_read
Looks good, but you will need an MTD uclass before doing this.
Yes.
Quoted 92 lines
Quoted 26 lines
}
static inline int spi_flash_write(spi_flash_t *flash, u32 offset,
size_t len, const void *buf)
{
// mtd_write
}
static inline int spi_flash_erase(spi_flash_t *flash, u32 offset,
size_t len)
{
// mtd_erase
}
static inline int spi_flash_protect(spi_flash_t *flash, u32 ofs, u32 len,
bool prot)
{
// mtd_lock
// mtd_unlock
}
include/linux/mtd/spi-nor.h
---------------------------
Quoted 13 lines
struct spi_nor {
   struct mtd_info  *mtd;

   // spi-nor hooks
   int (*read_reg)(struct spi_nor *nor, u8 cmd, u8 *val, int len);
   int (*write_reg)(struct spi_nor *nor, u8 cmd, u8 *data, int len);

   int (*read_mmap)(struct spi_nor *nor, void *data, void *offset,
                size_t len);
   int (*read)(struct spi_nor *nor, const u8 *opcode, size_t cmd_len,
        void *data, size_t data_len);
   int (*write)(struct spi_nor *nor, const u8 *cmd, size_t cmd_len,
                const void *data, size_t data_len);
Is this intended to me a new uclass that has a different API from MTD?
Quoted 49 lines
};
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_nor nor;
};
static int m25p80_probe(struct udevice *dev)
{
    struct spi_slave *spi   = dev_get_parentdata(dev);
    struct mtd_info  *mtd   = dev_get_uclass_priv(dev);
    struct m25p80    *flash = dev_get_priv(dev);
    struct spi_nor   *nor;

    nor = &flash->nor;

Quoted 8 lines

Quoted 3 lines
Is this a SPI nor device? See above question. It seems perhaps not, as dev_get_uclass_priv() returns an mtd_info, which suggests that it is an MTD uclass? This needs to be very clear...
struct spi_nor is a low level device for mtd other wards from user point-of-view, user doing an mtd operation on mtd flash device which is SPI nor in this case. Since the driver is MTD uclass, driver probe must assign hooks to spi_nor ops but struct spi_nor is not a uclass.

OK, so are you working on a uclass for MTD?

Regards,
Simon

Jagan Teki

author

2016-01-06 14:32 UTC #9 lore raw

Hi Simon,

Quoted 324 lines · Simon Glass

Hi Jagan,
Quoted 321 lines · Jagan Teki
Hi Simon,
Quoted 211 lines · Simon Glass
Hi Jagan,
Quoted 208 lines · Jagan Teki
Hi Simon,
Quoted 186 lines · Simon Glass
Hi Jagan,
Quoted 175 lines · Jagan Teki
Hi Simon,
Quoted 102 lines · Simon Glass
Hi Jagan,
Quoted 99 lines · Jagan Teki
Hi Simon,
I re-phrase all the question from previous thread and continue in this for more discussion on spi-nor development.
Quoted 6 lines
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.
Quoted 8 lines
Quoted 8 lines
cmd_sf
=======
MTD
=======
spi-nor or spi-flash
===============
"spi-nor to spi drivers" and spi-nor controller driver
=======================================
Quoted 5 lines
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.
Quoted 6 lines
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.
Quoted 8 lines
Quoted 6 lines
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.
Yes, I have not dropped SPI flash uclass fully on the series only ops.
Quoted 7 lines
Quoted 3 lines
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)
For now SPI NOR api's are same as SPI flash in terms of structure members but spi-nor hooks, definition and the calling context are different. Usually SPI flash ops are getting called from cmd_sf ==> spi_flash.h ==> sf-uclass.c and then finally goes into sf_ops.c for actual function definitions, but in case of SPI NOR spi-nor ops (defined in spi-nor controller drivers - sf_probe.c, fsl_qspi.c) are getting called from mtd_info ops definitions which are defined at spi-nor core (which is sf_ops.c as of now), and from cmd_sf => spi_flash.h => mtdcore.c and the finally goes into spi-nor for actual mtd ops definitions.
Unlike SPI flash, SPI nor framework will separate the flow from generic spi controller which one of the connected slave as spi nor flash (through sf_probe.c) vs spi-nor controller drivers like fsl_qspi.c
Quoted 30 lines
Quoted 15 lines
Detailed SPI-NOR with examples:
******************************
include/spi_flash.h
-------------------
struct spi_flash {
   struct spi_nor  *nor;
};
include/linux/mtd/spi-nor.h
---------------------------
Quoted 14 lines
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);
Quoted 4 lines
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?
Quoted 7 lines
Yes, SPI NOR is different from MTD and MTD operations which are defined at spi-nor will call the respective spi-nor controller driver spi-nor operations. which I explained above.
I think using MTD uclass is another approach where we can remove the udevice from SPI NOR and making all spi-nor controller driver as MTD_UCLASS - do you prefer this?
Quoted 5 lines
I think so, so far as I understand it. My initial concern with your approach was that you had a driver model uclass making calls into an API that was not driver-model-compatible. So creating an MTD uclass definitely seems cleaner to me. Also I've found that driver model conversion is best done starting from the bottom of the stack.
MTD uclass looks ok to me as well and with this I may drop spi_flash {} and will use mtd_info since from cmd_sf and below is sample code snippet, just written for understanding things will clear when patches submitted. Let me know your inputs on this.
cmd_sf.c
---------
spi_flash_t *flash;
spi_flash.h
-----------
Quoted 8 lines
Quoted 6 lines
typedef struct mtd_info spi_flash_t;
static inline int spi_flash_read(spi_flash_t *flash, u32 offset,
size_t len, void *buf)
{
    // mtd_read
Looks good, but you will need an MTD uclass before doing this.
Yes.
Quoted 95 lines
Quoted 26 lines
}
static inline int spi_flash_write(spi_flash_t *flash, u32 offset,
size_t len, const void *buf)
{
// mtd_write
}
static inline int spi_flash_erase(spi_flash_t *flash, u32 offset,
size_t len)
{
// mtd_erase
}
static inline int spi_flash_protect(spi_flash_t *flash, u32 ofs, u32 len,
bool prot)
{
// mtd_lock
// mtd_unlock
}
include/linux/mtd/spi-nor.h
---------------------------
Quoted 13 lines
struct spi_nor {
   struct mtd_info  *mtd;

   // spi-nor hooks
   int (*read_reg)(struct spi_nor *nor, u8 cmd, u8 *val, int len);
   int (*write_reg)(struct spi_nor *nor, u8 cmd, u8 *data, int len);

   int (*read_mmap)(struct spi_nor *nor, void *data, void *offset,
                size_t len);
   int (*read)(struct spi_nor *nor, const u8 *opcode, size_t cmd_len,
        void *data, size_t data_len);
   int (*write)(struct spi_nor *nor, const u8 *cmd, size_t cmd_len,
                const void *data, size_t data_len);
Is this intended to me a new uclass that has a different API from MTD?
Quoted 41 lines
};
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_nor nor;
};
Quoted 8 lines
static int m25p80_probe(struct udevice *dev)
{
    struct spi_slave *spi   = dev_get_parentdata(dev);
    struct mtd_info  *mtd   = dev_get_uclass_priv(dev);
    struct m25p80    *flash = dev_get_priv(dev);
    struct spi_nor   *nor;

    nor = &flash->nor;
Is this a SPI nor device? See above question. It seems perhaps not, as dev_get_uclass_priv() returns an mtd_info, which suggests that it is an MTD uclass? This needs to be very clear...

Quoted 6 lines

Quoted 4 lines
struct spi_nor is a low level device for mtd other wards from user point-of-view, user doing an mtd operation on mtd flash device which is SPI nor in this case. Since the driver is MTD uclass, driver probe must assign hooks to spi_nor ops but struct spi_nor is not a uclass.
OK, so are you working on a uclass for MTD?

Yes.

thanks!

Signature · 1 line

Jagan.