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

[Nishanth Menon]

On Wed, Jun 5, 2013 at 3:03 AM, Lubomir Popov <lpopov at mm-sol.com> wrote:
> Hi Tom,
>
> On 05/06/13 00:06, Tom Rini wrote:
>> On Mon, Jun 03, 2013 at 10:58:27PM +0300, 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 <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?
>>
>> OK, so at the time it was "nothing will really use this code except test
>> functions".  Looks like we have a use for mmc1_ldo9 code at least, so
>> lets rework the first patch for adding that + cleanups wrt constants.
> Well, I'm not quite sure that this LDO9 function would be the only one
> used (or LDO1 on the DRA7xx board). Judging from omapboot for the OMAP5
> boards for example, SMPS7 (it delivers the common 1.8 V I/O supply) is
> set to 'Forced PWM' mode in order to reduce board noise - there sure has
> been a reason to do so and sacrifice converter efficiency. Therefore I
> added similar functionality in my patch to the Palmas driver (and am
> explicitly calling it in my board init).
> The option to bypass LDO9 on OMAP5+TWL603x boards seems quite mandatory
> as well, if hardware is designed such that the SD card socket has a
> separate fixed 3.3 V supply which also powers the LDO9 input (the
> uEVM for example). On the DRA7xx+TPS659038 board the power scheme is
> different and this does not apply.
>

I hate this code for many reasons -
a) hsmmc is used on many OMAP and DM platforms to my knowledge.
b) what is being done here is to power on the LDO supplying MMC.

The implementation *should* be board specific! not an #ifdef madness
which works only on TI platforms.

Regards,
Nishanth Menon