[U-Boot] [PATCH] dm: implement a cfi flash uclass

[U-Boot] [PATCH] dm: implement a cfi flash uclass

[Thomas Chou]

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 <thomas@wytron.com.tw>
---
 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

diff --git a/common/board_r.c b/common/board_r.c
index aaf390e..86b606d 100644
--- a/common/board_r.c
+++ b/common/board_r.c
@@ -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;
diff --git a/drivers/mtd/Kconfig b/drivers/mtd/Kconfig
index 59278d1..4b52894 100644
--- a/drivers/mtd/Kconfig
+++ b/drivers/mtd/Kconfig
@@ -1,3 +1,14 @@
+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.
+
+endmenu
+
 source "drivers/mtd/nand/Kconfig"
 
 source "drivers/mtd/spi/Kconfig"
diff --git a/drivers/mtd/Makefile b/drivers/mtd/Makefile
index a623f4c..d86e2c2 100644
--- a/drivers/mtd/Makefile
+++ b/drivers/mtd/Makefile
@@ -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
diff --git a/drivers/mtd/cfi-flash-uclass.c b/drivers/mtd/cfi-flash-uclass.c
new file mode 100644
index 0000000..1db7e2f
--- /dev/null
+++ b/drivers/mtd/cfi-flash-uclass.c
@@ -0,0 +1,70 @@
+/*
+ * Copyright (C) 2015 Thomas Chou <thomas@wytron.com.tw>
+ *
+ * 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;
+}
+
+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;
+};
+
+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);
+	fdt_size_t size;
+
+	fdtdec_get_addr_size(gd->fdt_blob, dev->of_offset, "reg", &size);
+	plat->base = (phys_addr_t)ioremap(dev_get_addr(dev), size);
+
+	return 0;
+}
+
+static const struct udevice_id cfi_flash_ids[] = {
+	{ .compatible = "cfi-flash", },
+	{ }
+};
+
+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,
+};
diff --git a/drivers/mtd/cfi_flash.c b/drivers/mtd/cfi_flash.c
index 50983b8..6ec0877 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 <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
+phys_addr_t cfi_flash_bank_addr(int i)
+{
+	struct udevice *dev;
+	int ret;
+
+	ret = uclass_get_device(UCLASS_CFI_FLASH, i, &dev);
+	if (ret)
+		return ret;
+	if (!dev)
+		return -ENODEV;
+
+	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)
 {
diff --git a/include/cfi-flash.h b/include/cfi-flash.h
new file mode 100644
index 0000000..0064cba
--- /dev/null
+++ b/include/cfi-flash.h
@@ -0,0 +1,27 @@
+/*
+ * Copyright (C) 2015 Thomas Chou <thomas@wytron.com.tw>
+ *
+ * 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_ */
diff --git a/include/dm/uclass-id.h b/include/dm/uclass-id.h
index a6982ab..09e370c 100644
--- a/include/dm/uclass-id.h
+++ b/include/dm/uclass-id.h
@@ -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

[U-Boot] [PATCH] dm: implement a cfi flash uclass

[Simon Glass]

Hi Thomas,

On 8 October 2015 at 08:34, Thomas Chou <thomas@wytron.com.tw> wrote:
> 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 <thomas@wytron.com.tw>
> ---
>  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
>

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

> diff --git a/common/board_r.c b/common/board_r.c
> index aaf390e..86b606d 100644
> --- a/common/board_r.c
> +++ b/common/board_r.c
> @@ -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

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.

>         /* size of FLASH memory (final value) */
>         bd->bi_flashsize = flash_size;
> diff --git a/drivers/mtd/Kconfig b/drivers/mtd/Kconfig
> index 59278d1..4b52894 100644
> --- a/drivers/mtd/Kconfig
> +++ b/drivers/mtd/Kconfig
> @@ -1,3 +1,14 @@
> +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?

> +
> +endmenu
> +
>  source "drivers/mtd/nand/Kconfig"
>
>  source "drivers/mtd/spi/Kconfig"
> diff --git a/drivers/mtd/Makefile b/drivers/mtd/Makefile
> index a623f4c..d86e2c2 100644
> --- a/drivers/mtd/Makefile
> +++ b/drivers/mtd/Makefile
> @@ -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
> diff --git a/drivers/mtd/cfi-flash-uclass.c b/drivers/mtd/cfi-flash-uclass.c
> new file mode 100644
> index 0000000..1db7e2f
> --- /dev/null
> +++ b/drivers/mtd/cfi-flash-uclass.c
> @@ -0,0 +1,70 @@
> +/*
> + * Copyright (C) 2015 Thomas Chou <thomas@wytron.com.tw>
> + *
> + * 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;
> +}
> +
> +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?

> +
> +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);
> +       fdt_size_t size;
> +
> +       fdtdec_get_addr_size(gd->fdt_blob, dev->of_offset, "reg", &size);
> +       plat->base = (phys_addr_t)ioremap(dev_get_addr(dev), size);
> +
> +       return 0;
> +}
> +
> +static const struct udevice_id cfi_flash_ids[] = {
> +       { .compatible = "cfi-flash", },

Is there a binding somewhere for this?

> +       { }
> +};
> +
> +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,
> +};
> diff --git a/drivers/mtd/cfi_flash.c b/drivers/mtd/cfi_flash.c
> index 50983b8..6ec0877 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 <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
> +phys_addr_t cfi_flash_bank_addr(int i)
> +{
> +       struct udevice *dev;
> +       int ret;
> +
> +       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()?

> +
> +       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)
>  {
> diff --git a/include/cfi-flash.h b/include/cfi-flash.h
> new file mode 100644
> index 0000000..0064cba
> --- /dev/null
> +++ b/include/cfi-flash.h
> @@ -0,0 +1,27 @@
> +/*
> + * Copyright (C) 2015 Thomas Chou <thomas@wytron.com.tw>
> + *
> + * 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_ */
> diff --git a/include/dm/uclass-id.h b/include/dm/uclass-id.h
> index a6982ab..09e370c 100644
> --- a/include/dm/uclass-id.h
> +++ b/include/dm/uclass-id.h
> @@ -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
>

