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.ch...@samsung.com>
>>>> Signed-off-by: Kyungmin Park <kyungmin.p...@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
>
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
