[U-Boot] [PATCH] omap_hsmmc: Allow for board-specific MMC power init

[U-Boot] [PATCH] omap_hsmmc: Allow for board-specific MMC power init

[Paul Kocialkowski]

Some devices may use non-standard combinations of regulators to power MMC:
this allows these devices to provide a board-specific MMC power init function
to set everything up in their own way.

Signed-off-by: Paul Kocialkowski <contact@paulk.fr>
---
 arch/arm/include/asm/omap_mmc.h |    4 +++-
 drivers/mmc/omap_hsmmc.c        |    4 +++-
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/arch/arm/include/asm/omap_mmc.h b/arch/arm/include/asm/omap_mmc.h
index 617e22f..b6a8325 100644
--- a/arch/arm/include/asm/omap_mmc.h
+++ b/arch/arm/include/asm/omap_mmc.h
@@ -164,5 +164,7 @@ struct hsmmc {
 int omap_mmc_init(int dev_index, uint host_caps_mask, uint f_max, int cd_gpio,
 		int wp_gpio);
 
-
+#ifdef CONFIG_OMAP_HSMMC_BOARD_POWER_INIT
+void omap_hsmmc_board_power_init(void);
+#endif
 #endif /* OMAP_MMC_H_ */
diff --git a/drivers/mmc/omap_hsmmc.c b/drivers/mmc/omap_hsmmc.c
index ef2cbf9..ef4c5cf 100644
--- a/drivers/mmc/omap_hsmmc.c
+++ b/drivers/mmc/omap_hsmmc.c
@@ -136,7 +136,9 @@ static unsigned char mmc_board_init(struct mmc *mmc)
 	pbias_lite &= ~(PBIASLITEPWRDNZ1 | PBIASLITEPWRDNZ0);
 	writel(pbias_lite, &t2_base->pbias_lite);
 #endif
-#if defined(CONFIG_TWL4030_POWER)
+#if defined(CONFIG_OMAP_HSMMC_BOARD_POWER_INIT)
+	omap_hsmmc_board_power_init();
+#elif defined(CONFIG_TWL4030_POWER)
 	twl4030_power_mmc_init();
 	mdelay(100);	/* ramp-up delay from Linux code */
 #endif
-- 
1.7.9.5

[U-Boot] [PATCH] omap_hsmmc: Allow for board-specific MMC power init

[Igor Grinberg]

Hi Paul,

On 10/28/14 19:25, Paul Kocialkowski wrote:
> Some devices may use non-standard combinations of regulators to power MMC:
> this allows these devices to provide a board-specific MMC power init function
> to set everything up in their own way.
> 
> Signed-off-by: Paul Kocialkowski <contact@paulk.fr>
> ---
>  arch/arm/include/asm/omap_mmc.h |    4 +++-
>  drivers/mmc/omap_hsmmc.c        |    4 +++-
>  2 files changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm/include/asm/omap_mmc.h b/arch/arm/include/asm/omap_mmc.h
> index 617e22f..b6a8325 100644
> --- a/arch/arm/include/asm/omap_mmc.h
> +++ b/arch/arm/include/asm/omap_mmc.h
> @@ -164,5 +164,7 @@ struct hsmmc {
>  int omap_mmc_init(int dev_index, uint host_caps_mask, uint f_max, int cd_gpio,
>  		int wp_gpio);
>  
> -
> +#ifdef CONFIG_OMAP_HSMMC_BOARD_POWER_INIT

I'm not a huge fan of that approach, but if you add
yet another CONFIG_ option, I think it is a requirement to add
a documentation for it.

> +void omap_hsmmc_board_power_init(void);

Anyway, I would suggest adding a default
__weak board_mmc_power_init() or something like this
(which would be transfered into a callback in pdata once omap_hsmmc.c is).

Or... just no need for this patch at all, as board_mmc_init()
can be used for this...

> +#endif
>  #endif /* OMAP_MMC_H_ */
> diff --git a/drivers/mmc/omap_hsmmc.c b/drivers/mmc/omap_hsmmc.c
> index ef2cbf9..ef4c5cf 100644
> --- a/drivers/mmc/omap_hsmmc.c
> +++ b/drivers/mmc/omap_hsmmc.c
> @@ -136,7 +136,9 @@ static unsigned char mmc_board_init(struct mmc *mmc)
>  	pbias_lite &= ~(PBIASLITEPWRDNZ1 | PBIASLITEPWRDNZ0);
>  	writel(pbias_lite, &t2_base->pbias_lite);
>  #endif
> -#if defined(CONFIG_TWL4030_POWER)
> +#if defined(CONFIG_OMAP_HSMMC_BOARD_POWER_INIT)
> +	omap_hsmmc_board_power_init();
> +#elif defined(CONFIG_TWL4030_POWER)
>  	twl4030_power_mmc_init();
>  	mdelay(100);	/* ramp-up delay from Linux code */
>  #endif
> 

-- 
Regards,
Igor.

[U-Boot] [PATCH] omap_hsmmc: Allow for board-specific MMC power init

[Paul Kocialkowski]

Le mardi 28 octobre 2014 ? 20:02 +0200, Igor Grinberg a ?crit :
> Hi Paul,
> 
> On 10/28/14 19:25, Paul Kocialkowski wrote:
> > Some devices may use non-standard combinations of regulators to power MMC:
> > this allows these devices to provide a board-specific MMC power init function
> > to set everything up in their own way.
> > 
> > Signed-off-by: Paul Kocialkowski <contact@paulk.fr>
> > ---
> >  arch/arm/include/asm/omap_mmc.h |    4 +++-
> >  drivers/mmc/omap_hsmmc.c        |    4 +++-
> >  2 files changed, 6 insertions(+), 2 deletions(-)
> > 
> > diff --git a/arch/arm/include/asm/omap_mmc.h b/arch/arm/include/asm/omap_mmc.h
> > index 617e22f..b6a8325 100644
> > --- a/arch/arm/include/asm/omap_mmc.h
> > +++ b/arch/arm/include/asm/omap_mmc.h
> > @@ -164,5 +164,7 @@ struct hsmmc {
> >  int omap_mmc_init(int dev_index, uint host_caps_mask, uint f_max, int cd_gpio,
> >  		int wp_gpio);
> >  
> > -
> > +#ifdef CONFIG_OMAP_HSMMC_BOARD_POWER_INIT
> 
> I'm not a huge fan of that approach, but if you add
> yet another CONFIG_ option, I think it is a requirement to add
> a documentation for it.

That saddens me too, but I don't see how to do this in a better way for
now.

> > +void omap_hsmmc_board_power_init(void);
> 
> Anyway, I would suggest adding a default
> __weak board_mmc_power_init() or something like this
> (which would be transfered into a callback in pdata once omap_hsmmc.c is).

Well, there are two distinct scenarios here:
1. the board follows some reference design and VMMC1 is used to power
MMC1, VMMC2 for MMC2, then twl4030 has a function that will set
everything up correctly (with the addition I added earlier)
2. the board uses the regulators in a more or less random way and VMMC1
is not mapped to MMC1 power at all. This is the case on the LG Optimus
Black (P970). There is actually an auxiliary regulator IC that is in
charge of powering MMC1. Hence the need for board-specific behavior.

I think it's perfectly fine to keep using the twl4030 function when the
board follows the reference design.

Would that board_mmc_power_init you suggest be generic to all MMC
drivers? Then at which point should it be called? What would a generic
board_mmc_power_init have? TWL4030 is specific to OMAP3. Then perhaps
there should be a board_mmc_power_init implementation for OMAP3, that
can be overridden by a  board-specific function?

I'm a bit new to the U-Boot code, so I'd appreciate pointers on what
should go where.

> Or... just no need for this patch at all, as board_mmc_init()
> can be used for this...

Not in the context of the SPL, which is precisely why I needed this.

Thanks for your review!

> > +#endif
> >  #endif /* OMAP_MMC_H_ */
> > diff --git a/drivers/mmc/omap_hsmmc.c b/drivers/mmc/omap_hsmmc.c
> > index ef2cbf9..ef4c5cf 100644
> > --- a/drivers/mmc/omap_hsmmc.c
> > +++ b/drivers/mmc/omap_hsmmc.c
> > @@ -136,7 +136,9 @@ static unsigned char mmc_board_init(struct mmc *mmc)
> >  	pbias_lite &= ~(PBIASLITEPWRDNZ1 | PBIASLITEPWRDNZ0);
> >  	writel(pbias_lite, &t2_base->pbias_lite);
> >  #endif
> > -#if defined(CONFIG_TWL4030_POWER)
> > +#if defined(CONFIG_OMAP_HSMMC_BOARD_POWER_INIT)
> > +	omap_hsmmc_board_power_init();
> > +#elif defined(CONFIG_TWL4030_POWER)
> >  	twl4030_power_mmc_init();
> >  	mdelay(100);	/* ramp-up delay from Linux code */
> >  #endif
> > 
> 

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20141028/48fbfb6a/attachment.pgp>

[U-Boot] [PATCH] omap_hsmmc: Allow for board-specific MMC power init

[Igor Grinberg]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Hi Paul,

On 10/28/14 20:11, Paul Kocialkowski wrote:
> Le mardi 28 octobre 2014 ? 20:02 +0200, Igor Grinberg a ?crit :
>> Hi Paul,
>>
>> On 10/28/14 19:25, Paul Kocialkowski wrote:
>>> Some devices may use non-standard combinations of regulators to power MMC:
>>> this allows these devices to provide a board-specific MMC power init function
>>> to set everything up in their own way.
>>>
>>> Signed-off-by: Paul Kocialkowski <contact@paulk.fr>
>>> ---
>>>  arch/arm/include/asm/omap_mmc.h |    4 +++-
>>>  drivers/mmc/omap_hsmmc.c        |    4 +++-
>>>  2 files changed, 6 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/arch/arm/include/asm/omap_mmc.h b/arch/arm/include/asm/omap_mmc.h
>>> index 617e22f..b6a8325 100644
>>> --- a/arch/arm/include/asm/omap_mmc.h
>>> +++ b/arch/arm/include/asm/omap_mmc.h
>>> @@ -164,5 +164,7 @@ struct hsmmc {
>>>  int omap_mmc_init(int dev_index, uint host_caps_mask, uint f_max, int cd_gpio,
>>>  		int wp_gpio);
>>>  
>>> -
>>> +#ifdef CONFIG_OMAP_HSMMC_BOARD_POWER_INIT
>>
>> I'm not a huge fan of that approach, but if you add
>> yet another CONFIG_ option, I think it is a requirement to add
>> a documentation for it.
> 
> That saddens me too, but I don't see how to do this in a better way for
> now.
> 
>>> +void omap_hsmmc_board_power_init(void);
>>
>> Anyway, I would suggest adding a default
>> __weak board_mmc_power_init() or something like this
>> (which would be transfered into a callback in pdata once omap_hsmmc.c is).
> 
> Well, there are two distinct scenarios here:
> 1. the board follows some reference design and VMMC1 is used to power
> MMC1, VMMC2 for MMC2, then twl4030 has a function that will set
> everything up correctly (with the addition I added earlier)
> 2. the board uses the regulators in a more or less random way and VMMC1
> is not mapped to MMC1 power at all. This is the case on the LG Optimus
> Black (P970). There is actually an auxiliary regulator IC that is in
> charge of powering MMC1. Hence the need for board-specific behavior.
> 
> I think it's perfectly fine to keep using the twl4030 function when the
> board follows the reference design.
> 
> Would that board_mmc_power_init you suggest be generic to all MMC
> drivers?

That would be perfect, I think.

> Then at which point should it be called? What would a generic
> board_mmc_power_init have?

Well, those are the question you usually should answer when you write
common code. I hopefully will have time to look at this next week.

> TWL4030 is specific to OMAP3.

Well, in practice, it is, but it does not have to be...
Also, it is a question of how the board is wired...

> Then perhaps
> there should be a board_mmc_power_init implementation for OMAP3, that
> can be overridden by a  board-specific function?

Or, as you have proposed already, a common board_mmc_power_init() function
and a board can choose to run the twl4030_power_mmc_init()?

> 
> I'm a bit new to the U-Boot code, so I'd appreciate pointers on what
> should go where.
> 
>> Or... just no need for this patch at all, as board_mmc_init()
>> can be used for this...
> 
> Not in the context of the SPL, which is precisely why I needed this.

This might lead to a question - is current spl flexible enough to let
a board deal with the mmc power?

Tom?

> 
> Thanks for your review!
> 
>>> +#endif
>>>  #endif /* OMAP_MMC_H_ */
>>> diff --git a/drivers/mmc/omap_hsmmc.c b/drivers/mmc/omap_hsmmc.c
>>> index ef2cbf9..ef4c5cf 100644
>>> --- a/drivers/mmc/omap_hsmmc.c
>>> +++ b/drivers/mmc/omap_hsmmc.c
>>> @@ -136,7 +136,9 @@ static unsigned char mmc_board_init(struct mmc *mmc)
>>>  	pbias_lite &= ~(PBIASLITEPWRDNZ1 | PBIASLITEPWRDNZ0);
>>>  	writel(pbias_lite, &t2_base->pbias_lite);
>>>  #endif
>>> -#if defined(CONFIG_TWL4030_POWER)
>>> +#if defined(CONFIG_OMAP_HSMMC_BOARD_POWER_INIT)
>>> +	omap_hsmmc_board_power_init();
>>> +#elif defined(CONFIG_TWL4030_POWER)
>>>  	twl4030_power_mmc_init();
>>>  	mdelay(100);	/* ramp-up delay from Linux code */
>>>  #endif
>>>
>>
> 

- -- 
Regards,
Igor.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2

iQIcBAEBAgAGBQJUUOcyAAoJEBDE8YO64EfaA00P/RRjiz3AyVqGJdVcN85eRim4
oYxr2/q5jaj67zD6x6UD4eT/VSnJQXaxNEmFQVNjoZc4U6U0V7qjrTC2/USh6v6Q
GROIH7zK/uP9+rRnJWJYdjAjZMJNShNmUvP9/oIb8ZtvVKG1aEuRoRWYwlyirJr/
cmtzeFS/TwaWSgFrtDWmCXuXzyKUdB+A/AOjAIj7HMZjKV+gaSZMSSyv5H6uQK+3
eI7KUskYvTJFCKKez0jnXv3IkhKjeJxy7eKQ147nfQMcZJ+dby3cbJuS3aZSG2xI
jx+w6ruSm4DMYJeCsEoWhW1wFJtKrtqZ4TyYEQeC9PWoON/8b67Jy+SYTHnVBkJk
qR7UFEyttUIE6zulD5rf2SlxS0e96HkLqNOfaI2K8rqbKuNOy+ZlA8TKsaVx5QAH
Fsxn494TY38vgBTYQbf74pv7psw2mPT//FUF//HqTuAID9ne6pMgM+j+WE0GwWOI
CZw9ddutxj1ksFu0kvnldvfZ6F4yd4aJFPUPQEDa6/qkVrKEW7Y3Alov5gHQMHQ8
zkhyv60s3owlmtcvkRtmb4LcRh+/lm8+gO9u7jmFmtiIhkCuEQA7IopqqYRdibXC
SiKyGH53TTLg/zocixO1/vY/a5C2YHADaVJ6Rzzea6IDUeHBoWTKhU4vUudjOb5z
NCH2ncJjDN8moX9CBENr
=3a3G
-----END PGP SIGNATURE-----

[U-Boot] [PATCH] omap_hsmmc: Allow for board-specific MMC power init

[Pantelis Antoniou]

Hi Paul,

> On Oct 28, 2014, at 19:25 , Paul Kocialkowski <contact@paulk.fr> wrote:
> 
> Some devices may use non-standard combinations of regulators to power MMC:
> this allows these devices to provide a board-specific MMC power init function
> to set everything up in their own way.
> 
> Signed-off-by: Paul Kocialkowski <contact@paulk.fr>
> ---
> arch/arm/include/asm/omap_mmc.h |    4 +++-
> drivers/mmc/omap_hsmmc.c        |    4 +++-
> 2 files changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm/include/asm/omap_mmc.h b/arch/arm/include/asm/omap_mmc.h
> index 617e22f..b6a8325 100644
> --- a/arch/arm/include/asm/omap_mmc.h
> +++ b/arch/arm/include/asm/omap_mmc.h
> @@ -164,5 +164,7 @@ struct hsmmc {
> int omap_mmc_init(int dev_index, uint host_caps_mask, uint f_max, int cd_gpio,
> 		int wp_gpio);
> 
> -
> +#ifdef CONFIG_OMAP_HSMMC_BOARD_POWER_INIT
> +void omap_hsmmc_board_power_init(void);
> +#endif
> #endif /* OMAP_MMC_H_ */
> diff --git a/drivers/mmc/omap_hsmmc.c b/drivers/mmc/omap_hsmmc.c
> index ef2cbf9..ef4c5cf 100644
> --- a/drivers/mmc/omap_hsmmc.c
> +++ b/drivers/mmc/omap_hsmmc.c
> @@ -136,7 +136,9 @@ static unsigned char mmc_board_init(struct mmc *mmc)
> 	pbias_lite &= ~(PBIASLITEPWRDNZ1 | PBIASLITEPWRDNZ0);
> 	writel(pbias_lite, &t2_base->pbias_lite);
> #endif
> -#if defined(CONFIG_TWL4030_POWER)
> +#if defined(CONFIG_OMAP_HSMMC_BOARD_POWER_INIT)
> +	omap_hsmmc_board_power_init();
> +#elif defined(CONFIG_TWL4030_POWER)
> 	twl4030_power_mmc_init();
> 	mdelay(100);	/* ramp-up delay from Linux code */
> #endif
> -- 
> 1.7.9.5
> 

Igor covered most of what I was going to whine about.

I?m not a big fun of the define maze here, let?s not add to it, shall we?

A common board_mmc_power_init() function would be fine in this occasion.

Regards

? Pantelis

[U-Boot] [PATCH v2 1/2] mmc: Board-specific MMC power initializations

[Paul Kocialkowski]

Some devices may use non-standard combinations of regulators to power MMC:
this allows these devices to provide a board-specific MMC power init function
to set everything up in their own way.

Signed-off-by: Paul Kocialkowski <contact@paulk.fr>
---
 drivers/mmc/mmc.c | 8 ++++++++
 include/mmc.h     | 2 ++
 2 files changed, 10 insertions(+)

diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c
index 44a4feb..125f347 100644
--- a/drivers/mmc/mmc.c
+++ b/drivers/mmc/mmc.c
@@ -1277,6 +1277,12 @@ block_dev_desc_t *mmc_get_dev(int dev)
 }
 #endif
 
+/* board-specific MMC power initializations. */
+__weak int board_mmc_power_init(void)
+{
+	return -1;
+}
+
 int mmc_start_init(struct mmc *mmc)
 {
 	int err;
@@ -1293,6 +1299,8 @@ int mmc_start_init(struct mmc *mmc)
 	if (mmc->has_init)
 		return 0;
 
+	board_mmc_power_init();
+
 	/* made sure it's not NULL earlier */
 	err = mmc->cfg->ops->init(mmc);
 
diff --git a/include/mmc.h b/include/mmc.h
index 7f5f9bc..577e269 100644
--- a/include/mmc.h
+++ b/include/mmc.h
@@ -388,6 +388,8 @@ int mmc_legacy_init(int verbose);
 int board_mmc_init(bd_t *bis);
 int cpu_mmc_init(bd_t *bis);
 
+int board_mmc_power_init(void);
+
 /* Set block count limit because of 16 bit register limit on some hardware*/
 #ifndef CONFIG_SYS_MMC_MAX_BLK_COUNT
 #define CONFIG_SYS_MMC_MAX_BLK_COUNT 65535
-- 
1.9.1

[U-Boot] [PATCH v2 2/2] omap_hsmmc: Board-specific TWL4030 MMC power initializations

[Paul Kocialkowski]

Boards using the TWL4030 regulator may not all use the LDOs the same way
(e.g. MMC2 power can be controlled by another LDO than VMMC2).
This delegates TWL4030 MMC power initializations to board-specific functions,
that may still call twl4030_power_mmc_init for the default behavior.

Signed-off-by: Paul Kocialkowski <contact@paulk.fr>
---
 board/comelit/dig297/dig297.c          |  9 +++++++++
 board/compulab/cm_t35/cm_t35.c         | 11 +++++++++++
 board/corscience/tricorder/tricorder.c | 11 +++++++++++
 board/isee/igep00x0/igep00x0.c         | 11 +++++++++++
 board/logicpd/omap3som/omap3logic.c    | 11 +++++++++++
 board/logicpd/zoom1/zoom1.c            |  9 +++++++++
 board/matrix_vision/mvblx/mvblx.c      |  9 +++++++++
 board/nokia/rx51/rx51.c                |  9 +++++++++
 board/overo/overo.c                    | 11 +++++++++++
 board/pandora/pandora.c                |  9 +++++++++
 board/technexion/tao3530/tao3530.c     | 11 +++++++++++
 board/ti/beagle/beagle.c               | 11 +++++++++++
 board/ti/evm/evm.c                     | 11 +++++++++++
 board/ti/sdp3430/sdp.c                 |  9 +++++++++
 board/timll/devkit8000/devkit8000.c    | 11 +++++++++++
 drivers/mmc/omap_hsmmc.c               |  7 +------
 16 files changed, 154 insertions(+), 6 deletions(-)

diff --git a/board/comelit/dig297/dig297.c b/board/comelit/dig297/dig297.c
index 2b826df..784483b 100644
--- a/board/comelit/dig297/dig297.c
+++ b/board/comelit/dig297/dig297.c
@@ -133,6 +133,15 @@ int board_mmc_init(bd_t *bis)
 {
 	return omap_mmc_init(0, 0, 0, -1, -1);
 }
+
+int board_mmc_power_init(void)
+{
+#if defined(CONFIG_TWL4030_POWER)
+	twl4030_power_mmc_init();
+	mdelay(100);	/* ramp-up delay from Linux code */
+#endif
+	return 0;
+}
 #endif
 
 #ifdef CONFIG_CMD_NET
diff --git a/board/compulab/cm_t35/cm_t35.c b/board/compulab/cm_t35/cm_t35.c
index 0944903..93de1de 100644
--- a/board/compulab/cm_t35/cm_t35.c
+++ b/board/compulab/cm_t35/cm_t35.c
@@ -473,6 +473,17 @@ int board_mmc_init(bd_t *bis)
 }
 #endif
 
+#if defined(CONFIG_GENERIC_MMC)
+int board_mmc_power_init(void)
+{
+#if defined(CONFIG_TWL4030_POWER)
+	twl4030_power_mmc_init();
+	mdelay(100);	/* ramp-up delay from Linux code */
+#endif
+	return 0;
+}
+#endif
+
 /*
  * Routine: setup_net_chip_gmpc
  * Description: Setting up the configuration GPMC registers specific to the
diff --git a/board/corscience/tricorder/tricorder.c b/board/corscience/tricorder/tricorder.c
index 9e81bf3..9ade210 100644
--- a/board/corscience/tricorder/tricorder.c
+++ b/board/corscience/tricorder/tricorder.c
@@ -147,6 +147,17 @@ int board_mmc_init(bd_t *bis)
 }
 #endif
 
+#if defined(CONFIG_GENERIC_MMC)
+int board_mmc_power_init(void)
+{
+#if defined(CONFIG_TWL4030_POWER)
+	twl4030_power_mmc_init();
+	mdelay(100);	/* ramp-up delay from Linux code */
+#endif
+	return 0;
+}
+#endif
+
 /*
  * Routine: get_board_mem_timings
  * Description: If we use SPL then there is no x-loader nor config header
diff --git a/board/isee/igep00x0/igep00x0.c b/board/isee/igep00x0/igep00x0.c
index 3b2b1f1..dd88b75 100644
--- a/board/isee/igep00x0/igep00x0.c
+++ b/board/isee/igep00x0/igep00x0.c
@@ -137,6 +137,17 @@ int board_mmc_init(bd_t *bis)
 }
 #endif
 
+#if defined(CONFIG_GENERIC_MMC)
+int board_mmc_power_init(void)
+{
+#if defined(CONFIG_TWL4030_POWER)
+	twl4030_power_mmc_init();
+	mdelay(100);	/* ramp-up delay from Linux code */
+#endif
+	return 0;
+}
+#endif
+
 void set_fdt(void)
 {
 	switch (gd->bd->bi_arch_number) {
diff --git a/board/logicpd/omap3som/omap3logic.c b/board/logicpd/omap3som/omap3logic.c
index 075fe94..8e19b6b 100644
--- a/board/logicpd/omap3som/omap3logic.c
+++ b/board/logicpd/omap3som/omap3logic.c
@@ -128,6 +128,17 @@ int board_mmc_init(bd_t *bis)
 }
 #endif
 
+#if defined(CONFIG_GENERIC_MMC)
+int board_mmc_power_init(void)
+{
+#if defined(CONFIG_TWL4030_POWER)
+	twl4030_power_mmc_init();
+	mdelay(100);	/* ramp-up delay from Linux code */
+#endif
+	return 0;
+}
+#endif
+
 #ifdef CONFIG_SMC911X
 /* GPMC CS1 settings for Logic SOM LV/Torpedo LAN92xx Ethernet chip */
 static const u32 gpmc_lan92xx_config[] = {
diff --git a/board/logicpd/zoom1/zoom1.c b/board/logicpd/zoom1/zoom1.c
index 461a852..c06299c 100644
--- a/board/logicpd/zoom1/zoom1.c
+++ b/board/logicpd/zoom1/zoom1.c
@@ -96,6 +96,15 @@ int board_mmc_init(bd_t *bis)
 {
 	return omap_mmc_init(0, 0, 0, -1, -1);
 }
+
+int board_mmc_power_init(void)
+{
+#if defined(CONFIG_TWL4030_POWER)
+	twl4030_power_mmc_init();
+	mdelay(100);	/* ramp-up delay from Linux code */
+#endif
+	return 0;
+}
 #endif
 
 #ifdef CONFIG_CMD_NET
diff --git a/board/matrix_vision/mvblx/mvblx.c b/board/matrix_vision/mvblx/mvblx.c
index a69359f..1f4ca47 100644
--- a/board/matrix_vision/mvblx/mvblx.c
+++ b/board/matrix_vision/mvblx/mvblx.c
@@ -94,6 +94,15 @@ int board_mmc_init(bd_t *bis)
 	omap_mmc_init(1, 0, 0, -1, -1);
 	return 0;
 }
+
+int board_mmc_power_init(void)
+{
+#if defined(CONFIG_TWL4030_POWER)
+	twl4030_power_mmc_init();
+	mdelay(100);	/* ramp-up delay from Linux code */
+#endif
+	return 0;
+}
 #endif
 
 #if defined(CONFIG_CMD_NET)
diff --git a/board/nokia/rx51/rx51.c b/board/nokia/rx51/rx51.c
index c2e07db..e313fc2 100644
--- a/board/nokia/rx51/rx51.c
+++ b/board/nokia/rx51/rx51.c
@@ -659,3 +659,12 @@ int board_mmc_init(bd_t *bis)
 	omap_mmc_init(1, 0, 0, -1, -1);
 	return 0;
 }
+
+int board_mmc_power_init(void)
+{
+#if defined(CONFIG_TWL4030_POWER)
+	twl4030_power_mmc_init();
+	mdelay(100);	/* ramp-up delay from Linux code */
+#endif
+	return 0;
+}
diff --git a/board/overo/overo.c b/board/overo/overo.c
index 13220c5..d86eca4 100644
--- a/board/overo/overo.c
+++ b/board/overo/overo.c
@@ -437,3 +437,14 @@ int board_mmc_init(bd_t *bis)
 	return omap_mmc_init(0, 0, 0, -1, -1);
 }
 #endif
+
+#if defined(CONFIG_GENERIC_MMC)
+int board_mmc_power_init(void)
+{
+#if defined(CONFIG_TWL4030_POWER)
+	twl4030_power_mmc_init();
+	mdelay(100);	/* ramp-up delay from Linux code */
+#endif
+	return 0;
+}
+#endif
diff --git a/board/pandora/pandora.c b/board/pandora/pandora.c
index 146dcea..8fa0119 100644
--- a/board/pandora/pandora.c
+++ b/board/pandora/pandora.c
@@ -126,4 +126,13 @@ int board_mmc_init(bd_t *bis)
 {
 	return omap_mmc_init(0, 0, 0, -1, -1);
 }
+
+int board_mmc_power_init(void)
+{
+#if defined(CONFIG_TWL4030_POWER)
+	twl4030_power_mmc_init();
+	mdelay(100);	/* ramp-up delay from Linux code */
+#endif
+	return 0;
+}
 #endif
diff --git a/board/technexion/tao3530/tao3530.c b/board/technexion/tao3530/tao3530.c
index 44a8240..64f2313 100644
--- a/board/technexion/tao3530/tao3530.c
+++ b/board/technexion/tao3530/tao3530.c
@@ -188,6 +188,17 @@ int board_mmc_init(bd_t *bis)
 }
 #endif
 