Regards,
Simon

[U-Boot] [PATCH] dm: implement a cfi flash uclass

[Thomas Chou]

Hi Simon,

On 10/09/2015 05:36 PM, Simon Glass wrote:
> Can you create a sandbox driver for this so you can add a test?

Yes. I will add a sandbox driver and test later.

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

I will add this to the TODO list. Though the flash is probed to find the 
size and print message here in initr_flash. It is almost the same time 
for use.

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

Yes, I will do.

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

I will put it to the top, as I did it at the very early version.

> Is there a binding somewhere for this?

A dts binding of cfi-flash will be added.

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

Best regards,
Thomas

[U-Boot] [PATCH] dm: implement a cfi flash uclass

[Thomas Chou]

Hi Simon,

On 10/09/2015 09:31 PM, Thomas Chou wrote:
>>> +       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.

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.

Best regards,
Thomas

[U-Boot] [PATCH] dm: implement a cfi flash uclass

[Bin Meng]

Hi Thomas,

On Sun, Oct 11, 2015 at 1:16 PM, Thomas Chou <thomas@wytron.com.tw> wrote:
> Hi Simon,
>
> On 10/09/2015 09:31 PM, Thomas Chou wrote:
>>>>
>>>> +       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.
>
>
> 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.

Regards,
Bin

[U-Boot] [PATCH] dm: implement a cfi flash uclass

[Thomas Chou]

Hi Bin,

On 10/11/2015 01:41 PM, Bin Meng wrote:
> You can check fdtdec_get_pci_addr() to see how multiple "reg" tuples
> are decoded.

I see. So I will need to decode the cells.

cell = fdt_getprop(blob, node, prop_name, &len);

Thanks a lot for your help.

Best regards,
Thomas

[U-Boot] [PATCH v2] dm: implement a cfi flash uclass

[Thomas Chou]

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 <thomas@wytron.com.tw>
---
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

diff --git a/common/board_r.c b/common/board_r.c
index a4facf8..fceaea6 100644
--- a/common/board_r.c
+++ b/common/board_r.c
@@ -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;
diff --git a/configs/nios2-generic_defconfig b/configs/nios2-generic_defconfig
index 3d404b2..7b504ba 100644
--- a/configs/nios2-generic_defconfig
+++ b/configs/nios2-generic_defconfig
@@ -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
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..a04d6e8
--- /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 59278d1..b1addb5 100644
--- a/drivers/mtd/Kconfig
+++ b/drivers/mtd/Kconfig
@@ -1,3 +1,19 @@
+menu "CFI Flash Support"
+
+config CFI_FLASH
+	bool "Enable Driver Model for CFI Flash driver"
+	depends on DM
+	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. This driver uses the same API as
+	  drivers/mtd/cfi_flash.c. But now implemented by the uclass.
+
+endmenu
+
 source "drivers/mtd/nand/Kconfig"
 
 source "drivers/mtd/spi/Kconfig"
