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

[Stefan Roese <sr@denx.de>]

Hi Thomas,

On 30.10.2015 14:33, Thomas Chou 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.
> 
>   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
> 
> 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
> +	  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..a439d8b 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,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)
>   {
> @@ -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.

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

Thanks,
Stefan