dm: implement a cfi flash uclass

Implement a cfi flash uclass to work with drivers/mtd/cfi-flash.c. The flash base address is extracted from device tree, and passed to cfi_flash_bank_addr().

Signed-off-by Thomas Chou <[email protected]>

---
 common/board_r.c               |  3 ++
 drivers/mtd/Kconfig            | 11 +++++++
 drivers/mtd/Makefile           |  1 +
 drivers/mtd/cfi-flash-uclass.c | 70 ++++++++++++++++++++++++++++++++++++++++++
 drivers/mtd/cfi_flash.c        | 19 ++++++++++++
 include/cfi-flash.h            | 27 ++++++++++++++++
 include/dm/uclass-id.h         |  1 +
 7 files changed, 132 insertions(+)
 create mode 100644 drivers/mtd/cfi-flash-uclass.c
 create mode 100644 include/cfi-flash.h

common/board_r.c +1 -0

index aaf390e..86b606d 100644
@@ -38,6 +38,7 @@
 #include <miiphy.h>
 #endif
 #include <mmc.h>
+#include <mtd/cfi_flash.h>
 #include <nand.h>
 #include <onenand_uboot.h>
 #include <scsi.h>

common/board_r.c +0 -0

        /* size of FLASH memory (final value) */
        bd->bi_flashsize = flash_size;

drivers/mtd/Kconfig +0 -0

index 59278d1..4b52894 100644

drivers/mtd/Kconfig +3 -0

+
+endmenu
+
 source "drivers/mtd/nand/Kconfig"
 
 source "drivers/mtd/spi/Kconfig"

drivers/mtd/Makefile +1 -0

index a623f4c..d86e2c2 100644
@@ -11,6 +11,7 @@ endif
 obj-$(CONFIG_MTD_PARTITIONS) += mtdpart.o
 obj-$(CONFIG_MTD_CONCAT) += mtdconcat.o
 obj-$(CONFIG_HAS_DATAFLASH) += at45.o
+obj-$(CONFIG_DM_CFI_FLASH) += cfi-flash-uclass.o
 obj-$(CONFIG_FLASH_CFI_DRIVER) += cfi_flash.o
 obj-$(CONFIG_FLASH_CFI_MTD) += cfi_mtd.o
 obj-$(CONFIG_HAS_DATAFLASH) += dataflash.o

drivers/mtd/cfi-flash-uclass.c new file +25 -0

new file mode 100644
index 0000000..1db7e2f
@@ -0,0 +1,70 @@
+/*
+ * Copyright (C) 2015 Thomas Chou <[email protected]>
+ *
+ * SPDX-License-Identifier:    GPL-2.0+
+ */
+
+#include <common.h>
+#include <dm.h>
+#include <errno.h>
+#include <fdtdec.h>
+#include <cfi-flash.h>
+#include <asm/io.h>
+
+DECLARE_GLOBAL_DATA_PTR;
+
+/*
+ * Implement a cfi flash uclass to work with drivers/mtd/cfi-flash.c.
+ */
+
+phys_addr_t cfi_flash_get_base(struct udevice *dev)
+{
+       struct cfi_flash_dev_priv *uc_priv = dev_get_uclass_priv(dev);
+
+       return uc_priv->base;
+}

drivers/mtd/cfi-flash-uclass.c new file +14 -0

+
+static int cfi_flash_probe(struct udevice *dev)
+{
+       struct cfi_flash_dev_priv *uc_priv = dev_get_uclass_priv(dev);
+       struct cfi_flash_platdata *plat = dev->platdata;
+
+       uc_priv->base = plat->base;
+
+       return 0;
+}
+
+static int cfi_flash_ofdata_to_platdata(struct udevice *dev)
+{
+       struct cfi_flash_platdata *plat = dev_get_platdata(dev);

drivers/mtd/cfi-flash-uclass.c new file +11 -0

+       { }
+};
+
+U_BOOT_DRIVER(cfi_flash) = {
+       .name   = "cfi_flash",
+       .id     = UCLASS_CFI_FLASH,
+       .of_match = cfi_flash_ids,
+       .ofdata_to_platdata = cfi_flash_ofdata_to_platdata,
+       .platdata_auto_alloc_size = sizeof(struct cfi_flash_platdata),
+       .probe = cfi_flash_probe,
+};

drivers/mtd/cfi_flash.c +4 -0

index 50983b8..6ec0877 100644
@@ -18,6 +18,9 @@
 /* #define DEBUG       */
 
 #include <common.h>
+#include <dm.h>
+#include <errno.h>
+#include <cfi-flash.h>
 #include <asm/processor.h>
 #include <asm/io.h>
 #include <asm/byteorder.h>

@@ -87,10 +90,26 @@ static u16 cfi_flash_config_reg(int i)
 int cfi_flash_num_flash_banks = CONFIG_SYS_MAX_FLASH_BANKS_DETECT;
 #endif
 
+#ifdef CONFIG_DM_CFI_FLASH

drivers/mtd/cfi_flash.c +5 -0

