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

Simon Glass
2015-10-09T09:36:55Z

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