On 07/19/2013 02:00 PM, Tom Rini wrote:
> From: Greg Guyotte <gguyo...@ti.com>
>
> Add a driver for the TPS65217 PMIC that is found in the Beaglebone
> family of boards.
>
> Signed-off-by: Greg Guyotte <gguyo...@ti.com>
> [trini: Split and rework Greg's changes into new drivers/power
> framework]
> Signed-off-by: Tom Rini <tr...@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
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
