[U-Boot] [PATCH v2 4/6] am33xx: Provide platform data for mmc

[Adam Ford <aford173@gmail.com>]

On Wed, Apr 26, 2017 at 3:07 AM, Lokesh Vutla <lokeshvutla@ti.com> wrote:
> Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com>
> ---
> Changes since v1:
> - Update base addresses for MMC0 and MMC1
>  board/ti/am335x/board.c | 32 ++++++++++++++++++++++++++++++++
>  1 file changed, 32 insertions(+)
>
> diff --git a/board/ti/am335x/board.c b/board/ti/am335x/board.c
> index 3e842d3187..6786229680 100644
> --- a/board/ti/am335x/board.c
> +++ b/board/ti/am335x/board.c
> @@ -9,6 +9,7 @@
>   */
>
>  #include <common.h>
> +#include <dm.h>
>  #include <errno.h>
>  #include <spl.h>
>  #include <serial.h>
> @@ -26,6 +27,7 @@
>  #include <asm/emif.h>
>  #include <asm/gpio.h>
>  #include <asm/omap_sec_common.h>
> +#include <asm/omap_mmc.h>
>  #include <i2c.h>
>  #include <miiphy.h>
>  #include <cpsw.h>
> @@ -892,3 +894,33 @@ void board_fit_image_post_process(void **p_image, size_t *p_size)
>         secure_boot_verify_image(p_image, p_size);
>  }
>  #endif
> +
> +#if !CONFIG_IS_ENABLED(OF_CONTROL)
> +static const struct omap_hsmmc_plat am335x_mmc0_platdata = {
> +       .base_addr = (struct hsmmc *)OMAP_HSMMC1_BASE,
> +       .cfg.host_caps = MMC_MODE_HS_52MHz | MMC_MODE_HS | MMC_MODE_4BIT,
> +       .cfg.f_min = 400000,
> +       .cfg.f_max = 52000000,
> +       .cfg.voltages = MMC_VDD_32_33 | MMC_VDD_33_34 | MMC_VDD_165_195,
> +       .cfg.b_max = CONFIG_SYS_MMC_MAX_BLK_COUNT,
> +};

Do you need to address the offset factor here if you're going to
rebase from my series?  My series removed the offset from struct hsmmc
because I assummed (maybe wrongly) that everyone who was using DM_MMC
was using OF_CONFIG.  Since I assumed that people where using
OF_CONFIG, I removed the offset from the structure and moved it
elsewhere.

 struct hsmmc {
-#ifdef CONFIG_DM_MMC
- unsigned char res0[0x100];
-#endif

If you're going under the assumption that people might use DM_MMC and
without OF_CONTROL, I am concerned this may break because the offset
was removed.  I think that may be what Tony was trying to say.

If we update my patch to put the offset back, when OF_CONTROL is not
present, then it may break the functionality of OMAP3 users want to
use DM_MMC without OF_CONTROL which means, then the ifdef would grow.

struct hsmmc {
#if (defined(CONFIG_DM_MMC) && !defined(CONFIG_OF_CONTROL) &&
!defined(CONFIG_OMAP34XX))
unsigned char res0[0x100];
#endif

And that gets really ugly and part of what I was trying to avoid.

I wonder if we can somehow add the appropriate offset here like

 .base_addr = (struct hsmmc *) (OMAP_HSMMC1_BASE + 0x100)

I am just thinking out loud, so forgive the excessive chatter, but
since I don't have am33xx hardware, I can only give my ideas without
testing.


> +
> +U_BOOT_DEVICE(am335x_mmc0) = {
> +       .name = "omap_hsmmc",
> +       .platdata = &am335x_mmc0_platdata,
> +};
> +
> +static const struct omap_hsmmc_plat am335x_mmc1_platdata = {
> +       .base_addr = (struct hsmmc *)OMAP_HSMMC2_BASE,

See above on base_addr.

> +       .cfg.host_caps = MMC_MODE_HS_52MHz | MMC_MODE_HS | MMC_MODE_8BIT,
> +       .cfg.f_min = 400000,
> +       .cfg.f_max = 52000000,
> +       .cfg.voltages = MMC_VDD_32_33 | MMC_VDD_33_34 | MMC_VDD_165_195,
> +       .cfg.b_max = CONFIG_SYS_MMC_MAX_BLK_COUNT,
> +};
> +
> +U_BOOT_DEVICE(am335x_mmc1) = {
> +       .name = "omap_hsmmc",
> +       .platdata = &am335x_mmc1_platdata,
> +};
> +#endif
> --
> 2.11.0
>
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> https://lists.denx.de/listinfo/u-boot