[U-Boot] [PATCH 3/6] drivers/power/pmic: Add tps65910 driver
Dan Murphy
dmurphy at ti.com
Thu Jul 25 19:41:46 CEST 2013
On 07/19/2013 02:00 PM, Tom Rini wrote:
> From: "Philip, Avinash" <avinashphilip at ti.com>
>
> Add a driver for the TPS65910 PMIC that is found in the AM335x GP EVM,
> AM335x EVM SK and others.
Can we add a link to the technical manual?
http://www.ti.com/product/tps65910
> Signed-off-by: Philip, Avinash <avinashphilip at ti.com>
> [trini: Split and rework Avinash's changes into new drivers/power
> framework]
> Signed-off-by: Tom Rini <trini at ti.com>
> ---
> drivers/power/pmic/Makefile | 1 +
> drivers/power/pmic/pmic_tps65910.c | 69 +++++++++++++++++++++++++++++++
> include/power/tps65910.h | 79 ++++++++++++++++++++++++++++++++++++
> 3 files changed, 149 insertions(+)
> create mode 100644 drivers/power/pmic/pmic_tps65910.c
> create mode 100644 include/power/tps65910.h
>
> diff --git a/drivers/power/pmic/Makefile b/drivers/power/pmic/Makefile
> index 473cb80..1811080 100644
> --- a/drivers/power/pmic/Makefile
> +++ b/drivers/power/pmic/Makefile
> @@ -30,6 +30,7 @@ COBJS-$(CONFIG_POWER_MAX8997) += pmic_max8997.o
> COBJS-$(CONFIG_POWER_MUIC_MAX8997) += muic_max8997.o
> COBJS-$(CONFIG_POWER_MAX77686) += pmic_max77686.o
> COBJS-$(CONFIG_POWER_TPS65217) += pmic_tps65217.o
> +COBJS-$(CONFIG_POWER_TPS65910) += pmic_tps65910.o
>
> COBJS := $(COBJS-y)
> SRCS := $(COBJS:.o=.c)
> diff --git a/drivers/power/pmic/pmic_tps65910.c b/drivers/power/pmic/pmic_tps65910.c
> new file mode 100644
> index 0000000..0303f71
> --- /dev/null
> +++ b/drivers/power/pmic/pmic_tps65910.c
> @@ -0,0 +1,69 @@
> +/*
> + * (C) Copyright 2011-2013
> + * Texas Instruments, <www.ti.com>
> + *
> + * See file CREDITS for list of people who contributed to this
> + * project.
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License as
> + * published by the Free Software Foundation; either version 2 of
> + * the License, or (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
> + * MA 02111-1307 USA
> + */
> +
> +#include <common.h>
> +#include <i2c.h>
> +#include <power/tps65910.h>
> +
> +/*
> + * voltage switching for MPU frequency switching.
> + * @module = mpu - 0, core - 1
> + * @vddx_op_vol_sel = vdd voltage to set
No return identified here
> + */
> +int tps65910_voltage_update(unsigned int module, unsigned char vddx_op_vol_sel)
> +{
> + uchar buf[4];
If we only read and write one byte at a time why is this an array of 4?
> + unsigned int reg_offset;
> +
> + if (module == MPU)
> + reg_offset = TPS65910_VDD1_OP_REG;
> + else
> + reg_offset = TPS65910_VDD2_OP_REG;
This seems very specific to the implementation.
Can VDD2 ever be used for the MPU? Or are there constraints on the VDD2 SMPS that will not allow that?
> +
> + /* Select VDDx OP */
> + if (i2c_read(TPS65910_CTRL_I2C_ADDR, reg_offset, 1, buf, 1))
> + return 1;
I am going to assume that you changed this as well like you did for the TPS65217?
> +
> + buf[0] &= ~TPS65910_OP_REG_CMD_MASK;
> +
> + if (i2c_write(TPS65910_CTRL_I2C_ADDR, reg_offset, 1, buf, 1))
> + return 1;
> +
> + /* Configure VDDx OP Voltage */
> + if (i2c_read(TPS65910_CTRL_I2C_ADDR, reg_offset, 1, buf, 1))
> + return 1;
> +
> + buf[0] &= ~TPS65910_OP_REG_SEL_MASK;
> + buf[0] |= vddx_op_vol_sel;
> +
> + if (i2c_write(TPS65910_CTRL_I2C_ADDR, reg_offset, 1, buf, 1))
> + return 1;
> +
> + if (i2c_read(TPS65910_CTRL_I2C_ADDR, reg_offset, 1, buf, 1))
> + return 1;
> +
> + if ((buf[0] & TPS65910_OP_REG_SEL_MASK) != vddx_op_vol_sel)
> + return 1;
> +
> + return 0;
> +}
> diff --git a/include/power/tps65910.h b/include/power/tps65910.h
> new file mode 100644
> index 0000000..5942721
> --- /dev/null
> +++ b/include/power/tps65910.h
> @@ -0,0 +1,79 @@
> +/*
> + * (C) Copyright 2011-2013
> + * Texas Instruments, <www.ti.com>
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License as
> + * published by the Free Software Foundation version 2.
> + *
> + * This program is distributed "as is" WITHOUT ANY WARRANTY of any
> + * kind, whether express or implied; without even the implied warranty
> + * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + */
> +#ifndef __POWER_TPS65910_H__
> +#define __POWER_TPS65910_H__
> +
> +#define MPU 0
> +#define CORE 1
> +
> +int tps65910_voltage_update(unsigned int module, unsigned char vddx_op_vol_sel);
Nitpick - Don't the interface definitions go at the end of the file?
> +
> +#define TPS65910_SR_I2C_ADDR 0x12
> +#define TPS65910_CTRL_I2C_ADDR 0x2D
> +
> +/* PMIC Register offsets */
> +#define TPS65910_VDD1_REG 0x21
> +#define TPS65910_VDD1_OP_REG 0x22
> +#define TPS65910_VDD2_REG 0x24
> +#define TPS65910_VDD2_OP_REG 0x25
> +#define TPS65910_DEVCTRL_REG 0x3F
> +
> +/* VDD2 & VDD1 control register (VDD2_REG & VDD1_REG) */
> +#define TPS65910_VGAIN_SEL_MASK (0x3 << 6)
> +#define TPS65910_ILMAX_MASK (0x1 << 5)
> +#define TPS65910_TSTEP_MASK (0x7 << 2)
> +#define TPS65910_ST_MASK (0x3)
> +
> +#define TPS65910_REG_VGAIN_SEL_X1 (0x0 << 6)
> +#define TPS65910_REG_VGAIN_SEL_X1_0 (0x1 << 6)
> +#define TPS65910_REG_VGAIN_SEL_X3 (0x2 << 6)
> +#define TPS65910_REG_VGAIN_SEL_X4 (0x3 << 6)
> +
> +#define TPS65910_REG_ILMAX_1_0_A (0x0 << 5)
> +#define TPS65910_REG_ILMAX_1_5_A (0x1 << 5)
> +
> +#define TPS65910_REG_TSTEP_ (0x0 << 2)
> +#define TPS65910_REG_TSTEP_12_5 (0x1 << 2)
> +#define TPS65910_REG_TSTEP_9_4 (0x2 << 2)
> +#define TPS65910_REG_TSTEP_7_5 (0x3 << 2)
> +#define TPS65910_REG_TSTEP_6_25 (0x4 << 2)
> +#define TPS65910_REG_TSTEP_4_7 (0x5 << 2)
> +#define TPS65910_REG_TSTEP_3_12 (0x6 << 2)
> +#define TPS65910_REG_TSTEP_2_5 (0x7 << 2)
> +
> +#define TPS65910_REG_ST_OFF (0x0)
> +#define TPS65910_REG_ST_ON_HI_POW (0x1)
> +#define TPS65910_REG_ST_OFF_1 (0x2)
> +#define TPS65910_REG_ST_ON_LOW_POW (0x3)
> +
> +
Extra line
> +/* VDD2 & VDD1 voltage selection register. (VDD2_OP_REG & VDD1_OP_REG) */
> +#define TPS65910_OP_REG_SEL (0x7F)
> +
> +#define TPS65910_OP_REG_CMD_MASK (0x1 << 7)
> +#define TPS65910_OP_REG_CMD_OP (0x0 << 7)
> +#define TPS65910_OP_REG_CMD_SR (0x1 << 7)
> +
> +#define TPS65910_OP_REG_SEL_MASK (0x7F)
> +#define TPS65910_OP_REG_SEL_0_9_5 (0x1F) /* 0.9500 V */
> +#define TPS65910_OP_REG_SEL_1_1_3 (0x2E) /* 1.1375 V */
> +#define TPS65910_OP_REG_SEL_1_2_0 (0x33) /* 1.2000 V */
> +#define TPS65910_OP_REG_SEL_1_2_6 (0x38) /* 1.2625 V */
> +#define TPS65910_OP_REG_SEL_1_3_2_5 (0x3D) /* 1.3250 V */
> +
> +/* Device control register . (DEVCTRL_REG) */
> +#define TPS65910_DEVCTRL_REG_SR_CTL_I2C_MASK (0x1 << 4)
> +#define TPS65910_DEVCTRL_REG_SR_CTL_I2C_SEL_SR_I2C (0x0 << 4)
> +#define TPS65910_DEVCTRL_REG_SR_CTL_I2C_SEL_CTL_I2C (0x1 << 4)
> +#endif
Nitpick - #endif /* __POWER_TPS65910_H_ */
Is this not a family of parts?
http://www.ti.com/product/tps65910
TPS65910, TPS65910A, TPS65910A3, TPS659101, TPS659102, TPS659103
TPS659104, TPS659105, TPS659106, TPS659107, TPS659108, TPS659109
Do you think we could rename the definitions and the file to TPS65910x?
--
------------------
Dan Murphy
More information about the U-Boot
mailing list