[U-Boot] [PATCH v4 2/3] cfi_flash: convert to driver model

Simon Glass
2015-11-06T03:15:39Z

Hi Thomas,

On 3 November 2015 at 06:09, Thomas Chou <thomas@wytron.com.tw> wrote:
> Convert cfi flash to driver model.
>
> Signed-off-by: Thomas Chou <thomas@wytron.com.tw>
> ---
> v2
> add dts binding.
> add more help to Kconfig.
> move struct platdata to top of file as Simon suggested.
> v3
> change to MTD uclass.
> v4
> fix fdt addr and size cells in cfi_flash_probe().
> move probe uclass to cfi_flash_dm_init().
> add comment as suggested by Stefan.
>
> doc/device-tree-bindings/mtd/mtd-physmap.txt | 88 ++++++++++++++++++++++++++++
> drivers/mtd/Kconfig | 11 ++++
> drivers/mtd/cfi_flash.c | 81 +++++++++++++++++++++++++
> drivers/mtd/cfi_mtd.c | 2 +-
> include/mtd/cfi_flash.h | 3 +
> 5 files changed, 184 insertions(+), 1 deletion(-)
> create mode 100644 doc/device-tree-bindings/mtd/mtd-physmap.txt
>
> diff --git a/doc/device-tree-bindings/mtd/mtd-physmap.txt b/doc/device-tree-bindings/mtd/mtd-physmap.txt
> new file mode 100644
> index 0000000..4b8c489
> --- /dev/null
> +++ b/doc/device-tree-bindings/mtd/mtd-physmap.txt
> @@ -0,0 +1,88 @@
> +CFI or JEDEC memory-mapped NOR flash, MTD-RAM (NVRAM...)
> +
> +Flash chips (Memory Technology Devices) are often used for solid state
> +file systems on embedded devices.
> +
> + - compatible : should contain the specific model of mtd chip(s)
> + used, if known, followed by either "cfi-flash", "jedec-flash",
> + "mtd-ram" or "mtd-rom".
> + - reg : Address range(s) of the mtd chip(s)
> + It's possible to (optionally) define multiple "reg" tuples so that
> + non-identical chips can be described in one node.
> + - bank-width : Width (in bytes) of the bank. Equal to the
> + device width times the number of interleaved chips.
> + - device-width : (optional) Width of a single mtd chip. If
> + omitted, assumed to be equal to 'bank-width'.
> + - #address-cells, #size-cells : Must be present if the device has
> + sub-nodes representing partitions (see below). In this case
> + both #address-cells and #size-cells must be equal to 1.
> + - no-unaligned-direct-access: boolean to disable the default direct
> + mapping of the flash.
> + On some platforms (e.g. MPC5200) a direct 1:1 mapping may cause
> + problems with JFFS2 usage, as the local bus (LPB) doesn't support
> + unaligned accesses as implemented in the JFFS2 code via memcpy().
> + By defining "no-unaligned-direct-access", the flash will not be
> + exposed directly to the MTD users (e.g. JFFS2) any more.
> + - linux,mtd-name: allow to specify the mtd name for retro capability with
> + physmap-flash drivers as boot loader pass the mtd partition via the old
> + device name physmap-flash.
> + - use-advanced-sector-protection: boolean to enable support for the
> + advanced sector protection (Spansion: PPB - Persistent Protection
> + Bits) locking.
> +
> +For JEDEC compatible devices, the following additional properties
> +are defined:
> +
> + - vendor-id : Contains the flash chip's vendor id (1 byte).
> + - device-id : Contains the flash chip's device id (1 byte).
> +
> +For ROM compatible devices (and ROM fallback from cfi-flash), the following
> +additional (optional) property is defined:
> +
> + - erase-size : The chip's physical erase block size in bytes.
> +
> +The device tree may optionally contain sub-nodes describing partitions of the
> +address space. See partition.txt for more detail.
> +
> +Example:
> +
> + flash at ff000000 {
> + compatible = "amd,am29lv128ml", "cfi-flash";
> + reg = <ff000000 01000000>;
> + bank-width = <4>;
> + device-width = <1>;
> + #address-cells = <1>;
> + #size-cells = <1>;
> + fs at 0 {
> + label = "fs";
> + reg = <0 f80000>;
> + };
> + firmware at f80000 {
> + label ="firmware";
> + reg = <f80000 80000>;
> + read-only;
> + };
> + };
> +
> +Here an example with multiple "reg" tuples:
> +
> + flash at f0000000,0 {
> + #address-cells = <1>;
> + #size-cells = <1>;
> + compatible = "intel,pc48f4400p0vb", "cfi-flash";
> + reg = <0 0x00000000 0x02000000
> + 0 0x02000000 0x02000000>;
> + bank-width = <2>;
> + partition at 0 {
> + label = "test-part1";
> + reg = <0 0x04000000>;
> + };
> + };
> +
> +An example using SRAM:
> +
> + sram at 2,0 {
> + compatible = "samsung,k6f1616u6a", "mtd-ram";
> + reg = <2 0 0x00200000>;
> + bank-width = <2>;
> + };
> diff --git a/drivers/mtd/Kconfig b/drivers/mtd/Kconfig
> index 23dff48..367c4fe 100644
> --- a/drivers/mtd/Kconfig
> +++ b/drivers/mtd/Kconfig
> @@ -8,6 +8,17 @@ config MTD
> flash, RAM and similar chips, often used for solid state file
> systems on embedded devices.
>
> +config CFI_FLASH
> + bool "Enable Driver Model for CFI Flash driver"
> + depends on MTD
> + help
> + The Common Flash Interface specification was developed by Intel,
> + AMD and other flash manufactures that provides a universal method

