omap_hsmmc: Allow for board-specific MMC power init
Subsystems (2)
PATCH 1/1 omap_hsmmc: Allow for board-specific MMC power init
Some devices may use non-standard combinations of regulators to power MMC:
this allows these devices to provide a board-specific MMC power init function to set everything up in their own way.
--- arch/arm/include/asm/omap_mmc.h | 4 +++- drivers/mmc/omap_hsmmc.c | 4 +++- 2 files changed, 6 insertions(+), 2 deletions(-)
arch/arm/include/asm/omap_mmc.h +3 -1
@@ -164,5 +164,7 @@ struct hsmmc {int omap_mmc_init (int dev_index,uint host_caps_mask,uint f_max,int cd_gpio,int wp_gpio); - +#ifdef CONFIG_OMAP_HSMMC_BOARD_POWER_INIT +void omap_hsmmc_board_power_init (void ); +#endif #endif /* OMAP_MMC_H_ */
drivers/mmc/omap_hsmmc.c +3 -1
@@ -136,7 +136,9 @@ static unsigned char mmc_board_init(struct mmc *mmc) pbias_lite &= ~(PBIASLITEPWRDNZ1 |PBIASLITEPWRDNZ0 );writel (pbias_lite, &t2_base->pbias_lite);#endif -#if defined(CONFIG_TWL4030_POWER ) +#if defined(CONFIG_OMAP_HSMMC_BOARD_POWER_INIT ) +omap_hsmmc_board_power_init (); +#elif defined(CONFIG_TWL4030_POWER )twl4030_power_mmc_init ();mdelay (100 );/* ramp-up delay from Linux code */ #endif
Signature · 1 line
1.7.9.5
PATCH 1/1 omap_hsmmc: Allow for board-specific MMC power init
Hi Paul,
Quoted 15 lines · Paul Kocialkowski
Some devices may use non-standard combinations of regulators to power MMC:
this allows these devices to provide a board-specific MMC power init function to set everything up in their own way.Signed-off-by Paul Kocialkowski <[email protected]>--- arch/arm/include/asm/omap_mmc.h | 4 +++- drivers/mmc/omap_hsmmc.c | 4 +++- 2 files changed, 6 insertions(+), 2 deletions(-)arch/arm/include/asm/omap_mmc.h +0
-0
arch/arm/include/asm/omap_mmc.h struct hsmmc { · line 167
Quoted 5 lines
arch/arm/include/asm/omap_mmc.h +1
-1@@ -164,5 +164,7 @@ struct hsmmc {int omap_mmc_init (int dev_index,uint host_caps_mask,uint f_max,int cd_gpio,int wp_gpio); - +#ifdef CONFIG_OMAP_HSMMC_BOARD_POWER_INIT
I'm not a huge fan of that approach, but if you add yet another CONFIG_ option, I think it is a requirement to add a documentation for it.
arch/arm/include/asm/omap_mmc.h struct hsmmc { · line 168
Quoted 1 line
arch/arm/include/asm/omap_mmc.h +1
-0+void omap_hsmmc_board_power_init (void );
Anyway, I would suggest adding a default__weak board_mmc_power_init() or something like this
(which would be transfered into a callback in pdata once omap_hsmmc.c is).
Or... just no need for this patch at all, as board_mmc_init() can be used for this...
Quoted 18 lines
arch/arm/include/asm/omap_mmc.h +1
-0+#endif #endif /* OMAP_MMC_H_ */ drivers/mmc/omap_hsmmc.c +3
-1@@ -136,7 +136,9 @@ static unsigned char mmc_board_init(struct mmc *mmc) pbias_lite &= ~(PBIASLITEPWRDNZ1 |PBIASLITEPWRDNZ0 );writel (pbias_lite, &t2_base->pbias_lite);#endif -#if defined(CONFIG_TWL4030_POWER ) +#if defined(CONFIG_OMAP_HSMMC_BOARD_POWER_INIT ) +omap_hsmmc_board_power_init (); +#elif defined(CONFIG_TWL4030_POWER )twl4030_power_mmc_init ();mdelay (100 );/* ramp-up delay from Linux code */ #endif
Signature · 2 lines
Regards, Igor.
PATCH 1/1 omap_hsmmc: Allow for board-specific MMC power init
Le mardi 28 octobre 2014 ? 20:02 +0200, Igor Grinberg a ?crit :
Quoted 21 lines
Hi Paul,
Quoted 18 lines · Paul Kocialkowski
Some devices may use non-standard combinations of regulators to power MMC:
this allows these devices to provide a board-specific MMC power init function to set everything up in their own way.Signed-off-by Paul Kocialkowski <[email protected]>--- arch/arm/include/asm/omap_mmc.h | 4 +++- drivers/mmc/omap_hsmmc.c | 4 +++- 2 files changed, 6 insertions(+), 2 deletions(-)arch/arm/include/asm/omap_mmc.h +0
-0@@ -164,5 +164,7 @@ struct hsmmc {int omap_mmc_init (int dev_index,uint host_caps_mask,uint f_max,int cd_gpio,int wp_gpio);
Quoted 6 lines
arch/arm/include/asm/omap_mmc.h struct hsmmc { · line 167
Quoted 2 lines
arch/arm/include/asm/omap_mmc.h +1
-1- +#ifdef CONFIG_OMAP_HSMMC_BOARD_POWER_INIT I'm not a huge fan of that approach, but if you add yet another
CONFIG_option, I think it is a requirement to add a documentation for it.
That saddens me too, but I don't see how to do this in a better way for now.
Quoted 5 lines
arch/arm/include/asm/omap_mmc.h struct hsmmc { · line 168
Quoted 1 line
arch/arm/include/asm/omap_mmc.h +1
-0+void omap_hsmmc_board_power_init (void );Anyway, I would suggest adding a default
__weakboard_mmc_power_init()or something like this
(which would be transfered into a callback in pdata onceomap_hsmmc.cis).
Well, there are two distinct scenarios here:
1. the board follows some reference design and VMMC1 is used to power MMC1, VMMC2 for MMC2, then twl4030 has a function that will set everything up correctly (with the addition I added earlier)
2. the board uses the regulators in a more or less random way and VMMC1 is not mapped to MMC1 power at all. This is the case on the LG Optimus Black (P970). There is actually an auxiliary regulator IC that is in charge of powering MMC1. Hence the need for board-specific behavior.
I think it's perfectly fine to keep using the twl4030 function when the board follows the reference design.
Would that board_mmc_power_init you suggest be generic to all MMC drivers? Then at which point should it be called? What would a generic board_mmc_power_init have? TWL4030 is specific to OMAP3. Then perhaps there should be a board_mmc_power_init implementation for OMAP3, that can be overridden by a board-specific function?
I'm a bit new to the U-Boot code, so I'd appreciate pointers on what should go where.
Quoted 2 lines
Or... just no need for this patch at all, as
board_mmc_init()can be used for this...
Not in the context of the SPL, which is precisely why I needed this.
Thanks for your review!
Quoted 10 lines
Quoted 10 lines
arch/arm/include/asm/omap_mmc.h +1
-0+#endif #endif /* OMAP_MMC_H_ */ drivers/mmc/omap_hsmmc.c +0
-0@@ -136,7 +136,9 @@ static unsigned char mmc_board_init(struct mmc *mmc) pbias_lite &= ~(PBIASLITEPWRDNZ1 |PBIASLITEPWRDNZ0 );writel (pbias_lite, &t2_base->pbias_lite);#endif
Quoted 9 lines
drivers/mmc/omap_hsmmc.c static unsigned char mmc_board_init(struct mmc *mmc) · line 144
Quoted 8 lines
drivers/mmc/omap_hsmmc.c +3
-1-#if defined(CONFIG_TWL4030_POWER ) +#if defined(CONFIG_OMAP_HSMMC_BOARD_POWER_INIT ) +omap_hsmmc_board_power_init (); +#elif defined(CONFIG_TWL4030_POWER )twl4030_power_mmc_init ();mdelay (100 );/* ramp-up delay from Linux code */ #endif
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20141028/48fbfb6a/attachment.pgp>
PATCH 1/1 omap_hsmmc: Allow for board-specific MMC power init
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Hi Paul,
Quoted 47 lines · Paul Kocialkowski
Le mardi 28 octobre 2014 ? 20:02 +0200, Igor Grinberg a ?crit :
Quoted 21 lines
Hi Paul,
Quoted 18 lines · Paul Kocialkowski
Some devices may use non-standard combinations of regulators to power MMC:
this allows these devices to provide a board-specific MMC power init function to set everything up in their own way.Signed-off-by Paul Kocialkowski <[email protected]>--- arch/arm/include/asm/omap_mmc.h | 4 +++- drivers/mmc/omap_hsmmc.c | 4 +++- 2 files changed, 6 insertions(+), 2 deletions(-)arch/arm/include/asm/omap_mmc.h +0
-0@@ -164,5 +164,7 @@ struct hsmmc {int omap_mmc_init (int dev_index,uint host_caps_mask,uint f_max,int cd_gpio,int wp_gpio);Quoted 6 lines
arch/arm/include/asm/omap_mmc.h struct hsmmc { · line 167
Quoted 2 lines
arch/arm/include/asm/omap_mmc.h +1
-1- +#ifdef CONFIG_OMAP_HSMMC_BOARD_POWER_INIT I'm not a huge fan of that approach, but if you add yet another
CONFIG_option, I think it is a requirement to add a documentation for it.That saddens me too, but I don't see how to do this in a better way for now.
Quoted 5 lines
arch/arm/include/asm/omap_mmc.h struct hsmmc { · line 168
Quoted 1 line
arch/arm/include/asm/omap_mmc.h +1
-0+void omap_hsmmc_board_power_init (void );Anyway, I would suggest adding a default
__weakboard_mmc_power_init()or something like this
(which would be transfered into a callback in pdata onceomap_hsmmc.cis).Well, there are two distinct scenarios here:
1. the board follows some reference design and VMMC1 is used to power MMC1, VMMC2 for MMC2, then twl4030 has a function that will set everything up correctly (with the addition I added earlier)
2. the board uses the regulators in a more or less random way and VMMC1 is not mapped to MMC1 power at all. This is the case on the LG Optimus Black (P970). There is actually an auxiliary regulator IC that is in charge of powering MMC1. Hence the need for board-specific behavior.
Quoted 5 lines
I think it's perfectly fine to keep using the twl4030 function when the board follows the reference design.
Would that
board_mmc_power_inityou suggest be generic to all MMC drivers?
That would be perfect, I think.
Quoted 2 lines
Then at which point should it be called? What would a generic
board_mmc_power_inithave?
Well, those are the question you usually should answer when you write common code. I hopefully will have time to look at this next week.
Quoted 1 line
TWL4030 is specific to OMAP3.
Well, in practice, it is, but it does not have to be... Also, it is a question of how the board is wired...
Quoted 3 lines
Then perhaps
there should be aboard_mmc_power_initimplementation for OMAP3, that can be overridden by a board-specific function?
Or, as you have proposed already, a common board_mmc_power_init() function and a board can choose to run the twl4030_power_mmc_init()?
Quoted 8 lines
I'm a bit new to the U-Boot code, so I'd appreciate pointers on what should go where.
Quoted 2 lines
Or... just no need for this patch at all, as
board_mmc_init()can be used for this...Not in the context of the SPL, which is precisely why I needed this.
This might lead to a question - is current spl flexible enough to let a board deal with the mmc power?
Tom?
Quoted 23 lines
Thanks for your review!
Quoted 19 lines
Quoted 18 lines
arch/arm/include/asm/omap_mmc.h +1
-0+#endif #endif /* OMAP_MMC_H_ */ drivers/mmc/omap_hsmmc.c +3
-1@@ -136,7 +136,9 @@ static unsigned char mmc_board_init(struct mmc *mmc) pbias_lite &= ~(PBIASLITEPWRDNZ1 |PBIASLITEPWRDNZ0 );writel (pbias_lite, &t2_base->pbias_lite);#endif -#if defined(CONFIG_TWL4030_POWER ) +#if defined(CONFIG_OMAP_HSMMC_BOARD_POWER_INIT ) +omap_hsmmc_board_power_init (); +#elif defined(CONFIG_TWL4030_POWER )twl4030_power_mmc_init ();mdelay (100 );/* ramp-up delay from Linux code */ #endif
- --
Regards,
Igor.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2
iQIcBAEBAgAGBQJUUOcyAAoJEBDE8YO64EfaA00P/RRjiz3AyVqGJdVcN85eRim4 oYxr2/q5jaj67zD6x6UD4eT/VSnJQXaxNEmFQVNjoZc4U6U0V7qjrTC2/USh6v6Q GROIH7zK/uP9+rRnJWJYdjAjZMJNShNmUvP9/oIb8ZtvVKG1aEuRoRWYwlyirJr/ cmtzeFS/TwaWSgFrtDWmCXuXzyKUdB+A/AOjAIj7HMZjKV+gaSZMSSyv5H6uQK+3 eI7KUskYvTJFCKKez0jnXv3IkhKjeJxy7eKQ147nfQMcZJ+dby3cbJuS3aZSG2xI jx+w6ruSm4DMYJeCsEoWhW1wFJtKrtqZ4TyYEQeC9PWoON/8b67Jy+SYTHnVBkJk qR7UFEyttUIE6zulD5rf2SlxS0e96HkLqNOfaI2K8rqbKuNOy+ZlA8TKsaVx5QAH Fsxn494TY38vgBTYQbf74pv7psw2mPT//FUF//HqTuAID9ne6pMgM+j+WE0GwWOI CZw9ddutxj1ksFu0kvnldvfZ6F4yd4aJFPUPQEDa6/qkVrKEW7Y3Alov5gHQMHQ8 zkhyv60s3owlmtcvkRtmb4LcRh+/lm8+gO9u7jmFmtiIhkCuEQA7IopqqYRdibXC SiKyGH53TTLg/zocixO1/vY/a5C2YHADaVJ6Rzzea6IDUeHBoWTKhU4vUudjOb5z NCH2ncJjDN8moX9CBENr
=3a3G
-----END PGP SIGNATURE-----
PATCH 1/1 omap_hsmmc: Allow for board-specific MMC power init
In reply to Paul KocialkowskiHi Paul,
Quoted 43 lines · Paul Kocialkowski
Some devices may use non-standard combinations of regulators to power MMC:
this allows these devices to provide a board-specific MMC power init function to set everything up in their own way.Signed-off-by Paul Kocialkowski <[email protected]>---
arch/arm/include/asm/omap_mmc.h | 4 +++- drivers/mmc/omap_hsmmc.c | 4 +++- 2 files changed, 6 insertions(+), 2 deletions(-)arch/arm/include/asm/omap_mmc.h +3
-1@@ -164,5 +164,7 @@ struct hsmmc {int omap_mmc_init (int dev_index,uint host_caps_mask,uint f_max,int cd_gpio,int wp_gpio); - +#ifdef CONFIG_OMAP_HSMMC_BOARD_POWER_INIT +void omap_hsmmc_board_power_init (void ); +#endif #endif /* OMAP_MMC_H_ */ drivers/mmc/omap_hsmmc.c +3
-1@@ -136,7 +136,9 @@ static unsigned char mmc_board_init(struct mmc *mmc) pbias_lite &= ~(PBIASLITEPWRDNZ1 |PBIASLITEPWRDNZ0 );writel (pbias_lite, &t2_base->pbias_lite);#endif -#if defined(CONFIG_TWL4030_POWER ) +#if defined(CONFIG_OMAP_HSMMC_BOARD_POWER_INIT ) +omap_hsmmc_board_power_init (); +#elif defined(CONFIG_TWL4030_POWER )twl4030_power_mmc_init ();mdelay (100 );/* ramp-up delay from Linux code */ #endif Signature · 1 line
1.7.9.5
Igor covered most of what I was going to whine about.
I?m not a big fun of the define maze here, let?s not add to it, shall we?
A common board_mmc_power_init() function would be fine in this occasion.
Regards
? Pantelis
PATCH v2 1/2 mmc: Board-specific MMC power initializations
In reply to Paul KocialkowskiSome devices may use non-standard combinations of regulators to power MMC:
this allows these devices to provide a board-specific MMC power init function to set everything up in their own way.
--- drivers/mmc/mmc.c | 8 ++++++++ include/mmc.h | 2 ++ 2 files changed, 10 insertions(+)
drivers/mmc/mmc.c +8 -0
@@ -1277,6 +1277,12 @@ block_dev_desc_t *mmc_get_dev(int dev) }#endif +/* board-specific MMC power initializations. */ +__weak int board_mmc_power_init (void ) +{ +return -1 ; +} +int mmc_start_init (struct mmc *mmc) {int err;
@@ -1293,6 +1299,8 @@ int mmc_start_init(struct mmc *mmc)if (mmc->has_init)return 0 ; +board_mmc_power_init (); +/* made sure it's not NULL earlier */ err = mmc->cfg->ops->init (mmc);
include/mmc.h +2 -0
@@ -388,6 +388,8 @@ int mmc_legacy_init(int verbose);int board_mmc_init (bd_t *bis);int cpu_mmc_init (bd_t *bis); +int board_mmc_power_init (void ); +/* Set block count limit because of 16 bit register limit on some hardware*/ #ifndef CONFIG_SYS_MMC_MAX_BLK_COUNT #define CONFIG_SYS_MMC_MAX_BLK_COUNT 65535
Signature · 1 line
1.9.1
PATCH v2 2/2 omap_hsmmc: Board-specific TWL4030 MMC power initializations
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.
--- 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(-)
board/comelit/dig297/dig297.c +9 -0
@@ -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 */ +#endif +return 0 ; +}#endif #ifdef CONFIG_CMD_NET
board/compulab/cm_t35/cm_t35.c +11 -0
@@ -473,6 +473,17 @@ int board_mmc_init(bd_t *bis) }#endif +#if defined(CONFIG_GENERIC_MMC ) +int board_mmc_power_init (void ) +{ +#if defined(CONFIG_TWL4030_POWER ) +twl4030_power_mmc_init (); +mdelay (100 );/* ramp-up delay from Linux code */ +#endif +return 0 ; +} +#endif +/* * Routine: setup_net_chip_gmpc * Description: Setting up the configuration GPMC registers specific to the
board/corscience/tricorder/tricorder.c +11 -0
@@ -147,6 +147,17 @@ int board_mmc_init(bd_t *bis) }#endif +#if defined(CONFIG_GENERIC_MMC ) +int board_mmc_power_init (void ) +{ +#if defined(CONFIG_TWL4030_POWER ) +twl4030_power_mmc_init (); +mdelay (100 );/* ramp-up delay from Linux code */ +#endif +return 0 ; +} +#endif +/* * Routine: get_board_mem_timings * Description: If we use SPL then there is no x-loader nor config header
board/isee/igep00x0/igep00x0.c +11 -0
@@ -137,6 +137,17 @@ int board_mmc_init(bd_t *bis) }#endif +#if defined(CONFIG_GENERIC_MMC ) +int board_mmc_power_init (void ) +{ +#if defined(CONFIG_TWL4030_POWER ) +twl4030_power_mmc_init (); +mdelay (100 );/* ramp-up delay from Linux code */ +#endif +return 0 ; +} +#endif +void set_fdt (void ) {switch (gd->bd->bi_arch_number) {
board/logicpd/omap3som/omap3logic.c +11 -0
@@ -128,6 +128,17 @@ int board_mmc_init(bd_t *bis) }#endif +#if defined(CONFIG_GENERIC_MMC ) +int board_mmc_power_init (void ) +{ +#if defined(CONFIG_TWL4030_POWER ) +twl4030_power_mmc_init (); +mdelay (100 );/* ramp-up delay from Linux code */ +#endif +return 0 ; +} +#endif +#ifdef CONFIG_SMC911X /* GPMC CS1 settings for Logic SOM LV/Torpedo LAN92xx Ethernet chip */ static const u32 gpmc_lan92xx_config[] = {
board/logicpd/zoom1/zoom1.c +9 -0
@@ -96,6 +96,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 */ +#endif +return 0 ; +}#endif #ifdef CONFIG_CMD_NET
board/matrix_vision/mvblx/mvblx.c +9 -0
@@ -94,6 +94,15 @@ int board_mmc_init(bd_t *bis)omap_mmc_init (1 ,0 ,0 ,-1 ,-1 ); return 0; } + +int board_mmc_power_init (void ) +{ +#if defined(CONFIG_TWL4030_POWER ) +twl4030_power_mmc_init (); +mdelay (100 );/* ramp-up delay from Linux code */ +#endif +return 0 ; +}#endif #if defined(CONFIG_CMD_NET )
board/nokia/rx51/rx51.c +9 -0
@@ -659,3 +659,12 @@ int board_mmc_init(bd_t *bis)omap_mmc_init (1 ,0 ,0 ,-1 ,-1 );return 0 ; } + +int board_mmc_power_init (void ) +{ +#if defined(CONFIG_TWL4030_POWER ) +twl4030_power_mmc_init (); +mdelay (100 );/* ramp-up delay from Linux code */ +#endif +return 0 ; +}
board/overo/overo.c +11 -0
@@ -437,3 +437,14 @@ int board_mmc_init(bd_t *bis) return omap_mmc_init(0, 0, 0, -1, -1); }#endif + +#if defined(CONFIG_GENERIC_MMC ) +int board_mmc_power_init (void ) +{ +#if defined(CONFIG_TWL4030_POWER ) +twl4030_power_mmc_init (); +mdelay (100 );/* ramp-up delay from Linux code */ +#endif +return 0 ; +} +#endif
board/pandora/pandora.c +9 -0
@@ -126,4 +126,13 @@ 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 */ +#endif +return 0 ; +}#endif
board/technexion/tao3530/tao3530.c +11 -0
@@ -188,6 +188,17 @@ int board_mmc_init(bd_t *bis) }#endif +#if defined(CONFIG_GENERIC_MMC ) +int board_mmc_power_init (void ) +{ +#if defined(CONFIG_TWL4030_POWER ) +twl4030_power_mmc_init (); +mdelay (100 );/* ramp-up delay from Linux code */ +#endif +return 0 ; +} +#endif +#if defined(CONFIG_USB_EHCI ) && !defined(CONFIG_SPL_BUILD )/* Call usb_stop() before starting the kernel */ void show_boot_progress (int val)
board/ti/beagle/beagle.c +11 -0
@@ -521,6 +521,17 @@ int board_mmc_init(bd_t *bis) }#endif +#if defined(CONFIG_GENERIC_MMC ) +int board_mmc_power_init (void ) +{ +#if defined(CONFIG_TWL4030_POWER ) +twl4030_power_mmc_init (); +mdelay (100 );/* ramp-up delay from Linux code */ +#endif +return 0 ; +} +#endif +#if defined(CONFIG_USB_EHCI ) && !defined(CONFIG_SPL_BUILD )/* Call usb_stop() before starting the kernel */ void show_boot_progress (int val)
board/ti/evm/evm.c +11 -0
@@ -264,3 +264,14 @@ int board_mmc_init(bd_t *bis) return omap_mmc_init(0, 0, 0, -1, -1); }#endif + +#if defined(CONFIG_GENERIC_MMC ) +int board_mmc_power_init (void ) +{ +#if defined(CONFIG_TWL4030_POWER ) +twl4030_power_mmc_init (); +mdelay (100 );/* ramp-up delay from Linux code */ +#endif +return 0 ; +} +#endif
board/ti/sdp3430/sdp.c +9 -0
@@ -195,4 +195,13 @@ 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 */ +#endif +return 0 ; +}#endif
board/timll/devkit8000/devkit8000.c +11 -0
@@ -124,6 +124,17 @@ int board_mmc_init(bd_t *bis) }#endif +#if defined(CONFIG_GENERIC_MMC ) +int board_mmc_power_init (void ) +{ +#if defined(CONFIG_TWL4030_POWER ) +twl4030_power_mmc_init (); +mdelay (100 );/* ramp-up delay from Linux code */ +#endif +return 0 ; +} +#endif +#if defined(CONFIG_DRIVER_DM9000 ) & !defined(CONFIG_SPL_BUILD )/* * Routine: board_eth_init
drivers/mmc/omap_hsmmc.c +1 -6
@@ -130,12 +130,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);
Signature · 1 line
1.9.1
PATCH v2 2/2 omap_hsmmc: Board-specific TWL4030 MMC power initializations
Quoted 4 lines · Paul Kocialkowski
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_initfor the default behavior.
[snip]
board/comelit/dig297/dig297.c int board_mmc_init(bd_t *bis) · line 145
Quoted 9 lines
board/comelit/dig297/dig297.c +8
-0+int board_mmc_power_init (void ) +{ +#if defined(CONFIG_TWL4030_POWER ) +twl4030_power_mmc_init (); +mdelay (100 );/* ramp-up delay from Linux code */ +#endif +return 0 ; +}#endif
I think we can do away with the #if defined(CONFIG_TWL4030_POWER) part here since all of these boards will fail pretty spectacularly without that being set and if someone does remove that a link error is better than a crazy runtime problem.
[snip]
Quoted 8 lines
drivers/mmc/omap_hsmmc.c +0
-0@@ -130,12 +130,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);
drivers/mmc/omap_hsmmc.c static unsigned char mmc_board_init(struct mmc *mmc) · line 136
Quoted 10 lines
drivers/mmc/omap_hsmmc.c +1
-6-#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);
Since it's not clear from the context here, it's OK to drop the check on CONFIG_OMAP34XX here since there's already a test on it above where diff cut us off? Thanks!
Signature · 8 lines
Tom -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 836 bytes Desc: Digital signature URL: <http://lists.denx.de/pipermail/u-boot/attachments/20141104/ad6a1559/attachment.pgp>
PATCH v2 2/2 omap_hsmmc: Board-specific TWL4030 MMC power initializations
Quoted 5 lines
Quoted 4 lines
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_initfor the default behavior.[snip]
Quoted 14 lines
board/comelit/dig297/dig297.c int board_mmc_init(bd_t *bis) · line 145
Quoted 9 lines
board/comelit/dig297/dig297.c +8
-0+int board_mmc_power_init (void ) +{ +#if defined(CONFIG_TWL4030_POWER ) +twl4030_power_mmc_init (); +mdelay (100 );/* ramp-up delay from Linux code */ +#endif +return 0 ; +}#endif I think we can do away with the #if
defined(CONFIG_TWL4030_POWER)part here since all of these boards will fail pretty spectacularly without that being set and if someone does remove that a link error is better than a crazy runtime problem.
That makes sense, I'll do that in v5.
Quoted 15 lines
[snip]
Quoted 14 lines
drivers/mmc/omap_hsmmc.c +0
-6@@ -130,12 +130,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 )
Quoted 8 lines
drivers/mmc/omap_hsmmc.c static unsigned char mmc_board_init(struct mmc *mmc) · line 136
Quoted 4 lines
drivers/mmc/omap_hsmmc.c +1
-0+writel (pbias_lite |PBIASLITEPWRDNZ1 |PBIASSPEEDCTRL0 |PBIASLITEPWRDNZ0 , &t2_base->pbias_lite);Since it's not clear from the context here, it's OK to drop the check on
CONFIG_OMAP34XXhere since there's already a test on it above where diff cut us off? Thanks!
Indeed, that's the case (not sure this was actually more of a question than a statement).
Thanks for the review!
Signature · 13 lines
Paul Kocialkowski, Replicant developer Replicant is a fully free Android distribution Website: http://www.replicant.us/ Redmine: http://redmine.replicant.us/ -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: This is a digitally signed message part URL: <http://lists.denx.de/pipermail/u-boot/attachments/20141105/fb1440c8/attachment.pgp>
PATCH v2 1/2 mmc: Board-specific MMC power initializations
In reply to Paul KocialkowskiQuoted 15 lines · Paul Kocialkowski
Some devices may use non-standard combinations of regulators to power MMC:
this allows these devices to provide a board-specific MMC power init function to set everything up in their own way.Signed-off-by Paul Kocialkowski <[email protected]>--- drivers/mmc/mmc.c | 8 ++++++++ include/mmc.h | 2 ++ 2 files changed, 10 insertions(+)drivers/mmc/mmc.c +0
-0
drivers/mmc/mmc.c block_dev_desc_t *mmc_get_dev(int dev) · line 1284
Quoted 8 lines
drivers/mmc/mmc.c +5
-0@@ -1277,6 +1277,12 @@ block_dev_desc_t *mmc_get_dev(int dev) }#endif +/* board-specific MMC power initializations. */ +__weak int board_mmc_power_init (void ) +{ +return -1 ; +}
Since we don't check error return here which I think is fine just make this a void? Thanks!
Signature · 8 lines
Tom -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 836 bytes Desc: Digital signature URL: <http://lists.denx.de/pipermail/u-boot/attachments/20141104/66a3e08e/attachment.pgp>
PATCH v2 1/2 mmc: Board-specific MMC power initializations
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Hi Tom,
Quoted 20 lines · Tom Rini
Quoted 18 lines · Paul Kocialkowski
Some devices may use non-standard combinations of regulators to power MMC:
this allows these devices to provide a board-specific MMC power init function to set everything up in their own way.Signed-off-by Paul Kocialkowski <[email protected]>--- drivers/mmc/mmc.c | 8 ++++++++ include/mmc.h | 2 ++ 2 files changed, 10 insertions(+)drivers/mmc/mmc.c +0
-0@@ -1277,6 +1277,12 @@ block_dev_desc_t *mmc_get_dev(int dev) }#endif
Quoted 8 lines
drivers/mmc/mmc.c block_dev_desc_t *mmc_get_dev(int dev) · line 1284
Quoted 5 lines
drivers/mmc/mmc.c +5
-0+/* board-specific MMC power initializations. */ +__weak int board_mmc_power_init (void ) +{ +return -1 ; +}Since we don't check error return here which I think is fine just make this a void? Thanks!
There is v3 posted a while ago...
We have also agreed on v4..
- --
Regards,
Igor.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2
iQIcBAEBAgAGBQJUWRPOAAoJEBDE8YO64EfafPsP/iow4r72B6oCuDDh7g9+BWYg bO17RVU3PM14YgWQpybf/MXPgBSxyW3/4d69xrH5+TqEi9lOsHA/5X7ZW9r+xJh1 /JW1qwPKvqN9NdtGyPJppS0umoQoV77CzeTlLIdZ9emtozGSB/PpevAYK89HmrGl g5XYzeX/uPPE9MrJ1GdeEk+bGSq3N7rSEAzWIiJ50Ai7A4t1RTYRtNAqMbf4q/Ip WMO/MPiJk6Ybugk5vd91pJQkPtLIuFFEipUnIC8TSO7U8vWiOXKGgjH+aRvFhaLm 8YfU4Ym2vYZzIvpYbgHO6e2tKH0OhyzE/zZIDIGhOaXwPUsMbJquAvVEKz07XNu/ nOAeiBBhvTeMPehUe7jYaCymhxHJb3ZM29MOfz33a/GBskdMCNBtb/XoaibWbG1k ZfIMktv0SDObRmd/eUCNvl09YWj8JB7aTzFg20ZoeLoyfW7S1aJ1L0AymaOY20n0 A7MXpEVU1ddPu1rIh9hKm8G86i04aarQJ0kJ4vooCZI+qPLndwH+6Go0Zny3Vbj5 v1SudRROl0KUIoJd/Lt5nmJgCsFsas9xXgJsQNuK3avRrwlPcTykJv1eHbYFlNTq iFCjDAP7IU1iH0NBuJo7T2qPwPw9UMb3AQmo1JY/rFXopTVt2r0mkulZlAoMfWwy L2cveucMAOGnEIFxhngN
=acIT
-----END PGP SIGNATURE-----
PATCH v2 1/2 mmc: Board-specific MMC power initializations
Quoted 35 lines · Igor Grinberg
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1Hi Tom,
Quoted 20 lines · Tom Rini
Quoted 18 lines · Paul Kocialkowski
Some devices may use non-standard combinations of regulators to power MMC:
this allows these devices to provide a board-specific MMC power init function to set everything up in their own way.Signed-off-by Paul Kocialkowski <[email protected]>--- drivers/mmc/mmc.c | 8 ++++++++ include/mmc.h | 2 ++ 2 files changed, 10 insertions(+)drivers/mmc/mmc.c +0
-0@@ -1277,6 +1277,12 @@ block_dev_desc_t *mmc_get_dev(int dev) }#endif Quoted 8 lines
drivers/mmc/mmc.c block_dev_desc_t *mmc_get_dev(int dev) · line 1284
Quoted 5 lines
drivers/mmc/mmc.c +5
-0+/* board-specific MMC power initializations. */ +__weak int board_mmc_power_init (void ) +{ +return -1 ; +}Since we don't check error return here which I think is fine just make this a void? Thanks!
Quoted 2 lines
There is v3 posted a while ago...
We have also agreed on v4..
Yeah, oops, didn't delete these after catch-up. I'm still not sure we should continue adding more unchecked return values "just because".
Signature · 8 lines
Tom -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 836 bytes Desc: Digital signature URL: <http://lists.denx.de/pipermail/u-boot/attachments/20141104/313739eb/attachment.pgp>
PATCH v2 1/2 mmc: Board-specific MMC power initializations
Hi there, thanks for the review,
Le mardi 04 novembre 2014 ? 13:32 -0500, Tom Rini a ?crit :
Quoted 36 lines · Igor Grinberg
Quoted 35 lines
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1Hi Tom,
Quoted 28 lines · Tom Rini
Quoted 15 lines · Paul Kocialkowski
Some devices may use non-standard combinations of regulators to power MMC:
this allows these devices to provide a board-specific MMC power init function to set everything up in their own way.Signed-off-by Paul Kocialkowski <[email protected]>--- drivers/mmc/mmc.c | 8 ++++++++ include/mmc.h | 2 ++ 2 files changed, 10 insertions(+)drivers/mmc/mmc.c +0
-0drivers/mmc/mmc.c block_dev_desc_t *mmc_get_dev(int dev) · line 1284
Quoted 8 lines
drivers/mmc/mmc.c +5
-0@@ -1277,6 +1277,12 @@ block_dev_desc_t *mmc_get_dev(int dev) }#endif +/* board-specific MMC power initializations. */ +__weak int board_mmc_power_init (void ) +{ +return -1 ; +}Since we don't check error return here which I think is fine just make this a void? Thanks!
Quoted 2 lines
Quoted 2 lines
There is v3 posted a while ago...
We have also agreed on v4..
Note that v3 and v4 are the same, except that v3 didn't apply on top of master.
Quoted 2 lines
Yeah, oops, didn't delete these after catch-up. I'm still not sure we should continue adding more unchecked return values "just because".
I agree that we shouldn't have an unchecked return value. So we could either check the return value and print a warning, without aborting the init sequence (what Igor proposed initially) or just make this return void (what you both seem to agree on).
I'm fine with both solutions. I guess that enabling a regulator could fail (say, because of an i2c error), so there is still sense in returning int.
Let me know of what your definitive answer on this is. I'll make a new patchset probably this friday (I'm running on a very tight schedule until then).
Signature · 13 lines
Paul Kocialkowski, Replicant developer Replicant is a fully free Android distribution Website: http://www.replicant.us/ Redmine: http://redmine.replicant.us/ -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: This is a digitally signed message part URL: <http://lists.denx.de/pipermail/u-boot/attachments/20141105/a90469ed/attachment.pgp>
PATCH v2 1/2 mmc: Board-specific MMC power initializations
Quoted 53 lines · Paul Kocialkowski
Hi there, thanks for the review,
Le mardi 04 novembre 2014 ? 13:32 -0500, Tom Rini a ?crit :
Quoted 36 lines · Igor Grinberg
Quoted 35 lines
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1Hi Tom,
Quoted 28 lines · Tom Rini
Quoted 15 lines · Paul Kocialkowski
Some devices may use non-standard combinations of regulators to power MMC:
this allows these devices to provide a board-specific MMC power init function to set everything up in their own way.Signed-off-by Paul Kocialkowski <[email protected]>--- drivers/mmc/mmc.c | 8 ++++++++ include/mmc.h | 2 ++ 2 files changed, 10 insertions(+)drivers/mmc/mmc.c +0
-0drivers/mmc/mmc.c block_dev_desc_t *mmc_get_dev(int dev) · line 1284
Quoted 8 lines
drivers/mmc/mmc.c +5
-0@@ -1277,6 +1277,12 @@ block_dev_desc_t *mmc_get_dev(int dev) }#endif +/* board-specific MMC power initializations. */ +__weak int board_mmc_power_init (void ) +{ +return -1 ; +}Since we don't check error return here which I think is fine just make this a void? Thanks!
Quoted 2 lines
Quoted 2 lines
There is v3 posted a while ago...
We have also agreed on v4..Note that v3 and v4 are the same, except that v3 didn't apply on top of master.
Quoted 2 lines
Yeah, oops, didn't delete these after catch-up. I'm still not sure we should continue adding more unchecked return values "just because".
I agree that we shouldn't have an unchecked return value. So we could either check the return value and print a warning, without aborting the init sequence (what Igor proposed initially) or just make this return void (what you both seem to agree on).
Quoted 7 lines
I'm fine with both solutions. I guess that enabling a regulator could fail (say, because of an i2c error), so there is still sense in returning int.
Let me know of what your definitive answer on this is. I'll make a new patchset probably this friday (I'm running on a very tight schedule until then).
Lets go with void. Thanks!
Signature · 8 lines
Tom -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 836 bytes Desc: Digital signature URL: <http://lists.denx.de/pipermail/u-boot/attachments/20141105/c01d9351/attachment.pgp>