[U-Boot] [PATCH 02/10] power: Add support for TPS65090 PMU chip.

[Lukasz Majewski <l.majewski@samsung.com>]

Hi Simon,

> From: Tom Wai-Hong Tam <waihong@chromium.org>
> 
> This adds driver support for the TPS65090 PMU. Support includes
> hooking into the pmic infrastructure  so that the pmic commands
> can be used on the console. The TPS65090 supports the following
> functionality:
> 
> - fet enable/disable/querying
> - getting and setting of charge state
> 
> Even though it is connected to the pmic infrastructure it does
> not hook into the pmic charging charging infrastructure.

Can I ask what was the problem with adding support for "pmic bat
[state|charge]" command on this PMIC?

Was the framework unfriendly or there was no need to do that? 

> 
> Signed-off-by: Tom Wai-Hong Tam <waihong@chromium.org>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> Signed-off-by: Hatim Ali <hatim.rv@samsung.com>
> Signed-off-by: Katie Roberts-Hoffman <katierh@chromium.org>
> Signed-off-by: Rong Chang <rongchang@chromium.org>
> Signed-off-by: Sean Paul <seanpaul@chromium.org>
> Signed-off-by: Vincent Palatin <vpalatin@chromium.org>
> Signed-off-by: Aaron Durbin <adurbin@chromium.org>
> ---
> 
>  doc/device-tree-bindings/power/tps65090.txt |  21 ++
>  drivers/power/pmic/Makefile                 |   1 +
>  drivers/power/pmic/pmic_tps65090.c          | 296
> ++++++++++++++++++++++++++++
> include/fdtdec.h                            |   1 +
> include/power/tps65090_pmic.h               |  83 ++++++++
> lib/fdtdec.c                                |   1 + 6 files changed,
> 403 insertions(+) create mode 100644
> doc/device-tree-bindings/power/tps65090.txt create mode 100644
> drivers/power/pmic/pmic_tps65090.c create mode 100644
> include/power/tps65090_pmic.h
> 
> diff --git a/doc/device-tree-bindings/power/tps65090.txt
> b/doc/device-tree-bindings/power/tps65090.txt new file mode 100644
> index 0000000..6a8a884
> --- /dev/null
> +++ b/doc/device-tree-bindings/power/tps65090.txt
> @@ -0,0 +1,21 @@
> +TPSchrome binding
> +=================
> +
> +The device tree node which describes the operation of the Texas
> Instrument +TPS65090 power regulator chip is as follows:
> +
> +Required properties :
> +- compatible : "ti,tps65090"
> +- reg : slave address on the i2c bus
> +
> +The tps65090 node should appear as a subnode of the i2c bus that
> connects it. +
> +Example
> +=======
> +
> +       i2c at 12ca0000 {
> +               power-regulator at 48 {
> +                       compatible = "ti,tps65090"
> +                       reg = <0x48>;
> +               };
> +       };

Those bindings look very different from the one at Linux kernel for
this device. Therefore I assume that there was justification to not
stick to the linux kernel DT format.