diff --git a/drivers/mtd/Makefile b/drivers/mtd/Makefile
index a623f4c..0bcc76c 100644
--- a/drivers/mtd/Makefile
+++ b/drivers/mtd/Makefile
@@ -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_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
diff --git a/drivers/mtd/cfi-flash-uclass.c b/drivers/mtd/cfi-flash-uclass.c
new file mode 100644
index 0000000..62c914b
--- /dev/null
+++ b/drivers/mtd/cfi-flash-uclass.c
@@ -0,0 +1,70 @@
+/*
+ * Copyright (C) 2015 Thomas Chou <thomas@wytron.com.tw>
+ *
+ * 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;
+
+struct cfi_flash_platdata {
+	phys_addr_t base;
+};
+
+/*
+ * 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;
+}
+
+UCLASS_DRIVER(cfi_flash) = {
+	.id		= UCLASS_CFI_FLASH,
+	.name		= "cfi_flash",
+	.per_device_auto_alloc_size = sizeof(struct cfi_flash_dev_priv),
+};
+
+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);
+	fdt_size_t size;
+
+	fdtdec_get_addr_size(gd->fdt_blob, dev->of_offset, "reg", &size);
+	plat->base = (phys_addr_t)ioremap(dev_get_addr(dev), size);
+
+	return 0;
+}
+
+static const struct udevice_id cfi_flash_ids[] = {
+	{ .compatible = "cfi-flash", },
+	{ }
+};
+
+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,
+};
diff --git a/drivers/mtd/cfi_flash.c b/drivers/mtd/cfi_flash.c
index 50983b8..9204596 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 <cfi-flash.h>
 #include <asm/processor.h>
 #include <asm/io.h>
 #include <asm/byteorder.h>
@@ -87,10 +90,25 @@ 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
+phys_addr_t cfi_flash_bank_addr(int i)
+{
+	struct udevice *dev;
+	int ret;
+
+	/* FIXME: the current code supports only one bank */
+	ret = uclass_get_device(UCLASS_CFI_FLASH, i, &dev);
+	if (ret)
+		return ret;
+
+	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)
 {
diff --git a/include/cfi-flash.h b/include/cfi-flash.h
new file mode 100644
index 0000000..192df97
--- /dev/null
+++ b/include/cfi-flash.h
@@ -0,0 +1,27 @@
+/*
+ * Copyright (C) 2015 Thomas Chou <thomas@wytron.com.tw>
+ *
+ * SPDX-License-Identifier:	GPL-2.0+
+ */
+
+#ifndef _CFI_FLASH_H_
+#define _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	/* _CFI_FLASH_H_ */
diff --git a/include/dm/uclass-id.h b/include/dm/uclass-id.h
index a6982ab..09e370c 100644
--- a/include/dm/uclass-id.h
+++ b/include/dm/uclass-id.h
@@ -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

[U-Boot] [PATCH v2] dm: implement a cfi flash uclass

[Bin Meng]

+Simon,

Hi Thomas,

On Sun, Oct 11, 2015 at 3:30 PM, Thomas Chou <thomas@wytron.com.tw> wrote:
> 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 <thomas@wytron.com.tw>
> ---
> 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
>
> diff --git a/common/board_r.c b/common/board_r.c
> index a4facf8..fceaea6 100644
> --- a/common/board_r.c
> +++ b/common/board_r.c
> @@ -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;
> diff --git a/configs/nios2-generic_defconfig b/configs/nios2-generic_defconfig
> index 3d404b2..7b504ba 100644
> --- a/configs/nios2-generic_defconfig
> +++ b/configs/nios2-generic_defconfig
> @@ -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
> 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..a04d6e8
> --- /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";

PC48F4400P0VB should be lower cases

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

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?

[snip]

Regards,
Bin

[U-Boot] [PATCH v2] dm: implement a cfi flash uclass

[Thomas Chou]

Hi Bin,

On 10/11/2015 03:54 PM, Bin Meng wrote:
> 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.

Best regards,
Thomas

[U-Boot] [PATCH v2] dm: implement a cfi flash uclass

[Bin Meng]

Hi Thomas,

On Sun, Oct 11, 2015 at 4:54 PM, Thomas Chou <thomas@wytron.com.tw> wrote:
> Hi Bin,
>
> On 10/11/2015 03:54 PM, Bin Meng wrote:
>>
>> 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.

Regards,
Bin

[U-Boot] [PATCH v2] dm: implement a cfi flash uclass

[Thomas Chou]

Hi Bin,

On 10/11/2015 03:54 PM, Bin Meng wrote:
>> +       flash at f0000000,0 {
>> +               #address-cells = <1>;
>> +               #size-cells = <1>;
>> +               compatible = "intel,PC48F4400P0VB", "cfi-flash";
>
> PC48F4400P0VB should be lower cases

Thanks for your review. I will change it to lower cases.

Best regards,
Thomas

[U-Boot] [PATCH v2] dm: implement a cfi flash uclass

[Thomas Chou]

Hi Bin,

On 10/11/2015 05:10 PM, Bin Meng wrote:
> 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.

Best regards,
Thomas

[U-Boot] [PATCH v2] dm: implement a cfi flash uclass

[Bin Meng]

Hi Thomas,

On Sun, Oct 11, 2015 at 8:24 PM, Thomas Chou <thomas@wytron.com.tw> wrote:
> Hi Bin,
>
> On 10/11/2015 05:10 PM, Bin Meng wrote:
>>
>> 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?

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

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

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

Agreed!

Regards,
Bin

[U-Boot] [PATCH v2] dm: implement a cfi flash uclass

[Thomas Chou]

Hi Bin,

On 10/11/2015 08:43 PM, Bin Meng wrote:
> 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. :)

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

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.

Best regards,
Thomas

[U-Boot] [PATCH v2] dm: implement a cfi flash uclass

[Simon Glass]

Hi,

On 11 October 2015 at 07:29, Thomas Chou <thomas@wytron.com.tw> wrote:
> Hi Bin,
>
> On 10/11/2015 08:43 PM, Bin Meng wrote:
>>
>> 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.

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

[U-Boot] [PATCH v2] dm: implement a cfi flash uclass

[Thomas Chou]

Hi Simon,

On 10/12/2015 05:47 AM, Simon Glass wrote:
> 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.

Great. It has been my first choice to have a unified flash uclass.

Best regards,
Thomas

[U-Boot] [PATCH v2] dm: implement a cfi flash uclass

[Bin Meng]

Hi,

On Mon, Oct 12, 2015 at 5:47 AM, Simon Glass <sjg@chromium.org> wrote:
> Hi,
>
> On 11 October 2015 at 07:29, Thomas Chou <thomas@wytron.com.tw> wrote:
>> Hi Bin,
>>
>> On 10/11/2015 08:43 PM, Bin Meng wrote:
>>>
>>> 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. :)

These single drivers under a uclass is probably due to we have not
converted all other drivers to driver model yet.

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

Yep, that's really my concern regarding to a new 'class' to be created
but actually no additional driver will use that 'class'.

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

Yep, I believe this is a better approach.

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

Regards,
Bin

[U-Boot] [PATCH v3 1/3] dm: implement a MTD uclass

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

Signed-off-by: Thomas Chou <thomas@wytron.com.tw>
---
v3
  change to MTD uclass.

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

diff --git a/drivers/mtd/Kconfig b/drivers/mtd/Kconfig
index 59278d1..23dff48 100644
--- a/drivers/mtd/Kconfig
+++ b/drivers/mtd/Kconfig
@@ -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"
diff --git a/drivers/mtd/Makefile b/drivers/mtd/Makefile
index a623f4c..c23c0c1 100644
--- a/drivers/mtd/Makefile
+++ b/drivers/mtd/Makefile
@@ -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
diff --git a/drivers/mtd/mtd-uclass.c b/drivers/mtd/mtd-uclass.c
new file mode 100644
index 0000000..8bd3e6b
--- /dev/null
+++ b/drivers/mtd/mtd-uclass.c
@@ -0,0 +1,20 @@
+/*
+ * Copyright (C) 2015 Thomas Chou <thomas@wytron.com.tw>
+ *
+ * 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",
+};
diff --git a/include/dm/uclass-id.h b/include/dm/uclass-id.h
index 886a44c..fcc9784 100644
--- a/include/dm/uclass-id.h
+++ b/include/dm/uclass-id.h
@@ -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 */
diff --git a/include/linux/mtd/mtd.h b/include/linux/mtd/mtd.h
index e3d3fc7..3dd13e8 100644
--- a/include/linux/mtd/mtd.h
+++ b/include/linux/mtd/mtd.h
@@ -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 *udev;
 #endif
 	int usecount;
 };
diff --git a/include/mtd.h b/include/mtd.h
new file mode 100644
index 0000000..3f8c293
--- /dev/null
+++ b/include/mtd.h
@@ -0,0 +1,23 @@
+/*
+ * Copyright (C) 2015 Thomas Chou <thomas@wytron.com.tw>
+ *
+ * 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] [PATCH v3 2/3] cfi_flash: convert to driver model

[Thomas Chou]

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;
 
 #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
 	/* Init: no FLASHes known */
 	for (i = 0; i < CONFIG_SYS_MAX_FLASH_BANKS; ++i) {
 		flash_info[i].flash_id = FLASH_UNKNOWN;
@@ -2398,3 +2421,49 @@ unsigned long flash_init (void)
 
 	return (size);
 }
+
+#ifdef CONFIG_CFI_FLASH /* for driver model */
+static int cfi_flash_probe(struct udevice *dev)
+{
+	const void *blob = gd->fdt_blob;
+	int node = dev->of_offset;
+	const fdt32_t *cell;
+	phys_addr_t addr;
+	int addrs, sizes;
+	int len, idx;
+
+	/* decode regs, there may be multiple reg tuples. */
+	addrs = fdt_address_cells(blob, node);
+	sizes = fdt_size_cells(blob, node);
+	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 += addrs + sizes;
+	}
+	gd->bd->bi_flashstart = cfi_flash_base[0];
+#ifdef CONFIG_FLASH_CFI_MTD
+	dev->uclass_priv = &cfi_mtd_info[0];
+	cfi_mtd_info[0].udev = 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

[U-Boot] [PATCH v3 3/3] nios2: use cfi flash driver model

[Thomas Chou]

Use cfi flash driver model.

Signed-off-by: Thomas Chou <thomas@wytron.com.tw>
---
 configs/nios2-generic_defconfig | 2 ++
 include/configs/nios2-generic.h | 3 +--
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/configs/nios2-generic_defconfig b/configs/nios2-generic_defconfig
index e42a15e..0a6de6f 100644
--- a/configs/nios2-generic_defconfig
+++ b/configs/nios2-generic_defconfig
@@ -17,6 +17,8 @@ CONFIG_NET_RANDOM_ETHADDR=y
 CONFIG_ALTERA_PIO=y
 CONFIG_MISC=y
 CONFIG_ALTERA_SYSID=y
+CONFIG_MTD=y
+CONFIG_CFI_FLASH=y
 CONFIG_DM_ETH=y
 CONFIG_ALTERA_TSE=y
 CONFIG_ALTERA_JTAG_UART=y
diff --git a/include/configs/nios2-generic.h b/include/configs/nios2-generic.h
index 84faa4c..144534d 100644
--- a/include/configs/nios2-generic.h
+++ b/include/configs/nios2-generic.h
@@ -24,13 +24,12 @@
 /*
  * CFI Flash
  */
-#define CONFIG_SYS_FLASH_BASE		0xe0000000
 #define CONFIG_FLASH_CFI_DRIVER
 #define CONFIG_SYS_CFI_FLASH_STATUS_POLL /* fix amd flash issue */
 #define CONFIG_SYS_FLASH_CFI
 #define CONFIG_SYS_FLASH_USE_BUFFER_WRITE
 #define CONFIG_SYS_FLASH_PROTECTION
-#define CONFIG_SYS_MAX_FLASH_BANKS	1
+#define CONFIG_SYS_MAX_FLASH_BANKS_DETECT	1
 #define CONFIG_SYS_MAX_FLASH_SECT	512
 
 /*
-- 
2.5.0

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

[Stefan Roese]

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

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

[Thomas Chou]

Hi Stefan,

On 2015?11?02? 16:20, Stefan Roese wrote:
>> @@ -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?

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)) {
	}
}

Thanks a lot.

Best regards,
Thomas

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

[Stefan Roese]

Hi Thomas,

On 03.11.2015 01:23, Thomas Chou wrote:
>>>    #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?
>
> 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.

Thats good, thanks.

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

Thanks,
Stefan

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

[Thomas Chou]

Hi Stefan,

On 2015?11?03? 13:56, Stefan Roese wrote:
>> 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?

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.

Best regards,
Thomas

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

[Stefan Roese]

Hi Thomas,

On 03.11.2015 07:25, Thomas Chou wrote:
> On 2015?11?03? 13:56, Stefan Roese wrote:
>>> 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?
>
> 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.

Okay. So cfi_flash_probe() will get called assigning the base
addresses that are available. The real "probing" (meaning
detection) is happening later in flash_init().

I think adding (or extending) a comment here to describe this
would be helpful in v2.

Thanks,
Stefan

[U-Boot] [PATCH v4 1/3] dm: implement a MTD uclass

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

Signed-off-by: Thomas Chou <thomas@wytron.com.tw>
---
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

diff --git a/drivers/mtd/Kconfig b/drivers/mtd/Kconfig
index 59278d1..23dff48 100644
--- a/drivers/mtd/Kconfig
+++ b/drivers/mtd/Kconfig
@@ -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"
diff --git a/drivers/mtd/Makefile b/drivers/mtd/Makefile
index a623f4c..c23c0c1 100644
--- a/drivers/mtd/Makefile
+++ b/drivers/mtd/Makefile
@@ -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
diff --git a/drivers/mtd/mtd-uclass.c b/drivers/mtd/mtd-uclass.c
new file mode 100644
index 0000000..8bd3e6b
--- /dev/null
+++ b/drivers/mtd/mtd-uclass.c
@@ -0,0 +1,20 @@
+/*
+ * Copyright (C) 2015 Thomas Chou <thomas@wytron.com.tw>
+ *
+ * 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",
+};
diff --git a/include/dm/uclass-id.h b/include/dm/uclass-id.h
index 886a44c..fcc9784 100644
--- a/include/dm/uclass-id.h
+++ b/include/dm/uclass-id.h
@@ -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 */
diff --git a/include/flash.h b/include/flash.h
index dc0645e..f53ace7 100644
--- a/include/flash.h
+++ b/include/flash.h
@@ -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	*/
diff --git a/include/linux/mtd/mtd.h b/include/linux/mtd/mtd.h
index e3d3fc7..0ab6128 100644
--- a/include/linux/mtd/mtd.h
+++ b/include/linux/mtd/mtd.h
@@ -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;
 };
diff --git a/include/mtd.h b/include/mtd.h
new file mode 100644
index 0000000..3f8c293
--- /dev/null
+++ b/include/mtd.h
@@ -0,0 +1,23 @@
+/*
+ * Copyright (C) 2015 Thomas Chou <thomas@wytron.com.tw>
+ *
+ * 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] [PATCH v4 2/3] cfi_flash: convert to driver model

[Thomas Chou]

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
+	  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)) {
+	}
+}
+
+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];
+	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

[U-Boot] [PATCH v4 3/3] nios2: use cfi flash driver model

[Thomas Chou]

Use cfi flash driver model.

Signed-off-by: Thomas Chou <thomas@wytron.com.tw>
---
 configs/nios2-generic_defconfig | 2 ++
 include/configs/nios2-generic.h | 3 +--
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/configs/nios2-generic_defconfig b/configs/nios2-generic_defconfig
index e42a15e..0a6de6f 100644
--- a/configs/nios2-generic_defconfig
+++ b/configs/nios2-generic_defconfig
@@ -17,6 +17,8 @@ CONFIG_NET_RANDOM_ETHADDR=y
 CONFIG_ALTERA_PIO=y
 CONFIG_MISC=y
 CONFIG_ALTERA_SYSID=y
+CONFIG_MTD=y
+CONFIG_CFI_FLASH=y
 CONFIG_DM_ETH=y
 CONFIG_ALTERA_TSE=y
 CONFIG_ALTERA_JTAG_UART=y
diff --git a/include/configs/nios2-generic.h b/include/configs/nios2-generic.h
index 3a559f1..6e426d6 100644
--- a/include/configs/nios2-generic.h
+++ b/include/configs/nios2-generic.h
@@ -24,13 +24,12 @@
 /*
  * CFI Flash
  */
-#define CONFIG_SYS_FLASH_BASE		0xe0000000
 #define CONFIG_FLASH_CFI_DRIVER
 #define CONFIG_SYS_CFI_FLASH_STATUS_POLL /* fix amd flash issue */
 #define CONFIG_SYS_FLASH_CFI
 #define CONFIG_SYS_FLASH_USE_BUFFER_WRITE
 #define CONFIG_SYS_FLASH_PROTECTION
