| Message ID | 1374260426-9085-3-git-send-email-trini@ti.com |
|---|---|
| State | Superseded |
| Delegated to: | Tom Rini |
| Headers | show |
On 07/19/2013 02:00 PM, Tom Rini wrote: > From: "Philip, Avinash" <avinashphilip@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@ti.com> > [trini: Split and rework Avinash's changes into new drivers/power > framework] > Signed-off-by: Tom Rini <trini@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?
On Thu, Jul 25, 2013 at 12:41:46PM -0500, Dan Murphy wrote: > On 07/19/2013 02:00 PM, Tom Rini wrote: > > From: "Philip, Avinash" <avinashphilip@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 Yup, in the code. [snip] > > +/* > > + * voltage switching for MPU frequency switching. > > + * @module = mpu - 0, core - 1 > > + * @vddx_op_vol_sel = vdd voltage to set > > No return identified here Fixed. > > + */ > > +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? Fixed. > > + 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? Can? Probably. Will someone deviate from the reference design? Probably not. > > + > > + /* 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? Correct. > > +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? Sure. [snip] > 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? Fixed the other two minor things, and yes, but if we follow the kernel example here (note that we didn't copy code) it's still just tps65910 and tps65217 (tps65217 has a/b/c variants).
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 + */ +int tps65910_voltage_update(unsigned int module, unsigned char vddx_op_vol_sel) +{ + uchar buf[4]; + unsigned int reg_offset; + + if (module == MPU) + reg_offset = TPS65910_VDD1_OP_REG; + else + reg_offset = TPS65910_VDD2_OP_REG; + + /* Select VDDx OP */ + if (i2c_read(TPS65910_CTRL_I2C_ADDR, reg_offset, 1, buf, 1)) + return 1; + + 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); + +#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) + + +/* 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