> diff --git a/drivers/power/pmic/Makefile b/drivers/power/pmic/Makefile
> index 0b45ffa..7ed55e6 100644
> --- a/drivers/power/pmic/Makefile
> +++ b/drivers/power/pmic/Makefile
> @@ -9,5 +9,6 @@ obj-$(CONFIG_POWER_MAX8998) += pmic_max8998.o
>  obj-$(CONFIG_POWER_MAX8997) += pmic_max8997.o
>  obj-$(CONFIG_POWER_MUIC_MAX8997) += muic_max8997.o
>  obj-$(CONFIG_POWER_MAX77686) += pmic_max77686.o
> +obj-$(CONFIG_POWER_TPS65090) += pmic_tps65090.o
>  obj-$(CONFIG_POWER_TPS65217) += pmic_tps65217.o
>  obj-$(CONFIG_POWER_TPS65910) += pmic_tps65910.o
> diff --git a/drivers/power/pmic/pmic_tps65090.c
> b/drivers/power/pmic/pmic_tps65090.c new file mode 100644
> index 0000000..ef9f911
> --- /dev/null
> +++ b/drivers/power/pmic/pmic_tps65090.c
> @@ -0,0 +1,296 @@
> +/*
> + * Copyright (c) 2012 The Chromium OS Authors. All rights reserved.
> + * Use of this source code is governed by a BSD-style license that
> can be
> + * found in the LICENSE file.
> + *
> + * Alternatively, this software may be distributed under the terms
> of the
> + * GNU General Public License ("GPL") version 2 as published by the
> Free
> + * Software Foundation.

Would it be possible to tune this license description to be similar to 
SPDX-License-Identifier:	GPL-2.0+|BSD

> + */
> +
> +#include <common.h>
> +#include <errno.h>
> +#include <fdtdec.h>
> +#include <i2c.h>
> +#include <power/pmic.h>
> +#include <power/tps65090_pmic.h>
> +
> +DECLARE_GLOBAL_DATA_PTR;
> +
> +#define TPS65090_NAME "TPS65090_PMIC"
> +
> +/* TPS65090 register addresses */
> +enum {
> +       REG_CG_CTRL0 = 4,
> +       REG_CG_STATUS1 = 0xa,
> +       REG_FET1_CTRL = 0x0f,
> +       REG_FET2_CTRL,
> +       REG_FET3_CTRL,
> +       REG_FET4_CTRL,
> +       REG_FET5_CTRL,
> +       REG_FET6_CTRL,
> +       REG_FET7_CTRL,
> +       TPS65090_NUM_REGS,
> +};
> +
> +enum {
> +       CG_CTRL0_ENC_MASK       = 0x01,
> +
> +       MAX_FET_NUM     = 7,
> +       MAX_CTRL_READ_TRIES = 5,
> +
> +       /* TPS65090 FET_CTRL register values */
> +       FET_CTRL_TOFET          = 1 << 7,  /* Timeout, startup,
> overload */
> +       FET_CTRL_PGFET          = 1 << 4,  /* Power good for FET
> status */
> +       FET_CTRL_WAIT           = 3 << 2,  /* Overcurrent timeout max
> */
> +       FET_CTRL_ADENFET        = 1 << 1,  /* Enable output auto
> discharge */
> +       FET_CTRL_ENFET          = 1 << 0,  /* Enable FET */
> +};
> +
> +/**
> + * Checks for a valid FET number
> + *
> + * @param fet_id       FET number to check
> + * @return 0 if ok, -1 if FET value is out of range
> + */
> +static int tps65090_check_fet(unsigned int fet_id)
> +{
> +       if (fet_id == 0 || fet_id > MAX_FET_NUM) {
> +               debug("parameter fet_id is out of range, %u not in 1
> ~ %u\n",
> +                     fet_id, MAX_FET_NUM);
> +               return -1;

In the recent code (like trats/trats2, bugs fixed at existing power
infrastructure) I'm encouraging people to use error descriptions from
<errno.h>, not the -1/-2 values.

Can you fix this globally in this patch?

> +       }
> +
> +       return 0;
> +}
> +
> +/**
> + * Set the power state for a FET
> + *
> + * @param pmic         pmic structure for the tps65090
> + * @param fet_id       Fet number to set (1..MAX_FET_NUM)
> + * @param set          1 to power on FET, 0 to power off
> + * @return FET_ERR_COMMS if we got a comms error, FET_ERR_NOT_READY
> if the
> + * FET failed to change state. If all is ok, returns 0.
> + */
> +static int tps65090_fet_set(struct pmic *pmic, int fet_id, int set)
							      ^^^^
							      maybe
							      bool
							     	
