devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/4] mfd: TPS65218: Add support for TPS65218 PMIC
@ 2013-12-03  6:43 Keerthy
  2013-12-03  6:43 ` [PATCH 1/4] mfd: DT bindings " Keerthy
                   ` (3 more replies)
  0 siblings, 4 replies; 16+ messages in thread
From: Keerthy @ 2013-12-03  6:43 UTC (permalink / raw)
  To: rob.herring, pawel.moll, mark.rutland, swarren, ijc+devicetree,
	rob, sameo, lee.jones, grant.likely, lgirdwood, broonie
  Cc: j-keerthy, devicetree, linux-doc, linux-kernel, linux-omap

The TPS65218 chip is a power management IC for Portable Navigation Systems
and Tablet Computing devices. It contains the following components:

 - Regulators.
 - Over Temperature warning and Shut down.

This patch adds support for TPS65218 mfd device. At this time only
the regulator functionality is made available.

The patch series is dependent on:

http://www.spinics.net/lists/arm-kernel/msg289361.html

The series is boot tested on am43x-epos-evm board.

Keerthy (4):
  mfd: DT bindings for TPS65218 PMIC
  MFD: TPS65218: Add driver for the TPS65218 PMIC
  Regulators: TPS65218: Add Regulator driver for TPS65218 PMIC
  ARM: dts: AM437x: Add TPS65218 device tree nodes

 Documentation/devicetree/bindings/mfd/tps65218.txt |   27 ++
 .../devicetree/bindings/regulator/tps65218.txt     |   22 ++
 arch/arm/boot/dts/am43x-epos-evm.dts               |   41 ++
 drivers/mfd/Kconfig                                |   14 +
 drivers/mfd/Makefile                               |    1 +
 drivers/mfd/tps65218.c                             |  281 ++++++++++++++
 drivers/regulator/Kconfig                          |    9 +
 drivers/regulator/Makefile                         |    1 +
 drivers/regulator/tps65218-regulator.c             |  393 ++++++++++++++++++++
 include/linux/mfd/tps65218.h                       |  288 ++++++++++++++
 10 files changed, 1077 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mfd/tps65218.txt
 create mode 100644 Documentation/devicetree/bindings/regulator/tps65218.txt
 create mode 100644 drivers/mfd/tps65218.c
 create mode 100644 drivers/regulator/tps65218-regulator.c
 create mode 100644 include/linux/mfd/tps65218.h

-- 
1.7.9.5


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH 1/4] mfd: DT bindings for TPS65218 PMIC
  2013-12-03  6:43 [PATCH 0/4] mfd: TPS65218: Add support for TPS65218 PMIC Keerthy
@ 2013-12-03  6:43 ` Keerthy
  2013-12-03  9:36   ` Lee Jones
  2013-12-03  6:43 ` [PATCH 2/4] MFD: TPS65218: Add driver for the " Keerthy
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 16+ messages in thread
From: Keerthy @ 2013-12-03  6:43 UTC (permalink / raw)
  To: rob.herring, pawel.moll, mark.rutland, swarren, ijc+devicetree,
	rob, sameo, lee.jones, grant.likely, lgirdwood, broonie
  Cc: j-keerthy, devicetree, linux-doc, linux-kernel, linux-omap

Add DT bindings for TPS65218 PMIC.

Signed-off-by: Keerthy <j-keerthy@ti.com>
---
 Documentation/devicetree/bindings/mfd/tps65218.txt |   27 ++++++++++++++++++++
 .../devicetree/bindings/regulator/tps65218.txt     |   22 ++++++++++++++++
 2 files changed, 49 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mfd/tps65218.txt
 create mode 100644 Documentation/devicetree/bindings/regulator/tps65218.txt

diff --git a/Documentation/devicetree/bindings/mfd/tps65218.txt b/Documentation/devicetree/bindings/mfd/tps65218.txt
new file mode 100644
index 0000000..87cb7a8
--- /dev/null
+++ b/Documentation/devicetree/bindings/mfd/tps65218.txt
@@ -0,0 +1,27 @@
+The TPS65218 Integrated Power Management Chips.
+These chips are connected to an i2c bus.
+
+Required properties:
+- compatible : Must be "ti,tps65218";
+- interrupts : This i2c device has an IRQ line connected to the main SoC
+- interrupt-controller : Since the tps65218 support several interrupts
+  internally, it is considered as an interrupt controller cascaded to the SoC.
+- #interrupt-cells = <2>;
+- interrupt-parent : The parent interrupt controller.
+
+Optional node:
+- Child nodes contain in the tps65218.
+  It supports a number of features.
+  The children nodes will thus depend of the capability of the variant.
+
+Example:
+/*
+ * Integrated Power Management Chip
+ */
+tps@24 {
+    compatible = "ti,tps65218";
+    reg = <0x24>;
+    interrupt-controller;
+    #interrupt-cells = <2>;
+    interrupt-parent = <&gic>;
+};
diff --git a/Documentation/devicetree/bindings/regulator/tps65218.txt b/Documentation/devicetree/bindings/regulator/tps65218.txt
new file mode 100644
index 0000000..1ccf170
--- /dev/null
+++ b/Documentation/devicetree/bindings/regulator/tps65218.txt
@@ -0,0 +1,22 @@
+TPS65218 family of regulators
+
+Required properties:
+For tps65218 regulators/LDOs
+- compatible:
+  - "ti,tps65218-dcdc1" for DCDC1
+  - "ti,tps65218-dcdc2" for DCDC2
+  - "ti,tps65218-dcdc3" for DCDC3
+  - "ti,tps65218-dcdc4" for DCDC4
+  - "ti,tps65218-dcdc5" for DCDC5
+  - "ti,tps65218-dcdc6" for DCDC6
+  - "ti,tps65218-ldo1" for LDO1 LDO
+
+Optional properties:
+- Any optional property defined in bindings/regulator/regulator.txt
+
+Example:
+	xyz: regulator@0 {
+		compatible = "ti,tps65218-dcdc1";
+		regulator-min-microvolt  = <1000000>;
+		regulator-max-microvolt  = <3000000>;
+	};
-- 
1.7.9.5


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 2/4] MFD: TPS65218: Add driver for the TPS65218 PMIC
  2013-12-03  6:43 [PATCH 0/4] mfd: TPS65218: Add support for TPS65218 PMIC Keerthy
  2013-12-03  6:43 ` [PATCH 1/4] mfd: DT bindings " Keerthy
