[U-Boot] [RFC PATCH] mmc: omap_hsmmc: convert to use dm block devies
Convert OMAP hsmmc driver to use driver-model block devices.
Signed-off-by: Grygorii Strashko grygorii.strashko@ti.com --- Hi All,
First of all, sorry if my questions/problems are looks dumb, I'm very new with u-boot.
This is my attampt to enable CONFIG_BLK on OMAP platforms which is blocked now by omap_hsmmc driver. omap_hsmmc required to be updated to use driver-model block devices at minimum (and max to use dm_mmc_ops). Also, as per my understanding, CONFIG_BLK is blocker for other tasks like enabling driver model for OMAP sata devices.
With this patch I can boot from mmc on am335x-evm, but there are two problems I need help with: 1) My changes in Makefiles looks really ugly, but without them SPL build will fail because undef'ing in include/configs/am335x_evm.h does not take effect in Makefile (thanks Vignesh for the information [1]) and I, honestly, do not know how to fix it in better way, so I'd be appreciated for any help. Comparing to Vignesh's case, files which need to be excluded from build are generic and I worry that there can be dependecy from CONFIG_SPL_DM.
2) with this patch I can see error message in log "** Bad device size - mmc 0 **": U-Boot 2017.03-rc1-00020-g1a3b11a-dirty (Feb 02 2017 - 12:04:01 -0600)
CPU : AM335X-GP rev 2.0 Model: TI AM335x EVM DRAM: 1 GiB NAND: 256 MiB MMC: mmc@48060000mmc@47810000OMAP SD/MMC: 0, OMAP SD/MMC: 1 ** Bad device size - mmc 0 ** Using default environment
this message is triggered from: board_r.c: initr_env() - env_common.c: env_relocate() - env_fat.c: env_relocate_spec() - part.c: blk_get_device_part_str() because partitions are not initialized yet, as i understood,
Seems there are should be additional call to mmc_init():mmc_startup():part_init(), but I'm not sure where/how to add it correctly.
Thanks.
[1] http://lists.denx.de/pipermail/u-boot/2016-August/262447.html
configs/am335x_evm_defconfig | 1 - drivers/block/Makefile | 6 ++++- drivers/mmc/Makefile | 8 ++++--- drivers/mmc/omap_hsmmc.c | 55 ++++++++++++++++++++++++++++++++------------ include/configs/am335x_evm.h | 1 + 5 files changed, 51 insertions(+), 20 deletions(-)
diff --git a/configs/am335x_evm_defconfig b/configs/am335x_evm_defconfig index 6f624e9..d36e7d4 100644 --- a/configs/am335x_evm_defconfig +++ b/configs/am335x_evm_defconfig @@ -30,7 +30,6 @@ CONFIG_CMD_GPIO=y CONFIG_CMD_EXT4_WRITE=y CONFIG_OF_CONTROL=y CONFIG_OF_LIST="am335x-evm am335x-bone am335x-boneblack am335x-evmsk am335x-bonegreen am335x-icev2" -# CONFIG_BLK is not set CONFIG_DFU_MMC=y CONFIG_DFU_NAND=y CONFIG_DFU_RAM=y diff --git a/drivers/block/Makefile b/drivers/block/Makefile index 41217c1..44baee3 100644 --- a/drivers/block/Makefile +++ b/drivers/block/Makefile @@ -5,12 +5,16 @@ # SPDX-License-Identifier: GPL-2.0+ #
-obj-$(CONFIG_BLK) += blk-uclass.o +obj-$(CONFIG_$(SPL_)BLK) += blk-uclass.o
ifndef CONFIG_BLK obj-y += blk_legacy.o endif
+ifdef SPL_ +obj-y += blk_legacy.o +endif + obj-$(CONFIG_AHCI) += ahci-uclass.o obj-$(CONFIG_DM_SCSI) += scsi-uclass.o
diff --git a/drivers/mmc/Makefile b/drivers/mmc/Makefile index 6af7f79..bc16c78 100644 --- a/drivers/mmc/Makefile +++ b/drivers/mmc/Makefile @@ -5,14 +5,16 @@ # SPDX-License-Identifier: GPL-2.0+ #
-ifdef CONFIG_DM_MMC -obj-$(CONFIG_GENERIC_MMC) += mmc-uclass.o -endif +obj-$(CONFIG_$(SPL_)DM_MMC) += mmc-uclass.o
ifndef CONFIG_BLK obj-$(CONFIG_GENERIC_MMC) += mmc_legacy.o endif
+ifdef SPL_ +obj-$(CONFIG_GENERIC_MMC) += mmc_legacy.o +endif + obj-$(CONFIG_ARM_PL180_MMCI) += arm_pl180_mmci.o obj-$(CONFIG_ATMEL_SDHCI) += atmel_sdhci.o obj-$(CONFIG_BFIN_SDH) += bfin_sdh.o diff --git a/drivers/mmc/omap_hsmmc.c b/drivers/mmc/omap_hsmmc.c index b326846..fa5d72b 100644 --- a/drivers/mmc/omap_hsmmc.c +++ b/drivers/mmc/omap_hsmmc.c @@ -53,9 +53,16 @@ DECLARE_GLOBAL_DATA_PTR; #define SYSCTL_SRC (1 << 25) #define SYSCTL_SRD (1 << 26)
+struct omap_hsmmc_plat { + struct mmc_config cfg; + struct mmc mmc; +}; + struct omap_hsmmc_data { struct hsmmc *base_addr; +#ifndef CONFIG_DM_MMC struct mmc_config cfg; +#endif #ifdef OMAP_HSMMC_USE_GPIO #ifdef CONFIG_DM_MMC struct gpio_desc cd_gpio; /* Change Detect GPIO */ @@ -137,7 +144,7 @@ static unsigned char mmc_board_init(struct mmc *mmc)
#if defined(CONFIG_OMAP54XX) || defined(CONFIG_OMAP44XX) /* PBIAS config needed for MMC1 only */ - if (mmc->block_dev.devnum == 0) + if (mmc_get_blk_desc(mmc)->devnum == 0) vmmc_pbias_config(LDO_VOLT_3V0); #endif
@@ -171,15 +178,24 @@ void mmc_init_stream(struct hsmmc *mmc_base) } writel(readl(&mmc_base->con) & ~INIT_INITSTREAM, &mmc_base->con); } +struct omap_hsmmc_data *omap_hsmmc_get_data(struct mmc *mmc) +{ +#ifdef CONFIG_DM_MMC + return dev_get_priv(mmc->dev); +#else + return (struct omap_hsmmc_data *)mmc->priv; +#endif +}
static int omap_hsmmc_init_setup(struct mmc *mmc) { + struct omap_hsmmc_data *priv = omap_hsmmc_get_data(mmc); struct hsmmc *mmc_base; unsigned int reg_val; unsigned int dsor; ulong start;
- mmc_base = ((struct omap_hsmmc_data *)mmc->priv)->base_addr; + mmc_base = priv->base_addr; mmc_board_init(mmc);
writel(readl(&mmc_base->sysconfig) | MMC_SOFTRESET, @@ -284,11 +300,12 @@ static void mmc_reset_controller_fsm(struct hsmmc *mmc_base, u32 bit) static int omap_hsmmc_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd, struct mmc_data *data) { + struct omap_hsmmc_data *priv = omap_hsmmc_get_data(mmc); struct hsmmc *mmc_base; unsigned int flags, mmc_stat; ulong start;
- mmc_base = ((struct omap_hsmmc_data *)mmc->priv)->base_addr; + mmc_base = priv->base_addr; start = get_timer(0); while ((readl(&mmc_base->pstate) & (DATI_MASK | CMDI_MASK)) != 0) { if (get_timer(0) - start > MAX_RETRY_MS) { @@ -513,11 +530,12 @@ static int mmc_write_data(struct hsmmc *mmc_base, const char *buf,
static int omap_hsmmc_set_ios(struct mmc *mmc) { + struct omap_hsmmc_data *priv = omap_hsmmc_get_data(mmc); struct hsmmc *mmc_base; unsigned int dsor = 0; ulong start;
- mmc_base = ((struct omap_hsmmc_data *)mmc->priv)->base_addr; + mmc_base = priv->base_addr; /* configue bus width */ switch (mmc->bus_width) { case 8: @@ -571,7 +589,7 @@ static int omap_hsmmc_set_ios(struct mmc *mmc) #ifdef CONFIG_DM_MMC static int omap_hsmmc_getcd(struct mmc *mmc) { - struct omap_hsmmc_data *priv = mmc->priv; + struct omap_hsmmc_data *priv = omap_hsmmc_get_data(mmc); int value;
value = dm_gpio_get_value(&priv->cd_gpio); @@ -586,7 +604,7 @@ static int omap_hsmmc_getcd(struct mmc *mmc)
static int omap_hsmmc_getwp(struct mmc *mmc) { - struct omap_hsmmc_data *priv = mmc->priv; + struct omap_hsmmc_data *priv = omap_hsmmc_get_data(mmc); int value;
value = dm_gpio_get_value(&priv->wp_gpio); @@ -727,6 +745,7 @@ int omap_mmc_init(int dev_index, uint host_caps_mask, uint f_max, int cd_gpio, static int omap_hsmmc_ofdata_to_platdata(struct udevice *dev) { struct omap_hsmmc_data *priv = dev_get_priv(dev); + struct omap_hsmmc_plat *plat = dev_get_platdata(dev); const void *fdt = gd->fdt_blob; int node = dev->of_offset; struct mmc_config *cfg; @@ -734,7 +753,7 @@ static int omap_hsmmc_ofdata_to_platdata(struct udevice *dev)
priv->base_addr = map_physmem(dev_get_addr(dev), sizeof(struct hsmmc *), MAP_NOCACHE); - cfg = &priv->cfg; + cfg = &plat->cfg;
cfg->host_caps = MMC_MODE_HS_52MHz | MMC_MODE_HS; val = fdtdec_get_int(fdt, node, "bus-width", -1); @@ -766,29 +785,33 @@ static int omap_hsmmc_ofdata_to_platdata(struct udevice *dev) return 0; }
+static int omap_hsmmc_bind(struct udevice *dev) +{ + struct omap_hsmmc_plat *plat = dev_get_platdata(dev); + + return mmc_bind(dev, &plat->mmc, &plat->cfg); +} + + static int omap_hsmmc_probe(struct udevice *dev) { + struct omap_hsmmc_plat *plat = dev_get_platdata(dev); struct mmc_uclass_priv *upriv = dev_get_uclass_priv(dev); struct omap_hsmmc_data *priv = dev_get_priv(dev); struct mmc_config *cfg; - struct mmc *mmc;
- cfg = &priv->cfg; + cfg = &plat->cfg; cfg->name = "OMAP SD/MMC"; cfg->ops = &omap_hsmmc_ops;
- mmc = mmc_create(cfg, priv); - if (mmc == NULL) - return -1; - #ifdef OMAP_HSMMC_USE_GPIO gpio_request_by_name(dev, "cd-gpios", 0, &priv->cd_gpio, GPIOD_IS_IN); gpio_request_by_name(dev, "wp-gpios", 0, &priv->wp_gpio, GPIOD_IS_IN); #endif
- mmc->dev = dev; - upriv->mmc = mmc; + upriv->mmc = &plat->mmc;
+ printf("%s", plat->mmc.dev->name); return 0; }
@@ -804,7 +827,9 @@ U_BOOT_DRIVER(omap_hsmmc) = { .id = UCLASS_MMC, .of_match = omap_hsmmc_ids, .ofdata_to_platdata = omap_hsmmc_ofdata_to_platdata, + .bind = omap_hsmmc_bind, .probe = omap_hsmmc_probe, .priv_auto_alloc_size = sizeof(struct omap_hsmmc_data), + .platdata_auto_alloc_size = sizeof(struct omap_hsmmc_plat), }; #endif diff --git a/include/configs/am335x_evm.h b/include/configs/am335x_evm.h index 7390015..6368fb8 100644 --- a/include/configs/am335x_evm.h +++ b/include/configs/am335x_evm.h @@ -285,6 +285,7 @@ */ #ifdef CONFIG_SPL_BUILD #undef CONFIG_DM_MMC +#undef CONFIG_BLK #undef CONFIG_TIMER #undef CONFIG_DM_USB #undef CONFIG_DM_NAND
On Friday 03 February 2017 03:48 AM, Strashko, Grygorii wrote:
Convert OMAP hsmmc driver to use driver-model block devices.
Signed-off-by: Grygorii Strashko grygorii.strashko@ti.com
Hi All,
First of all, sorry if my questions/problems are looks dumb, I'm very new with u-boot.
This is my attampt to enable CONFIG_BLK on OMAP platforms which is blocked now by omap_hsmmc driver. omap_hsmmc required to be updated to use driver-model block devices at minimum (and max to use dm_mmc_ops). Also, as per my understanding, CONFIG_BLK is blocker for other tasks like enabling driver model for OMAP sata devices.
With this patch I can boot from mmc on am335x-evm, but there are two problems I need help with:
- My changes in Makefiles looks really ugly, but without them SPL build will
fail because undef'ing in include/configs/am335x_evm.h does not take effect in Makefile (thanks Vignesh for the information [1]) and I, honestly, do not know how to fix it in better way, so I'd be appreciated for any help. Comparing to Vignesh's case, files which need to be excluded from build are generic and I worry that there can be dependecy from CONFIG_SPL_DM.
diff --git a/configs/am335x_evm_defconfig b/configs/am335x_evm_defconfig index 6f624e9..d36e7d4 100644 --- a/configs/am335x_evm_defconfig +++ b/configs/am335x_evm_defconfig @@ -30,7 +30,6 @@ CONFIG_CMD_GPIO=y CONFIG_CMD_EXT4_WRITE=y CONFIG_OF_CONTROL=y CONFIG_OF_LIST="am335x-evm am335x-bone am335x-boneblack am335x-evmsk am335x-bonegreen am335x-icev2" -# CONFIG_BLK is not set CONFIG_DFU_MMC=y CONFIG_DFU_NAND=y CONFIG_DFU_RAM=y diff --git a/drivers/block/Makefile b/drivers/block/Makefile index 41217c1..44baee3 100644 --- a/drivers/block/Makefile +++ b/drivers/block/Makefile @@ -5,12 +5,16 @@ # SPDX-License-Identifier: GPL-2.0+ #
-obj-$(CONFIG_BLK) += blk-uclass.o +obj-$(CONFIG_$(SPL_)BLK) += blk-uclass.o
ifndef CONFIG_BLK obj-y += blk_legacy.o endif
+ifdef SPL_ +obj-y += blk_legacy.o +endif
I am facing a same problem with DM_ETH as well. How about something like:
ifeq ($(CONFIG_$(SPL_)DM)$(CONFIG_BLK),yy) obj-y += blk_uclass.o else obj-y += blk_legacy.o endif
Is this an acceptable solution?
On Fri, Feb 03, 2017 at 09:57:03AM +0530, Vignesh R wrote:
On Friday 03 February 2017 03:48 AM, Strashko, Grygorii wrote:
Convert OMAP hsmmc driver to use driver-model block devices.
Signed-off-by: Grygorii Strashko grygorii.strashko@ti.com
Hi All,
First of all, sorry if my questions/problems are looks dumb, I'm very new with u-boot.
This is my attampt to enable CONFIG_BLK on OMAP platforms which is blocked now by omap_hsmmc driver. omap_hsmmc required to be updated to use driver-model block devices at minimum (and max to use dm_mmc_ops). Also, as per my understanding, CONFIG_BLK is blocker for other tasks like enabling driver model for OMAP sata devices.
With this patch I can boot from mmc on am335x-evm, but there are two problems I need help with:
- My changes in Makefiles looks really ugly, but without them SPL build will
fail because undef'ing in include/configs/am335x_evm.h does not take effect in Makefile (thanks Vignesh for the information [1]) and I, honestly, do not know how to fix it in better way, so I'd be appreciated for any help. Comparing to Vignesh's case, files which need to be excluded from build are generic and I worry that there can be dependecy from CONFIG_SPL_DM.
diff --git a/configs/am335x_evm_defconfig b/configs/am335x_evm_defconfig index 6f624e9..d36e7d4 100644 --- a/configs/am335x_evm_defconfig +++ b/configs/am335x_evm_defconfig @@ -30,7 +30,6 @@ CONFIG_CMD_GPIO=y CONFIG_CMD_EXT4_WRITE=y CONFIG_OF_CONTROL=y CONFIG_OF_LIST="am335x-evm am335x-bone am335x-boneblack am335x-evmsk am335x-bonegreen am335x-icev2" -# CONFIG_BLK is not set CONFIG_DFU_MMC=y CONFIG_DFU_NAND=y CONFIG_DFU_RAM=y diff --git a/drivers/block/Makefile b/drivers/block/Makefile index 41217c1..44baee3 100644 --- a/drivers/block/Makefile +++ b/drivers/block/Makefile @@ -5,12 +5,16 @@ # SPDX-License-Identifier: GPL-2.0+ #
-obj-$(CONFIG_BLK) += blk-uclass.o +obj-$(CONFIG_$(SPL_)BLK) += blk-uclass.o
ifndef CONFIG_BLK obj-y += blk_legacy.o endif
+ifdef SPL_ +obj-y += blk_legacy.o +endif
I am facing a same problem with DM_ETH as well. How about something like:
ifeq ($(CONFIG_$(SPL_)DM)$(CONFIG_BLK),yy) obj-y += blk_uclass.o else obj-y += blk_legacy.o endif
Is this an acceptable solution?
No, I think this is really highlighting that we need to figure out a solution to using DM inside of SPL on these classes of devices. I think the last time I started to think out loud about how to solve this my suggestion was to make a "dummy" board dts file that is correct enough for SPL needs and that we can bring up full U-Boot on. Off the top of my head, the differences between BBB/BBW/GP-EVM/EVM-SK/BBG are not things we would see inside of SPL. In a similar manner we should be able to get am43xx going, and dra7xx/am57xx going.
On 02/03/2017 11:23 AM, Tom Rini wrote:
On Fri, Feb 03, 2017 at 09:57:03AM +0530, Vignesh R wrote:
On Friday 03 February 2017 03:48 AM, Strashko, Grygorii wrote:
Convert OMAP hsmmc driver to use driver-model block devices.
Signed-off-by: Grygorii Strashko grygorii.strashko@ti.com
Hi All,
First of all, sorry if my questions/problems are looks dumb, I'm very new with u-boot.
This is my attampt to enable CONFIG_BLK on OMAP platforms which is blocked now by omap_hsmmc driver. omap_hsmmc required to be updated to use driver-model block devices at minimum (and max to use dm_mmc_ops). Also, as per my understanding, CONFIG_BLK is blocker for other tasks like enabling driver model for OMAP sata devices.
With this patch I can boot from mmc on am335x-evm, but there are two problems I need help with:
- My changes in Makefiles looks really ugly, but without them SPL build will
fail because undef'ing in include/configs/am335x_evm.h does not take effect in Makefile (thanks Vignesh for the information [1]) and I, honestly, do not know how to fix it in better way, so I'd be appreciated for any help. Comparing to Vignesh's case, files which need to be excluded from build are generic and I worry that there can be dependecy from CONFIG_SPL_DM.
diff --git a/configs/am335x_evm_defconfig b/configs/am335x_evm_defconfig index 6f624e9..d36e7d4 100644 --- a/configs/am335x_evm_defconfig +++ b/configs/am335x_evm_defconfig @@ -30,7 +30,6 @@ CONFIG_CMD_GPIO=y CONFIG_CMD_EXT4_WRITE=y CONFIG_OF_CONTROL=y CONFIG_OF_LIST="am335x-evm am335x-bone am335x-boneblack am335x-evmsk am335x-bonegreen am335x-icev2" -# CONFIG_BLK is not set CONFIG_DFU_MMC=y CONFIG_DFU_NAND=y CONFIG_DFU_RAM=y diff --git a/drivers/block/Makefile b/drivers/block/Makefile index 41217c1..44baee3 100644 --- a/drivers/block/Makefile +++ b/drivers/block/Makefile @@ -5,12 +5,16 @@ # SPDX-License-Identifier: GPL-2.0+ #
-obj-$(CONFIG_BLK) += blk-uclass.o +obj-$(CONFIG_$(SPL_)BLK) += blk-uclass.o
ifndef CONFIG_BLK obj-y += blk_legacy.o endif
+ifdef SPL_ +obj-y += blk_legacy.o +endif
I am facing a same problem with DM_ETH as well. How about something like:
ifeq ($(CONFIG_$(SPL_)DM)$(CONFIG_BLK),yy) obj-y += blk_uclass.o else obj-y += blk_legacy.o endif
Is this an acceptable solution?
No, I think this is really highlighting that we need to figure out a solution to using DM inside of SPL on these classes of devices. I think the last time I started to think out loud about how to solve this my suggestion was to make a "dummy" board dts file that is correct enough for SPL needs and that we can bring up full U-Boot on. Off the top of my head, the differences between BBB/BBW/GP-EVM/EVM-SK/BBG are not things we would see inside of SPL. In a similar manner we should be able to get am43xx going, and dra7xx/am57xx going.
We don't really need DT support in SPL for using DM, we could statically instantiate device drivers using a board file type system, platform_add_device(device_data) etc..
My biggest objection is in SPL bloat. What do we get by adding DM to SPL? Code re-use is nice, but SPL had hard needs in speed and size that will continue to be eroded by these changes.
Perhaps it is time to re-consider adding separate defconfigs/Makefiles for SPL?
U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
On Mon, Feb 06, 2017 at 11:31:14AM -0600, Andrew F. Davis wrote:
On 02/03/2017 11:23 AM, Tom Rini wrote:
On Fri, Feb 03, 2017 at 09:57:03AM +0530, Vignesh R wrote:
On Friday 03 February 2017 03:48 AM, Strashko, Grygorii wrote:
Convert OMAP hsmmc driver to use driver-model block devices.
Signed-off-by: Grygorii Strashko grygorii.strashko@ti.com
Hi All,
First of all, sorry if my questions/problems are looks dumb, I'm very new with u-boot.
This is my attampt to enable CONFIG_BLK on OMAP platforms which is blocked now by omap_hsmmc driver. omap_hsmmc required to be updated to use driver-model block devices at minimum (and max to use dm_mmc_ops). Also, as per my understanding, CONFIG_BLK is blocker for other tasks like enabling driver model for OMAP sata devices.
With this patch I can boot from mmc on am335x-evm, but there are two problems I need help with:
- My changes in Makefiles looks really ugly, but without them SPL build will
fail because undef'ing in include/configs/am335x_evm.h does not take effect in Makefile (thanks Vignesh for the information [1]) and I, honestly, do not know how to fix it in better way, so I'd be appreciated for any help. Comparing to Vignesh's case, files which need to be excluded from build are generic and I worry that there can be dependecy from CONFIG_SPL_DM.
diff --git a/configs/am335x_evm_defconfig b/configs/am335x_evm_defconfig index 6f624e9..d36e7d4 100644 --- a/configs/am335x_evm_defconfig +++ b/configs/am335x_evm_defconfig @@ -30,7 +30,6 @@ CONFIG_CMD_GPIO=y CONFIG_CMD_EXT4_WRITE=y CONFIG_OF_CONTROL=y CONFIG_OF_LIST="am335x-evm am335x-bone am335x-boneblack am335x-evmsk am335x-bonegreen am335x-icev2" -# CONFIG_BLK is not set CONFIG_DFU_MMC=y CONFIG_DFU_NAND=y CONFIG_DFU_RAM=y diff --git a/drivers/block/Makefile b/drivers/block/Makefile index 41217c1..44baee3 100644 --- a/drivers/block/Makefile +++ b/drivers/block/Makefile @@ -5,12 +5,16 @@ # SPDX-License-Identifier: GPL-2.0+ #
-obj-$(CONFIG_BLK) += blk-uclass.o +obj-$(CONFIG_$(SPL_)BLK) += blk-uclass.o
ifndef CONFIG_BLK obj-y += blk_legacy.o endif
+ifdef SPL_ +obj-y += blk_legacy.o +endif
I am facing a same problem with DM_ETH as well. How about something like:
ifeq ($(CONFIG_$(SPL_)DM)$(CONFIG_BLK),yy) obj-y += blk_uclass.o else obj-y += blk_legacy.o endif
Is this an acceptable solution?
No, I think this is really highlighting that we need to figure out a solution to using DM inside of SPL on these classes of devices. I think the last time I started to think out loud about how to solve this my suggestion was to make a "dummy" board dts file that is correct enough for SPL needs and that we can bring up full U-Boot on. Off the top of my head, the differences between BBB/BBW/GP-EVM/EVM-SK/BBG are not things we would see inside of SPL. In a similar manner we should be able to get am43xx going, and dra7xx/am57xx going.
We don't really need DT support in SPL for using DM, we could statically instantiate device drivers using a board file type system, platform_add_device(device_data) etc..
My biggest objection is in SPL bloat. What do we get by adding DM to SPL? Code re-use is nice, but SPL had hard needs in speed and size that will continue to be eroded by these changes.
Perhaps it is time to re-consider adding separate defconfigs/Makefiles for SPL?
The problem we need to solve is to also clean up the code and keep it clean. The fine line we need to walk is the ability to fit into both reasonably small and still reasonably supported SoCs and the need to continue to be able to re-use code and unify code. And part of the answer here may be to say that newer compilers will be the required minimum in some platforms too. For example with am335x_hs: gcc-6.1.1 (Debian 6.1.1-9): $ size am335x_hs_evm/u-boot-spl text data bss dec hex filename 37810 2452 198920 239182 3a64e am335x_hs_evm/u-boot-spl gcc-4.9.0: $ size am335x_hs_evm/u-boot-spl text data bss dec hex filename 42082 2452 198980 243514 3b73a am335x_hs_evm/u-boot-spl
Since we're just talking text my assumption is that the fix for https://gcc.gnu.org/bugzilla/show_bug.cgi?id=54303 being available vs not (it's also not fixed in gcc-5) is what is making up that large difference. And going from 698 bytes headroom to 4970 bytes headroom will make things a little easier to deal with.
Hi,
On 2 February 2017 at 15:18, Grygorii Strashko grygorii.strashko@ti.com wrote:
Convert OMAP hsmmc driver to use driver-model block devices.
Signed-off-by: Grygorii Strashko grygorii.strashko@ti.com
Hi All,
First of all, sorry if my questions/problems are looks dumb, I'm very new with u-boot.
This is my attampt to enable CONFIG_BLK on OMAP platforms which is blocked now by omap_hsmmc driver. omap_hsmmc required to be updated to use driver-model block devices at minimum (and max to use dm_mmc_ops). Also, as per my understanding, CONFIG_BLK is blocker for other tasks like enabling driver model for OMAP sata devices.
With this patch I can boot from mmc on am335x-evm, but there are two problems I need help with:
- My changes in Makefiles looks really ugly, but without them SPL build will
fail because undef'ing in include/configs/am335x_evm.h does not take effect in Makefile (thanks Vignesh for the information [1]) and I, honestly, do not know how to fix it in better way, so I'd be appreciated for any help. Comparing to Vignesh's case, files which need to be excluded from build are generic and I worry that there can be dependecy from CONFIG_SPL_DM.
It would be great to enable CONFIG_BLK and CONFIG_DM_MMC in SPL. Perhaps Tom's message (later in this thread) about the gcc bug might help. The size difference is small, but not 0 unfortunately.
- with this patch I can see error message in log "** Bad device size - mmc 0 **":
U-Boot 2017.03-rc1-00020-g1a3b11a-dirty (Feb 02 2017 - 12:04:01 -0600)
CPU : AM335X-GP rev 2.0 Model: TI AM335x EVM DRAM: 1 GiB NAND: 256 MiB MMC: mmc@48060000mmc@47810000OMAP SD/MMC: 0, OMAP SD/MMC: 1 ** Bad device size - mmc 0 ** Using default environment
this message is triggered from: board_r.c: initr_env()
- env_common.c: env_relocate()
- env_fat.c: env_relocate_spec()
- part.c: blk_get_device_part_str()
because partitions are not initialized yet, as i understood,
Seems there are should be additional call to mmc_init():mmc_startup():part_init(), but I'm not sure where/how to add it correctly.
Does this help?
http://patchwork.ozlabs.org/patch/717710/
Regards, Simon
Thanks.
[1] http://lists.denx.de/pipermail/u-boot/2016-August/262447.html
configs/am335x_evm_defconfig | 1 - drivers/block/Makefile | 6 ++++- drivers/mmc/Makefile | 8 ++++--- drivers/mmc/omap_hsmmc.c | 55 ++++++++++++++++++++++++++++++++------------ include/configs/am335x_evm.h | 1 + 5 files changed, 51 insertions(+), 20 deletions(-)
diff --git a/configs/am335x_evm_defconfig b/configs/am335x_evm_defconfig index 6f624e9..d36e7d4 100644 --- a/configs/am335x_evm_defconfig +++ b/configs/am335x_evm_defconfig @@ -30,7 +30,6 @@ CONFIG_CMD_GPIO=y CONFIG_CMD_EXT4_WRITE=y CONFIG_OF_CONTROL=y CONFIG_OF_LIST="am335x-evm am335x-bone am335x-boneblack am335x-evmsk am335x-bonegreen am335x-icev2" -# CONFIG_BLK is not set CONFIG_DFU_MMC=y CONFIG_DFU_NAND=y CONFIG_DFU_RAM=y diff --git a/drivers/block/Makefile b/drivers/block/Makefile index 41217c1..44baee3 100644 --- a/drivers/block/Makefile +++ b/drivers/block/Makefile @@ -5,12 +5,16 @@ # SPDX-License-Identifier: GPL-2.0+ #
-obj-$(CONFIG_BLK) += blk-uclass.o +obj-$(CONFIG_$(SPL_)BLK) += blk-uclass.o
ifndef CONFIG_BLK obj-y += blk_legacy.o endif
+ifdef SPL_ +obj-y += blk_legacy.o +endif
obj-$(CONFIG_AHCI) += ahci-uclass.o obj-$(CONFIG_DM_SCSI) += scsi-uclass.o
diff --git a/drivers/mmc/Makefile b/drivers/mmc/Makefile index 6af7f79..bc16c78 100644 --- a/drivers/mmc/Makefile +++ b/drivers/mmc/Makefile @@ -5,14 +5,16 @@ # SPDX-License-Identifier: GPL-2.0+ #
-ifdef CONFIG_DM_MMC -obj-$(CONFIG_GENERIC_MMC) += mmc-uclass.o -endif +obj-$(CONFIG_$(SPL_)DM_MMC) += mmc-uclass.o
ifndef CONFIG_BLK obj-$(CONFIG_GENERIC_MMC) += mmc_legacy.o endif
+ifdef SPL_ +obj-$(CONFIG_GENERIC_MMC) += mmc_legacy.o +endif
obj-$(CONFIG_ARM_PL180_MMCI) += arm_pl180_mmci.o obj-$(CONFIG_ATMEL_SDHCI) += atmel_sdhci.o obj-$(CONFIG_BFIN_SDH) += bfin_sdh.o diff --git a/drivers/mmc/omap_hsmmc.c b/drivers/mmc/omap_hsmmc.c index b326846..fa5d72b 100644 --- a/drivers/mmc/omap_hsmmc.c +++ b/drivers/mmc/omap_hsmmc.c @@ -53,9 +53,16 @@ DECLARE_GLOBAL_DATA_PTR; #define SYSCTL_SRC (1 << 25) #define SYSCTL_SRD (1 << 26)
+struct omap_hsmmc_plat {
struct mmc_config cfg;struct mmc mmc;+};
struct omap_hsmmc_data { struct hsmmc *base_addr; +#ifndef CONFIG_DM_MMC struct mmc_config cfg; +#endif #ifdef OMAP_HSMMC_USE_GPIO #ifdef CONFIG_DM_MMC struct gpio_desc cd_gpio; /* Change Detect GPIO */ @@ -137,7 +144,7 @@ static unsigned char mmc_board_init(struct mmc *mmc)
#if defined(CONFIG_OMAP54XX) || defined(CONFIG_OMAP44XX) /* PBIAS config needed for MMC1 only */
if (mmc->block_dev.devnum == 0)
if (mmc_get_blk_desc(mmc)->devnum == 0) vmmc_pbias_config(LDO_VOLT_3V0);#endif
@@ -171,15 +178,24 @@ void mmc_init_stream(struct hsmmc *mmc_base) } writel(readl(&mmc_base->con) & ~INIT_INITSTREAM, &mmc_base->con); } +struct omap_hsmmc_data *omap_hsmmc_get_data(struct mmc *mmc) +{ +#ifdef CONFIG_DM_MMC
return dev_get_priv(mmc->dev);+#else
return (struct omap_hsmmc_data *)mmc->priv;+#endif +}
static int omap_hsmmc_init_setup(struct mmc *mmc) {
struct omap_hsmmc_data *priv = omap_hsmmc_get_data(mmc); struct hsmmc *mmc_base; unsigned int reg_val; unsigned int dsor; ulong start;
mmc_base = ((struct omap_hsmmc_data *)mmc->priv)->base_addr;
mmc_base = priv->base_addr; mmc_board_init(mmc); writel(readl(&mmc_base->sysconfig) | MMC_SOFTRESET,@@ -284,11 +300,12 @@ static void mmc_reset_controller_fsm(struct hsmmc *mmc_base, u32 bit) static int omap_hsmmc_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd, struct mmc_data *data) {
struct omap_hsmmc_data *priv = omap_hsmmc_get_data(mmc); struct hsmmc *mmc_base; unsigned int flags, mmc_stat; ulong start;
mmc_base = ((struct omap_hsmmc_data *)mmc->priv)->base_addr;
mmc_base = priv->base_addr; start = get_timer(0); while ((readl(&mmc_base->pstate) & (DATI_MASK | CMDI_MASK)) != 0) { if (get_timer(0) - start > MAX_RETRY_MS) {@@ -513,11 +530,12 @@ static int mmc_write_data(struct hsmmc *mmc_base, const char *buf,
static int omap_hsmmc_set_ios(struct mmc *mmc) {
struct omap_hsmmc_data *priv = omap_hsmmc_get_data(mmc); struct hsmmc *mmc_base; unsigned int dsor = 0; ulong start;
mmc_base = ((struct omap_hsmmc_data *)mmc->priv)->base_addr;
mmc_base = priv->base_addr; /* configue bus width */ switch (mmc->bus_width) { case 8:@@ -571,7 +589,7 @@ static int omap_hsmmc_set_ios(struct mmc *mmc) #ifdef CONFIG_DM_MMC static int omap_hsmmc_getcd(struct mmc *mmc) {
struct omap_hsmmc_data *priv = mmc->priv;
struct omap_hsmmc_data *priv = omap_hsmmc_get_data(mmc); int value; value = dm_gpio_get_value(&priv->cd_gpio);@@ -586,7 +604,7 @@ static int omap_hsmmc_getcd(struct mmc *mmc)
static int omap_hsmmc_getwp(struct mmc *mmc) {
struct omap_hsmmc_data *priv = mmc->priv;
struct omap_hsmmc_data *priv = omap_hsmmc_get_data(mmc); int value; value = dm_gpio_get_value(&priv->wp_gpio);@@ -727,6 +745,7 @@ int omap_mmc_init(int dev_index, uint host_caps_mask, uint f_max, int cd_gpio, static int omap_hsmmc_ofdata_to_platdata(struct udevice *dev) { struct omap_hsmmc_data *priv = dev_get_priv(dev);
struct omap_hsmmc_plat *plat = dev_get_platdata(dev); const void *fdt = gd->fdt_blob; int node = dev->of_offset; struct mmc_config *cfg;@@ -734,7 +753,7 @@ static int omap_hsmmc_ofdata_to_platdata(struct udevice *dev)
priv->base_addr = map_physmem(dev_get_addr(dev), sizeof(struct hsmmc *), MAP_NOCACHE);
cfg = &priv->cfg;
cfg = &plat->cfg; cfg->host_caps = MMC_MODE_HS_52MHz | MMC_MODE_HS; val = fdtdec_get_int(fdt, node, "bus-width", -1);@@ -766,29 +785,33 @@ static int omap_hsmmc_ofdata_to_platdata(struct udevice *dev) return 0; }
+static int omap_hsmmc_bind(struct udevice *dev) +{
struct omap_hsmmc_plat *plat = dev_get_platdata(dev);return mmc_bind(dev, &plat->mmc, &plat->cfg);+}
static int omap_hsmmc_probe(struct udevice *dev) {
struct omap_hsmmc_plat *plat = dev_get_platdata(dev); struct mmc_uclass_priv *upriv = dev_get_uclass_priv(dev); struct omap_hsmmc_data *priv = dev_get_priv(dev); struct mmc_config *cfg;
struct mmc *mmc;cfg = &priv->cfg;
cfg = &plat->cfg; cfg->name = "OMAP SD/MMC"; cfg->ops = &omap_hsmmc_ops;
mmc = mmc_create(cfg, priv);if (mmc == NULL)return -1;#ifdef OMAP_HSMMC_USE_GPIO gpio_request_by_name(dev, "cd-gpios", 0, &priv->cd_gpio, GPIOD_IS_IN); gpio_request_by_name(dev, "wp-gpios", 0, &priv->wp_gpio, GPIOD_IS_IN); #endif
mmc->dev = dev;upriv->mmc = mmc;
upriv->mmc = &plat->mmc;printf("%s", plat->mmc.dev->name); return 0;}
@@ -804,7 +827,9 @@ U_BOOT_DRIVER(omap_hsmmc) = { .id = UCLASS_MMC, .of_match = omap_hsmmc_ids, .ofdata_to_platdata = omap_hsmmc_ofdata_to_platdata,
.bind = omap_hsmmc_bind, .probe = omap_hsmmc_probe, .priv_auto_alloc_size = sizeof(struct omap_hsmmc_data),.platdata_auto_alloc_size = sizeof(struct omap_hsmmc_plat),}; #endif diff --git a/include/configs/am335x_evm.h b/include/configs/am335x_evm.h index 7390015..6368fb8 100644 --- a/include/configs/am335x_evm.h +++ b/include/configs/am335x_evm.h @@ -285,6 +285,7 @@ */ #ifdef CONFIG_SPL_BUILD #undef CONFIG_DM_MMC +#undef CONFIG_BLK #undef CONFIG_TIMER #undef CONFIG_DM_USB
#undef CONFIG_DM_NAND
2.10.1.dirty
On 02/10/2017 10:21 AM, Simon Glass wrote:
Hi,
On 2 February 2017 at 15:18, Grygorii Strashko grygorii.strashko@ti.com wrote:
Convert OMAP hsmmc driver to use driver-model block devices.
Signed-off-by: Grygorii Strashko grygorii.strashko@ti.com
Hi All,
First of all, sorry if my questions/problems are looks dumb, I'm very new with u-boot.
This is my attampt to enable CONFIG_BLK on OMAP platforms which is blocked now by omap_hsmmc driver. omap_hsmmc required to be updated to use driver-model block devices at minimum (and max to use dm_mmc_ops). Also, as per my understanding, CONFIG_BLK is blocker for other tasks like enabling driver model for OMAP sata devices.
With this patch I can boot from mmc on am335x-evm, but there are two problems I need help with:
- My changes in Makefiles looks really ugly, but without them SPL build will
fail because undef'ing in include/configs/am335x_evm.h does not take effect in Makefile (thanks Vignesh for the information [1]) and I, honestly, do not know how to fix it in better way, so I'd be appreciated for any help. Comparing to Vignesh's case, files which need to be excluded from build are generic and I worry that there can be dependecy from CONFIG_SPL_DM.
It would be great to enable CONFIG_BLK and CONFIG_DM_MMC in SPL. Perhaps Tom's message (later in this thread) about the gcc bug might help. The size difference is small, but not 0 unfortunately.
- with this patch I can see error message in log "** Bad device size - mmc 0 **":
U-Boot 2017.03-rc1-00020-g1a3b11a-dirty (Feb 02 2017 - 12:04:01 -0600)
CPU : AM335X-GP rev 2.0 Model: TI AM335x EVM DRAM: 1 GiB NAND: 256 MiB MMC: mmc@48060000mmc@47810000OMAP SD/MMC: 0, OMAP SD/MMC: 1 ** Bad device size - mmc 0 ** Using default environment
this message is triggered from: board_r.c: initr_env()
- env_common.c: env_relocate()
- env_fat.c: env_relocate_spec()
- part.c: blk_get_device_part_str()
because partitions are not initialized yet, as i understood,
Seems there are should be additional call to mmc_init():mmc_startup():part_init(), but I'm not sure where/how to add it correctly.
Does this help?
It seems like issue with "Bad device size..." was fixed by patch "mmc: init mmc block devices on probe" [1].
This patch (my patch) with your patch [2] cause boot hang on am335x-evm when mmc1 (mmc3 in DT) is probed (i've tried to revert [1] - result is the same).
[1] https://patchwork.ozlabs.org/patch/719652/ [2] http://patchwork.ozlabs.org/patch/717710/
participants (5)
-
Andrew F. Davis -
Grygorii Strashko -
Simon Glass -
Tom Rini -
Vignesh R