[U-Boot] dm: Introduce SPI-NOR framework
Simon Glass
sjg at chromium.org
Wed Jan 6 01:24:48 CET 2016
Hi Jagan,
On 17 December 2015 at 10:06, Jagan Teki <jteki at openedev.com> wrote:
> Hi Simon,
>
> On 15 December 2015 at 03:44, Simon Glass <sjg at chromium.org> wrote:
>> Hi Jagan,
>>
>> On 11 December 2015 at 09:57, Jagan Teki <jteki at openedev.com> wrote:
>>> Hi Simon,
>>>
>>> On 11 December 2015 at 07:35, Simon Glass <sjg at chromium.org> wrote:
>>>> 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.
>>>
>>> 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
>>
>> Looks good, but you will need an MTD uclass before doing this.
>
> Yes.
>
>>
>>> }
>>>
>>> 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);
>>
>> Is this intended to me a new uclass that has a different API from MTD?
>>
>>> };
>>>
>>> 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;
>>
>> 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
More information about the U-Boot
mailing list