| Message ID | 1475921271-29093-1-git-send-email-peng.fan@nxp.com |
|---|---|
| State | Superseded |
| Delegated to: | Stefano Babic |
| Headers | show |
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? > > 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? > +{ > + 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? > + } > + > + ret = regulator_set_enable(vmmc_supply, true); > + if (ret) { > + puts("Error enabling VMMC supply\n"); > + return ret; > + } > +#endif > + return 0; > +} > + > int mmc_start_init(struct mmc *mmc) > { > bool no_card; > @@ -1606,7 +1632,9 @@ int mmc_start_init(struct mmc *mmc) > #ifdef CONFIG_FSL_ESDHC_ADAPTER_IDENT > mmc_adapter_card_type_ident(); > #endif > - board_mmc_power_init(); > + err = mmc_power_init(mmc); > + if (err) > + return err; > > #ifdef CONFIG_DM_MMC_OPS > /* The device has already been probed ready for use */ >
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.
Hi Peng, On 10/11/2016 03:37 PM, Peng Fan wrote: > 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. Then just remain this as future work. :) I think there is no use-case in uboot yet.. > > " > 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? Ok. Nothing. When you send patch v2, I will apply on u-boot-mmc, after checking. Thanks! Best Reagrds, Jaehoon Chung > > Any comments? > > Regards, > Peng. > > >
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) +{ + 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; + } + + ret = regulator_set_enable(vmmc_supply, true); + if (ret) { + puts("Error enabling VMMC supply\n"); + return ret; + } +#endif + return 0; +} + int mmc_start_init(struct mmc *mmc) { bool no_card; @@ -1606,7 +1632,9 @@ int mmc_start_init(struct mmc *mmc) #ifdef CONFIG_FSL_ESDHC_ADAPTER_IDENT mmc_adapter_card_type_ident(); #endif - board_mmc_power_init(); + err = mmc_power_init(mmc); + if (err) + return err; #ifdef CONFIG_DM_MMC_OPS /* The device has already been probed ready for use */
In device tree, there is vmmc-supply property for SD/MMC. Introduce mmc_power_init function to handle vmmc-supply. 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(-)