On Wed, 14 Aug 2013 11:57:06 -0400 Tom Rini trini@ti.com wrote:
On Wed, Aug 14, 2013 at 05:08:12PM +0200, Lukasz Majewski wrote:
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
At the high level, not yet. We don't have battery support (but fixing that to be optional in the core wouldn't be hard) but the general pmic code assumes one pmic charger per binary.
As fair as I remember, there is no such assumption. The pmic driver allocates each pmic object separately (which can be distinguished by unique name - also many instances of the same devices are possible). Each power device is treated in the same way (described by struct pmic), no matter if it is a battery, charger, PMIC or MUIC.
Then each reference is done via struct pmic *p pointer. The charger is not needed to use the generic pmic_reg_write().
The tps65217_reg_read() method is used at board/ti/am335x/board.c - [PATCH v2 6/6] am335x_evm: am33xx_spl_board_init function and scale core frequency
It is a similar use to pmic_init_max8997(void) defined at board/samsung/trats/trats.c
We need both in the same binary (since we decide at run-time if it's one of the boards with 65910 or 65217).
The pmic core can register both devices, then with OF decide to which one refer with e.g. p->name field.
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?
Without the MAX family datasheets handy, I'm not sure how exactly the tx_num stuff maps to the password concept the TI parts have. Skimming the kernel mfd drivers implies to me that logic ends up being per-chip (or at least vendor).
We have spent some time with Stefano to provide correct read/write for the following:
- 1,2,3 bytes transfers - little + big endian data format support - support for SPI and I2C.
This is already implemented at pmic_reg_write().
[snip]
+/**
- 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?
I'm not sure.
At least we shall give it a try.
[snip]
+#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
I assume it's a style thing I've overlooked, so sure, not a problem in general.
Ok, thanks.
I'm aware, that current pmic framework has some shortcomings, but I also believe that it can be developed to serve as a unified power management framework for u-boot users.
Regards, Lukasz Majewski