On Wed, 14 Aug 2013 11:57:06 -0400 Tom Rini <tr...@ti.com> wrote: > On Wed, Aug 14, 2013 at 05:08:12PM +0200, Lukasz Majewski wrote: > > Hi Tom, Greg > > > > > 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> > > > > > > --- > > > Changes in v2: > > > - Address Dan's comments > > > - Change to SPDX license tag > > > - Add TRM link in the header > > > > > > Signed-off-by: Tom Rini <tr...@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
signature.asc
Description: PGP signature
_______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot