Hi Lubomir,,
On Tuesday 04 June 2013 01:28 AM, Lubomir Popov wrote:
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 <balaj...@ti.com>

add dra mmc pbias support and ldo1 power on

Signed-off-by: Balaji T K <balaj...@ti.com>
Signed-off-by: Lokesh Vutla <lokeshvu...@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.
Ahh sorry, I was in a dilemma whether to ask this or not. Since it is posted I assumed that the patch ll get merged. I have already posted a patch on top of your patch.
Ill wait for Tom to comment.

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.
Yes, TPS659038 has same LDOx_CTRL register layout.

Thanks,
Lokesh

Thanks
Lokesh

[snip]

Best regards,
Lubo


_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Reply via email to