| Message ID | 1395856590-21917-3-git-send-email-sjg@chromium.org |
|---|---|
| State | Changes Requested |
| Delegated to: | Minkyu Kang |
| Headers | show |
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@12ca0000 { > + power-regulator@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, > ®)) > + 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, ®); > + 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
Hi Lukasz, On 27 March 2014 11:59, Lukasz Majewski <l.majewski@samsung.com> wrote: > 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? > It's not great. I spent a bit of time trying again. It think the issues are: - no device tree support - no comments in the pmic.h file so it's hard do know what everything is for - the battery charging command should really be common, not specific to each driver I did actually create a battery system in the Chromium source tree a while back, but never sent it upstream, partly because the pmic stuff was there and I was not sure how to get it into that framework. I think maybe if someone can comment the pmic.h file then I could have another try. But it would be a separate series to this one, which is focussed on the LCD, not the battery. > > > > > 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@12ca0000 { > > + power-regulator@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. > Yes they pre-date the kernel. But now that the kernel has support I will pull those in. For the parts we use it is the same. > > > 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 > Yes, will do. > > > + */ > > + > > +#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? > Yes good idea. > > > + } > > + > > + 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, > > ®)) > > + 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) > OK > > > + 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. > OK > > > + /* > > + * 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, ®); > > + 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 > Not easily. As mentioned above this is quite a bit of work. For a later series I think. Regards, Simon
Hi Simon, > Hi Lukasz, > > On 27 March 2014 11:59, Lukasz Majewski <l.majewski@samsung.com> > wrote: 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? > > It's not great. I spent a bit of time trying again. It think the > issues are: > > - no device tree support > - no comments in the pmic.h file so it's hard do know what everything > is for You are right here - the lack of DT support is the main problem here. As Tom Rini pointed out - it is problematic in this framework to support more than one instance of the same PMIC device. I must confess that I've overlooked this use case. > - the battery charging command should really be common, not specific > to each driver I agree. We can try to discuss one solution suitable for Exynos4 and 5. > > I did actually create a battery system in the Chromium source tree a > while back, but never sent it upstream, partly because the pmic stuff > was there and I was not sure how to get it into that framework. > > I think maybe if someone can comment the pmic.h file then I could > have another try. But it would be a separate series to this one, > which is focussed on the LCD, not the battery. For a quick reference please consider Trats and Trats2. If you need any more help, then please write an e-mail to me. > > > > > 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@12ca0000 { > > + power-regulator@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. > > Yes they pre-date the kernel. But now that the kernel has support I > will pull those in. For the parts we use it is the same. That would be great, thanks. > > > 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 > > Yes, will do. > > > > + */ > > + > > +#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? > > Yes good idea. > > > > + } > > + > > + 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, > > ®)) > > + 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) > > OK > > > + 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. > > OK > > > + /* > > + * 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, ®); > > + 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 > > Not easily. As mentioned above this is quite a bit of work. For a > later series I think. I'm looking forward for questions :-). > Regards, > Simon >
Hi Lukasz, On 31 March 2014 08:33, Lukasz Majewski <l.majewski@samsung.com> wrote: [snip] > > > > This can be easily added to be used at "pmic bat charge" command. > > > > Please look into the ./drivers/power/mfd/pmic_max77693.c > > > > Not easily. As mentioned above this is quite a bit of work. For a > > later series I think. > > I'm looking forward for questions :-). > Do you think you might submit a patch that adds comments to the header files? That would help a lot. Also now that driver model is merged, do you think we should try to move PMICs to that, or would it be a later step? Regards, Simon
Hi Simon, On Mon, 31 Mar 2014 11:27:11 -0600 Simon Glass <sjg@chromium.org> wrote: > Hi Lukasz, > > On 31 March 2014 08:33, Lukasz Majewski <l.majewski@samsung.com> > wrote: [snip] > > > > > > > > This can be easily added to be used at "pmic bat charge" command. > > > > > > Please look into the ./drivers/power/mfd/pmic_max77693.c > > > > > > Not easily. As mentioned above this is quite a bit of work. For a > > > later series I think. > > > > I'm looking forward for questions :-). > > > > Do you think you might submit a patch that adds comments to the header > files? That would help a lot. I will do my best. Up till then would you consider looking at the following link: http://u-boot.10912.n7.nabble.com/PATCH-v7-00-26-pmic-Redesign-PMIC-framework-to-support-multiple-instances-of-devices-tt140662.html#none It should shed some light on the PMIC design. > > Also now that driver model is merged, do you think we should try to > move PMICs to that, or would it be a later step? PMIC should be integrated with the device model. Also it shall support DT. When it will happen depends on the effort needed. Help more than welcome. > > Regards, > Simon Best regards, Lukasz Majewski
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@12ca0000 { + power-regulator@48 { + compatible = "ti,tps65090" + reg = <0x48>; + }; + }; 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. + */ + +#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; + } + + 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) +{ + 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, ®)) + 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; + + pmic = pmic_get(TPS65090_NAME); + if (pmic == NULL) + 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); + } + /* + * 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, ®); + 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) +{ + 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)