@ 2013-12-03  6:43 ` Keerthy
  2013-12-03  9:24   ` Lee Jones
  2013-12-03 15:19   ` Mark Brown
  2013-12-03  6:43 ` [PATCH 3/4] Regulators: TPS65218: Add Regulator driver for " Keerthy
  2013-12-03  6:43 ` [PATCH 4/4] ARM: dts: AM437x: Add TPS65218 device tree nodes Keerthy
  3 siblings, 2 replies; 16+ messages in thread
From: Keerthy @ 2013-12-03  6:43 UTC (permalink / raw)
  To: rob.herring, pawel.moll, mark.rutland, swarren, ijc+devicetree,
	rob, sameo, lee.jones, grant.likely, lgirdwood, broonie
  Cc: j-keerthy, devicetree, linux-doc, linux-kernel, linux-omap

The TPS65218 chip is a power management IC for Portable Navigation Systems
and Tablet Computing devices. It contains the following components:

 - Regulators.
 - Over Temperature warning and Shut down.

This patch adds support for tps65218 mfd device. At this time only
the regulator functionality is made available.

Signed-off-by: Keerthy <j-keerthy@ti.com>
---
 drivers/mfd/Kconfig          |   14 ++
 drivers/mfd/Makefile         |    1 +
 drivers/mfd/tps65218.c       |  281 +++++++++++++++++++++++++++++++++++++++++
 include/linux/mfd/tps65218.h |  288 ++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 584 insertions(+)
 create mode 100644 drivers/mfd/tps65218.c
 create mode 100644 include/linux/mfd/tps65218.h

diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index 62a60ca..b8f8b30 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -833,6 +833,20 @@ config MFD_TPS65217
 	  This driver can also be built as a module.  If so, the module
 	  will be called tps65217.
 
+config MFD_TPS65218
+	tristate "TI TPS65218 Power Management chips"
+	depends on I2C
+	select MFD_CORE
+	select REGMAP_I2C
+	help
+	  If you say yes here you get support for the TPS65218 series of
+	  Power Management chips.
+	  These include voltage regulators, gpio and other features
+	  that are often used in portable devices.
+
+	  This driver can also be built as a module.  If so, the module
+	  will be called tps65218.
+
 config MFD_TPS6586X
 	bool "TI TPS6586x Power Management chips"
 	depends on I2C=y
diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index 8a28dc9..76668da 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -62,6 +62,7 @@ obj-$(CONFIG_TPS6105X)		+= tps6105x.o
 obj-$(CONFIG_TPS65010)		+= tps65010.o
 obj-$(CONFIG_TPS6507X)		+= tps6507x.o
 obj-$(CONFIG_MFD_TPS65217)	+= tps65217.o
+obj-$(CONFIG_MFD_TPS65218)	+= tps65218.o
 obj-$(CONFIG_MFD_TPS65910)	+= tps65910.o
 tps65912-objs                   := tps65912-core.o tps65912-irq.o
 obj-$(CONFIG_MFD_TPS65912)	+= tps65912.o
diff --git a/drivers/mfd/tps65218.c b/drivers/mfd/tps65218.c
new file mode 100644
index 0000000..8c61640
--- /dev/null
+++ b/drivers/mfd/tps65218.c
@@ -0,0 +1,281 @@
+/*
+ * TPS65218 chip family multi-function driver
+ *
+ * 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/device.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/init.h>
+#include <linux/i2c.h>
+#include <linux/slab.h>
+#include <linux/regmap.h>
+#include <linux/err.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/irq.h>
+#include <linux/interrupt.h>
+#include <linux/mutex.h>
+
+#include <linux/mfd/core.h>
+#include <linux/mfd/tps65218.h>
+
+#define TPS65218_PASSWORD_REGS_UNLOCK   0x7D
+
+/**
+ * tps65218_reg_read: Read a single tps65218 register.
+ *
+ * @tps: Device to read from.
+ * @reg: Register to read.
+ * @val: Contians the value
+ */
+int tps65218_reg_read(struct tps65218 *tps, unsigned int reg,
+			unsigned int *val)
+{
+	return regmap_read(tps->regmap, reg, val);
+}
+EXPORT_SYMBOL_GPL(tps65218_reg_read);
+
+/**
+ * tps65218_reg_write: Write a single tps65218 register.
+ *
+ * @tps65218: Device to write to.
+ * @reg: Register to write to.
+ * @val: Value to write.
+ * @level: Password protected level
+ */
+int tps65218_reg_write(struct tps65218 *tps, unsigned int reg,
+			unsigned int val, unsigned int level)
+{
+	int ret;
+	unsigned int xor_reg_val;
+
+	switch (level) {
+	case TPS65218_PROTECT_NONE:
+		return regmap_write(tps->regmap, reg, val);
+	case TPS65218_PROTECT_L1:
+		xor_reg_val = reg ^ TPS65218_PASSWORD_REGS_UNLOCK;
+		ret = regmap_write(tps->regmap, TPS65218_REG_PASSWORD,
+							xor_reg_val);
+		if (ret < 0)
+			return ret;
+
+		return regmap_write(tps->regmap, reg, val);
+	default:
+		return -EINVAL;
+	}
+}
+EXPORT_SYMBOL_GPL(tps65218_reg_write);
+
+/**
+ * tps65218_update_bits: Modify bits w.r.t mask, val and level.
+ *
+ * @tps65218: Device to write to.
+ * @reg: Register to read-write to.
+ * @mask: Mask.
+ * @val: Value to write.
+ * @level: Password protected level
+ */
+static int tps65218_update_bits(struct tps65218 *tps, unsigned int reg,
+		unsigned int mask, unsigned int val, unsigned int level)
+{
+	int ret;
+	unsigned int data;
+
+	ret = tps65218_reg_read(tps, reg, &data);
+	if (ret) {
+		dev_err(tps->dev, "Read from reg 0x%x failed\n", reg);
+		return ret;
+	}
+
+	data &= ~mask;
+	data |= val & mask;
+
+	mutex_lock(&tps->tps_lock);
+	ret = tps65218_reg_write(tps, reg, data, level);
+	if (ret)
+		dev_err(tps->dev, "Write for reg 0x%x failed\n", reg);
+	mutex_unlock(&tps->tps_lock);
+
+	return ret;
+}
+
+int tps65218_set_bits(struct tps65218 *tps, unsigned int reg,
+		unsigned int mask, unsigned int val, unsigned int level)
+{
+	return tps65218_update_bits(tps, reg, mask, val, level);
+}
+EXPORT_SYMBOL_GPL(tps65218_set_bits);
+
+int tps65218_clear_bits(struct tps65218 *tps, unsigned int reg,
+		unsigned int mask, unsigned int level)
+{
+	return tps65218_update_bits(tps, reg, mask, 0, level);
+}
+EXPORT_SYMBOL_GPL(tps65218_clear_bits);
+
+static struct regmap_config tps65218_regmap_config = {
+	.reg_bits = 8,
+	.val_bits = 8,
+};
+
+static const struct regmap_irq tps65218_irqs[] = {
+	/* INT1 IRQs */
+	[TPS65218_PRGC_IRQ] = {
+		.mask = TPS65218_INT1_PRGC,
+	},
+	[TPS65218_CC_AQC_IRQ] = {
+		.mask = TPS65218_INT1_CC_AQC,
+	},
+	[TPS65218_HOT_IRQ] = {
+		.mask = TPS65218_INT1_HOT,
+	},
+	[TPS65218_PB_IRQ] = {
+		.mask = TPS65218_INT1_PB,
+	},
+	[TPS65218_AC_IRQ] = {
+		.mask = TPS65218_INT1_AC,
+	},
+	[TPS65218_VPRG_IRQ] = {
+		.mask = TPS65218_INT1_VPRG,
+	},
+	[TPS65218_INVALID1_IRQ] = {
+	},
+	[TPS65218_INVALID2_IRQ] = {
+	},
+	/* INT2 IRQs*/
+	[TPS65218_LS1_I_IRQ] = {
+		.mask = TPS65218_INT2_LS1_I,
+		.reg_offset = 1,
+	},
+	[TPS65218_LS2_I_IRQ] = {
+		.mask = TPS65218_INT2_LS2_I,
+		.reg_offset = 1,
+	},
+	[TPS65218_LS3_I_IRQ] = {
+		.mask = TPS65218_INT2_LS3_I,
+		.reg_offset = 1,
+	},
+	[TPS65218_LS1_F_IRQ] = {
+		.mask = TPS65218_INT2_LS1_F,
+		.reg_offset = 1,
+	},
+	[TPS65218_LS2_F_IRQ] = {
+		.mask = TPS65218_INT2_LS2_F,
+		.reg_offset = 1,
+	},
+	[TPS65218_LS3_F_IRQ] = {
+		.mask = TPS65218_INT2_LS3_F,
+		.reg_offset = 1,
+	},
+	[TPS65218_INVALID3_IRQ] = {
+	},
+	[TPS65218_INVALID4_IRQ] = {
+	},
+};
+
+static struct regmap_irq_chip tps65218_irq_chip = {
+	.name = "tps65218",
+	.irqs = tps65218_irqs,
+	.num_irqs = ARRAY_SIZE(tps65218_irqs),
+
+	.num_regs = 2,
+	.mask_base = TPS65218_REG_INT_MASK1,
+};
+
+static const struct of_device_id of_tps65218_match_table[] = {
+	{ .compatible = "ti,tps65218", },
+	{ /* end */ }
+};
+
+static int tps65218_probe(struct i2c_client *client,
+				const struct i2c_device_id *ids)
+{
+	struct tps65218 *tps;
+	const struct of_device_id *match;
+	int ret;
+
+	match = of_match_device(of_tps65218_match_table, &client->dev);
+	if (!match) {
+		dev_err(&client->dev,
+			"Failed to find matching dt id\n");
+		return -EINVAL;
+	}
+
+	tps = devm_kzalloc(&client->dev, sizeof(*tps), GFP_KERNEL);
+	if (!tps)
+		return -ENOMEM;
+
+	i2c_set_clientdata(client, tps);
+	tps->dev = &client->dev;
+	tps->irq = client->irq;
+	tps->regmap = devm_regmap_init_i2c(client, &tps65218_regmap_config);
+	if (IS_ERR(tps->regmap)) {
+		ret = PTR_ERR(tps->regmap);
+		dev_err(tps->dev, "Failed to allocate register map: %d\n",
+			ret);
+		return ret;
+	}
+
+	mutex_init(&tps->tps_lock);
+
+	ret = regmap_add_irq_chip(tps->regmap, tps->irq,
+			IRQF_ONESHOT, 0, &tps65218_irq_chip,
+			&tps->irq_data);
+	if (ret < 0)
+		return ret;
+
+	ret = of_platform_populate(client->dev.of_node, NULL, NULL,
+				   &client->dev);
+	if (ret < 0)
+		goto err_irq;
+
+	return 0;
+
+err_irq:
+	regmap_del_irq_chip(tps->irq, tps->irq_data);
+
+	return ret;
+}
+
+static int tps65218_remove(struct i2c_client *client)
+{
+	struct tps65218 *tps = i2c_get_clientdata(client);
+
+	regmap_del_irq_chip(tps->irq, tps->irq_data);
+
+	return 0;
+}
+
+static const struct i2c_device_id tps65218_id_table[] = {
+	{"tps65218", TPS65218},
+};
+MODULE_DEVICE_TABLE(i2c, tps65218_id_table);
+
+static struct i2c_driver tps65218_driver = {
+	.driver		= {
+		.name	= "tps65218",
+		.owner	= THIS_MODULE,
+		.of_match_table = of_tps65218_match_table,
+	},
+	.probe		= tps65218_probe,
+	.remove		= tps65218_remove,
+	.id_table       = tps65218_id_table,
+};
+
+module_i2c_driver(tps65218_driver);
+
+MODULE_AUTHOR("J Keerthy <j-keerthy@ti.com>");
+MODULE_DESCRIPTION("TPS65218 chip family multi-function driver");
+MODULE_LICENSE("GPL v2");
diff --git a/include/linux/mfd/tps65218.h b/include/linux/mfd/tps65218.h
new file mode 100644
index 0000000..84b9c99
--- /dev/null
+++ b/include/linux/mfd/tps65218.h
@@ -0,0 +1,288 @@
+/*
+ * linux/mfd/tps65218.h
+ *
+ * Functions to access TPS65219 power management chip.
+ *
+ * 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.
+ */
+
+#ifndef __LINUX_MFD_TPS65218_H
+#define __LINUX_MFD_TPS65218_H
+
+#include <linux/i2c.h>
+#include <linux/regulator/driver.h>
+#include <linux/regulator/machine.h>
+#include <linux/bitops.h>
+
+/* TPS chip id list */
+#define TPS65218			0xF0
+
+/* I2C ID for TPS65218 part */
+#define TPS65218_I2C_ID			0x24
+
+/* All register addresses */
+#define TPS65218_REG_CHIPID		0x00
+#define TPS65218_REG_INT1		0x01
+#define TPS65218_REG_INT2		0x02
+#define TPS65218_REG_INT_MASK1		0x03
+#define TPS65218_REG_INT_MASK2		0x04
+#define TPS65218_REG_STATUS		0x05
+#define TPS65218_REG_CONTROL		0x06
+#define TPS65218_REG_FLAG		0x07
+
+#define TPS65218_REG_PASSWORD		0x10
+#define TPS65218_REG_ENABLE1		0x11
+#define TPS65218_REG_ENABLE2		0x12
+#define TPS65218_REG_CONFIG1		0x13
+#define TPS65218_REG_CONFIG2		0x14
+#define TPS65218_REG_CONFIG3		0x15
+#define TPS65218_REG_CONTROL_DCDC1	0x16
+#define TPS65218_REG_CONTROL_DCDC2	0x17
+#define TPS65218_REG_CONTROL_DCDC3	0x18
+#define TPS65218_REG_CONTROL_DCDC4	0x19
+#define TPS65218_REG_CONTRL_SLEW_RATE	0x1A
+#define TPS65218_REG_CONTROL_LDO1	0x1B
+#define TPS65218_REG_SEQ1		0x20
+#define TPS65218_REG_SEQ2		0x21
+#define TPS65218_REG_SEQ3		0x22
+#define TPS65218_REG_SEQ4		0x23
+#define TPS65218_REG_SEQ5		0x24
+#define TPS65218_REG_SEQ6		0x25
+#define TPS65218_REG_SEQ7		0x26
+
+/* Register field definitions */
+#define TPS65218_CHIPID_CHIP_MASK	0xF8
+#define TPS65218_CHIPID_REV_MASK	0x07
+
+#define TPS65218_INT1_VPRG		BIT(5)
+#define TPS65218_INT1_AC		BIT(4)
+#define TPS65218_INT1_PB		BIT(3)
+#define TPS65218_INT1_HOT		BIT(2)
+#define TPS65218_INT1_CC_AQC		BIT(1)
+#define TPS65218_INT1_PRGC		BIT(0)
+
+#define TPS65218_INT2_LS3_F		BIT(5)
+#define TPS65218_INT2_LS2_F		BIT(4)
+#define TPS65218_INT2_LS1_F		BIT(3)
+#define TPS65218_INT2_LS3_I		BIT(2)
+#define TPS65218_INT2_LS2_I		BIT(1)
+#define TPS65218_INT2_LS1_I		BIT(0)
+
+#define TPS65218_INT_MASK1_VPRG		BIT(5)
+#define TPS65218_INT_MASK1_AC		BIT(4)
+#define TPS65218_INT_MASK1_PB		BIT(3)
+#define TPS65218_INT_MASK1_HOT		BIT(2)
+#define TPS65218_INT_MASK1_CC_AQC	BIT(1)
+#define TPS65218_INT_MASK1_PRGC		BIT(0)
+
+#define TPS65218_INT_MASK2_LS3_F	BIT(5)
+#define TPS65218_INT_MASK2_LS2_F	BIT(4)
+#define TPS65218_INT_MASK2_LS1_F	BIT(3)
+#define TPS65218_INT_MASK2_LS3_I	BIT(2)
+#define TPS65218_INT_MASK2_LS2_I	BIT(1)
+#define TPS65218_INT_MASK2_LS1_I	BIT(0)
+
+#define TPS65218_STATUS_FSEAL		BIT(7)
+#define TPS65218_STATUS_EE		BIT(6)
+#define TPS65218_STATUS_AC_STATE	BIT(5)
+#define TPS65218_STATUS_PB_STATE	BIT(4)
+#define TPS65218_STATUS_STATE_MASK	0xC
+#define TPS65218_STATUS_CC_STAT		0x3
+
+#define TPS65218_CONTROL_OFFNPFO	BIT(1)
+#define TPS65218_CONTROL_CC_AQ	BIT(0)
+
+#define TPS65218_FLAG_GPO3_FLG		BIT(7)
+#define TPS65218_FLAG_GPO2_FLG		BIT(6)
+#define TPS65218_FLAG_GPO1_FLG		BIT(5)
+#define TPS65218_FLAG_LDO1_FLG		BIT(4)
+#define TPS65218_FLAG_DC4_FLG		BIT(3)
+#define TPS65218_FLAG_DC3_FLG		BIT(2)
+#define TPS65218_FLAG_DC2_FLG		BIT(1)
+#define TPS65218_FLAG_DC1_FLG		BIT(0)
+
+#define TPS65218_ENABLE1_DC6_EN		BIT(5)
+#define TPS65218_ENABLE1_DC5_EN		BIT(4)
+#define TPS65218_ENABLE1_DC4_EN		BIT(3)
+#define TPS65218_ENABLE1_DC3_EN		BIT(2)
+#define TPS65218_ENABLE1_DC2_EN		BIT(1)
+#define TPS65218_ENABLE1_DC1_EN		BIT(0)
+
+#define TPS65218_ENABLE2_GPIO3		BIT(6)
+#define TPS65218_ENABLE2_GPIO2		BIT(5)
+#define TPS65218_ENABLE2_GPIO1		BIT(4)
+#define TPS65218_ENABLE2_LS3_EN		BIT(3)
+#define TPS65218_ENABLE2_LS2_EN		BIT(2)
+#define TPS65218_ENABLE2_LS1_EN		BIT(1)
+#define TPS65218_ENABLE2_LDO1_EN	BIT(0)
+
+
+#define TPS65218_CONFIG1_TRST		BIT(7)
+#define TPS65218_CONFIG1_GPO2_BUF	BIT(6)
+#define TPS65218_CONFIG1_IO1_SEL	BIT(5)
+#define TPS65218_CONFIG1_PGDLY_MASK	0x18
+#define TPS65218_CONFIG1_STRICT		BIT(2)
+#define TPS65218_CONFIG1_UVLO_MASK	0x3
+
+#define TPS65218_CONFIG2_DC12_RST	BIT(7)
+#define TPS65218_CONFIG2_UVLOHYS	BIT(6)
+#define TPS65218_CONFIG2_LS3ILIM_MASK	0xC
+#define TPS65218_CONFIG2_LS2ILIM_MASK	0x3
+
+#define TPS65218_CONFIG3_LS3NPFO	BIT(5)
+#define TPS65218_CONFIG3_LS2NPFO	BIT(4)
+#define TPS65218_CONFIG3_LS1NPFO	BIT(3)
+#define TPS65218_CONFIG3_LS3DCHRG	BIT(2)
+#define TPS65218_CONFIG3_LS2DCHRG	BIT(1)
+#define TPS65218_CONFIG3_LS1DCHRG	BIT(0)
+
+#define TPS65218_CONTROL_DCDC1_PFM	BIT(7)
+#define TPS65218_CONTROL_DCDC1_MASK	0x7F
+
+#define TPS65218_CONTROL_DCDC2_PFM	BIT(7)
+#define TPS65218_CONTROL_DCDC2_MASK	0x3F
+
+#define TPS65218_CONTROL_DCDC3_PFM	BIT(7)
+#define TPS65218_CONTROL_DCDC3_MASK	0x3F
+
+#define TPS65218_CONTROL_DCDC4_PFM	BIT(7)
+#define TPS65218_CONTROL_DCDC4_MASK	0x3F
+
+#define TPS65218_SLEW_RATE_GO		BIT(7)
+#define TPS65218_SLEW_RATE_GODSBL	BIT(6)
+#define TPS65218_SLEW_RATE_SLEW_MASK	0x7
+
+#define TPS65218_CONTROL_LDO1_MASK	0x3F
+
+#define TPS65218_SEQ1_DLY8		BIT(7)
+#define TPS65218_SEQ1_DLY7		BIT(6)
+#define TPS65218_SEQ1_DLY6		BIT(5)
+#define TPS65218_SEQ1_DLY5		BIT(4)
+#define TPS65218_SEQ1_DLY4		BIT(3)
+#define TPS65218_SEQ1_DLY3		BIT(2)
+#define TPS65218_SEQ1_DLY2		BIT(1)
+#define TPS65218_SEQ1_DLY1		BIT(0)
+
+#define TPS65218_SEQ2_DLYFCTR		BIT(7)
+#define TPS65218_SEQ2_DLY9		BIT(0)
+
+#define TPS65218_SEQ3_DC2_SEQ_MASK	0xF0
+#define TPS65218_SEQ3_DC1_SEQ_MASK	0xF
+
+#define TPS65218_SEQ4_DC4_SEQ_MASK	0xF0
+#define TPS65218_SEQ4_DC3_SEQ_MASK	0xF
+
+#define TPS65218_SEQ5_DC6_SEQ_MASK	0xF0
+#define TPS65218_SEQ5_DC5_SEQ_MASK	0xF
+
+#define TPS65218_SEQ6_LS1_SEQ_MASK	0xF0
+#define TPS65218_SEQ6_LDO1_SEQ_MASK	0xF
+
+#define TPS65218_SEQ7_GPO3_SEQ_MASK	0xF0
+#define TPS65218_SEQ7_GPO1_SEQ_MASK	0xF
+#define TPS65218_PROTECT_NONE		0
+#define TPS65218_PROTECT_L1		1
+
+enum tps65218_regulator_id {
+	/* DCDC's */
+	TPS65218_DCDC_1,
+	TPS65218_DCDC_2,
+	TPS65218_DCDC_3,
+	TPS65218_DCDC_4,
+	TPS65218_DCDC_5,
+	TPS65218_DCDC_6,
+	/* LDOs */
+	TPS65218_LDO_1,
+};
+
+#define TPS65218_MAX_REG_ID		TPS65218_LDO_1
+
+/* Number of step-down converters available */
+#define TPS65218_NUM_DCDC		6
+/* Number of LDO voltage regulators available */
+#define TPS65218_NUM_LDO		1
+/* Number of total regulators available */
+#define TPS65218_NUM_REGULATOR		(TPS65218_NUM_DCDC + TPS65218_NUM_LDO)
+
+/* Define the TPS65218 IRQ numbers */
+enum tps65218_irqs {
+	/* INT1 registers */
+	TPS65218_PRGC_IRQ,
+	TPS65218_CC_AQC_IRQ,
+	TPS65218_HOT_IRQ,
+	TPS65218_PB_IRQ,
+	TPS65218_AC_IRQ,
+	TPS65218_VPRG_IRQ,
+	TPS65218_INVALID1_IRQ,
+	TPS65218_INVALID2_IRQ,
+	/* INT2 registers */
+	TPS65218_LS1_I_IRQ,
+	TPS65218_LS2_I_IRQ,
+	TPS65218_LS3_I_IRQ,
+	TPS65218_LS1_F_IRQ,
+	TPS65218_LS2_F_IRQ,
+	TPS65218_LS3_F_IRQ,
+	TPS65218_INVALID3_IRQ,
+	TPS65218_INVALID4_IRQ,
+};
+
+/**
+ * struct tps_info - packages regulator constraints
+ * @id:			Id of the regulator
+ * @name:		Voltage regulator name
+ * @min_uV:		minimum micro volts
+ * @max_uV:		minimum micro volts
+ * @vsel_to_uv:		Function pointer to get voltage from selector
+ * @uv_to_vsel:		Function pointer to get selector from voltage
+ *
+ * This data is used to check the regualtor voltage limits while setting.
+ */
+struct tps_info {
+	int id;
+	const char *name;
+	int min_uV;
+	int max_uV;
+	int (*vsel_to_uv)(unsigned int vsel);
+	int (*uv_to_vsel)(int uV, unsigned int *vsel);
+};
+
+/**
+ * struct tps65218 - tps65218 sub-driver chip access routines
+ *
+ * Device data may be used to access the TPS65218 chip
+ */
+
+struct tps65218 {
+	struct device *dev;
+	unsigned int id;
+
+	struct mutex tps_lock;		/* lock guarding the data structure */
+	/* IRQ Data */
+	int irq;
+	u32 irq_mask;
+	struct regmap_irq_chip_data *irq_data;
+	struct regulator_desc desc[TPS65218_NUM_REGULATOR];
+	struct regulator_dev *rdev[TPS65218_NUM_REGULATOR];
+	struct tps_info *info[TPS65218_NUM_REGULATOR];
+	struct regmap *regmap;
+};
+
+int tps65218_reg_read(struct tps65218 *tps, unsigned int reg,
+					unsigned int *val);
+int tps65218_reg_write(struct tps65218 *tps, unsigned int reg,
+			unsigned int val, unsigned int level);
+int tps65218_set_bits(struct tps65218 *tps, unsigned int reg,
+		unsigned int mask, unsigned int val, unsigned int level);
+int tps65218_clear_bits(struct tps65218 *tps, unsigned int reg,
+		unsigned int mask, unsigned int level);
+
+#endif /*  __LINUX_MFD_TPS65218_H */
-- 
1.7.9.5


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 3/4] Regulators: TPS65218: Add Regulator driver for TPS65218 PMIC
  2013-12-03  6:43 [PATCH 0/4] mfd: TPS65218: Add support for TPS65218 PMIC Keerthy
  2013-12-03  6:43 ` [PATCH 1/4] mfd: DT bindings " Keerthy
  2013-12-03  6:43 ` [PATCH 2/4] MFD: TPS65218: Add driver for the " Keerthy
@ 2013-12-03  6:43 ` Keerthy
  2013-12-03 15:16   ` Mark Brown
  2013-12-04  7:39   ` Manish Badarkhe
  2013-12-03  6:43 ` [PATCH 4/4] ARM: dts: AM437x: Add TPS65218 device tree nodes Keerthy
  3 siblings, 2 replies; 16+ messages in thread
From: Keerthy @ 2013-12-03  6:43 UTC (permalink / raw)
  To: rob.herring, pawel.moll, mark.rutland, swarren, ijc+devicetree,
	rob, sameo, lee.jones, grant.likely, lgirdwood, broonie
  Cc: j-keerthy, devicetree, linux-doc, linux-kernel, linux-omap

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-keerthy@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 0000000..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 + 900000;
+	else
+		uV = (vsel - 26) * 50000 + 1550000;
+
+	return uV;
+}
+
+static int tps65218_ldo1_dcdc3_uv_to_vsel(int uV, unsigned int *vsel)
+{
+	if (uV <= 1550000)
+		*vsel = DIV_ROUND_UP(uV - 900000, 25000);
+	else
+		*vsel = 26 + DIV_ROUND_UP(uV - 1550000, 50000);
+
+	return 0;
+}
+
+static int tps65218_dcdc1_2_vsel_to_uv(unsigned int vsel)
+{
+	int uV = 0;
+
+	if (vsel <= 50)
+		uV = vsel * 10000 + 850000;
+	else
+		uV = (vsel - 50) * 25000 + 1350000;
+
+	return uV;
+}
+
+static int tps65218_dcdc1_2_uv_to_vsel(int uV, unsigned int *vsel)
+{
+	if (uV <= 1350000)
+		*vsel = DIV_ROUND_UP(uV - 850000, 10000);
+	else
+		*vsel = 50 + DIV_ROUND_UP(uV - 1350000, 25000);
+
+	return 0;
+}
+
+static int tps65218_dcd4_vsel_to_uv(unsigned int vsel)
+{
+	int uV = 0;
+
+	if (vsel <= 15)
+		uV = vsel * 25000 + 1175000;
+	else
+		uV = (vsel - 15) * 50000 + 1550000;
+
+	return uV;
+}
+
+static int tps65218_dcdc4_uv_to_vsel(int uV, unsigned int *vsel)
+{
+	if (uV <= 1550000)
+		*vsel = DIV_ROUND_UP(uV - 1175000, 25000);
+	else
+		*vsel = 15 + DIV_ROUND_UP(uV - 1550000, 50000);
+
+	return 0;
+}
+
+static struct tps_info tps65218_pmic_regs[] = {
+	TPS65218_INFO(0, "DCDC1", 850000, 1675000, tps65218_dcdc1_2_vsel_to_uv,
+		      tps65218_dcdc1_2_uv_to_vsel),
+	TPS65218_INFO(1, "DCDC2", 850000, 1675000, tps65218_dcdc1_2_vsel_to_uv,
+		      tps65218_dcdc1_2_uv_to_vsel),
+	TPS65218_INFO(2, "DCDC3", 900000, 3400000,
+		      tps65218_ldo1_dcdc3_vsel_to_uv,
+		      tps65218_ldo1_dcdc3_uv_to_vsel),
+	TPS65218_INFO(3, "DCDC4", 1175000, 3400000, tps65218_dcd4_vsel_to_uv,
+		      tps65218_dcdc4_uv_to_vsel),
+	TPS65218_INFO(4, "DCDC5", 1000000, 1000000, NULL, NULL),
+	TPS65218_INFO(5, "DCDC6", 1800000, 1800000, NULL, NULL),
+	TPS65218_INFO(6, "LDO1", 900000, 3400000,
+		      tps65218_ldo1_dcdc3_vsel_to_uv,
+		      tps65218_ldo1_dcdc3_uv_to_vsel),
+};
+
+#define TPS65218_OF_MATCH(comp, label) \
+	{ \
+		.compatible = comp, \
+		.data = &label, \
+	}
+
+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);
+
+static int tps65218_pmic_set_voltage_sel(struct regulator_dev *dev,
+					 unsigned selector)
+{
+	int ret;
+	struct tps65218 *tps = rdev_get_drvdata(dev);
+	unsigned int rid = rdev_get_id(dev);
+
+	/* Set the voltage based on vsel value and write protect level is 2 */
+	ret = tps65218_set_bits(tps, dev->desc->vsel_reg, dev->desc->vsel_mask,
+				selector, TPS65218_PROTECT_L1);
+
+	/* Set GO bit for DCDC1/2 to initiate voltage transistion */
+	switch (rid) {
+	case TPS65218_DCDC_1:
+	case TPS65218_DCDC_2:
+		ret = tps65218_set_bits(tps, TPS65218_REG_CONTRL_SLEW_RATE,
+					TPS65218_SLEW_RATE_GO,
+					TPS65218_SLEW_RATE_GO,
+					TPS65218_PROTECT_L1);
+		break;
+	}
+
+	return ret;
+}
+
+static int tps65218_pmic_map_voltage(struct regulator_dev *dev,
+				     int min_uV, int max_uV)
+{
+	struct tps65218 *tps = rdev_get_drvdata(dev);
+	unsigned int sel, rid = rdev_get_id(dev);
+	int ret;
+
+	if (rid < TPS65218_DCDC_1 || rid > TPS65218_LDO_1)
+		return -EINVAL;
+
+	if (min_uV < tps->info[rid]->min_uV)
+		min_uV = tps->info[rid]->min_uV;
+
+	if (max_uV < tps->info[rid]->min_uV || min_uV > tps->info[rid]->max_uV)
+		return -EINVAL;
+
+	ret = tps->info[rid]->uv_to_vsel(min_uV, &sel);
+	if (ret)
+		return ret;
+
+	return sel;
+}
+
+static int tps65218_pmic_list_voltage(struct regulator_dev *dev,
+					unsigned selector)
+{
+	struct tps65218 *tps = rdev_get_drvdata(dev);
+	unsigned int rid = rdev_get_id(dev);
+
+	if (rid < TPS65218_DCDC_1 || rid > TPS65218_LDO_1)
+		return -EINVAL;
+
+	if (selector >= dev->desc->n_voltages)
+		return -EINVAL;
+
+	return tps->info[rid]->vsel_to_uv(selector);
+}
+
+static int tps65218_pmic_enable(struct regulator_dev *dev)
+{
+	struct tps65218 *tps = rdev_get_drvdata(dev);
+	unsigned int rid = rdev_get_id(dev);
+
+	if (rid < TPS65218_DCDC_1 || rid > TPS65218_LDO_1)
+		return -EINVAL;
+
+	/* Enable the regulator and password protection is level 1 */
+	return tps65218_set_bits(tps, dev->desc->enable_reg,
+				 dev->desc->enable_mask, dev->desc->enable_mask,
+				 TPS65218_PROTECT_L1);
+}
+
+static int tps65218_pmic_disable(struct regulator_dev *dev)
+{
+	struct tps65218 *tps = rdev_get_drvdata(dev);
+	unsigned int rid = rdev_get_id(dev);
+
+	if (rid < TPS65218_DCDC_1 || rid > TPS65218_LDO_1)
+		return -EINVAL;
+
+	/* Disable the regulator and password protection is level 1 */
+	return tps65218_clear_bits(tps, dev->desc->enable_reg,
+				   dev->desc->enable_mask, TPS65218_PROTECT_L1);
+}
+
+static int tps65218_set_voltage_time_sel(struct regulator_dev *rdev,
+	unsigned int old_selector, unsigned int new_selector)
+{
+	int old_uv, new_uv;
+
+	old_uv = tps65218_pmic_list_voltage(rdev, old_selector);
+	if (old_uv < 0)
+		return old_uv;
+
+	new_uv = tps65218_pmic_list_voltage(rdev, new_selector);
+	if (new_uv < 0)
+		return new_uv;
+
+	return DIV_ROUND_UP(abs(old_uv - new_uv), tps65218_ramp_delay);
+}
+
+/* Operations permitted on DCDC1, DCDC2 */
+static struct regulator_ops tps65218_dcdc12_ops = {
+	.is_enabled		= regulator_is_enabled_regmap,
+	.enable			= tps65218_pmic_enable,
+	.disable		= tps65218_pmic_disable,
+	.get_voltage_sel	= regulator_get_voltage_sel_regmap,
+	.set_voltage_sel	= tps65218_pmic_set_voltage_sel,
+	.list_voltage		= tps65218_pmic_list_voltage,
+	.map_voltage		= tps65218_pmic_map_voltage,
+	.set_voltage_time_sel	= tps65218_set_voltage_time_sel,
+};
+
+/* Operations permitted on DCDC3, DCDC4 and LDO1 */
+static struct regulator_ops tps65218_ldo1_dcdc34_ops = {
+	.is_enabled		= regulator_is_enabled_regmap,
+	.enable			= tps65218_pmic_enable,
+	.disable		= tps65218_pmic_disable,
+	.get_voltage_sel	= regulator_get_voltage_sel_regmap,
+	.set_voltage_sel	= tps65218_pmic_set_voltage_sel,
+	.list_voltage		= tps65218_pmic_list_voltage,
+	.map_voltage		= tps65218_pmic_map_voltage,
+};
+
+/* Operations permitted on DCDC5, DCDC6 */
+static struct regulator_ops tps65218_dcdc56_pmic_ops = {
+	.is_enabled		= regulator_is_enabled_regmap,
+	.enable			= tps65218_pmic_enable,
+	.disable		= tps65218_pmic_disable,
+};
+
+static const struct regulator_desc regulators[] = {
+	TPS65218_REGULATOR("DCDC1", TPS65218_DCDC_1, tps65218_dcdc12_ops, 64,
+			   TPS65218_REG_CONTROL_DCDC1,
+			   TPS65218_CONTROL_DCDC1_MASK,
+			   TPS65218_REG_ENABLE1, TPS65218_ENABLE1_DC1_EN, NULL),
+	TPS65218_REGULATOR("DCDC2", TPS65218_DCDC_2, tps65218_dcdc12_ops, 64,
+			   TPS65218_REG_CONTROL_DCDC2,
+			   TPS65218_CONTROL_DCDC2_MASK,
+			   TPS65218_REG_ENABLE1, TPS65218_ENABLE1_DC2_EN, NULL),
+	TPS65218_REGULATOR("DCDC3", TPS65218_DCDC_3, tps65218_ldo1_dcdc34_ops,
+			   64, TPS65218_REG_CONTROL_DCDC3,
+			   TPS65218_CONTROL_DCDC3_MASK, TPS65218_REG_ENABLE1,
+			   TPS65218_ENABLE1_DC3_EN, NULL),
+	TPS65218_REGULATOR("DCDC4", TPS65218_DCDC_4, tps65218_ldo1_dcdc34_ops,
+			   53, TPS65218_REG_CONTROL_DCDC4,
+			   TPS65218_CONTROL_DCDC4_MASK,
+			   TPS65218_REG_ENABLE1, TPS65218_ENABLE1_DC4_EN, NULL),
+	TPS65218_REGULATOR("DCDC5", TPS65218_DCDC_5, tps65218_dcdc56_pmic_ops,
+			   1, -1, -1, TPS65218_REG_ENABLE1,
+			   TPS65218_ENABLE1_DC5_EN, NULL),
+	TPS65218_REGULATOR("DCDC6", TPS65218_DCDC_6, tps65218_dcdc56_pmic_ops,
+			   1, -1, -1, TPS65218_REG_ENABLE1,
+			   TPS65218_ENABLE1_DC6_EN, NULL),
+	TPS65218_REGULATOR("LDO1", TPS65218_LDO_1, tps65218_ldo1_dcdc34_ops, 64,
+			   TPS65218_REG_CONTROL_LDO1,
+			   TPS65218_CONTROL_LDO1_MASK, TPS65218_REG_ENABLE2,
+			   TPS65218_ENABLE2_LDO1_EN, NULL),
+};
+
+static int tps65218_regulator_probe(struct platform_device *pdev)
+{
+	struct tps65218 *tps = dev_get_drvdata(pdev->dev.parent);
+	struct regulator_init_data *init_data;
+	const struct tps_info	*template;
+	struct regulator_dev *rdev;
+	const struct of_device_id	*match;
+	struct regulator_config config = { };
+	int id;
+
+	match = of_match_device(tps65218_of_match, &pdev->dev);
+	if (match) {
+		template = match->data;
+		id = template->id;
+		init_data = of_get_regulator_init_data(&pdev->dev,
+						      pdev->dev.of_node);
+	} else {
+		return -ENODEV;
+	}
+
+	platform_set_drvdata(pdev, tps);
+
+	tps->info[id] = &tps65218_pmic_regs[id];
+	config.dev = &pdev->dev;
+	config.init_data = init_data;
+	config.driver_data = tps;
+	config.regmap = tps->regmap;
+	config.of_node = pdev->dev.of_node;
+
+	rdev = regulator_register(&regulators[id], &config);
+	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;
+}
+
+static int tps65218_regulator_remove(struct platform_device *pdev)
+{
+	struct tps65218 *tps = platform_get_drvdata(pdev);
+	const struct of_device_id	*match;
+	const struct tps_info		*template;
+
+	match = of_match_device(tps65218_of_match, &pdev->dev);
+	template = match->data;
+	regulator_unregister(tps->rdev[template->id]);
+	platform_set_drvdata(pdev, NULL);
+
+	return 0;
+}
+
+static struct platform_driver tps65218_regulator_driver = {
+	.driver = {
+		.name = "tps65218-pmic",
+		.owner = THIS_MODULE,
+		.of_match_table = of_match_ptr(tps65218_of_match),
+	},
+	.probe = tps65218_regulator_probe,
+	.remove = tps65218_regulator_remove,
+};
+
+module_platform_driver(tps65218_regulator_driver);
+
+MODULE_AUTHOR("J Keerthy <j-keerthy@ti.com>");
+MODULE_DESCRIPTION("TPS65218 voltage regulator driver");
+MODULE_ALIAS("platform:tps65218-pmic");
+MODULE_LICENSE("GPL v2");
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 4/4] ARM: dts: AM437x: Add TPS65218 device tree nodes
  2013-12-03  6:43 [PATCH 0/4] mfd: TPS65218: Add support for TPS65218 PMIC Keerthy
                   ` (2 preceding siblings ...)
  2013-12-03  6:43 ` [PATCH 3/4] Regulators: TPS65218: Add Regulator driver for " Keerthy
@ 2013-12-03  6:43 ` Keerthy
  3 siblings, 0 replies; 16+ messages in thread
