[U-Boot] [PATCH V4 0/6] SMDK5420: Add S2MPS11 pmic support to SMDK5420
This patchset adds support for S2MPS11 pmic on SMDK5420
Changes since V3: - Removed pmic_deselect() function and made one common function for i2c bus selection. - Made some cosmetic changes suggested by Minkyu Kang mk7.kang@samsung.com
Changes since V2: - Rebased on V6 version patchset sent by Rajeshwari http://comments.gmane.org/gmane.comp.boot-loaders.u-boot/172653
Changes since V1: - In patch "exynos: Use common pmic_reg_update() definition" moved pmic_reg_update() from drivers/power/power_i2c.c to power_core.c suggested by Lukasz Majewski l.majewski@samsung.com - Changed the License details to GPL 2.0+ license for below pathces SMDK5420: S2MPS11: Adds the register settings for S2MPS11 exynos: Add a common DT based PMIC driver initialization - corrected the typo error in "config: SMDK5420: Enable S2MPS11 pmic" patch header
Leela Krishna Amudala (6): exynos: Use common pmic_reg_update() definition power: Explicitly select pmic device's bus FDT: Exynos5420: Add compatible srings for PMIC SMDK5420: S2MPS11: Adds the register settings for S2MPS11 exynos: Add a common DT based PMIC driver initialization config: SMDK5420: Enable S2MPS11 pmic
board/samsung/common/board.c | 39 ++++++----- drivers/power/pmic/Makefile | 1 + drivers/power/pmic/pmic_common.c | 97 ++++++++++++++++++++++++++ drivers/power/power_core.c | 34 +++++++++ drivers/power/power_i2c.c | 48 +++++++++++-- include/configs/smdk5420.h | 4 ++ include/fdtdec.h | 1 + include/power/pmic.h | 35 ++++++++++ include/power/s2mps11_pmic.h | 141 ++++++++++++++++++++++++++++++++++++++ lib/fdtdec.c | 1 + 10 files changed, 378 insertions(+), 23 deletions(-) create mode 100644 drivers/power/pmic/pmic_common.c create mode 100644 include/power/s2mps11_pmic.h
This function is used by different Exynos platforms, put it in the common file.
Signed-off-by: Vadim Bendebury vbendeb@chromium.org Signed-off-by: Leela Krishna Amudala l.krishna@samsung.com Reviewed-by: Doug Anderson dianders@google.com Reviewed-by: Lukasz Majewski l.majewski@samsung.com Acked-by: Simon Glass sjg@chromium.org --- board/samsung/common/board.c | 19 ------------------- drivers/power/power_core.c | 20 ++++++++++++++++++++ include/power/pmic.h | 1 + 3 files changed, 21 insertions(+), 19 deletions(-)
diff --git a/board/samsung/common/board.c b/board/samsung/common/board.c index cd873bc..b3b84c1 100644 --- a/board/samsung/common/board.c +++ b/board/samsung/common/board.c @@ -162,25 +162,6 @@ static int board_init_cros_ec_devices(const void *blob)
#if defined(CONFIG_POWER) #ifdef CONFIG_POWER_MAX77686 -static int pmic_reg_update(struct pmic *p, int reg, uint regval) -{ - u32 val; - int ret = 0; - - ret = pmic_reg_read(p, reg, &val); - if (ret) { - debug("%s: PMIC %d register read failed\n", __func__, reg); - return -1; - } - val |= regval; - ret = pmic_reg_write(p, reg, val); - if (ret) { - debug("%s: PMIC %d register write failed\n", __func__, reg); - return -1; - } - return 0; -} - static int max77686_init(void) { struct pmic *p; diff --git a/drivers/power/power_core.c b/drivers/power/power_core.c index 29ccc83..a1c4fd0 100644 --- a/drivers/power/power_core.c +++ b/drivers/power/power_core.c @@ -208,6 +208,26 @@ int do_pmic(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) return CMD_RET_SUCCESS; }
+int pmic_reg_update(struct pmic *p, int reg, u32 val) +{ + u32 regval; + int ret; + + ret = pmic_reg_read(p, reg, ®val); + if (ret) { + debug("%s: PMIC %d register read failed\n", __func__, reg); + return -1; + } + + regval |= val; + ret = pmic_reg_write(p, reg, regval); + if (ret) { + debug("%s: PMIC %d register write failed\n", __func__, reg); + return -1; + } + return 0; +} + U_BOOT_CMD( pmic, CONFIG_SYS_MAXARGS, 1, do_pmic, "PMIC", diff --git a/include/power/pmic.h b/include/power/pmic.h index 0e7aa31..e0b9139 100644 --- a/include/power/pmic.h +++ b/include/power/pmic.h @@ -83,6 +83,7 @@ int pmic_probe(struct pmic *p); int pmic_reg_read(struct pmic *p, u32 reg, u32 *val); int pmic_reg_write(struct pmic *p, u32 reg, u32 val); int pmic_set_output(struct pmic *p, u32 reg, int ldo, int on); +int pmic_reg_update(struct pmic *p, int reg, u32 val);
#define pmic_i2c_addr (p->hw.i2c.addr) #define pmic_i2c_tx_num (p->hw.i2c.tx_num)
The current pmic i2c code assumes the current i2c bus is the same as the pmic device's bus. There is nothing ensuring that to be true. Therefore, select the proper bus before performing a transaction.
Signed-off-by: Aaron Durbin adurbin@chromium.org Signed-off-by: Simon Glass sjg@chromium.org Signed-off-by: Leela Krishna Amudala l.krishna@samsung.com Reviewed-by: Doug Anderson dianders@google.com Acked-by: Simon Glass sjg@chromium.org --- drivers/power/power_i2c.c | 48 ++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 43 insertions(+), 5 deletions(-)
diff --git a/drivers/power/power_i2c.c b/drivers/power/power_i2c.c index ac76870..e4986bb 100644 --- a/drivers/power/power_i2c.c +++ b/drivers/power/power_i2c.c @@ -16,9 +16,29 @@ #include <i2c.h> #include <compiler.h>
+static int pmic_select(int bus) +{ + int ret, old_bus; + + old_bus = i2c_get_bus_num(); + + if (old_bus == bus) + return old_bus; + + debug("%s: Select bus %d\n", __func__, bus); + ret = i2c_set_bus_num(bus); + if (ret) { + debug("%s: Cannot select pmic, err %d\n", __func__, ret); + return -1; + } + + return old_bus; +} + int pmic_reg_write(struct pmic *p, u32 reg, u32 val) { unsigned char buf[4] = { 0 }; + int ret, old_bus;
if (check_reg(p, reg)) return -1; @@ -52,23 +72,34 @@ int pmic_reg_write(struct pmic *p, u32 reg, u32 val) return -1; }
- if (i2c_write(pmic_i2c_addr, reg, 1, buf, pmic_i2c_tx_num)) + old_bus = pmic_select(p->bus); + if (old_bus < 0) return -1;
- return 0; + ret = i2c_write(pmic_i2c_addr, reg, 1, buf, pmic_i2c_tx_num); + pmic_select(old_bus); + + return ret; }
int pmic_reg_read(struct pmic *p, u32 reg, u32 *val) { unsigned char buf[4] = { 0 }; u32 ret_val = 0; + int ret, old_bus;
if (check_reg(p, reg)) return -1;
- if (i2c_read(pmic_i2c_addr, reg, 1, buf, pmic_i2c_tx_num)) + old_bus = pmic_select(p->bus); + if (old_bus < 0) return -1;
+ ret = i2c_read(pmic_i2c_addr, reg, 1, buf, pmic_i2c_tx_num); + pmic_select(old_bus); + if (ret) + return ret; + switch (pmic_i2c_tx_num) { case 3: if (p->sensor_byte_order == PMIC_SENSOR_BYTE_ORDER_BIG) @@ -98,9 +129,16 @@ int pmic_reg_read(struct pmic *p, u32 reg, u32 *val)
int pmic_probe(struct pmic *p) { - i2c_set_bus_num(p->bus); + int ret, old_bus; + + old_bus = pmic_select(p->bus); + if (old_bus < 0) + return -1; + debug("Bus: %d PMIC:%s probed!\n", p->bus, p->name); - if (i2c_probe(pmic_i2c_addr)) { + ret = i2c_probe(pmic_i2c_addr); + pmic_select(old_bus); + if (ret) { printf("Can't find PMIC:%s\n", p->name); return -1; }
Add required compatible strings for PMIC S2MPS11
Signed-off-by: Leela Krishna Amudala l.krishna@samsung.com Acked-by: Simon Glass sjg@chromium.org --- include/fdtdec.h | 1 + lib/fdtdec.c | 1 + 2 files changed, 2 insertions(+)
diff --git a/include/fdtdec.h b/include/fdtdec.h index 433d6a7..80744f5 100644 --- a/include/fdtdec.h +++ b/include/fdtdec.h @@ -87,6 +87,7 @@ enum fdt_compat_id { COMPAT_INFINEON_SLB9635_TPM, /* Infineon SLB9635 TPM */ COMPAT_INFINEON_SLB9645_TPM, /* Infineon SLB9645 TPM */ COMPAT_SAMSUNG_EXYNOS5_I2C, /* Exynos5 High Speed I2C Controller */ + COMPAT_SAMSUNG_S2MPS11_PMIC, /* S2MPS11 PMIC */
COMPAT_COUNT, }; diff --git a/lib/fdtdec.c b/lib/fdtdec.c index 207314f..b4bc57a 100644 --- a/lib/fdtdec.c +++ b/lib/fdtdec.c @@ -60,6 +60,7 @@ static const char * const compat_names[COMPAT_COUNT] = { COMPAT(INFINEON_SLB9635_TPM, "infineon,slb9635-tpm"), COMPAT(INFINEON_SLB9645_TPM, "infineon,slb9645-tpm"), COMPAT(SAMSUNG_EXYNOS5_I2C, "samsung,exynos5-hsi2c"), + COMPAT(SAMSUNG_S2MPS11_PMIC, "samsung,s2mps11-pmic"), };
const char *fdtdec_get_compatible(enum fdt_compat_id id)
Adds the register settings, addresses and voltages associated with S2MPS11
Signed-off-by: Alim Akhtar alim.akhtar@samsung.com Signed-off-by: Leela Krishna Amudala l.krishna@samsung.com Reviewed-by: Vadim Bendebury vbendeb@google.com Reviewed-by: Lukasz Majewski l.majewski@samsung.com Acked-by: Simon Glass sjg@chromium.org --- include/power/s2mps11_pmic.h | 141 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 141 insertions(+) create mode 100644 include/power/s2mps11_pmic.h
diff --git a/include/power/s2mps11_pmic.h b/include/power/s2mps11_pmic.h new file mode 100644 index 0000000..20c781d --- /dev/null +++ b/include/power/s2mps11_pmic.h @@ -0,0 +1,141 @@ +/* + * s2mps11_pmic.h + * + * Copyright (c) 2012 Samsung Electronics Co., Ltd + * http://www.samsung.com + * + * SPDX-License-Identifier: GPL-2.0+ + * + */ +#ifndef __S2MPS11_H +#define __S2MPS11_H + +/* S2MPS11 registers */ +enum s2mps11_reg { + S2MPS11_REG_ID, + S2MPS11_REG_INT1, + S2MPS11_REG_INT2, + S2MPS11_REG_INT3, + S2MPS11_REG_INT1M, + S2MPS11_REG_INT2M, + S2MPS11_REG_INT3M, + S2MPS11_REG_ST1, + S2MPS11_REG_ST2, + S2MPS11_REG_OFFSRC, + S2MPS11_REG_PWRONSRC, + S2MPS11_REG_RTC_CTRL, + S2MPS11_REG_CTRL1, + S2MPS11_REG_ETC_TEST, + S2MPS11_REG_RSVD3, + S2MPS11_REG_BU_CHG, + S2MPS11_REG_RAMP, + S2MPS11_REG_RAMP_BUCK, + S2MPS11_REG_LDO1_8, + S2MPS11_REG_LDO9_16, + S2MPS11_REG_LDO17_24, + S2MPS11_REG_LDO25_32, + S2MPS11_REG_LDO33_38, + S2MPS11_REG_LDO1_8_1, + S2MPS11_REG_LDO9_16_1, + S2MPS11_REG_LDO17_24_1, + S2MPS11_REG_LDO25_32_1, + S2MPS11_REG_LDO33_38_1, + S2MPS11_REG_OTP_ADRL, + S2MPS11_REG_OTP_ADRH, + S2MPS11_REG_OTP_DATA, + S2MPS11_REG_MON1SEL, + S2MPS11_REG_MON2SEL, + S2MPS11_REG_LEE, + S2MPS11_REG_RSVD_NO, + S2MPS11_REG_UVLO, + S2MPS11_REG_LEE_NO, + S2MPS11_REG_B1CTRL1, + S2MPS11_REG_B1CTRL2, + S2MPS11_REG_B2CTRL1, + S2MPS11_REG_B2CTRL2, + S2MPS11_REG_B3CTRL1, + S2MPS11_REG_B3CTRL2, + S2MPS11_REG_B4CTRL1, + S2MPS11_REG_B4CTRL2, + S2MPS11_REG_B5CTRL1, + S2MPS11_REG_BUCK5_SW, + S2MPS11_REG_B5CTRL2, + S2MPS11_REG_B5CTRL3, + S2MPS11_REG_B5CTRL4, + S2MPS11_REG_B5CTRL5, + S2MPS11_REG_B6CTRL1, + S2MPS11_REG_B6CTRL2, + S2MPS11_REG_B7CTRL1, + S2MPS11_REG_B7CTRL2, + S2MPS11_REG_B8CTRL1, + S2MPS11_REG_B8CTRL2, + S2MPS11_REG_B9CTRL1, + S2MPS11_REG_B9CTRL2, + S2MPS11_REG_B10CTRL1, + S2MPS11_REG_B10CTRL2, + S2MPS11_REG_L1CTRL, + S2MPS11_REG_L2CTRL, + S2MPS11_REG_L3CTRL, + S2MPS11_REG_L4CTRL, + S2MPS11_REG_L5CTRL, + S2MPS11_REG_L6CTRL, + S2MPS11_REG_L7CTRL, + S2MPS11_REG_L8CTRL, + S2MPS11_REG_L9CTRL, + S2MPS11_REG_L10CTRL, + S2MPS11_REG_L11CTRL, + S2MPS11_REG_L12CTRL, + S2MPS11_REG_L13CTRL, + S2MPS11_REG_L14CTRL, + S2MPS11_REG_L15CTRL, + S2MPS11_REG_L16CTRL, + S2MPS11_REG_L17CTRL, + S2MPS11_REG_L18CTRL, + S2MPS11_REG_L19CTRL, + S2MPS11_REG_L20CTRL, + S2MPS11_REG_L21CTRL, + S2MPS11_REG_L22CTRL, + S2MPS11_REG_L23CTRL, + S2MPS11_REG_L24CTRL, + S2MPS11_REG_L25CTRL, + S2MPS11_REG_L26CTRL, + S2MPS11_REG_L27CTRL, + S2MPS11_REG_L28CTRL, + S2MPS11_REG_L29CTRL, + S2MPS11_REG_L30CTRL, + S2MPS11_REG_L31CTRL, + S2MPS11_REG_L32CTRL, + S2MPS11_REG_L33CTRL, + S2MPS11_REG_L34CTRL, + S2MPS11_REG_L35CTRL, + S2MPS11_REG_L36CTRL, + S2MPS11_REG_L37CTRL, + S2MPS11_REG_L38CTRL, + + S2MPS11_NUM_OF_REGS, +}; + +/* I2C device address for pmic S2MPS11 */ +#define S2MPS11_I2C_ADDR (0xCC >> 1) +#define S2MPS11_BUS_NUM 4 + +/* Value to set voltage as 1V */ +#define S2MPS11_BUCK_CTRL2_1V 0x40 +/* Value to set voltage as 1.2V */ +#define S2MPS11_BUCK_CTRL2_1_2V 0x60 +/* Value to set voltage as 1.2625V */ +#define S2MPS11_BUCK_CTRL2_1_2625V 0x6A + +/* Buck register addresses */ +#define S2MPS11_BUCK1_CTRL2 0x26 +#define S2MPS11_BUCK2_CTRL2 0x28 +#define S2MPS11_BUCK3_CTRL2 0x2a +#define S2MPS11_BUCK4_CTRL2 0x2c +#define S2MPS11_BUCK6_CTRL2 0x34 +#define S2MPS11_LDO22_CTRL 0x52 + +#define S2MPS11_DEVICE_NAME "S2MPS11_PMIC" + +#define S2MPS11_RTC_CTRL_32KHZ_CP_EN (1 << 1) +#define S2MPS11_RTC_CTRL_JIT (1 << 4) +#endif /* __LINUX_MFD_S2MPS11_H */
Most of i2c PMIC drivers follow the same initialization sequence, let's generalize it in a common file.
The initialization function finds the PMIC in the device tree, and if found - registers it in the list of known PMICs and initializes it, iterating through the table of settings supplied by the caller.
Signed-off-by: Vadim Bendebury vbendeb@chromium.org Signed-off-by: Leela Krishna Amudala l.krishna@samsung.com Reviewed-by: Doug Anderson dianders@google.com Acked-by: Simon Glass sjg@chromium.org --- board/samsung/common/board.c | 22 +++++++++ drivers/power/pmic/Makefile | 1 + drivers/power/pmic/pmic_common.c | 97 ++++++++++++++++++++++++++++++++++++++ drivers/power/power_core.c | 14 ++++++ include/power/pmic.h | 34 +++++++++++++ 5 files changed, 168 insertions(+) create mode 100644 drivers/power/pmic/pmic_common.c
diff --git a/board/samsung/common/board.c b/board/samsung/common/board.c index b3b84c1..d23a7a6 100644 --- a/board/samsung/common/board.c +++ b/board/samsung/common/board.c @@ -23,6 +23,7 @@ #include <power/pmic.h> #include <asm/arch/sromc.h> #include <power/max77686_pmic.h> +#include <power/s2mps11_pmic.h>
DECLARE_GLOBAL_DATA_PTR;
@@ -160,6 +161,25 @@ static int board_init_cros_ec_devices(const void *blob) } #endif
+#ifdef CONFIG_POWER_S2MPS11 +int board_init_s2mps11(void) +{ + const struct pmic_init_ops pmic_ops[] = { + {PMIC_REG_WRITE, S2MPS11_BUCK1_CTRL2, S2MPS11_BUCK_CTRL2_1V}, + {PMIC_REG_WRITE, S2MPS11_BUCK2_CTRL2, + S2MPS11_BUCK_CTRL2_1_2625V}, + {PMIC_REG_WRITE, S2MPS11_BUCK3_CTRL2, S2MPS11_BUCK_CTRL2_1V}, + {PMIC_REG_WRITE, S2MPS11_BUCK4_CTRL2, S2MPS11_BUCK_CTRL2_1V}, + {PMIC_REG_WRITE, S2MPS11_BUCK6_CTRL2, S2MPS11_BUCK_CTRL2_1V}, + {PMIC_REG_UPDATE, S2MPS11_REG_RTC_CTRL, + S2MPS11_RTC_CTRL_32KHZ_CP_EN | S2MPS11_RTC_CTRL_JIT}, + {PMIC_REG_BAIL} + }; + + return pmic_common_init(COMPAT_SAMSUNG_S2MPS11_PMIC, pmic_ops); +} +#endif + #if defined(CONFIG_POWER) #ifdef CONFIG_POWER_MAX77686 static int max77686_init(void) @@ -263,6 +283,8 @@ int power_init_board(void)
#ifdef CONFIG_POWER_MAX77686 ret = max77686_init(); +#elif defined(CONFIG_POWER_S2MPS11) + ret = board_init_s2mps11(); #endif
return ret; diff --git a/drivers/power/pmic/Makefile b/drivers/power/pmic/Makefile index 0b45ffa..5e236a3 100644 --- a/drivers/power/pmic/Makefile +++ b/drivers/power/pmic/Makefile @@ -11,3 +11,4 @@ obj-$(CONFIG_POWER_MUIC_MAX8997) += muic_max8997.o obj-$(CONFIG_POWER_MAX77686) += pmic_max77686.o obj-$(CONFIG_POWER_TPS65217) += pmic_tps65217.o obj-$(CONFIG_POWER_TPS65910) += pmic_tps65910.o +obj-$(CONFIG_POWER) += pmic_common.o diff --git a/drivers/power/pmic/pmic_common.c b/drivers/power/pmic/pmic_common.c new file mode 100644 index 0000000..3117ae2 --- /dev/null +++ b/drivers/power/pmic/pmic_common.c @@ -0,0 +1,97 @@ +/* + * Copyright (c) 2013 The Chromium OS Authors. All rights reserved. + * + * SPDX-License-Identifier: GPL-2.0+ + */ +#include <common.h> +#include <fdtdec.h> +#include <errno.h> +#include <power/pmic.h> +#include <power/s2mps11_pmic.h> +#include <power/max77686_pmic.h> + +DECLARE_GLOBAL_DATA_PTR; + +static unsigned pmic_number_of_regs(enum fdt_compat_id pmic_compat) +{ + switch (pmic_compat) { + case COMPAT_SAMSUNG_S2MPS11_PMIC: + return S2MPS11_NUM_OF_REGS; + default: + break; + } + return 0; +} + +int pmic_common_init(enum fdt_compat_id pmic_compat, + const struct pmic_init_ops *pmic_ops) +{ + const void *blob = gd->fdt_blob; + struct pmic *p; + int node, parent, ret; + unsigned number_of_regs = pmic_number_of_regs(pmic_compat); + const char *pmic_name, *comma; + + if (!number_of_regs) { + printf("%s: %s - not a supported PMIC\n", + __func__, fdtdec_get_compatible(pmic_compat)); + return -1; + } + + node = fdtdec_next_compatible(blob, 0, pmic_compat); + if (node < 0) { + debug("PMIC: Error %s. No node for %s in device tree\n", + fdt_strerror(node), fdtdec_get_compatible(pmic_compat)); + return node; + } + + pmic_name = fdtdec_get_compatible(pmic_compat); + comma = strchr(pmic_name, ','); + if (comma) + pmic_name = comma + 1; + + p = pmic_alloc(); + + if (!p) { + printf("%s: POWER allocation error!\n", __func__); + return -ENOMEM; + } + parent = fdt_parent_offset(blob, node); + if (parent < 0) { + debug("%s: Cannot find node parent\n", __func__); + return -1; + } + + p->bus = i2c_get_bus_num_fdt(parent); + if (p->bus < 0) { + debug("%s: Cannot find I2C bus\n", __func__); + return -1; + } + p->hw.i2c.addr = fdtdec_get_int(blob, node, "reg", 9); + + p->name = pmic_name; + p->interface = PMIC_I2C; + p->hw.i2c.tx_num = 1; + p->number_of_regs = number_of_regs; + p->compat_id = pmic_compat; + + ret = 0; + while ((pmic_ops->reg_op != PMIC_REG_BAIL) && !ret) { + if (pmic_ops->reg_op == PMIC_REG_WRITE) + ret = pmic_reg_write(p, + pmic_ops->reg_addr, + pmic_ops->reg_value); + else + ret = pmic_reg_update(p, + pmic_ops->reg_addr, + pmic_ops->reg_value); + pmic_ops++; + } + + if (ret) + printf("%s: Failed accessing reg 0x%x of %s\n", + __func__, pmic_ops[-1].reg_addr, p->name); + else + printf("PMIC %s initialized\n", p->name); + return ret; +} diff --git a/drivers/power/power_core.c b/drivers/power/power_core.c index a1c4fd0..f40be31 100644 --- a/drivers/power/power_core.c +++ b/drivers/power/power_core.c @@ -228,6 +228,20 @@ int pmic_reg_update(struct pmic *p, int reg, u32 val) return 0; }
+struct pmic *pmic_get_by_id(enum fdt_compat_id pmic_compat) +{ + struct pmic *p; + + list_for_each_entry(p, &pmic_list, list) { + if (p->compat_id == pmic_compat) { + debug("%s: pmic %s -> 0x%p\n", __func__, p->name, p); + return p; + } + } + + return NULL; +} + U_BOOT_CMD( pmic, CONFIG_SYS_MAXARGS, 1, do_pmic, "PMIC", diff --git a/include/power/pmic.h b/include/power/pmic.h index e0b9139..e0982e3 100644 --- a/include/power/pmic.h +++ b/include/power/pmic.h @@ -12,6 +12,7 @@ #include <linux/list.h> #include <i2c.h> #include <power/power_chrg.h> +#include <fdtdec.h>
enum { PMIC_I2C, PMIC_SPI, PMIC_NONE}; enum { I2C_PMIC, I2C_NUM, }; @@ -72,6 +73,7 @@ struct pmic {
struct pmic *parent; struct list_head list; + enum fdt_compat_id compat_id; };
int pmic_init(unsigned char bus); @@ -84,6 +86,38 @@ int pmic_reg_read(struct pmic *p, u32 reg, u32 *val); int pmic_reg_write(struct pmic *p, u32 reg, u32 val); int pmic_set_output(struct pmic *p, u32 reg, int ldo, int on); int pmic_reg_update(struct pmic *p, int reg, u32 val); +/* + * Find registered PMIC based on its compatibility ID. + * + * @param pmic_compat compatibility ID of the PMIC to search for. + * @return pointer to the relevant 'struct pmic' on success or NULL + */ +struct pmic *pmic_get_by_id(enum fdt_compat_id pmic_compat); + +enum pmic_reg_op { PMIC_REG_BAIL, PMIC_REG_WRITE, PMIC_REG_UPDATE }; +struct pmic_init_ops { + enum pmic_reg_op reg_op; + u8 reg_addr; + u8 reg_value; +}; + +/** + * Common function used to intialize an i2c based PMIC. + * + * This function finds the PMIC in the device tree based on its compatibility + * ID. If found, the struct pmic is allocated, initialized and registered. + * + * Then the table of initialization settings is scanned and the PMIC registers + * are set as dictated by the table contents, + * + * @param pmic_compat compatibility ID f the PMIC to be initialized. + * @param pmic_ops a pointer to the table containing PMIC initialization + * settings. The last entry contains reg_op + * of PMIC_REG_BAIL. + * @return zero on success, nonzero on failure + */ +int pmic_common_init(enum fdt_compat_id pmic_compat, + const struct pmic_init_ops *pmic_ops);
#define pmic_i2c_addr (p->hw.i2c.addr) #define pmic_i2c_tx_num (p->hw.i2c.tx_num)
On 06/01/14 20:49, Leela Krishna Amudala wrote:
Most of i2c PMIC drivers follow the same initialization sequence, let's generalize it in a common file.
The initialization function finds the PMIC in the device tree, and if found - registers it in the list of known PMICs and initializes it, iterating through the table of settings supplied by the caller.
Signed-off-by: Vadim Bendebury vbendeb@chromium.org Signed-off-by: Leela Krishna Amudala l.krishna@samsung.com Reviewed-by: Doug Anderson dianders@google.com Acked-by: Simon Glass sjg@chromium.org
board/samsung/common/board.c | 22 +++++++++ drivers/power/pmic/Makefile | 1 + drivers/power/pmic/pmic_common.c | 97 ++++++++++++++++++++++++++++++++++++++ drivers/power/power_core.c | 14 ++++++ include/power/pmic.h | 34 +++++++++++++ 5 files changed, 168 insertions(+) create mode 100644 drivers/power/pmic/pmic_common.c
diff --git a/board/samsung/common/board.c b/board/samsung/common/board.c index b3b84c1..d23a7a6 100644 --- a/board/samsung/common/board.c +++ b/board/samsung/common/board.c @@ -23,6 +23,7 @@ #include <power/pmic.h> #include <asm/arch/sromc.h> #include <power/max77686_pmic.h> +#include <power/s2mps11_pmic.h>
Do we need to include those two header files (max77686 and s2mps11) together?
DECLARE_GLOBAL_DATA_PTR;
@@ -160,6 +161,25 @@ static int board_init_cros_ec_devices(const void *blob) } #endif
+#ifdef CONFIG_POWER_S2MPS11
please move this block into "#if defined(CONFIG_POWER)".
+int board_init_s2mps11(void) +{
- const struct pmic_init_ops pmic_ops[] = {
{PMIC_REG_WRITE, S2MPS11_BUCK1_CTRL2, S2MPS11_BUCK_CTRL2_1V},{PMIC_REG_WRITE, S2MPS11_BUCK2_CTRL2,S2MPS11_BUCK_CTRL2_1_2625V},{PMIC_REG_WRITE, S2MPS11_BUCK3_CTRL2, S2MPS11_BUCK_CTRL2_1V},{PMIC_REG_WRITE, S2MPS11_BUCK4_CTRL2, S2MPS11_BUCK_CTRL2_1V},{PMIC_REG_WRITE, S2MPS11_BUCK6_CTRL2, S2MPS11_BUCK_CTRL2_1V},{PMIC_REG_UPDATE, S2MPS11_REG_RTC_CTRL,S2MPS11_RTC_CTRL_32KHZ_CP_EN | S2MPS11_RTC_CTRL_JIT},{PMIC_REG_BAIL}- };
- return pmic_common_init(COMPAT_SAMSUNG_S2MPS11_PMIC, pmic_ops);
+} +#endif
#if defined(CONFIG_POWER) #ifdef CONFIG_POWER_MAX77686 static int max77686_init(void) @@ -263,6 +283,8 @@ int power_init_board(void)
#ifdef CONFIG_POWER_MAX77686 ret = max77686_init(); +#elif defined(CONFIG_POWER_S2MPS11)
- ret = board_init_s2mps11();
#endif
return ret; diff --git a/drivers/power/pmic/Makefile b/drivers/power/pmic/Makefile index 0b45ffa..5e236a3 100644 --- a/drivers/power/pmic/Makefile +++ b/drivers/power/pmic/Makefile @@ -11,3 +11,4 @@ obj-$(CONFIG_POWER_MUIC_MAX8997) += muic_max8997.o obj-$(CONFIG_POWER_MAX77686) += pmic_max77686.o obj-$(CONFIG_POWER_TPS65217) += pmic_tps65217.o obj-$(CONFIG_POWER_TPS65910) += pmic_tps65910.o +obj-$(CONFIG_POWER) += pmic_common.o diff --git a/drivers/power/pmic/pmic_common.c b/drivers/power/pmic/pmic_common.c new file mode 100644 index 0000000..3117ae2 --- /dev/null +++ b/drivers/power/pmic/pmic_common.c @@ -0,0 +1,97 @@ +/*
- Copyright (c) 2013 The Chromium OS Authors. All rights reserved.
Please write on the author of this file.
- SPDX-License-Identifier: GPL-2.0+
- */
+#include <common.h> +#include <fdtdec.h> +#include <errno.h> +#include <power/pmic.h> +#include <power/s2mps11_pmic.h> +#include <power/max77686_pmic.h>
Why common driver need specific pmic's header file? It is wrong architecture.
+DECLARE_GLOBAL_DATA_PTR;
+static unsigned pmic_number_of_regs(enum fdt_compat_id pmic_compat) +{
- switch (pmic_compat) {
- case COMPAT_SAMSUNG_S2MPS11_PMIC:
return S2MPS11_NUM_OF_REGS;- default:
break;- }
- return 0;
+}
+int pmic_common_init(enum fdt_compat_id pmic_compat,
const struct pmic_init_ops *pmic_ops)+{
- const void *blob = gd->fdt_blob;
- struct pmic *p;
- int node, parent, ret;
- unsigned number_of_regs = pmic_number_of_regs(pmic_compat);
- const char *pmic_name, *comma;
- if (!number_of_regs) {
printf("%s: %s - not a supported PMIC\n",__func__, fdtdec_get_compatible(pmic_compat));return -1;- }
- node = fdtdec_next_compatible(blob, 0, pmic_compat);
- if (node < 0) {
debug("PMIC: Error %s. No node for %s in device tree\n",fdt_strerror(node), fdtdec_get_compatible(pmic_compat));return node;- }
- pmic_name = fdtdec_get_compatible(pmic_compat);
- comma = strchr(pmic_name, ',');
- if (comma)
pmic_name = comma + 1;- p = pmic_alloc();
- if (!p) {
printf("%s: POWER allocation error!\n", __func__);return -ENOMEM;- }
- parent = fdt_parent_offset(blob, node);
- if (parent < 0) {
debug("%s: Cannot find node parent\n", __func__);return -1;- }
- p->bus = i2c_get_bus_num_fdt(parent);
- if (p->bus < 0) {
debug("%s: Cannot find I2C bus\n", __func__);return -1;- }
- p->hw.i2c.addr = fdtdec_get_int(blob, node, "reg", 9);
- p->name = pmic_name;
- p->interface = PMIC_I2C;
- p->hw.i2c.tx_num = 1;
- p->number_of_regs = number_of_regs;
- p->compat_id = pmic_compat;
- ret = 0;
- while ((pmic_ops->reg_op != PMIC_REG_BAIL) && !ret) {
if (pmic_ops->reg_op == PMIC_REG_WRITE)ret = pmic_reg_write(p,pmic_ops->reg_addr,pmic_ops->reg_value);elseret = pmic_reg_update(p,pmic_ops->reg_addr,pmic_ops->reg_value);pmic_ops++;- }
- if (ret)
printf("%s: Failed accessing reg 0x%x of %s\n",__func__, pmic_ops[-1].reg_addr, p->name);
pmic_ops[-1].reg_addr, is it right?
- else
printf("PMIC %s initialized\n", p->name);- return ret;
+} diff --git a/drivers/power/power_core.c b/drivers/power/power_core.c index a1c4fd0..f40be31 100644 --- a/drivers/power/power_core.c +++ b/drivers/power/power_core.c @@ -228,6 +228,20 @@ int pmic_reg_update(struct pmic *p, int reg, u32 val) return 0; }
+struct pmic *pmic_get_by_id(enum fdt_compat_id pmic_compat) +{
- struct pmic *p;
- list_for_each_entry(p, &pmic_list, list) {
if (p->compat_id == pmic_compat) {debug("%s: pmic %s -> 0x%p\n", __func__, p->name, p);return p;}- }
- return NULL;
+}
U_BOOT_CMD( pmic, CONFIG_SYS_MAXARGS, 1, do_pmic, "PMIC", diff --git a/include/power/pmic.h b/include/power/pmic.h index e0b9139..e0982e3 100644 --- a/include/power/pmic.h +++ b/include/power/pmic.h @@ -12,6 +12,7 @@ #include <linux/list.h> #include <i2c.h> #include <power/power_chrg.h> +#include <fdtdec.h>
enum { PMIC_I2C, PMIC_SPI, PMIC_NONE}; enum { I2C_PMIC, I2C_NUM, }; @@ -72,6 +73,7 @@ struct pmic {
struct pmic *parent; struct list_head list;
- enum fdt_compat_id compat_id;
};
int pmic_init(unsigned char bus); @@ -84,6 +86,38 @@ int pmic_reg_read(struct pmic *p, u32 reg, u32 *val); int pmic_reg_write(struct pmic *p, u32 reg, u32 val); int pmic_set_output(struct pmic *p, u32 reg, int ldo, int on); int pmic_reg_update(struct pmic *p, int reg, u32 val); +/*
- Find registered PMIC based on its compatibility ID.
- @param pmic_compat compatibility ID of the PMIC to search for.
- @return pointer to the relevant 'struct pmic' on success or NULL
- */
+struct pmic *pmic_get_by_id(enum fdt_compat_id pmic_compat);
+enum pmic_reg_op { PMIC_REG_BAIL, PMIC_REG_WRITE, PMIC_REG_UPDATE }; +struct pmic_init_ops {
- enum pmic_reg_op reg_op;
- u8 reg_addr;
- u8 reg_value;
u8? why? according to pmic.h, all of pmic operations are allowed u32.
+};
+/**
- Common function used to intialize an i2c based PMIC.
- This function finds the PMIC in the device tree based on its compatibility
- ID. If found, the struct pmic is allocated, initialized and registered.
- Then the table of initialization settings is scanned and the PMIC registers
- are set as dictated by the table contents,
- @param pmic_compat compatibility ID f the PMIC to be initialized.
- @param pmic_ops a pointer to the table containing PMIC initialization
settings. The last entry contains reg_op
of PMIC_REG_BAIL.
- @return zero on success, nonzero on failure
- */
+int pmic_common_init(enum fdt_compat_id pmic_compat,
const struct pmic_init_ops *pmic_ops);#define pmic_i2c_addr (p->hw.i2c.addr) #define pmic_i2c_tx_num (p->hw.i2c.tx_num)
Thanks, Minkyu Kang.
Hi Minkyu Kang,
Thanks for reviewing the patch, Please check my comments inline.
On 1/7/14, Minkyu Kang mk7.kang@samsung.com wrote:
On 06/01/14 20:49, Leela Krishna Amudala wrote:
Most of i2c PMIC drivers follow the same initialization sequence, let's generalize it in a common file.
The initialization function finds the PMIC in the device tree, and if found - registers it in the list of known PMICs and initializes it, iterating through the table of settings supplied by the caller.
Signed-off-by: Vadim Bendebury vbendeb@chromium.org Signed-off-by: Leela Krishna Amudala l.krishna@samsung.com Reviewed-by: Doug Anderson dianders@google.com Acked-by: Simon Glass sjg@chromium.org
board/samsung/common/board.c | 22 +++++++++ drivers/power/pmic/Makefile | 1 + drivers/power/pmic/pmic_common.c | 97 ++++++++++++++++++++++++++++++++++++++ drivers/power/power_core.c | 14 ++++++ include/power/pmic.h | 34 +++++++++++++ 5 files changed, 168 insertions(+) create mode 100644 drivers/power/pmic/pmic_common.c
diff --git a/board/samsung/common/board.c b/board/samsung/common/board.c index b3b84c1..d23a7a6 100644 --- a/board/samsung/common/board.c +++ b/board/samsung/common/board.c @@ -23,6 +23,7 @@ #include <power/pmic.h> #include <asm/arch/sromc.h> #include <power/max77686_pmic.h> +#include <power/s2mps11_pmic.h>
Do we need to include those two header files (max77686 and s2mps11) together?
Okay, I'll make it conditional inclusion
DECLARE_GLOBAL_DATA_PTR;
@@ -160,6 +161,25 @@ static int board_init_cros_ec_devices(const void *blob) } #endif
+#ifdef CONFIG_POWER_S2MPS11
please move this block into "#if defined(CONFIG_POWER)".
Okay, will move it
+int board_init_s2mps11(void) +{
- const struct pmic_init_ops pmic_ops[] = {
{PMIC_REG_WRITE, S2MPS11_BUCK1_CTRL2, S2MPS11_BUCK_CTRL2_1V},{PMIC_REG_WRITE, S2MPS11_BUCK2_CTRL2,S2MPS11_BUCK_CTRL2_1_2625V},{PMIC_REG_WRITE, S2MPS11_BUCK3_CTRL2, S2MPS11_BUCK_CTRL2_1V},{PMIC_REG_WRITE, S2MPS11_BUCK4_CTRL2, S2MPS11_BUCK_CTRL2_1V},{PMIC_REG_WRITE, S2MPS11_BUCK6_CTRL2, S2MPS11_BUCK_CTRL2_1V},{PMIC_REG_UPDATE, S2MPS11_REG_RTC_CTRL,S2MPS11_RTC_CTRL_32KHZ_CP_EN | S2MPS11_RTC_CTRL_JIT},{PMIC_REG_BAIL}- };
- return pmic_common_init(COMPAT_SAMSUNG_S2MPS11_PMIC, pmic_ops);
+} +#endif
#if defined(CONFIG_POWER) #ifdef CONFIG_POWER_MAX77686 static int max77686_init(void) @@ -263,6 +283,8 @@ int power_init_board(void)
#ifdef CONFIG_POWER_MAX77686 ret = max77686_init(); +#elif defined(CONFIG_POWER_S2MPS11)
- ret = board_init_s2mps11();
#endif
return ret; diff --git a/drivers/power/pmic/Makefile b/drivers/power/pmic/Makefile index 0b45ffa..5e236a3 100644 --- a/drivers/power/pmic/Makefile +++ b/drivers/power/pmic/Makefile @@ -11,3 +11,4 @@ obj-$(CONFIG_POWER_MUIC_MAX8997) += muic_max8997.o obj-$(CONFIG_POWER_MAX77686) += pmic_max77686.o obj-$(CONFIG_POWER_TPS65217) += pmic_tps65217.o obj-$(CONFIG_POWER_TPS65910) += pmic_tps65910.o +obj-$(CONFIG_POWER) += pmic_common.o diff --git a/drivers/power/pmic/pmic_common.c b/drivers/power/pmic/pmic_common.c new file mode 100644 index 0000000..3117ae2 --- /dev/null +++ b/drivers/power/pmic/pmic_common.c @@ -0,0 +1,97 @@ +/*
- Copyright (c) 2013 The Chromium OS Authors. All rights reserved.
Please write on the author of this file.
Okay, will do that
- SPDX-License-Identifier: GPL-2.0+
- */
+#include <common.h> +#include <fdtdec.h> +#include <errno.h> +#include <power/pmic.h> +#include <power/s2mps11_pmic.h> +#include <power/max77686_pmic.h>
Why common driver need specific pmic's header file? It is wrong architecture.
Here, in this file we are using PMIC compact ID to find the number of registers in a PMIC (now currently have support for only S2MPS11 and other PMICs info may added in future), so we need those headers.
+DECLARE_GLOBAL_DATA_PTR;
+static unsigned pmic_number_of_regs(enum fdt_compat_id pmic_compat) +{
- switch (pmic_compat) {
- case COMPAT_SAMSUNG_S2MPS11_PMIC:
return S2MPS11_NUM_OF_REGS;- default:
break;- }
- return 0;
+}
+int pmic_common_init(enum fdt_compat_id pmic_compat,
const struct pmic_init_ops *pmic_ops)+{
- const void *blob = gd->fdt_blob;
- struct pmic *p;
- int node, parent, ret;
- unsigned number_of_regs = pmic_number_of_regs(pmic_compat);
- const char *pmic_name, *comma;
- if (!number_of_regs) {
printf("%s: %s - not a supported PMIC\n",__func__, fdtdec_get_compatible(pmic_compat));return -1;- }
- node = fdtdec_next_compatible(blob, 0, pmic_compat);
- if (node < 0) {
debug("PMIC: Error %s. No node for %s in device tree\n",fdt_strerror(node), fdtdec_get_compatible(pmic_compat));return node;- }
- pmic_name = fdtdec_get_compatible(pmic_compat);
- comma = strchr(pmic_name, ',');
- if (comma)
pmic_name = comma + 1;- p = pmic_alloc();
- if (!p) {
printf("%s: POWER allocation error!\n", __func__);return -ENOMEM;- }
- parent = fdt_parent_offset(blob, node);
- if (parent < 0) {
debug("%s: Cannot find node parent\n", __func__);return -1;- }
- p->bus = i2c_get_bus_num_fdt(parent);
- if (p->bus < 0) {
debug("%s: Cannot find I2C bus\n", __func__);return -1;- }
- p->hw.i2c.addr = fdtdec_get_int(blob, node, "reg", 9);
- p->name = pmic_name;
- p->interface = PMIC_I2C;
- p->hw.i2c.tx_num = 1;
- p->number_of_regs = number_of_regs;
- p->compat_id = pmic_compat;
- ret = 0;
- while ((pmic_ops->reg_op != PMIC_REG_BAIL) && !ret) {
if (pmic_ops->reg_op == PMIC_REG_WRITE)ret = pmic_reg_write(p,pmic_ops->reg_addr,pmic_ops->reg_value);elseret = pmic_reg_update(p,pmic_ops->reg_addr,pmic_ops->reg_value);pmic_ops++;- }
- if (ret)
printf("%s: Failed accessing reg 0x%x of %s\n",__func__, pmic_ops[-1].reg_addr, p->name);pmic_ops[-1].reg_addr, is it right?
Yes, this is right because if you see the above while() loop we are incrementing the pmic_ops pointer after reg_write/reg_update and if any of them returns error value ,while() loop will break and the pmic_ops pointing to the next instance address. so to print the values in the previous instance pointer we are using pmic_ops[-1].
- else
printf("PMIC %s initialized\n", p->name);- return ret;
+} diff --git a/drivers/power/power_core.c b/drivers/power/power_core.c index a1c4fd0..f40be31 100644 --- a/drivers/power/power_core.c +++ b/drivers/power/power_core.c @@ -228,6 +228,20 @@ int pmic_reg_update(struct pmic *p, int reg, u32 val) return 0; }
+struct pmic *pmic_get_by_id(enum fdt_compat_id pmic_compat) +{
- struct pmic *p;
- list_for_each_entry(p, &pmic_list, list) {
if (p->compat_id == pmic_compat) {debug("%s: pmic %s -> 0x%p\n", __func__, p->name, p);return p;}- }
- return NULL;
+}
U_BOOT_CMD( pmic, CONFIG_SYS_MAXARGS, 1, do_pmic, "PMIC", diff --git a/include/power/pmic.h b/include/power/pmic.h index e0b9139..e0982e3 100644 --- a/include/power/pmic.h +++ b/include/power/pmic.h @@ -12,6 +12,7 @@ #include <linux/list.h> #include <i2c.h> #include <power/power_chrg.h> +#include <fdtdec.h>
enum { PMIC_I2C, PMIC_SPI, PMIC_NONE}; enum { I2C_PMIC, I2C_NUM, }; @@ -72,6 +73,7 @@ struct pmic {
struct pmic *parent; struct list_head list;
- enum fdt_compat_id compat_id;
};
int pmic_init(unsigned char bus); @@ -84,6 +86,38 @@ int pmic_reg_read(struct pmic *p, u32 reg, u32 *val); int pmic_reg_write(struct pmic *p, u32 reg, u32 val); int pmic_set_output(struct pmic *p, u32 reg, int ldo, int on); int pmic_reg_update(struct pmic *p, int reg, u32 val); +/*
- Find registered PMIC based on its compatibility ID.
- @param pmic_compat compatibility ID of the PMIC to search for.
- @return pointer to the relevant 'struct pmic' on success or NULL
- */
+struct pmic *pmic_get_by_id(enum fdt_compat_id pmic_compat);
+enum pmic_reg_op { PMIC_REG_BAIL, PMIC_REG_WRITE, PMIC_REG_UPDATE }; +struct pmic_init_ops {
- enum pmic_reg_op reg_op;
- u8 reg_addr;
- u8 reg_value;
u8? why? according to pmic.h, all of pmic operations are allowed u32.
Currently I don't have S2MPS11 data sheet with me, but I remember that it has 1 byte size registers. so the structure declared like that.
Best Wishes, Leela krishna.
+};
+/**
- Common function used to intialize an i2c based PMIC.
- This function finds the PMIC in the device tree based on its
compatibility
- ID. If found, the struct pmic is allocated, initialized and
registered.
- Then the table of initialization settings is scanned and the PMIC
registers
- are set as dictated by the table contents,
- @param pmic_compat compatibility ID f the PMIC to be initialized.
- @param pmic_ops a pointer to the table containing PMIC
initialization
settings. The last entry contains reg_op
of PMIC_REG_BAIL.
- @return zero on success, nonzero on failure
- */
+int pmic_common_init(enum fdt_compat_id pmic_compat,
const struct pmic_init_ops *pmic_ops);#define pmic_i2c_addr (p->hw.i2c.addr) #define pmic_i2c_tx_num (p->hw.i2c.tx_num)
Thanks, Minkyu Kang. _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
On 16/01/14 13:05, Leela Krishna Amudala wrote:
Hi Minkyu Kang,
Thanks for reviewing the patch, Please check my comments inline.
On 1/7/14, Minkyu Kang mk7.kang@samsung.com wrote:
On 06/01/14 20:49, Leela Krishna Amudala wrote:
Most of i2c PMIC drivers follow the same initialization sequence, let's generalize it in a common file.
The initialization function finds the PMIC in the device tree, and if found - registers it in the list of known PMICs and initializes it, iterating through the table of settings supplied by the caller.
Signed-off-by: Vadim Bendebury vbendeb@chromium.org Signed-off-by: Leela Krishna Amudala l.krishna@samsung.com Reviewed-by: Doug Anderson dianders@google.com Acked-by: Simon Glass sjg@chromium.org
board/samsung/common/board.c | 22 +++++++++ drivers/power/pmic/Makefile | 1 + drivers/power/pmic/pmic_common.c | 97 ++++++++++++++++++++++++++++++++++++++ drivers/power/power_core.c | 14 ++++++ include/power/pmic.h | 34 +++++++++++++ 5 files changed, 168 insertions(+) create mode 100644 drivers/power/pmic/pmic_common.c
+#include <common.h> +#include <fdtdec.h> +#include <errno.h> +#include <power/pmic.h> +#include <power/s2mps11_pmic.h> +#include <power/max77686_pmic.h>
Why common driver need specific pmic's header file? It is wrong architecture.
Here, in this file we are using PMIC compact ID to find the number of registers in a PMIC (now currently have support for only S2MPS11 and other PMICs info may added in future), so we need those headers.
So, it's a wrong architecture. It (find the number of registers) is a PMIC specific feature.
+DECLARE_GLOBAL_DATA_PTR;
+static unsigned pmic_number_of_regs(enum fdt_compat_id pmic_compat)
As I said, this function is not a common feature.
+{
- switch (pmic_compat) {
- case COMPAT_SAMSUNG_S2MPS11_PMIC:
return S2MPS11_NUM_OF_REGS;- default:
break;- }
- return 0;
+}
+int pmic_common_init(enum fdt_compat_id pmic_compat,
const struct pmic_init_ops *pmic_ops)+{
- const void *blob = gd->fdt_blob;
- struct pmic *p;
- int node, parent, ret;
- unsigned number_of_regs = pmic_number_of_regs(pmic_compat);
- const char *pmic_name, *comma;
- if (!number_of_regs) {
printf("%s: %s - not a supported PMIC\n",__func__, fdtdec_get_compatible(pmic_compat));return -1;- }
- node = fdtdec_next_compatible(blob, 0, pmic_compat);
- if (node < 0) {
debug("PMIC: Error %s. No node for %s in device tree\n",fdt_strerror(node), fdtdec_get_compatible(pmic_compat));return node;- }
- pmic_name = fdtdec_get_compatible(pmic_compat);
- comma = strchr(pmic_name, ',');
- if (comma)
pmic_name = comma + 1;- p = pmic_alloc();
- if (!p) {
printf("%s: POWER allocation error!\n", __func__);return -ENOMEM;- }
- parent = fdt_parent_offset(blob, node);
- if (parent < 0) {
debug("%s: Cannot find node parent\n", __func__);return -1;- }
- p->bus = i2c_get_bus_num_fdt(parent);
- if (p->bus < 0) {
debug("%s: Cannot find I2C bus\n", __func__);return -1;- }
- p->hw.i2c.addr = fdtdec_get_int(blob, node, "reg", 9);
- p->name = pmic_name;
- p->interface = PMIC_I2C;
- p->hw.i2c.tx_num = 1;
- p->number_of_regs = number_of_regs;
- p->compat_id = pmic_compat;
- ret = 0;
- while ((pmic_ops->reg_op != PMIC_REG_BAIL) && !ret) {
if (pmic_ops->reg_op == PMIC_REG_WRITE)ret = pmic_reg_write(p,pmic_ops->reg_addr,pmic_ops->reg_value);elseret = pmic_reg_update(p,pmic_ops->reg_addr,pmic_ops->reg_value);pmic_ops++;- }
- if (ret)
printf("%s: Failed accessing reg 0x%x of %s\n",__func__, pmic_ops[-1].reg_addr, p->name);pmic_ops[-1].reg_addr, is it right?
Yes, this is right because if you see the above while() loop we are incrementing the pmic_ops pointer after reg_write/reg_update and if any of them returns error value ,while() loop will break and the pmic_ops pointing to the next instance address. so to print the values in the previous instance pointer we are using pmic_ops[-1].
maybe your code will work correctly but, the point is pmic_ops[-1] is not always right. please think about it.
If I modify your code, then I'll return error immediately in while loop.
- else
printf("PMIC %s initialized\n", p->name);- return ret;
+} diff --git a/drivers/power/power_core.c b/drivers/power/power_core.c index a1c4fd0..f40be31 100644 --- a/drivers/power/power_core.c +++ b/drivers/power/power_core.c @@ -228,6 +228,20 @@ int pmic_reg_update(struct pmic *p, int reg, u32 val) return 0; }
+struct pmic *pmic_get_by_id(enum fdt_compat_id pmic_compat) +{
- struct pmic *p;
- list_for_each_entry(p, &pmic_list, list) {
if (p->compat_id == pmic_compat) {debug("%s: pmic %s -> 0x%p\n", __func__, p->name, p);return p;}- }
- return NULL;
+}
U_BOOT_CMD( pmic, CONFIG_SYS_MAXARGS, 1, do_pmic, "PMIC", diff --git a/include/power/pmic.h b/include/power/pmic.h index e0b9139..e0982e3 100644 --- a/include/power/pmic.h +++ b/include/power/pmic.h @@ -12,6 +12,7 @@ #include <linux/list.h> #include <i2c.h> #include <power/power_chrg.h> +#include <fdtdec.h>
enum { PMIC_I2C, PMIC_SPI, PMIC_NONE}; enum { I2C_PMIC, I2C_NUM, }; @@ -72,6 +73,7 @@ struct pmic {
struct pmic *parent; struct list_head list;
- enum fdt_compat_id compat_id;
};
int pmic_init(unsigned char bus); @@ -84,6 +86,38 @@ int pmic_reg_read(struct pmic *p, u32 reg, u32 *val); int pmic_reg_write(struct pmic *p, u32 reg, u32 val); int pmic_set_output(struct pmic *p, u32 reg, int ldo, int on); int pmic_reg_update(struct pmic *p, int reg, u32 val); +/*
- Find registered PMIC based on its compatibility ID.
- @param pmic_compat compatibility ID of the PMIC to search for.
- @return pointer to the relevant 'struct pmic' on success or NULL
- */
+struct pmic *pmic_get_by_id(enum fdt_compat_id pmic_compat);
+enum pmic_reg_op { PMIC_REG_BAIL, PMIC_REG_WRITE, PMIC_REG_UPDATE }; +struct pmic_init_ops {
- enum pmic_reg_op reg_op;
- u8 reg_addr;
- u8 reg_value;
u8? why? according to pmic.h, all of pmic operations are allowed u32.
Currently I don't have S2MPS11 data sheet with me, but I remember that it has 1 byte size registers. so the structure declared like that.
It means you want to support S2MPS11 only. right? Then why you make a common file?
Thanks, Minkyu Kang.
Hi,
On 1/16/14, Minkyu Kang mk7.kang@samsung.com wrote:
On 16/01/14 13:05, Leela Krishna Amudala wrote:
Hi Minkyu Kang,
Thanks for reviewing the patch, Please check my comments inline.
On 1/7/14, Minkyu Kang mk7.kang@samsung.com wrote:
On 06/01/14 20:49, Leela Krishna Amudala wrote:
Most of i2c PMIC drivers follow the same initialization sequence, let's generalize it in a common file.
The initialization function finds the PMIC in the device tree, and if found - registers it in the list of known PMICs and initializes it, iterating through the table of settings supplied by the caller.
Signed-off-by: Vadim Bendebury vbendeb@chromium.org Signed-off-by: Leela Krishna Amudala l.krishna@samsung.com Reviewed-by: Doug Anderson dianders@google.com Acked-by: Simon Glass sjg@chromium.org
board/samsung/common/board.c | 22 +++++++++ drivers/power/pmic/Makefile | 1 + drivers/power/pmic/pmic_common.c | 97 ++++++++++++++++++++++++++++++++++++++ drivers/power/power_core.c | 14 ++++++ include/power/pmic.h | 34 +++++++++++++ 5 files changed, 168 insertions(+) create mode 100644 drivers/power/pmic/pmic_common.c
+#include <common.h> +#include <fdtdec.h> +#include <errno.h> +#include <power/pmic.h> +#include <power/s2mps11_pmic.h> +#include <power/max77686_pmic.h>
Why common driver need specific pmic's header file? It is wrong architecture.
Here, in this file we are using PMIC compact ID to find the number of registers in a PMIC (now currently have support for only S2MPS11 and other PMICs info may added in future), so we need those headers.
So, it's a wrong architecture. It (find the number of registers) is a PMIC specific feature.
Okay, Then in that case I'll move this function to a new file or some suitable location and call it here or if you have any suggestions please tell me.
+DECLARE_GLOBAL_DATA_PTR;
+static unsigned pmic_number_of_regs(enum fdt_compat_id pmic_compat)
As I said, this function is not a common feature.
Okay, will move it to suitable location.
+{
- switch (pmic_compat) {
- case COMPAT_SAMSUNG_S2MPS11_PMIC:
return S2MPS11_NUM_OF_REGS;- default:
break;- }
- return 0;
+}
+int pmic_common_init(enum fdt_compat_id pmic_compat,
const struct pmic_init_ops *pmic_ops)+{
- const void *blob = gd->fdt_blob;
- struct pmic *p;
- int node, parent, ret;
- unsigned number_of_regs = pmic_number_of_regs(pmic_compat);
- const char *pmic_name, *comma;
- if (!number_of_regs) {
printf("%s: %s - not a supported PMIC\n",__func__, fdtdec_get_compatible(pmic_compat));return -1;- }
- node = fdtdec_next_compatible(blob, 0, pmic_compat);
- if (node < 0) {
debug("PMIC: Error %s. No node for %s in device tree\n",fdt_strerror(node), fdtdec_get_compatible(pmic_compat));return node;- }
- pmic_name = fdtdec_get_compatible(pmic_compat);
- comma = strchr(pmic_name, ',');
- if (comma)
pmic_name = comma + 1;- p = pmic_alloc();
- if (!p) {
printf("%s: POWER allocation error!\n", __func__);return -ENOMEM;- }
- parent = fdt_parent_offset(blob, node);
- if (parent < 0) {
debug("%s: Cannot find node parent\n", __func__);return -1;- }
- p->bus = i2c_get_bus_num_fdt(parent);
- if (p->bus < 0) {
debug("%s: Cannot find I2C bus\n", __func__);return -1;- }
- p->hw.i2c.addr = fdtdec_get_int(blob, node, "reg", 9);
- p->name = pmic_name;
- p->interface = PMIC_I2C;
- p->hw.i2c.tx_num = 1;
- p->number_of_regs = number_of_regs;
- p->compat_id = pmic_compat;
- ret = 0;
- while ((pmic_ops->reg_op != PMIC_REG_BAIL) && !ret) {
if (pmic_ops->reg_op == PMIC_REG_WRITE)ret = pmic_reg_write(p,pmic_ops->reg_addr,pmic_ops->reg_value);elseret = pmic_reg_update(p,pmic_ops->reg_addr,pmic_ops->reg_value);pmic_ops++;- }
- if (ret)
printf("%s: Failed accessing reg 0x%x of %s\n",__func__, pmic_ops[-1].reg_addr, p->name);pmic_ops[-1].reg_addr, is it right?
Yes, this is right because if you see the above while() loop we are incrementing the pmic_ops pointer after reg_write/reg_update and if any of them returns error value ,while() loop will break and the pmic_ops pointing to the next instance address. so to print the values in the previous instance pointer we are using pmic_ops[-1].
maybe your code will work correctly but, the point is pmic_ops[-1] is not always right. please think about it.
If I modify your code, then I'll return error immediately in while loop.
Okay, I'll change this.
- else
printf("PMIC %s initialized\n", p->name);- return ret;
+} diff --git a/drivers/power/power_core.c b/drivers/power/power_core.c index a1c4fd0..f40be31 100644 --- a/drivers/power/power_core.c +++ b/drivers/power/power_core.c @@ -228,6 +228,20 @@ int pmic_reg_update(struct pmic *p, int reg, u32 val) return 0; }
+struct pmic *pmic_get_by_id(enum fdt_compat_id pmic_compat) +{
- struct pmic *p;
- list_for_each_entry(p, &pmic_list, list) {
if (p->compat_id == pmic_compat) {debug("%s: pmic %s -> 0x%p\n", __func__, p->name, p);return p;}- }
- return NULL;
+}
U_BOOT_CMD( pmic, CONFIG_SYS_MAXARGS, 1, do_pmic, "PMIC", diff --git a/include/power/pmic.h b/include/power/pmic.h index e0b9139..e0982e3 100644 --- a/include/power/pmic.h +++ b/include/power/pmic.h @@ -12,6 +12,7 @@ #include <linux/list.h> #include <i2c.h> #include <power/power_chrg.h> +#include <fdtdec.h>
enum { PMIC_I2C, PMIC_SPI, PMIC_NONE}; enum { I2C_PMIC, I2C_NUM, }; @@ -72,6 +73,7 @@ struct pmic {
struct pmic *parent; struct list_head list;
- enum fdt_compat_id compat_id;
};
int pmic_init(unsigned char bus); @@ -84,6 +86,38 @@ int pmic_reg_read(struct pmic *p, u32 reg, u32 *val); int pmic_reg_write(struct pmic *p, u32 reg, u32 val); int pmic_set_output(struct pmic *p, u32 reg, int ldo, int on); int pmic_reg_update(struct pmic *p, int reg, u32 val); +/*
- Find registered PMIC based on its compatibility ID.
- @param pmic_compat compatibility ID of the PMIC to search for.
- @return pointer to the relevant 'struct pmic' on success or NULL
- */
+struct pmic *pmic_get_by_id(enum fdt_compat_id pmic_compat);
+enum pmic_reg_op { PMIC_REG_BAIL, PMIC_REG_WRITE, PMIC_REG_UPDATE }; +struct pmic_init_ops {
- enum pmic_reg_op reg_op;
- u8 reg_addr;
- u8 reg_value;
u8? why? according to pmic.h, all of pmic operations are allowed u32.
Currently I don't have S2MPS11 data sheet with me, but I remember that it has 1 byte size registers. so the structure declared like that.
It means you want to support S2MPS11 only. right? Then why you make a common file?
I'm trying to make it as a common stuff so started with S2MPS11.
Okay, I'll use u32 to make it compatible with other PMIC chips.
Thanks, Leela Krishna.
Thanks, Minkyu Kang. _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
configure S2MPS11 pmic on SMDK5420
Signed-off-by: Leela Krishna Amudala l.krishna@samsung.com Acked-by: Simon Glass sjg@chromium.org --- include/configs/smdk5420.h | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/include/configs/smdk5420.h b/include/configs/smdk5420.h index 447f8e5..46aeec0 100644 --- a/include/configs/smdk5420.h +++ b/include/configs/smdk5420.h @@ -53,4 +53,8 @@
#define CONFIG_MAX_I2C_NUM 11
+#define CONFIG_POWER +#define CONFIG_POWER_I2C +#define CONFIG_POWER_S2MPS11 + #endif /* __CONFIG_5420_H */
On 06/01/14 20:49, Leela Krishna Amudala wrote:
configure S2MPS11 pmic on SMDK5420
Signed-off-by: Leela Krishna Amudala l.krishna@samsung.com Acked-by: Simon Glass sjg@chromium.org
include/configs/smdk5420.h | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/include/configs/smdk5420.h b/include/configs/smdk5420.h index 447f8e5..46aeec0 100644 --- a/include/configs/smdk5420.h +++ b/include/configs/smdk5420.h @@ -53,4 +53,8 @@
#define CONFIG_MAX_I2C_NUM 11
+#define CONFIG_POWER +#define CONFIG_POWER_I2C
already defined at exynos5-dt.h
+#define CONFIG_POWER_S2MPS11
#endif /* __CONFIG_5420_H */
Thanks, Minkyu Kang.
Hi Minkyu Kang,
On 1/7/14, Minkyu Kang mk7.kang@samsung.com wrote:
On 06/01/14 20:49, Leela Krishna Amudala wrote:
configure S2MPS11 pmic on SMDK5420
Signed-off-by: Leela Krishna Amudala l.krishna@samsung.com Acked-by: Simon Glass sjg@chromium.org
include/configs/smdk5420.h | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/include/configs/smdk5420.h b/include/configs/smdk5420.h index 447f8e5..46aeec0 100644 --- a/include/configs/smdk5420.h +++ b/include/configs/smdk5420.h @@ -53,4 +53,8 @@
#define CONFIG_MAX_I2C_NUM 11
+#define CONFIG_POWER +#define CONFIG_POWER_I2C
already defined at exynos5-dt.h
Okay, will remove it.
+#define CONFIG_POWER_S2MPS11
#endif /* __CONFIG_5420_H */
Thanks, Minkyu Kang. _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
participants (2)
-
Leela Krishna Amudala -
Minkyu Kang