+#if defined(CONFIG_GENERIC_MMC)
+int board_mmc_power_init(void)
+{
+#if defined(CONFIG_TWL4030_POWER)
+	twl4030_power_mmc_init();
+	mdelay(100);	/* ramp-up delay from Linux code */
+#endif
+	return 0;
+}
+#endif
+
 #if defined(CONFIG_USB_EHCI) && !defined(CONFIG_SPL_BUILD)
 /* Call usb_stop() before starting the kernel */
 void show_boot_progress(int val)
diff --git a/board/ti/beagle/beagle.c b/board/ti/beagle/beagle.c
index 94b99bf..b47e44b 100644
--- a/board/ti/beagle/beagle.c
+++ b/board/ti/beagle/beagle.c
@@ -521,6 +521,17 @@ int board_mmc_init(bd_t *bis)
 }
 #endif
 
+#if defined(CONFIG_GENERIC_MMC)
+int board_mmc_power_init(void)
+{
+#if defined(CONFIG_TWL4030_POWER)
+	twl4030_power_mmc_init();
+	mdelay(100);	/* ramp-up delay from Linux code */
+#endif
+	return 0;
+}
+#endif
+
 #if defined(CONFIG_USB_EHCI) && !defined(CONFIG_SPL_BUILD)
 /* Call usb_stop() before starting the kernel */
 void show_boot_progress(int val)
