| Message ID | 1389008957-21493-6-git-send-email-l.krishna@samsung.com |
|---|---|
| State | Changes Requested |
| Delegated to: | Minkyu Kang |
| Headers | show |
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); > + 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); 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); >> + 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); > > 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); >>> + 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); >> >> 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); >>>> + 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); >>> >>> 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 >
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)