[U-Boot] [PATCH v2 04/12] power: Add support for TPS65090 PMU chip.
Simon Glass
sjg at chromium.org
Tue May 20 13:46:49 CEST 2014
Hi Minkyu,
On 15 May 2014 00:52, Minkyu Kang <mk7.kang at samsung.com> wrote:
> On 03/04/14 08:24, Simon Glass wrote:
>> From: Tom Wai-Hong Tam <waihong at 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.
>>
>> The device tree binding is from Linux, but only a small subset of
>> functionality is supported.
>>
>> Signed-off-by: Tom Wai-Hong Tam <waihong at chromium.org>
>> Signed-off-by: Hatim Ali <hatim.rv at samsung.com>
>> Signed-off-by: Katie Roberts-Hoffman <katierh at chromium.org>
>> Signed-off-by: Rong Chang <rongchang at chromium.org>
>> Signed-off-by: Sean Paul <seanpaul at chromium.org>
>> Signed-off-by: Vincent Palatin <vpalatin at chromium.org>
>> Signed-off-by: Aaron Durbin <adurbin at chromium.org>
>> Signed-off-by: Simon Glass <sjg at chromium.org>
>> ---
>>
>> Changes in v2:
>> - Removed non-device-tree operation
>> - Use errno.h for error return codes everywhere
>> - Plumb into pmic charging framework
>> - Fix style nit
>> - Use correct license header
>> - Use new Linux device tree bindings
>> - Add partial battery charging support
>>
>> doc/device-tree-bindings/power/tps65090.txt | 17 ++
>> doc/device-tree-bindings/regulator/tps65090.txt | 122 +++++++++
>> drivers/power/pmic/Makefile | 1 +
>> drivers/power/pmic/pmic_tps65090.c | 312 ++++++++++++++++++++++++
>> include/fdtdec.h | 1 +
>> include/power/tps65090_pmic.h | 73 ++++++
>> lib/fdtdec.c | 1 +
>> 7 files changed, 527 insertions(+)
>> create mode 100644 doc/device-tree-bindings/power/tps65090.txt
>> create mode 100644 doc/device-tree-bindings/regulator/tps65090.txt
>> create mode 100644 drivers/power/pmic/pmic_tps65090.c
>> create mode 100644 include/power/tps65090_pmic.h
>>
[snip]
>> diff --git a/drivers/power/pmic/pmic_tps65090.c b/drivers/power/pmic/pmic_tps65090.c
>> new file mode 100644
>> index 0000000..8558b43
>> --- /dev/null
>> +++ b/drivers/power/pmic/pmic_tps65090.c
>> @@ -0,0 +1,312 @@
>> +/*
>> + * Copyright (c) 2012 The Chromium OS Authors.
>> + *
>> + * SPDX-License-Identifier: GPL-2.0+
>> + */
>> +
>> +#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_IRQ1 = 0,
>> + 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 {
>> + IRQ1_VBATG = 1 << 3,
>> + 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 */
>> +};
>
> Are there any special reason to use enum?
Yes it provides symbols for debugging and avoids needing brackets for
expressions.
>
>> +
>> +/**
>> + * Checks for a valid FET number
>> + *
>> + * @param fet_id FET number to check
>> + * @return 0 if ok, -EINVAL 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 -EINVAL;
>> + }
>> +
>> + 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 -EIO if we got a comms error, -EAGAIN if the FET failed to
>> + * change state. If all is ok, returns 0.
>> + */
>> +static int tps65090_fet_set(struct pmic *pmic, int fet_id, bool set)
>> +{
>> + 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 -EIO;
>> +
>> + /* 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, ®))
>> + return -EIO;
>> +
>> + /* 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 -EAGAIN;
>
> EAGAIN? why?
This means that we managed to request that the FET change state, but
could not confirm that it did so. It is therefore worth trying again.
>
>> +}
>> +
>> +int tps65090_fet_enable(unsigned int fet_id)
>> +{
>> + struct pmic *pmic;
>> + ulong start;
>> + int loops;
>> + int ret;
>> +
>> + ret = tps65090_check_fet(fet_id);
>> + if (ret)
>> + return ret;
>> +
>> + pmic = pmic_get(TPS65090_NAME);
>> + if (!pmic)
>> + return -EACCES;
>> +
>> + start = get_timer(0);
>> + for (loops = 0;; loops++) {
>> + ret = tps65090_fet_set(pmic, fet_id, true);
>> + 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, false);
>> + }
>> +
>> + 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);
>> +
>> + /*
>> + * 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.
>> + */
>
> I can't understand it.
> If it is a special case, then it should be controlled at each caller.
This is a problem with the chip itself so we wanted to localise the
changes here, as is done in the kernel. It may happen with any
application that uses this chip - the LCD backlight is just an example
as it happens on snow.
>
>> + if (ret == -EAGAIN)
>> + ret = 0;
>
> It looks weird.
This is the required logic to get this chip to work properly - see the
comment above.
>
>> +
>> + return ret;
>> +}
>> +
>> +int tps65090_fet_disable(unsigned int fet_id)
>> +{
>> + struct pmic *pmic;
>> + int ret;
>> +
>> + ret = tps65090_check_fet(fet_id);
>> + if (ret)
>> + return ret;
>> +
>> + pmic = pmic_get(TPS65090_NAME);
>> + if (!pmic)
>> + return -EACCES;
>> + ret = tps65090_fet_set(pmic, fet_id, false);
>> +
>> + return ret;
>> +}
>> +
>> +int tps65090_fet_is_enabled(unsigned int fet_id)
>> +{
>> + struct pmic *pmic;
>> + u32 reg;
>> + int ret;
>> +
>> + ret = tps65090_check_fet(fet_id);
>> + if (ret)
>> + return ret;
>> +
>> + pmic = pmic_get(TPS65090_NAME);
>> + if (!pmic)
>> + return -ENODEV;
>> + ret = pmic_reg_read(pmic, REG_FET1_CTRL + fet_id - 1, ®);
>> + if (ret) {
>> + debug("fail to read FET%u_CTRL register over I2C", fet_id);
>> + return -EIO;
>> + }
>> +
>> + return reg & FET_CTRL_ENFET;
>> +}
>> +
>> +int tps65090_get_charging(void)
>> +{
>> + struct pmic *pmic;
>> + u32 val;
>> + int ret;
>> +
>> + pmic = pmic_get(TPS65090_NAME);
>> + if (!pmic)
>> + return -EACCES;
>> +
>> + ret = pmic_reg_read(pmic, REG_CG_CTRL0, &val);
>> + if (ret)
>> + return ret;
>> +
>> + return val & CG_CTRL0_ENC_MASK ? 1 : 0;
>
> How about it?
>
> return !!(val & CG_CTRL0_ENC_MASK);
done
>
>> +}
>> +
>> +static int tps65090_charger_state(struct pmic *pmic, int state,
>> + int current)
>> +{
>> + u32 val;
>> + int ret;
>> +
>> + ret = pmic_reg_read(pmic, REG_CG_CTRL0, &val);
>> + if (!ret) {
>> + if (state == PMIC_CHARGER_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)
>> +{
>> + struct pmic *pmic;
>> + u32 val;
>> + int ret;
>> +
>> + pmic = pmic_get(TPS65090_NAME);
>> + if (!pmic)
>> + return -EACCES;
>> +
>> + ret = pmic_reg_read(pmic, REG_CG_STATUS1, &val);
>> + if (ret)
>> + return ret;
>> +
>> + return val;
>> +}
>> +
>> +static int tps65090_charger_bat_present(struct pmic *pmic)
>> +{
>> + u32 val;
>> + int ret;
>> +
>> + ret = pmic_reg_read(pmic, REG_IRQ1, &val);
>> + if (ret)
>> + return ret;
>> +
>> + return val & IRQ1_VBATG ? true : false;
>
> return !!(val & IRQ1_VBATG);
>> +}
>> +
>> +
>
> please remove extra blank line.
done
>
>> +static struct power_chrg power_chrg_pmic_ops = {
>> + .chrg_bat_present = tps65090_charger_bat_present,
>> + .chrg_state = tps65090_charger_state,
>> +};
>> +
>> +int tps65090_init(void)
>> +{
>> + struct pmic *p;
>> + int bus;
>> + int addr;
>> +
>
> please remove blank line.
done
>
>> + 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 -EINVAL;
>> + }
>> +
>> + bus = i2c_get_bus_num_fdt(parent);
>> + if (p->bus < 0) {
>> + debug("%s: Cannot find I2C bus\n", __func__);
>> + return -ENOENT;
>> + }
>> + addr = fdtdec_get_int(blob, node, "reg", TPS65090_I2C_ADDR);
>> + 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;
>> + p->chrg = &power_chrg_pmic_ops;
>> +
>> + puts("TPS65090 PMIC init\n");
>> +
>> + return 0;
>> +}
>> diff --git a/include/fdtdec.h b/include/fdtdec.h
>> index 3196cf6..669ab90 100644
>> --- a/include/fdtdec.h
>> +++ b/include/fdtdec.h
>> @@ -92,6 +92,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..dcf99c9
>> --- /dev/null
>> +++ b/include/power/tps65090_pmic.h
>> @@ -0,0 +1,73 @@
>> +/*
>> + * Copyright (c) 2012 The Chromium OS Authors.
>> + *
>> + * SPDX-License-Identifier: GPL-2.0+
>> + */
>> +
>> +#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,
>> +};
>
> Are there any special reason to use enum?
See comment above.
>
>> +
>> +/**
>> + * 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 33265ec..e15e0ad 100644
>> --- a/lib/fdtdec.c
>> +++ b/lib/fdtdec.c
>> @@ -65,6 +65,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)
>>
>
Regards,
Simon
More information about the U-Boot
mailing list