diff --git a/board/ti/evm/evm.c b/board/ti/evm/evm.c
index 81dd081..c41a760 100644
--- a/board/ti/evm/evm.c
+++ b/board/ti/evm/evm.c
@@ -264,3 +264,14 @@ int board_mmc_init(bd_t *bis)
 	return omap_mmc_init(0, 0, 0, -1, -1);
 }
 #endif
+
+#if defined(CONFIG_GENERIC_MMC)
+int board_mmc_power_init(void)
+{
+#if defined(CONFIG_TWL4030_POWER)
+	twl4030_power_mmc_init();
+	mdelay(100);	/* ramp-up delay from Linux code */
+#endif
+	return 0;
+}
+#endif
diff --git a/board/ti/sdp3430/sdp.c b/board/ti/sdp3430/sdp.c
index 957940d..44e0418 100644
--- a/board/ti/sdp3430/sdp.c
+++ b/board/ti/sdp3430/sdp.c
@@ -195,4 +195,13 @@ int board_mmc_init(bd_t *bis)
 {
 	return omap_mmc_init(0, 0, 0, -1, -1);
 }
+
+int board_mmc_power_init(void)
+{
+#if defined(CONFIG_TWL4030_POWER)
+	twl4030_power_mmc_init();
+	mdelay(100);	/* ramp-up delay from Linux code */
+#endif
+	return 0;
+}
 #endif