From: Keerthy @ 2013-12-03  6:43 UTC (permalink / raw)
  To: rob.herring, pawel.moll, mark.rutland, swarren, ijc+devicetree,
	rob, sameo, lee.jones, grant.likely, lgirdwood, broonie
  Cc: j-keerthy, devicetree, linux-doc, linux-kernel, linux-omap

Add TPS65218 device tree nodes. This patch is tested on am43x-epos-evm board.

Signed-off-by: Keerthy <j-keerthy@ti.com>
---
 arch/arm/boot/dts/am43x-epos-evm.dts |   41 ++++++++++++++++++++++++++++++++++
 1 file changed, 41 insertions(+)

diff --git a/arch/arm/boot/dts/am43x-epos-evm.dts b/arch/arm/boot/dts/am43x-epos-evm.dts
index fbf9c4c..ad7000a 100644
--- a/arch/arm/boot/dts/am43x-epos-evm.dts
+++ b/arch/arm/boot/dts/am43x-epos-evm.dts
@@ -156,6 +156,47 @@
 		reg = <0x50>;
 	};
 
+	tps65218: tps65218@24 {
+		reg = <0x24>;
+		compatible = "ti,tps65218";
+		interrupts = <GIC_SPI 7 IRQ_TYPE_NONE>; /* NMIn */
+		interrupt-parent = <&gic>;
+		interrupt-controller;
+		#interrupt-cells = <2>;
+
+		dcdc1: regulator-dcdc1 {
+			compatible = "ti,tps65218-dcdc1";
+			/* VDD_CORE voltage limits min of OPP50 and max of OPP100 */
+			regulator-name = "vdd_core";
+			regulator-min-microvolt = <912000>;
+			regulator-max-microvolt = <1144000>;
+			regulator-boot-on;
+			regulator-always-on;
+		};
+
+		dcdc2: regulator-dcdc2 {
+			compatible = "ti,tps65218-dcdc2";
+			/* VDD_MPU voltage limits min of OPP50 and max of OPP_NITRO */
+			regulator-name = "vdd_mpu";
+			regulator-min-microvolt = <912000>;
+			regulator-max-microvolt = <1378000>;
+			regulator-boot-on;
+			regulator-always-on;
+		};
+
+		dcdc3: regulator-dcdc3 {
+			compatible = "ti,tps65218-dcdc3";
+		};
+
+		ldo1: regulator-ldo1 {
+			compatible = "ti,tps65218-ldo1";
+			regulator-min-microvolt = <1800000>;
+			regulator-max-microvolt = <1800000>;
+			regulator-boot-on;
+			regulator-always-on;
+		};
+	};
+
 	pixcir_ts@5c {
 		compatible = "pixcir,pixcir_ts";
 		reg = <0x5c>;
-- 
1.7.9.5


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH 2/4] MFD: TPS65218: Add driver for the TPS65218 PMIC
  2013-12-03  6:43 ` [PATCH 2/4] MFD: TPS65218: Add driver for the " Keerthy
@ 2013-12-03  9:24   ` Lee Jones
  2013-12-03 10:06     ` Keerthy
  2013-12-03 15:19   ` Mark Brown
  1 sibling, 1 reply; 16+ messages in thread
