Re: [U-Boot] [PATCH V2 09/12] mmc: omap_hsmmc: add mmc1 pbias, ldo1
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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