Re: [PATCH 3/4] Regulators: TPS65218: Add Regulator driver for TPS65218 PMIC

2013-12-03 Thread Mark Brown
On Tue, Dec 03, 2013 at 12:13:24PM +0530, Keerthy wrote:

 +static int tps65218_ldo1_dcdc3_vsel_to_uv(unsigned int vsel)
 +{
 + int uV = 0;
 +
 + if (vsel = 26)
 + uV = vsel * 25000 + 90;
 + else
 + uV = (vsel - 26) * 5 + 155;
 +
 + return uV;
 +}

Use regulator_map_voltage_linear_range() (and similarly for most of the
other functions).

 +static const struct of_device_id tps65218_of_match[] = {
 + TPS65218_OF_MATCH(ti,tps65218-dcdc1, tps65218_pmic_regs[0]),
 + TPS65218_OF_MATCH(ti,tps65218-dcdc2, tps65218_pmic_regs[1]),
 + TPS65218_OF_MATCH(ti,tps65218-dcdc3, tps65218_pmic_regs[2]),
 + TPS65218_OF_MATCH(ti,tps65218-dcdc4, tps65218_pmic_regs[3]),
 + TPS65218_OF_MATCH(ti,tps65218-dcdc5, tps65218_pmic_regs[4]),
 + TPS65218_OF_MATCH(ti,tps65218-dcdc6, tps65218_pmic_regs[5]),
 + TPS65218_OF_MATCH(ti,tps65218-ldo1, tps65218_pmic_regs[6]),
 +};
 +MODULE_DEVICE_TABLE(of, tps65218_of_match);

Indexing into another array by magic number like this is both error
prone and hard to read.  Either use defined constants or individual
variables for the things being referenced.


signature.asc
Description: Digital signature


Re: [PATCH 3/4] Regulators: TPS65218: Add Regulator driver for TPS65218 PMIC

2013-12-03 Thread Keerthy

Thanks for the review Mark.

On Tuesday 03 December 2013 08:46 PM, Mark Brown wrote:

On Tue, Dec 03, 2013 at 12:13:24PM +0530, Keerthy wrote:


+static int tps65218_ldo1_dcdc3_vsel_to_uv(unsigned int vsel)
+{
+   int uV = 0;
+
+   if (vsel = 26)
+   uV = vsel * 25000 + 90;
+   else
+   uV = (vsel - 26) * 5 + 155;
+
+   return uV;
+}

Use regulator_map_voltage_linear_range() (and similarly for most of the
other functions).


Ok.


+static const struct of_device_id tps65218_of_match[] = {
+   TPS65218_OF_MATCH(ti,tps65218-dcdc1, tps65218_pmic_regs[0]),
+   TPS65218_OF_MATCH(ti,tps65218-dcdc2, tps65218_pmic_regs[1]),
+   TPS65218_OF_MATCH(ti,tps65218-dcdc3, tps65218_pmic_regs[2]),
+   TPS65218_OF_MATCH(ti,tps65218-dcdc4, tps65218_pmic_regs[3]),
+   TPS65218_OF_MATCH(ti,tps65218-dcdc5, tps65218_pmic_regs[4]),
+   TPS65218_OF_MATCH(ti,tps65218-dcdc6, tps65218_pmic_regs[5]),
+   TPS65218_OF_MATCH(ti,tps65218-ldo1, tps65218_pmic_regs[6]),
+};
+MODULE_DEVICE_TABLE(of, tps65218_of_match);

Indexing into another array by magic number like this is both error
prone and hard to read.  Either use defined constants or individual
variables for the things being referenced.

Okay.

Regards,
Keerthy
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 3/4] Regulators: TPS65218: Add Regulator driver for TPS65218 PMIC

2013-12-03 Thread Manish Badarkhe
Hi Keerthy,

 +   rdev = regulator_register(regulators[id], config);

Can you make use of devm_regulator_register instead?

 +   if (IS_ERR(rdev)) {
 +   dev_err(tps-dev, failed to register %s regulator\n,
 +   pdev-name);
 +   return PTR_ERR(rdev);
 +   }
 +
 +   /* Save regulator */
 +   tps-rdev[id] = rdev;
 +
 +   return 0;
 +}


Best Regards
Manish Badarkhe
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 3/4] Regulators: TPS65218: Add Regulator driver for TPS65218 PMIC

2013-12-02 Thread Keerthy
This patch adds support for TPS65218 PMIC regulators.

The regulators set consists of 6 DCDCs and 1 LDO. The output
voltages are configurable and are meant to supply power to the
main processor and other components.

