[U-Boot] [PATCH v2 08/12] sunxi: Add axp209 pmic support

[Hans de Goede <hdegoede@redhat.com>]

Hi,

On 06/05/2014 08:40 PM, Ian Campbell wrote:
> On Tue, 2014-06-03 at 21:41 +0200, Hans de Goede wrote:
>> +int axp209_set_ldo3(int mvolt)
>> +{
>> +	int cfg = axp209_mvolt_to_cfg(mvolt, 700, 2275, 25);
>> +
>> +	if (mvolt == -1)
>> +		cfg = 0x80;	/* determined by LDO3IN pin */
> 
> Thus would seem more natural as 
> 	if (mvolt == -1)
> 		cfg = 0x80;	/* comment... */
> 	else
> 		cfg = axp209_mvolt_to_cfg(mvolt, 700, 2275, 25);
> 

I Agree that that would be better, fixed in my tree:

https://github.com/jwrdegoede/u-boot-sunxi/commits/sunxi-upstreaming

Note I often rebase + force push this branch.

> BTW, I've just noticed that cfg is often an int but axp209_write etc all
> deal in u8's. Do you not want to be dealing with u8's throughout?

Agreed that this should be u8 everywhere, fixed this for both the axp209
and axp152 code.

Note I won't be re-posting these until the i2c controller conversion to
the CONFIG_SYS_I2C framework is sorted out.

>> +void axp209_poweroff(void)
>> +{
>> +	u8 val;
>> +
>> +	if (axp209_read(AXP209_SHUTDOWN, &val) != 0)
>> +		return;
>> +
>> +	val |= AXP209_POWEROFF;
>> +
>> +	if (axp209_write(AXP209_SHUTDOWN, val) != 0)
>> +		return;
>> +
>> +	udelay(10000);		/* wait for power to drain */
> 
> Is this essentially a "wait to die" loop? i.e. we expect power to
> disappear while in the middle of it. In which case shouldn't it be
> infinite? What would happen if we were to return from this function?
> Nothing actually calls it AFAICT so maybe you can just punt on the whole
> thing and drop the function for now...

I agree that just dropping this function for now is best, done.

Regards,

Hans