+
+       return cfi_flash_get_base(dev);
+}
+#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)
 {

include/cfi-flash.h new file +27 -0

new file mode 100644
index 0000000..0064cba
@@ -0,0 +1,27 @@
+/*
+ * Copyright (C) 2015 Thomas Chou <[email protected]>
+ *
+ * SPDX-License-Identifier:    GPL-2.0+
+ */
+
+#ifndef _DM_CFI_FLASH_H_
+#define _DM_CFI_FLASH_H_
+
+/*
+ * Get the cfi flash base address
+ *
+ * @dev: The cfi flash device
+ * @return: the cfi flash base address
+ */
+phys_addr_t cfi_flash_get_base(struct udevice *dev);
+
+/*
+ * struct cfi_flash_dev_priv - information about a device used by the uclass
+ *
+ * @base: the cfi flash base address
+ */
+struct cfi_flash_dev_priv {
+       phys_addr_t base;
+};
+
+#endif /* _DM_CFI_FLASH_H_ */

include/dm/uclass-id.h +1 -1

index a6982ab..09e370c 100644
@@ -25,6 +25,7 @@ enum uclass_id {
        UCLASS_SIMPLE_BUS,      /* bus with child devices */
 
        /* U-Boot uclasses start here - in alphabetical order */
+       UCLASS_CFI_FLASH,       /* cfi flash */
        UCLASS_CLK,             /* Clock source, e.g. used by peripherals */
        UCLASS_CPU,             /* CPU, typically part of an SoC */
        UCLASS_CROS_EC,         /* Chrome OS EC */
--

2.1.4

Can you create a sandbox driver for this so you can add a test?

common/board_r.c static int initr_flash(void) · line 354

Quoted 7 lines

common/board_r.c +2 -0

@@ -348,6 +349,8 @@ static int initr_flash(void)
         /* update start of FLASH memory    */
  #ifdef CONFIG_SYS_FLASH_BASE
         bd->bi_flashstart = CONFIG_SYS_FLASH_BASE;
+#else
+       bd->bi_flashstart = cfi_flash_bank_addr(0);
  #endif

Can we make this dynamic - i.e. only probe the device when it is used? Then we could remove initr_flash() in the DM case.

drivers/mtd/Kconfig line 8

Quoted 8 lines

drivers/mtd/Kconfig +8 -0

+menu "CFI Flash Support"
+
+config DM_CFI_FLASH
+       bool "Enable Driver Model for CFI Flash drivers"
+       depends on DM
+       help
+         Enable driver model for CFI flash access. It uses the same API as
+         drivers/mtd/cfi_flash.c. But now implemented by the uclass.

In the help can you explain what CFI is and what it is for?

drivers/mtd/cfi-flash-uclass.c line 35

Quoted 9 lines

drivers/mtd/cfi-flash-uclass.c new file +9 -0

+UCLASS_DRIVER(cfi_flash) = {
+       .id             = UCLASS_CFI_FLASH,
+       .name           = "cfi_flash",
+       .per_device_auto_alloc_size = sizeof(struct cfi_flash_dev_priv),
+};
+
+struct cfi_flash_platdata {
+       phys_addr_t base;
+};

Can you put this declaration at the top of the file?

Is there a binding somewhere for this?

Quoted 7 lines

drivers/mtd/cfi_flash.c +7 -0

+#ifdef CONFIG_DM_CFI_FLASH
+phys_addr_t cfi_flash_bank_addr(int i)
+{
+       struct udevice *dev;
+       int ret;
+
+       ret = uclass_get_device(UCLASS_CFI_FLASH, i, &dev);

drivers/mtd/cfi_flash.c static u16 cfi_flash_config_reg(int i) · line 103

Quoted 4 lines

drivers/mtd/cfi_flash.c +4 -0

+       if (ret)
+               return ret;
+       if (!dev)
+               return -ENODEV;

That function will never return a NULL dev, unless it returns an error. It is different from uclass_first_device(). Also are you sure you want uclass_get_device() and not uclass_get_device_by_seq()?

Quoted 9 lines

drivers/mtd/cfi_flash.c static u16 cfi_flash_config_reg(int i) · line 103

Quoted 5 lines

drivers/mtd/cfi_flash.c +5 -0

+       ret = uclass_get_device(UCLASS_CFI_FLASH, i, &dev);
+       if (ret)
+               return ret;
+       if (!dev)
+               return -ENODEV;

That function will never return a NULL dev, unless it returns an error. It is different from uclass_first_device(). Also are you sure you want uclass_get_device() and not uclass_get_device_by_seq()?

Yes, I should use by_seq. Thanks for reminding.

Hi Simon,

Quoted 8 lines · Thomas Chou

Quoted 8 lines

Quoted 6 lines

drivers/mtd/cfi_flash.c +5 -0

+       ret = uclass_get_device(UCLASS_CFI_FLASH, i, &dev);
+       if (ret)
+               return ret;
+       if (!dev)
+               return -ENODEV;

Quoted 6 lines

Quoted 3 lines

That function will never return a NULL dev, unless it returns an error. It is different from uclass_first_device(). Also are you sure you want uclass_get_device() and not uclass_get_device_by_seq()?

Yes, I should use by_seq. Thanks for reminding.

I tried uclass_get_device_by_seq(), but it failed without proper seq assignment. So I reverted uclass_get_device(), as we didn't assign seq for a single cfi-flash.

cfi_flash_64m: flash at 0x0 {
        compatible = "cfi-flash";
        reg = <0x00000000 0x04000000>;

I have a problem on device tree decoding. Would you please give me some light on decoding with multiple "reg" tuples, like this,

        reg = <0 0x00000000 0x02000000
               0 0x02000000 0x02000000>;

or,

tse_mac: ethernet at 0x4000 {
        compatible = "altr,tse-1.0";
        reg = <0x00004000 0x00000400>,
                <0x00004400 0x00000040>,
                <0x00004800 0x00000040>,
                <0x00002000 0x00002000>;
        reg-names = "control_port", "rx_csr", "tx_csr", "s1";

Thank you in advance.

You can check fdtdec_get_pci_addr() to see how multiple "reg" tuples are decoded.

Implement a cfi flash uclass to work with drivers/mtd/cfi-flash.c. The flash base address is extracted from device tree, and passed to cfi_flash_bank_addr().

The current code supports only one bank. It should be extended to support multiple banks by decoding multiple "reg" tuples, eg,

        reg = <0 0x00000000 0x02000000
               0 0x02000000 0x02000000>;

Signed-off-by Thomas Chou <[email protected]>

---
v2
add dts binding.
add more help to Kconfig.
move struct platdata to top of file as Simon suggested.

 common/board_r.c                             |  3 +
 configs/nios2-generic_defconfig              |  1 +
 doc/device-tree-bindings/mtd/mtd-physmap.txt | 88 ++++++++++++++++++++++++++++
 drivers/mtd/Kconfig                          | 16 +++++
 drivers/mtd/Makefile                         |  1 +
 drivers/mtd/cfi-flash-uclass.c               | 70 ++++++++++++++++++++++
 drivers/mtd/cfi_flash.c                      | 18 ++++++
 include/cfi-flash.h                          | 27 +++++++++
 include/dm/uclass-id.h                       |  1 +
 9 files changed, 225 insertions(+)
 create mode 100644 doc/device-tree-bindings/mtd/mtd-physmap.txt
 create mode 100644 drivers/mtd/cfi-flash-uclass.c
 create mode 100644 include/cfi-flash.h

common/board_r.c +3 -0

index a4facf8..fceaea6 100644
@@ -38,6 +38,7 @@
 #include <miiphy.h>
 #endif
 #include <mmc.h>
+#include <mtd/cfi_flash.h>
 #include <nand.h>
 #include <onenand_uboot.h>
 #include <scsi.h>

@@ -348,6 +349,8 @@ static int initr_flash(void)
        /* update start of FLASH memory    */
 #ifdef CONFIG_SYS_FLASH_BASE
        bd->bi_flashstart = CONFIG_SYS_FLASH_BASE;
+#else
+       bd->bi_flashstart = cfi_flash_bank_addr(0);
 #endif
        /* size of FLASH memory (final value) */
        bd->bi_flashsize = flash_size;

configs/nios2-generic_defconfig +1 -0

index 3d404b2..7b504ba 100644
@@ -18,6 +18,7 @@ CONFIG_NET_RANDOM_ETHADDR=y
 CONFIG_ALTERA_PIO=y
 CONFIG_MISC=y
 CONFIG_ALTERA_SYSID=y
+CONFIG_CFI_FLASH=y
 CONFIG_ALTERA_JTAG_UART=y
 CONFIG_ALTERA_JTAG_UART_BYPASS=y
 CONFIG_TIMER=y

doc/device-tree-bindings/mtd/mtd-physmap.txt new file +62 -0

new file mode 100644
index 0000000..a04d6e8
@@ -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>;

doc/device-tree-bindings/mtd/mtd-physmap.txt new file +6 -0

+               reg = <0 0x00000000 0x02000000
+                      0 0x02000000 0x02000000>;
+               bank-width = <2>;
+               partition at 0 {
+                       label = "test-part1";
+                       reg = <0 0x04000000>;

By looking at the implementation, is it really necessary to add a new uclass for cfi-flash, given we only need get the flash base address? There are no cfi-flash specific ops defined.
Why not just read its base address from device tree?

Hi Bin,

Quoted 5 lines · Bin Meng

By looking at the implementation, is it really necessary to add a new uclass for cfi-flash, given we only need get the flash base address? There are no cfi-flash specific ops defined.
Why not just read its base address from device tree?

Yes, there is not flash ops involved at the moment.

I have tried to minimize the change to existing cfi_flash driver, and got it just works when I am converting all nios2 related devices to driver model and device tree control.

Yet it is just the beginning to convert cfi flash to driver model. It will include per device flash_info and flash related operations in the full extent.

It's unlikely we are going to create another instance of cfi-flash driver, isn't it? Then it's just a single driver which does not make cfi-flash a "class". We can just update the cfi-flash driver to read its flash base address directly from device tree, without the need to go through driver model.

Hi Bin,

Quoted 6 lines · Bin Meng

It's unlikely we are going to create another instance of cfi-flash driver, isn't it? Then it's just a single driver which does not make cfi-flash a "class". We can just update the cfi-flash driver to read its flash base address directly from device tree, without the need to go through driver model.

It is an on-going process to convert existing drivers to driver model. The spi-flash has driver model now. How do you think it is unlikely that cfi-flash should be converted to driver-model?

It is not just base address. There is device binding with compatible ids. There will be resources allocation, too. Please don't limit your imagination by my poor coding skill.

There are three good things to u-boot in recent years, the Kconfig, device tree control and driver model. Together, they provide more flexibility to processors on FPGA, like nios2. Here almost everything is programmable, and it is difficult to hard code driver setup in the old way.

Huge thanks to all the contributors. Thank you too, Bin.

The spi-flash is converted to driver model, which is good, as there are spi flashes from different vendors which have different op codes thus need different drivers to handle. But for cfi-flash, almost every cfi-flash we see in the market conforms to the same CFI spec, thus we only need one driver (drivers/mtd/cfi_flash.c) to work with all these flashes, right? Unless I am missing something recently, eg: some vendors started to created flashes which are not 100% compatible with the CFI spec? If this is the only single driver, I don't see the need to create a special driver model uclass for it. Just an open discussion. I am not saying we should, or we should not :)

Quoted 3 lines

It is not just base address. There is device binding with compatible ids. There will be resources allocation, too. Please don't limit your imagination by my poor coding skill.

All there can be obtained from device tree. Being a non-DM driver does not prevent you from using device tree.

Hi Bin,

Quoted 11 lines · Bin Meng

The spi-flash is converted to driver model, which is good, as there are spi flashes from different vendors which have different op codes thus need different drivers to handle. But for cfi-flash, almost every cfi-flash we see in the market conforms to the same CFI spec, thus we only need one driver (drivers/mtd/cfi_flash.c) to work with all these flashes, right? Unless I am missing something recently, eg: some vendors started to created flashes which are not 100% compatible with the CFI spec? If this is the only single driver, I don't see the need to create a special driver model uclass for it. Just an open discussion. I am not saying we should, or we should not :)

I had the same question myself. But I found that there are several uclasses which has only one driver. No worries. :)

Quoted 7 lines

Quoted 5 lines

It is not just base address. There is device binding with compatible ids. There will be resources allocation, too. Please don't limit your imagination
by my poor coding skill.

Quoted 2 lines

All there can be obtained from device tree. Being a non-DM driver does not prevent you from using device tree.

It is true. Yet using driver model does have some advantages over non-DM. It is more unified and dynamic. So I chose the DM way when I have to add device tree binding. You may find my patch quite trivial. Thanks to the DM framework.

I don't see any problem with this in general, although the point with CFI is that you are really implementing the uclass, as there will not going to be any drivers (as I understand it).

Looking at SPI flash, it does have an API (read, write, erase). Could CFI use the same API? That might mean that we could unify the two and share a uclass.

One approach may be to use a name like UCLASS_FLASH, and add some operations to it. Then we can say that we have a real uclass. The operations could come in a separate flash perhaps.

Hi,

Quoted 18 lines · Thomas Chou

Hi Bin,

Quoted 11 lines · Bin Meng

The spi-flash is converted to driver model, which is good, as there are spi flashes from different vendors which have different op codes thus need different drivers to handle. But for cfi-flash, almost every cfi-flash we see in the market conforms to the same CFI spec, thus we only need one driver (drivers/mtd/cfi_flash.c) to work with all these flashes, right? Unless I am missing something recently, eg: some vendors started to created flashes which are not 100% compatible with the CFI spec? If this is the only single driver, I don't see the need to create a special driver model uclass for it. Just an open discussion. I am not saying we should, or we should not :)

I had the same question myself. But I found that there are several uclasses which has only one driver. No worries. :)

I don't see any problem with this in general, although the point with CFI is that you are really implementing the uclass, as there will not going to be any drivers (as I understand it).

Looking at SPI flash, it does have an API (read, write, erase). Could CFI use the same API? That might mean that we could unify the two and share a uclass.

One approach may be to use a name like UCLASS_FLASH, and add some operations to it. Then we can say that we have a real uclass. The operations could come in a separate flash perhaps.

Quoted 11 lines

Quoted 9 lines

Quoted 5 lines

It is not just base address. There is device binding with compatible ids. There will be resources allocation, too. Please don't limit your imagination
by my poor coding skill.

All there can be obtained from device tree. Being a non-DM driver does not prevent you from using device tree.

Quoted 4 lines

It is true. Yet using driver model does have some advantages over non-DM. It is more unified and dynamic. So I chose the DM way when I have to add device tree binding. You may find my patch quite trivial. Thanks to the DM framework.

Regards,
Simon

doc/device-tree-bindings/mtd/mtd-physmap.txt line 72

Quoted 4 lines

doc/device-tree-bindings/mtd/mtd-physmap.txt new file +4 -0

+       flash at f0000000,0 {
+               #address-cells = <1>;
+               #size-cells = <1>;
+               compatible = "intel,PC48F4400P0VB", "cfi-flash";

PC48F4400P0VB should be lower cases

Convert cfi flash to driver model.

Signed-off-by Thomas Chou <[email protected]>

---
v2
add dts binding.
add more help to Kconfig.
move struct platdata to top of file as Simon suggested.
v3
change to MTD uclass.

  doc/device-tree-bindings/mtd/mtd-physmap.txt | 88 ++++++++++++++++++++++++++++
  drivers/mtd/Kconfig                          | 11 ++++
  drivers/mtd/cfi_flash.c                      | 69 ++++++++++++++++++++++
  drivers/mtd/cfi_mtd.c                        |  2 +-
  include/mtd/cfi_flash.h                      |  3 +
  5 files changed, 172 insertions(+), 1 deletion(-)

create mode 100644 doc/device-tree-bindings/mtd/mtd-physmap.txt

doc/device-tree-bindings/mtd/mtd-physmap.txt new file +88 -0

new file mode 100644
index 0000000..4b8c489
@@ -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>;
+	};

drivers/mtd/Kconfig +11 -0

index 23dff48..367c4fe 100644
@@ -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
+	  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"

drivers/mtd/cfi_flash.c +14 -0

index fc7a878..a439d8b 100644
@@ -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,19 @@ 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
+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)
  {

drivers/mtd/cfi_flash.c +0 -0

  #ifdef CONFIG_SYS_FLASH_PROTECTION
  	/* read environment from EEPROM */

@@ -2322,6 +2337,14 @@ unsigned long flash_init (void)
  	getenv_f("unlock", s, sizeof(s));

drivers/mtd/cfi_flash.c unsigned long flash_init (void) · line 2332

Quoted 5 lines

drivers/mtd/cfi_flash.c +1 -0

@@ -2315,6 +2329,7 @@ unsigned long flash_init (void)
   {
   	unsigned long size = 0;
   	int i;
+	struct udevice *dev;

I'm seeing this warning while compiling for some powerpc boards (e.g. canyonlands):

drivers/mtd/cfi_flash.c: In function 'flash_init':
drivers/mtd/cfi_flash.c:2332:18: warning: unused variable 'dev' [-Wunused-variable]

Perhaps an "__maybe_unused" needed here.

Quoted 7 lines

drivers/mtd/cfi_flash.c +0 -0

   #ifdef CONFIG_SYS_FLASH_PROTECTION
   	/* read environment from EEPROM */

@@ -2322,6 +2337,14 @@ unsigned long flash_init (void)
   	getenv_f("unlock", s, sizeof(s));
   #endif
 

drivers/mtd/cfi_flash.c unsigned long flash_init (void) · line 2347

Quoted 8 lines

drivers/mtd/cfi_flash.c +8 -0

+#ifdef CONFIG_CFI_FLASH /* for driver model */
+	cfi_flash_num_flash_banks = 0;
+	/* probe every MTD device */
+	for (uclass_first_device(UCLASS_MTD, &dev);
+	     dev;
+	     uclass_next_device(&dev)) {
+	}
+#endif

What is this loop above exactly doing?

Quoted 6 lines

Quoted 6 lines

   #ifdef CONFIG_SYS_FLASH_PROTECTION
       /* read environment from EEPROM */

drivers/mtd/cfi_flash.c +0 -0

@@ -2322,6 +2337,14 @@ unsigned long flash_init (void)
       getenv_f("unlock", s, sizeof(s));
   #endif
 

Quoted 10 lines

drivers/mtd/cfi_flash.c unsigned long flash_init (void) · line 2347

Quoted 8 lines

drivers/mtd/cfi_flash.c +8 -0

+#ifdef CONFIG_CFI_FLASH /* for driver model */
+    cfi_flash_num_flash_banks = 0;
+    /* probe every MTD device */
+    for (uclass_first_device(UCLASS_MTD, &dev);
+         dev;
+         uclass_next_device(&dev)) {
+    }
+#endif

What is this loop above exactly doing?

I want to get the cfi-flash device probed and flash_info[] got initialized. So I look in every mtd uclass device.

I will use CONFIG_IS_ENABLED to remove the warning above.

if (CONFIG_IS_ENABLED(CFI_FLASH)) {
     struct udevice *dev;
     cfi_flash_num_flash_banks = 0;
     /* probe every MTD device to find cfi-flash device */
     for (uclass_first_device(UCLASS_MTD, &dev);
          dev;
          uclass_next_device(&dev)) {
     }
}

Quoted 9 lines

if (CONFIG_IS_ENABLED(CFI_FLASH)) {
     struct udevice *dev;
     cfi_flash_num_flash_banks = 0;
     /* probe every MTD device to find cfi-flash device */
     for (uclass_first_device(UCLASS_MTD, &dev);
          dev;
          uclass_next_device(&dev)) {
     }
}

I'm still not an expert in the DM internals. I fail to see, where the actual probing is happening in the loop. Is it buried in the uclass_first_device() call?

Quoted 10 lines · Stefan Roese

Quoted 9 lines

if (CONFIG_IS_ENABLED(CFI_FLASH)) {
     struct udevice *dev;
     cfi_flash_num_flash_banks = 0;
     /* probe every MTD device to find cfi-flash device */
     for (uclass_first_device(UCLASS_MTD, &dev);
          dev;
          uclass_next_device(&dev)) {
     }
}

Quoted 3 lines

I'm still not an expert in the DM internals. I fail to see, where the actual probing is happening in the loop. Is it buried in the uclass_first_device() call?

The probing is happening in both uclass_first_device() and uclass_next_device().

The uclass_first_device() will probe the first device. While uclass_next_device() will probe the rest if they exist.

Convert cfi flash to driver model.

Signed-off-by Thomas Chou <[email protected]>

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

Quoted 14 lines · Thomas Chou

Convert cfi flash to driver model.

Signed-off-by Thomas Chou <[email protected]>

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

Thanks Thomas. Looks pretty good now. If nobody else objects, I'll pull this series in via the cfi-flash repository in the next days.

Quoted 16 lines · Stefan Roese

Quoted 14 lines · Thomas Chou

Convert cfi flash to driver model.

Signed-off-by Thomas Chou <[email protected]>

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

Quoted 3 lines

Thanks Thomas. Looks pretty good now. If nobody else objects, I'll pull this series in via the cfi-flash repository in the next days.

May I pick up this series and push into u-boot-nios? I have some nios2 related patches depended on this. It would be helpful to send them to Tom in correct sequence. Thanks a lot.

Hi Thomas,

Quoted 21 lines · Thomas Chou

Quoted 19 lines · Stefan Roese

Quoted 14 lines · Thomas Chou

Convert cfi flash to driver model.

Signed-off-by Thomas Chou <[email protected]>

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

Thanks Thomas. Looks pretty good now. If nobody else objects, I'll pull this series in via the cfi-flash repository in the next days.

Quoted 3 lines

May I pick up this series and push into u-boot-nios? I have some nios2 related patches depended on this. It would be helpful to send them to Tom in correct sequence. Thanks a lot.

I am okay with you picking up these patches and pushing them via your nios2 repository. But we should wait until Simon is okay with this DM stuff.

Convert cfi flash to driver model.

Signed-off-by Thomas Chou <[email protected]>

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

doc/device-tree-bindings/mtd/mtd-physmap.txt new file +88 -0

new file mode 100644
index 0000000..4b8c489
@@ -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>;
+       };

drivers/mtd/Kconfig +0 -0

index 23dff48..367c4fe 100644

drivers/mtd/Kconfig +5 -0

+         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"

drivers/mtd/cfi_flash.c +11 -0

index fc7a878..5863699 100644
@@ -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;

drivers/mtd/cfi_flash.c +32 -0

+}
+
+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;

drivers/mtd/cfi_flash.c +18 -0

+       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 */

drivers/mtd/cfi_mtd.c +1 -1

index 709a486..dcd7ab0 100644
@@ -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];

include/mtd/cfi_flash.h +3 -1

index 52572b9..c1632f3 100644
@@ -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

drivers/mtd/Kconfig config MTD · line 16

Quoted 6 lines

drivers/mtd/Kconfig +6 -0

+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

Quoted 11 lines

drivers/mtd/cfi_flash.c +11 -0

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

drivers/mtd/cfi_flash.c static u16 cfi_flash_config_reg(int i) · line 110

Quoted 5 lines

drivers/mtd/cfi_flash.c +5 -0

+        */
+       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.

drivers/mtd/cfi_flash.c unsigned long flash_init (void) · line 2463

Quoted 2 lines

drivers/mtd/cfi_flash.c +2 -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?

Hi Simon,

Quoted 11 lines · Simon Glass

Quoted 9 lines

drivers/mtd/Kconfig +5 -0

+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

Quoted 2 lines

flash manufacturers. It provides a universal method

OK. Thanks.

Quoted 12 lines

Quoted 12 lines

drivers/mtd/cfi_flash.c +12 -0

+#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.
+        */

Quoted 8 lines

drivers/mtd/cfi_flash.c static u16 cfi_flash_config_reg(int i) · line 110

Quoted 4 lines

drivers/mtd/cfi_flash.c +4 -0

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

This is the place that the device is going to be used. In flash_init() the cfi flash is probed to find flash type and size.

There may be other MTD devices, so we scan through them. If there is not cfi flash device, cfi_flash_num_flash_banks will be zero and the flash_init will return size as zero. This is not an error.

Quoted 4 lines

Quoted 2 lines

drivers/mtd/cfi_flash.c +2 -0

+#ifdef CONFIG_FLASH_CFI_MTD
+       dev->uclass_priv = &cfi_mtd_info[0];

Quoted 2 lines

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

I planned this as follow-up patches which will merge cfi-mtd.c and use mtd ops as major flash ops. Then the mtd_info will be auto-allocated. This is quite similar to Jagan's work on spi-nor.

Implement a Memory Technology Device (MTD) uclass. It should include most flash drivers in the future. Though no uclass ops are defined yet, the MTD ops could be used.

The NAND flash driver is based on MTD. The CFI flash and SPI flash support MTD, too. It should make sense to convert them to MTD uclass.

Signed-off-by Thomas Chou <[email protected]>

---
v3
change to MTD uclass.
v4
add mtd_info to flash_info in flash.h.

 drivers/mtd/Kconfig      | 12 ++++++++++++
 drivers/mtd/Makefile     |  1 +
 drivers/mtd/mtd-uclass.c | 20 ++++++++++++++++++++
 include/dm/uclass-id.h   |  1 +
 include/flash.h          |  3 +++
 include/linux/mtd/mtd.h  |  3 +++
 include/mtd.h            | 23 +++++++++++++++++++++++
 7 files changed, 63 insertions(+)
 create mode 100644 drivers/mtd/mtd-uclass.c
 create mode 100644 include/mtd.h

drivers/mtd/Kconfig +12 -0

index 59278d1..23dff48 100644
@@ -1,3 +1,15 @@
+menu "MTD Support"
+
+config MTD
+       bool "Enable Driver Model for MTD drivers"
+       depends on DM
+       help
+         Enable driver model for Memory Technology Devices (MTD), such as
+         flash, RAM and similar chips, often used for solid state file
+         systems on embedded devices.
+
+endmenu
+
 source "drivers/mtd/nand/Kconfig"
 
 source "drivers/mtd/spi/Kconfig"

drivers/mtd/Makefile +1 -0

index a623f4c..c23c0c1 100644
@@ -8,6 +8,7 @@
 ifneq (,$(findstring y,$(CONFIG_MTD_DEVICE)$(CONFIG_CMD_NAND)$(CONFIG_CMD_ONENAND)$(CONFIG_CMD_SF)))
 obj-y += mtdcore.o mtd_uboot.o
 endif
+obj-$(CONFIG_MTD) += mtd-uclass.o
 obj-$(CONFIG_MTD_PARTITIONS) += mtdpart.o
 obj-$(CONFIG_MTD_CONCAT) += mtdconcat.o
 obj-$(CONFIG_HAS_DATAFLASH) += at45.o

drivers/mtd/mtd-uclass.c new file +20 -0

new file mode 100644
index 0000000..8bd3e6b
@@ -0,0 +1,20 @@
+/*
+ * Copyright (C) 2015 Thomas Chou <[email protected]>
+ *
+ * SPDX-License-Identifier:    GPL-2.0+
+ */
+
+#include <common.h>
+#include <dm.h>
+#include <errno.h>
+#include <mtd.h>
+
+/*
+ * Implement a MTD uclass which should include most flash drivers.
+ * The uclass private is pointed to mtd_info.
+ */
+
+UCLASS_DRIVER(mtd) = {
+       .id             = UCLASS_MTD,
+       .name           = "mtd",
+};

include/dm/uclass-id.h +1 -0

index 886a44c..fcc9784 100644
@@ -42,6 +42,7 @@ enum uclass_id {
        UCLASS_MISC,            /* Miscellaneous device */
        UCLASS_MMC,             /* SD / MMC card or chip */
        UCLASS_MOD_EXP,         /* RSA Mod Exp device */
+       UCLASS_MTD,             /* Memory Technology Device (MTD) device */
        UCLASS_PCH,             /* x86 platform controller hub */
        UCLASS_PCI,             /* PCI bus */
        UCLASS_PCI_GENERIC,     /* Generic PCI bus device */

include/flash.h +3 -0

index dc0645e..f53ace7 100644
@@ -44,6 +44,9 @@ typedef struct {
        ulong   addr_unlock2;           /* unlock address 2 for AMD flash roms  */
        const char *name;               /* human-readable name                  */
 #endif
+#ifdef CONFIG_MTD
+       struct mtd_info *mtd;
+#endif
 } flash_info_t;
 
 extern flash_info_t flash_info[]; /* info for FLASH chips      */

include/linux/mtd/mtd.h +3 -0

index e3d3fc7..0ab6128 100644
@@ -18,6 +18,7 @@
 
 #include <asm/div64.h>
 #else
+#include <dm.h>
 #include <linux/compat.h>
 #include <mtd/mtd-abi.h>
 #include <asm/errno.h>

@@ -272,6 +273,8 @@ struct mtd_info {
        struct module *owner;
 #ifndef __UBOOT__
        struct device dev;
+#else
+       struct udevice *dev;
 #endif
        int usecount;
 };

include/mtd.h new file +23 -1

new file mode 100644
index 0000000..3f8c293
@@ -0,0 +1,23 @@
+/*
+ * Copyright (C) 2015 Thomas Chou <[email protected]>
+ *
+ * SPDX-License-Identifier:    GPL-2.0+
+ */
+
+#ifndef _MTD_H_
+#define _MTD_H_
+
+#include <linux/mtd/mtd.h>
+
+/*
+ * Get mtd_info structure of the dev, which is stored as uclass private.
+ *
+ * @dev: The MTD device
+ * @return: pointer to mtd_info, NULL on error
+ */
+static inline struct mtd_info *mtd_get_info(struct udevice *dev)
+{
+       return dev_get_uclass_priv(dev);
+}
+
+#endif /* _MTD_H_ */
--

2.5.0

_______________________________________________
U-Boot mailing list
U-Boot at lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Hi Thomas,

Quoted 4 lines · Thomas Chou

Implement a Memory Technology Device (MTD) uclass. It should include most flash drivers in the future. Though no uclass ops are defined yet, the MTD ops could be used.

Quoted 3 lines

The NAND flash driver is based on MTD. The CFI flash and SPI flash support MTD, too. It should make sense to convert them to MTD uclass.

Why does MTD require driver model? Should drivers like nand, cfi or etc register mtd core should need to move on dm?

Hi Jagan,

Quoted 14 lines · Jagan Teki

Hi Thomas,

Quoted 8 lines · Thomas Chou

Implement a Memory Technology Device (MTD) uclass. It should include most flash drivers in the future. Though no uclass ops are defined yet, the MTD ops could be used.

The NAND flash driver is based on MTD. The CFI flash and SPI flash support MTD, too. It should make sense to convert them to MTD uclass.

Quoted 2 lines

Why does MTD require driver model? Should drivers like nand, cfi or etc register mtd core should need to move on dm?

The driver model combined with device tree control of u-boot offers dynamic binding of drivers and devices. It is expected that all drivers will be converted to driver model, including nand, cfi and spi flash.

Quoted 17 lines · Thomas Chou

Hi Jagan,

Quoted 14 lines · Jagan Teki

Hi Thomas,

Quoted 8 lines · Thomas Chou

Implement a Memory Technology Device (MTD) uclass. It should include most flash drivers in the future. Though no uclass ops are defined yet, the MTD ops could be used.

The NAND flash driver is based on MTD. The CFI flash and SPI flash support MTD, too. It should make sense to convert them to MTD uclass.

Quoted 7 lines

Quoted 2 lines

Why does MTD require driver model? Should drivers like nand, cfi or etc register mtd core should need to move on dm?

The driver model combined with device tree control of u-boot offers dynamic binding of drivers and devices. It is expected that all drivers will be converted to driver model, including nand, cfi and spi flash.

So, mtd_info ops like _erase, _write and _read will also change or something like this

struct dm_mtd_info {
    struct mtd_info *info;
    struct udevice *dev;
};

Quoted 17 lines · Thomas Chou

Hi Jagan,

Quoted 14 lines · Jagan Teki

Hi Thomas,

Quoted 8 lines · Thomas Chou

Implement a Memory Technology Device (MTD) uclass. It should include most flash drivers in the future. Though no uclass ops are defined yet, the MTD ops could be used.

The NAND flash driver is based on MTD. The CFI flash and SPI flash support MTD, too. It should make sense to convert them to MTD uclass.

Quoted 7 lines

Quoted 2 lines

Why does MTD require driver model? Should drivers like nand, cfi or etc register mtd core should need to move on dm?

The driver model combined with device tree control of u-boot offers dynamic binding of drivers and devices. It is expected that all drivers will be converted to driver model, including nand, cfi and spi flash.

So, mtd_info ops like _erase, _write and _read will also change or something like this

struct dm_mtd_info {
     struct mtd_info *info;
     struct udevice *dev;
};

See for example, I have recently added MTD support to spi_flash [1] [2]

[1] https://patchwork.ozlabs.org/patch/529397/
[2] https://patchwork.ozlabs.org/patch/529399/

Quoted 34 lines · Jagan Teki

Quoted 26 lines · Thomas Chou

Hi Jagan,

Quoted 11 lines · Jagan Teki

Hi Thomas,

Quoted 6 lines · Thomas Chou

Implement a Memory Technology Device (MTD) uclass. It should include most flash drivers in the future. Though no uclass ops are defined yet, the MTD ops could be used.

Quoted 8 lines

Quoted 3 lines

The NAND flash driver is based on MTD. The CFI flash and SPI flash support MTD, too. It should make sense to convert them to MTD uclass.

Why does MTD require driver model? Should drivers like nand, cfi or etc register mtd core should need to move on dm?

Quoted 4 lines

The driver model combined with device tree control of u-boot offers dynamic
binding of drivers and devices. It is expected that all drivers will be converted to driver model, including nand, cfi and spi flash.

Quoted 7 lines

So, mtd_info ops like _erase, _write and _read will also change or something like this

struct dm_mtd_info {
     struct mtd_info *info;
     struct udevice *dev;
};

Not exactly. I included udevice in mtd_info as it was device for Linux.

include/linux/mtd/mtd.h +2 -0

@@ -272,6 +273,8 @@ struct mtd_info {
        struct module *owner;
 #ifndef __UBOOT__
        struct device dev;
+#else
+       struct udevice *dev;
 #endif
        int usecount;
 };

I think the mtd ops is more complete and widely used. There might be no need to reinvent the dm_mtd ops. The mtd uclass priv is set to mtd_info and we can get it with mtd_get_info(dev). Then call mtd ops, like mtd_read() mtd_write and mtd_erase(), directly.

Quoted 4 lines

See for example, I have recently added MTD support to spi_flash [1] [2]

[1] https://patchwork.ozlabs.org/patch/529397/
[2] https://patchwork.ozlabs.org/patch/529399/

It seems we are working toward the same direction. :)

Simon suggested that we can have an unified flash class (for all cfi, spi and nand flash) after the discussion between Bin Meng and I. So I dropped the earlier cfi-flash uclass, and found the mtd might be a better uclass. We see the same point, "MTD has proven core for flash operations".

The work on cfi-flash is not complete yet. It needs to reshape to use mtd ops like your earlier patches. But I have to work on others.

The spi-flash uclass should be merged into mtd uclass and use mtd ops. Maybe you will be interested and will help. Thanks in advance.

The nand flash is more ready. But need to convert to driver model.

Quoted 1 line

It seems we are working toward the same direction. :)

Sorry, I couldn't understand this looks we're in different direction.

Let me explain what I thought about mtd_info usage. udevice should be part of underlying flash structure's like cfi, nand and spi_flash and mtd_info should be used for it's core api's like _erase. _read and _write and underlying driver will use their global structure that include's mtd and udevice as a function pointer like this.

struct spi_flash {
    struct mtd_info *info;
    struct udevice *device;
}

struct spi_flash_priv {
         struct spi_flash        flash;
         struct mtd_info         mtd;
};

static int spi_flash_std_probe(struct udevice *dev)

{
         struct spi_flash_priv *priv = dev_get_uclass_priv(dev);
         struct spi_slave *spi = dev_get_parent_priv(dev);
         struct spi_flash *flash;
         int ret;

         flash = &priv->flash;
         flash->mtd = &priv->mtd;

         flash->spi = spi;
         flash->priv = priv;

         priv->mtd.priv = flash;
         flash->dev = dev;

}

U_BOOT_DRIVER(spi_flash_std) = {

         .name           = "spi_flash_std",
         .id             = UCLASS_SPI_FLASH,
         .of_match       = spi_flash_std_ids,
         .probe          = spi_flash_std_probe,

.priv_auto_alloc_size = sizeof(struct spi_flash_priv),
};

This is the way I have implemented mtd on spi-flash[1] [2]
[1] https://patchwork.ozlabs.org/patch/529397/
[2] https://patchwork.ozlabs.org/patch/529399/

Please explain how this related your approach of adding udevice to mtd.

Implement a Memory Technology Device (MTD) uclass. It should include most flash drivers in the future. Though no uclass ops are defined yet, the MTD ops could be used.

The NAND flash driver is based on MTD. The CFI flash and SPI flash support MTD, too. It should make sense to convert them to MTD uclass.

Signed-off-by Thomas Chou <[email protected]>

---
v3
change to MTD uclass.
v4
add mtd_info to flash_info in flash.h.

 drivers/mtd/Kconfig      | 12 ++++++++++++
 drivers/mtd/Makefile     |  1 +
 drivers/mtd/mtd-uclass.c | 20 ++++++++++++++++++++
 include/dm/uclass-id.h   |  1 +
 include/flash.h          |  3 +++
 include/linux/mtd/mtd.h  |  3 +++
 include/mtd.h            | 23 +++++++++++++++++++++++
 7 files changed, 63 insertions(+)
 create mode 100644 drivers/mtd/mtd-uclass.c
 create mode 100644 include/mtd.h

drivers/mtd/Kconfig +12 -0

index 59278d1..23dff48 100644
@@ -1,3 +1,15 @@
+menu "MTD Support"
+
+config MTD
+       bool "Enable Driver Model for MTD drivers"
+       depends on DM
+       help
+         Enable driver model for Memory Technology Devices (MTD), such as
+         flash, RAM and similar chips, often used for solid state file
+         systems on embedded devices.
+
+endmenu
+
 source "drivers/mtd/nand/Kconfig"
 
 source "drivers/mtd/spi/Kconfig"

drivers/mtd/Makefile +1 -0

index a623f4c..c23c0c1 100644
@@ -8,6 +8,7 @@
 ifneq (,$(findstring y,$(CONFIG_MTD_DEVICE)$(CONFIG_CMD_NAND)$(CONFIG_CMD_ONENAND)$(CONFIG_CMD_SF)))
 obj-y += mtdcore.o mtd_uboot.o
 endif
+obj-$(CONFIG_MTD) += mtd-uclass.o
 obj-$(CONFIG_MTD_PARTITIONS) += mtdpart.o
 obj-$(CONFIG_MTD_CONCAT) += mtdconcat.o
 obj-$(CONFIG_HAS_DATAFLASH) += at45.o

drivers/mtd/mtd-uclass.c new file +20 -0

new file mode 100644
index 0000000..8bd3e6b
@@ -0,0 +1,20 @@
+/*
+ * Copyright (C) 2015 Thomas Chou <[email protected]>
+ *
+ * SPDX-License-Identifier:    GPL-2.0+
+ */
+
+#include <common.h>
+#include <dm.h>
+#include <errno.h>
+#include <mtd.h>
+
+/*
+ * Implement a MTD uclass which should include most flash drivers.
+ * The uclass private is pointed to mtd_info.
+ */
+
+UCLASS_DRIVER(mtd) = {
+       .id             = UCLASS_MTD,
+       .name           = "mtd",
+};

include/dm/uclass-id.h +1 -0

index 886a44c..fcc9784 100644
@@ -42,6 +42,7 @@ enum uclass_id {
        UCLASS_MISC,            /* Miscellaneous device */
        UCLASS_MMC,             /* SD / MMC card or chip */
        UCLASS_MOD_EXP,         /* RSA Mod Exp device */
+       UCLASS_MTD,             /* Memory Technology Device (MTD) device */
        UCLASS_PCH,             /* x86 platform controller hub */
        UCLASS_PCI,             /* PCI bus */
        UCLASS_PCI_GENERIC,     /* Generic PCI bus device */

include/flash.h +3 -0

index dc0645e..f53ace7 100644
@@ -44,6 +44,9 @@ typedef struct {
        ulong   addr_unlock2;           /* unlock address 2 for AMD flash roms  */
        const char *name;               /* human-readable name                  */
 #endif
+#ifdef CONFIG_MTD
+       struct mtd_info *mtd;
+#endif
 } flash_info_t;
 
 extern flash_info_t flash_info[]; /* info for FLASH chips      */

include/linux/mtd/mtd.h +0 -0

index e3d3fc7..0ab6128 100644

include/linux/mtd/mtd.h +2 -0

 #include <linux/compat.h>
 #include <mtd/mtd-abi.h>
 #include <asm/errno.h>

@@ -272,6 +273,8 @@ struct mtd_info {
        struct module *owner;
 #ifndef __UBOOT__
        struct device dev;
+#else
+       struct udevice *dev;
 #endif
        int usecount;
 };

include/mtd.h new file +11 -0

new file mode 100644
index 0000000..3f8c293
@@ -0,0 +1,23 @@
+/*
+ * Copyright (C) 2015 Thomas Chou <[email protected]>
+ *
+ * SPDX-License-Identifier:    GPL-2.0+
+ */
+
+#ifndef _MTD_H_
+#define _MTD_H_
+
+#include <linux/mtd/mtd.h>
+

include/mtd.h new file +2 -1

+
+#endif /* _MTD_H_ */
--

2.5.0

Quoted 6 lines

include/linux/mtd/mtd.h +0 -0

index e3d3fc7..0ab6128 100644
@@ -18,6 +18,7 @@
 

include/linux/mtd/mtd.h line 21

Quoted 3 lines

include/linux/mtd/mtd.h +1 -0

  #include <asm/div64.h>
  #else
+#include <dm.h>

I'm not keen on adding this header here. Why is it needed? Can we instead include <m.h> in the C files that need it?

include/linux/mtd/mtd.h struct mtd_info { · line 276

Quoted 8 lines

include/linux/mtd/mtd.h +1 -0

  #include <linux/compat.h>
  #include <mtd/mtd-abi.h>
  #include <asm/errno.h>

@@ -272,6 +273,8 @@ struct mtd_info {
         struct module *owner;
  #ifndef __UBOOT__
         struct device dev;
+#else

include/linux/mtd/mtd.h struct mtd_info { · line 280

Quoted 4 lines

include/linux/mtd/mtd.h +1 -0

+       struct udevice *dev;
  #endif
         int usecount;
  };

Hi Simon,

Quoted 7 lines · Simon Glass

Quoted 7 lines

include/linux/mtd/mtd.h +0 -0

index e3d3fc7..0ab6128 100644
@@ -18,6 +18,7 @@
 

Quoted 8 lines

include/linux/mtd/mtd.h line 21

Quoted 3 lines

include/linux/mtd/mtd.h +1 -0

  #include <asm/div64.h>
  #else
+#include <dm.h>

I'm not keen on adding this header here. Why is it needed? Can we instead include <m.h> in the C files that need it?

It is needed for the udevice in mtd_info. Some drivers use mtd.h but are not converted to driver model. Maybe I should remove the dm here, and add an #elif for the udevice below?

Quoted 9 lines

include/linux/mtd/mtd.h struct mtd_info { · line 276

Quoted 9 lines

include/linux/mtd/mtd.h +1 -0

  #include <linux/compat.h>
  #include <mtd/mtd-abi.h>
  #include <asm/errno.h>

@@ -272,6 +273,8 @@ struct mtd_info {
         struct module *owner;
  #ifndef __UBOOT__
         struct device dev;
+#else

#elif CONFIG_IS_ENABLED(MTD)

Quoted 4 lines

include/linux/mtd/mtd.h struct mtd_info { · line 280

Quoted 4 lines

include/linux/mtd/mtd.h +1 -0

+       struct udevice *dev;
  #endif
         int usecount;
  };

Thanks a lot for your review.

Best regards,
Thomas

Convert cfi flash to driver model.

Signed-off-by Thomas Chou <[email protected]>

---
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.
v5
use uclass priv mtd_info instead of cfi_mtd_info[].

 doc/device-tree-bindings/mtd/mtd-physmap.txt | 88 ++++++++++++++++++++++++++++
 drivers/mtd/Kconfig                          | 11 ++++
 drivers/mtd/cfi_flash.c                      | 78 ++++++++++++++++++++++++
 3 files changed, 177 insertions(+)
 create mode 100644 doc/device-tree-bindings/mtd/mtd-physmap.txt

Convert cfi flash to driver model.

Signed-off-by Thomas Chou <[email protected]>

---
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.
v5
use uclass priv mtd_info instead of cfi_mtd_info[].

 doc/device-tree-bindings/mtd/mtd-physmap.txt | 88 ++++++++++++++++++++++++++++
 drivers/mtd/Kconfig                          | 11 ++++
 drivers/mtd/cfi_flash.c                      | 78 ++++++++++++++++++++++++
 3 files changed, 177 insertions(+)
 create mode 100644 doc/device-tree-bindings/mtd/mtd-physmap.txt

doc/device-tree-bindings/mtd/mtd-physmap.txt new file +88 -0

new file mode 100644
index 0000000..4b8c489
@@ -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>;
+       };

drivers/mtd/Kconfig +11 -0

index 23dff48..57e4b86 100644
@@ -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. 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"

drivers/mtd/cfi_flash.c +12 -0

index fc7a878..e3cb598 100644
@@ -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;
+       /*

Quoted 14 lines

drivers/mtd/cfi_flash.c +14 -0

+#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;

drivers/mtd/cfi_flash.c static u16 cfi_flash_config_reg(int i) · line 111

Quoted 3 lines

drivers/mtd/cfi_flash.c +3 -0

+            uclass_next_device(&dev)) {
+       }
+}

I think this is for probing MTD_UCLASS drivers is it? for my understanding MTD should be generic to all the flash variants if so this probing shouldn't be CFI specific or If MTD uclass is specific to CFI this implementation is correct. Can you comment which one is true.

Hi Jagan,

Quoted 16 lines · Jagan Teki

Quoted 16 lines

drivers/mtd/cfi_flash.c +15 -0

+#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)) {

Quoted 8 lines

Review

Quoted 2 lines

(quoted diff) +2 -0

+       }
+}

I think this is for probing MTD_UCLASS drivers is it? for my understanding MTD should be generic to all the flash variants if so this probing shouldn't be CFI specific or If MTD uclass is specific to CFI this implementation is correct. Can you comment which one is true.

This probing is not specific to CFI flash. It will probe all flash variants of MTD uclass.

Quoted 2 lines

This probing is not specific to CFI flash. It will probe all flash variants of MTD uclass.

This is something I'm unclear, suppose if I need to use MTD uclass in spi-flash drivers shall I enable CONFIG_CFI_FLASH? If so that depends on CONFIG_CFI_FLASH_DRIVER as well right? then it is not correct to enable CFI_FLASH_DRIVER for other flash variants like spi-flash right?

Hi Jagan,

Quoted 10 lines · Jagan Teki

Quoted 4 lines

This probing is not specific to CFI flash. It will probe all flash variants
of MTD uclass.

This is something I'm unclear, suppose if I need to use MTD uclass in spi-flash drivers shall I enable CONFIG_CFI_FLASH? If so that depends on CONFIG_CFI_FLASH_DRIVER as well right? then it is not correct to enable CFI_FLASH_DRIVER for other flash variants like spi-flash right?

There is no need to select CFI_FLASH if there is no CFI flash. The probing here is only because we want to display the size of parallel flash in initr_flash() of board_r.c.

Normally, devices are probed when they are used. You should be able to use "sf probe" for MTD uclass in the same way you do for SPI_FLASH uclass.

Quoted 15 lines · Thomas Chou

Hi Jagan,

Quoted 10 lines · Jagan Teki

Quoted 4 lines

This probing is not specific to CFI flash. It will probe all flash variants
of MTD uclass.

This is something I'm unclear, suppose if I need to use MTD uclass in spi-flash drivers shall I enable CONFIG_CFI_FLASH? If so that depends on CONFIG_CFI_FLASH_DRIVER as well right? then it is not correct to enable CFI_FLASH_DRIVER for other flash variants like spi-flash right?

Quoted 3 lines

There is no need to select CFI_FLASH if there is no CFI flash. The probing here is only because we want to display the size of parallel flash in initr_flash() of board_r.c.

So, this is the way drivers/mtd/altera_qspi.c getting probed?

Quoted 4 lines

Normally, devices are probed when they are used. You should be able to use "sf probe" for MTD uclass in the same way you do for SPI_FLASH uclass.

True, suppose if I wanted to probe spi-flash MTD uclass drivers w/o using "commands" like sf probe then I need to implement dm_scan node logic on mtd-uclass.c correct?