Hi Minkyu, On 15 May 2014 00:52, Minkyu Kang <mk7.k...@samsung.com> wrote: > On 03/04/14 08:24, Simon Glass wrote: >> From: Tom Wai-Hong Tam <waih...@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 <waih...@chromium.org> >> Signed-off-by: Hatim Ali <hatim...@samsung.com> >> Signed-off-by: Katie Roberts-Hoffman <kati...@chromium.org> >> Signed-off-by: Rong Chang <rongch...@chromium.org> >> Signed-off-by: Sean Paul <seanp...@chromium.org> >> Signed-off-by: Vincent Palatin <vpala...@chromium.org> >> Signed-off-by: Aaron Durbin <adur...@chromium.org> >> Signed-off-by: Simon Glass <s...@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 _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot