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

2013-06-06 Thread Lubomir Popov
Hi Tom,

On 05/06/13 16:45, Tom Rini wrote:
 On Wed, Jun 05, 2013 at 11:03:26AM +0300, Lubomir Popov 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 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.

 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.
 
 OK, lets see.  That so lets keep your patch as-is, since we've now got
 -ffunction-sections/-fdata-sections/--gc-sections on ARM for main
 U-Boot, these small things won't hurt like they used to.
 
OK, but then I would like to do some cleanup first - remove the audio
power stuff (shall have it in my board file), as well as either sort out
the function naming:

- Those functions that are specific to a SoC+PMIC combination are
named e.g. twl603x_... or tps659038_... so that they explicitly
indicate the hardware that they are working with (actually almost all
functions are such). This is however sort of regression, and requires
fixes in the files calling these functions;

or, alternatively:

- Introduce generic functions with fixed names, palmas_bla_bla(),
sort of wrappers, which in their bodies perform the appropriate action
based on the #ifdefs defining the platform hardware (where we could also
define the particular LDO which for example a palmas_mmc1_poweron_ldo()
generic function would manipulate). Drawback: again #ifdefs.
Advantage: single place where this stuff is located, and where other
PMIC/LDO combinations can be added without affecting other code.
And this generic palmas_mmc1_poweron_ldo() function would be called
by another generic function, e.g. omap_sdmmc_poweron(), located in
the board file, only if needed by the particular hardware.
omap_sdmmc_poweron(), on its hand, is the function that is to be called
from within the pbias routines in omap_hsmmc.c, and not the hardware-
dependant functions directly. So we get the abstraction.

What do you think? Lokesh, your opinion?

Regards,
Lubo
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


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

2013-06-06 Thread Lokesh Vutla

Hi Lubomir,
On Thursday 06 June 2013 12:55 PM, Lubomir Popov wrote:

Hi Tom,

On 05/06/13 16:45, Tom Rini wrote:

On Wed, Jun 05, 2013 at 11:03:26AM +0300, Lubomir Popov 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 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.

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.


OK, lets see.  That so lets keep your patch as-is, since we've now got
-ffunction-sections/-fdata-sections/--gc-sections on ARM for main
U-Boot, these small things won't hurt like they used to.


OK, but then I would like to do some cleanup first - remove the audio
power stuff (shall have it in my board file), as well as either sort out
the function naming:

- Those functions that are specific to a SoC+PMIC combination are
named e.g. twl603x_... or tps659038_... so that they explicitly
indicate the hardware that they are working with (actually almost all
functions are such). This is however sort of regression, and requires
fixes in the files calling these functions;

or, alternatively:

- Introduce generic functions with fixed names, palmas_bla_bla(),
sort of wrappers, which in their bodies perform the appropriate action
based on the #ifdefs defining the platform hardware (where we could also
define the particular LDO which for example a palmas_mmc1_poweron_ldo()
generic function would manipulate). Drawback: again #ifdefs.
Advantage: single place where this stuff is located, and where other
PMIC/LDO combinations can be added without affecting other code.

I think, we can have function pointers for and can populate data in the
beginning or from board file based on Soc, similarly what we did for
prcm structure.
Regards,
Lokesh

And this generic palmas_mmc1_poweron_ldo() function would be called
by another generic function, e.g. omap_sdmmc_poweron(), located in
the board file, only if needed by the particular hardware.
omap_sdmmc_poweron(), on its hand, is the function that is to be called
from within the pbias routines in omap_hsmmc.c, and not the hardware-
dependant functions directly. So we get the abstraction.

What do you think? Lokesh, your opinion?

Regards,
Lubo



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


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

2013-06-06 Thread Lubomir Popov
Hi Lokesh,

On 06/06/13 14:26, Lokesh Vutla wrote:
 Hi Lubomir,
 On Thursday 06 June 2013 12:55 PM, Lubomir Popov wrote:
 Hi Tom,

 On 05/06/13 16:45, Tom Rini wrote:
 On Wed, Jun 05, 2013 at 11:03:26AM +0300, Lubomir Popov wrote:

[snip]

 OK, lets see.  That so lets keep your patch as-is, since we've now got
 -ffunction-sections/-fdata-sections/--gc-sections on ARM for main
 U-Boot, these small things won't hurt like they used to.

 OK, but then I would like to do some cleanup first - remove the audio
 power stuff (shall have it in my board file), as well as either sort out
 the function naming:

 - Those functions that are specific to a SoC+PMIC combination are
 named e.g. twl603x_... or tps659038_... so that they explicitly
 indicate the hardware that they are working with (actually almost all
 functions are such). This is however sort of regression, and requires
 fixes in the files calling these functions;

 or, alternatively:

 - Introduce generic functions with fixed names, palmas_bla_bla(),
 sort of wrappers, which in their bodies perform the appropriate action
 based on the #ifdefs defining the platform hardware (where we could also
 define the particular LDO which for example a palmas_mmc1_poweron_ldo()
 generic function would manipulate). Drawback: again #ifdefs.
 Advantage: single place where this stuff is located, and where other
 PMIC/LDO combinations can be added without affecting other code.
 I think, we can have function pointers for and can populate data in the
 beginning or from board file based on Soc, similarly what we did for
 prcm structure.
 Regards,
 Lokesh
OK, sounds reasonable. I think this should be done in a future release
however, after careful investigation and planning. At present, I guess,
we are staying with the current situation.

Today I shall submit an updated version of my patch to the palmas
driver - sort of compromise between clean code and ease of use. I
have included your stuff there, so should work out of the box on
the dra7xx_evm. Please note that now we have a semi-generic function
to power on the appropriate SDMMC LDO: the old palmas_mmc1_poweron_ldo(),
which you shall have to call in omap_hsmmc. Differentiation of which
particular LDO to control within which PMIC is done in driver, based
on the board #ifdefs.

If Tom approves this patch and applies it, we shall all be happy with
working boards, although the code may not be perfect.

I would also like to ask you to send me a Register Manual of the
TPS659038/9, if possible. If you have any NDA concerns, then just
check if the LDO1 control register has a BYPASS option and tell
me. Thanks.

Best regards,
Lubo

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


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

2013-06-05 Thread Lokesh Vutla

On Wednesday 05 June 2013 02:36 AM, 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 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.

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.

Ok. Ill add the first patch + cleanups and resend it.

Thanks,
Lokesh




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


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

2013-06-05 Thread Lubomir Popov
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 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.

 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.

Best regards,
Lubo
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


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

2013-06-05 Thread Tom Rini
On Wed, Jun 05, 2013 at 11:03:26AM +0300, Lubomir Popov 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 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.
 
  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.

OK, lets see.  That so lets keep your patch as-is, since we've now got
-ffunction-sections/-fdata-sections/--gc-sections on ARM for main
U-Boot, these small things won't hurt like they used to.

-- 
Tom


signature.asc
Description: Digital signature
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


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

2013-06-05 Thread Nishanth Menon
On Wed, Jun 5, 2013 at 3:03 AM, Lubomir Popov lpo...@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 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.

 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
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


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

2013-06-05 Thread Lubomir Popov
Hi Nishanth,

On 05/06/13 17:01, Nishanth Menon wrote:
 On Wed, Jun 5, 2013 at 3:03 AM, Lubomir Popov lpo...@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 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.

 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.
Sorry, but I can't get if hsmmc is discussed here, or power.