From: Lee Jones @ 2013-12-03  9:24 UTC (permalink / raw)
  To: Keerthy
  Cc: rob.herring, pawel.moll, mark.rutland, swarren, ijc+devicetree,
	rob, sameo, grant.likely, lgirdwood, broonie, devicetree,
	linux-doc, linux-kernel, linux-omap

> The TPS65218 chip is a power management IC for Portable Navigation Systems
> and Tablet Computing devices. It contains the following components:
> 
>  - Regulators.
>  - Over Temperature warning and Shut down.
> 
> This patch adds support for tps65218 mfd device. At this time only
> the regulator functionality is made available.
> 
> Signed-off-by: Keerthy <j-keerthy@ti.com>
> ---
>  drivers/mfd/Kconfig          |   14 ++
>  drivers/mfd/Makefile         |    1 +
>  drivers/mfd/tps65218.c       |  281 +++++++++++++++++++++++++++++++++++++++++
>  include/linux/mfd/tps65218.h |  288 ++++++++++++++++++++++++++++++++++++++++++

<snip>

> +config MFD_TPS65218
> +	tristate "TI TPS65218 Power Management chips"
> +	depends on I2C
> +	select MFD_CORE
> +	select REGMAP_I2C
> +	help
> +	  If you say yes here you get support for the TPS65218 series of
> +	  Power Management chips.
> +	  These include voltage regulators, gpio and other features
> +	  that are often used in portable devices.

