From mboxrd@z Thu Jan  1 00:00:00 1970
From: Hans de Goede <hdegoede@redhat.com>
Date: Mon, 09 Jun 2014 10:25:43 +0200
Subject: [U-Boot] [PATCH v2 08/12] sunxi: Add axp209 pmic support
In-Reply-To: <1401993642.15729.176.camel@hastur.hellion.org.uk>
References: <1401824522-11353-1-git-send-email-hdegoede@redhat.com>	
	<1401824522-11353-9-git-send-email-hdegoede@redhat.com>
	<1401993642.15729.176.camel@hastur.hellion.org.uk>
Message-ID: <53956F87.2000700@redhat.com>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

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