Hi Heiko, On 20 May 2015 at 12:16, Heiko Schocher <h...@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@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. I wouldn't see this is a better approach where mtd code is considered as to be unknown thing for sf. > > I tested this functions with using UBI on the SPI NOR on the > aristainetos and aristianetos2 boards... I added for example in > > diff --git a/drivers/mtd/spi/sf_mtd.c b/drivers/mtd/spi/sf_mtd.c > index 0b9cb62..6063ed7 100644 > --- a/drivers/mtd/spi/sf_mtd.c > +++ b/drivers/mtd/spi/sf_mtd.c > @@ -39,6 +40,7 @@ static int spi_flash_mtd_read(struct mtd_info *mtd, loff_t > from, size_t len, > struct spi_flash *flash = mtd->priv; > int err; > > +printf("%s ****\n", __func__); > err = spi_flash_read(flash, from, len, buf); > if (!err) > *retlen = len; > > and see: > > => sf probe > SF: Detected N25Q128A with page size 256 Bytes, erase size 64 KiB, total 16 > MiB > => mtdparts > > device nor0 <spi3.1>, # parts = 4 > #: name size offset mask_flags > 0: u-boot 0x000d0000 0x00000000 0 > 1: env 0x00010000 0x000d0000 0 > 2: env-red 0x00010000 0x000e0000 0 > 3: rescue-system 0x00f10000 0x000f0000 0 > > device nand0 <gpmi-nand>, # parts = 1 > #: name size offset mask_flags > 0: ubi 0x40000000 0x00000000 0 > > active partition: nor0,0 - (u-boot) 0x000d0000 @ 0x00000000 > > defaults: > mtdids : none > mtdparts: none > => ubi part rescue-system > UBI: default fastmap pool size: 10 > UBI: default fastmap WL pool size: 25 > UBI: attaching mtd2 to ubi0 > UBI DBG gen (pid 1): sizeof(struct ubi_ainf_peb) 48 > UBI DBG gen (pid 1): sizeof(struct ubi_wl_entry) 20 > UBI DBG gen (pid 1): min_io_size 1 > UBI DBG gen (pid 1): max_write_size 256 > UBI DBG gen (pid 1): hdrs_min_io_size 1 > UBI DBG gen (pid 1): ec_hdr_alsize 64 > UBI DBG gen (pid 1): vid_hdr_alsize 64 > UBI DBG gen (pid 1): vid_hdr_offset 64 > UBI DBG gen (pid 1): vid_hdr_aloffset 64 > UBI DBG gen (pid 1): vid_hdr_shift 0 > UBI DBG gen (pid 1): leb_start 128 > UBI DBG gen (pid 1): max_erroneous 24 > UBI DBG gen (pid 1): process PEB 0 > UBI DBG bld (pid 1): scan PEB 0 > UBI DBG io (pid 1): read EC header from PEB 0 > UBI DBG io (pid 1): read 64 bytes from PEB 0:0 > spi_flash_mtd_read **** > UBI DBG io (pid 1): read VID header from PEB 0 > UBI DBG io (pid 1): read 64 bytes from PEB 0:64 > spi_flash_mtd_read **** > [...] > > UBI uses the MTD layer ... the sf command not ... > Hope this helps? > > bye, > Heiko > > >> On 27 April 2015 at 11:12, Heiko Schocher <h...@denx.de> wrote: >>> >>> From: Daniel Schwierzeck <daniel.schwierz...@gmail.com> >>> >>> add MTD layer driver for spi, original patch from: >>> >>> http://git.denx.de/?p=u-boot/u-boot-mips.git;a=commitdiff;h=bb246819cdc90493dd7089eaa51b9e639765cced >>> >>> changes from Heiko Schocher against this patch: >>> - remove compile error if not defining CONFIG_SPI_FLASH_MTD: >>> >>> LD drivers/mtd/spi/built-in.o >>> drivers/mtd/spi/sf_probe.o: In function `spi_flash_mtd_unregister': >>> /home/hs/abb/imx6/u-boot/drivers/mtd/spi/sf_internal.h:168: multiple >>> definition of `spi_flash_mtd_unregister' >>> >>> drivers/mtd/spi/sf_params.o:/home/hs/abb/imx6/u-boot/drivers/mtd/spi/sf_internal.h:168: >>> first defined here >>> drivers/mtd/spi/sf_ops.o: In function `spi_flash_mtd_unregister': >>> /home/hs/abb/imx6/u-boot/drivers/mtd/spi/sf_internal.h:168: multiple >>> definition of `spi_flash_mtd_unregister' >>> >>> drivers/mtd/spi/sf_params.o:/home/hs/abb/imx6/u-boot/drivers/mtd/spi/sf_internal.h:168: >>> first defined here >>> make[1]: *** [drivers/mtd/spi/built-in.o] Fehler 1 >>> make: *** [drivers/mtd/spi] Fehler 2 >>> >>> - add a README entry. >>> - add correct writebufsize, to fit with Linux v3.14 >>> MTD, UBI/UBIFS sync. >>> >>> Signed-off-by: Daniel Schwierzeck <daniel.schwierz...@gmail.com> >>> Signed-off-by: Heiko Schocher <h...@denx.de> >>> >>> --- >>> >>> Changes in v6: None >>> Changes in v2: >>> - add comment from Daniel Schwierzeck: >>> fix compile error from original patch with >>> "static inline" rather than "static __maybe_unused" >>> Series-changes: 3 >>> - rebase with d6c1ffc7d23f4fe4ae8c91101861055b8e1501b6 >>> Series-changes: 4 >>> - rebased against 385a08a60f042061b004642d6b9bb6cfb794ad5a >>> Series-changes: 5 >>> - no changes >>> Series-changes: 6 >>> - add comments from Jagan Teki: >>> move code, which checks if flash pointer is used >>> into a new patch. >>> - use #ifdef in Code >>> - call mtd register before the spi_release_bus >>> >>> README | 3 ++ >>> common/cmd_sf.c | 2 - >>> drivers/mtd/spi/Makefile | 1 + >>> drivers/mtd/spi/sf_internal.h | 5 ++ >>> drivers/mtd/spi/sf_mtd.c | 104 >>> ++++++++++++++++++++++++++++++++++++++++++ >>> drivers/mtd/spi/sf_probe.c | 10 ++-- >>> 6 files changed, 119 insertions(+), 6 deletions(-) >>> create mode 100644 drivers/mtd/spi/sf_mtd.c >>> >>> diff --git a/README b/README >>> index fc1fd52..36f6fc9 100644 >>> --- a/README >>> +++ b/README >>> @@ -3097,6 +3097,9 @@ CBFS (Coreboot Filesystem) support >>> operation will not execute. The only way to exit this >>> hardware-protected mode is to drive W#/VPP HIGH. >>> >>> + CONFIG_SPI_FLASH_MTD >>> + add MTD translation layer driver. >>> + >>> - SystemACE Support: >>> CONFIG_SYSTEMACE >>> >>> diff --git a/common/cmd_sf.c b/common/cmd_sf.c >>> index 6aabf39..0250011 100644 >>> --- a/common/cmd_sf.c >>> +++ b/common/cmd_sf.c >>> @@ -139,8 +139,6 @@ static int do_spi_flash_probe(int argc, char * const >>> argv[]) >>> return 1; >>> } >>> >>> - if (flash) >>> - spi_flash_free(flash); >>> flash = new; >>> #endif >>> >>> diff --git a/drivers/mtd/spi/Makefile b/drivers/mtd/spi/Makefile >>> index c61b784e..f8580cd 100644 >>> --- a/drivers/mtd/spi/Makefile >>> +++ b/drivers/mtd/spi/Makefile >>> @@ -17,5 +17,6 @@ obj-$(CONFIG_SPI_FLASH) += sf_probe.o >>> #endif >>> obj-$(CONFIG_CMD_SF) += sf.o >>> obj-$(CONFIG_SPI_FLASH) += sf_ops.o sf_params.o >>> +obj-$(CONFIG_SPI_FLASH_MTD) += sf_mtd.o >>> obj-$(CONFIG_SPI_FLASH_SANDBOX) += sandbox.o >>> obj-$(CONFIG_SPI_M95XXX) += eeprom_m95xxx.o >>> diff --git a/drivers/mtd/spi/sf_internal.h >>> b/drivers/mtd/spi/sf_internal.h >>> index 785f7a9..8a2eb6e 100644 >>> --- a/drivers/mtd/spi/sf_internal.h >>> +++ b/drivers/mtd/spi/sf_internal.h >>> @@ -221,4 +221,9 @@ int spi_flash_read_common(struct spi_flash *flash, >>> const u8 *cmd, >>> int spi_flash_cmd_read_ops(struct spi_flash *flash, u32 offset, >>> size_t len, void *data); >>> >>> +#ifdef CONFIG_SPI_FLASH_MTD >>> +int spi_flash_mtd_register(struct spi_flash *flash); >>> +void spi_flash_mtd_unregister(void); >>> +#endif >>> + >>> #endif /* _SF_INTERNAL_H_ */ >>> diff --git a/drivers/mtd/spi/sf_mtd.c b/drivers/mtd/spi/sf_mtd.c >>> new file mode 100644 >>> index 0000000..0b9cb62 >>> --- /dev/null >>> +++ b/drivers/mtd/spi/sf_mtd.c >>> @@ -0,0 +1,104 @@ >>> +/* >>> + * Copyright (C) 2012-2014 Daniel Schwierzeck, >>> daniel.schwierz...@gmail.com >>> + * >>> + * SPDX-License-Identifier: GPL-2.0+ >>> + */ >>> + >>> +#include <common.h> >>> +#include <malloc.h> >>> +#include <asm/errno.h> >>> +#include <linux/mtd/mtd.h> >>> +#include <spi_flash.h> >>> + >>> +static struct mtd_info sf_mtd_info; >>> +static char sf_mtd_name[8]; >>> + >>> +static int spi_flash_mtd_erase(struct mtd_info *mtd, struct erase_info >>> *instr) >>> +{ >>> + struct spi_flash *flash = mtd->priv; >>> + int err; >>> + >>> + instr->state = MTD_ERASING; >>> + >>> + err = spi_flash_erase(flash, instr->addr, instr->len); >>> + if (err) { >>> + instr->state = MTD_ERASE_FAILED; >>> + instr->fail_addr = MTD_FAIL_ADDR_UNKNOWN; >>> + return -EIO; >>> + } >>> + >>> + instr->state = MTD_ERASE_DONE; >>> + mtd_erase_callback(instr); >>> + >>> + return 0; >>> +} >>> + >>> +static int spi_flash_mtd_read(struct mtd_info *mtd, loff_t from, size_t >>> len, >>> + size_t *retlen, u_char *buf) >>> +{ >>> + struct spi_flash *flash = mtd->priv; >>> + int err; >>> + >>> + err = spi_flash_read(flash, from, len, buf); >>> + if (!err) >>> + *retlen = len; >>> + >>> + return err; >>> +} >>> + >>> +static int spi_flash_mtd_write(struct mtd_info *mtd, loff_t to, size_t >>> len, >>> + size_t *retlen, const u_char *buf) >>> +{ >>> + struct spi_flash *flash = mtd->priv; >>> + int err; >>> + >>> + err = spi_flash_write(flash, to, len, buf); >>> + if (!err) >>> + *retlen = len; >>> + >>> + return err; >>> +} >>> + >>> +static void spi_flash_mtd_sync(struct mtd_info *mtd) >>> +{ >>> +} >>> + >>> +static int spi_flash_mtd_number(void) >>> +{ >>> +#ifdef CONFIG_SYS_MAX_FLASH_BANKS >>> + return CONFIG_SYS_MAX_FLASH_BANKS; >>> +#else >>> + return 0; >>> +#endif >>> +} >>> + >>> +int spi_flash_mtd_register(struct spi_flash *flash) >>> +{ >>> + memset(&sf_mtd_info, 0, sizeof(sf_mtd_info)); >>> + sprintf(sf_mtd_name, "nor%d", spi_flash_mtd_number()); >>> + >>> + sf_mtd_info.name = sf_mtd_name; >>> + sf_mtd_info.type = MTD_NORFLASH; >>> + sf_mtd_info.flags = MTD_CAP_NORFLASH; >>> + sf_mtd_info.writesize = 1; >>> + sf_mtd_info.writebufsize = flash->page_size; >>> + >>> + sf_mtd_info._erase = spi_flash_mtd_erase; >>> + sf_mtd_info._read = spi_flash_mtd_read; >>> + sf_mtd_info._write = spi_flash_mtd_write; >>> + sf_mtd_info._sync = spi_flash_mtd_sync; >> >> >> Even if I remove this every thing as usual, like "sf erase" will call >> from cmd_sf >> there it calls mtd_arg_off_size, to detected off and size from >> partition and after >> sf_flash will call erase ops from sf_ops.c. >> >> As it's a mtd call, I thought sf_mtd_into._erase will intern calls >> erase ops from sf_ops.c >> What is this behavior could you please help me? >> >>> + >>> + sf_mtd_info.size = flash->size; >>> + sf_mtd_info.priv = flash; >>> + >>> + /* Only uniform flash devices for now */ >>> + sf_mtd_info.numeraseregions = 0; >>> + sf_mtd_info.erasesize = flash->sector_size; >>> + >>> + return add_mtd_device(&sf_mtd_info); >>> +} >>> + >>> +void spi_flash_mtd_unregister(void) >>> +{ >>> + del_mtd_device(&sf_mtd_info); >>> +} >>> diff --git a/drivers/mtd/spi/sf_probe.c b/drivers/mtd/spi/sf_probe.c >>> index d19138d..2342972 100644 >>> --- a/drivers/mtd/spi/sf_probe.c >>> +++ b/drivers/mtd/spi/sf_probe.c >>> @@ -397,10 +397,9 @@ int spi_flash_probe_slave(struct spi_slave *spi, >>> struct spi_flash *flash) >>> if (spi_enable_wp_pin(flash)) >>> puts("Enable WP pin failed\n"); >>> >>> - /* Release spi bus */ >>> - spi_release_bus(spi); >>> - >>> - return 0; >>> +#ifdef CONFIG_SPI_FLASH_MTD >>> + ret = spi_flash_mtd_register(flash); >>> +#endif >>> >>> err_read_id: >>> spi_release_bus(spi); >>> @@ -450,6 +449,9 @@ struct spi_flash *spi_flash_probe_fdt(const void >>> *blob, int slave_node, >>> >>> void spi_flash_free(struct spi_flash *flash) >>> { >>> +#ifdef CONFIG_SPI_FLASH_MTD >>> + spi_flash_mtd_unregister(); >>> +#endif >>> spi_free_slave(flash->spi); >>> free(flash); >>> } >>> -- >>> 2.1.0 thanks! -- Jagan | Openedev. _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot