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

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