[U-Boot] [PATCH V2 09/12] mmc: omap_hsmmc: add mmc1 pbias, ldo1

[Lubomir Popov]

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 at ti.com>
>>>
>>> add dra mmc pbias support and ldo1 power on
>>>
>>> Signed-off-by: Balaji T K <balajitk at ti.com>
>>> Signed-off-by: Lokesh Vutla <lokeshvutla at 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