[U-Boot] [PATCH v3 2/2] omap_hsmmc: Board-specific TWL4030 MMC power initializations
Igor Grinberg
grinberg at compulab.co.il
Sun Nov 2 14:40:00 CET 2014
Hi Paul,
On 11/01/14 12:52, Paul Kocialkowski wrote:
> Boards using the TWL4030 regulator may not all use the LDOs the same way
> (e.g. MMC2 power can be controlled by another LDO than VMMC2).
> This delegates TWL4030 MMC power initializations to board-specific functions,
> that may still call twl4030_power_mmc_init for the default behavior.
>
> Signed-off-by: Paul Kocialkowski <contact at paulk.fr>
This mostly looks good, several suggestions below though.
> ---
> board/comelit/dig297/dig297.c | 9 +++++++++
> board/compulab/cm_t35/cm_t35.c | 11 +++++++++++
> board/corscience/tricorder/tricorder.c | 11 +++++++++++
> board/isee/igep00x0/igep00x0.c | 11 +++++++++++
> board/logicpd/omap3som/omap3logic.c | 11 +++++++++++
> board/logicpd/zoom1/zoom1.c | 9 +++++++++
> board/matrix_vision/mvblx/mvblx.c | 9 +++++++++
> board/nokia/rx51/rx51.c | 9 +++++++++
> board/overo/overo.c | 11 +++++++++++
> board/pandora/pandora.c | 9 +++++++++
> board/technexion/tao3530/tao3530.c | 11 +++++++++++
> board/ti/beagle/beagle.c | 11 +++++++++++
> board/ti/evm/evm.c | 11 +++++++++++
> board/ti/sdp3430/sdp.c | 9 +++++++++
> board/timll/devkit8000/devkit8000.c | 11 +++++++++++
> drivers/mmc/omap_hsmmc.c | 7 +------
> 16 files changed, 154 insertions(+), 6 deletions(-)
>
> diff --git a/board/comelit/dig297/dig297.c b/board/comelit/dig297/dig297.c
> index 2b826df..784483b 100644
> --- a/board/comelit/dig297/dig297.c
> +++ b/board/comelit/dig297/dig297.c
> @@ -133,6 +133,15 @@ int board_mmc_init(bd_t *bis)
> {
> return omap_mmc_init(0, 0, 0, -1, -1);
> }
> +
> +int board_mmc_power_init(void)
> +{
> +#if defined(CONFIG_TWL4030_POWER)
> + twl4030_power_mmc_init();
> + mdelay(100); /* ramp-up delay from Linux code */
I guess all twl4030 based boards have to have this ramp up delay, right?
If so, why don't we want to hide it inside the twl4030_power_mmc_init()
function and not spread it across all boards?
> +#endif
> + return 0;
> +}
Also, what do you think of the below suggestion:
Leave the twl4030_power_mmc_init() call inside the omap_hsmmc.c
(as it seems that many boards want it) as it currently is.
This way you will not need to patch each board file, yet a board
has the ability to control that call via the CONFIG_TWL4030_POWER
and have a board specific callback should it need one.
I would also change envelope the call to twl4030_power_mmc_init()
to something like CONFIG_TWL4030_MMC_POWER instead of
CONFIG_TWL4030_POWER, so it will be more flexible, but that has
little to do with this patch.
> #endif
>
> #ifdef CONFIG_CMD_NET
[...]
> diff --git a/drivers/mmc/omap_hsmmc.c b/drivers/mmc/omap_hsmmc.c
> index ef2cbf9..6fb78b3 100644
> --- a/drivers/mmc/omap_hsmmc.c
> +++ b/drivers/mmc/omap_hsmmc.c
> @@ -135,12 +135,7 @@ static unsigned char mmc_board_init(struct mmc *mmc)
> pbias_lite = readl(&t2_base->pbias_lite);
> pbias_lite &= ~(PBIASLITEPWRDNZ1 | PBIASLITEPWRDNZ0);
> writel(pbias_lite, &t2_base->pbias_lite);
> -#endif
> -#if defined(CONFIG_TWL4030_POWER)
> - twl4030_power_mmc_init();
> - mdelay(100); /* ramp-up delay from Linux code */
> -#endif
> -#if defined(CONFIG_OMAP34XX)
> +
> writel(pbias_lite | PBIASLITEPWRDNZ1 |
> PBIASSPEEDCTRL0 | PBIASLITEPWRDNZ0,
> &t2_base->pbias_lite);
>
--
Regards,
Igor.
More information about the U-Boot
mailing list