[U-Boot] [PATCH v6 1/4] mtd, spi: add MTD layer driver
Heiko Schocher denx
hs at denx.de
Tue Jun 16 11:18:01 CEST 2015
Hello Jagan,
Am 16.06.2015 um 10:52 schrieb Jagan Teki:
> Hi Heiko,
>
> On 16 June 2015 at 14:13, Heiko Schocher denx <hs at 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 <hs at 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 at 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 <hs at denx.de> wrote:
>>>>>>
>>>>>>
>>>>>> From: Daniel Schwierzeck <daniel.schwierzeck at 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.schwierzeck at gmail.com>
>>>>>> Signed-off-by: Heiko Schocher <hs at 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.schwierzeck at 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
More information about the U-Boot
mailing list