[U-Boot] [PATCH] power: pmic/regulator: Add basic support for TPS65910

Simon Glass sjg at chromium.org
Mon Nov 20 15:38:56 UTC 2017


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.

[...]

> 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.

> 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?

> +       }
> +
> +       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.

> +{
> +       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?

> +       }
> +       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().

> +       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.

> +               /* neither do we change the gain, nor do we allow shutdown or

/*
 * Neither do we...
 * ...
 */

> +                *  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.

> +
> +/* 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.

> +
> +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


More information about the U-Boot mailing list