From mboxrd@z Thu Jan 1 00:00:00 1970 From: Heiko Schocher denx Date: Tue, 16 Jun 2015 10:43:50 +0200 Subject: [U-Boot] [PATCH v6 1/4] mtd, spi: add MTD layer driver In-Reply-To: References: <1430113328-29184-1-git-send-email-hs@denx.de> <1430113328-29184-2-git-send-email-hs@denx.de> <555C2DC3.6030400@denx.de> Message-ID: <557FE1C6.6010701@denx.de> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Hello Jagan, Am 16.06.2015 um 10:04 schrieb Jagan Teki: > Hi Heiko, > > On 20 May 2015 at 12:16, Heiko Schocher 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 , # 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? thanks! bye, Heiko >> 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 , # 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 , # 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 wrote: >>>> >>>> From: Daniel Schwierzeck >>>> >>>> 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 >>>> Signed-off-by: Heiko Schocher >>>> >>>> --- >>>> >>>> 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.schwierzeck at gmail.com >>>> + * >>>> + * SPDX-License-Identifier: GPL-2.0+ >>>> + */ >>>> + >>>> +#include >>>> +#include >>>> +#include >>>> +#include >>>> +#include >>>> + >>>> +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! > -- DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany