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

[Jagan Teki <jteki@openedev.com>]

Hi Thomas,

On 7 November 2015 at 13:27, 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.
> 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
>
> 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..57e4b86 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. 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..e3cb598 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)) {
> +       }
> +}

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.

thanks!
-- 
Jagan.