> +{
> +       int retry;
> +       u32 reg, value;
> +
> +       value = FET_CTRL_ADENFET | FET_CTRL_WAIT;
> +       if (set)
> +               value |= FET_CTRL_ENFET;
> +
> +       if (pmic_reg_write(pmic, REG_FET1_CTRL + fet_id - 1, value))
> +               return FET_ERR_COMMS;
> +       /* Try reading until we get a result */
> +       for (retry = 0; retry < MAX_CTRL_READ_TRIES; retry++) {
> +               if (pmic_reg_read(pmic, REG_FET1_CTRL + fet_id - 1,
> &reg))
> +                       return FET_ERR_COMMS;
> +
> +               /* Check that the fet went into the expected state */
> +               if (!!(reg & FET_CTRL_PGFET) == set)
> +                       return 0;
> +
> +               /* If we got a timeout, there is no point in waiting
> longer */
> +               if (reg & FET_CTRL_TOFET)
> +                       break;
> +
> +               mdelay(1);
> +       }
> +
> +       debug("FET %d: Power good should have set to %d but
> reg=%#02x\n",
> +             fet_id, set, reg);
> +       return FET_ERR_NOT_READY;
> +}
> +
> +int tps65090_fet_enable(unsigned int fet_id)
> +{
> +       int loops;
> +       ulong start;
> +       struct pmic *pmic;
> +       int ret = 0;
> +
> +       if (tps65090_check_fet(fet_id))
> +               return -1;

		As I've described above - maybe -ENODEV?

> +
> +       pmic = pmic_get(TPS65090_NAME);
> +       if (pmic == NULL)
	     It seems like in the code I tend to use:
		if (!pmic)

> +               return -1;
> +
> +       start = get_timer(0);
> +       for (loops = 0;; loops++) {
> +               ret = tps65090_fet_set(pmic, fet_id, 1);
> +               if (!ret)
> +                       break;
> +
> +               if (get_timer(start) > 100)
> +                       break;
> +
> +               /* Turn it off and try again until we time out */
> +               tps65090_fet_set(pmic, fet_id, 0);
> +       }
> +
> +       if (ret) {
> +               debug("%s: FET%d failed to power on: time=%lums,
> loops=%d\n",
> +                     __func__, fet_id, get_timer(start), loops);
> +       } else if (loops) {
> +               debug("%s: FET%d powered on after %lums, loops=%d\n",
> +                     __func__, fet_id, get_timer(start), loops);
> +       }

Here the parenthesis can be omitted.

> +       /*
> +        * Unfortunately, there are some conditions where the power
> +        * good bit will be 0, but the fet still comes up. One such
> +        * case occurs with the lcd backlight. We'll just return 0
> here
> +        * and assume that the fet will eventually come up.
> +        */
> +       if (ret == FET_ERR_NOT_READY)
> +               ret = 0;
> +
> +       return ret;
> +}
> +
> +int tps65090_fet_disable(unsigned int fet_id)
> +{
> +       int ret;
> +       struct pmic *pmic;
> +
> +       if (tps65090_check_fet(fet_id))
> +               return -1;
> +
> +       pmic = pmic_get(TPS65090_NAME);
> +       if (pmic == NULL)
> +               return -1;
> +       ret = tps65090_fet_set(pmic, fet_id, 0);
> +
> +       return ret;
> +}
> +
> +int tps65090_fet_is_enabled(unsigned int fet_id)
> +{
> +       u32 reg;
> +       int ret;
> +       struct pmic *pmic;
> +
> +       if (tps65090_check_fet(fet_id))
> +               return -1;
> +       pmic = pmic_get(TPS65090_NAME);
> +       if (pmic == NULL)
> +               return -1;
> +       ret = pmic_reg_read(pmic, REG_FET1_CTRL + fet_id - 1, &reg);
> +       if (ret) {
> +               debug("fail to read FET%u_CTRL register over I2C",
> fet_id);
> +               return -2;
> +       }
> +
> +       return reg & FET_CTRL_ENFET;
> +}
> +
> +int tps65090_get_charging(void)
> +{
> +       u32 val;
> +       int ret;
> +       struct pmic *pmic;
> +
> +       pmic = pmic_get(TPS65090_NAME);
> +       if (pmic == NULL)
> +               return -1;
> +       ret = pmic_reg_read(pmic, REG_CG_CTRL0, &val);
> +       if (ret)
> +               return ret;
> +       return val & CG_CTRL0_ENC_MASK ? 1 : 0;
> +}
> +
> +int tps65090_set_charge_enable(int enable)