For OMAP5+TWL603x the LDO powering MMC (actually the removable card
interface only; eMMC is another story) is turned on automatically at
power-on by the PMIC sequencer, with a default voltage and mode --
otherwise we would not be able to boot from a card (ROM code does not
touch the PMIC at all). We are talking here about the possibility to
have additional control over this LDO, which should be board-specific,
I agree. On the OMAP5 boards, for example, the call to
palmas_mmc1_poweron_ldo() from within omap_hsmmc actually does not
turn on LDO9 - it is on at this moment anyway. The call just makes
it switch from the default bypass mode (with Vout = Vin = 3.3 V) to
regulation mode and Vout = 3.0 V. Why is this done is yet another
question; to me it seems useless (and possibly wrong) when the card
is powered with a fixed voltage of 3.3 V.
Therefore it seems reasonable to count on the PMIC defaults and
remove this call from omap_hsmmc altogether, thus disengaging the
PMIC driver from hsmmc, at least for OMAP5.

For OMAP4 things are somewhat different. Here the TWL6030 PMIC powers
both the OMAP interface and the card socket, and in addition can
automatically power off the MMC LDO upon detecting card removal.
ROM code *does* access the MMC LDO to turn it on and set it to 3.0 V
(it starts by default at 1.8 V), but only if booting from a card.
So here the call to PMIC driver should stay.

Other OMAPs and derivatives - other scenarios.

Anyway, omap_hsmmc.c is built for TI platforms only. If you mean
the #ifdefs here, yes, things could be cleaned up by moving the SoC-
specific pbias stuff to the corresponding board files (with the
expense of redundancy), but this is quite an amount of work... I'm
not volunteering... ;) Moreover, this 

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

2013-06-05 Thread Nishanth Menon
On Wed, Jun 5, 2013 at 11:35 AM, Lubomir Popov lpo...@mm-sol.com wrote:
 Hi Nishanth,

 On 05/06/13 17:01, Nishanth Menon wrote:
 On Wed, Jun 5, 2013 at 3:03 AM, Lubomir Popov lpo...@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 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.

 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.
 Sorry, but I can't get if hsmmc is discussed here, or power.

 For OMAP5+TWL603x the LDO powering MMC (actually the removable card
 interface only; eMMC is another story) is turned on automatically at
 power-on by the PMIC sequencer, with a default voltage and mode --
 otherwise we would not be able to boot from a card (ROM code does not
 touch the PMIC at all). We are talking here about the possibility to
 have additional control over this LDO, which should be board-specific,
 I agree. On the OMAP5 boards, for example, the call to
 palmas_mmc1_poweron_ldo() from within omap_hsmmc actually does not
 turn on LDO9 - it is on at this moment anyway. The call just makes
 it switch from the default bypass mode (with Vout = Vin = 3.3 V) to
 regulation mode and Vout = 3.0 V. Why is this done is yet another
 question; to me it seems useless (and possibly wrong) when the card
 is powered with a fixed voltage of 3.3 V.
 Therefore it seems reasonable to count on the PMIC defaults and
 remove this call from omap_hsmmc altogether, thus disengaging the
 PMIC driver from hsmmc, at least for OMAP5.

 For OMAP4 things are somewhat different. Here the TWL6030 PMIC powers
 both the OMAP interface and the card socket, and in addition can
 automatically power off the MMC LDO upon detecting card removal.
 ROM code *does* access the MMC LDO to turn it on and set it to 3.0 V
 (it starts by default at 1.8 V), but only if booting from a card.
 So here the call to PMIC driver should stay.

 Other OMAPs and derivatives - other scenarios.

 Anyway, omap_hsmmc.c is built for TI platforms only. If you mean
 the #ifdefs here, yes, things could be cleaned up by moving the SoC-
 specific pbias stuff to the corresponding board files (with the
 expense of 

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

2013-06-05 Thread Lubomir Popov
Hi Nishanth,

 On Wed, Jun 5, 2013 at 11:35 AM, Lubomir Popov lpo...@mm-sol.com wrote:
 Hi Nishanth,

 On 05/06/13 17:01, Nishanth Menon wrote:
 On Wed, Jun 5, 2013 at 3:03 AM, Lubomir Popov lpo...@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 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.

 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.
 Sorry, but I can't get if hsmmc is discussed here, or power.

 For OMAP5+TWL603x the LDO powering MMC (actually the removable card
 interface only; eMMC is another story) is turned on automatically at
 power-on by the PMIC sequencer, with a default voltage and mode --
 otherwise we would not be able to boot from a card (ROM code does not
 touch the PMIC at all). We are talking here about the possibility to
 have additional control over this LDO, which should be board-specific,
 I agree. On the OMAP5 boards, for example, the call to
 palmas_mmc1_poweron_ldo() from within omap_hsmmc actually does not
 turn on LDO9 - it is on at this moment anyway. The call just makes
 it switch from the default bypass mode (with Vout = Vin = 3.3 V) to
 regulation mode and Vout = 3.0 V. Why is this done is yet another
 question; to me it seems useless (and possibly wrong) when the card
 is powered with a fixed voltage of 3.3 V.
 Therefore it seems reasonable to count on the PMIC defaults and
 remove this call from omap_hsmmc altogether, thus disengaging the
 PMIC driver from hsmmc, at least for OMAP5.

 For OMAP4 things are somewhat different. Here the TWL6030 PMIC powers
 both the OMAP interface and the card socket, and in addition can
 automatically power off the MMC LDO upon detecting card removal.
 ROM code *does* access the MMC LDO to turn it on and set it to 3.0 V
 (it starts by default at 1.8 V), but only if booting from a card.
 So here the call to PMIC driver should stay.

 Other OMAPs and derivatives - other scenarios.

 Anyway, omap_hsmmc.c is built for TI platforms only. If you mean
 the #ifdefs here, yes, things could be cleaned up by moving the SoC-
 specific pbias stuff to the corresponding board files (with 

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

2013-06-04 Thread Tom Rini
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 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.
 
 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.

-- 
Tom


signature.asc
Description: Digital signature
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


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

2013-06-03 Thread Lokesh Vutla

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]


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_ADDR0x58

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



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


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

2013-06-03 Thread 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 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.

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

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


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

2013-06-03 Thread Lokesh Vutla

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


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

2013-05-30 Thread Lokesh Vutla
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(-)

diff --git a/arch/arm/include/asm/arch-omap5/omap.h 
b/arch/arm/include/asm/arch-omap5/omap.h
index 8105c14..9abb663 100644
--- a/arch/arm/include/asm/arch-omap5/omap.h
+++ b/arch/arm/include/asm/arch-omap5/omap.h
@@ -106,9 +106,10 @@
 /* CONTROL_EFUSE_2 */
 #define CONTROL_EFUSE_2_NMOS_PMOS_PTV_CODE_1   0x00ffc000
 
+#define SDCARD_BIAS_PWRDNZ (1  27)
 #define SDCARD_PWRDNZ  (1  26)
 #define SDCARD_BIAS_HIZ_MODE   (1  25)
-#define SDCARD_BIAS_PWRDNZ (1  22)
+#define SDCARD_BIAS_PWRDNZ2(1  22)
 #define SDCARD_PBIASLITE_VMODE (1  21)
 
 #ifndef __ASSEMBLY__
diff --git a/drivers/mmc/omap_hsmmc.c b/drivers/mmc/omap_hsmmc.c
index afdfa88..27d1f76 100644
--- a/drivers/mmc/omap_hsmmc.c
+++ b/drivers/mmc/omap_hsmmc.c
@@ -113,23 +113,25 @@ static void omap5_pbias_config(struct mmc *mmc)
u32 value = 0;
 
value = readl((*ctrl)-control_pbias);
-   value = ~(SDCARD_PWRDNZ | SDCARD_BIAS_PWRDNZ);
-   value |= SDCARD_BIAS_HIZ_MODE;
+   value = ~SDCARD_PWRDNZ;
+   writel(value, (*ctrl)-control_pbias);
+   udelay(10); /* wait 10 us */
+   value = ~SDCARD_BIAS_PWRDNZ;
writel(value, (*ctrl)-control_pbias);
 
-   palmas_mmc1_poweron_ldo();
+#if defined(CONFIG_DRA7XX)
+   palmas_mmc1_poweron_ldo1();
+#else
+   palmas_mmc1_poweron_ldo9();
+#endif
 
value = readl((*ctrl)-control_pbias);
-   value = ~SDCARD_BIAS_HIZ_MODE;
-   value |= SDCARD_PBIASLITE_VMODE | SDCARD_PWRDNZ | SDCARD_BIAS_PWRDNZ;
+   value |= SDCARD_BIAS_PWRDNZ;
writel(value, (*ctrl)-control_pbias);
-
-   value = readl((*ctrl)-control_pbias);
-   if (value  (1  23)) {
-   value = ~(SDCARD_PWRDNZ | SDCARD_BIAS_PWRDNZ);
-   value |= SDCARD_BIAS_HIZ_MODE;
-   writel(value, (*ctrl)-control_pbias);
-   }
+   udelay(150); /* wait 150 us */
+   value |= SDCARD_PWRDNZ;
+   writel(value, (*ctrl)-control_pbias);
+   udelay(150); /* wait 150 us */
 }
 #endif
 
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 */
+   val = 0x2b; /* (3 - 0.9) * 20 + 1 */
+
+   if (palmas_i2c_write_u8(TPS659038_CHIP_ADDR, LDO1_VOLTAGE, val)) {
+   printf(tps659038: could not set LDO1 voltage\n);
+   return 1;
+   }
+
+   /* TURN ON LDO9 */
+   val = LDO_ON | LDO_MODE_SLEEP | LDO_MODE_ACTIVE;
+
+   if (palmas_i2c_write_u8(TPS659038_CHIP_ADDR, LDO1_CTRL, val)) {
+   printf(tps659038: could not turn on LDO1\n);
+   return 1;
+   }
+
+   return 0;
+}
diff --git a/include/configs/omap5_common.h b/include/configs/omap5_common.h
index 83b91d1..ddf2ad4 100644
--- a/include/configs/omap5_common.h
+++ b/include/configs/omap5_common.h
@@ -238,6 +238,10 @@
 #define CONFIG_SYS_DEFAULT_LPDDR2_TIMINGS
 #endif
 
