power: pmic/regulator: Add basic support for TPS65910
PATCH v2 1/1 power: pmic/regulator: Add basic support for TPS65910
Texas Instrument's TPS65910 PMIC contains 3 buck DC-DC converts, one boost DC-DC converter and 8 LDOs. This patch implements driver model support for the TPS65910 PMIC and its regulators making the get/set API for regulator value/enable available.
This patch depends on the patch "am33xx: Add a function to query MPU voltage in uV" to build correctly. For boards relying on the DT include file tps65910.dtsi the v3 patch "power: extend prefix match to regulator-name property" and an appropriate regulator naming is also required.
---
Changes in v2:
- move information about regulator supply power from the PMIC to the regulator driver which greatly simplifies and streamlines the code of both drivers (thanks Simon!)
- use dev_get_uclass_platdata() to get the regulator constraints instead of reinventing the wheel and reading them from the DT
- make pmic_tps65910_write() and pmic_tps65910_read() return the results of the i2c operation instead of obscuring it
- generally review and eventually fix return values
- modify regulator description in Kconfig
- get_ctrl_reg_from_unit_addr() uses a lookup array instead of a switch statement
- rename some local functions to prevent confusion with generic functions
- reformatting code for better readability
drivers/power/pmic/Kconfig | 8 + drivers/power/pmic/Makefile | 1 + drivers/power/pmic/pmic_tps65910_dm.c | 98 ++++++ drivers/power/regulator/Kconfig | 8 + drivers/power/regulator/Makefile | 1 + drivers/power/regulator/tps65910_regulator.c | 456 +++++++++++++++++++++++++++ include/power/tps65910_pmic.h | 129 ++++++++ 7 files changed, 701 insertions(+) create mode 100644 drivers/power/pmic/pmic_tps65910_dm.c create mode 100644 drivers/power/regulator/tps65910_regulator.c create mode 100644 include/power/tps65910_pmic.h
drivers/power/pmic/Kconfig +8 -0
@@ -201,3 +201,11 @@ config POWER_MC34VR500 The MC34VR500 is used in conjunction with the FSL T1 and LS1 series SoC. It provides 4 buck DC-DC convertors and 5 LDOs, and it is accessed via an I2C interface. + +config DM_PMIC_TPS65910 + bool "Enable driver for Texas Instruments TPS65910 PMIC" + depends on DM_PMIC + ---help--- + The TPS65910 is a PMIC containing 3 buck DC-DC converters, one boost + DC-DC converter, 8 LDOs and a RTC. This driver binds the SMPS and LDO + pmic children.
drivers/power/pmic/Makefile +1 -0
@@ -19,6 +19,7 @@ obj-$(CONFIG_PMIC_RK8XX) += rk8xx.o obj-$(CONFIG_PMIC_RN5T567) += rn5t567.o obj-$(CONFIG_PMIC_TPS65090) += tps65090.o obj-$(CONFIG_PMIC_S5M8767) += s5m8767.o +obj-$(CONFIG_DM_PMIC_TPS65910) += pmic_tps65910_dm.o obj-$(CONFIG_$(SPL_)PMIC_PALMAS) += palmas.o obj-$(CONFIG_$(SPL_)PMIC_LP873X) += lp873x.o obj-$(CONFIG_$(SPL_)PMIC_LP87565) += lp87565.o
drivers/power/pmic/pmic_tps65910_dm.c new file +98 -0
98 lines not on this page · raw message
drivers/power/regulator/Kconfig +8 -0
@@ -168,3 +168,11 @@ config DM_REGULATOR_LP87565 LP87565 series of PMICs have 4 single phase BUCKs that can also be configured in multi phase modes. The driver implements get/set api for value and enable. + +config DM_REGULATOR_TPS65910 + bool "Enable driver for TPS65910 PMIC regulators" + depends on DM_PMIC_TPS65910 + ---help--- + The TPS65910 PMIC provides 4 SMPSs and 8 LDOs. This driver supports all + regulator types of the TPS65910 (BUCK, BOOST and LDO). It implements + the get/set api for value and enable.
drivers/power/regulator/Makefile +1 -0
@@ -20,3 +20,4 @@ obj-$(CONFIG_REGULATOR_TPS65090) += tps65090_regulator.o obj-$(CONFIG_$(SPL_)DM_REGULATOR_PALMAS) += palmas_regulator.o obj-$(CONFIG_$(SPL_)DM_REGULATOR_LP873X) += lp873x_regulator.o obj-$(CONFIG_$(SPL_)DM_REGULATOR_LP87565) += lp87565_regulator.o +obj-$(CONFIG_DM_REGULATOR_TPS65910) += tps65910_regulator.o
drivers/power/regulator/tps65910_regulator.c new file +456 -0
456 lines not on this page · raw message
include/power/tps65910_pmic.h new file +129 -0
129 lines not on this page · raw message
Signature · 1 line
2.7.4
PATCH v2 1/1 power: pmic/regulator: Add basic support for TPS65910
Quoted 29 lines · Felix Brack
Texas Instrument's TPS65910 PMIC contains 3 buck DC-DC converts, one boost DC-DC converter and 8 LDOs. This patch implements driver model support for the TPS65910 PMIC and its regulators making the get/set API for regulator value/enable available.
This patch depends on the patch "am33xx: Add a function to query MPU voltage in uV" to build correctly. For boards relying on the DT include filetps65910.dtsithe v3 patch "power: extend prefix match to regulator-name property" and an appropriate regulator naming is also required.Signed-off-by Felix Brack <[email protected]>---
Changes in v2:
- move information about regulator supply power from the PMIC to the regulator driver which greatly simplifies and streamlines the code of both drivers (thanks Simon!)
- usedev_get_uclass_platdata()to get the regulator constraints instead of reinventing the wheel and reading them from the DT
- makepmic_tps65910_write()andpmic_tps65910_read()return the results of the i2c operation instead of obscuring it
- generally review and eventually fix return values
- modify regulator description inKconfig
-get_ctrl_reg_from_unit_addr()uses a lookup array instead of a switch statement
- rename some local functions to prevent confusion with generic functions
- reformatting code for better readability
Quoted 11 lines
drivers/power/pmic/Kconfig | 8 + drivers/power/pmic/Makefile | 1 + drivers/power/pmic/pmic_tps65910_dm.c | 98 ++++++ drivers/power/regulator/Kconfig | 8 + drivers/power/regulator/Makefile | 1 + drivers/power/regulator/tps65910_regulator.c | 456 +++++++++++++++++++++++++++ include/power/tps65910_pmic.h | 129 ++++++++ 7 files changed, 701 insertions(+) create mode 100644 drivers/power/pmic/pmic_tps65910_dm.c create mode 100644 drivers/power/regulator/tps65910_regulator.c create mode 100644 include/power/tps65910_pmic.h
This looks good to me. I just have a few more nits.
[..]
Quoted 27 lines
drivers/power/regulator/Makefile +1
-0@@ -20,3 +20,4 @@ obj-$(CONFIG_REGULATOR_TPS65090) += tps65090_regulator.o obj-$(CONFIG_$(SPL_)DM_REGULATOR_PALMAS) += palmas_regulator.o obj-$(CONFIG_$(SPL_)DM_REGULATOR_LP873X) += lp873x_regulator.o obj-$(CONFIG_$(SPL_)DM_REGULATOR_LP87565) += lp87565_regulator.o +obj-$(CONFIG_DM_REGULATOR_TPS65910) += tps65910_regulator.odrivers/power/regulator/tps65910_regulator.c new file +12
-0@@ -0,0 +1,456 @@ +/* +* Copyright (C) EETS GmbH, 2017, Felix Brack <[email protected]> +* +* SPDX-License-Identifier: GPL-2.0+ +*/ + +#include <common.h> +#include <dm.h> +#include <power/pmic.h> +#include <power/regulator.h> +#include <power/tps65910_pmic.h> +
[..]
drivers/power/regulator/tps65910_regulator.c line 135
Quoted 3 lines
drivers/power/regulator/tps65910_regulator.c new file +3
-0+static int get_ctrl_reg_from_unit_addr (const int unit_addr) +{ +if (unit_addr >=0 && unit_addr <ARRAY_SIZE (ctrl_regs))
You could make that parameter uint if you like and avoid the first check.
drivers/power/regulator/tps65910_regulator.c line 144
Quoted 9 lines
drivers/power/regulator/tps65910_regulator.c new file +9
-0+return ctrl_regs[unit_addr]; +return -ENXIO ; +} + +static int tps65910_regulator_get_value (struct udevice *dev, +const struct regulator_props *rgp) +{ +int sel; +u8 val;
int val
(particularly as you check for val < 0 below)
Please fix in rest of file also. In general these small-size variables just force the compiler to mask values and don't add any benefit IMO. We may as well use int.
Quoted 7 lines
drivers/power/regulator/tps65910_regulator.c new file +7
-0+int vout =0 ; +struct tps65910_regulator_pdata *pdata = dev->platdata; +int vin = pdata->supply; + + val =pmic_reg_read (dev->parent, rgp->reg); +if (val <0 ) +return val;
drivers/power/regulator/tps65910_regulator.c line 161
Quoted 10 lines
drivers/power/regulator/tps65910_regulator.c new file +10
-0+ sel = (val &TPS65910_SEL_MASK ) >>2 ; + vout = (vin >= *(rgp->vin_min + sel)) ? *(rgp->vout + sel) :0 ; + vout = ((val &TPS65910_SUPPLY_STATE_MASK ) ==1 ) ? vout :0 ; + +return vout; +} + +static int tps65910_ldo_get_value (struct udevice *dev) +{ +struct tps65910_regulator_pdata *pdata = dev->platdata;
dev_get_platdata(dev)
Quoted 27 lines
drivers/power/regulator/tps65910_regulator.c new file +27
-0+int vin = pdata->supply; + +switch (dev->driver_data) { +case TPS65910_UNIT_VRTC : +/* VRTC is fixed and can't be turned off */ +return (vin >=2500000 ) ?1830000 :0 ; +case TPS65910_UNIT_VDIG1 : +return tps65910_regulator_get_value (dev, &ldo_props_vdig1); +case TPS65910_UNIT_VDIG2 : +return tps65910_regulator_get_value (dev, &ldo_props_vdig2); +case TPS65910_UNIT_VPLL : +return tps65910_regulator_get_value (dev, &ldo_props_vpll); +case TPS65910_UNIT_VDAC : +return tps65910_regulator_get_value (dev, &ldo_props_vdac); +case TPS65910_UNIT_VAUX1 : +return tps65910_regulator_get_value (dev, &ldo_props_vaux1); +case TPS65910_UNIT_VAUX2 : +return tps65910_regulator_get_value (dev, &ldo_props_vaux2); +case TPS65910_UNIT_VAUX33 : +return tps65910_regulator_get_value (dev, &ldo_props_vaux33); +case TPS65910_UNIT_VMMC : +return tps65910_regulator_get_value (dev, &ldo_props_vmmc); + } + +return 0 ; +} +
[..]
drivers/power/regulator/tps65910_regulator.c line 256
Quoted 10 lines
drivers/power/regulator/tps65910_regulator.c new file +10
-0+static int tps65910_get_enable (struct udevice *dev) +{ +int reg, ret; +u8 val; + + reg =get_ctrl_reg_from_unit_addr (dev->driver_data); +if (reg <0 ) +return reg; + + ret =pmic_read (dev->parent, reg, &val,1 );
Does pmic_reg_read() work here? Then you don't need the u8 val.
Quoted 7 lines
drivers/power/regulator/tps65910_regulator.c new file +7
-0+if (ret) +return ret; + +/* bits 1:0 of regulator control register define state */ +return ((val &TPS65910_SUPPLY_STATE_MASK ) ==1 ); +} +
[..]
drivers/power/regulator/tps65910_regulator.c line 307
Quoted 5 lines
drivers/power/regulator/tps65910_regulator.c new file +5
-0+static int tps65910_buck_get_value (struct udevice *dev) +{ +int vout =0 ; + +switch (dev->driver_data) {
dev_get_driver_data(dev)
But please can you store this in your priv/platdata structure so you set it up in probe() or ofdata_to_pdata()? Ideally this should be an enum field. Similar below.
Quoted 4 lines
drivers/power/regulator/tps65910_regulator.c new file +4
-0+case TPS65910_UNIT_VIO : + vout =tps65910_regulator_get_value (dev, &smps_props_vio); +break ; +case TPS65910_UNIT_VDD1 :
drivers/power/regulator/tps65910_regulator.c line 321
Quoted 10 lines
drivers/power/regulator/tps65910_regulator.c new file +10
-0+ vout =buck_get_vdd1_vdd2_value (dev->parent,TPS65910_REG_VDD1 ); +break ; +case TPS65910_UNIT_VDD2 : + vout =buck_get_vdd1_vdd2_value (dev->parent,TPS65910_REG_VDD2 ); +break ; + } + +return vout; +} +
[..]
Regards,
Simon