diff --git a/board/timll/devkit8000/devkit8000.c b/board/timll/devkit8000/devkit8000.c
index bcbee73..ef7fd0c 100644
--- a/board/timll/devkit8000/devkit8000.c
+++ b/board/timll/devkit8000/devkit8000.c
@@ -124,6 +124,17 @@ int board_mmc_init(bd_t *bis)
 }
 #endif
 
+#if defined(CONFIG_GENERIC_MMC)
+int board_mmc_power_init(void)
+{
+#if defined(CONFIG_TWL4030_POWER)
+	twl4030_power_mmc_init();
+	mdelay(100);	/* ramp-up delay from Linux code */
+#endif
+	return 0;
+}
+#endif
+
 #if defined(CONFIG_DRIVER_DM9000) & !defined(CONFIG_SPL_BUILD)
 /*
  * Routine: board_eth_init
diff --git a/drivers/mmc/omap_hsmmc.c b/drivers/mmc/omap_hsmmc.c
index 5b0c302..6e93a07 100644
--- a/drivers/mmc/omap_hsmmc.c
+++ b/drivers/mmc/omap_hsmmc.c
@@ -130,12 +130,7 @@ static unsigned char mmc_board_init(struct mmc *mmc)
 	pbias_lite = readl(&t2_base->pbias_lite);
 	pbias_lite &= ~(PBIASLITEPWRDNZ1 | PBIASLITEPWRDNZ0);
 	writel(pbias_lite, &t2_base->pbias_lite);
-#endif
-#if defined(CONFIG_TWL4030_POWER)
-	twl4030_power_mmc_init();
-	mdelay(100);	/* ramp-up delay from Linux code */
-#endif
-#if defined(CONFIG_OMAP34XX)
+
 	writel(pbias_lite | PBIASLITEPWRDNZ1 |
 		PBIASSPEEDCTRL0 | PBIASLITEPWRDNZ0,
 		&t2_base->pbias_lite);
-- 
1.9.1

[U-Boot] [PATCH v2 2/2] omap_hsmmc: Board-specific TWL4030 MMC power initializations

[Tom Rini]

On Sat, Nov 01, 2014 at 11:35:44AM +0100, Paul Kocialkowski wrote:

> Boards using the TWL4030 regulator may not all use the LDOs the same way
> (e.g. MMC2 power can be controlled by another LDO than VMMC2).
> This delegates TWL4030 MMC power initializations to board-specific functions,
> that may still call twl4030_power_mmc_init for the default behavior.
[snip]
> +int board_mmc_power_init(void)
> +{
> +#if defined(CONFIG_TWL4030_POWER)
> +	twl4030_power_mmc_init();
> +	mdelay(100);	/* ramp-up delay from Linux code */
> +#endif
> +	return 0;
> +}
>  #endif

I think we can do away with the #if defined(CONFIG_TWL4030_POWER) part
here since all of these boards will fail pretty spectacularly without
that being set and if someone does remove that a link error is better
than a crazy runtime problem.

[snip]
> diff --git a/drivers/mmc/omap_hsmmc.c b/drivers/mmc/omap_hsmmc.c
> index 5b0c302..6e93a07 100644
> --- a/drivers/mmc/omap_hsmmc.c
> +++ b/drivers/mmc/omap_hsmmc.c
> @@ -130,12 +130,7 @@ static unsigned char mmc_board_init(struct mmc *mmc)
>  	pbias_lite = readl(&t2_base->pbias_lite);
>  	pbias_lite &= ~(PBIASLITEPWRDNZ1 | PBIASLITEPWRDNZ0);
>  	writel(pbias_lite, &t2_base->pbias_lite);
> -#endif
> -#if defined(CONFIG_TWL4030_POWER)
> -	twl4030_power_mmc_init();
> -	mdelay(100);	/* ramp-up delay from Linux code */
> -#endif
> -#if defined(CONFIG_OMAP34XX)
> +
>  	writel(pbias_lite | PBIASLITEPWRDNZ1 |
>  		PBIASSPEEDCTRL0 | PBIASLITEPWRDNZ0,
>  		&t2_base->pbias_lite);