+#ifndef CONFIG_SPL_BUILD
+#define CONFIG_PALMAS_POWER
+#endif
+
 /* Defines for SPL */
 #define CONFIG_SPL
 #define CONFIG_SPL_FRAMEWORK
diff --git a/include/configs/omap5_uevm.h b/include/configs/omap5_uevm.h
index ba81e30..f4a2d31 100644
--- a/include/configs/omap5_uevm.h
+++ b/include/configs/omap5_uevm.h
@@ -39,11 +39,6 @@
 #define CONFIG_SYS_NS16550_COM3UART3_BASE
 #define CONFIG_BAUDRATE115200
 
-/* TWL6035 */
-#ifndef CONFIG_SPL_BUILD
-#define CONFIG_PALMAS_POWER
-#endif
-
 /* MMC ENV related defines */
 #define CONFIG_ENV_IS_IN_MMC
 #define CONFIG_SYS_MMC_ENV_DEV 1   /* SLOT2: eMMC(1) */
diff --git a/include/palmas.h b/include/palmas.h
index 3b18589..4218e18 100644
--- a/include/palmas.h
+++ b/include/palmas.h
@@ -28,8 +28,11 @@
 
 /* I2C chip addresses */
 #define PALMAS_CHIP_ADDR   0x48
+#define TPS659038_CHIP_ADDR0x58
 
 /* 0x1XY translates to page 1, register address 0xXY */
+#define LDO1_CTRL  0x50
+#define LDO1_VOLTAGE  

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

2013-05-30 Thread Lubomir Popov
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]
  
 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.

 + 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.

 +
 + 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.

 +
 + 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
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot