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 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. >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. > > 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. _______________________________________________ U-Boot mailing list [email protected] http://lists.denx.de/mailman/listinfo/u-boot