Since it's not clear from the context here, it's OK to drop the check on
CONFIG_OMAP34XX here since there's already a test on it above where diff
cut us off?  Thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20141104/ad6a1559/attachment.pgp>

[U-Boot] [PATCH v2 1/2] mmc: Board-specific MMC power initializations

[Tom Rini]

On Sat, Nov 01, 2014 at 11:35:43AM +0100, Paul Kocialkowski wrote:

> Some devices may use non-standard combinations of regulators to power MMC:
> this allows these devices to provide a board-specific MMC power init function
> to set everything up in their own way.
> 
> Signed-off-by: Paul Kocialkowski <contact@paulk.fr>
> ---
>  drivers/mmc/mmc.c | 8 ++++++++
>  include/mmc.h     | 2 ++
>  2 files changed, 10 insertions(+)
> 
> diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c
> index 44a4feb..125f347 100644
> --- a/drivers/mmc/mmc.c
> +++ b/drivers/mmc/mmc.c
> @@ -1277,6 +1277,12 @@ block_dev_desc_t *mmc_get_dev(int dev)
>  }
>  #endif
>  
> +/* board-specific MMC power initializations. */
> +__weak int board_mmc_power_init(void)
> +{
> +	return -1;
> +}

Since we don't check error return here which I think is fine just make
this a void?  Thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20141104/66a3e08e/attachment.pgp>

[U-Boot] [PATCH v2 1/2] mmc: Board-specific MMC power initializations

[Igor Grinberg]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Hi Tom,