Signed-off-by: Keerthy j-keer...@ti.com
---
 drivers/regulator/Kconfig  |9 +
 drivers/regulator/Makefile |1 +
 drivers/regulator/tps65218-regulator.c |  393 
 3 files changed, 403 insertions(+)
 create mode 100644 drivers/regulator/tps65218-regulator.c

diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig
index ce785f4..42f94eb 100644
--- a/drivers/regulator/Kconfig
+++ b/drivers/regulator/Kconfig
@@ -498,6 +498,15 @@ config REGULATOR_TPS65217
  voltage regulators. It supports software based voltage control
  for different voltage domains
 
+config REGULATOR_TPS65218
+   tristate TI TPS65218 Power regulators
+   depends on MFD_TPS65218
+   help
+ This driver supports TPS65218 voltage regulator chips. TPS65218
+ provides six step-down converters and one general-purpose LDO
+ voltage regulators. It supports software based voltage control
+ for different voltage domains
+
 config REGULATOR_TPS6524X
tristate TI TPS6524X Power regulators
depends on SPI
diff --git a/drivers/regulator/Makefile b/drivers/regulator/Makefile
index 01c597e..dfbfdf9 100644
--- a/drivers/regulator/Makefile
+++ b/drivers/regulator/Makefile
@@ -65,6 +65,7 @@ obj-$(CONFIG_REGULATOR_TPS65023) += tps65023-regulator.o
 obj-$(CONFIG_REGULATOR_TPS6507X) += tps6507x-regulator.o
 obj-$(CONFIG_REGULATOR_TPS65090) += tps65090-regulator.o
 obj-$(CONFIG_REGULATOR_TPS65217) += tps65217-regulator.o
+obj-$(CONFIG_REGULATOR_TPS65218) += tps65218-regulator.o
 obj-$(CONFIG_REGULATOR_TPS6524X) += tps6524x-regulator.o
 obj-$(CONFIG_REGULATOR_TPS6586X) += tps6586x-regulator.o
 obj-$(CONFIG_REGULATOR_TPS65910) += tps65910-regulator.o
diff --git a/drivers/regulator/tps65218-regulator.c 
b/drivers/regulator/tps65218-regulator.c
new file mode 100644
index 000..4989c83
--- /dev/null
+++ b/drivers/regulator/tps65218-regulator.c
@@ -0,0 +1,393 @@
+/*
+ * tps65218-regulator.c
+ *
+ * Regulator driver for TPS65218 PMIC
+ *
+ * Copyright (C) 2013 Texas Instruments Incorporated - http://www.ti.com/
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed as is WITHOUT ANY WARRANTY of any
+ * kind, whether expressed or implied; without even the implied warranty
+ * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License version 2 for more details.
+ */
+
+#include linux/kernel.h
+#include linux/module.h
+#include linux/device.h
+#include linux/init.h
+#include linux/err.h
+#include linux/platform_device.h
+#include linux/of_device.h
+#include linux/regulator/of_regulator.h
+#include linux/regulator/driver.h
+#include linux/regulator/machine.h
+#include linux/mfd/tps65218.h
+
+static unsigned int tps65218_ramp_delay = 4000;
+
+#define TPS65218_REGULATOR(_name, _id, _ops, _n, _vr, _vm, _er, _em, _t) \
+   {   \
+   .name   = _name,\
+   .id = _id,  \
+   .ops= _ops,\
+   .n_voltages = _n,   \
+   .type   = REGULATOR_VOLTAGE,\
+   .owner  = THIS_MODULE,  \
+   .vsel_reg   = _vr,  \
+   .vsel_mask  = _vm,  \
+   .enable_reg = _er,  \
+   .enable_mask= _em,  \
+   .volt_table = _t,   \
+   }   \
+
+#define TPS65218_INFO(_id, _nm, _min, _max, _f1, _f2)  \
+   {   \
+   .id = _id,  \
+   .name   = _nm,  \
+   .min_uV = _min, \
+   .max_uV = _max, \
+   .vsel_to_uv = _f1,  \
+   .uv_to_vsel = _f2,  \
+   }
+
+static int tps65218_ldo1_dcdc3_vsel_to_uv(unsigned int vsel)
+{
+   int uV = 0;
+
+   if (vsel = 26)
+   uV = vsel * 25000 + 90;
+   else
+   uV = (vsel - 26) * 5 + 155;
+
+   return uV;
+}
+
+static int tps65218_ldo1_dcdc3_uv_to_vsel(int uV, unsigned int *vsel)
+{
+   if (uV = 155)
+   *vsel = DIV_ROUND_UP(uV - 90, 25000);
+   else
+   *vsel = 26 + DIV_ROUND_UP(uV - 155, 5);
+
+   return 0;
+}
+