On Wednesday 07 June 2017 02:38 AM, Simon Glass wrote:

Hi,

On 1 June 2017 at 23:19, Keerthy j-keerthy@ti.com wrote:

...

Add support to bind the regulators/child nodes with the pmic.

Signed-off-by: Keerthy j-keerthy@ti.com

drivers/power/pmic/Kconfig | 7 ++++ drivers/power/pmic/Makefile | 1 + drivers/power/pmic/lp87565.c | 85 ++++++++++++++++++++++++++++++++++++++++++++ include/power/lp87565.h | 12 +++++++ 4 files changed, 105 insertions(+) create mode 100644 drivers/power/pmic/lp87565.c create mode 100644 include/power/lp87565.h

diff --git a/drivers/power/pmic/Kconfig b/drivers/power/pmic/Kconfig index 3f50c12..e3f9e4d 100644 --- a/drivers/power/pmic/Kconfig +++ b/drivers/power/pmic/Kconfig @@ -188,6 +188,13 @@ config PMIC_LP873X The LP873X is a PMIC containing couple of LDOs and couple of SMPS. This driver binds the pmic children.

+config PMIC_LP87565

•   bool "Enable driver for Texas Instruments LP87565 PMIC"
  

•   depends on DM_PMIC
  

•   ---help---
  

•   The LP87565 is a PMIC containing a bunch of SMPS.
  

•   This driver binds the pmic children.
  

config POWER_MC34VR500 bool "Enable driver for Freescale MC34VR500 PMIC" ---help--- diff --git a/drivers/power/pmic/Makefile b/drivers/power/pmic/Makefile index f409e3a..f488799 100644 --- a/drivers/power/pmic/Makefile +++ b/drivers/power/pmic/Makefile @@ -21,6 +21,7 @@ obj-$(CONFIG_PMIC_TPS65090) += tps65090.o obj-$(CONFIG_PMIC_S5M8767) += s5m8767.o obj-$(CONFIG_$(SPL_)PMIC_PALMAS) += palmas.o obj-$(CONFIG_$(SPL_)PMIC_LP873X) += lp873x.o +obj-$(CONFIG_$(SPL_)PMIC_LP87565) += lp87565.o

obj-$(CONFIG_POWER_LTC3676) += pmic_ltc3676.o obj-$(CONFIG_POWER_MAX77696) += pmic_max77696.o diff --git a/drivers/power/pmic/lp87565.c b/drivers/power/pmic/lp87565.c new file mode 100644 index 0000000..e82bb4a --- /dev/null +++ b/drivers/power/pmic/lp87565.c @@ -0,0 +1,85 @@ +/*

• • (C) Copyright 2017 Texas Instruments Incorporated, <www.ti.com>
• • Keerthy j-keerthy@ti.com
• • SPDX-License-Identifier: GPL-2.0+
• */

+#include <common.h> +#include <fdtdec.h> +#include <errno.h> +#include <dm.h> +#include <i2c.h> +#include <power/pmic.h> +#include <power/regulator.h> +#include <power/lp87565.h> +#include <dm/device.h>

+DECLARE_GLOBAL_DATA_PTR;

+static const struct pmic_child_info pmic_children_info[] = {

•   { .prefix = "buck", .driver = LP87565_BUCK_DRIVER },
  

•   { },
  

+};

+static int lp87565_write(struct udevice *dev, uint reg, const uint8_t *buff,

•                     int len)
  

+{

•   if (dm_i2c_write(dev, reg, buff, len)) {
  

•           error("write error to device: %p register: %#x!", dev, reg);
  

•           return -EIO;
  

•   }
  

•   return 0;
  

+}

+static int lp87565_read(struct udevice *dev, uint reg, uint8_t *buff, int len) +{

•   if (dm_i2c_read(dev, reg, buff, len)) {
  

•           error("read error from device: %p register: %#x!", dev, reg);
  

•           return -EIO;
  

•   }
  

•   return 0;
  

+}

+static int lp87565_bind(struct udevice *dev) +{

•   int regulators_node;
  

•   const void *blob = gd->fdt_blob;
  

•   int children;
  

•   int node = dev->of_offset;
  

•   regulators_node = fdt_subnode_offset(blob, node, "regulators");
  

Can you please update this to livetree? See rk8xx for an example.

Okay i believe using:

regulators_node = dev_read_subnode(dev, "regulators");

I will do that.

...

•   if (regulators_node <= 0) {
  

•           printf("%s: %s reg subnode not found!", __func__, dev->name);
  

•           return -ENXIO;
  

•   }
  

•   children = pmic_bind_children(dev, regulators_node, pmic_children_info);
  

•   if (!children)
  

•           printf("%s: %s - no child found\n", __func__, dev->name);
  

•   /* Always return success for this device */
  

•   return 0;
  

+}

+static struct dm_pmic_ops lp87565_ops = {

•   .read = lp87565_read,
  

•   .write = lp87565_write,
  

+};