Perhaps you should add a note that only the regulator component is
currently supported.

<snip>

> new file mode 100644
> index 0000000..8c61640
> --- /dev/null
> +++ b/drivers/mfd/tps65218.c
> @@ -0,0 +1,281 @@
> +/*
> + * TPS65218 chip family multi-function driver

Instead of calling it an MFD driver (is there such a thing?) I think
you should mention its true purpose, as per the datasheet.

<snip>

> +static const struct of_device_id of_tps65218_match_table[] = {
> +	{ .compatible = "ti,tps65218", },
> +	{ /* end */ }

I think the end comment is superfluous.

However, if you _really_ want to put something in there, I dislike
"/* Sentinel */" the least.

> +static int tps65218_probe(struct i2c_client *client,
> +				const struct i2c_device_id *ids)
> +{

<snip>

> +	ret = of_platform_populate(client->dev.of_node, NULL, NULL,
> +				   &client->dev);

What are you trying to do here?

Register each regulator as a platform device?

<snip>

> +static const struct i2c_device_id tps65218_id_table[] = {
> +	{"tps65218", TPS65218},
> +};

This has a different structure to of_tps65218_match_table, please
ensure they're the same. I prefer spaces after '{' and before '}'.

<snip>

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/4] mfd: DT bindings for TPS65218 PMIC
  2013-12-03  6:43 ` [PATCH 1/4] mfd: DT bindings " Keerthy
@ 2013-12-03  9:36   ` Lee Jones
  2013-12-03 10:52     ` Keerthy
  0 siblings, 1 reply; 16+ messages in thread
From: Lee Jones @ 2013-12-03  9:36 UTC (permalink / raw)
  To: Keerthy
  Cc: rob.herring, pawel.moll, mark.rutland, swarren, ijc+devicetree,
	rob, sameo, grant.likely, lgirdwood, broonie, devicetree,
	linux-doc, linux-kernel, linux-omap

> Add DT bindings for TPS65218 PMIC.
> 
> Signed-off-by: Keerthy <j-keerthy@ti.com>
> ---
>  Documentation/devicetree/bindings/mfd/tps65218.txt |   27 ++++++++++++++++++++
>  .../devicetree/bindings/regulator/tps65218.txt     |   22 ++++++++++++++++
>  2 files changed, 49 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/mfd/tps65218.txt
>  create mode 100644 Documentation/devicetree/bindings/regulator/tps65218.txt
> 
> diff --git a/Documentation/devicetree/bindings/mfd/tps65218.txt b/Documentation/devicetree/bindings/mfd/tps65218.txt
> new file mode 100644
> index 0000000..87cb7a8
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mfd/tps65218.txt
> @@ -0,0 +1,27 @@
> +The TPS65218 Integrated Power Management Chips.
> +These chips are connected to an i2c bus.

                                   I2C

> +Required properties:
> +- compatible : Must be "ti,tps65218";
> +- interrupts : This i2c device has an IRQ line connected to the main SoC

                       I2C

> +- interrupt-controller : Since the tps65218 support several interrupts

                                               support(s) (hosts?)

> +  internally, it is considered as an interrupt controller cascaded to the SoC.
> +- #interrupt-cells = <2>;
> +- interrupt-parent : The parent interrupt controller.

           Phandle to ...

> +Optional node:

            node(s):

> +- Child nodes contain in the tps65218.
> +  It supports a number of features.

Please re-phase the above two sentences to something decipherable.

> +  The children nodes will thus depend of the capability of the variant.

Please, go on ...

> +Example:
> +/*
> + * Integrated Power Management Chip
> + */
> +tps@24 {
> +    compatible = "ti,tps65218";
> +    reg = <0x24>;
> +    interrupt-controller;
> +    #interrupt-cells = <2>;
> +    interrupt-parent = <&gic>;
> +};

Perhaps it would be better to centralise the documentation inclusive
of the regulator contingent. Or at least provide a _full_ example in
each document.

> diff --git a/Documentation/devicetree/bindings/regulator/tps65218.txt b/Documentation/devicetree/bindings/regulator/tps65218.txt
> new file mode 100644
> index 0000000..1ccf170
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/regulator/tps65218.txt
> @@ -0,0 +1,22 @@
> +TPS65218 family of regulators
> +
> +Required properties:
> +For tps65218 regulators/LDOs
> +- compatible:
> +  - "ti,tps65218-dcdc1" for DCDC1
> +  - "ti,tps65218-dcdc2" for DCDC2
> +  - "ti,tps65218-dcdc3" for DCDC3
> +  - "ti,tps65218-dcdc4" for DCDC4
> +  - "ti,tps65218-dcdc5" for DCDC5
> +  - "ti,tps65218-dcdc6" for DCDC6
> +  - "ti,tps65218-ldo1" for LDO1 LDO

Why aren't you using 'regulator-compatible'?

> +Optional properties:
> +- Any optional property defined in bindings/regulator/regulator.txt
> +
> +Example:
> +	xyz: regulator@0 {

Genuine question: Is the @0 meaningful?

> +		compatible = "ti,tps65218-dcdc1";
> +		regulator-min-microvolt  = <1000000>;
> +		regulator-max-microvolt  = <3000000>;
> +	};

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 2/4] MFD: TPS65218: Add driver for the TPS65218 PMIC
  2013-12-03  9:24   ` Lee Jones
