From mboxrd@z Thu Jan 1 00:00:00 1970 From: Lokesh Vutla Date: Mon, 3 Jun 2013 16:31:36 +0530 Subject: [U-Boot] [PATCH V2 09/12] mmc: omap_hsmmc: add mmc1 pbias, ldo1 In-Reply-To: <51A7617B.8040503@mm-sol.com> References: <1369919979-26497-1-git-send-email-lokeshvutla@ti.com> <1369919979-26497-10-git-send-email-lokeshvutla@ti.com> <51A7617B.8040503@mm-sol.com> Message-ID: <51AC7790.8010600@ti.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de 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 >> >> add dra mmc pbias support and ldo1 power on >> >> Signed-off-by: Balaji T K >> Signed-off-by: Lokesh Vutla >> --- >> 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 >