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

Ian Campbell ijc at hellion.org.uk
Thu Jun 5 20:40:42 CEST 2014


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);

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?

> +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...

Ian.



More information about the U-Boot mailing list