This can be easily added to be used at "pmic bat charge" command.

Please look into the ./drivers/power/mfd/pmic_max77693.c

> +{
> +       u32 val;
> +       int ret;
> +       struct pmic *pmic;
> +
> +       pmic = pmic_get(TPS65090_NAME);
> +       if (pmic == NULL)
> +               return -1;
> +
> +       ret = pmic_reg_read(pmic, REG_CG_CTRL0, &val);
> +       if (!ret) {
> +               if (enable)
> +                       val |= CG_CTRL0_ENC_MASK;
> +               else
> +                       val &= ~CG_CTRL0_ENC_MASK;
> +               ret = pmic_reg_write(pmic, REG_CG_CTRL0, val);
> +       }
> +       if (ret) {
> +               debug("%s: Failed to read/write register\n",
> __func__);
> +               return ret;
> +       }
> +       return 0;
> +}
> +
> +int tps65090_get_status(void)
> +{
> +       u32 val;
> +       int ret;
> +       struct pmic *pmic;
> +
> +       pmic = pmic_get(TPS65090_NAME);
> +
> +       if (pmic == NULL)
> +               return -1;
> +       ret = pmic_reg_read(pmic, REG_CG_STATUS1, &val);
> +       if (ret)
> +               return ret;
> +       return val;
> +}
> +
> +int tps65090_init(void)
> +{
> +       int bus;
> +       int addr;
> +       struct pmic *p;
> +
> +#ifdef CONFIG_OF_CONTROL
> +       const void *blob = gd->fdt_blob;
> +       int node, parent;
> +
> +       node = fdtdec_next_compatible(blob, 0, COMPAT_TI_TPS65090);
> +       if (node < 0) {
> +               debug("PMIC: No node for PMIC Chip in device tree\n");
> +               debug("node = %d\n", node);
> +               return -ENODEV;
> +       }
> +
> +       parent = fdt_parent_offset(blob, node);
> +       if (parent < 0) {
> +               debug("%s: Cannot find node parent\n", __func__);
> +               return -1;
> +       }
> +
> +       bus = i2c_get_bus_num_fdt(parent);
> +       if (p->bus < 0) {
> +               debug("%s: Cannot find I2C bus\n", __func__);
> +               return -1;
> +       }
> +       addr = fdtdec_get_int(blob, node, "reg", TPS65090_I2C_ADDR);
> +#else
> +       bus = CONFIG_POWER_TPS65090_BUS;
> +       addr = TPS65090_I2C_ADDR;
> +#endif
> +       p = pmic_alloc();
> +
> +       if (!p) {
> +               printf("%s: POWER allocation error!\n", __func__);
> +               return -ENOMEM;
> +       }
> +
> +       p->name = TPS65090_NAME;
> +       p->bus = bus;
> +       p->interface = PMIC_I2C;
> +       p->number_of_regs = TPS65090_NUM_REGS;
> +       p->hw.i2c.addr = addr;
> +       p->hw.i2c.tx_num = 1;
> +
> +       puts("TPS65090 PMIC init\n");
> +
> +       return 0;
> +}
> diff --git a/include/fdtdec.h b/include/fdtdec.h
> index 6e859ce..8f17ed3 100644
> --- a/include/fdtdec.h
> +++ b/include/fdtdec.h
> @@ -90,6 +90,7 @@ enum fdt_compat_id {
>         COMPAT_SAMSUNG_EXYNOS5_I2C,     /* Exynos5 High Speed I2C
> Controller */ COMPAT_SANDBOX_HOST_EMULATION,  /* Sandbox emulation of
> a function */ COMPAT_SANDBOX_LCD_SDL,         /* Sandbox LCD
> emulation with SDL */
> +       COMPAT_TI_TPS65090,             /* Texas Instrument TPS65090
> */
> 
>         COMPAT_COUNT,
>  };
> diff --git a/include/power/tps65090_pmic.h
> b/include/power/tps65090_pmic.h new file mode 100644
> index 0000000..fde30eb
> --- /dev/null
> +++ b/include/power/tps65090_pmic.h
> @@ -0,0 +1,83 @@
> +/*
> + * Copyright (c) 2012 The Chromium OS Authors. All rights reserved.
> + * Use of this source code is governed by a BSD-style license that
> can be
> + * found in the LICENSE file.
> + *
> + * Alternatively, this software may be distributed under the terms
> of the
> + * GNU General Public License ("GPL") version 2 as published by the
> Free
> + * Software Foundation.
> + */
> +
> +#ifndef __TPS65090_PMIC_H_
> +#define __TPS65090_PMIC_H_
> +
> +/* I2C device address for TPS65090 PMU */
> +#define TPS65090_I2C_ADDR      0x48
> +
> +enum {
> +       /* Status register fields */
> +       TPS65090_ST1_OTC        = 1 << 0,
> +       TPS65090_ST1_OCC        = 1 << 1,
> +       TPS65090_ST1_STATE_SHIFT = 4,
> +       TPS65090_ST1_STATE_MASK = 0xf << TPS65090_ST1_STATE_SHIFT,
> +};
> +
> +/* FET errors */
> +enum {
> +       FET_ERR_COMMS           = -1,   /* FET comms error */
> +       FET_ERR_NOT_READY       = -2,   /* FET is not yet ready -
> retry */ +};
> +
> +/**
> + * Enable FET
> + *
> + * @param      fet_id  FET ID, value between 1 and 7
> + * @return     0 on success, non-0 on failure
> + */
> +int tps65090_fet_enable(unsigned int fet_id);
> +
> +/**
> + * Disable FET
> + *
> + * @param      fet_id  FET ID, value between 1 and 7
> + * @return     0 on success, non-0 on failure
> + */
> +int tps65090_fet_disable(unsigned int fet_id);
> +
> +/**
> + * Is FET enabled?
> + *
> + * @param      fet_id  FET ID, value between 1 and 7
> + * @return     1 enabled, 0 disabled, negative value on failure
> + */
> +int tps65090_fet_is_enabled(unsigned int fet_id);
> +
> +/**
> + * Enable / disable the battery charger
> + *
> + * @param enable       0 to disable charging, non-zero to enable
> + */
> +int tps65090_set_charge_enable(int enable);
> +
> +/**
> + * Check whether we have enabled battery charging
> + *
> + * @return 1 if enabled, 0 if disabled
> + */
> +int tps65090_get_charging(void);
> +
> +/**
> + * Return the value of the status register
> + *
> + * @return status register value, or -1 on error
> + */
> +int tps65090_get_status(void);
> +
> +/**
> + * Initialize the TPS65090 PMU.
> + *
> + * @return     0 on success, non-0 on failure
> + */
> +int tps65090_init(void);
> +
> +#endif /* __TPS65090_PMIC_H_ */
> diff --git a/lib/fdtdec.c b/lib/fdtdec.c
> index f65ab4f..beee1d0 100644
> --- a/lib/fdtdec.c
> +++ b/lib/fdtdec.c
> @@ -63,6 +63,7 @@ static const char * const
> compat_names[COMPAT_COUNT] = { COMPAT(SAMSUNG_EXYNOS5_I2C,
> "samsung,exynos5-hsi2c"), COMPAT(SANDBOX_HOST_EMULATION,
> "sandbox,host-emulation"), COMPAT(SANDBOX_LCD_SDL, "sandbox,lcd-sdl"),
> +       COMPAT(TI_TPS65090, "ti,tps65090"),
>  };
> 
>  const char *fdtdec_get_compatible(enum fdt_compat_id id)
> --
> 1.9.1.423.g4596e3a



-- 
Best regards,

Lukasz Majewski

Samsung R&D Institute Poland (SRPOL) | Linux Platform Group