[U-Boot] [PATCH] power: pmic/regulator: Add basic support for TPS65910
Felix Brack
fb at ltec.ch
Wed Nov 22 10:39:42 UTC 2017
Hello Simon,
Many thanks for taking the time to review my patch.
On 20.11.2017 16:38, Simon Glass wrote:
> Hi Felix,
>
> On 8 November 2017 at 04:04, Felix Brack <fb at ltec.ch> wrote:
>> 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 v2 patch "power: extend prefix match
>> to regulator-name property" and an appropriate regulator naming is
>> also required.
>>
>> Signed-off-by: Felix Brack <fb at ltec.ch>
>> ---
>>
>> drivers/power/pmic/Kconfig | 8 +
>> drivers/power/pmic/Makefile | 1 +
>> drivers/power/pmic/pmic_tps65910_dm.c | 138 ++++++++
>> drivers/power/regulator/Kconfig | 7 +
>> drivers/power/regulator/Makefile | 1 +
>> drivers/power/regulator/tps65910_regulator.c | 493 +++++++++++++++++++++++++++
>> include/power/tps65910_pmic.h | 130 +++++++
>> 7 files changed, 778 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
>>
>> diff --git a/drivers/power/pmic/Kconfig b/drivers/power/pmic/Kconfig
>> index e3f9e4d..5d49c93 100644
>> --- a/drivers/power/pmic/Kconfig
>> +++ b/drivers/power/pmic/Kconfig
>> @@ -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.
>
> Great!
>
>> diff --git a/drivers/power/pmic/Makefile b/drivers/power/pmic/Makefile
>> index f7bdfa5..7d6c583 100644
>> --- a/drivers/power/pmic/Makefile
>> +++ b/drivers/power/pmic/Makefile
>> @@ -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
>> diff --git a/drivers/power/pmic/pmic_tps65910_dm.c b/drivers/power/pmic/pmic_tps65910_dm.c
>> new file mode 100644
>> index 0000000..1410657
>> --- /dev/null
>> +++ b/drivers/power/pmic/pmic_tps65910_dm.c
>> @@ -0,0 +1,138 @@
>> +/*
>> + * Copyright (C) EETS GmbH, 2017, Felix Brack <f.brack at eets.ch>
>> + *
>> + * SPDX-License-Identifier: GPL-2.0+
>> + */
>> +
>> +#include <common.h>
>> +#include <dm.h>
>> +#include <i2c.h>
>> +#include <power/pmic.h>
>> +#include <power/regulator.h>
>> +#include <power/tps65910_pmic.h>
>> +
>> +DECLARE_GLOBAL_DATA_PTR;
>> +
>> +static const struct pmic_child_info pmic_children_info[] = {
>> + { .prefix = "ldo_", .driver = TPS65910_LDO_DRIVER },
>> + { .prefix = "buck_", .driver = TPS65910_BUCK_DRIVER },
>> + { .prefix = "boost_", .driver = TPS65910_BOOST_DRIVER },
>> + { },
>> +};
>> +
>> +static const char * const supply_names[] = {
>> + "vccio-supply",
>> + "vcc1-supply",
>> + "vcc2-supply",
>> + "vcc3-supply",
>> + "vcc4-supply",
>> + "vcc5-supply",
>> + "vcc6-supply",
>> + "vcc7-supply",
>> +};
>> +
>> +static int pmic_tps65910_reg_count(struct udevice *dev)
>> +{
>> + return TPS65910_NUM_REGS;
>> +}
>> +
>> +static int pmic_tps65910_write(struct udevice *dev, uint reg, const u8 *buffer,
>> + int len)
>> +{
>> + if (dm_i2c_write(dev, reg, buffer, len)) {
>> + error("%s write error on register %02x\n", dev->name, reg);
>> + return -EIO;
>
> Can you return ret here instead (and in cases below)? I does not seem
> necessary to obscure the original error.
>
> [...]
>
Yes for the calls to dm_i2c_write() and dm_i2c_read(). How about the
call to ofnode_valid()? Is it okay to return -ENXIO instead of NULL in
case of failure?
>> diff --git a/drivers/power/regulator/Kconfig b/drivers/power/regulator/Kconfig
>> index c82a936..2d6a150 100644
>> --- a/drivers/power/regulator/Kconfig
>> +++ b/drivers/power/regulator/Kconfig
>> @@ -168,3 +168,10 @@ 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 implements
>> + the get/set api for value and enable for these regulators.
>
> I think that last sentence should be split into two.
>
Okay
>> diff --git a/drivers/power/regulator/Makefile b/drivers/power/regulator/Makefile
>> index 18fb870..3eef297 100644
>> --- a/drivers/power/regulator/Makefile
>> +++ b/drivers/power/regulator/Makefile
>> @@ -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
>> diff --git a/drivers/power/regulator/tps65910_regulator.c b/drivers/power/regulator/tps65910_regulator.c
>> new file mode 100644
>> index 0000000..d212b70
>> --- /dev/null
>> +++ b/drivers/power/regulator/tps65910_regulator.c
>> @@ -0,0 +1,493 @@
>> +/*
>> + * Copyright (C) EETS GmbH, 2017, Felix Brack <f.brack at eets.ch>
>> + *
>> + * 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>
>> +
>> +#define VOUT_CHOICE_COUNT 4
>> +
>> +/*
>> + * struct regulator_props - Properties of a LDO and VIO SMPS regulator
>> + *
>> + * All of these regulators allow setting one out of four output voltages.
>> + * These output voltages are only achievable when supplying the regulator
>> + * with a minimum input voltage.
>> + *
>> + * @vin_min[]: minimum supply input voltage in uV required to achieve the
>> + * corresponding vout[] voltage
>> + * @vout[]: regulator output voltage in uV
>> + * @reg: I2C register used to set regulator voltage
>> + */
>> +struct regulator_props {
>> + int vin_min[VOUT_CHOICE_COUNT];
>> + int vout[VOUT_CHOICE_COUNT];
>> + int reg;
>> +};
>> +
>> +static const struct regulator_props ldo_props_vdig1 = {
>> + .vin_min = { 1700000, 2100000, 2700000, 3200000 },
>> + .vout = { 1200000, 1500000, 1800000, 2700000 },
>> + .reg = TPS65910_REG_VDIG1
>> +};
>> +
>> +static const struct regulator_props ldo_props_vdig2 = {
>> + .vin_min = { 1700000, 1700000, 1700000, 2700000 },
>> + .vout = { 1000000, 1100000, 1200000, 1800000 },
>> + .reg = TPS65910_REG_VDIG2
>> +};
>> +
>> +static const struct regulator_props ldo_props_vpll = {
>> + .vin_min = { 2700000, 2700000, 2700000, 3000000 },
>> + .vout = { 1000000, 1100000, 1800000, 2500000 },
>> + .reg = TPS65910_REG_VPLL
>> +};
>> +
>> +static const struct regulator_props ldo_props_vdac = {
>> + .vin_min = { 2700000, 3000000, 3200000, 3200000 },
>> + .vout = { 1800000, 2600000, 2800000, 2850000 },
>> + .reg = TPS65910_REG_VDAC
>> +};
>> +
>> +static const struct regulator_props ldo_props_vaux1 = {
>> + .vin_min = { 2700000, 3200000, 3200000, 3200000 },
>> + .vout = { 1800000, 2500000, 2800000, 2850000 },
>> + .reg = TPS65910_REG_VAUX1
>> +};
>> +
>> +static const struct regulator_props ldo_props_vaux2 = {
>> + .vin_min = { 2700000, 3200000, 3200000, 3600000 },
>> + .vout = { 1800000, 2800000, 2900000, 3300000 },
>> + .reg = TPS65910_REG_VAUX2
>> +};
>> +
>> +static const struct regulator_props ldo_props_vaux33 = {
>> + .vin_min = { 2700000, 2700000, 3200000, 3600000 },
>> + .vout = { 1800000, 2000000, 2800000, 3300000 },
>> + .reg = TPS65910_REG_VAUX33
>> +};
>> +
>> +static const struct regulator_props ldo_props_vmmc = {
>> + .vin_min = { 2700000, 3200000, 3200000, 3600000 },
>> + .vout = { 1800000, 2800000, 3000000, 3300000 },
>> + .reg = TPS65910_REG_VMMC
>> +};
>> +
>> +static const struct regulator_props smps_props_vio = {
>> + .vin_min = { 3200000, 3200000, 4000000, 4400000 },
>> + .vout = { 1500000, 1800000, 2500000, 3300000 },
>> + .reg = TPS65910_REG_VIO
>> +};
>> +
>> +static int get_ctrl_reg_from_unit_addr(const int unit_addr)
>> +{
>> + switch (unit_addr) {
>> + case TPS65910_UNIT_VRTC:
>> + return TPS65910_REG_VRTC;
>> + case TPS65910_UNIT_VIO:
>> + return TPS65910_REG_VIO;
>> + case TPS65910_UNIT_VDD1:
>> + return TPS65910_REG_VDD1;
>> + case TPS65910_UNIT_VDD2:
>> + return TPS65910_REG_VDD2;
>> + case TPS65910_UNIT_VDD3:
>> + return TPS65910_REG_VDD3;
>> + case TPS65910_UNIT_VDIG1:
>> + return TPS65910_REG_VDIG1;
>> + case TPS65910_UNIT_VDIG2:
>> + return TPS65910_REG_VDIG2;
>> + case TPS65910_UNIT_VPLL:
>> + return TPS65910_REG_VPLL;
>> + case TPS65910_UNIT_VDAC:
>> + return TPS65910_REG_VDAC;
>> + case TPS65910_UNIT_VAUX1:
>> + return TPS65910_REG_VAUX1;
>> + case TPS65910_UNIT_VAUX2:
>> + return TPS65910_REG_VAUX2;
>> + case TPS65910_UNIT_VAUX33:
>> + return TPS65910_REG_VAUX33;
>> + case TPS65910_UNIT_VMMC:
>> + return TPS65910_REG_VMMC;
>
> I'm guess this cannot be done with an array lookup?
>
It could be done, as the unit numbers are consecutive (for now) and
starting at 0. I don't like that piece of code either. Should I replace
it with a static array of constants? I'm just not sure ...
>> + }
>> +
>> + return -ENXIO;
>> +}
>> +
>> +static int simple_regulator_get_value(struct udevice *dev,
>> + const struct regulator_props *rgp)
>
> Can you rename this to have the same prefix as the rest of your
> driver? Otherwise people might think it is a generic function.
>
Okay
>> +{
>> + int sel;
>> + u8 val;
>> + 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;
>> + 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;
>> + 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 simple_regulator_get_value(dev, &ldo_props_vdig1);
>> + case TPS65910_UNIT_VDIG2:
>> + return simple_regulator_get_value(dev, &ldo_props_vdig2);
>> + case TPS65910_UNIT_VPLL:
>> + return simple_regulator_get_value(dev, &ldo_props_vpll);
>> + case TPS65910_UNIT_VDAC:
>> + return simple_regulator_get_value(dev, &ldo_props_vdac);
>> + case TPS65910_UNIT_VAUX1:
>> + return simple_regulator_get_value(dev, &ldo_props_vaux1);
>> + case TPS65910_UNIT_VAUX2:
>> + return simple_regulator_get_value(dev, &ldo_props_vaux2);
>> + case TPS65910_UNIT_VAUX33:
>> + return simple_regulator_get_value(dev, &ldo_props_vaux33);
>> + case TPS65910_UNIT_VMMC:
>> + return simple_regulator_get_value(dev, &ldo_props_vmmc);
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static int simple_regulator_set_value(struct udevice *dev,
>> + const struct regulator_props *ldo,
>> + int uV)
>> +{
>> + u8 val;
>> + int sel = 0;
>> + struct tps65910_regulator_pdata *pdata = dev->platdata;
>> +
>> + do {
>> + /* we only allow exact voltage matches */
>> + if (uV == *(ldo->vout + sel))
>> + break;
>> + } while (++sel < VOUT_CHOICE_COUNT);
>> + if (sel == VOUT_CHOICE_COUNT)
>> + return -EINVAL;
>> + if (pdata->supply < *(ldo->vin_min + sel))
>> + return -EINVAL;
>> +
>> + val = pmic_reg_read(dev->parent, ldo->reg);
>> + if (val < 0)
>> + return val;
>> + val &= ~TPS65910_SEL_MASK;
>> + val |= sel << 2;
>> + return pmic_reg_write(dev->parent, ldo->reg, val);
>> +}
>> +
>> +static int tps65910_ldo_set_value(struct udevice *dev, int uV)
>> +{
>> + struct tps65910_regulator_pdata *pdata = dev->platdata;
>> + int vin = pdata->supply;
>> +
>> + switch (dev->driver_data) {
>> + case TPS65910_UNIT_VRTC:
>> + /* VRTC is fixed to 1.83V and can't be turned off */
>> + if (vin < 2500000)
>> + return -EINVAL;
>> + return 0;
>> + case TPS65910_UNIT_VDIG1:
>> + return simple_regulator_set_value(dev, &ldo_props_vdig1, uV);
>> + case TPS65910_UNIT_VDIG2:
>> + return simple_regulator_set_value(dev, &ldo_props_vdig2, uV);
>> + case TPS65910_UNIT_VPLL:
>> + return simple_regulator_set_value(dev, &ldo_props_vpll, uV);
>> + case TPS65910_UNIT_VDAC:
>> + return simple_regulator_set_value(dev, &ldo_props_vdac, uV);
>> + case TPS65910_UNIT_VAUX1:
>> + return simple_regulator_set_value(dev, &ldo_props_vaux1, uV);
>> + case TPS65910_UNIT_VAUX2:
>> + return simple_regulator_set_value(dev, &ldo_props_vaux2, uV);
>> + case TPS65910_UNIT_VAUX33:
>> + return simple_regulator_set_value(dev, &ldo_props_vaux33, uV);
>> + case TPS65910_UNIT_VMMC:
>> + return simple_regulator_set_value(dev, &ldo_props_vmmc, uV);
>> + }
>> +
>> + return 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);
>> + if (ret)
>> + return ret;
>> +
>> + /* bits 2:0 of regulator control register define state */
>> + return ((val & TPS65910_SUPPLY_STATE_MASK) == 1);
>> +}
>> +
>> +static int tps65910_set_enable(struct udevice *dev, bool enable)
>> +{
>> + int reg;
>> + uint clr, set;
>> +
>> + reg = get_ctrl_reg_from_unit_addr(dev->driver_data);
>> + if (reg < 0)
>> + return reg;
>> +
>> + if (enable) {
>> + clr = 0x02;
>> + set = 0x01;
>> + } else {
>> + clr = 0x03;
>> + set = 0x00;
>
> Do you not have defines / enums for these values?
>
Not yet, I'll try to invent some.
>> + }
>> + return pmic_clrsetbits(dev->parent, reg, clr, set);
>> +}
>> +
>> +static int tps65910_ldo_probe(struct udevice *dev)
>> +{
>> + int ret = 0;
>> + int unit = dev->driver_data;
>> + struct tps65910_pdata *pmic_pdata = dev->parent->platdata;
>> + struct tps65910_regulator_pdata *pdata = dev->platdata;
>> +
>> + switch (unit) {
>> + case TPS65910_UNIT_VRTC:
>> + pdata->supply = *(pmic_pdata->supply + TPS65910_SUPPLY_VCC7);
>> + break;
>> + case TPS65910_UNIT_VDIG1:
>> + case TPS65910_UNIT_VDIG2:
>> + pdata->supply = *(pmic_pdata->supply + TPS65910_SUPPLY_VCC6);
>> + break;
>> + case TPS65910_UNIT_VPLL:
>> + case TPS65910_UNIT_VDAC:
>> + pdata->supply = *(pmic_pdata->supply + TPS65910_SUPPLY_VCC5);
>> + break;
>> + case TPS65910_UNIT_VAUX1:
>> + case TPS65910_UNIT_VAUX2:
>> + pdata->supply = *(pmic_pdata->supply + TPS65910_SUPPLY_VCC4);
>> + break;
>> + case TPS65910_UNIT_VAUX33:
>> + case TPS65910_UNIT_VMMC:
>> + pdata->supply = *(pmic_pdata->supply + TPS65910_SUPPLY_VCC3);
>> + break;
>> + default:
>> + ret = -ENXIO;
>> + }
>> +
>> + return ret;
>> +}
>> +
>> +static int buck_get_vdd1_vdd2_value(struct udevice *dev, int reg_vdd)
>> +{
>> + int gain;
>> + u8 val = pmic_reg_read(dev, reg_vdd);
>> +
>> + if (val < 0)
>> + return val;
>> + gain = (val & TPS65910_GAIN_SEL_MASK) >> 6;
>> + gain = (gain == 0) ? 1 : gain;
>> + val = pmic_reg_read(dev, reg_vdd + 1);
>> + if (val < 0)
>> + return val;
>> + if (val & TPS65910_VDD_SR_MASK)
>> + /* use smart reflex value instead */
>> + val = pmic_reg_read(dev, reg_vdd + 2);
>> + if (val < 0)
>> + return val;
>> + return (562500 + (val & TPS65910_VDD_SEL_MASK) * 12500) * gain;
>> +}
>> +
>> +static int tps65910_buck_get_value(struct udevice *dev)
>> +{
>> + int vout = 0;
>> +
>> + switch (dev->driver_data) {
>> + case TPS65910_UNIT_VIO:
>> + vout = simple_regulator_get_value(dev, &smps_props_vio);
>> + break;
>> + case TPS65910_UNIT_VDD1:
>> + 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;
>> +}
>> +
>> +static int buck_set_vdd1_vdd2_value(struct udevice *dev, int uV)
>> +{
>> + int ret, reg_vdd, gain;
>> + u32 limit;
>> + int val;
>> +
>> + switch (dev->driver_data) {
>> + case TPS65910_UNIT_VDD1:
>> + reg_vdd = TPS65910_REG_VDD1;
>> + break;
>> + case TPS65910_UNIT_VDD2:
>> + reg_vdd = TPS65910_REG_VDD2;
>> + break;
>> + default:
>> + return -EINVAL;
>> + }
>> +
>> + /* check setpoint is within limits */
>> + ret = ofnode_read_u32(dev->node, "regulator-min-microvolt", &limit);
>
> This should be read at the start and stored somewhere. In general you
> should not be checking the device outside of ofdata_to_platdata() /
> probe().
>
Okay. I will use regulator's ofdata_to_platdata() then.
>> + if (ret) {
>> + /* too dangerous without limit */
>> + error("missing regulator-min-microvolt property for %s\n",
>> + dev->name);
>> + return ret;
>> + }
>> + if (uV < limit) {
>> + error("voltage %duV for %s too low\n",
>> + limit, dev->name);
>> + return -EINVAL;
>> + }
>> + ret = ofnode_read_u32(dev->node, "regulator-max-microvolt", &limit);
>> + if (ret) {
>> + /* too dangerous without limit */
>> + error("missing regulator-max-microvolt property for %s\n",
>> + dev->name);
>> + return ret;
>> + }
>> + if (uV > limit) {
>> + error("voltage %duV for %s too high\n",
>> + limit, dev->name);
>> + return -EINVAL;
>> + }
>> +
>> + val = pmic_reg_read(dev->parent, reg_vdd);
>> + if (val < 0)
>> + return val;
>> + gain = (val & TPS65910_GAIN_SEL_MASK) >> 6;
>> + gain = (gain == 0) ? 1 : gain;
>> + val = ((uV / gain) - 562500) / 12500;
>> + if ((val < 3) || (val > 75))
>
> You don't need all the brackets on this line.
>
Okay
>> + /* neither do we change the gain, nor do we allow shutdown or
>
> /*
> * Neither do we...
> * ...
> */
>
Okay
>> + * any approximate value (for now)
>> + */
>> + return -EPERM;
>> + val &= TPS65910_VDD_SEL_MASK;
>> + ret = pmic_reg_write(dev->parent, reg_vdd + 1, val);
>> + if (ret)
>> + return ret;
>> + return 0;
>> +}
>> +
>> +static int tps65910_buck_set_value(struct udevice *dev, int uV)
>> +{
>> + if (dev->driver_data == TPS65910_UNIT_VIO)
>> + return simple_regulator_set_value(dev, &smps_props_vio, uV);
>> +
>> + return buck_set_vdd1_vdd2_value(dev, uV);
>> +}
>> +
>> +static int tps65910_buck_probe(struct udevice *dev)
>> +{
>> + int ret = 0;
>> + int unit = dev->driver_data;
>> + struct tps65910_pdata *pmic_pdata = dev->parent->platdata;
>> + struct tps65910_regulator_pdata *pdata = dev->platdata;
>> +
>> + switch (unit) {
>> + case TPS65910_UNIT_VIO:
>> + pdata->supply = *(pmic_pdata->supply + TPS65910_SUPPLY_VCCIO);
>> + break;
>> + case TPS65910_UNIT_VDD1:
>> + pdata->supply = *(pmic_pdata->supply + TPS65910_SUPPLY_VCC1);
>> + break;
>> + case TPS65910_UNIT_VDD2:
>> + pdata->supply = *(pmic_pdata->supply + TPS65910_SUPPLY_VCC2);
>> + break;
>> + default:
>> + ret = -ENXIO;
>> + }
>> + return ret;
>> +}
>> +
>> +static int tps65910_boost_get_value(struct udevice *dev)
>> +{
>> + int vout;
>> + struct tps65910_regulator_pdata *pdata = dev->platdata;
>> +
>> + vout = (pdata->supply >= 3000000) ? 5000000 : 0;
>> + return vout;
>> +}
>> +
>> +static int tps65910_boost_probe(struct udevice *dev)
>> +{
>> + int unit = dev->driver_data;
>> + struct tps65910_pdata *pmic_pdata = dev->parent->platdata;
>> + struct tps65910_regulator_pdata *pdata = dev->platdata;
>> +
>> + if (unit != TPS65910_UNIT_VDD3)
>> + return -ENXIO;
>> +
>> + pdata->supply = *(pmic_pdata->supply + TPS65910_SUPPLY_VCC7);
>> + return 0;
>> +}
>> +
>> +static const struct dm_regulator_ops tps65910_boost_ops = {
>> + .get_value = tps65910_boost_get_value,
>> + .get_enable = tps65910_get_enable,
>> + .set_enable = tps65910_set_enable,
>> +};
>> +
>> +U_BOOT_DRIVER(tps65910_boost) = {
>> + .name = TPS65910_BOOST_DRIVER,
>> + .id = UCLASS_REGULATOR,
>> + .ops = &tps65910_boost_ops,
>> + .probe = tps65910_boost_probe,
>> + .platdata_auto_alloc_size = sizeof(struct tps65910_regulator_pdata),
>> +};
>> +
>> +static const struct dm_regulator_ops tps65910_buck_ops = {
>> + .get_value = tps65910_buck_get_value,
>> + .set_value = tps65910_buck_set_value,
>> + .get_enable = tps65910_get_enable,
>> + .set_enable = tps65910_set_enable,
>> +};
>> +
>> +U_BOOT_DRIVER(tps65910_buck) = {
>> + .name = TPS65910_BUCK_DRIVER,
>> + .id = UCLASS_REGULATOR,
>> + .ops = &tps65910_buck_ops,
>> + .probe = tps65910_buck_probe,
>> + .platdata_auto_alloc_size = sizeof(struct tps65910_regulator_pdata),
>> +};
>> +
>> +static const struct dm_regulator_ops tps65910_ldo_ops = {
>> + .get_value = tps65910_ldo_get_value,
>> + .set_value = tps65910_ldo_set_value,
>> + .get_enable = tps65910_get_enable,
>> + .set_enable = tps65910_set_enable,
>> +};
>> +
>> +U_BOOT_DRIVER(tps65910_ldo) = {
>> + .name = TPS65910_LDO_DRIVER,
>> + .id = UCLASS_REGULATOR,
>> + .ops = &tps65910_ldo_ops,
>> + .probe = tps65910_ldo_probe,
>> + .platdata_auto_alloc_size = sizeof(struct tps65910_regulator_pdata),
>> +};
>> diff --git a/include/power/tps65910_pmic.h b/include/power/tps65910_pmic.h
>> new file mode 100644
>> index 0000000..23e031e
>> --- /dev/null
>> +++ b/include/power/tps65910_pmic.h
>> @@ -0,0 +1,130 @@
>> +/*
>> + * Copyright (C) EETS GmbH, 2017, Felix Brack <f.brack at eets.ch>
>> + *
>> + * SPDX-License-Identifier: GPL-2.0+
>> + */
>> +
>> +#ifndef __TPS65910_PMIC_H_
>> +#define __TPS65910_PMIC_H_
>> +
>> +#define TPS65910_I2C_SEL_MASK (0x1 << 4)
>> +#define TPS65910_VDD_SR_MASK (0x1 << 7)
>> +#define TPS65910_GAIN_SEL_MASK (0x3 << 6)
>> +#define TPS65910_VDD_SEL_MASK (0x7f)
>> +#define TPS65910_SEL_MASK (0x3 << 2)
>> +#define TPS65910_SUPPLY_STATE_MASK (0x3)
>
> Might be easier to read if you tab out the values.
>
Sure, okay.
>> +
>> +/* i2c registers */
>> +enum {
>> + TPS65910_REG_RTC_SEC = 0x00,
>> + TPS65910_REG_RTC_MIN,
>> + TPS65910_REG_RTC_HOUR,
>> + TPS65910_REG_RTC_DAY,
>> + TPS65910_REG_RTC_MONTH,
>> + TPS65910_REG_RTC_YEAR,
>> + TPS65910_REG_RTC_WEEK,
>> + TPS65910_REG_RTC_ALARM_SEC = 0x08,
>> + TPS65910_REG_RTC_ALARM_MIN,
>> + TPS65910_REG_RTC_ALARM_HOUR,
>> + TPS65910_REG_RTC_ALARM_DAY,
>> + TPS65910_REG_RTC_ALARM_MONTH,
>> + TPS65910_REG_RTC_ALARM_YEAR,
>> + TPS65910_REG_RTC_CTRL = 0x10,
>> + TPS65910_REG_RTC_STAT,
>> + TPS65910_REG_RTC_INT,
>> + TPS65910_REG_RTC_COMP_LSB,
>> + TPS65910_REG_RTC_COMP_MSB,
>> + TPS65910_REG_RTC_RESISTOR_PRG,
>> + TPS65910_REG_RTC_RESET_STAT,
>> + TPS65910_REG_BACKUP1,
>> + TPS65910_REG_BACKUP2,
>> + TPS65910_REG_BACKUP3,
>> + TPS65910_REG_BACKUP4,
>> + TPS65910_REG_BACKUP5,
>> + TPS65910_REG_PUADEN,
>> + TPS65910_REG_REF,
>> + TPS65910_REG_VRTC,
>> + TPS65910_REG_VIO = 0x20,
>> + TPS65910_REG_VDD1,
>> + TPS65910_REG_VDD1_VAL,
>> + TPS65910_REG_VDD1_VAL_SR,
>> + TPS65910_REG_VDD2,
>> + TPS65910_REG_VDD2_VAL,
>> + TPS65910_REG_VDD2_VAL_SR,
>> + TPS65910_REG_VDD3,
>> + TPS65910_REG_VDIG1 = 0x30,
>> + TPS65910_REG_VDIG2,
>> + TPS65910_REG_VAUX1,
>> + TPS65910_REG_VAUX2,
>> + TPS65910_REG_VAUX33,
>> + TPS65910_REG_VMMC,
>> + TPS65910_REG_VPLL,
>> + TPS65910_REG_VDAC,
>> + TPS65910_REG_THERM,
>> + TPS65910_REG_BATTERY_BACKUP_CHARGE,
>> + TPS65910_REG_DCDC_CTRL = 0x3e,
>> + TPS65910_REG_DEVICE_CTRL,
>> + TPS65910_REG_DEVICE_CTRL2,
>> + TPS65910_REG_SLEEP_KEEP_LDO_ON,
>> + TPS65910_REG_SLEEP_KEEP_RES_ON,
>> + TPS65910_REG_SLEEP_SET_LDO_OFF,
>> + TPS65910_REG_SLEEP_SET_RES_OFF,
>> + TPS65910_REG_EN1_LDO_ASS,
>> + TPS65910_REG_EM1_SMPS_ASS,
>> + TPS65910_REG_EN2_LDO_ASS,
>> + TPS65910_REG_EM2_SMPS_ASS,
>> + TPS65910_REG_INT_STAT = 0x50,
>> + TPS65910_REG_INT_MASK,
>> + TPS65910_REG_INT_STAT2,
>> + TPS65910_REG_INT_MASK2,
>> + TPS65910_REG_GPIO = 0x60,
>> + TPS65910_REG_JTAGREVNUM = 0x80,
>> + TPS65910_NUM_REGS
>> +};
>> +
>> +/* chip supplies */
>> +enum {
>> + TPS65910_SUPPLY_VCCIO = 0x00,
>> + TPS65910_SUPPLY_VCC1,
>> + TPS65910_SUPPLY_VCC2,
>> + TPS65910_SUPPLY_VCC3,
>> + TPS65910_SUPPLY_VCC4,
>> + TPS65910_SUPPLY_VCC5,
>> + TPS65910_SUPPLY_VCC6,
>> + TPS65910_SUPPLY_VCC7,
>> + TPS65910_NUM_SUPPLIES
>> +};
>> +
>> +/* regulator unit numbers */
>> +enum {
>> + TPS65910_UNIT_VRTC = 0x00,
>> + TPS65910_UNIT_VIO,
>> + TPS65910_UNIT_VDD1,
>> + TPS65910_UNIT_VDD2,
>> + TPS65910_UNIT_VDD3,
>> + TPS65910_UNIT_VDIG1,
>> + TPS65910_UNIT_VDIG2,
>> + TPS65910_UNIT_VPLL,
>> + TPS65910_UNIT_VDAC,
>> + TPS65910_UNIT_VAUX1,
>> + TPS65910_UNIT_VAUX2,
>> + TPS65910_UNIT_VAUX33,
>> + TPS65910_UNIT_VMMC,
>> + TPS65910_UNIT_VBB,
>> +};
>> +
>> +/* platform data */
>> +struct tps65910_pdata {
>> + u32 supply[TPS65910_NUM_SUPPLIES]; /* regulator supply voltage in uV */
>> +};
>
> Is this used outside the driver? Do you need one driver to access
> another's platform data? That seems dodgy.
>
No. You are right: no need to place it in the header file. I will move
it to pmic_tps65910_dm.c file.
>> +
>> +struct tps65910_regulator_pdata {
>> + u32 supply; /* regulator supply voltage in uV */
>> +};
>> +
>> +/* driver names */
>> +#define TPS65910_BUCK_DRIVER "tps65910_buck"
>> +#define TPS65910_BOOST_DRIVER "tps65910_boost"
>> +#define TPS65910_LDO_DRIVER "tps65910_ldo"
>> +
>> + #endif /* __TPS65910_PMIC_H_ */
>> --
>> 2.7.4
>>
>
> Regards,
> Simon
>
Regards, Felix
More information about the U-Boot
mailing list