-----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 <cont...@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 mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot