[U-Boot] [PATCH V4 5/6] exynos: Add a common DT based PMIC driver initialization

[Minkyu Kang <mk7.kang@samsung.com>]

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.