On 11/04/14 17:56, Tom Rini wrote:
> On Sat, Nov 01, 2014 at 11:35:43AM +0100, Paul Kocialkowski wrote:
> 
>> Some devices may use non-standard combinations of regulators to power MMC:
>> this allows these devices to provide a board-specific MMC power init function
>> to set everything up in their own way.
>>
>> Signed-off-by: Paul Kocialkowski <contact@paulk.fr>
>> ---
>>  drivers/mmc/mmc.c | 8 ++++++++
>>  include/mmc.h     | 2 ++
>>  2 files changed, 10 insertions(+)
>>
>> diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c
>> index 44a4feb..125f347 100644
>> --- a/drivers/mmc/mmc.c
>> +++ b/drivers/mmc/mmc.c
>> @@ -1277,6 +1277,12 @@ block_dev_desc_t *mmc_get_dev(int dev)
>>  }
>>  #endif
>>  
>> +/* board-specific MMC power initializations. */
>> +__weak int board_mmc_power_init(void)
>> +{
>> +	return -1;
>> +}
> 
> Since we don't check error return here which I think is fine just make
> this a void?  Thanks!

There is v3 posted a while ago...
We have also agreed on v4..


- -- 
Regards,
Igor.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2

iQIcBAEBAgAGBQJUWRPOAAoJEBDE8YO64EfafPsP/iow4r72B6oCuDDh7g9+BWYg
bO17RVU3PM14YgWQpybf/MXPgBSxyW3/4d69xrH5+TqEi9lOsHA/5X7ZW9r+xJh1
/JW1qwPKvqN9NdtGyPJppS0umoQoV77CzeTlLIdZ9emtozGSB/PpevAYK89HmrGl
g5XYzeX/uPPE9MrJ1GdeEk+bGSq3N7rSEAzWIiJ50Ai7A4t1RTYRtNAqMbf4q/Ip
WMO/MPiJk6Ybugk5vd91pJQkPtLIuFFEipUnIC8TSO7U8vWiOXKGgjH+aRvFhaLm
8YfU4Ym2vYZzIvpYbgHO6e2tKH0OhyzE/zZIDIGhOaXwPUsMbJquAvVEKz07XNu/
nOAeiBBhvTeMPehUe7jYaCymhxHJb3ZM29MOfz33a/GBskdMCNBtb/XoaibWbG1k
ZfIMktv0SDObRmd/eUCNvl09YWj8JB7aTzFg20ZoeLoyfW7S1aJ1L0AymaOY20n0
A7MXpEVU1ddPu1rIh9hKm8G86i04aarQJ0kJ4vooCZI+qPLndwH+6Go0Zny3Vbj5
v1SudRROl0KUIoJd/Lt5nmJgCsFsas9xXgJsQNuK3avRrwlPcTykJv1eHbYFlNTq
iFCjDAP7IU1iH0NBuJo7T2qPwPw9UMb3AQmo1JY/rFXopTVt2r0mkulZlAoMfWwy
L2cveucMAOGnEIFxhngN
=acIT
-----END PGP SIGNATURE-----

[U-Boot] [PATCH v2 1/2] mmc: Board-specific MMC power initializations

[Tom Rini]

On Tue, Nov 04, 2014 at 07:58:38PM +0200, Igor Grinberg wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
> 
> Hi Tom,
> 
> On 11/04/14 17:56, Tom Rini wrote:
> > On Sat, Nov 01, 2014 at 11:35:43AM +0100, Paul Kocialkowski wrote:
> > 
> >> Some devices may use non-standard combinations of regulators to power MMC:
> >> this allows these devices to provide a board-specific MMC power init function
> >> to set everything up in their own way.
> >>
> >> Signed-off-by: Paul Kocialkowski <contact@paulk.fr>
> >> ---
> >>  drivers/mmc/mmc.c | 8 ++++++++
> >>  include/mmc.h     | 2 ++
> >>  2 files changed, 10 insertions(+)
> >>
> >> diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c
> >> index 44a4feb..125f347 100644
> >> --- a/drivers/mmc/mmc.c
> >> +++ b/drivers/mmc/mmc.c
> >> @@ -1277,6 +1277,12 @@ block_dev_desc_t *mmc_get_dev(int dev)
> >>  }
> >>  #endif
> >>  
> >> +/* board-specific MMC power initializations. */
> >> +__weak int board_mmc_power_init(void)
> >> +{
> >> +	return -1;
> >> +}
> > 
> > Since we don't check error return here which I think is fine just make
> > this a void?  Thanks!
> 
> There is v3 posted a while ago...
> We have also agreed on v4..

Yeah, oops, didn't delete these after catch-up.  I'm still not sure we
should continue adding more unchecked return values "just because".

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20141104/313739eb/attachment.pgp>

[U-Boot] [PATCH v2 1/2] mmc: Board-specific MMC power initializations

[Paul Kocialkowski]

Hi there, thanks for the review,

Le mardi 04 novembre 2014 ? 13:32 -0500, Tom Rini a ?crit :
> On Tue, Nov 04, 2014 at 07:58:38PM +0200, Igor Grinberg wrote:
> > -----BEGIN PGP SIGNED MESSAGE-----
> > Hash: SHA1
> > 
> > Hi Tom,
> > 
> > On 11/04/14 17:56, Tom Rini wrote:
> > > On Sat, Nov 01, 2014 at 11:35:43AM +0100, Paul Kocialkowski wrote:
> > > 
> > >> Some devices may use non-standard combinations of regulators to power MMC:
> > >> this allows these devices to provide a board-specific MMC power init function
> > >> to set everything up in their own way.
> > >>
> > >> Signed-off-by: Paul Kocialkowski <contact@paulk.fr>
> > >> ---
> > >>  drivers/mmc/mmc.c | 8 ++++++++
> > >>  include/mmc.h     | 2 ++
> > >>  2 files changed, 10 insertions(+)
> > >>
> > >> diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c
> > >> index 44a4feb..125f347 100644
> > >> --- a/drivers/mmc/mmc.c
> > >> +++ b/drivers/mmc/mmc.c
> > >> @@ -1277,6 +1277,12 @@ block_dev_desc_t *mmc_get_dev(int dev)
> > >>  }
> > >>  #endif
> > >>  
> > >> +/* board-specific MMC power initializations. */
> > >> +__weak int board_mmc_power_init(void)
> > >> +{
> > >> +	return -1;
> > >> +}
> > > 
> > > Since we don't check error return here which I think is fine just make
> > > this a void?  Thanks!
> > 
> > There is v3 posted a while ago...
> > We have also agreed on v4..