@ 2013-12-03 10:06     ` Keerthy
  2013-12-03 11:21       ` Lee Jones
  0 siblings, 1 reply; 16+ messages in thread
From: Keerthy @ 2013-12-03 10:06 UTC (permalink / raw)
  To: Lee Jones
  Cc: Keerthy, rob.herring, pawel.moll, mark.rutland, swarren,
	ijc+devicetree, rob, sameo, grant.likely, lgirdwood, broonie,
	devicetree, linux-doc, linux-kernel, linux-omap

On Tuesday 03 December 2013 02:54 PM, Lee Jones wrote:
>> The TPS65218 chip is a power management IC for Portable Navigation Systems
>> and Tablet Computing devices. It contains the following components:
>>
>>   - Regulators.
>>   - Over Temperature warning and Shut down.
>>
>> This patch adds support for tps65218 mfd device. At this time only
>> the regulator functionality is made available.
>>
>> Signed-off-by: Keerthy <j-keerthy@ti.com>
>> ---
>>   drivers/mfd/Kconfig          |   14 ++
>>   drivers/mfd/Makefile         |    1 +
>>   drivers/mfd/tps65218.c       |  281 +++++++++++++++++++++++++++++++++++++++++
>>   include/linux/mfd/tps65218.h |  288 ++++++++++++++++++++++++++++++++++++++++++
> <snip>
>
>> +config MFD_TPS65218
>> +	tristate "TI TPS65218 Power Management chips"
>> +	depends on I2C
>> +	select MFD_CORE
>> +	select REGMAP_I2C
>> +	help
>> +	  If you say yes here you get support for the TPS65218 series of
>> +	  Power Management chips.
>> +	  These include voltage regulators, gpio and other features
>> +	  that are often used in portable devices.
> Perhaps you should add a note that only the regulator component is
> currently supported.
Ok. I will add it.

