[U-Boot] dm: Introduce SPI-NOR framework
Simon Glass
sjg at chromium.org
Fri Dec 11 03:05:24 CET 2015
Hi Jagan,
On 8 December 2015 at 08:36, Jagan Teki <jteki at openedev.com> wrote:
> Hi Simon,
>
> On 8 December 2015 at 08:22, Simon Glass <sjg at chromium.org> wrote:
>> Hi Jagan,
>>
>> On 3 December 2015 at 03:10, Jagan Teki <jteki at openedev.com> wrote:
>>> Hi Simon,
>>>
>>> I re-phrase all the question from previous thread and continue in this for
>>> more discussion on spi-nor development.
>>>
>>>> Is it intended that SPI flash should be a driver for the MTD uclass?
>>>> Others would be NAND and CFI. From what I can tell MTD has the same
>>>> operations as SPI flash (erase, read, write) and adds a lot more.
>>>>
>>>> Or is SPI flash really a separate uclass from MTD, with SPI flash
>>>> being at a higher level?
>>>
>>> Based on my "sf: MTD support" series SPI flash is a separate uclass from MTD
>>> and it uses mtd_info operations.
>>>
>>>>> cmd_sf
>>>>> =======
>>>>> MTD
>>>>> =======
>>>>> spi-nor or spi-flash
>>>>> ===============
>>>>> "spi-nor to spi drivers" and spi-nor controller driver
>>>>> =======================================
>>>> Your diagram above suggests that MTD calls into SPI NOR or SPI flash,
>>>> but when I look at (for exampe) spi_flash_erase(), it is calling
>>>> mtd_erase(), suggesting that it is above MTD in the software stack,
>>>> the opposite of your diagram above.
>>>>
>>>
>>> Will explain this clearly in below description.
>>>
>>>> Conceptually this seems problematic.
>>>>
>>>> SPI flash is a uclass and supports driver model. It has operations,
>>>> etc. Your patches remove the operations in favour of calling MTD. But
>>>> MTD does not support driver model. This is getting really messy.
>>>>
>>>
>>> Will explain this clearly in below description.
>>>
>>>
>>> Introducing SPI-NOR:
>>> ********************
>>>
>>> Some of the spi drivers or spi controllers at drivers/spi/*
>>> not a real spi controllers, unlike normal spi controllers
>>> those operates varienty of spi devices among spi-nor flash is
>>> one of them but instead these were specially designed for
>>> to operate spi-nor flash devices - these are spi-nor controllers.
>>> example: drivers/spi/fsl_qspi.c
>>>
>>> The problem with these were sitting at drivers/spi is entire
>>> spi layer becomes spi-nor flash oriented which is absolutely
>>> wrong indication where spi layer gets effected more with
>>> flash operations - So this SPI-NOR will resolve this issue
>>> by separating all spi-nor flash operations from spi layer
>>> and by creating a generic layer called SPI-NOR core where it
>>> deals with all SPI-NOR flash activities. The basic idea is
>>> taken from Linux spi-nor framework.
>>>
>>> SPI-NOR Block diagram:
>>> *********************
>>>
>>> =========
>>> cmd_sf.c
>>> ===========
>>> spi_flash.h
>>> ===========
>>> mtdcore.c
>>> ===========
>>> spi-nor.c
>>> ========================
>>> m25p80.c fsl_qspi.c
>>> ========== ===========
>>> spi-uclass spi-nor hw
>>> ========== ===========
>>> spi_drivers
>>> ===========
>>> spi-bus hw
>>> ==========
>>>
>>> Note:
>>> - spi-nor.c is a common core for m25p80.c which is a spi-nor to spi driver
>>> interface layer and for fsl_qspi.c which is a typical spi-nor
>>> controller driver.
>>> - Challenging task is about probe.
>>>
>>> SPI-NOR Development plan:
>>> *************************
>>>
>>> a) Adding MTD core support:
>>> From command point of view the spi-flash is like a mtd device having
>>> erase/write/read ops with offset, addr and size, but from lower layers the
>>> spi-flash becomes either spi-nor or spi-nand so they have their own specific
>>> features like struct spi_nor {}.
>>>
>>> This is the reason for calling MTD from command layer and the lower layer
>>> as SPI_NOR uclass.
>>>
>>> b) Drop spi_flash uclass:
>>> Since spi_flash is a generic term for serial flash devices among
>>> these spi-nor and spi-nand are the real device categories so add
>>> driver model to these categories.
>>>
>>> I sent the series [1] for above a) and b)
>>
>> It doesn't look like that series drops the SPI flash uclass.
>
> Yes, I have not dropped SPI flash uclass fully on the series only ops.
>
>>
>>>
>>> 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
>
>>
>>>
>>> Detailed SPI-NOR with examples:
>>> ******************************
>>>
>>> include/spi_flash.h
>>> -------------------
>>>
>>> struct spi_flash {
>>> struct spi_nor *nor;
>>> };
>>>
>>> include/linux/mtd/spi-nor.h
>>> ---------------------------
>>>
>>> struct spi_nor {
>>> struct udevice *dev;
>>> struct mtd_info *info;
>>>
>>> // spi-nor hooks
>>> int (*read_reg)(struct udevice *dev, u8 cmd, u8 *val, int len);
>>> int (*write_reg)(struct udevice *dev, u8 cmd, u8 *data, int len);
>>>
>>> int (*read_mmap)(struct udevice *dev, void *data, void *offset,
>>> size_t len);
>>> int (*read)(struct udevice *dev, const u8 *opcode, size_t cmd_len,
>>> void *data, size_t data_len);
>>> int (*write)(struct udevice *dev, const u8 *cmd, size_t cmd_len,
>>> const void *data, size_t data_len);
>>
>> But here you show SPI nor with its own operations. So does SPI nor
>> call into MTD? What is your plan to move MTD to driver model and
>> create a MTD uclass? Also, does this mean that each SPI nor device
>> will have an MTD sub-device?
>
> 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.
>
>>
>>> };
>>>
>>> drivers/mtd/spi-nor/spi-nor.c
>>> -----------------------------
>>>
>>> static int spi_nor_erase(struct mtd_info *mtd, struct erase_info *instr)
>>> {
>>> // call to spi-nor erase nor->erase()
>>> }
>>>
>>> static int spi_nor_read(struct mtd_info *mtd, loff_t from, size_t len,
>>> size_t *retlen, u_char *buf)
>>> {
>>> // call to spi-nor erase nor->read()
>>> }
>>>
>>> static int spi_nor_write(struct mtd_info *mtd, loff_t to, size_t len,
>>> size_t *retlen, const u_char *buf)
>>> {
>>> // call to spi-nor erase nor->erase()
>>> }
>>>
>>> int spi_nor_scan(struct spi_nor *nor)
>>> {
>>> struct mtd_info *mtd = nor->mtd;
>>>
>>> // read_jedec
>>>
>>> // assign mtd hooks
>>>
>>> // other spi-nor stuff
>>>
>>> }
>>>
>>> drivers/mtd/spi-nor/m25p80.c
>>> ----------------------------
>>>
>>> struct m25p80 {
>>> struct spi_slave *spi;
>>> struct mtd_info mtd;
>>> };
>>>
>>> static int m25p80_probe(struct udevice *dev)
>>> {
>>> struct spi_slave *spi = dev_get_parentdata(dev);
>>> struct spi_nor *nor = dev_get_uclass_priv(dev);
>>> struct m25p80 *flash = dev_get_priv(dev);
>>> struct spi_flash *sflash;
>>>
>>> spi_nor_scan(nor);
>>>
>>> add_mtd_device(&flash->mtd);
>>>
>>> }
>>>
>>> // spi-nor hooks
>>> static const struct dm_spi_flash_ops m25p80_ops = {
>>> .read = m25p80_read,
>>> .write = m25p80_write,
>>> .erase = m25p80_erase,
>>> };
>>>
>>> static const struct udevice_id m25p80_ids[] = {
>>> { .compatible = "jedec,spi-nor" },
>>> { }
>>> };
>>>
>>> U_BOOT_DRIVER(m25p80) = {
>>> .name = "m25p80",
>>> .id = UCLASS_SPI_NOR,
>>> .of_match = m25p80_ids,
>>> .probe = m25p80_probe,
>>> .priv_auto_alloc_size = sizeof(struct m25p80),
>>> .ops = &m25p80_ops,
>>> };
>>>
>>> drivers/mtd/spi-nor/fsl-qspi.c
>>> ------------------------------
>>>
>>> struct fsl_qspi {
>>> struct mtd_info mtd;
>>> };
>>>
>>> static int fsl_probe(struct udevice *dev)
>>> {
>>> struct spi_nor *nor = dev_get_uclass_priv(dev);
>>> struct fsl_qspi *q = dev_get_priv(dev);
>>> struct spi_flash *sflash;
>>>
>>> spi_nor_scan(nor);
>>>
>>> add_mtd_device(&q->mtd);
>>>
>>> }
>>>
>>> // spi-nor hooks
>>> static const struct dm_spi_nor_ops fsl_qspi_ops = {
>>> .read = fsl_qspi_read,
>>> .write = fsl_qspi_write,
>>> .erase = fsl_qspi_erase,
>>> };
>>>
>>> static const struct udevice_id fsl_qspi_ids[] = {
>>> { .compatible = "fsl,vf610-qspi" },
>>> { }
>>> };
>>>
>>> U_BOOT_DRIVER(fsl_qspi) = {
>>> .name = "fsl_qspi",
>>> .id = UCLASS_SPI_NOR,
>>> .of_match = fsl_qspi_ids,
>>> .probe = fsl_qspi_probe,
>>> .priv_auto_alloc_size = sizeof(struct fsl_qspi),
>>> ops = &fsl_qspi_ops,
>>> };
>>>
>>> About non-dm handling
>>> *********************
>>>
>>> We still maintain the driver similar to m25p80.c for non-dm interface till
>>> all spi-drivers converted into dm.
>>>
>>> [1] https://www.mail-archive.com/u-boot@lists.denx.de/msg193802.html
>
> thanks!
> --
> Jagan.
Regards,
Simon
More information about the U-Boot
mailing list