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

[Igor Grinberg <grinberg@compulab.co.il>]

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

Hi Paul,

On 11/02/14 20:51, Paul Kocialkowski wrote:
> Hi Igor,
> 
>> On 11/01/14 12:52, 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     | 1 +
>>>  2 files changed, 9 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();
>>> +
>>
>> Shouldn't we have some error handling here?
> 
> I noticed how weak implementations tend to return -1 and not have their
> return code checked so much (see the other two such functions in mmc.c).
> I would be fine with checking the return code and returning 0 in the
> weak implementation.
> 
> If you think that's better, I'll make a new version with that.

Well, otherwise it does not make sense to return int, does it?
IMO it is a good practice to check the return value, and may be
emit a warning or something (I'm not really sure if it should
abort the init sequence).
Since I can't propose a good handling now, may be we should leave
it for now and see if someone comes up with a good case for it.
I'm fine with your decision.

> 
>>>  	/* made sure it's not NULL earlier */
>>>  	err = mmc->cfg->ops->init(mmc);
>>>  
>>> diff --git a/include/mmc.h b/include/mmc.h
>>> index d74a190..aaea644 100644
>>> --- a/include/mmc.h
>>> +++ b/include/mmc.h
>>> @@ -385,6 +385,7 @@ struct mmc *mmc_spi_init(uint bus, uint cs, uint speed, uint mode);
>>>  int mmc_legacy_init(int verbose);
>>>  #endif
>>>  
>>> +int board_mmc_power_init(void);
>>>  int board_mmc_init(bd_t *bis);
>>>  int cpu_mmc_init(bd_t *bis);
>>>  int mmc_get_env_addr(struct mmc *mmc, int copy, u32 *env_addr);
>>>
>>
> 

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

iQIcBAEBAgAGBQJUVzBHAAoJEBDE8YO64EfayhsP/jV2YAR6jH6gZJBNn/wGnHId
a8U0YGG+f30RBPCdfCiraSRpL1C876wsf+InNDAHgo4HSaRdRwF4tUqWj78SpH9F
QTo3qcA6S8S4GIRJ2+q+3BmtlEuYPWOZOi41erhaUF3kMLw6cMt+gL0Ggx4OXQAo
2bhSeQaaGynoFJwC0dIf0QhDjRkPCHbNh2IgROfRye0a7/0pF4tN/tvjLSxw8w54
3457N4TYreGEmMMrtNi/psekMYTjR6JFve3CwiMvOXiwOJGz/d6qj7vhw4Ba7q5i
LE385Zj24Bi/+m7ls7hp5I522O0tkShGgPI3XvCiT4xrjXDBihsWG9RGsLSc5vBY
EimiztfLVu3alc1cxclK9tq0p+FwyUNwXsUED+oR6t56/qr/BSWiSIDAMfN1zcfI
paK9zXDoGvhg6T0pK+SsRvAE4D7vg+XT8WSU52l6/h8TRKB6r3cupSMXAyjmUpLl
1pEozVsg4qW6rJawQOiCjl0YU3ixeVwk0vmWWh+gBOMMLsuikQpFI5s6zUXteqnl
IHFS6oiYW/SVppHrd95624suZQPTsrH6+4CvmBwmtBGbmErchsgfj8qjP/ilpn1a
Y6Kv0mj2ndkgdFAc7zIEpwzF9yPsewVg8nc+vYLezyT7ePeTNYLpPWgQTL6qEdHH
lcK7qYNEaYWIUMkbQM5P
=7kRq
-----END PGP SIGNATURE-----