| Message ID | 1444289667-23775-1-git-send-email-thomas@wytron.com.tw |
|---|---|
| State | Accepted, archived |
| Delegated to: | Thomas Chou |
| Headers | show |
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
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
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@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@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
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@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@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
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
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 */
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