On Thu, 20 Jul 2023 at 02:48, Svyatoslav Ryhel <clamo...@gmail.com> wrote:
>
> Add support to bind the regulators/child nodes with the pmic.
> Also adds the pmic i2c based read/write functions to access pmic
> registers.
>
> Signed-off-by: Svyatoslav Ryhel <clamo...@gmail.com>
> ---
> doc/device-tree-bindings/pmic/tps65911.txt | 78 ++++++++++++++++++++++
> drivers/power/pmic/pmic_tps65910_dm.c | 49 +++++++++++++-
> include/power/tps65910_pmic.h | 52 +++++++++++++++
> 3 files changed, 176 insertions(+), 3 deletions(-)
> create mode 100644 doc/device-tree-bindings/pmic/tps65911.txt
>
Reviewed-by: Simon Glass <s...@chromium.org>
nits below
[..]
> diff --git a/drivers/power/pmic/pmic_tps65910_dm.c
> b/drivers/power/pmic/pmic_tps65910_dm.c
> index e03ddc98d7..770010d53d 100644
> --- a/drivers/power/pmic/pmic_tps65910_dm.c
> +++ b/drivers/power/pmic/pmic_tps65910_dm.c
> @@ -11,13 +11,19 @@
> #include <power/regulator.h>
> #include <power/tps65910_pmic.h>
>
> -static const struct pmic_child_info pmic_children_info[] = {
> +static const struct pmic_child_info tps65910_children_info[] = {
> { .prefix = "ldo_", .driver = TPS65910_LDO_DRIVER },
> { .prefix = "buck_", .driver = TPS65910_BUCK_DRIVER },
> { .prefix = "boost_", .driver = TPS65910_BOOST_DRIVER },
> { },
> };
>
> +static const struct pmic_child_info tps65911_children_info[] = {
> + { .prefix = "ldo", .driver = TPS65911_LDO_DRIVER },
> + { .prefix = "vdd", .driver = TPS65911_VDD_DRIVER },
> + { },
> +};
> +
> static int pmic_tps65910_reg_count(struct udevice *dev)
> {
> return TPS65910_NUM_REGS;
> @@ -47,10 +53,29 @@ static int pmic_tps65910_read(struct udevice *dev, uint
> reg, u8 *buffer,
> return ret;
> }
>
> +static int pmic_tps65910_poweroff(struct udevice *dev)
> +{
> + int ret;
> +
> + ret = pmic_reg_read(dev, TPS65910_REG_DEVICE_CTRL);
> + if (ret < 0)
> + return ret;
> +
> + ret |= DEVCTRL_PWR_OFF_MASK;
Please use a separate var for this. We use ret for return values, as
you have above.
> +
> + pmic_reg_write(dev, TPS65910_REG_DEVICE_CTRL, ret);
> +
> + ret |= DEVCTRL_DEV_OFF_MASK;
> + ret &= ~DEVCTRL_DEV_ON_MASK;
> +
> + return pmic_reg_write(dev, TPS65910_REG_DEVICE_CTRL, ret);
> +}
> +
> static int pmic_tps65910_bind(struct udevice *dev)
> {
> ofnode regulators_node;
> int children;
> + int type = dev_get_driver_data(dev);
>
> regulators_node = dev_read_subnode(dev, "regulators");
> if (!ofnode_valid(regulators_node)) {
> @@ -58,7 +83,19 @@ static int pmic_tps65910_bind(struct udevice *dev)
> return -EINVAL;
> }
>
> - children = pmic_bind_children(dev, regulators_node,
> pmic_children_info);
> + switch (type) {
Why not always binding them?
> + case TPS65910:
> + children = pmic_bind_children(dev, regulators_node,
> + tps65910_children_info);
> + break;
> + case TPS65911:
> + children = pmic_bind_children(dev, regulators_node,
> + tps65911_children_info);
> + break;
> + default:
> + log_err("unknown PMIC type\n");
> + }
> +
> if (!children)
> debug("%s has no children (regulators)\n", dev->name);
>
> @@ -67,6 +104,10 @@ static int pmic_tps65910_bind(struct udevice *dev)
>
> static int pmic_tps65910_probe(struct udevice *dev)
> {
> + struct uc_pmic_priv *priv = dev_get_uclass_priv(dev);
> +
> + priv->sys_pow_ctrl = dev_read_bool(dev, "ti,system-power-controller");
> +
> /* use I2C control interface instead of I2C smartreflex interface to
> * access smartrefelex registers VDD1_OP_REG, VDD1_SR_REG, VDD2_OP_REG
> * and VDD2_SR_REG
> @@ -76,13 +117,15 @@ static int pmic_tps65910_probe(struct udevice *dev)
> }
>
> static struct dm_pmic_ops pmic_tps65910_ops = {
> + .poweroff = pmic_tps65910_poweroff,
> .reg_count = pmic_tps65910_reg_count,
> .read = pmic_tps65910_read,
> .write = pmic_tps65910_write,
> };
>
> static const struct udevice_id pmic_tps65910_match[] = {
> - { .compatible = "ti,tps65910" },
> + { .compatible = "ti,tps65910", .data = TPS65910 },
> + { .compatible = "ti,tps65911", .data = TPS65911 },
> { /* sentinel */ }
> };
>
> diff --git a/include/power/tps65910_pmic.h b/include/power/tps65910_pmic.h
> index 66214786d3..282863805a 100644
> --- a/include/power/tps65910_pmic.h
> +++ b/include/power/tps65910_pmic.h
> @@ -6,6 +6,9 @@
> #ifndef __TPS65910_PMIC_H_
> #define __TPS65910_PMIC_H_
>
> +#define TPS65910 1
> +#define TPS65911 2
> +
> #define TPS65910_I2C_SEL_MASK (0x1 << 4)
> #define TPS65910_VDD_SR_MASK (0x1 << 7)
> #define TPS65910_GAIN_SEL_MASK (0x3 << 6)
> @@ -17,6 +20,11 @@
> #define TPS65910_SUPPLY_STATE_OFF 0x0
> #define TPS65910_SUPPLY_STATE_ON 0x1
>
> +/* TPS65910 DEVICE_CTRL bits */
> +#define DEVCTRL_PWR_OFF_MASK BIT(7)
> +#define DEVCTRL_DEV_ON_MASK BIT(2)
> +#define DEVCTRL_DEV_OFF_MASK BIT(0)
> +
> /* i2c registers */
> enum {
> TPS65910_REG_RTC_SEC = 0x00,
> @@ -126,4 +134,48 @@ struct tps65910_regulator_pdata {
> #define TPS65910_BOOST_DRIVER "tps65910_boost"
> #define TPS65910_LDO_DRIVER "tps65910_ldo"
>
> +/* tps65911 i2c registers */
> +enum {
> + TPS65911_REG_VIO = 0x20,
> + TPS65911_REG_VDD1,
> + TPS65911_REG_VDD1_OP,
> + TPS65911_REG_VDD1_SR,
> + TPS65911_REG_VDD2,
> + TPS65911_REG_VDD2_OP,
> + TPS65911_REG_VDD2_SR,
> + TPS65911_REG_VDDCTRL,
> + TPS65911_REG_VDDCTRL_OP,
> + TPS65911_REG_VDDCTRL_SR,
> + TPS65911_REG_LDO1 = 0x30,
> + TPS65911_REG_LDO2,
> + TPS65911_REG_LDO5,
> + TPS65911_REG_LDO8,
> + TPS65911_REG_LDO7,
> + TPS65911_REG_LDO6,
> + TPS65911_REG_LDO4,
> + TPS65911_REG_LDO3,
> +};
> +
> +#define TPS65911_VDD_NUM 4
> +#define TPS65911_LDO_NUM 8
> +
> +#define TPS65911_VDD_VOLT_MAX 1500000
> +#define TPS65911_VDD_VOLT_MIN 600000
> +#define TPS65911_VDD_VOLT_BASE 562500
> +
> +#define TPS65911_LDO_VOLT_MAX 3300000
> +#define TPS65911_LDO_VOLT_BASE 800000
> +
> +#define TPS65911_LDO_SEL_MASK (0x3f << 2)
> +
> +#define TPS65911_LDO124_VOLT_MAX_HEX 0x32
> +#define TPS65911_LDO358_VOLT_MAX_HEX 0x19
> +#define TPS65911_LDO358_VOLT_MIN_HEX 0x02
> +
> +#define TPS65911_LDO124_VOLT_STEP 50000
> +#define TPS65911_LDO358_VOLT_STEP 100000
> +
> +#define TPS65911_VDD_DRIVER "tps65911_vdd"
> +#define TPS65911_LDO_DRIVER "tps65911_ldo"
Drop the TPS65911 prefixes if you can...this is only included in one
file i think.
> +
> #endif /* __TPS65910_PMIC_H_ */
> --
> 2.39.2
>
Regards,
Simon
