| Message ID | 1414838144-4270-1-git-send-email-contact@paulk.fr |
|---|---|
| State | Superseded |
| Delegated to: | Tom Rini |
| Headers | show |
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!
-----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-----
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".
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).
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!
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
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(+)