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");
elseerr = 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'tneeded.
}}
- 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:
- eMMC OK, card present and broken -> print informative error message
(to recover user will remove the card)
eMMC OK + card inserted and init OK = no problem
eMMC OK + card not inserted = no problem
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