On 22 June 2015 at 17:23, Daniel Schwierzeck <[email protected]> wrote: > 2015-06-22 8:43 GMT+02:00 Jagan Teki <[email protected]>: >> On 16 June 2015 at 15:36, Daniel Schwierzeck >> <[email protected]> wrote: >>> 2015-06-16 11:36 GMT+02:00 Jagan Teki <[email protected]>: >>>> On 16 June 2015 at 14:48, Heiko Schocher denx <[email protected]> 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 <[email protected]> wrote: >>>>>>> >>>>>>> Hello Jagan, >>>>>>> >>>>>>> >>>>>>> Am 16.06.2015 um 10:04 schrieb Jagan Teki: >>>>>>>> >>>>>>>> >>>>>>>> Hi Heiko, >>>>>>>> >>>>>>>> On 20 May 2015 at 12:16, Heiko Schocher <[email protected]> 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@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 > > If you want to use cmd_mtdparts with SPI flash, you have to enable > CONFIG_SPI_FLASH_MTD to have your SPI flash device registered with the > MTD layer. > >> >> zynq-uboot> mtdparts add nor0 0x10000@0x0 env >> mtdparts variable not set, see 'help mtdparts' >> >> and then I will able to do partition read/write using sf_cmd. > > No. cmd_mtdparts operates on the MTD device list. That does not mean > that sf_cmd have to use mtd_read()/mtd_write() functions. Heiko only > added an option for convenience to use MTD partitions with sf_cmd. But > that option only derive offset and size from a MTD partition to > delegate it to spi_flash_read()/spi_flash_write(). That is a > difference to cmd_nand, where nand_read()/nand_write() always delegate > to mtd_read()/mtd_write(). > >> >> 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@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. > > yes for cmd_mtdparts you will need sf_mtd to have your SPI flash > device registered with the MTD layer because cmd_mtdparts operates on > the global MTD device list. The callbacks _read()/_write()/_erase() in > the mtd_info struct are unused then. Are you concerned about four > unused callback functions with minimal code footprint in your use > case? In that case we could add a CONFIG_SPI_FLASH_MTD_OPS option > which conditionally compile the callbacks.
I just calculate the text size of elf it seems ~290 bytes got increased with raw mtd partitioning use case, so I'm going without using any macro. > > Maybe it makes sense in the long term if all flash media is > consistently accessed through the MTD layer especially in conjunction > with driver model and device-tree. If we can replace cmd_nand, > cmd_flash, cmd_sf etc. with a single cmd_mtd respectively env_nand, > env_flash, env_sf with a single env_mtd we could likely reduce the > code footprint. But that discussion is independent from the sf_mtd > patch. I will send the same series again with some useful comments and one typo fix, please find that re-comment so-that I shall pull these stuff on my tree. thanks! -- Jagan | openedev. _______________________________________________ U-Boot mailing list [email protected] http://lists.denx.de/mailman/listinfo/u-boot