> <snip>
>
>> new file mode 100644
>> index 0000000..8c61640
>> --- /dev/null
>> +++ b/drivers/mfd/tps65218.c
>> @@ -0,0 +1,281 @@
>> +/*
>> + * TPS65218 chip family multi-function driver
> Instead of calling it an MFD driver (is there such a thing?) I think
> you should mention its true purpose, as per the datasheet.

Ok.

>
> <snip>
>
>> +static const struct of_device_id of_tps65218_match_table[] = {
>> +	{ .compatible = "ti,tps65218", },
>> +	{ /* end */ }
> I think the end comment is superfluous.
>
> However, if you _really_ want to put something in there, I dislike
> "/* Sentinel */" the least.

I will remove it.
>
>> +static int tps65218_probe(struct i2c_client *client,
>> +				const struct i2c_device_id *ids)
>> +{
> <snip>
>
>> +	ret = of_platform_populate(client->dev.of_node, NULL, NULL,
>> +				   &client->dev);
> What are you trying to do here?
>
> Register each regulator as a platform device?

Yeah. The probe will be called for every regulator separately.

> <snip>
>
>> +static const struct i2c_device_id tps65218_id_table[] = {
>> +	{"tps65218", TPS65218},
>> +};
> This has a different structure to of_tps65218_match_table, please
> ensure they're the same. I prefer spaces after '{' and before '}'.

Ok. I will add a space after '{' and before '}'. Sorry I did not
follow tps65218_id_table being different than of_tps65218_match_table.

One is of the type of_device_id and the other i2c_device_id.
>
> <snip>
>


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/4] mfd: DT bindings for TPS65218 PMIC
  2013-12-03  9:36   ` Lee Jones
@ 2013-12-03 10:52     ` Keerthy
  2013-12-03 11:04       ` Lee Jones
  0 siblings, 1 reply; 16+ messages in thread
From: Keerthy @ 2013-12-03 10:52 UTC (permalink / raw)
  To: Lee Jones
  Cc: Keerthy, rob.herring, pawel.moll, mark.rutland, swarren,
	ijc+devicetree, rob, sameo, grant.likely, lgirdwood, broonie,
	devicetree, linux-doc, linux-kernel, linux-omap

On Tuesday 03 December 2013 03:06 PM, Lee Jones wrote:
>> Add DT bindings for TPS65218 PMIC.
>>
>> Signed-off-by: Keerthy <j-keerthy@ti.com>
>> ---
>>   Documentation/devicetree/bindings/mfd/tps65218.txt |   27 ++++++++++++++++++++
>>   .../devicetree/bindings/regulator/tps65218.txt     |   22 ++++++++++++++++
>>   2 files changed, 49 insertions(+)
>>   create mode 100644 Documentation/devicetree/bindings/mfd/tps65218.txt
>>   create mode 100644 Documentation/devicetree/bindings/regulator/tps65218.txt
>>
>> diff --git a/Documentation/devicetree/bindings/mfd/tps65218.txt b/Documentation/devicetree/bindings/mfd/tps65218.txt
>> new file mode 100644
>> index 0000000..87cb7a8
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/mfd/tps65218.txt
>> @@ -0,0 +1,27 @@
>> +The TPS65218 Integrated Power Management Chips.
>> +These chips are connected to an i2c bus.
>                                     I2C

Ok.

>> +Required properties:
>> +- compatible : Must be "ti,tps65218";
>> +- interrupts : This i2c device has an IRQ line connected to the main SoC
>                         I2C

Ok.

>> +- interrupt-controller : Since the tps65218 support several interrupts
>                                                 support(s) (hosts?)

Yes Supports multiple interrupts from the sub-modules.

>> +  internally, it is considered as an interrupt controller cascaded to the SoC.
>> +- #interrupt-cells = <2>;
>> +- interrupt-parent : The parent interrupt controller.
>             Phandle to ...
>
>> +Optional node:
>              node(s):

Ok.

>
>> +- Child nodes contain in the tps65218.
>> +  It supports a number of features.
> Please re-phase the above two sentences to something decipherable.

Ok.

>> +  The children nodes will thus depend of the capability of the variant.
> Please, go on ...

Ok.

>> +Example:
>> +/*
>> + * Integrated Power Management Chip
>> + */
>> +tps@24 {
>> +    compatible = "ti,tps65218";
>> +    reg = <0x24>;
>> +    interrupt-controller;
>> +    #interrupt-cells = <2>;
>> +    interrupt-parent = <&gic>;
>> +};
> Perhaps it would be better to centralise the documentation inclusive
> of the regulator contingent. Or at least provide a _full_ example in
> each document.

Ok.

>
>> diff --git a/Documentation/devicetree/bindings/regulator/tps65218.txt b/Documentation/devicetree/bindings/regulator/tps65218.txt
>> new file mode 100644
>> index 0000000..1ccf170
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/regulator/tps65218.txt
>> @@ -0,0 +1,22 @@
>> +TPS65218 family of regulators
>> +
>> +Required properties:
>> +For tps65218 regulators/LDOs
>> +- compatible:
>> +  - "ti,tps65218-dcdc1" for DCDC1
>> +  - "ti,tps65218-dcdc2" for DCDC2
>> +  - "ti,tps65218-dcdc3" for DCDC3
>> +  - "ti,tps65218-dcdc4" for DCDC4
>> +  - "ti,tps65218-dcdc5" for DCDC5
>> +  - "ti,tps65218-dcdc6" for DCDC6
>> +  - "ti,tps65218-ldo1" for LDO1 LDO
> Why aren't you using 'regulator-compatible'?

I referred tps65217.txt the predecessors and used the compatible.
I also referred twl-regulator.txt.

>
>> +Optional properties:
>> +- Any optional property defined in bindings/regulator/regulator.txt
>> +
>> +Example:
>> +	xyz: regulator@0 {
> Genuine question: Is the @0 meaningful?

@0 is not the best example. @24 is what i use on the am43x.
I will change it to @24.

>> +		compatible = "ti,tps65218-dcdc1";
>> +		regulator-min-microvolt  = <1000000>;
>> +		regulator-max-microvolt  = <3000000>;
>> +	};

Thanks for reviewing.

Kind Regards,
Keerthy

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/4] mfd: DT bindings for TPS65218 PMIC
  2013-12-03 10:52     ` Keerthy