flash manufacturers. It provides a universal method

> + for probing the capabilities of flash devices. If you wish to
> + support any device that is CFI-compliant, you need to enable this
> + option. Visit <http://www.amd.com/products/nvd/overview/cfi.html>
> + for more information on CFI.
> +
> endmenu
>
> source "drivers/mtd/nand/Kconfig"
> diff --git a/drivers/mtd/cfi_flash.c b/drivers/mtd/cfi_flash.c
> index fc7a878..5863699 100644
> --- a/drivers/mtd/cfi_flash.c
> +++ b/drivers/mtd/cfi_flash.c
> @@ -18,6 +18,9 @@
> /* #define DEBUG */
>
> #include <common.h>
> +#include <dm.h>
> +#include <errno.h>
> +#include <fdt_support.h>
> #include <asm/processor.h>
> #include <asm/io.h>
> #include <asm/byteorder.h>
> @@ -47,6 +50,8 @@
> * reading and writing ... (yes there is such a Hardware).
> */
>
> +DECLARE_GLOBAL_DATA_PTR;
> +
> static uint flash_offset_cfi[2] = { FLASH_OFFSET_CFI, FLASH_OFFSET_CFI_ALT };
> #ifdef CONFIG_FLASH_CFI_MTD
> static uint flash_verbose = 1;
> @@ -87,10 +92,36 @@ static u16 cfi_flash_config_reg(int i)
> int cfi_flash_num_flash_banks = CONFIG_SYS_MAX_FLASH_BANKS_DETECT;
> #endif
>
> +#ifdef CONFIG_CFI_FLASH /* for driver model */
> +static void cfi_flash_init_dm(void)
> +{
> + struct udevice *dev;
> +
> + cfi_flash_num_flash_banks = 0;
> + /*
> + * The uclass_first_device() will probe the first device and
> + * uclass_next_device() will probe the rest if they exist. So
> + * that cfi_flash_probe() will get called assigning the base
> + * addresses that are available.
> + */
> + for (uclass_first_device(UCLASS_MTD, &dev);
> + dev;
> + uclass_next_device(&dev)) {
> + }

Why do you need to probe these before they are used? Also I think you
need error checking.

> +}
> +
> +static phys_addr_t cfi_flash_base[CFI_MAX_FLASH_BANKS];
> +
> +phys_addr_t cfi_flash_bank_addr(int i)
> +{
> + return cfi_flash_base[i];
> +}
> +#else
> __weak phys_addr_t cfi_flash_bank_addr(int i)
> {
> return ((phys_addr_t [])CONFIG_SYS_FLASH_BANKS_LIST)[i];
> }
> +#endif
>
> __weak unsigned long cfi_flash_bank_size(int i)
> {
> @@ -2322,6 +2353,10 @@ unsigned long flash_init (void)
> getenv_f("unlock", s, sizeof(s));
> #endif
>
> +#ifdef CONFIG_CFI_FLASH /* for driver model */
> + cfi_flash_init_dm();
> +#endif
> +
> /* Init: no FLASHes known */
> for (i = 0; i < CONFIG_SYS_MAX_FLASH_BANKS; ++i) {
> flash_info[i].flash_id = FLASH_UNKNOWN;
> @@ -2398,3 +2433,49 @@ unsigned long flash_init (void)
>
> return (size);
> }
> +
> +#ifdef CONFIG_CFI_FLASH /* for driver model */
> +static int cfi_flash_probe(struct udevice *dev)
> +{
> + void *blob = (void *)gd->fdt_blob;
> + int node = dev->of_offset;
> + const fdt32_t *cell;
> + phys_addr_t addr;
> + int parent, addrc, sizec;
> + int len, idx;
> +
> + parent = fdt_parent_offset(blob, node);
> + of_bus_default_count_cells(blob, parent, &addrc, &sizec);
> + /* decode regs, there may be multiple reg tuples. */
> + cell = fdt_getprop(blob, node, "reg", &len);
> + if (!cell)
> + return -ENOENT;
> + idx = 0;
> + len /= sizeof(fdt32_t);
> + while (idx < len) {
> + addr = fdt_translate_address((void *)blob,
> + node, cell + idx);
> + cfi_flash_base[cfi_flash_num_flash_banks++] = addr;
> + idx += addrc + sizec;
> + }
> + gd->bd->bi_flashstart = cfi_flash_base[0];
> +#ifdef CONFIG_FLASH_CFI_MTD
> + dev->uclass_priv = &cfi_mtd_info[0];

Could you put cfi_mtd_info in a struct and use auto-allocation to allocate it?

> + cfi_mtd_info[0].dev = dev;
> +#endif
> + return 0;
> +}
> +
> +static const struct udevice_id cfi_flash_ids[] = {
> + { .compatible = "cfi-flash" },
> + { .compatible = "jedec-flash" },
> + {}
> +};
> +
> +U_BOOT_DRIVER(cfi_flash) = {
> + .name = "cfi_flash",
> + .id = UCLASS_MTD,
> + .of_match = cfi_flash_ids,
> + .probe = cfi_flash_probe,
> +};
> +#endif /* CONFIG_CFI_FLASH */
> diff --git a/drivers/mtd/cfi_mtd.c b/drivers/mtd/cfi_mtd.c
> index 709a486..dcd7ab0 100644
> --- a/drivers/mtd/cfi_mtd.c
> +++ b/drivers/mtd/cfi_mtd.c
> @@ -15,7 +15,7 @@
> #include <linux/mtd/concat.h>
> #include <mtd/cfi_flash.h>
>
> -static struct mtd_info cfi_mtd_info[CFI_MAX_FLASH_BANKS];
> +struct mtd_info cfi_mtd_info[CFI_MAX_FLASH_BANKS];
> static char cfi_mtd_names[CFI_MAX_FLASH_BANKS][16];
> #ifdef CONFIG_MTD_CONCAT
> static char c_mtd_name[16];
> diff --git a/include/mtd/cfi_flash.h b/include/mtd/cfi_flash.h
> index 52572b9..c1632f3 100644
> --- a/include/mtd/cfi_flash.h
> +++ b/include/mtd/cfi_flash.h
> @@ -8,6 +8,8 @@
> #ifndef __CFI_FLASH_H__
> #define __CFI_FLASH_H__
>
> +#include <linux/mtd/mtd.h>
> +
> #define FLASH_CMD_CFI 0x98
> #define FLASH_CMD_READ_ID 0x90
> #define FLASH_CMD_RESET 0xff
> @@ -164,6 +166,7 @@ extern int cfi_flash_num_flash_banks;
> #else
> #define CFI_MAX_FLASH_BANKS CONFIG_SYS_MAX_FLASH_BANKS
> #endif
> +extern struct mtd_info cfi_mtd_info[];
>
> void flash_write_cmd(flash_info_t * info, flash_sect_t sect,
> uint offset, u32 cmd);
> --
> 2.5.0
>

Regards,
Simon