From mboxrd@z Thu Jan  1 00:00:00 1970
From: Peng Fan <van.freenix@gmail.com>
Date: Tue, 11 Oct 2016 14:37:41 +0800
Subject: [U-Boot] [PATCH] mmc: introduce mmc_power_init
In-Reply-To: <a2cabc7d-97f9-f1f9-d8d5-c54a3e014c78@samsung.com>
References: <CGME20161008100826epcas1p235f9ee38e9487a1a704e72bbeace3f77@epcas1p2.samsung.com>
 <1475921271-29093-1-git-send-email-peng.fan@nxp.com>
 <a2cabc7d-97f9-f1f9-d8d5-c54a3e014c78@samsung.com>
Message-ID: <20161011063738.GA31608@linux-7smt.suse>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

Hi Jaehoon,
On Mon, Oct 10, 2016 at 01:19:41PM +0900, Jaehoon Chung wrote:
>Hi Peng,
>
>On 10/08/2016 07:07 PM, Peng Fan wrote:
>> In device tree, there is vmmc-supply property for SD/MMC.
>> Introduce mmc_power_init function to handle vmmc-supply.
>
>As i know, vqmmc-supply should be optional. Do you have a plan to add this?

In the dts for my board, there is no vqmmc-supply. So I did not add this.
Let me add it in V2 as the following.

"
ret = device_get_supply_regulator(mmc->dev, "vqmmc-supply",
				  &vqmmc_supply);
if (ret) {
	debug("No vqmmc supply\n");
	return 0;
}

ret = regulator_set_enable(vqmmc_supply, true);
if (ret) {
	puts("Error enabling VQMMC supply\n");
	return ret;
}
"


>
>> 
>> mmc_power_init will first invoke board_mmc_power_init to
>> avoid break boards which already implement board_mmc_power_init.
>> 
>> If DM_MMC and DM_REGULATOR is defined, the regulator
>> will be enabled to power up the device.
>> 
>> Signed-off-by: Peng Fan <peng.fan@nxp.com>
>> Cc: Jaehoon Chung <jh80.chung@samsung.com>
>> ---
>> 
>> The RFC patset thread: http://lists.denx.de/pipermail/u-boot/2016-April/251019.html
>> V1: Use a generic way to handle vmmc supply, but not let vendor driver
>>     to handle it.
>> 
>>  drivers/mmc/mmc.c | 30 +++++++++++++++++++++++++++++-
>>  1 file changed, 29 insertions(+), 1 deletion(-)
>> 
>> diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c
>> index 0312da9..c361098 100644
>> --- a/drivers/mmc/mmc.c
>> +++ b/drivers/mmc/mmc.c
>> @@ -15,6 +15,7 @@
>>  #include <errno.h>
>>  #include <mmc.h>
>>  #include <part.h>
>> +#include <power/regulator.h>
>>  #include <malloc.h>
>>  #include <memalign.h>
>>  #include <linux/list.h>
>> @@ -1582,6 +1583,31 @@ __weak void board_mmc_power_init(void)
>>  {
>>  }
>>  
>> +int mmc_power_init(struct mmc *mmc)
>
>Can be static?

Sure. Fix in V2.

>
>> +{
>> +	board_mmc_power_init();
>> +
>> +#if defined(CONFIG_DM_MMC) && defined(CONFIG_DM_REGULATOR) && \
>> +	!defined(CONFIG_SPL_BUILD)
>> +	struct udevice *vmmc_supply;
>> +	int ret;
>> +
>> +	ret = device_get_supply_regulator(mmc->dev, "vmmc-supply",
>> +					  &vmmc_supply);
>> +	if (ret) {
>> +		debug("No vmmc supply\n");
>> +		return 0;
>
>"return 0" is Right? Doesn't need to return error?

In U-Boot, not every board supports regulator now. Some boards may
supports DM MMC, but DM REGULATOR not supported now. So I did not
use error code here. Or use puts, but not debug?

Any comments?

Regards,
Peng.