@ 2013-12-03 11:04       ` Lee Jones
  0 siblings, 0 replies; 16+ messages in thread
From: Lee Jones @ 2013-12-03 11:04 UTC (permalink / raw)
  To: Keerthy
  Cc: Keerthy, rob.herring, pawel.moll, mark.rutland, swarren,
	ijc+devicetree, rob, sameo, grant.likely, lgirdwood, broonie,
	devicetree, linux-doc, linux-kernel, linux-omap

> >>+- interrupt-controller : Since the tps65218 support several interrupts
> >                                                support(s) (hosts?)
> 
> Yes Supports multiple interrupts from the sub-modules.

I think 'support' should be the plural 'supports'.

<snip>

> >Why aren't you using 'regulator-compatible'?
> 
> I referred tps65217.txt the predecessors and used the compatible.
> I also referred twl-regulator.txt.

That doesn't mean that they are correct/better. ;)

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 2/4] MFD: TPS65218: Add driver for the TPS65218 PMIC
  2013-12-03 10:06     ` Keerthy
@ 2013-12-03 11:21       ` Lee Jones
  2013-12-03 13:28         ` Mark Brown
  0 siblings, 1 reply; 16+ messages in thread
From: Lee Jones @ 2013-12-03 11:21 UTC (permalink / raw)
  To: Keerthy
  Cc: Keerthy, rob.herring, pawel.moll, mark.rutland, swarren,
	ijc+devicetree, rob, sameo, grant.likely, lgirdwood, broonie,
	devicetree, linux-doc, linux-kernel, linux-omap

> >>+	ret = of_platform_populate(client->dev.of_node, NULL, NULL,
> >>+				   &client->dev);
> >What are you trying to do here?
> >
> >Register each regulator as a platform device?
> 
> Yeah. The probe will be called for every regulator separately.

So there are two schools of thought on this. One is that if you want
to do it that way you should use the MFD framework to do this for you.
There is no need to recreate functionality that already exists. The
other is that you shouldn't be doing this at all with regulators. Mark
likes the idea of having a single regulator controller node which
contains all of these individual regulator sub-nodes and you initiate
a single call to for_each_child_of_node() within the driver in order
to register them all.

<snip>

> >This has a different structure to of_tps65218_match_table, please
> >ensure they're the same. I prefer spaces after '{' and before '}'.
> 
> Ok. I will add a space after '{' and before '}'. Sorry I did not
> follow tps65218_id_table being different than of_tps65218_match_table.
> 
> One is of the type of_device_id and the other i2c_device_id.

Shouldn't matter, it's just a formatting thing.

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 2/4] MFD: TPS65218: Add driver for the TPS65218 PMIC
  2013-12-03 11:21       ` Lee Jones
@ 2013-12-03 13:28         ` Mark Brown
  0 siblings, 0 replies; 16+ messages in thread
From: Mark Brown @ 2013-12-03 13:28 UTC (permalink / raw)
  To: Lee Jones
  Cc: Keerthy, Keerthy, rob.herring, pawel.moll, mark.rutland, swarren,
	ijc+devicetree, rob, sameo, grant.likely, lgirdwood, devicetree,
	linux-doc, linux-kernel, linux-omap

[-- Attachment #1: Type: text/plain, Size: 628 bytes --]

On Tue, Dec 03, 2013 at 11:21:07AM +0000, Lee Jones wrote:

> other is that you shouldn't be doing this at all with regulators. Mark
> likes the idea of having a single regulator controller node which
> contains all of these individual regulator sub-nodes and you initiate
> a single call to for_each_child_of_node() within the driver in order
> to register them all.

That's not really the case, it depends on how the hardware is
structured.  If the regulators are not laid out in a regular fashion or
otherwise reusable then the above big bag of regulators makes sense.  If
the hardware is reusable then things are different.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 3/4] Regulators: TPS65218: Add Regulator driver for TPS65218 PMIC
  2013-12-03  6:43 ` [PATCH 3/4] Regulators: TPS65218: Add Regulator driver for " Keerthy
@ 2013-12-03 15:16   ` Mark Brown
       [not found]     ` <20131203151654.GB27568-GFdadSzt00ze9xe1eoZjHA@public.gmane.org>
  2013-12-04  7:39   ` Manish Badarkhe
  1 sibling, 1 reply; 16+ messages in thread
From: Mark Brown @ 2013-12-03 15:16 UTC (permalink / raw)
  To: Keerthy
  Cc: rob.herring, pawel.moll, mark.rutland, swarren, ijc+devicetree,
	rob, sameo, lee.jones, grant.likely, lgirdwood, devicetree,
	linux-doc, linux-kernel, linux-omap

[-- Attachment #1: Type: text/plain, Size: 1127 bytes --]

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 + 900000;
> +	else
> +		uV = (vsel - 26) * 50000 + 1550000;
> +
> +	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.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 2/4] MFD: TPS65218: Add driver for the TPS65218 PMIC
  2013-12-03  6:43 ` [PATCH 2/4] MFD: TPS65218: Add driver for the " Keerthy
  2013-12-03  9:24   ` Lee Jones
@ 2013-12-03 15:19   ` Mark Brown
  1 sibling, 0 replies; 16+ messages in thread
From: Mark Brown @ 2013-12-03 15:19 UTC (permalink / raw)
  To: Keerthy
  Cc: rob.herring, pawel.moll, mark.rutland, swarren, ijc+devicetree,
	rob, sameo, lee.jones, grant.likely, lgirdwood, devicetree,
	linux-doc, linux-kernel, linux-omap

[-- Attachment #1: Type: text/plain, Size: 274 bytes --]

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

> +static struct regmap_config tps65218_regmap_config = {
> +	.reg_bits = 8,
> +	.val_bits = 8,
> +};

You should consider using a register cache for at least the interrupt
mask registers, it'll improve performance.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 3/4] Regulators: TPS65218: Add Regulator driver for TPS65218 PMIC
       [not found]     ` <20131203151654.GB27568-GFdadSzt00ze9xe1eoZjHA@public.gmane.org>
@ 2013-12-04  5:59       ` Keerthy
  0 siblings, 0 replies; 16+ messages in thread
From: Keerthy @ 2013-12-04  5:59 UTC (permalink / raw)
  To: Mark Brown
  Cc: Keerthy, rob.herring-bsGFqQB8/DxBDgjK7y7TUQ,
	pawel.moll-5wv7dgnIgG8, mark.rutland-5wv7dgnIgG8,
	swarren-3lzwWm7+Weoh9ZMKESR00Q,
	ijc+devicetree-KcIKpvwj1kUDXYZnReoRVg, rob-VoJi6FS/r0vR7s880joybQ,
	sameo-VuQAYsv1563Yd54FQh9/CA, lee.jones-QSEj5FYQhm4dnm+yROfE0A,
	grant.likely-QSEj5FYQhm4dnm+yROfE0A,
	lgirdwood-Re5JQEeQqe8AvxtiuMwx3w,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-doc-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-omap-u79uwXL29TY76Z2rM5mHXA

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 + 900000;
>> +	else
>> +		uV = (vsel - 26) * 50000 + 1550000;
>> +
>> +	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 devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 3/4] Regulators: TPS65218: Add Regulator driver for TPS65218 PMIC
  2013-12-03  6:43 ` [PATCH 3/4] Regulators: TPS65218: Add Regulator driver for " Keerthy
  2013-12-03 15:16   ` Mark Brown
@ 2013-12-04  7:39   ` Manish Badarkhe
  1 sibling, 0 replies; 16+ messages in thread
From: Manish Badarkhe @ 2013-12-04  7:39 UTC (permalink / raw)
  To: Keerthy
  Cc: Rob Herring, pawel.moll, Mark Rutland, Stephen Warren,
	ijc+devicetree, Rob Landley, sameo, lee.jones, grant.likely,
	Liam Girdwood, Mark Brown, devicetree@vger.kernel.org,
	linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-omap

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2013-12-04  7:39 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-12-03  6:43 [PATCH 0/4] mfd: TPS65218: Add support for TPS65218 PMIC Keerthy
2013-12-03  6:43 ` [PATCH 1/4] mfd: DT bindings " Keerthy
2013-12-03  9:36   ` Lee Jones
2013-12-03 10:52     ` Keerthy
2013-12-03 11:04       ` Lee Jones
2013-12-03  6:43 ` [PATCH 2/4] MFD: TPS65218: Add driver for the " Keerthy
2013-12-03  9:24   ` Lee Jones
2013-12-03 10:06     ` Keerthy
2013-12-03 11:21       ` Lee Jones
2013-12-03 13:28         ` Mark Brown
2013-12-03 15:19   ` Mark Brown
2013-12-03  6:43 ` [PATCH 3/4] Regulators: TPS65218: Add Regulator driver for " Keerthy
2013-12-03 15:16   ` Mark Brown
     [not found]     ` <20131203151654.GB27568-GFdadSzt00ze9xe1eoZjHA@public.gmane.org>
2013-12-04  5:59       ` Keerthy
2013-12-04  7:39   ` Manish Badarkhe
2013-12-03  6:43 ` [PATCH 4/4] ARM: dts: AM437x: Add TPS65218 device tree nodes Keerthy

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).