From mboxrd@z Thu Jan 1 00:00:00 1970 From: Dan Murphy Date: Thu, 25 Jul 2013 12:41:46 -0500 Subject: [U-Boot] [PATCH 3/6] drivers/power/pmic: Add tps65910 driver In-Reply-To: <1374260426-9085-3-git-send-email-trini@ti.com> References: <1374260426-9085-1-git-send-email-trini@ti.com> <1374260426-9085-3-git-send-email-trini@ti.com> Message-ID: <51F1635A.1030504@ti.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de On 07/19/2013 02:00 PM, Tom Rini wrote: > From: "Philip, Avinash" > > 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 > [trini: Split and rework Avinash's changes into new drivers/power > framework] > Signed-off-by: Tom Rini > --- > 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, > + * > + * 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 > +#include > +#include > + > +/* > + * 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, > + * > + * 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