-#define CONFIG_SYS_MAX_FLASH_BANKS	1
+#define CONFIG_SYS_MAX_FLASH_BANKS_DETECT	1
 #define CONFIG_SYS_MAX_FLASH_SECT	512
 
 /*
-- 
2.5.0

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

[Stefan Roese]

On 03.11.2015 14:09, 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.
> 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.

Thanks,
Stefan

[U-Boot] [PATCH v4 1/3] dm: implement a MTD uclass

[Jagan Teki]

Hi Thomas,

On 3 November 2015 at 18:39, Thomas Chou <thomas@wytron.com.tw> wrote:
> 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.

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

>
> Signed-off-by: Thomas Chou <thomas@wytron.com.tw>
> ---
> 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
>
> diff --git a/drivers/mtd/Kconfig b/drivers/mtd/Kconfig
> index 59278d1..23dff48 100644
> --- a/drivers/mtd/Kconfig
> +++ b/drivers/mtd/Kconfig
> @@ -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"
> diff --git a/drivers/mtd/Makefile b/drivers/mtd/Makefile
> index a623f4c..c23c0c1 100644
> --- a/drivers/mtd/Makefile
> +++ b/drivers/mtd/Makefile
> @@ -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
> diff --git a/drivers/mtd/mtd-uclass.c b/drivers/mtd/mtd-uclass.c
> new file mode 100644
> index 0000000..8bd3e6b
> --- /dev/null
> +++ b/drivers/mtd/mtd-uclass.c
> @@ -0,0 +1,20 @@
> +/*
> + * Copyright (C) 2015 Thomas Chou <thomas@wytron.com.tw>
> + *
> + * 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",
> +};
> diff --git a/include/dm/uclass-id.h b/include/dm/uclass-id.h
> index 886a44c..fcc9784 100644
> --- a/include/dm/uclass-id.h
> +++ b/include/dm/uclass-id.h
> @@ -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 */
> diff --git a/include/flash.h b/include/flash.h
> index dc0645e..f53ace7 100644
> --- a/include/flash.h
> +++ b/include/flash.h
> @@ -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      */
> diff --git a/include/linux/mtd/mtd.h b/include/linux/mtd/mtd.h
> index e3d3fc7..0ab6128 100644
> --- a/include/linux/mtd/mtd.h
> +++ b/include/linux/mtd/mtd.h
> @@ -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;
>  };
> diff --git a/include/mtd.h b/include/mtd.h
> new file mode 100644
> index 0000000..3f8c293
> --- /dev/null
> +++ b/include/mtd.h
> @@ -0,0 +1,23 @@
> +/*
> + * Copyright (C) 2015 Thomas Chou <thomas@wytron.com.tw>
> + *
> + * 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



-- 
Jagan | openedev.

[U-Boot] [PATCH v4 1/3] dm: implement a MTD uclass

[Thomas Chou]

Hi Jagan,

On 2015?11?03? 22:41, Jagan Teki wrote:
> Hi Thomas,
>
> On 3 November 2015 at 18:39, Thomas Chou <thomas@wytron.com.tw> wrote:
>> 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.
>
> 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.

Best regards,
Thomas

[U-Boot] [PATCH v4 1/3] dm: implement a MTD uclass

[Jagan Teki]

On 3 November 2015 at 20:19, Thomas Chou <thomas@wytron.com.tw> wrote:
> Hi Jagan,
>
> On 2015?11?03? 22:41, Jagan Teki wrote:
>>
>> Hi Thomas,
>>
>> On 3 November 2015 at 18:39, Thomas Chou <thomas@wytron.com.tw> wrote:
>>>
>>> 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.
>>
>>
>> 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;
};


thanks!
-- 
Jagan | openedev.

[U-Boot] [PATCH v4 1/3] dm: implement a MTD uclass

[Jagan Teki]

On 3 November 2015 at 20:25, Jagan Teki <jteki@openedev.com> wrote:
> On 3 November 2015 at 20:19, Thomas Chou <thomas@wytron.com.tw> wrote:
>> Hi Jagan,
>>
>> On 2015?11?03? 22:41, Jagan Teki wrote:
>>>
>>> Hi Thomas,
>>>
>>> On 3 November 2015 at 18:39, Thomas Chou <thomas@wytron.com.tw> wrote:
>>>>
>>>> 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.
>>>
>>>
>>> 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/

-- 
Jagan | openedev.

[U-Boot] [PATCH v4 1/3] dm: implement a MTD uclass

[Thomas Chou]

On 2015?11?03? 22:55, Jagan Teki wrote:
> On 3 November 2015 at 20:19, Thomas Chou <thomas@wytron.com.tw> wrote:
>> Hi Jagan,
>>
>> On 2015?11?03? 22:41, Jagan Teki wrote:
>>>
>>> Hi Thomas,
>>>
>>> On 3 November 2015 at 18:39, Thomas Chou <thomas@wytron.com.tw> wrote:
>>>>
>>>> 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.
>>>
>>>
>>> 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;
> };

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

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

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

Best regards,
Thomas

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

[Thomas Chou]

Hi Stefan,

On 2015?11?03? 21:54, Stefan Roese wrote:
> On 03.11.2015 14:09, 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.
>> 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.

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.

Best regards,
Thomas

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

[Simon Glass]

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

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

[Thomas Chou]

Hi Simon,

On 2015?11?06? 11:15, Simon Glass wrote:
>> +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
>

OK. Thanks.

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

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.

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

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.

Best regards,
Thomas

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

[Stefan Roese]

Hi Thomas,

On 06.11.2015 02:09, Thomas Chou wrote:
> On 2015?11?03? 21:54, Stefan Roese wrote:
>> On 03.11.2015 14:09, 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.
>>> 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.
>
> 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.

Thanks,
Stefan

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

[Thomas Chou]

Hi Stefan,

On 2015?11?06? 14:04, Stefan Roese wrote:
> Hi Thomas,
>
> On 06.11.2015 02:09, Thomas Chou wrote:
>> On 2015?11?03? 21:54, Stefan Roese wrote:
>>> On 03.11.2015 14:09, 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.
>>>> 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.
>>
>> 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.

Sure. I will wait Simon. Thanks a lot.

Best regards,
Thomas

[U-Boot] [PATCH v4 1/3] dm: implement a MTD uclass

[Simon Glass]

On 3 November 2015 at 06:09, Thomas Chou <thomas@wytron.com.tw> wrote:
> 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 <thomas@wytron.com.tw>
> ---
> 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
>
> diff --git a/drivers/mtd/Kconfig b/drivers/mtd/Kconfig
> index 59278d1..23dff48 100644
> --- a/drivers/mtd/Kconfig
> +++ b/drivers/mtd/Kconfig
> @@ -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"
> diff --git a/drivers/mtd/Makefile b/drivers/mtd/Makefile
> index a623f4c..c23c0c1 100644
> --- a/drivers/mtd/Makefile
> +++ b/drivers/mtd/Makefile
> @@ -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
> diff --git a/drivers/mtd/mtd-uclass.c b/drivers/mtd/mtd-uclass.c
> new file mode 100644
> index 0000000..8bd3e6b
> --- /dev/null
> +++ b/drivers/mtd/mtd-uclass.c
> @@ -0,0 +1,20 @@
> +/*
> + * Copyright (C) 2015 Thomas Chou <thomas@wytron.com.tw>
> + *
> + * 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",
> +};
> diff --git a/include/dm/uclass-id.h b/include/dm/uclass-id.h
> index 886a44c..fcc9784 100644
> --- a/include/dm/uclass-id.h
> +++ b/include/dm/uclass-id.h
> @@ -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 */
> diff --git a/include/flash.h b/include/flash.h
> index dc0645e..f53ace7 100644
> --- a/include/flash.h
> +++ b/include/flash.h
> @@ -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      */
> diff --git a/include/linux/mtd/mtd.h b/include/linux/mtd/mtd.h
> index e3d3fc7..0ab6128 100644
> --- a/include/linux/mtd/mtd.h
> +++ b/include/linux/mtd/mtd.h
> @@ -18,6 +18,7 @@
>
>  #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/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;
>  };
> diff --git a/include/mtd.h b/include/mtd.h
> new file mode 100644
> index 0000000..3f8c293
> --- /dev/null
> +++ b/include/mtd.h
> @@ -0,0 +1,23 @@
> +/*
> + * Copyright (C) 2015 Thomas Chou <thomas@wytron.com.tw>
> + *
> + * 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);
> +}

That function is a nice idea I think.

> +
> +#endif /* _MTD_H_ */
> --
> 2.5.0
>