+static const struct udevice_id lp87565_ids[] = {

•   { .compatible = "ti,lp87565", .data = LP87565 },
  

•   { .compatible = "ti,lp87565-q1", .data = LP87565_Q1 },
  

•   { }
  

+};

+U_BOOT_DRIVER(pmic_lp87565) = {

•   .name = "lp87565_pmic",
  

•   .id = UCLASS_PMIC,
  

•   .of_match = lp87565_ids,
  

•   .bind = lp87565_bind,
  

•   .ops = &lp87565_ops,
  

+}; diff --git a/include/power/lp87565.h b/include/power/lp87565.h new file mode 100644 index 0000000..48e1386 --- /dev/null +++ b/include/power/lp87565.h @@ -0,0 +1,12 @@ +#define LP87565 0x0 +#define LP87565_Q1 0x1

+#define LP87565_BUCK_NUM 6

+/* Drivers name */ +#define LP87565_BUCK_DRIVER "lp87565_buck"

+#define LP87565_BUCK_VOLT_MASK 0xFF +#define LP87565_BUCK_VOLT_MAX_HEX 0xFF +#define LP87565_BUCK_VOLT_MAX 3360000

+#define LP87565_BUCK_MODE_MASK 0x80

1.9.1

Regards, Simon

Hi Keerthy,

On 1 June 2017 at 23:19, Keerthy j-keerthy@ti.com wrote:

The driver provides regulator set/get voltage enable/disable functions for lp87565 family of PMICs.

Signed-off-by: Keerthy j-keerthy@ti.com

drivers/power/regulator/Kconfig | 8 ++ drivers/power/regulator/Makefile | 1 + drivers/power/regulator/lp87565_regulator.c | 199 ++++++++++++++++++++++++++++ 3 files changed, 208 insertions(+) create mode 100644 drivers/power/regulator/lp87565_regulator.c

diff --git a/drivers/power/regulator/Kconfig b/drivers/power/regulator/Kconfig index ef057e0..203292f 100644 --- a/drivers/power/regulator/Kconfig +++ b/drivers/power/regulator/Kconfig @@ -149,3 +149,11 @@ config DM_REGULATOR_LP873X This enables implementation of driver-model regulator uclass features for REGULATOR LP873X and the family of LP873X PMICs. The driver implements get/set api for: value and enable.

+config DM_REGULATOR_LP87565

•   bool "Enable driver for LP87565 PMIC regulators"
  

•    depends on PMIC_LP87565
  

•   ---help---
  

•   This enables implementation of driver-model regulator uclass
  

•   features for REGULATOR LP87565 and the family of LP87565 PMICs.
  

•   The driver implements get/set api for value and enable.
  

What regulators are available? LDOs / bucks ?

diff --git a/drivers/power/regulator/Makefile b/drivers/power/regulator/Makefile index 3e01021..ce14d08 100644 --- a/drivers/power/regulator/Makefile +++ b/drivers/power/regulator/Makefile @@ -18,3 +18,4 @@ obj-$(CONFIG_DM_REGULATOR_SANDBOX) += sandbox.o 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 diff --git a/drivers/power/regulator/lp87565_regulator.c b/drivers/power/regulator/lp87565_regulator.c new file mode 100644 index 0000000..f6a552e --- /dev/null +++ b/drivers/power/regulator/lp87565_regulator.c @@ -0,0 +1,199 @@ +/*

• • (C) Copyright 2016
• • Texas Instruments Incorporated, <www.ti.com>
• • Keerthy j-keerthy@ti.com
• • SPDX-License-Identifier: GPL-2.0+
• */

+#include <common.h> +#include <fdtdec.h> +#include <errno.h> +#include <dm.h> +#include <i2c.h> +#include <power/pmic.h> +#include <power/regulator.h> +#include <power/lp87565.h>

+DECLARE_GLOBAL_DATA_PTR;

+static const char lp87565_buck_ctrl1[LP87565_BUCK_NUM] = {0x2, 0x4, 0x6, 0x8, 0x2, 0x6}; +static const char lp87565_buck_vout[LP87565_BUCK_NUM] = {0xA, 0xC, 0xE, 0x10, 0xA, 0xE };

+static int lp87565_buck_enable(struct udevice *dev, int op, bool *enable) +{

•   int ret;
  

•   unsigned int adr;
  

•   struct dm_regulator_uclass_platdata *uc_pdata;
  

•   uc_pdata = dev_get_uclass_platdata(dev);
  

•   adr = uc_pdata->ctrl_reg;
  

•   ret = pmic_reg_read(dev->parent, adr);
  

•   if (ret < 0)
  

•           return ret;
  

•   if (op == PMIC_OP_GET) {
  

•           ret &= LP87565_BUCK_MODE_MASK;
  

•           if (ret)
  

•                   *enable = true;
  

•           else
  

•                   *enable = false;
  

•           return 0;
  

•   } else if (op == PMIC_OP_SET) {
  

•           if (*enable)
  

•                   ret |= LP87565_BUCK_MODE_MASK;
  

•           else
  

•                   ret &= ~(LP87565_BUCK_MODE_MASK);
  

Drop ()

•           ret = pmic_reg_write(dev->parent, adr, ret);
  

•           if (ret)
  

•                   return ret;
  

•   }
  

•   return 0;
  

+}

