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

[Lukasz Majewski <l.majewski@samsung.com>]

Hi Tom, Greg

> From: Greg Guyotte <gguyotte@ti.com>
> 
> Add a driver for the TPS65217 PMIC that is found in the Beaglebone
> family of boards.
> 
> Signed-off-by: Greg Guyotte <gguyotte@ti.com>
> [trini: Split and rework Greg's changes into new drivers/power
> framework]
> Signed-off-by: Tom Rini <trini@ti.com>
> 
> ---
> Changes in v2:
> - Address Dan's comments
> - Change to SPDX license tag
> - Add TRM link in the header
> 
> Signed-off-by: Tom Rini <trini@ti.com>
> ---
>  drivers/power/pmic/Makefile        |    1 +
>  drivers/power/pmic/pmic_tps65217.c |  109
> ++++++++++++++++++++++++++++++++++++
> include/power/tps65217.h           |   79 ++++++++++++++++++++++++++
> 3 files changed, 189 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 f054470..ac2b625 100644
> --- a/drivers/power/pmic/Makefile
> +++ b/drivers/power/pmic/Makefile
> @@ -13,6 +13,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..36e9024
> --- /dev/null
> +++ b/drivers/power/pmic/pmic_tps65217.c
> @@ -0,0 +1,109 @@
> +/*
> + * (C) Copyright 2011-2013
> + * Texas Instruments, <www.ti.com>
> + *
> + * SPDX-License-Identifier:	GPL-2.0+
> + */
> +
> +#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
> + * @return:		 0 for success, not 0 on failure.
> + */
> +int tps65217_reg_read(uchar src_reg, uchar *src_val)
> +{
> +	return i2c_read(TPS65217_CHIP_PM, src_reg, 1, src_val, 1);

Would it be possible to comply with pmic driver model?
It can be found at ./drivers/power/power_core.c

Moreover the generic function for reading/writing data to/from pmic is
already defined at ./drivers/power/power_{i2c|spi}.c 

Maybe it would be possible to use/modify the already available code?

> +}
> +
> +/**
> + *  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
> + *			   TPS65217_PROT_LEVEL_NONE,
> + *			   TPS65217_PROT_LEVEL_1 or
> TPS65217_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, not 0 on failure, as
> per the i2c API
> + */
> +int tps65217_reg_write(uchar prot_level, uchar dest_reg, uchar
> dest_val,

The same as above.

> +		       uchar mask)
> +{
> +	uchar read_val;
> +	uchar xor_reg;
> +	int ret;
> +
> +	/*
> +	 * If we are affecting only a bit field, read dest_reg and
> apply the
> +	 * mask
> +	 */
> +	if (mask != TPS65217_MASK_ALL_BITS) {
> +		ret = i2c_read(TPS65217_CHIP_PM, dest_reg, 1,
> &read_val, 1);
> +		if (ret)
> +			return ret;
> +		read_val &= (~mask);
> +		read_val |= (dest_val & mask);
> +		dest_val = read_val;
> +	}
> +
> +	if (prot_level > 0) {
> +		xor_reg = dest_reg ^ TPS65217_PASSWORD_UNLOCK;
> +		ret = i2c_write(TPS65217_CHIP_PM, TPS65217_PASSWORD,
> 1,
> +				&xor_reg, 1);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	ret = i2c_write(TPS65217_CHIP_PM, dest_reg, 1, &dest_val, 1);
> +	if (ret)
> +		return ret;
> +
> +	if (prot_level == TPS65217_PROT_LEVEL_2) {
> +		ret = i2c_write(TPS65217_CHIP_PM, TPS65217_PASSWORD,
> 1,
> +				&xor_reg, 1);
> +		if (ret)
> +			return ret;
> +
> +		ret = i2c_write(TPS65217_CHIP_PM, dest_reg, 1,
> &dest_val, 1);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	return 0;
> +}
> +
> +/**
> + * tps65217_voltage_update() - Function to change a voltage level,
> as this
> + *			       is a multi-step process.
> + * @dc_cntrl_reg:	       DC voltage control register to
> change.
> + * @volt_sel:		       New value for the voltage
> register
> + * @return:		       0 for success, not 0 on failure.
> + */
> +int tps65217_voltage_update(uchar dc_cntrl_reg, uchar volt_sel)

Maybe pmic_set_output() method from ./drivers/power/power_core.c can be
reused?

> +{
> +	if ((dc_cntrl_reg != TPS65217_DEFDCDC1) &&
> +	    (dc_cntrl_reg != TPS65217_DEFDCDC2) &&
> +	    (dc_cntrl_reg != TPS65217_DEFDCDC3))
> +		return 1;
> +
> +	/* set voltage level */
> +	if (tps65217_reg_write(TPS65217_PROT_LEVEL_2, dc_cntrl_reg,
> volt_sel,
> +			       TPS65217_MASK_ALL_BITS))
> +		return 1;
> +
> +	/* set GO bit to initiate voltage transition */
> +	if (tps65217_reg_write(TPS65217_PROT_LEVEL_2,
> TPS65217_DEFSLEW,
> +			       TPS65217_DCDC_GO, TPS65217_DCDC_GO))
> +		return 1;
> +
> +	return 0;
> +}
> diff --git a/include/power/tps65217.h b/include/power/tps65217.h
> new file mode 100644
> index 0000000..f4c7a2b
> --- /dev/null
> +++ b/include/power/tps65217.h
> @@ -0,0 +1,79 @@
> +/*
> + * (C) Copyright 2011-2013
> + * Texas Instruments, <www.ti.com>
> + *
> + * SPDX-License-Identifier:	GPL-2.0+
> + *
> + * For more details, please see the TRM at
> http://www.ti.com/product/tps65217a
> + */
> +
> +#ifndef __POWER_TPS65217_H__
> +#define __POWER_TPS65217_H__
> +
> +/* I2C chip address */
> +#define TPS65217_CHIP_PM			0x24
> +
> +/* Registers */
> +#define TPS65217_CHIPID				0x00
> +#define TPS65217_POWER_PATH			0x01
> +#define TPS65217_INTERRUPT			0x02
> +#define TPS65217_CHGCONFIG0			0x03
> +#define TPS65217_CHGCONFIG1			0x04
> +#define TPS65217_CHGCONFIG2			0x05
> +#define TPS65217_CHGCONFIG3			0x06
> +#define TPS65217_WLEDCTRL1			0x07
> +#define TPS65217_WLEDCTRL2			0x08
> +#define TPS65217_MUXCTRL			0x09
> +#define TPS65217_STATUS				0x0A
> +#define TPS65217_PASSWORD			0x0B
> +#define TPS65217_PGOOD				0x0C
> +#define TPS65217_DEFPG				0x0D
> +#define TPS65217_DEFDCDC1			0x0E
> +#define TPS65217_DEFDCDC2			0x0F
> +#define TPS65217_DEFDCDC3			0x10
> +#define TPS65217_DEFSLEW			0x11
> +#define TPS65217_DEFLDO1			0x12
> +#define TPS65217_DEFLDO2			0x13
> +#define TPS65217_DEFLS1				0x14
> +#define TPS65217_DEFLS2				0x15
> +#define TPS65217_ENABLE				0x16
> +#define TPS65217_DEFUVLO			0x18
> +#define TPS65217_SEQ1				0x19
> +#define TPS65217_SEQ2				0x1A
> +#define TPS65217_SEQ3				0x1B
> +#define TPS65217_SEQ4				0x1C
> +#define TPS65217_SEQ5				0x1D
> +#define TPS65217_SEQ6				0x1E
> +

Shouldn't the above registers be defined as enum?

For example at ./include/power/max8997_pmic.h
/* MAX 8997 registers */
enum {
	MAX8997_REG_PMIC_ID0	= 0x00,
	MAX8997_REG_PMIC_ID1	= 0x01,
	MAX8997_REG_INTSRC	= 0x02,
	....
	PMIC_NUM_OF_REGS
}

> +#define TPS65217_PROT_LEVEL_NONE		0x00
> +#define TPS65217_PROT_LEVEL_1			0x01
> +#define TPS65217_PROT_LEVEL_2			0x02
> +
> +#define TPS65217_PASSWORD_LOCK_FOR_WRITE	0x00
> +#define TPS65217_PASSWORD_UNLOCK		0x7D
> +
> +#define TPS65217_DCDC_GO			0x80
> +
> +#define TPS65217_MASK_ALL_BITS			0xFF
> +
> +#define TPS65217_USB_INPUT_CUR_LIMIT_MASK	0x03
> +#define TPS65217_USB_INPUT_CUR_LIMIT_100MA	0x00
> +#define TPS65217_USB_INPUT_CUR_LIMIT_500MA	0x01
> +#define TPS65217_USB_INPUT_CUR_LIMIT_1300MA	0x02
> +#define TPS65217_USB_INPUT_CUR_LIMIT_1800MA	0x03
> +
> +#define TPS65217_DCDC_VOLT_SEL_1275MV		0x0F
> +#define TPS65217_DCDC_VOLT_SEL_1325MV		0x11
> +
> +#define TPS65217_LDO_MASK			0x1F
> +#define TPS65217_LDO_VOLTAGE_OUT_1_8		0x06
> +#define TPS65217_LDO_VOLTAGE_OUT_3_3		0x1F
> +
> +#define TPS65217_PWR_SRC_USB_BITMASK		0x4
> +#define TPS65217_PWR_SRC_AC_BITMASK		0x8
> +
> +int 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);
> +#endif	/* __POWER_TPS65217_H__ */


-- 
Best regards,

Lukasz Majewski

Samsung R&D Institute Poland (SRPOL) | Linux Platform Group