Regards,
Simon

[U-Boot] [PATCH v4 1/3] dm: implement a MTD uclass

[Thomas Chou]

Hi Simon,

On 2015?11?06? 20:06, Simon Glass wrote:
>> diff --git a/include/linux/mtd/mtd.h b/include/linux/mtd/mtd.h
>> index e3d3fc7..0ab6128 100644
>> --- a/include/linux/mtd/mtd.h
>> +++ b/include/linux/mtd/mtd.h
>> @@ -18,6 +18,7 @@
>>
>>   #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?

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

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

Thanks a lot for your review.

Best regards,
Thomas

[U-Boot] [PATCH v4 1/3] dm: implement a MTD uclass

[Simon Glass]

Hi Thomas,

On 6 November 2015 at 05:25, Thomas Chou <thomas@wytron.com.tw> wrote:
> Hi Simon,
>
> On 2015?11?06? 20:06, Simon Glass wrote:
>>>
>>> diff --git a/include/linux/mtd/mtd.h b/include/linux/mtd/mtd.h
>>> index e3d3fc7..0ab6128 100644
>>> --- a/include/linux/mtd/mtd.h
>>> +++ b/include/linux/mtd/mtd.h
>>> @@ -18,6 +18,7 @@
>>>
>>>   #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?

Are you sure? It should be possible to declare a 'struct udevice *'
without this header.

>
>>>   #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)
>
>>> +       struct udevice *dev;
>>>   #endif
>>>          int usecount;
>>>   };
>
>
> Thanks a lot for your review.
>
> Best regards,
> Thomas

Regards,
Simon

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

[Simon Glass]

Hi Thomas,

On 5 November 2015 at 20:34, Thomas Chou <thomas@wytron.com.tw> wrote:
> Hi Simon,
>
> On 2015?11?06? 11:15, Simon Glass wrote:
>>>
>>> +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
>>
>
> OK. Thanks.
>
>>> +#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.
>
>
> 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.
>
>>> +#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?
>
>
> 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.

Hopefully you can move cfi_mtd_info into uclass-private data at some point.

Reviewed-by: Simon Glass <sjg@chromium.org>

Regards,
Simon

[U-Boot] [PATCH v5 1/3] dm: implement a MTD uclass

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

Signed-off-by: Thomas Chou <thomas@wytron.com.tw>
---
v3
  change to MTD uclass.
v4
  add mtd_info to flash_info in flash.h.
v5
  auto allocate uclass priv for mtd_info as suggested by Simon.
  remove #include <dm.h> in linux/mtd/mtd.h.

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

diff --git a/drivers/mtd/Kconfig b/drivers/mtd/Kconfig
index 59278d1..23dff48 100644
--- a/drivers/mtd/Kconfig
+++ b/drivers/mtd/Kconfig
@@ -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"
diff --git a/drivers/mtd/Makefile b/drivers/mtd/Makefile
index a623f4c..c23c0c1 100644
--- a/drivers/mtd/Makefile
+++ b/drivers/mtd/Makefile
@@ -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
diff --git a/drivers/mtd/mtd-uclass.c b/drivers/mtd/mtd-uclass.c
new file mode 100644
index 0000000..7b7c48e
--- /dev/null
+++ b/drivers/mtd/mtd-uclass.c
@@ -0,0 +1,21 @@
+/*
+ * Copyright (C) 2015 Thomas Chou <thomas@wytron.com.tw>
+ *
+ * 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",
+	.per_device_auto_alloc_size = sizeof(struct mtd_info),
+};
diff --git a/include/dm/uclass-id.h b/include/dm/uclass-id.h
index d0cf4ce..327de34 100644
--- a/include/dm/uclass-id.h
+++ b/include/dm/uclass-id.h
@@ -43,6 +43,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 */
diff --git a/include/flash.h b/include/flash.h
index 5754cf9..13e0384 100644
--- a/include/flash.h
+++ b/include/flash.h
@@ -41,6 +41,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	*/
diff --git a/include/linux/mtd/mtd.h b/include/linux/mtd/mtd.h
index e3d3fc7..c2cd3df 100644
--- a/include/linux/mtd/mtd.h
+++ b/include/linux/mtd/mtd.h
@@ -272,6 +272,8 @@ struct mtd_info {
 	struct module *owner;
 #ifndef __UBOOT__
 	struct device dev;
+#else
+	struct udevice *dev;
 #endif
 	int usecount;
 };
diff --git a/include/mtd.h b/include/mtd.h
new file mode 100644
index 0000000..3f8c293
--- /dev/null
+++ b/include/mtd.h
@@ -0,0 +1,23 @@
+/*
+ * Copyright (C) 2015 Thomas Chou <thomas@wytron.com.tw>
+ *
+ * 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] [PATCH v5 2/3] cfi_flash: convert to driver model

[Thomas Chou]

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)) {
+	}
+}
+
+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,46 @@ 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];
+
+	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 */
-- 
2.5.0

[U-Boot] [PATCH v5 3/3] nios2: use cfi flash driver model

[Thomas Chou]

Use cfi flash driver model.

Signed-off-by: Thomas Chou <thomas@wytron.com.tw>
---
 configs/nios2-generic_defconfig | 2 ++
 include/configs/nios2-generic.h | 3 +--
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/configs/nios2-generic_defconfig b/configs/nios2-generic_defconfig
index e42a15e..0a6de6f 100644
--- a/configs/nios2-generic_defconfig
+++ b/configs/nios2-generic_defconfig
@@ -17,6 +17,8 @@ CONFIG_NET_RANDOM_ETHADDR=y
 CONFIG_ALTERA_PIO=y
 CONFIG_MISC=y
 CONFIG_ALTERA_SYSID=y
+CONFIG_MTD=y
+CONFIG_CFI_FLASH=y
 CONFIG_DM_ETH=y
 CONFIG_ALTERA_TSE=y
 CONFIG_ALTERA_JTAG_UART=y
diff --git a/include/configs/nios2-generic.h b/include/configs/nios2-generic.h
index dbd39d6..4569de8 100644
--- a/include/configs/nios2-generic.h
+++ b/include/configs/nios2-generic.h
@@ -24,13 +24,12 @@
 /*
  * CFI Flash
  */
-#define CONFIG_SYS_FLASH_BASE		0xe0000000
 #define CONFIG_FLASH_CFI_DRIVER
 #define CONFIG_SYS_CFI_FLASH_STATUS_POLL /* fix amd flash issue */
 #define CONFIG_SYS_FLASH_CFI
 #define CONFIG_SYS_FLASH_USE_BUFFER_WRITE
 #define CONFIG_SYS_FLASH_PROTECTION
-#define CONFIG_SYS_MAX_FLASH_BANKS	1
+#define CONFIG_SYS_MAX_FLASH_BANKS_DETECT	1
 #define CONFIG_SYS_MAX_FLASH_SECT	512
 
 /*
-- 
2.5.0

[U-Boot] [PATCH v4 1/3] dm: implement a MTD uclass

[Jagan Teki]

Hi Thomas/Simon,

On 4 November 2015 at 08:42, Thomas Chou <thomas@wytron.com.tw> wrote:
>
>
> On 2015?11?03? 22:55, Jagan Teki wrote:
>>
>> On 3 November 2015 at 20:19, Thomas Chou <thomas@wytron.com.tw> wrote:
>>>
>>> Hi Jagan,
>>>
>>> On 2015?11?03? 22:41, Jagan Teki wrote:
>>>>
>>>>
>>>> Hi Thomas,
>>>>
>>>> On 3 November 2015 at 18:39, Thomas Chou <thomas@wytron.com.tw> wrote:
>>>>>
>>>>>
>>>>> 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.
>>>>
>>>>
>>>>
>>>> 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;
>> };
>
>
> Not exactly. I included udevice in mtd_info as it was device for Linux.
>
> @@ -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.
>
>>  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. :)

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.

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

thanks!
-- 
Jagan | openedev.

[U-Boot] [PATCH v4 1/3] dm: implement a MTD uclass

[Thomas Chou]

HI Jagan,

On 2015?11?07? 20:35, Jagan Teki wrote:
>> 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.

Please see v5 of this patch and v3 of altera qspi.

The uclass priv of mtd class is an auto-allocated mtd_info. The 
spi_flash_priv includes both mtd_info and spi_flash. So the only 
difference is the spi_flash. This is because cfi uses struct flash, 
while spi flash uses struct spi_flash. So it is better to leave the 
struct flash to driver allocation.

The mtd->priv points to struct spi_flash for spi flash, and points to 
struct flash for cfi flash. It serves the same purpose.

The struct flash has *mtd. The struct spi_flash has *mtd, too.

I added struct udevice *dev to mtd_info. The struct spi_flash has *dev, 
but struct flash has not.

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

struct flash {
...
struct mtd_info *info;
}

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

Simply,
struct mtd_info.


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

mtd = dev_get_uclass_priv(dev);
flash->mtd = mtd;

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

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

flash->mtd->dev is the same as spi's flash->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.
>

The flash ops which u-boot commands calls are built upon mtd ops.

eg,
write_buff()
{
   mtd_write();
}

flash_erase()
{
   mtd_erase();
}

Please let me know what do you think. Thanks. :)

Best regards,
Thomas

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

[Simon Glass]

On 6 November 2015 at 23:57, 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

Reviewed-by: Simon Glass <sjg@chromium.org>

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

[Jagan Teki]

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.

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

[Thomas Chou]

Hi Jagan,

On 2015?12?06? 16:23, Jagan Teki wrote:
>> +#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.

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

Best regards,
Thomas

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

[Jagan Teki]

Hi Thomas,

On 6 December 2015 at 17:07, Thomas Chou <thomas@wytron.com.tw> wrote:
> Hi Jagan,
>
> On 2015?12?06? 16:23, Jagan Teki wrote:
>>>
>>> +#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.
>
>
> 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?

thanks!
-- 
Jagan.

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

[Thomas Chou]

Hi Jagan,

On 2015?12?06? 21:10, Jagan Teki wrote:
>> 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.

Best regards,
Thomas

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

[Jagan Teki]

On 6 December 2015 at 19:05, Thomas Chou <thomas@wytron.com.tw> wrote:
> Hi Jagan,
>
> On 2015?12?06? 21:10, Jagan Teki wrote:
>>>
>>> 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.

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

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

thanks!
-- 
Jagan.



Sent with MailTrack

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

[Thomas Chou]

Hi Jagan,

On 2015?12?06? 23:03, Jagan Teki wrote:
> On 6 December 2015 at 19:05, Thomas Chou <thomas@wytron.com.tw> wrote:
>> Hi Jagan,
>>
>> On 2015?12?06? 21:10, Jagan Teki wrote:
>>>>
>>>> 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.
>
> So, this is the way drivers/mtd/altera_qspi.c getting probed?

Yes.

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

I am not sure whether the dm scan node should be on mtd uclass or 
specific drivers. Could you give more details?

Best regards,
Thomas