Hello Jagan,

Am 16.06.2015 um 10:52 schrieb Jagan Teki:
Hi Heiko,

On 16 June 2015 at 14:13, Heiko Schocher denx <h...@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 <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.


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?

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 ...

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 <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!


--
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Reply via email to