+static int lp87565_buck_volt2hex(int uV)

I'm a bit unsure about the word 'hex' here. Would it be better to use 'val'?

+{

•   if (uV > LP87565_BUCK_VOLT_MAX)
  

•           return -EINVAL;
  

•   else if (uV > 1400000)
  

•           return (uV - 1420000) / 20000 + 0x9E;
  

•   else if (uV > 730000)
  

•           return (uV - 735000) / 5000 + 0x18;
  

•   else if (uV >= 500000)
  

•           return (uV - 500000) / 10000;
  

•   else
  

•           return -EINVAL;
  

+}

+static int lp87565_buck_hex2volt(int hex) +{

•   if (hex > LP87565_BUCK_VOLT_MAX_HEX)
  

•           return -EINVAL;
  

•   else if (hex > 0x9D)
  

•           return 1400000 + (hex - 0x9D) * 20000;
  

•   else if (hex > 0x17)
  

•           return 730000 + (hex - 0x17) * 5000;
  

•   else if (hex >= 0x0)
  

•           return 500000 + hex * 10000;
  

•   else
  

•           return -EINVAL;
  

+}

+static int lp87565_buck_val(struct udevice *dev, int op, int *uV) +{

•   unsigned int hex, adr;
  

•   int ret;
  

•   struct dm_regulator_uclass_platdata *uc_pdata;
  

•   uc_pdata = dev_get_uclass_platdata(dev);
  

•   if (op == PMIC_OP_GET)
  

•           *uV = 0;
  

•   adr = uc_pdata->volt_reg;
  

•   ret = pmic_reg_read(dev->parent, adr);
  

•   if (ret < 0)
  

•           return ret;
  

•   if (op == PMIC_OP_GET) {
  

•           ret &= LP87565_BUCK_VOLT_MASK;
  

•           ret = lp87565_buck_hex2volt(ret);
  

•           if (ret < 0)
  

•                   return ret;
  

•           *uV = ret;
  

•           return 0;
  

•   }
  

•   hex = lp87565_buck_volt2hex(*uV);
  

•   if (hex < 0)
  

•           return hex;
  

•   ret &= 0x0;
  

•   ret |= hex;
  

Can you just use ret = hex ?

•   ret = pmic_reg_write(dev->parent, adr, ret);
  

•   return ret;
  

+}

+static int lp87565_buck_probe(struct udevice *dev) +{

•   struct dm_regulator_uclass_platdata *uc_pdata;
  

•   int idx;
  

•   uc_pdata = dev_get_uclass_platdata(dev);
  

•   uc_pdata->type = REGULATOR_TYPE_BUCK;
  

•   idx = dev->driver_data;
  

•   if (idx == 0 || idx == 1 || idx == 2 || idx == 3) {
  

•           debug("Single phase regulator\n");
  

•   } else if (idx == 23) {
  

•           idx = 5;
  

•   } else if (idx == 10) {
  

•           idx = 4;
  

•   } else {
  

•           printf("Wrong ID for regulator\n");
  

•           return -1;
  

-EINVAL or something like that. -1 is -EPERM

•   }
  

•   uc_pdata->ctrl_reg = lp87565_buck_ctrl1[idx];
  

•   uc_pdata->volt_reg = lp87565_buck_vout[idx];
  

•   return 0;
  

+}

+static int buck_get_value(struct udevice *dev) +{

•   int uV;
  

•   int ret;
  

•   ret = lp87565_buck_val(dev, PMIC_OP_GET, &uV);
  

•   if (ret)
  

•           return ret;
  

•   return uV;
  

+}

+static int buck_set_value(struct udevice *dev, int uV) +{

•   return lp87565_buck_val(dev, PMIC_OP_SET, &uV);
  

+}

+static bool buck_get_enable(struct udevice *dev) +{

•   bool enable = false;
  

•   int ret;
  

•   ret = lp87565_buck_enable(dev, PMIC_OP_GET, &enable);
  

•   if (ret)
  

•           return ret;
  

•   return enable;
  

+}

+static int buck_set_enable(struct udevice *dev, bool enable) +{

•   return lp87565_buck_enable(dev, PMIC_OP_SET, &enable);
  

+}

+static const struct dm_regulator_ops lp87565_buck_ops = {

•   .get_value  = buck_get_value,
  

•   .set_value  = buck_set_value,
  

•   .get_enable = buck_get_enable,
  

•   .set_enable = buck_set_enable,
  

+};

+U_BOOT_DRIVER(lp87565_buck) = {

•   .name = LP87565_BUCK_DRIVER,
  

•   .id = UCLASS_REGULATOR,
  

•   .ops = &lp87565_buck_ops,
  

•   .probe = lp87565_buck_probe,
  

+};

1.9.1

Regards, Simon