Hi Lokesh,
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]
- /* 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.
Please be aware that my above mentioned patch has not been reviewed/ tested/acked/nacked/whatever by nobody (except possibly a quick look by Nishanth Menon, who had some objections). I wrote it when bringing up a custom OMAP5 board, and most probably it shall not go into mainline in its current form, if ever. I gave it only as an example of how things could be done cleaner. Feel free to use the code as you wish, but I'm afraid that applying it as a patch to your tree and basing upon it might run you into problems when you later sync with mainline.
Tom, your opinion?
- 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.
But are you sure that the TPS659038 has the same LDOx_CTRL register layout as the TWL6035/37? It belongs to the family, yes, but I don't have a Register Manual for this chip... Hope you have checked.
Thanks Lokesh
[snip]
Best regards, Lubo