[U-Boot] [PATCH] power: LP8720 regulator support
This adds support for the LP8720 i2c regulator, as found in e.g. the LG Optimus Black (P970), codename sniper. This code supports setting up and enabling one of the 5 LDOs that the IC provides. Other more advanced features are unsupported.
Signed-off-by: Paul Kocialkowski contact@paulk.fr --- drivers/power/Makefile | 1 + drivers/power/lp8720.c | 107 ++++++++++++++++++++++++++++++++++++++++++++++++ include/lp8720.h | 93 +++++++++++++++++++++++++++++++++++++++++ 3 files changed, 201 insertions(+) create mode 100644 drivers/power/lp8720.c create mode 100644 include/lp8720.h
diff --git a/drivers/power/Makefile b/drivers/power/Makefile index dc64e4d..65be5a0 100644 --- a/drivers/power/Makefile +++ b/drivers/power/Makefile @@ -9,6 +9,7 @@ obj-$(CONFIG_AXP152_POWER) += axp152.o obj-$(CONFIG_AXP209_POWER) += axp209.o obj-$(CONFIG_EXYNOS_TMU) += exynos-tmu.o obj-$(CONFIG_FTPMU010_POWER) += ftpmu010.o +obj-$(CONFIG_LP8720_POWER) += lp8720.o obj-$(CONFIG_TPS6586X_POWER) += tps6586x.o obj-$(CONFIG_TWL4030_POWER) += twl4030.o obj-$(CONFIG_TWL6030_POWER) += twl6030.o diff --git a/drivers/power/lp8720.c b/drivers/power/lp8720.c new file mode 100644 index 0000000..ac7fc11 --- /dev/null +++ b/drivers/power/lp8720.c @@ -0,0 +1,107 @@ +/* + * Copyright (C) 2014 Paul Kocialkowski contact@paulk.fr + * + * SPDX-License-Identifier: GPL-2.0+ + */ + +#include <common.h> +#include <i2c.h> +#include <asm/gpio.h> +#include <lp8720.h> + +static struct lp8720_info lp8720_info; + +static int lp8720_write(u8 reg, u8 val) +{ + return i2c_write(lp8720_info.chip_idsel, reg, 1, &val, 1); +} + +static int lp8720_read(u8 reg, u8 *val) +{ + return i2c_read(lp8720_info.chip_idsel, reg, 1, val, 1); +} + +int lp8720_init(int enable_gpio, int chip_idsel) +{ + int ret; + + if (enable_gpio) { + if (!gpio_is_valid(enable_gpio)) + return -1; + + ret = gpio_request(enable_gpio, "lp8720_en"); + if (ret) + return ret; + + ret = gpio_direction_output(enable_gpio, 0); + if (ret) + return ret; + } + + lp8720_info.enable_gpio = enable_gpio; + lp8720_info.chip_idsel = chip_idsel; + + return 0; +} + +int lp8720_enable(void) +{ + int ret; + + if (lp8720_info.enable_gpio) { + ret = gpio_set_value(lp8720_info.enable_gpio, 1); + if (ret) + return ret; + } + + return 0; +} + +int lp8720_is_enabled(void) +{ + if (lp8720_info.enable_gpio) + return gpio_get_value(lp8720_info.enable_gpio); + + /* Assume LP8720 enabled when there is no enable GPIO */ + return 1; +} + +int lp8720_ldo_enable(u8 ldo_enable) +{ + u8 val; + int ret; + + ret = lp8720_read(LP8720_ENABLE_BITS, &val); + if (ret) + return ret; + + val |= ldo_enable; + + ret = lp8720_write(LP8720_ENABLE_BITS, val); + if (ret) + return ret; + + /* Enable the IC */ + if (!lp8720_is_enabled()) + lp8720_enable(); + + return 0; +} + +int lp8720_ldo_voltage(u8 ldo_reg, u8 voltage, u8 delay) +{ + u8 val; + int ret; + + /* Register V field */ + val = voltage & 0x1F; + + /* Register T field */ + val |= (delay & 0x07) << 5; + + ret = lp8720_write(ldo_reg, val); + if (ret) + return ret; + + return 0; +} diff --git a/include/lp8720.h b/include/lp8720.h new file mode 100644 index 0000000..033f2c4 --- /dev/null +++ b/include/lp8720.h @@ -0,0 +1,93 @@ +/* + * Copyright (C) 2014 Paul Kocialkowski contact@paulk.fr + * + * SPDX-License-Identifier: GPL-2.0+ + */ + +#ifndef LP8720_H +#define LP8720_H + +#include <common.h> + +/* + * Chip ID selection + */ + +#define LP8720_CHIP_IDSEL_VBATT 0x7F +#define LP8720_CHIP_IDSEL_HI_Z 0x7C +#define LP8720_CHIP_IDSEL_GND 0x7D + +/* + * Registers + */ + +#define LP8720_GENERAL_SETTINGS 0x00 +#define LP8720_LDO1_SETTINGS 0x01 +#define LP8720_LDO2_SETTINGS 0x02 +#define LP8720_LDO3_SETTINGS 0x03 +#define LP8720_LDO4_SETTINGS 0x04 +#define LP8720_LDO5_SETTINGS 0x05 +#define LP8720_BUCK_SETTINGS1 0x06 +#define LP8720_BUCK_SETTINGS2 0x07 +#define LP8720_ENABLE_BITS 0x08 +#define LP8720_PULLDOWN_BITS 0x09 +#define LP8720_STATUS_BITS 0x0A +#define LP8720_INTERRUPT_BITS 0x0B +#define LP8720_INTERRUPT_MASK 0x0C + +/* + * Values + */ + +/* LP8720_GENERAL_SETTINGS */ +#define LP8720_25_US_TIME_STEP (1 << 0) +#define LP8720_50_US_TIME_STEP (0 << 0) + +/* LP8720_LDO*_SETTINGS */ +#define LP8720_LDO1235_V_12 0x00 +#define LP8720_LDO1235_V_18 0x0C +#define LP8720_LDO1235_V_30 0x1D +#define LP8720_LDO1235_V_33 0x1F + +#define LP8720_LDO4_V_12 0x00 +#define LP8720_LDO4_V_14 0x09 +#define LP8720_LDO4_V_18 0x11 +#define LP8720_LDO4_V_28 0x1E + +#define LP8720_DELAY_0 0 +#define LP8720_DELAY_1_TS 1 +#define LP8720_DELAY_2_TS 2 +#define LP8720_DELAY_3_TS 3 +#define LP8720_DELAY_4_TS 4 +#define LP8720_DELAY_5_TS 5 +#define LP8720_DELAY_6_TS 6 +#define LP8720_DELAY_NO_START 7 + +/* LP8720_ENABLE_BITS */ +#define LP8720_LDO1_EN (1 << 0) +#define LP8720_LDO2_EN (1 << 1) +#define LP8720_LDO3_EN (1 << 2) +#define LP8720_LDO4_EN (1 << 3) +#define LP8720_LDO5_EN (1 << 4) +#define LP8720_BUCK_EN (1 << 5) + +/* + * Driver info + */ + +struct lp8720_info { + int enable_gpio; + int chip_idsel; +}; + +/* + * Declarations + */ + +int lp8720_init(int enable_gpio, int chip_idsel); +int lp8720_enable(void); +int lp8720_is_enabled(void); +int lp8720_ldo_enable(u8 ldo_enable); +int lp8720_ldo_voltage(u8 ldo_reg, u8 voltage, u8 delay); + +#endif
Le mardi 28 octobre 2014 à 18:32 +0100, Paul Kocialkowski a écrit :
This adds support for the LP8720 i2c regulator, as found in e.g. the LG Optimus Black (P970), codename sniper. This code supports setting up and enabling one of the 5 LDOs that the IC provides. Other more advanced features are unsupported.
Signed-off-by: Paul Kocialkowski contact@paulk.fr
drivers/power/Makefile | 1 + drivers/power/lp8720.c | 107 ++++++++++++++++++++++++++++++++++++++++++++++++ include/lp8720.h | 93 +++++++++++++++++++++++++++++++++++++++++ 3 files changed, 201 insertions(+) create mode 100644 drivers/power/lp8720.c create mode 100644 include/lp8720.h
diff --git a/drivers/power/Makefile b/drivers/power/Makefile index dc64e4d..65be5a0 100644 --- a/drivers/power/Makefile +++ b/drivers/power/Makefile @@ -9,6 +9,7 @@ obj-$(CONFIG_AXP152_POWER) += axp152.o obj-$(CONFIG_AXP209_POWER) += axp209.o obj-$(CONFIG_EXYNOS_TMU) += exynos-tmu.o obj-$(CONFIG_FTPMU010_POWER) += ftpmu010.o +obj-$(CONFIG_LP8720_POWER) += lp8720.o obj-$(CONFIG_TPS6586X_POWER) += tps6586x.o obj-$(CONFIG_TWL4030_POWER) += twl4030.o obj-$(CONFIG_TWL6030_POWER) += twl6030.o diff --git a/drivers/power/lp8720.c b/drivers/power/lp8720.c new file mode 100644 index 0000000..ac7fc11 --- /dev/null +++ b/drivers/power/lp8720.c @@ -0,0 +1,107 @@ +/*
- Copyright (C) 2014 Paul Kocialkowski contact@paulk.fr
- SPDX-License-Identifier: GPL-2.0+
- */
+#include <common.h> +#include <i2c.h> +#include <asm/gpio.h> +#include <lp8720.h>
+static struct lp8720_info lp8720_info;
+static int lp8720_write(u8 reg, u8 val) +{
- return i2c_write(lp8720_info.chip_idsel, reg, 1, &val, 1);
+}
+static int lp8720_read(u8 reg, u8 *val) +{
- return i2c_read(lp8720_info.chip_idsel, reg, 1, val, 1);
+}
Should I ifdef for I2C and GPIO support? It seems that GPIO support only has board-sepcific config options, so this may be hard. There is CONFIG_DM_GPIO for driver model, but it is apparently not always used, especially not on SPL.
+int lp8720_init(int enable_gpio, int chip_idsel) +{
- int ret;
- if (enable_gpio) {
I'm guessing that in the case of a provided negative GPIO, this will return -1, which is not appropriate behavior (the regulator can still be used without an enable GPIO). I should probbaly directly go with gpio_is_valid.
if (!gpio_is_valid(enable_gpio))return -1;ret = gpio_request(enable_gpio, "lp8720_en");if (ret)return ret;ret = gpio_direction_output(enable_gpio, 0);if (ret)return ret;- }
- lp8720_info.enable_gpio = enable_gpio;
- lp8720_info.chip_idsel = chip_idsel;
- return 0;
+}
+int lp8720_enable(void) +{
- int ret;
- if (lp8720_info.enable_gpio) {
ret = gpio_set_value(lp8720_info.enable_gpio, 1);if (ret)return ret;- }
- return 0;
+}
+int lp8720_is_enabled(void) +{
- if (lp8720_info.enable_gpio)
return gpio_get_value(lp8720_info.enable_gpio);- /* Assume LP8720 enabled when there is no enable GPIO */
- return 1;
+}
+int lp8720_ldo_enable(u8 ldo_enable) +{
- u8 val;
- int ret;
- ret = lp8720_read(LP8720_ENABLE_BITS, &val);
- if (ret)
return ret;- val |= ldo_enable;
- ret = lp8720_write(LP8720_ENABLE_BITS, val);
- if (ret)
return ret;- /* Enable the IC */
- if (!lp8720_is_enabled())
lp8720_enable();- return 0;
+}
+int lp8720_ldo_voltage(u8 ldo_reg, u8 voltage, u8 delay) +{
- u8 val;
- int ret;
- /* Register V field */
- val = voltage & 0x1F;
- /* Register T field */
- val |= (delay & 0x07) << 5;
- ret = lp8720_write(ldo_reg, val);
- if (ret)
return ret;- return 0;
+} diff --git a/include/lp8720.h b/include/lp8720.h new file mode 100644 index 0000000..033f2c4 --- /dev/null +++ b/include/lp8720.h @@ -0,0 +1,93 @@ +/*
- Copyright (C) 2014 Paul Kocialkowski contact@paulk.fr
- SPDX-License-Identifier: GPL-2.0+
- */
+#ifndef LP8720_H +#define LP8720_H
+#include <common.h>
+/*
- Chip ID selection
- */
+#define LP8720_CHIP_IDSEL_VBATT 0x7F +#define LP8720_CHIP_IDSEL_HI_Z 0x7C +#define LP8720_CHIP_IDSEL_GND 0x7D
+/*
- Registers
- */
+#define LP8720_GENERAL_SETTINGS 0x00 +#define LP8720_LDO1_SETTINGS 0x01 +#define LP8720_LDO2_SETTINGS 0x02 +#define LP8720_LDO3_SETTINGS 0x03 +#define LP8720_LDO4_SETTINGS 0x04 +#define LP8720_LDO5_SETTINGS 0x05 +#define LP8720_BUCK_SETTINGS1 0x06 +#define LP8720_BUCK_SETTINGS2 0x07 +#define LP8720_ENABLE_BITS 0x08 +#define LP8720_PULLDOWN_BITS 0x09 +#define LP8720_STATUS_BITS 0x0A +#define LP8720_INTERRUPT_BITS 0x0B +#define LP8720_INTERRUPT_MASK 0x0C
+/*
- Values
- */
+/* LP8720_GENERAL_SETTINGS */ +#define LP8720_25_US_TIME_STEP (1 << 0) +#define LP8720_50_US_TIME_STEP (0 << 0)
+/* LP8720_LDO*_SETTINGS */ +#define LP8720_LDO1235_V_12 0x00 +#define LP8720_LDO1235_V_18 0x0C +#define LP8720_LDO1235_V_30 0x1D +#define LP8720_LDO1235_V_33 0x1F
+#define LP8720_LDO4_V_12 0x00 +#define LP8720_LDO4_V_14 0x09 +#define LP8720_LDO4_V_18 0x11 +#define LP8720_LDO4_V_28 0x1E
+#define LP8720_DELAY_0 0 +#define LP8720_DELAY_1_TS 1 +#define LP8720_DELAY_2_TS 2 +#define LP8720_DELAY_3_TS 3 +#define LP8720_DELAY_4_TS 4 +#define LP8720_DELAY_5_TS 5 +#define LP8720_DELAY_6_TS 6 +#define LP8720_DELAY_NO_START 7
+/* LP8720_ENABLE_BITS */ +#define LP8720_LDO1_EN (1 << 0) +#define LP8720_LDO2_EN (1 << 1) +#define LP8720_LDO3_EN (1 << 2) +#define LP8720_LDO4_EN (1 << 3) +#define LP8720_LDO5_EN (1 << 4) +#define LP8720_BUCK_EN (1 << 5)
+/*
- Driver info
- */
+struct lp8720_info {
- int enable_gpio;
- int chip_idsel;
+};
+/*
- Declarations
- */
+int lp8720_init(int enable_gpio, int chip_idsel); +int lp8720_enable(void); +int lp8720_is_enabled(void); +int lp8720_ldo_enable(u8 ldo_enable); +int lp8720_ldo_voltage(u8 ldo_reg, u8 voltage, u8 delay);
+#endif
On Sat, Nov 01, 2014 at 12:01:37PM +0100, Paul Kocialkowski wrote:
Le mardi 28 octobre 2014 à 18:32 +0100, Paul Kocialkowski a écrit :
This adds support for the LP8720 i2c regulator, as found in e.g. the LG Optimus Black (P970), codename sniper. This code supports setting up and enabling one of the 5 LDOs that the IC provides. Other more advanced features are unsupported.
Signed-off-by: Paul Kocialkowski contact@paulk.fr
drivers/power/Makefile | 1 + drivers/power/lp8720.c | 107 ++++++++++++++++++++++++++++++++++++++++++++++++ include/lp8720.h | 93 +++++++++++++++++++++++++++++++++++++++++ 3 files changed, 201 insertions(+) create mode 100644 drivers/power/lp8720.c create mode 100644 include/lp8720.h
diff --git a/drivers/power/Makefile b/drivers/power/Makefile index dc64e4d..65be5a0 100644 --- a/drivers/power/Makefile +++ b/drivers/power/Makefile @@ -9,6 +9,7 @@ obj-$(CONFIG_AXP152_POWER) += axp152.o obj-$(CONFIG_AXP209_POWER) += axp209.o obj-$(CONFIG_EXYNOS_TMU) += exynos-tmu.o obj-$(CONFIG_FTPMU010_POWER) += ftpmu010.o +obj-$(CONFIG_LP8720_POWER) += lp8720.o obj-$(CONFIG_TPS6586X_POWER) += tps6586x.o obj-$(CONFIG_TWL4030_POWER) += twl4030.o obj-$(CONFIG_TWL6030_POWER) += twl6030.o diff --git a/drivers/power/lp8720.c b/drivers/power/lp8720.c new file mode 100644 index 0000000..ac7fc11 --- /dev/null +++ b/drivers/power/lp8720.c @@ -0,0 +1,107 @@ +/*
- Copyright (C) 2014 Paul Kocialkowski contact@paulk.fr
- SPDX-License-Identifier: GPL-2.0+
- */
+#include <common.h> +#include <i2c.h> +#include <asm/gpio.h> +#include <lp8720.h>
+static struct lp8720_info lp8720_info;
+static int lp8720_write(u8 reg, u8 val) +{
- return i2c_write(lp8720_info.chip_idsel, reg, 1, &val, 1);
+}
+static int lp8720_read(u8 reg, u8 *val) +{
- return i2c_read(lp8720_info.chip_idsel, reg, 1, val, 1);
+}
Should I ifdef for I2C and GPIO support? It seems that GPIO support only has board-sepcific config options, so this may be hard. There is CONFIG_DM_GPIO for driver model, but it is apparently not always used, especially not on SPL.
Don't code for what you can't test. Which way are you using this on the Optimus? And in general, any comments here Przemyslaw? Thanks!
Hello,
On 11/04/2014 04:58 PM, Tom Rini wrote:
On Sat, Nov 01, 2014 at 12:01:37PM +0100, Paul Kocialkowski wrote:
Le mardi 28 octobre 2014 à 18:32 +0100, Paul Kocialkowski a écrit :
This adds support for the LP8720 i2c regulator, as found in e.g. the LG Optimus Black (P970), codename sniper. This code supports setting up and enabling one of the 5 LDOs that the IC provides. Other more advanced features are unsupported.
Signed-off-by: Paul Kocialkowski contact@paulk.fr
drivers/power/Makefile | 1 + drivers/power/lp8720.c | 107 ++++++++++++++++++++++++++++++++++++++++++++++++ include/lp8720.h | 93 +++++++++++++++++++++++++++++++++++++++++ 3 files changed, 201 insertions(+) create mode 100644 drivers/power/lp8720.c create mode 100644 include/lp8720.h
diff --git a/drivers/power/Makefile b/drivers/power/Makefile index dc64e4d..65be5a0 100644 --- a/drivers/power/Makefile +++ b/drivers/power/Makefile @@ -9,6 +9,7 @@ obj-$(CONFIG_AXP152_POWER) += axp152.o obj-$(CONFIG_AXP209_POWER) += axp209.o obj-$(CONFIG_EXYNOS_TMU) += exynos-tmu.o obj-$(CONFIG_FTPMU010_POWER) += ftpmu010.o +obj-$(CONFIG_LP8720_POWER) += lp8720.o obj-$(CONFIG_TPS6586X_POWER) += tps6586x.o obj-$(CONFIG_TWL4030_POWER) += twl4030.o obj-$(CONFIG_TWL6030_POWER) += twl6030.o diff --git a/drivers/power/lp8720.c b/drivers/power/lp8720.c new file mode 100644 index 0000000..ac7fc11 --- /dev/null +++ b/drivers/power/lp8720.c @@ -0,0 +1,107 @@ +/*
- Copyright (C) 2014 Paul Kocialkowski contact@paulk.fr
- SPDX-License-Identifier: GPL-2.0+
- */
+#include <common.h> +#include <i2c.h> +#include <asm/gpio.h> +#include <lp8720.h>
+static struct lp8720_info lp8720_info;
+static int lp8720_write(u8 reg, u8 val) +{
- return i2c_write(lp8720_info.chip_idsel, reg, 1, &val, 1);
+}
+static int lp8720_read(u8 reg, u8 *val) +{
- return i2c_read(lp8720_info.chip_idsel, reg, 1, val, 1);
+}
Should I ifdef for I2C and GPIO support? It seems that GPIO support only has board-sepcific config options, so this may be hard. There is CONFIG_DM_GPIO for driver model, but it is apparently not always used, especially not on SPL.
Don't code for what you can't test. Which way are you using this on the Optimus? And in general, any comments here Przemyslaw? Thanks!
U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Thanks for cc Tom, I have some comments in a next message.
Paul, there are no patches for the board, which you mentioned about. Usually we don't add dead code to U-Boot. If you are going to add new board - just add it with all required drivers in a one patchset - then it is consistent and easy to review.
Best regards,
Le mardi 04 novembre 2014 à 18:46 +0100, Przemyslaw Marczak a écrit :
Hello,
On 11/04/2014 04:58 PM, Tom Rini wrote:
On Sat, Nov 01, 2014 at 12:01:37PM +0100, Paul Kocialkowski wrote:
Le mardi 28 octobre 2014 à 18:32 +0100, Paul Kocialkowski a écrit :
This adds support for the LP8720 i2c regulator, as found in e.g. the LG Optimus Black (P970), codename sniper. This code supports setting up and enabling one of the 5 LDOs that the IC provides. Other more advanced features are unsupported.
Signed-off-by: Paul Kocialkowski contact@paulk.fr
drivers/power/Makefile | 1 + drivers/power/lp8720.c | 107 ++++++++++++++++++++++++++++++++++++++++++++++++ include/lp8720.h | 93 +++++++++++++++++++++++++++++++++++++++++ 3 files changed, 201 insertions(+) create mode 100644 drivers/power/lp8720.c create mode 100644 include/lp8720.h
diff --git a/drivers/power/Makefile b/drivers/power/Makefile index dc64e4d..65be5a0 100644 --- a/drivers/power/Makefile +++ b/drivers/power/Makefile @@ -9,6 +9,7 @@ obj-$(CONFIG_AXP152_POWER) += axp152.o obj-$(CONFIG_AXP209_POWER) += axp209.o obj-$(CONFIG_EXYNOS_TMU) += exynos-tmu.o obj-$(CONFIG_FTPMU010_POWER) += ftpmu010.o +obj-$(CONFIG_LP8720_POWER) += lp8720.o obj-$(CONFIG_TPS6586X_POWER) += tps6586x.o obj-$(CONFIG_TWL4030_POWER) += twl4030.o obj-$(CONFIG_TWL6030_POWER) += twl6030.o diff --git a/drivers/power/lp8720.c b/drivers/power/lp8720.c new file mode 100644 index 0000000..ac7fc11 --- /dev/null +++ b/drivers/power/lp8720.c @@ -0,0 +1,107 @@ +/*
- Copyright (C) 2014 Paul Kocialkowski contact@paulk.fr
- SPDX-License-Identifier: GPL-2.0+
- */
+#include <common.h> +#include <i2c.h> +#include <asm/gpio.h> +#include <lp8720.h>
+static struct lp8720_info lp8720_info;
+static int lp8720_write(u8 reg, u8 val) +{
- return i2c_write(lp8720_info.chip_idsel, reg, 1, &val, 1);
+}
+static int lp8720_read(u8 reg, u8 *val) +{
- return i2c_read(lp8720_info.chip_idsel, reg, 1, val, 1);
+}
Should I ifdef for I2C and GPIO support? It seems that GPIO support only has board-sepcific config options, so this may be hard. There is CONFIG_DM_GPIO for driver model, but it is apparently not always used, especially not on SPL.
Don't code for what you can't test. Which way are you using this on the Optimus? And in general, any comments here Przemyslaw? Thanks!
I am using that code both on SPL and regular U-Boot. I'll just drop the idea of adding ifdef on GPIO and go with it all the way. On the Optimus Black, a GPIO is used to enable the regulators.
Paul, there are no patches for the board, which you mentioned about. Usually we don't add dead code to U-Boot. If you are going to add new board - just add it with all required drivers in a one patchset - then it is consistent and easy to review.
I understand that. I'll keep your previous comments aside and will rework the LP8720 code a bit later. I'll focus on having all the changes I need on already-existing code accepted first and then submit new code (that relies on these new features), including the Optimus Black board, configs and the the LP8720 regulator.
Thanks!
Hello Paul,
On 11/01/2014 12:01 PM, Paul Kocialkowski wrote:
Le mardi 28 octobre 2014 à 18:32 +0100, Paul Kocialkowski a écrit :
This adds support for the LP8720 i2c regulator, as found in e.g. the LG Optimus Black (P970), codename sniper. This code supports setting up and enabling one of the 5 LDOs that the IC provides. Other more advanced features are unsupported.
Signed-off-by: Paul Kocialkowski contact@paulk.fr
drivers/power/Makefile | 1 + drivers/power/lp8720.c | 107 ++++++++++++++++++++++++++++++++++++++++++++++++ include/lp8720.h | 93 +++++++++++++++++++++++++++++++++++++++++ 3 files changed, 201 insertions(+) create mode 100644 drivers/power/lp8720.c create mode 100644 include/lp8720.h
diff --git a/drivers/power/Makefile b/drivers/power/Makefile index dc64e4d..65be5a0 100644 --- a/drivers/power/Makefile +++ b/drivers/power/Makefile @@ -9,6 +9,7 @@ obj-$(CONFIG_AXP152_POWER) += axp152.o obj-$(CONFIG_AXP209_POWER) += axp209.o obj-$(CONFIG_EXYNOS_TMU) += exynos-tmu.o obj-$(CONFIG_FTPMU010_POWER) += ftpmu010.o +obj-$(CONFIG_LP8720_POWER) += lp8720.o obj-$(CONFIG_TPS6586X_POWER) += tps6586x.o obj-$(CONFIG_TWL4030_POWER) += twl4030.o obj-$(CONFIG_TWL6030_POWER) += twl6030.o diff --git a/drivers/power/lp8720.c b/drivers/power/lp8720.c new file mode 100644 index 0000000..ac7fc11 --- /dev/null +++ b/drivers/power/lp8720.c @@ -0,0 +1,107 @@ +/*
- Copyright (C) 2014 Paul Kocialkowski contact@paulk.fr
- SPDX-License-Identifier: GPL-2.0+
- */
+#include <common.h> +#include <i2c.h> +#include <asm/gpio.h> +#include <lp8720.h>
+static struct lp8720_info lp8720_info;
+static int lp8720_write(u8 reg, u8 val) +{
- return i2c_write(lp8720_info.chip_idsel, reg, 1, &val, 1);
+}
+static int lp8720_read(u8 reg, u8 *val) +{
- return i2c_read(lp8720_info.chip_idsel, reg, 1, val, 1);
+}
At present we have API for the PMIC drivers. It's located here: drivers/power/ and for i2c operations, you can use this: drivers/power/power_i2c.c Please look at some pmic drivers, some simple is: drivers/power/pmic/pmic_pfuze100.c and some more complicated with device-tree: drivers/power/pmic/pmic_max77686.c
We have also some drivers without the pmic framework support - but we are going to keep clean code, rather than introducing api for each driver.
Should I ifdef for I2C and GPIO support? It seems that GPIO support only has board-sepcific config options, so this may be hard. There is CONFIG_DM_GPIO for driver model, but it is apparently not always used, especially not on SPL.
+int lp8720_init(int enable_gpio, int chip_idsel) +{
- int ret;
- if (enable_gpio) {
I'm guessing that in the case of a provided negative GPIO, this will return -1, which is not appropriate behavior (the regulator can still be used without an enable GPIO). I should probbaly directly go with gpio_is_valid.
if (!gpio_is_valid(enable_gpio))return -1;ret = gpio_request(enable_gpio, "lp8720_en");if (ret)return ret;ret = gpio_direction_output(enable_gpio, 0);if (ret)return ret;- }
- lp8720_info.enable_gpio = enable_gpio;
- lp8720_info.chip_idsel = chip_idsel;
- return 0;
+}
+int lp8720_enable(void) +{
- int ret;
- if (lp8720_info.enable_gpio) {
ret = gpio_set_value(lp8720_info.enable_gpio, 1);if (ret)return ret;- }
- return 0;
+}
+int lp8720_is_enabled(void) +{
- if (lp8720_info.enable_gpio)
return gpio_get_value(lp8720_info.enable_gpio);- /* Assume LP8720 enabled when there is no enable GPIO */
- return 1;
+}
This function is quite unclean. The argument and the function name suggests that you enable one ldo, but you modify all ldo bits. Please modify this code with getting "ldo" argument as chosen LDO number.
int lp8720_ldo_enable(int ldo)
+int lp8720_ldo_enable(u8 ldo_enable) +{
- u8 val;
- int ret;
- ret = lp8720_read(LP8720_ENABLE_BITS, &val);
- if (ret)
return ret;- val |= ldo_enable;
what about:
val |= (1 << (ldo - 1)); (for ldo number starting from "1")
Here you should also add LP8720_ENABLE_BITS_MASK
- ret = lp8720_write(LP8720_ENABLE_BITS, val);
- if (ret)
return ret;- /* Enable the IC */
- if (!lp8720_is_enabled())
lp8720_enable();- return 0;
+}
Please use u32 for register as pmic api uses. int lp8720_ldo_voltage(u32 ldo_reg, u32 voltage)
+int lp8720_ldo_voltage(u8 ldo_reg, u8 voltage, u8 delay)
Setting the delay is a different operation than setting the voltage, and it not fits to the pmic api.
So you should add additional function like: int lp8720_ldo_startup_delay(u32 ldo_reg, u32 delay)
+{
- u8 val;
- int ret;
- /* Register V field */
- val = voltage & 0x1F;
- /* Register T field */
- val |= (delay & 0x07) << 5;
- ret = lp8720_write(ldo_reg, val);
- if (ret)
return ret;- return 0;
+} diff --git a/include/lp8720.h b/include/lp8720.h new file mode 100644 index 0000000..033f2c4 --- /dev/null +++ b/include/lp8720.h @@ -0,0 +1,93 @@ +/*
- Copyright (C) 2014 Paul Kocialkowski contact@paulk.fr
- SPDX-License-Identifier: GPL-2.0+
- */
+#ifndef LP8720_H +#define LP8720_H
+#include <common.h>
+/*
- Chip ID selection
- */
+#define LP8720_CHIP_IDSEL_VBATT 0x7F +#define LP8720_CHIP_IDSEL_HI_Z 0x7C +#define LP8720_CHIP_IDSEL_GND 0x7D
+/*
- Registers
- */
+#define LP8720_GENERAL_SETTINGS 0x00 +#define LP8720_LDO1_SETTINGS 0x01 +#define LP8720_LDO2_SETTINGS 0x02 +#define LP8720_LDO3_SETTINGS 0x03 +#define LP8720_LDO4_SETTINGS 0x04 +#define LP8720_LDO5_SETTINGS 0x05 +#define LP8720_BUCK_SETTINGS1 0x06 +#define LP8720_BUCK_SETTINGS2 0x07 +#define LP8720_ENABLE_BITS 0x08 +#define LP8720_PULLDOWN_BITS 0x09 +#define LP8720_STATUS_BITS 0x0A +#define LP8720_INTERRUPT_BITS 0x0B +#define LP8720_INTERRUPT_MASK 0x0C
+/*
- Values
- */
+/* LP8720_GENERAL_SETTINGS */ +#define LP8720_25_US_TIME_STEP (1 << 0) +#define LP8720_50_US_TIME_STEP (0 << 0)
+/* LP8720_LDO*_SETTINGS */ +#define LP8720_LDO1235_V_12 0x00 +#define LP8720_LDO1235_V_18 0x0C +#define LP8720_LDO1235_V_30 0x1D +#define LP8720_LDO1235_V_33 0x1F
+#define LP8720_LDO4_V_12 0x00 +#define LP8720_LDO4_V_14 0x09 +#define LP8720_LDO4_V_18 0x11 +#define LP8720_LDO4_V_28 0x1E
+#define LP8720_DELAY_0 0 +#define LP8720_DELAY_1_TS 1 +#define LP8720_DELAY_2_TS 2 +#define LP8720_DELAY_3_TS 3 +#define LP8720_DELAY_4_TS 4 +#define LP8720_DELAY_5_TS 5 +#define LP8720_DELAY_6_TS 6 +#define LP8720_DELAY_NO_START 7
+/* LP8720_ENABLE_BITS */ +#define LP8720_LDO1_EN (1 << 0) +#define LP8720_LDO2_EN (1 << 1) +#define LP8720_LDO3_EN (1 << 2) +#define LP8720_LDO4_EN (1 << 3) +#define LP8720_LDO5_EN (1 << 4) +#define LP8720_BUCK_EN (1 << 5)
+/*
- Driver info
- */
+struct lp8720_info {
- int enable_gpio;
- int chip_idsel;
+};
+/*
- Declarations
- */
+int lp8720_init(int enable_gpio, int chip_idsel); +int lp8720_enable(void); +int lp8720_is_enabled(void); +int lp8720_ldo_enable(u8 ldo_enable); +int lp8720_ldo_voltage(u8 ldo_reg, u8 voltage, u8 delay);
+#endif
U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Maybe the new pmic framework will be ready before you add the new board, and it should be more flexible. It allows setting custom modes for each ldo - like ldo startup delay in your driver.
But if you will add the new board before the new pmic framework, then this code should be generally ok. Just please use the present pmic framework.
Best regards,
participants (3)
-
Paul Kocialkowski -
Przemyslaw Marczak -
Tom Rini