[U-Boot] [PATCH] arm: trats: add the sd-card power control function.

[Lukasz Majewski]

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 at samsung.com>
> >> Signed-off-by: Kyungmin Park <kyungmin.park at 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>