[U-Boot] [PATCH 2/6] drivers/power/pmic: Add tps65217 driver

Dan Murphy dmurphy at ti.com
Tue Jul 23 20:34:15 CEST 2013


On 07/19/2013 02:00 PM, Tom Rini wrote:
> From: Greg Guyotte <gguyotte at ti.com>
>
> Add a driver for the TPS65217 PMIC that is found in the Beaglebone
> family of boards.
>
> Signed-off-by: Greg Guyotte <gguyotte at ti.com>
> [trini: Split and rework Greg'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_tps65217.c |  108 ++++++++++++++++++++++++++++++++++++
>  include/power/tps65217.h           |   92 ++++++++++++++++++++++++++++++
>  3 files changed, 201 insertions(+)
>  create mode 100644 drivers/power/pmic/pmic_tps65217.c
>  create mode 100644 include/power/tps65217.h
>
> diff --git a/drivers/power/pmic/Makefile b/drivers/power/pmic/Makefile
> index 14d426f..473cb80 100644
> --- a/drivers/power/pmic/Makefile
> +++ b/drivers/power/pmic/Makefile
> @@ -29,6 +29,7 @@ COBJS-$(CONFIG_POWER_MAX8998) += pmic_max8998.o
>  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	:= $(COBJS-y)
>  SRCS	:= $(COBJS:.o=.c)
> diff --git a/drivers/power/pmic/pmic_tps65217.c b/drivers/power/pmic/pmic_tps65217.c
> new file mode 100644
> index 0000000..c84bbcd
> --- /dev/null
> +++ b/drivers/power/pmic/pmic_tps65217.c
> @@ -0,0 +1,108 @@
> +/*
> + * (C) Copyright 2011-2013
Curious if this is the first time in why does it have a 2011 copyright?
> + * 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/tps65217.h>
> +
> +/**
> + * tps65217_reg_read() - Generic function that can read a TPS65217 register
> + * @src_reg:	  Source register address
> + * @src_val:	  Address of destination variable
No return defined here in the brief
> + */
> +uchar tps65217_reg_read(uchar src_reg, uchar *src_val)
> +{
> +	if (i2c_read(TPS65217_CHIP_PM, src_reg, 1, src_val, 1))
> +		return 1;
This may be nit picky but generally in error cases we return negative.
Also why not return an error from errno?

Also why an uchar when you are returning an int?

> +	return 0;
> +}
> +
> +/**
> + *  tps65217_reg_write() - Generic function that can write a TPS65217 PMIC
> + *			 register or bit field regardless of protection
> + *			 level.
> + *
> + *  @prot_level:	Register password protection.
> + *		      use PROT_LEVEL_NONE, PROT_LEVEL_1, or PROT_LEVEL_2
> + *  @dest_reg:	  Register address to write.
> + *  @dest_val:	  Value to write.
> + *  @mask:	      Bit mask (8 bits) to be applied.  Function will only
> + *		      change bits that are set in the bit mask.
> + *
> + *  @return:	    0 for success, 1 for failure.
> + */
> +int tps65217_reg_write(uchar prot_level, uchar dest_reg, uchar dest_val,
is prot_level a uchar or int?
Also would it not be better to have an interface that will check for mask and do the read and
just have a dedicated write function?
> +		       uchar mask)
> +{
> +	uchar read_val;
> +	uchar xor_reg;
> +
> +	/*
> +	 * If we are affecting only a bit field, read dest_reg and apply the
> +	 * mask
> +	 */
> +	if (mask != MASK_ALL_BITS) {
> +		if (i2c_read(TPS65217_CHIP_PM, dest_reg, 1, &read_val, 1))
> +			return 1;
> +		read_val &= (~mask);
> +		read_val |= (dest_val & mask);
> +		dest_val = read_val;
> +	}
> +
> +	if (prot_level > 0) {
> +		xor_reg = dest_reg ^ PASSWORD_UNLOCK;
> +		if (i2c_write(TPS65217_CHIP_PM, PASSWORD, 1, &xor_reg, 1))
> +			return 1;
Same comment as above
> +	}
> +
> +	if (i2c_write(TPS65217_CHIP_PM, dest_reg, 1, &dest_val, 1))
> +		return 1;
> +
> +	if (prot_level == PROT_LEVEL_2) {
> +		if (i2c_write(TPS65217_CHIP_PM, PASSWORD, 1, &xor_reg, 1))
> +			return 1;
> +
> +		if (i2c_write(TPS65217_CHIP_PM, dest_reg, 1, &dest_val, 1))
> +			return 1;
> +	}
> +
> +	return 0;
> +}
> +
> +int tps65217_voltage_update(uchar dc_cntrl_reg, uchar volt_sel)
No header for the interface
> +{
> +	if ((dc_cntrl_reg != DEFDCDC1) && (dc_cntrl_reg != DEFDCDC2) &&
> +	    (dc_cntrl_reg != DEFDCDC3))
What do these magic numbers mean?  Are these HEX numbers or a string?
> +		return 1;
> +
> +	/* set voltage level */
> +	if (tps65217_reg_write(PROT_LEVEL_2, dc_cntrl_reg, volt_sel,
> +			       MASK_ALL_BITS))
> +		return 1;
> +
> +	/* set GO bit to initiate voltage transition */
> +	if (tps65217_reg_write(PROT_LEVEL_2, DEFSLEW, DCDC_GO, DCDC_GO))
> +		return 1;
> +
> +	return 0;
> +}
> diff --git a/include/power/tps65217.h b/include/power/tps65217.h
> new file mode 100644
> index 0000000..c12a709
> --- /dev/null
> +++ b/include/power/tps65217.h
> @@ -0,0 +1,92 @@
> +/*
> + * (C) Copyright 2011-2013
Same copyright comment as above
> + * 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
> + */
> +
> +#ifndef __TPS65217_H__
> +#define __TPS65217_H__
> +
> +/* I2C chip address */
> +#define TPS65217_CHIP_PM		0x24
> +
> +/* Registers */
> +#define CHIPID				0x00
> +#define POWER_PATH			0x01
> +#define INTERRUPT			0x02
> +#define CHGCONFIG0			0x03
> +#define CHGCONFIG1			0x04
> +#define CHGCONFIG2			0x05
> +#define CHGCONFIG3			0x06
> +#define WLEDCTRL1			0x07
> +#define WLEDCTRL2			0x08
> +#define MUXCTRL				0x09
> +#define STATUS				0x0A
> +#define PASSWORD			0x0B
> +#define PGOOD				0x0C
> +#define DEFPG				0x0D
> +#define DEFDCDC1			0x0E
> +#define DEFDCDC2			0x0F
> +#define DEFDCDC3			0x10
> +#define DEFSLEW				0x11
> +#define DEFLDO1				0x12
> +#define DEFLDO2				0x13
> +#define DEFLS1				0x14
> +#define DEFLS2				0x15
> +#define ENABLE				0x16
> +#define DEFUVLO				0x18
> +#define SEQ1				0x19
> +#define SEQ2				0x1A
> +#define SEQ3				0x1B
> +#define SEQ4				0x1C
> +#define SEQ5				0x1D
> +#define SEQ6				0x1E
> +
> +#define PROT_LEVEL_NONE			0x00
Are these registers or a mask now?
> +#define PROT_LEVEL_1			0x01
> +#define PROT_LEVEL_2			0x02
> +
> +#define PASSWORD_LOCK_FOR_WRITE		0x00
> +#define PASSWORD_UNLOCK			0x7D
> +
> +#define DCDC_GO				0x80
> +
> +#define MASK_ALL_BITS			0xFF
> +
> +#define USB_INPUT_CUR_LIMIT_MASK	0x03
> +#define USB_INPUT_CUR_LIMIT_100MA	0x00
> +#define USB_INPUT_CUR_LIMIT_500MA	0x01
> +#define USB_INPUT_CUR_LIMIT_1300MA	0x02
> +#define USB_INPUT_CUR_LIMIT_1800MA	0x03
> +
> +#define DCDC_VOLT_SEL_1275MV		0x0F
> +#define DCDC_VOLT_SEL_1325MV		0x11
> +
> +#define LDO_MASK			0x1F
> +#define LDO_VOLTAGE_OUT_3_3		0x1F
> +
> +#define PWR_SRC_USB_BITMASK		0x4
> +#define PWR_SRC_AC_BITMASK		0x8
> +
> +uchar tps65217_reg_read(uchar src_reg, uchar *src_val);
> +int tps65217_reg_write(uchar prot_level, uchar dest_reg, uchar dest_val,
> +		       uchar mask);
> +int tps65217_voltage_update(uchar dc_cntrl_reg, uchar volt_sel);
Are these interfaces supposed to be accessed by an outside object?

Typically there should be no direct register access from other objects.
> +#endif


-- 
------------------
Dan Murphy



More information about the U-Boot mailing list