[U-Boot] [PATCH v6 1/4] mtd, spi: add MTD layer driver
Jagan Teki
jteki at openedev.com
Mon Jun 22 08:43:52 CEST 2015
On 16 June 2015 at 15:36, Daniel Schwierzeck
<daniel.schwierzeck at gmail.com> wrote:
> 2015-06-16 11:36 GMT+02:00 Jagan Teki <jteki at openedev.com>:
>> On 16 June 2015 at 14:48, Heiko Schocher denx <hs at denx.de> wrote:
>>> Hello Jagan,
>>>
>>>
>>> Am 16.06.2015 um 10:52 schrieb Jagan Teki:
>>>>
>>>> Hi Heiko,
>>>>
>>>> On 16 June 2015 at 14:13, Heiko Schocher denx <hs at denx.de> wrote:
>>>>>
>>>>> Hello Jagan,
>>>>>
>>>>>
>>>>> Am 16.06.2015 um 10:04 schrieb Jagan Teki:
>>>>>>
>>>>>>
>>>>>> Hi Heiko,
>>>>>>
>>>>>> On 20 May 2015 at 12:16, Heiko Schocher <hs at denx.de> wrote:
>>>>>>>
>>>>>>>
>>>>>>> Hello Jagan,
>>>>>>>
>>>>>>> Am 19.05.2015 22:09, schrieb Jagan Teki:
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> Hi Heiko,
>>>>>>>>
>>>>>>>> I have tested this sf-mtd stuff, please see below and enabled prints
>>>>>>>> in all the func calls.
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> Thanks for testing!
>>>>>>>
>>>>>>>
>>>>>>>> zynq-uboot> mtdparts add nor0 0x10000 at 0x0 env
>>>>>>>> mtdparts variable not set, see 'help mtdparts'
>>>>>>>> zynq-uboot> mtdparts
>>>>>>>>
>>>>>>>> device nor0 <zynq-sf.0>, # parts = 1
>>>>>>>> #: name size offset mask_flags
>>>>>>>> 0: env 0x00010000 0x00000000 0
>>>>>>>>
>>>>>>>> active partition: nor0,0 - (env) 0x00010000 @ 0x00000000
>>>>>>>>
>>>>>>>> defaults:
>>>>>>>> mtdids : nor0=zynq-sf.0
>>>>>>>> mtdparts: none
>>>>>>>> zynq-uboot> sf erase env 0x10000
>>>>>>>> spi_flash_erase
>>>>>>>> spi_flash_cmd_erase_ops
>>>>>>>> SF: erase d8 0 0 0 (0)
>>>>>>>> SF: 65536 bytes @ 0x0 Erased: OK
>>>>>>>> zynq-uboot> mw.b 0x100 0x44 0x10000
>>>>>>>> zynq-uboot> sf write 0x100 env
>>>>>>>> device 0 offset 0x0, size 0x10000
>>>>>>>> spi_flash_cmd_write_ops
>>>>>>>> SF: 65536 bytes @ 0x0 Written: OK
>>>>>>>> zynq-uboot> sf read 0x40000 env
>>>>>>>> device 0 offset 0x0, size 0x10000
>>>>>>>> spi_flash_cmd_read_ops
>>>>>>>> off = 0x10000, len = 0
>>>>>>>> SF: 65536 bytes @ 0x0 Read: OK
>>>>>>>> zynq-uboot> cmp.b 0x100 0x40000 0x10000
>>>>>>>> Total of 65536 byte(s) were the same
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> Looks good ...
>>>>>>>
>>>>>>>> I wonder none of the sf_mtd_info._* getting called, why?
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> Hmm.. good question .. you use the "sf ..." commands, they do not
>>>>>>> use the mtd interface, right?
>>>>>>
>>>>>>
>>>>>>
>>>>>> I'm fine with the testing, but mtd code in sf seems used only for in UBI
>>>>>> only.
>>>>>
>>>>>
>>>>>
>>>>> a fast "grep mtd_read" in the u-boot source shows, that yaffs2
>>>>> also uses mtd_read ... I did no yaffs2 test, but I think, yaffs2 can
>>>>> be used also with spi flashes now ... if this is wise, I don;t know ...
>>>>>
>>>>> I tested with UBI on a spi flash, and that works ... in the same
>>>>> fashion it currently does on nand for example ...
>>>>>
>>>>>> I wouldn't see this is a better approach where mtd code is considered as
>>>>>> to
>>>>>> be unknown thing for sf.
>>>>>
>>>>>
>>>>>
>>>>> I do not understand you here complete ...
>>>>>
>>>>> drivers/mtd/spi/sf_mtd.c
>>>>>
>>>>> adds just spi flash specific functions to integrate spi flashes
>>>>> into mtd ... and mtd users can then read/write/erase
>>>>> with the mtd_* functions ...
>>>>>
>>>>> Maybe someone has time to convert common/sf.c to use them?
>>>>>
>>>>> So, the final question is, can this patches go into mainline?
>>>>
>>>>
>>>> The only point I'm concerned here is If I need to use sf with mtd support
>>>> without using ubifs or any flash filesystem, the code in sf_mtd.c ops
>>>> becomes
>>>> unused.
>>>
>>>
>>> Why you want to enable mtd support for sf, if you not use it?
>>> do not define CONFIG_SPI_FLASH_MTD in this case?
>>
>> Suppose I need sf-mtd and enabled CONFIG_SPI_FLASH_MTD so
>> when I did it from sf_cmd the code in sf-mtd.c is never been used for
>> erase/read/write. Is that true right?
>
> you obviously do not understand the intention of sf_mtd. It is only an
> adapter for translating mtd_read/mtd_write commands into
> spi_flash_read/spi_flash_write commands. It is not intended to use it
> within sf_cmd or the SPI flash subsystem. Such an adapter is needed
> for subsystems like UBI which can only operate on top of the MTD
> layer. Thus if you want to use UBI on SPI flash, you will need sf_mtd.
> And using UBI on SPI flash is a legitimate use-case for many users.
Say, for example I'm trying to do raw mtd partition without use of any flash
filesystem say UBI. for this also I must t register the spi flash to mtd right?
So I will enable CONFIG_SPI_FLASH_MTD and add the mtd partitions
zynq-uboot> mtdparts add nor0 0x10000 at 0x0 env
mtdparts variable not set, see 'help mtdparts'
and then I will able to do partition read/write using sf_cmd.
>From your point above "It is not intended to use it within sf_cmd or
the SPI flash subsystem."
If I didn't enable or register with mtd, how come the device gets register
with mtd and how could I add the partitions?
zynq-uboot> mtdparts add nor0 0x10000 at 0x0 env
mtdparts variable not set, see 'help mtdparts'
Device nor0 not found!
So my point was if I do a raw mtd partition with spi flash I must use sf_mtd
but there is no usage of sf_mtd_info ops code.
I'm just concentrated on raw mtd usage on spi flash, think on that area and
let me know your inputs.
>
> BTW: this is the same approach as for CFI NOR flash. The CFI driver
> regularly works without MTD support, but you can enable the optional
> cfi_mtd driver if you need it.
>
>>
>>>
>>>> This seems to be a code size increasing factor which is obviously not a
>>>> good
>>>> point for bootloader atleast for u-boot, what do you think?
>>>>
>>>> I agreed your concerned for someone may add support to common/cmd_sf.c
>>>> in future, but I'm bit worried to go this.
>>>
>>>
>>> Why? Its the same state as it is in the nand subsystem ...
>>
>> Agreed but until unless if you have filesystem link to cmd_sf to partitioning.
>
> Again sf_mtd is an optional driver for users who need MTD support on
> SPI flash. All others do not need to enable sf_mtd. Thus there is no
> increasing of code size.
thanks!
--
Jagan | openedev.
More information about the U-Boot
mailing list