Note that v3 and v4 are the same, except that v3 didn't apply on top of
master.

> Yeah, oops, didn't delete these after catch-up.  I'm still not sure we
> should continue adding more unchecked return values "just because".

I agree that we shouldn't have an unchecked return value. So we could
either check the return value and print a warning, without aborting the
init sequence (what Igor proposed initially) or just make this return
void (what you both seem to agree on).

I'm fine with both solutions. I guess that enabling a regulator could
fail (say, because of an i2c error), so there is still sense in
returning int.

Let me know of what your definitive answer on this is. I'll make a new
patchset probably this friday (I'm running on a very tight schedule
until then).

-- 
Paul Kocialkowski, Replicant developer

Replicant is a fully free Android distribution

Website: http://www.replicant.us/
Redmine: http://redmine.replicant.us/
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20141105/a90469ed/attachment.pgp>

[U-Boot] [PATCH v2 2/2] omap_hsmmc: Board-specific TWL4030 MMC power initializations

[Paul Kocialkowski]

> > Boards using the TWL4030 regulator may not all use the LDOs the same way
> > (e.g. MMC2 power can be controlled by another LDO than VMMC2).
> > This delegates TWL4030 MMC power initializations to board-specific functions,
> > that may still call twl4030_power_mmc_init for the default behavior.
> [snip]
> > +int board_mmc_power_init(void)
> > +{
> > +#if defined(CONFIG_TWL4030_POWER)
> > +	twl4030_power_mmc_init();
> > +	mdelay(100);	/* ramp-up delay from Linux code */
> > +#endif
> > +	return 0;
> > +}
> >  #endif
> 
> I think we can do away with the #if defined(CONFIG_TWL4030_POWER) part
> here since all of these boards will fail pretty spectacularly without
> that being set and if someone does remove that a link error is better
> than a crazy runtime problem.

That makes sense, I'll do that in v5.

> [snip]
> > diff --git a/drivers/mmc/omap_hsmmc.c b/drivers/mmc/omap_hsmmc.c
> > index 5b0c302..6e93a07 100644
> > --- a/drivers/mmc/omap_hsmmc.c
> > +++ b/drivers/mmc/omap_hsmmc.c
> > @@ -130,12 +130,7 @@ static unsigned char mmc_board_init(struct mmc *mmc)
> >  	pbias_lite = readl(&t2_base->pbias_lite);
> >  	pbias_lite &= ~(PBIASLITEPWRDNZ1 | PBIASLITEPWRDNZ0);
> >  	writel(pbias_lite, &t2_base->pbias_lite);
> > -#endif
> > -#if defined(CONFIG_TWL4030_POWER)
> > -	twl4030_power_mmc_init();
> > -	mdelay(100);	/* ramp-up delay from Linux code */
> > -#endif
> > -#if defined(CONFIG_OMAP34XX)
> > +
> >  	writel(pbias_lite | PBIASLITEPWRDNZ1 |
> >  		PBIASSPEEDCTRL0 | PBIASLITEPWRDNZ0,
> >  		&t2_base->pbias_lite);
> 
> Since it's not clear from the context here, it's OK to drop the check on
> CONFIG_OMAP34XX here since there's already a test on it above where diff
> cut us off?  Thanks!

Indeed, that's the case (not sure this was actually more of a question
than a statement).

Thanks for the review!
-- 
Paul Kocialkowski, Replicant developer

Replicant is a fully free Android distribution

Website: http://www.replicant.us/
Redmine: http://redmine.replicant.us/
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20141105/fb1440c8/attachment.pgp>

[U-Boot] [PATCH v2 1/2] mmc: Board-specific MMC power initializations

[Tom Rini]

On Wed, Nov 05, 2014 at 06:35:20PM +0100, Paul Kocialkowski wrote:
> Hi there, thanks for the review,
> 
> Le mardi 04 novembre 2014 ? 13:32 -0500, Tom Rini a ?crit :
> > On Tue, Nov 04, 2014 at 07:58:38PM +0200, Igor Grinberg wrote:
> > > -----BEGIN PGP SIGNED MESSAGE-----
> > > Hash: SHA1
> > > 
> > > Hi Tom,
> > > 
> > > On 11/04/14 17:56, Tom Rini wrote:
> > > > On Sat, Nov 01, 2014 at 11:35:43AM +0100, Paul Kocialkowski wrote:
> > > > 
> > > >> Some devices may use non-standard combinations of regulators to power MMC:
> > > >> this allows these devices to provide a board-specific MMC power init function
> > > >> to set everything up in their own way.
> > > >>
> > > >> Signed-off-by: Paul Kocialkowski <contact@paulk.fr>
> > > >> ---
> > > >>  drivers/mmc/mmc.c | 8 ++++++++
> > > >>  include/mmc.h     | 2 ++
> > > >>  2 files changed, 10 insertions(+)
> > > >>
> > > >> diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c
> > > >> index 44a4feb..125f347 100644
> > > >> --- a/drivers/mmc/mmc.c
> > > >> +++ b/drivers/mmc/mmc.c
> > > >> @@ -1277,6 +1277,12 @@ block_dev_desc_t *mmc_get_dev(int dev)
> > > >>  }
> > > >>  #endif
> > > >>  
> > > >> +/* board-specific MMC power initializations. */
> > > >> +__weak int board_mmc_power_init(void)
> > > >> +{
> > > >> +	return -1;
> > > >> +}
> > > > 
> > > > Since we don't check error return here which I think is fine just make
> > > > this a void?  Thanks!
> > > 
> > > There is v3 posted a while ago...
> > > We have also agreed on v4..
> 
> Note that v3 and v4 are the same, except that v3 didn't apply on top of
> master.
> 
> > Yeah, oops, didn't delete these after catch-up.  I'm still not sure we
> > should continue adding more unchecked return values "just because".
> 
> I agree that we shouldn't have an unchecked return value. So we could
> either check the return value and print a warning, without aborting the
> init sequence (what Igor proposed initially) or just make this return
> void (what you both seem to agree on).
> 
> I'm fine with both solutions. I guess that enabling a regulator could
> fail (say, because of an i2c error), so there is still sense in
> returning int.
> 
> Let me know of what your definitive answer on this is. I'll make a new
> patchset probably this friday (I'm running on a very tight schedule
> until then).

Lets go with void.  Thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20141105/c01d9351/attachment.pgp>