* [U-Boot] [PATCH] arm: trats: add the sd-card power control function.
@ 2013-10-31 13:37 Jaehoon Chung
2013-10-31 17:03 ` Lukasz Majewski
0 siblings, 1 reply; 5+ messages in thread
From: Jaehoon Chung @ 2013-10-31 13:37 UTC (permalink / raw)
To: u-boot
To use the sd-card, VTF_2.8V will be enabled.
Before this patch, VTF_2.8V is always disabled.(card can't initialize.)
When card is detected, SD-card power will enable with sd_ldo_control().
Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
board/samsung/trats/trats.c | 38 ++++++++++++++++++++++++++++++++------
1 file changed, 32 insertions(+), 6 deletions(-)
diff --git a/board/samsung/trats/trats.c b/board/samsung/trats/trats.c
index 7f61d17..9b6dd12 100644
--- a/board/samsung/trats/trats.c
+++ b/board/samsung/trats/trats.c
@@ -408,11 +408,31 @@ int checkboard(void)
#endif
#ifdef CONFIG_GENERIC_MMC
+static int sd_ldo_control(int on)
+{
+ struct pmic *p = pmic_get("MAX8997_PMIC");
+ u32 val = 0;
+ int ret = 0;
+ if (pmic_probe(p))
+ return -1;
+
+ /* LDO17 VTF: 2.8V */
+ val = max8997_reg_ldo(2800000) | (on? EN_LDO : DIS_LDO);
+ ret |= pmic_reg_write(p, MAX8997_REG_LDO17CTRL, val);
+
+ if (ret) {
+ puts("MAX8997 SD setting error!\n");
+ return -1;
+ }
+
+ return 0;
+}
+
int board_mmc_init(bd_t *bis)
{
struct exynos4_gpio_part2 *gpio =
(struct exynos4_gpio_part2 *)samsung_get_base_gpio_part2();
- int err;
+ int err, err_sd = 0;
/* eMMC_EN: SD_0_CDn: GPK0[2] Output High */
s5p_gpio_direction_output(&gpio->k0, 2, 1);
@@ -438,14 +458,20 @@ int board_mmc_init(bd_t *bis)
* GPX3[4] T-flash detect pin
*/
if (!s5p_gpio_get_value(&gpio->x3, 4)) {
- err = exynos_pinmux_config(PERIPH_ID_SDMMC2, PINMUX_FLAG_NONE);
- if (err)
+ err_sd = exynos_pinmux_config(PERIPH_ID_SDMMC2, PINMUX_FLAG_NONE);
+ if (err_sd)
debug("SDMMC2 not configured\n");
- else
- err = s5p_mmc_init(2, 4);
+ else {
+ if (!sd_ldo_control(1)) {
+ err_sd = s5p_mmc_init(2, 4);
+ if (err_sd) {
+ sd_ldo_control(0);
+ }
+ }
+ }
}
- return err;
+ return err & err_sd;
}
#endif
--
1.7.9.5
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [U-Boot] [PATCH] arm: trats: add the sd-card power control function.
2013-10-31 13:37 [U-Boot] [PATCH] arm: trats: add the sd-card power control function Jaehoon Chung
@ 2013-10-31 17:03 ` Lukasz Majewski
2013-11-01 1:02 ` Jaehoon Chung
0 siblings, 1 reply; 5+ messages in thread
From: Lukasz Majewski @ 2013-10-31 17:03 UTC (permalink / raw)
To: u-boot
Hi Jaehoon,
> To use the sd-card, VTF_2.8V will be enabled.
> Before this patch, VTF_2.8V is always disabled.(card can't
> initialize.) When card is detected, SD-card power will enable with
> sd_ldo_control().
>
> Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> ---
> board/samsung/trats/trats.c | 38
> ++++++++++++++++++++++++++++++++------ 1 file changed, 32
> insertions(+), 6 deletions(-)
>
> diff --git a/board/samsung/trats/trats.c b/board/samsung/trats/trats.c
> index 7f61d17..9b6dd12 100644
> --- a/board/samsung/trats/trats.c
> +++ b/board/samsung/trats/trats.c
> @@ -408,11 +408,31 @@ int checkboard(void)
> #endif
>
> #ifdef CONFIG_GENERIC_MMC
> +static int sd_ldo_control(int on)
> +{
> + struct pmic *p = pmic_get("MAX8997_PMIC");
> + u32 val = 0;
> + int ret = 0;
> + if (pmic_probe(p))
> + return -1;
Maybe -1 -> -ENODEV
> +
> + /* LDO17 VTF: 2.8V */
> + val = max8997_reg_ldo(2800000) | (on? EN_LDO : DIS_LDO);
^^^ on ?
> + ret |= pmic_reg_write(p, MAX8997_REG_LDO17CTRL, val);
^^^^^ ret =
> +
> + if (ret) {
> + puts("MAX8997 SD setting error!\n");
Please use error() instead of puts()
> + return -1;
> + }
> +
> + return 0;
> +}
> +
> int board_mmc_init(bd_t *bis)
> {
> struct exynos4_gpio_part2 *gpio =
> (struct exynos4_gpio_part2
> *)samsung_get_base_gpio_part2();
> - int err;
> + int err, err_sd = 0;
I think that the err_sd is not needed here. Please use the err as is
was at the original code.
>
> /* eMMC_EN: SD_0_CDn: GPK0[2] Output High */
> s5p_gpio_direction_output(&gpio->k0, 2, 1);
> @@ -438,14 +458,20 @@ int board_mmc_init(bd_t *bis)
> * GPX3[4] T-flash detect pin
> */
> if (!s5p_gpio_get_value(&gpio->x3, 4)) {
> - err = exynos_pinmux_config(PERIPH_ID_SDMMC2,
> PINMUX_FLAG_NONE);
> - if (err)
> + err_sd = exynos_pinmux_config(PERIPH_ID_SDMMC2,
> PINMUX_FLAG_NONE);
> + if (err_sd)
> debug("SDMMC2 not configured\n");
> - else
> - err = s5p_mmc_init(2, 4);
> + else {
> + if (!sd_ldo_control(1)) {
> + err_sd = s5p_mmc_init(2, 4);
> + if (err_sd) {
> + sd_ldo_control(0);
> + }
Parenthesis with this if aren't needed.
> + }
> + }
> }
>
> - return err;
> + return err & err_sd;
> }
> #endif
>
--
Best regards,
Lukasz Majewski
Samsung R&D Institute Poland (SRPOL) | Linux Platform Group
^ permalink raw reply [flat|nested] 5+ messages in thread
* [U-Boot] [PATCH] arm: trats: add the sd-card power control function.
2013-10-31 17:03 ` Lukasz Majewski
@ 2013-11-01 1:02 ` Jaehoon Chung
2013-11-03 22:24 ` Lukasz Majewski
0 siblings, 1 reply; 5+ messages in thread
From: Jaehoon Chung @ 2013-11-01 1:02 UTC (permalink / raw)
To: u-boot
Hi Lukasz,
Thanks for your comments.
On 11/01/2013 02:03 AM, Lukasz Majewski wrote:
> Hi Jaehoon,
>
>> To use the sd-card, VTF_2.8V will be enabled.
>> Before this patch, VTF_2.8V is always disabled.(card can't
>> initialize.) When card is detected, SD-card power will enable with
>> sd_ldo_control().
>>
>> Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com>
>> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
>> ---
>> board/samsung/trats/trats.c | 38
>> ++++++++++++++++++++++++++++++++------ 1 file changed, 32
>> insertions(+), 6 deletions(-)
>>
>> diff --git a/board/samsung/trats/trats.c b/board/samsung/trats/trats.c
>> index 7f61d17..9b6dd12 100644
>> --- a/board/samsung/trats/trats.c
>> +++ b/board/samsung/trats/trats.c
>> @@ -408,11 +408,31 @@ int checkboard(void)
>> #endif
>>
>> #ifdef CONFIG_GENERIC_MMC
>> +static int sd_ldo_control(int on)
>> +{
>> + struct pmic *p = pmic_get("MAX8997_PMIC");
>> + u32 val = 0;
>> + int ret = 0;
>> + if (pmic_probe(p))
>> + return -1;
>
> Maybe -1 -> -ENODEV
Will fix..
>
>> +
>> + /* LDO17 VTF: 2.8V */
>> + val = max8997_reg_ldo(2800000) | (on? EN_LDO : DIS_LDO);
> ^^^ on ?
Will fix
>> + ret |= pmic_reg_write(p, MAX8997_REG_LDO17CTRL, val);
> ^^^^^ ret =
Will fix.
>> +
>> + if (ret) {
>> + puts("MAX8997 SD setting error!\n");
>
> Please use error() instead of puts()
Using error().
>
>> + return -1;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> int board_mmc_init(bd_t *bis)
>> {
>> struct exynos4_gpio_part2 *gpio =
>> (struct exynos4_gpio_part2
>> *)samsung_get_base_gpio_part2();
>> - int err;
>> + int err, err_sd = 0;
>
> I think that the err_sd is not needed here. Please use the err as is
> was at the original code.
As like the goni, I think sd_err needs herre.
We need to consider the cases when eMMC is intiailzed and SD-card is failed.
If SD-card is failed, then it's always retunred "Fail", although eMMC is initialized.
how about?
After replying your, i will send the fixing patch.
Best Regards,
Jaehoon Chung
>
>>
>> /* eMMC_EN: SD_0_CDn: GPK0[2] Output High */
>> s5p_gpio_direction_output(&gpio->k0, 2, 1);
>> @@ -438,14 +458,20 @@ int board_mmc_init(bd_t *bis)
>> * GPX3[4] T-flash detect pin
>> */
>> if (!s5p_gpio_get_value(&gpio->x3, 4)) {
>> - err = exynos_pinmux_config(PERIPH_ID_SDMMC2,
>> PINMUX_FLAG_NONE);
>> - if (err)
>> + err_sd = exynos_pinmux_config(PERIPH_ID_SDMMC2,
>> PINMUX_FLAG_NONE);
>> + if (err_sd)
>> debug("SDMMC2 not configured\n");
>> - else
>> - err = s5p_mmc_init(2, 4);
>> + else {
>> + if (!sd_ldo_control(1)) {
>> + err_sd = s5p_mmc_init(2, 4);
>> + if (err_sd) {
>> + sd_ldo_control(0);
>> + }
> Parenthesis with this if aren't needed.
>> + }
>> + }
>> }
>>
>> - return err;
>> + return err & err_sd;
>> }
>> #endif
>>
>
>
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* [U-Boot] [PATCH] arm: trats: add the sd-card power control function.
2013-11-01 1:02 ` Jaehoon Chung
@ 2013-11-03 22:24 ` Lukasz Majewski
2013-11-04 1:28 ` Jaehoon Chung
0 siblings, 1 reply; 5+ messages in thread
From: Lukasz Majewski @ 2013-11-03 22:24 UTC (permalink / raw)
To: u-boot
Hi Jaehoon,
> Hi Lukasz,
>
> Thanks for your comments.
>
> On 11/01/2013 02:03 AM, Lukasz Majewski wrote:
> > Hi Jaehoon,
> >
> >> To use the sd-card, VTF_2.8V will be enabled.
> >> Before this patch, VTF_2.8V is always disabled.(card can't
> >> initialize.) When card is detected, SD-card power will enable with
> >> sd_ldo_control().
> >>
> >> Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com>
> >> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> >> ---
> >> board/samsung/trats/trats.c | 38
> >> ++++++++++++++++++++++++++++++++------ 1 file changed, 32
> >> insertions(+), 6 deletions(-)
> >>
> >> diff --git a/board/samsung/trats/trats.c
> >> b/board/samsung/trats/trats.c index 7f61d17..9b6dd12 100644
> >> --- a/board/samsung/trats/trats.c
> >> +++ b/board/samsung/trats/trats.c
> >> @@ -408,11 +408,31 @@ int checkboard(void)
> >> #endif
> >>
> >> #ifdef CONFIG_GENERIC_MMC
> >> +static int sd_ldo_control(int on)
> >> +{
> >> + struct pmic *p = pmic_get("MAX8997_PMIC");
> >> + u32 val = 0;
> >> + int ret = 0;
> >> + if (pmic_probe(p))
> >> + return -1;
> >
> > Maybe -1 -> -ENODEV
> Will fix..
> >
> >> +
> >> + /* LDO17 VTF: 2.8V */
> >> + val = max8997_reg_ldo(2800000) | (on? EN_LDO : DIS_LDO);
> > ^^^ on ?
> Will fix
> >> + ret |= pmic_reg_write(p, MAX8997_REG_LDO17CTRL, val);
> > ^^^^^ ret =
> Will fix.
> >> +
> >> + if (ret) {
> >> + puts("MAX8997 SD setting error!\n");
> >
> > Please use error() instead of puts()
> Using error().
> >
> >> + return -1;
> >> + }
> >> +
> >> + return 0;
> >> +}
> >> +
> >> int board_mmc_init(bd_t *bis)
> >> {
> >> struct exynos4_gpio_part2 *gpio =
> >> (struct exynos4_gpio_part2
> >> *)samsung_get_base_gpio_part2();
> >> - int err;
> >> + int err, err_sd = 0;
> >
> > I think that the err_sd is not needed here. Please use the err as is
> > was at the original code.
> As like the goni, I think sd_err needs herre.
> We need to consider the cases when eMMC is intiailzed and SD-card is
> failed. If SD-card is failed, then it's always retunred "Fail",
> although eMMC is initialized. how about?
I think that it is perfectly correct when we insert SD card (which is
signaled by change of card detect pin - X3[4]) and when we fail to
initialize it to display the error message and pass the error code out
of the function.
Then user can read the error cause from the console and perform hard
reset with SD card removed.
> After replying your, i will send the fixing patch.
>
> Best Regards,
> Jaehoon Chung
> >
> >>
> >> /* eMMC_EN: SD_0_CDn: GPK0[2] Output High */
> >> s5p_gpio_direction_output(&gpio->k0, 2, 1);
> >> @@ -438,14 +458,20 @@ int board_mmc_init(bd_t *bis)
> >> * GPX3[4] T-flash detect pin
> >> */
> >> if (!s5p_gpio_get_value(&gpio->x3, 4)) {
> >> - err = exynos_pinmux_config(PERIPH_ID_SDMMC2,
> >> PINMUX_FLAG_NONE);
> >> - if (err)
> >> + err_sd = exynos_pinmux_config(PERIPH_ID_SDMMC2,
> >> PINMUX_FLAG_NONE);
> >> + if (err_sd)
> >> debug("SDMMC2 not configured\n");
> >> - else
> >> - err = s5p_mmc_init(2, 4);
> >> + else {
> >> + if (!sd_ldo_control(1)) {
> >> + err_sd = s5p_mmc_init(2, 4);
> >> + if (err_sd) {
> >> + sd_ldo_control(0);
> >> + }
> > Parenthesis with this if aren't
> > needed.
> >> + }
> >> + }
> >> }
> >>
> >> - return err;
> >> + return err & err_sd;
To be honest I'm frowned upon when errors are ANDed. In my opinion they
shall be ORed together.
So I think that for the trats.c we only need to do the following:
if (!s5p_gpio_get_value(&gpio->x3, 4)) {
err |= exynos_pinmux_config(PERIPH_ID_SDMMC2, PINMUX_FLAG_NONE);
if (err) {
debug("SDMMC2 not configured\n");
} else {
if (!sd_ldo_control(1)) {
err |= s5p_mmc_init(2, 4);
if (err) {
sd_ldo_control(0);
error("SD catd init failed\
- remove it\n");
}
}
}
}
return err;
Please consider following scenarios:
1. eMMC OK, card present and broken -> print informative error message
(to recover user will remove the card)
2. eMMC OK + card inserted and init OK = no problem
3. eMMC OK + card not inserted = no problem
4. eMMC initialization broken + card inserted and init OK - we will not
boot the device from eMMC, so we will not be able to use SD card at
u-boot.
Please correct me if I'm wrong and/or I've overlooked some useful
scenario for you.
Best regards,
Lukasz Majewski
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20131103/95b38749/attachment.pgp>
^ permalink raw reply [flat|nested] 5+ messages in thread
* [U-Boot] [PATCH] arm: trats: add the sd-card power control function.
2013-11-03 22:24 ` Lukasz Majewski
@ 2013-11-04 1:28 ` Jaehoon Chung
0 siblings, 0 replies; 5+ messages in thread
From: Jaehoon Chung @ 2013-11-04 1:28 UTC (permalink / raw)
To: u-boot
Hi, Lukasz,
On 11/04/2013 07:24 AM, Lukasz Majewski wrote:
> Hi Jaehoon,
>
>> Hi Lukasz,
>>
>> Thanks for your comments.
>>
>> On 11/01/2013 02:03 AM, Lukasz Majewski wrote:
>>> Hi Jaehoon,
>>>
>>>> To use the sd-card, VTF_2.8V will be enabled.
>>>> Before this patch, VTF_2.8V is always disabled.(card can't
>>>> initialize.) When card is detected, SD-card power will enable with
>>>> sd_ldo_control().
>>>>
>>>> Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com>
>>>> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
>>>> ---
>>>> board/samsung/trats/trats.c | 38
>>>> ++++++++++++++++++++++++++++++++------ 1 file changed, 32
>>>> insertions(+), 6 deletions(-)
>>>>
>>>> diff --git a/board/samsung/trats/trats.c
>>>> b/board/samsung/trats/trats.c index 7f61d17..9b6dd12 100644
>>>> --- a/board/samsung/trats/trats.c
>>>> +++ b/board/samsung/trats/trats.c
>>>> @@ -408,11 +408,31 @@ int checkboard(void)
>>>> #endif
>>>>
>>>> #ifdef CONFIG_GENERIC_MMC
>>>> +static int sd_ldo_control(int on)
>>>> +{
>>>> + struct pmic *p = pmic_get("MAX8997_PMIC");
>>>> + u32 val = 0;
>>>> + int ret = 0;
>>>> + if (pmic_probe(p))
>>>> + return -1;
>>>
>>> Maybe -1 -> -ENODEV
>> Will fix..
>>>
>>>> +
>>>> + /* LDO17 VTF: 2.8V */
>>>> + val = max8997_reg_ldo(2800000) | (on? EN_LDO : DIS_LDO);
>>> ^^^ on ?
>> Will fix
>>>> + ret |= pmic_reg_write(p, MAX8997_REG_LDO17CTRL, val);
>>> ^^^^^ ret =
>> Will fix.
>>>> +
>>>> + if (ret) {
>>>> + puts("MAX8997 SD setting error!\n");
>>>
>>> Please use error() instead of puts()
>> Using error().
>>>
>>>> + return -1;
>>>> + }
>>>> +
>>>> + return 0;
>>>> +}
>>>> +
>>>> int board_mmc_init(bd_t *bis)
>>>> {
>>>> struct exynos4_gpio_part2 *gpio =
>>>> (struct exynos4_gpio_part2
>>>> *)samsung_get_base_gpio_part2();
>>>> - int err;
>>>> + int err, err_sd = 0;
>>>
>>> I think that the err_sd is not needed here. Please use the err as is
>>> was at the original code.
>> As like the goni, I think sd_err needs herre.
>> We need to consider the cases when eMMC is intiailzed and SD-card is
>> failed. If SD-card is failed, then it's always retunred "Fail",
>> although eMMC is initialized. how about?
>
> I think that it is perfectly correct when we insert SD card (which is
> signaled by change of card detect pin - X3[4]) and when we fail to
> initialize it to display the error message and pass the error code out
> of the function.
>
> Then user can read the error cause from the console and perform hard
> reset with SD card removed.
>
>
>> After replying your, i will send the fixing patch.
>>
>> Best Regards,
>> Jaehoon Chung
>>>
>>>>
>>>> /* eMMC_EN: SD_0_CDn: GPK0[2] Output High */
>>>> s5p_gpio_direction_output(&gpio->k0, 2, 1);
>>>> @@ -438,14 +458,20 @@ int board_mmc_init(bd_t *bis)
>>>> * GPX3[4] T-flash detect pin
>>>> */
>>>> if (!s5p_gpio_get_value(&gpio->x3, 4)) {
>>>> - err = exynos_pinmux_config(PERIPH_ID_SDMMC2,
>>>> PINMUX_FLAG_NONE);
>>>> - if (err)
>>>> + err_sd = exynos_pinmux_config(PERIPH_ID_SDMMC2,
>>>> PINMUX_FLAG_NONE);
>>>> + if (err_sd)
>>>> debug("SDMMC2 not configured\n");
>>>> - else
>>>> - err = s5p_mmc_init(2, 4);
>>>> + else {
>>>> + if (!sd_ldo_control(1)) {
>>>> + err_sd = s5p_mmc_init(2, 4);
>>>> + if (err_sd) {
>>>> + sd_ldo_control(0);
>>>> + }
>>> Parenthesis with this if aren't
>>> needed.
>>>> + }
>>>> + }
>>>> }
>>>>
>>>> - return err;
>>>> + return err & err_sd;
>
> To be honest I'm frowned upon when errors are ANDed. In my opinion they
> shall be ORed together.
>
>
> So I think that for the trats.c we only need to do the following:
>
> if (!s5p_gpio_get_value(&gpio->x3, 4)) {
> err |= exynos_pinmux_config(PERIPH_ID_SDMMC2, PINMUX_FLAG_NONE);
> if (err) {
> debug("SDMMC2 not configured\n");
> } else {
> if (!sd_ldo_control(1)) {
> err |= s5p_mmc_init(2, 4);
> if (err) {
> sd_ldo_control(0);
> error("SD catd init failed\
> - remove it\n");
> }
> }
> }
> }
>
> return err;
>
> Please consider following scenarios:
>
> 1. eMMC OK, card present and broken -> print informative error message
> (to recover user will remove the card)
>
> 2. eMMC OK + card inserted and init OK = no problem
>
> 3. eMMC OK + card not inserted = no problem
>
> 4. eMMC initialization broken + card inserted and init OK - we will not
> boot the device from eMMC, so we will not be able to use SD card at
> u-boot.
>
> Please correct me if I'm wrong and/or I've overlooked some useful
> scenario for you.
As your opinion, I will test and rework the patch.
Best Regards,
Jaehoon Chung
>
> Best regards,
> Lukasz Majewski
>
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2013-11-04 1:28 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-10-31 13:37 [U-Boot] [PATCH] arm: trats: add the sd-card power control function Jaehoon Chung
2013-10-31 17:03 ` Lukasz Majewski
2013-11-01 1:02 ` Jaehoon Chung
2013-11-03 22:24 ` Lukasz Majewski
2013-11-04 1:28 ` Jaehoon Chung
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox