On Fri, 2014-05-30 at 11:06 +0200, Hans de Goede wrote:
> From: Henrik Nordstrom <hen...@henriknordstrom.net>
>
> Add support for the x-powers axp209 pmic which is found on most A10, A13 and
> A20 boards.
>
> While changing the boards.cfg lines for the Cubietruck, add myself as board
> maintainer for the Cubietruck.
I don't know much about these PMIC things, so just some general
comments/questions.
> #include <asm/arch/clock.h>
> #include <asm/arch/dram.h>
> #include <asm/arch/gpio.h>
> @@ -116,12 +119,35 @@ void i2c_init_board(void)
> #ifdef CONFIG_SPL_BUILD
> void sunxi_board_init(void)
> {
> + int power_failed = 0;
bool?
> unsigned long ramsize;
>
> +#ifdef CONFIG_AXP209_POWER
> + power_failed |= axp209_init();
> + power_failed |= axp209_set_dcdc2(1400);
> + power_failed |= axp209_set_dcdc3(1250);
> + power_failed |= axp209_set_ldo2(3000);
> + power_failed |= axp209_set_ldo3(2800);
> + power_failed |= axp209_set_ldo4(2800);
I take it that it is safe to keep poking at things after one of them
fails?
> +#endif
> +
> printf("DRAM:");
> ramsize = sunxi_dram_init();
> printf(" %lu MiB\n", ramsize >> 20);
> if (!ramsize)
> hang();
> +
> + /*
> + * Only clock up the CPU to full speed if we are reasonably
> + * assured it's being powered with suitable core voltage
> + */
> + if (!power_failed)
> +#ifdef CONFIG_SUN7I
> + clock_set_pll1(912000000);
> +#else
> + clock_set_pll1(1008000000);
#define CLK_FULL_SPEED in the relevant sun?i-config.h-es?
> +#endif
> + else
> + printf("Failed to set core voltage! Can't set CPU frequency\n");
> }
> #endif
> diff --git a/drivers/power/axp209.c b/drivers/power/axp209.c
> new file mode 100644
> index 0000000..a3f9d52
> --- /dev/null
> +++ b/drivers/power/axp209.c
> @@ -0,0 +1,198 @@
> +/*
> + * (C) Copyright 2012
> + * Henrik Nordstrom <hen...@henriknordstrom.net>
> + *
> + * SPDX-License-Identifier: GPL-2.0+
> + */
> +
> +#include <common.h>
> +#include <i2c.h>
> +#include <axp209.h>
> +
> +enum axp209_reg {
> + AXP209_POWER_STATUS = 0x00,
> + AXP209_CHIP_VERSION = 0x03,
> + AXP209_DCDC2_VOLTAGE = 0x23,
> + AXP209_DCDC3_VOLTAGE = 0x27,
> + AXP209_LDO24_VOLTAGE = 0x28,
> + AXP209_LDO3_VOLTAGE = 0x29,
> + AXP209_IRQ_STATUS3 = 0x4a,
> + AXP209_IRQ_STATUS5 = 0x4c,
> + AXP209_SHUTDOWN = 0x32,
> +};
> +
> +#define AXP209_POWER_STATUS_ON_BY_DC (1<<0)
> +
> +#define AXP209_IRQ3_PEK_SHORT (1<<1)
> +#define AXP209_IRQ3_PEK_LONG (1<<0)
> +
> +#define AXP209_IRQ5_PEK_UP (1<<6)
> +#define AXP209_IRQ5_PEK_DOWN (1<<5)
Only IRQ5 is used?
> +
> +int axp209_write(enum axp209_reg reg, u8 val)
> +{
> + return i2c_write(0x34, reg, 1, &val, 1);
> +}
> +
> +int axp209_read(enum axp209_reg reg, u8 *val)
> +{
> + return i2c_read(0x34, reg, 1, val, 1);
> +}
> +
> +int axp209_set_dcdc2(int mvolt)
> +{
> + int cfg = (mvolt - 700) / 25;
> + int rc;
> + u8 current;
> +
> + if (cfg < 0)
> + cfg = 0;
Can we make mvolt and cfg unsigned? (I suppose you'd then need a range
check on mvolt before the subtraction, so perhaps it doesn't save much?)
> + if (cfg > (1 << 6) - 1)
> + cfg = (1 << 6) - 1;
#define AXP209_DCDC2_MAX?
> +int axp209_set_dcdc3(int mvolt)
> +{
> + int cfg = (mvolt - 700) / 25;
> + u8 reg;
> + int rc;
> +
> + if (cfg < 0)
> + cfg = 0;
> + if (cfg > (1 << 7) - 1)
> + cfg = (1 << 7) - 1;
Some two comments as for DCDC2 (which I won't bother making for
subsequent clocks).
perhaps cfg = clamp(cfg, 0, 1<<7-1)? Or even cfg = mvolt_to_cfg(mvolt,
700, 25, 0, 1<<7-1)?
> + rc = axp209_write(AXP209_DCDC3_VOLTAGE, cfg);
> + rc |= axp209_read(AXP209_DCDC3_VOLTAGE, ®);
Is read safe if the write failed? Since the reg is never used I assume
it's just some sort of sync? Or do you need to confirm the contents
"took"? If not you could remove reg and readback into cfg.
> + rc = axp209_read(AXP209_LDO24_VOLTAGE, ®);
> + if (rc)
> + return rc;
> +
> + reg = (reg & 0x0f) | (cfg << 4);
Do we know what these magic numbers mean?
(ah, you have a comment on the use of the lower 4 bits, but not the
upper 4 used here...)
> + rc = axp209_write(AXP209_LDO24_VOLTAGE, reg);
> + if (rc)
> + return rc;
> +
> + return 0;
> +}
> +
> +int axp209_set_ldo3(int mvolt)
> +{
> + int cfg = (mvolt - 700) / 25;
> +
> + if (cfg < 0)
> + cfg = 0;
> + if (cfg > 127)
> + cfg = 127;
> + if (mvolt == -1)
> + cfg = 0x80; /* detemined by LDO3IN pin */
"determined"
> +
> + return axp209_write(AXP209_LDO3_VOLTAGE, cfg);
> +}
> +
> +int axp209_set_ldo4(int mvolt)
> +{
> + int cfg, rc;
> + static const int vindex[] = {
> + 1250, 1300, 1400, 1500, 1600, 1700, 1800, 1900, 2000, 2500,
> + 2700, 2800, 3000, 3100, 3200, 3300
> + };
> + u8 reg;
> +
> + /* Translate mvolt to register cfg value, requested <= selected */
> + for (cfg = 15; vindex[cfg] > mvolt && cfg > 0; cfg--);
> +
> + rc = axp209_read(AXP209_LDO24_VOLTAGE, ®);
> + if (rc)
> + return rc;
> +
> + /* LDO4 configuration is in lower 4 bits */
> + reg = (reg & 0xf0) | (cfg << 0);
> + rc = axp209_write(AXP209_LDO24_VOLTAGE, reg);
> + if (rc)
> + return rc;
> +
> + return 0;
> +}
> +
> +void axp209_poweroff(void)
> +{
> + u8 val;
> +
> + if (axp209_read(AXP209_SHUTDOWN, &val) != 0)
> + return;
> +
> + val |= 1 << 7;
#define?
> +
> + if (axp209_write(AXP209_SHUTDOWN, val) != 0)
> + return;
> +
> + udelay(10000); /* wait for power to drain */
> +}
Ian.
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
