Hi Lubomir, On Thursday 30 May 2013 07:56 PM, Lubomir Popov wrote:
Hi Lokesh,
On 30/05/13 16:19, Lokesh Vutla wrote:
From: Balaji T K balajitk@ti.com
add dra mmc pbias support and ldo1 power on
Signed-off-by: Balaji T K balajitk@ti.com Signed-off-by: Lokesh Vutla lokeshvutla@ti.com
arch/arm/include/asm/arch-omap5/omap.h | 3 ++- drivers/mmc/omap_hsmmc.c | 26 ++++++++++++++------------ drivers/power/palmas.c | 25 ++++++++++++++++++++++++- include/configs/omap5_common.h | 4 ++++ include/configs/omap5_uevm.h | 5 ----- include/palmas.h | 6 +++++- 6 files changed, 49 insertions(+), 20 deletions(-)
[snip]
diff --git a/drivers/power/palmas.c b/drivers/power/palmas.c index 09c832d..1bcff52 100644 --- a/drivers/power/palmas.c +++ b/drivers/power/palmas.c @@ -28,7 +28,7 @@ void palmas_init_settings(void) return; }
-int palmas_mmc1_poweron_ldo(void) +int palmas_mmc1_poweron_ldo9(void) { u8 val = 0;
@@ -50,3 +50,26 @@ int palmas_mmc1_poweron_ldo(void)
return 0; }
+int palmas_mmc1_poweron_ldo1(void) +{
- u8 val = 0;
- /* set LDO9 TWL6035 to 3V */
LDO9? TWL6035? If this function is used on the DRA7xx boards only (with TPS659038), you should add some comment above.
Ok ll add the comment.
- val = 0x2b; /* (3 - 0.9) * 20 + 1 */
Why not use definitions for the voltage? You could take them from http://patchwork.ozlabs.org/patch/244103/ where some values are defined.
Yes, Ill rebase this patch on top of your patch and use those defines.
- if (palmas_i2c_write_u8(TPS659038_CHIP_ADDR, LDO1_VOLTAGE, val)) {
printf("tps659038: could not set LDO1 voltage\n");return 1;- }
- /* TURN ON LDO9 */
LDO9?
- val = LDO_ON | LDO_MODE_SLEEP | LDO_MODE_ACTIVE;
Bit LDO_ON in all LDOx_CTRL Palmas registers is Read-Only (and reflects the current status of the LDO). While it makes no harm to try writing to it, this may be misleading about actual LDO operation, and anyway has no sense.
Yes, I see a similar update in your patch for LDO9. ll do the same for LDO1 also.
Thanks Lokesh
- if (palmas_i2c_write_u8(TPS659038_CHIP_ADDR, LDO1_CTRL, val)) {
printf("tps659038: could not turn on LDO1\n");return 1;- }
[snip]
/* I2C chip addresses */ #define PALMAS_CHIP_ADDR 0x48 +#define TPS659038_CHIP_ADDR 0x58
Now we have a mess again. The files were recently renamed from twl6035.x to palmas.x, implying that palmas is the generic family name of a series of PMICs. Having TPS659038_CHIP_ADDR above is OK, but then we should have TWL603X_CHIP_ADDR instead of PALMAS_CHIP_ADDR.
Best regards, Lubomir