2012-05-08 15:43:32

by Rhyland Klein

[permalink] [raw]
Subject: [PATCH 0/6 v3] Update TPS65910 to boot using devicetree

This patch set updates the tps65910 driver to boot using devicetree.

This patch set now uses the new addition to the of_regulator code,
of_regulator_match to find all the regulator init data for the device.

Rhyland Klein (6):
mfd: tps65910: Commonize regmap access through header
regulator: tps65910: Add device tree bindings
mfd: tps65910: Add device-tree support
regulator: tps65910 regulator: add device tree support
mfd: tps65910-irq: Add devicetree init support
ARM: Tegra: Add support for TPS65910 PMIC

Documentation/devicetree/bindings/mfd/tps65910.txt | 133 ++++++++++++++
arch/arm/boot/dts/tegra-cardhu.dts | 91 ++++++++++
drivers/gpio/gpio-tps65910.c | 14 +-
drivers/mfd/tps65910-irq.c | 55 ++++---
drivers/mfd/tps65910.c | 120 +++++++++----
drivers/regulator/tps65910-regulator.c | 190 +++++++++++++++-----
include/linux/mfd/tps65910.h | 29 +++-
7 files changed, 523 insertions(+), 109 deletions(-)
create mode 100644 Documentation/devicetree/bindings/mfd/tps65910.txt


2012-05-08 15:43:42

by Rhyland Klein

[permalink] [raw]
Subject: [PATCH 2/6 v3] regulator: tps65910: Add device tree bindings

Add device tree bindings for TI's tps65910 pmic.

Signed-off-by: Rhyland Klein <[email protected]>
---
v3: No change since v2.
v2: Moved ext-sleep-control into regulator nodes

Documentation/devicetree/bindings/mfd/tps65910.txt | 133 ++++++++++++++++++++
1 files changed, 133 insertions(+), 0 deletions(-)
create mode 100644 Documentation/devicetree/bindings/mfd/tps65910.txt

diff --git a/Documentation/devicetree/bindings/mfd/tps65910.txt b/Documentation/devicetree/bindings/mfd/tps65910.txt
new file mode 100644
index 0000000..645f5ea
--- /dev/null
+++ b/Documentation/devicetree/bindings/mfd/tps65910.txt
@@ -0,0 +1,133 @@
+TPS65910 Power Management Integrated Circuit
+
+Required properties:
+- compatible: "ti,tps65910" or "ti,tps65911"
+- reg: I2C slave address
+- interrupts: the interrupt outputs of the controller
+- #gpio-cells: number of cells to describe a GPIO, this should be 2.
+ The first cell is the GPIO number.
+ The second cell is used to specify additional options <unused>.
+- gpio-controller: mark the device as a GPIO controller
+- #interrupt-cells: the number of cells to describe an IRQ, this should be 2.
+ The first cell is the IRQ number.
+ The second cell is the flags, encoded as the trigger masks from
+ Documentation/devicetree/bindings/interrupts.txt
+- regulators: This is the list of child nodes that specify the regulator
+ initialization data for defined regulators. Not all regulators for the given
+ device need to be present. The definition for each of these nodes is defined
+ using the standard binding for regulators found at
+ Documentation/devicetree/bindings/regulator/regulator.txt.
+
+ The valid names for regulators are:
+ tps65910: vrtc, vio, vdd1, vdd2, vdd3, vdig1, vdig2, vpll, vdac, vaux1,
+ vaux2, vaux33, vmmc
+ tps65911: vrtc, vio, vdd1, vdd3, vddctrl, ldo1, ldo2, ldo3, ldo4, ldo5,
+ ldo6, ldo7, ldo8
+
+Optional properties:
+- ti,vmbch-threshold: (tps65911) main battery charged threshold
+ comparator. (see VMBCH_VSEL in TPS65910 datasheet)
+- ti,vmbch2-threshold: (tps65911) main battery discharged threshold
+ comparator. (see VMBCH_VSEL in TPS65910 datasheet)
+- ti,en-gpio-sleep: enable sleep control for gpios
+ There should be 9 entries here, one for each gpio.
+
+Regulator Optional properties:
+- ti,regulator-ext-sleep-control: enable external sleep
+ control through external inputs [0 (not enabled), 1 (EN1), 2 (EN2) or 4(EN3)]
+ If this property is not defined, it defaults to 0 (not enabled).
+
+Example:
+
+ pmu: tps65910@d2 {
+ compatible = "ti,tps65910";
+ reg = <0xd2>;
+ interrupt-parent = <&intc>;
+ interrupts = < 0 118 0x04 >;
+
+ #gpio-cells = <2>;
+ gpio-controller;
+
+ #interrupt-cells = <2>;
+ interrupt-controller;
+
+ ti,vmbch-threshold = 0;
+ ti,vmbch2-threshold = 0;
+
+ ti,en-gpio-sleep = <0 0 1 0 0 0 0 0 0>;
+
+ regulators {
+ vdd1_reg: vdd1 {
+ regulator-min-microvolt = < 600000>;
+ regulator-max-microvolt = <1500000>;
+ regulator-always-on;
+ regulator-boot-on;
+ ti,regulator-ext-sleep-control = <0>;
+ };
+ vdd2_reg: vdd2 {
+ regulator-min-microvolt = < 600000>;
+ regulator-max-microvolt = <1500000>;
+ regulator-always-on;
+ regulator-boot-on;
+ ti,regulator-ext-sleep-control = <4>;
+ };
+ vddctrl_reg: vddctrl {
+ regulator-min-microvolt = < 600000>;
+ regulator-max-microvolt = <1400000>;
+ regulator-always-on;
+ regulator-boot-on;
+ ti,regulator-ext-sleep-control = <0>;
+ };
+ vio_reg: vio {
+ regulator-min-microvolt = <1500000>;
+ regulator-max-microvolt = <1800000>;
+ regulator-always-on;
+ regulator-boot-on;
+ ti,regulator-ext-sleep-control = <1>;
+ };
+ ldo1_reg: ldo1 {
+ regulator-min-microvolt = <1000000>;
+ regulator-max-microvolt = <3300000>;
+ ti,regulator-ext-sleep-control = <0>;
+ };
+ ldo2_reg: ldo2 {
+ regulator-min-microvolt = <1050000>;
+ regulator-max-microvolt = <1050000>;
+ ti,regulator-ext-sleep-control = <0>;
+ };
+ ldo3_reg: ldo3 {
+ regulator-min-microvolt = <1000000>;
+ regulator-max-microvolt = <3300000>;
+ ti,regulator-ext-sleep-control = <0>;
+ };
+ ldo4_reg: ldo4 {
+ regulator-min-microvolt = <1000000>;
+ regulator-max-microvolt = <3300000>;
+ regulator-always-on;
+ ti,regulator-ext-sleep-control = <0>;
+ };
+ ldo5_reg: ldo5 {
+ regulator-min-microvolt = <1000000>;
+ regulator-max-microvolt = <3300000>;
+ ti,regulator-ext-sleep-control = <0>;
+ };
+ ldo6_reg: ldo6 {
+ regulator-min-microvolt = <1200000>;
+ regulator-max-microvolt = <1200000>;
+ ti,regulator-ext-sleep-control = <0>;
+ };
+ ldo7_reg: ldo7 {
+ regulator-min-microvolt = <1200000>;
+ regulator-max-microvolt = <1200000>;
+ regulator-always-on;
+ regulator-boot-on;
+ ti,regulator-ext-sleep-control = <1>;
+ };
+ ldo8_reg: ldo8 {
+ regulator-min-microvolt = <1000000>;
+ regulator-max-microvolt = <3300000>;
+ regulator-always-on;
+ ti,regulator-ext-sleep-control = <1>;
+ };
+ };
+ };
--
1.7.0.4

2012-05-08 15:43:59

by Rhyland Klein

[permalink] [raw]
Subject: [PATCH 6/6 v3] ARM: Tegra: Add support for TPS65910 PMIC

Add support for the tps65910 pmic on cardhu.

Signed-off-by: Rhyland Klein <[email protected]>
---
v3: no change from v2.
v2: updated to move ext-sleep-control into the regulator nodes

arch/arm/boot/dts/tegra-cardhu.dts | 91 ++++++++++++++++++++++++++++++++++++
1 files changed, 91 insertions(+), 0 deletions(-)

diff --git a/arch/arm/boot/dts/tegra-cardhu.dts b/arch/arm/boot/dts/tegra-cardhu.dts
index facb950..ae1afe9 100644
--- a/arch/arm/boot/dts/tegra-cardhu.dts
+++ b/arch/arm/boot/dts/tegra-cardhu.dts
@@ -123,6 +123,97 @@
micdet-delay = <100>;
gpio-cfg = <0xffffffff 0xffffffff 0 0xffffffff 0xffffffff>;
};
+
+ pmic: tps65911@2d {
+ compatible = "ti,tps65911";
+ reg = <0x2d>;
+ interrupts = < 0 118 0x04 >;
+
+ #gpio-cells = <2>;
+ gpio-controller;
+
+ #interrupt-cells = <2>;
+ interrupt-controller;
+
+ ti,vmbch-threshold = <0>;
+ ti,vmbch2-threshold = <0>;
+ ti,en-gpio-sleep = <0 0 1 0 0 0 0 0 0>;
+
+ regulators {
+ vdd1_reg: vdd1 {
+ regulator-min-microvolt = < 600000>;
+ regulator-max-microvolt = <1500000>;
+ regulator-always-on;
+ regulator-boot-on;
+ ti,regulator-ext-sleep-control = <4>;
+ };
+ vdd2_reg: vdd2 {
+ regulator-min-microvolt = < 600000>;
+ regulator-max-microvolt = <1500000>;
+ regulator-always-on;
+ regulator-boot-on;
+ ti,regulator-ext-sleep-control = <0>;
+ };
+ vddctrl_reg: vddctrl {
+ regulator-min-microvolt = < 600000>;
+ regulator-max-microvolt = <1400000>;
+ regulator-always-on;
+ regulator-boot-on;
+ ti,regulator-ext-sleep-control = <1>;
+ };
+ vio_reg: vio {
+ regulator-min-microvolt = <1500000>;
+ regulator-max-microvolt = <1800000>;
+ regulator-always-on;
+ regulator-boot-on;
+ ti,regulator-ext-sleep-control = <0>;
+ };
+ ldo1_reg: ldo1 {
+ regulator-min-microvolt = <1000000>;
+ regulator-max-microvolt = <3300000>;
+ ti,regulator-ext-sleep-control = <0>;
+ };
+ ldo2_reg: ldo2 {
+ regulator-min-microvolt = <1050000>;
+ regulator-max-microvolt = <1050000>;
+ ti,regulator-ext-sleep-control = <0>;
+ };
+ ldo3_reg: ldo3 {
+ regulator-min-microvolt = <1000000>;
+ regulator-max-microvolt = <3300000>;
+ ti,regulator-ext-sleep-control = <0>;
+ };
+ ldo4_reg: ldo4 {
+ regulator-min-microvolt = <1000000>;
+ regulator-max-microvolt = <3300000>;
+ regulator-always-on;
+ ti,regulator-ext-sleep-control = <0>;
+ };
+ ldo5_reg: ldo5 {
+ regulator-min-microvolt = <1000000>;
+ regulator-max-microvolt = <3300000>;
+ ti,regulator-ext-sleep-control = <0>;
+ };
+ ldo6_reg: ldo6 {
+ regulator-min-microvolt = <1200000>;
+ regulator-max-microvolt = <1200000>;
+ ti,regulator-ext-sleep-control = <0>;
+ };
+ ldo7_reg: ldo7 {
+ regulator-min-microvolt = <1200000>;
+ regulator-max-microvolt = <1200000>;
+ regulator-always-on;
+ regulator-boot-on;
+ ti,regulator-ext-sleep-control = <1>;
+ };
+ ldo8_reg: ldo8 {
+ regulator-min-microvolt = <1000000>;
+ regulator-max-microvolt = <3300000>;
+ regulator-always-on;
+ ti,regulator-ext-sleep-control = <1>;
+ };
+ };
+ };
};

sdhci@78000000 {
--
1.7.0.4

2012-05-08 15:43:58

by Rhyland Klein

[permalink] [raw]
Subject: [PATCH 3/6 v3] mfd: tps65910: Add device-tree support

Add device tree based initialization support for TI's tps65910 pmic.

Signed-off-by: Rhyland Klein <[email protected]>
---
v3: no change since v2.
v2: split mfd portion into single change to add dt support
added of_node to config before calling regulator_register
removed passing pdata to mfd_cell child devices
removed use_dt_for_init_data flag

drivers/mfd/tps65910.c | 80 +++++++++++++++++++++++++++++++++++++++++++++++-
1 files changed, 79 insertions(+), 1 deletions(-)

diff --git a/drivers/mfd/tps65910.c b/drivers/mfd/tps65910.c
index f221153..4b52e95 100644
--- a/drivers/mfd/tps65910.c
+++ b/drivers/mfd/tps65910.c
@@ -23,6 +23,7 @@
#include <linux/mfd/core.h>
#include <linux/regmap.h>
#include <linux/mfd/tps65910.h>
+#include <linux/of_device.h>

static struct mfd_cell tps65910s[] = {
{
@@ -129,6 +130,77 @@ err_sleep_init:
return ret;
}

+#ifdef CONFIG_OF
+static struct of_device_id tps65910_of_match[] = {
+ { .compatible = "ti,tps65910", .data = (void *)TPS65910},
+ { .compatible = "ti,tps65911", .data = (void *)TPS65911},
+ { },
+};
+MODULE_DEVICE_TABLE(of, tps65910_of_match);
+
+static struct tps65910_board *tps65910_parse_dt(struct i2c_client *client,
+ int *chip_id)
+{
+ struct device_node *np = client->dev.of_node;
+ struct tps65910_board *board_info;
+ unsigned int prop;
+ const struct of_device_id *match;
+ unsigned int prop_array[TPS6591X_MAX_NUM_GPIO];
+ int ret = 0;
+ int idx;
+
+ match = of_match_device(tps65910_of_match, &client->dev);
+ if (!match) {
+ dev_err(&client->dev, "Failed to find matching dt id\n");
+ return NULL;
+ }
+
+ *chip_id = (int)match->data;
+
+ board_info = devm_kzalloc(&client->dev, sizeof(*board_info),
+ GFP_KERNEL);
+ if (!board_info) {
+ dev_err(&client->dev, "Failed to allocate pdata\n");
+ return NULL;
+ }
+
+ ret = of_property_read_u32(np, "ti,vmbch-threshold", &prop);
+ if (!ret)
+ board_info->vmbch_threshold = prop;
+ else if (*chip_id == TPS65911)
+ dev_warn(&client->dev, "VMBCH-Threshold not specified");
+
+ ret = of_property_read_u32(np, "ti,vmbch2-threshold", &prop);
+ if (!ret)
+ board_info->vmbch2_threshold = prop;
+ else if (*chip_id == TPS65911)
+ dev_warn(&client->dev, "VMBCH2-Threshold not specified");
+
+ ret = of_property_read_u32_array(np, "ti,en-gpio-sleep",
+ prop_array, TPS6591X_MAX_NUM_GPIO);
+ if (!ret)
+ for (idx = 0; idx < ARRAY_SIZE(prop_array); idx++)
+ board_info->en_gpio_sleep[idx] = (prop_array[idx] != 0);
+ else if (ret != -EINVAL) {
+ dev_err(&client->dev,
+ "error reading property ti,en-gpio-sleep: %d\n.", ret);
+ return NULL;
+ }
+
+
+ board_info->irq = client->irq;
+ board_info->irq_base = -1;
+ board_info->gpio_base = -1;
+
+ return board_info;
+}
+#else
+static inline struct tps65910_board *tps65910_parse_dt(
+ struct i2c_client *client)
+{
+ return NULL;
+}
+#endif

static int tps65910_i2c_probe(struct i2c_client *i2c,
const struct i2c_device_id *id)
@@ -137,8 +209,13 @@ static int tps65910_i2c_probe(struct i2c_client *i2c,
struct tps65910_board *pmic_plat_data;
struct tps65910_platform_data *init_data;
int ret = 0;
+ int chip_id = id->driver_data;

pmic_plat_data = dev_get_platdata(&i2c->dev);
+
+ if (!pmic_plat_data && i2c->dev.of_node)
+ pmic_plat_data = tps65910_parse_dt(i2c, &chip_id);
+
if (!pmic_plat_data)
return -EINVAL;

@@ -155,7 +232,7 @@ static int tps65910_i2c_probe(struct i2c_client *i2c,
i2c_set_clientdata(i2c, tps65910);
tps65910->dev = &i2c->dev;
tps65910->i2c_client = i2c;
- tps65910->id = id->driver_data;
+ tps65910->id = chip_id;
mutex_init(&tps65910->io_mutex);

tps65910->regmap = regmap_init_i2c(i2c, &tps65910_regmap_config);
@@ -215,6 +292,7 @@ static struct i2c_driver tps65910_i2c_driver = {
.driver = {
.name = "tps65910",
.owner = THIS_MODULE,
+ .of_match_table = of_match_ptr(tps65910_of_match),
},
.probe = tps65910_i2c_probe,
.remove = tps65910_i2c_remove,
--
1.7.0.4

2012-05-08 15:43:56

by Rhyland Klein

[permalink] [raw]
Subject: [PATCH 4/6 v3] regulator: tps65910 regulator: add device tree support

Add devicetree based initialization support for TI tps65910 regulators.

Signed-off-by: Rhyland Klein <[email protected]>
---
v3: Modified to use new api of_regulator_match to fetch all regulator
init data from device node.

v2: split off regulator specific dt initialization

drivers/regulator/tps65910-regulator.c | 108 ++++++++++++++++++++++++++++++++
1 files changed, 108 insertions(+), 0 deletions(-)

diff --git a/drivers/regulator/tps65910-regulator.c b/drivers/regulator/tps65910-regulator.c
index 09fb5cd..67791e1 100644
--- a/drivers/regulator/tps65910-regulator.c
+++ b/drivers/regulator/tps65910-regulator.c
@@ -23,6 +23,7 @@
#include <linux/slab.h>
#include <linux/gpio.h>
#include <linux/mfd/tps65910.h>
+#include <linux/regulator/of_regulator.h>

#define TPS65910_SUPPLY_STATE_ENABLED 0x1
#define EXT_SLEEP_CONTROL (TPS65910_SLEEP_CONTROL_EXT_INPUT_EN1 | \
@@ -1047,6 +1048,105 @@ static int tps65910_set_ext_sleep_config(struct tps65910_reg *pmic,
return ret;
}

+#ifdef CONFIG_OF
+
+static struct of_regulator_match tps65910_matches[] = {
+ { .name = "VRTC", .driver_data = (void *) &tps65910_regs[0] },
+ { .name = "VIO", .driver_data = (void *) &tps65910_regs[1] },
+ { .name = "VDD1", .driver_data = (void *) &tps65910_regs[2] },
+ { .name = "VDD2", .driver_data = (void *) &tps65910_regs[3] },
+ { .name = "VDD3", .driver_data = (void *) &tps65910_regs[4] },
+ { .name = "VDIG1", .driver_data = (void *) &tps65910_regs[5] },
+ { .name = "VDIG2", .driver_data = (void *) &tps65910_regs[6] },
+ { .name = "VPLL", .driver_data = (void *) &tps65910_regs[7] },
+ { .name = "VDAC", .driver_data = (void *) &tps65910_regs[8] },
+ { .name = "VAUX1", .driver_data = (void *) &tps65910_regs[9] },
+ { .name = "VAUX2", .driver_data = (void *) &tps65910_regs[10] },
+ { .name = "VAUX33", .driver_data = (void *) &tps65910_regs[11] },
+ { .name = "VMMC", .driver_data = (void *) &tps65910_regs[12] },
+};
+
+static struct of_regulator_match tps65911_matches[] = {
+ { .name = "VRTC", .driver_data = (void *) &tps65911_regs[0] },
+ { .name = "VIO", .driver_data = (void *) &tps65911_regs[1] },
+ { .name = "VDD1", .driver_data = (void *) &tps65911_regs[2] },
+ { .name = "VDD2", .driver_data = (void *) &tps65911_regs[3] },
+ { .name = "VDDCTRL", .driver_data = (void *) &tps65911_regs[4] },
+ { .name = "LDO1", .driver_data = (void *) &tps65911_regs[5] },
+ { .name = "LDO2", .driver_data = (void *) &tps65911_regs[6] },
+ { .name = "LDO3", .driver_data = (void *) &tps65911_regs[7] },
+ { .name = "LDO4", .driver_data = (void *) &tps65911_regs[8] },
+ { .name = "LDO5", .driver_data = (void *) &tps65911_regs[9] },
+ { .name = "LDO6", .driver_data = (void *) &tps65911_regs[10] },
+ { .name = "LDO7", .driver_data = (void *) &tps65911_regs[11] },
+ { .name = "LDO8", .driver_data = (void *) &tps65911_regs[12] },
+};
+
+static struct tps65910_board *tps65910_parse_dt_reg_data(
+ struct platform_device *pdev)
+{
+ struct tps65910_board *pmic_plat_data;
+ struct tps65910 *tps65910 = dev_get_drvdata(pdev->dev.parent);
+ struct device_node *np = pdev->dev.parent->of_node;
+ struct device_node *regulators;
+ struct of_regulator_match *matches;
+ unsigned int prop;
+ int idx = 0, ret, count;
+
+ pmic_plat_data = devm_kzalloc(&pdev->dev, sizeof(*pmic_plat_data),
+ GFP_KERNEL);
+
+ if (!pmic_plat_data) {
+ dev_err(&pdev->dev, "Failure to alloc pdata for regulators.\n");
+ return NULL;
+ }
+
+ regulators = of_find_node_by_name(np, "regulators");
+
+ switch (tps65910_chip_id(tps65910)) {
+ case TPS65910:
+ count = ARRAY_SIZE(tps65910_matches);
+ matches = tps65910_matches;
+ break;
+ case TPS65911:
+ count = ARRAY_SIZE(tps65911_matches);
+ matches = tps65911_matches;
+ break;
+ default:
+ pr_err("Invalid tps chip version\n");
+ return NULL;
+ }
+
+ ret = of_regulator_match(pdev->dev.parent, regulators, matches, count);
+ if (ret < 0) {
+ dev_err(&pdev->dev, "Error parsing regulator init data: %d\n",
+ ret);
+ return NULL;
+ }
+
+ for (idx = 0; idx < count; idx++) {
+ if (!matches[idx].init_data || !matches[idx].of_node)
+ continue;
+
+ pmic_plat_data->tps65910_pmic_init_data[idx] =
+ matches[idx].init_data;
+
+ ret = of_property_read_u32(matches[idx].of_node,
+ "ti,regulator-ext-sleep-control", &prop);
+ if (!ret)
+ pmic_plat_data->regulator_ext_sleep_control[idx] = prop;
+ }
+
+ return pmic_plat_data;
+}
+#else
+static inline struct tps65910_board *tps65910_parse_dt_reg_data(
+ struct platform_device *pdev)
+{
+ return 0;
+}
+#endif
+
static __devinit int tps65910_probe(struct platform_device *pdev)
{
struct tps65910 *tps65910 = dev_get_drvdata(pdev->dev.parent);
@@ -1059,6 +1159,9 @@ static __devinit int tps65910_probe(struct platform_device *pdev)
int i, err;

pmic_plat_data = dev_get_platdata(tps65910->dev);
+ if (!pmic_plat_data && tps65910->dev->of_node)
+ pmic_plat_data = tps65910_parse_dt_reg_data(pdev);
+
if (!pmic_plat_data)
return -EINVAL;

@@ -1166,6 +1269,11 @@ static __devinit int tps65910_probe(struct platform_device *pdev)
config.driver_data = pmic;
config.regmap = tps65910->regmap;

+#ifdef CONFIG_OF
+ config.of_node = of_find_node_by_name(tps65910->dev->of_node,
+ info->name);
+#endif
+
rdev = regulator_register(&pmic->desc[i], &config);
if (IS_ERR(rdev)) {
dev_err(tps65910->dev,
--
1.7.0.4

2012-05-08 15:44:41

by Rhyland Klein

[permalink] [raw]
Subject: [PATCH 5/6 v3] mfd: tps65910-irq: Add devicetree init support

Add support for initializing when boot using devicetree. The main difference
is that the irq_base will not be setup, so it needs to be manually handled.

Signed-off-by: Rhyland Klein <[email protected]>
---
v3: no change from v2.
v2: split off irq specific changes based on previous review comments.

drivers/mfd/tps65910-irq.c | 21 +++++++++++++++------
1 files changed, 15 insertions(+), 6 deletions(-)

diff --git a/drivers/mfd/tps65910-irq.c b/drivers/mfd/tps65910-irq.c
index 0f1ff7f..066a30f 100644
--- a/drivers/mfd/tps65910-irq.c
+++ b/drivers/mfd/tps65910-irq.c
@@ -180,12 +180,6 @@ int tps65910_irq_init(struct tps65910 *tps65910, int irq,
return -EINVAL;
}

- tps65910->irq_mask = 0xFFFFFF;
-
- mutex_init(&tps65910->irq_lock);
- tps65910->chip_irq = irq;
- tps65910->irq_base = pdata->irq_base;
-
switch (tps65910_chip_id(tps65910)) {
case TPS65910:
tps65910->irq_num = TPS65910_NUM_IRQ;
@@ -195,6 +189,21 @@ int tps65910_irq_init(struct tps65910 *tps65910, int irq,
break;
}

+ if (pdata->irq_base <= 0)
+ pdata->irq_base = irq_alloc_descs(-1, 0, tps65910->irq_num, -1);
+
+ if (pdata->irq_base <= 0) {
+ dev_err(tps65910->dev, "Failed to allocate irq descs: %d\n",
+ pdata->irq_base);
+ return pdata->irq_base;
+ }
+
+ tps65910->irq_mask = 0xFFFFFF;
+
+ mutex_init(&tps65910->irq_lock);
+ tps65910->chip_irq = irq;
+ tps65910->irq_base = pdata->irq_base;
+
/* Register with genirq */
for (cur_irq = tps65910->irq_base;
cur_irq < tps65910->irq_num + tps65910->irq_base;
--
1.7.0.4

2012-05-08 15:43:55

by Rhyland Klein

[permalink] [raw]
Subject: [PATCH 1/6 v3] mfd: tps65910: Commonize regmap access through header

This change removes the read/write callback functions in favor of common
regmap accessors inside the header file. This change also makes use of
regmap_read/write for single register access which maps better onto what this
driver actually needs.

Signed-off-by: Rhyland Klein <[email protected]>
---
v3: No change from v2 (other than rebased)
v2: implemented switching tps65910 drivers over to more directly accessing
the regmap rather than going through utility functions.

drivers/gpio/gpio-tps65910.c | 14 +++---
drivers/mfd/tps65910-irq.c | 34 +++++++-------
drivers/mfd/tps65910.c | 40 ++++------------
drivers/regulator/tps65910-regulator.c | 82 +++++++++++++++-----------------
include/linux/mfd/tps65910.h | 29 ++++++++++--
5 files changed, 97 insertions(+), 102 deletions(-)

diff --git a/drivers/gpio/gpio-tps65910.c b/drivers/gpio/gpio-tps65910.c
index 7eef648..bc155f2 100644
--- a/drivers/gpio/gpio-tps65910.c
+++ b/drivers/gpio/gpio-tps65910.c
@@ -23,9 +23,9 @@
static int tps65910_gpio_get(struct gpio_chip *gc, unsigned offset)
{
struct tps65910 *tps65910 = container_of(gc, struct tps65910, gpio);
- uint8_t val;
+ unsigned int val;

- tps65910->read(tps65910, TPS65910_GPIO0 + offset, 1, &val);
+ tps65910_reg_read(tps65910, TPS65910_GPIO0 + offset, &val);

if (val & GPIO_STS_MASK)
return 1;
@@ -39,10 +39,10 @@ static void tps65910_gpio_set(struct gpio_chip *gc, unsigned offset,
struct tps65910 *tps65910 = container_of(gc, struct tps65910, gpio);

if (value)
- tps65910_set_bits(tps65910, TPS65910_GPIO0 + offset,
+ tps65910_reg_set_bits(tps65910, TPS65910_GPIO0 + offset,
GPIO_SET_MASK);
else
- tps65910_clear_bits(tps65910, TPS65910_GPIO0 + offset,
+ tps65910_reg_clear_bits(tps65910, TPS65910_GPIO0 + offset,
GPIO_SET_MASK);
}

@@ -54,7 +54,7 @@ static int tps65910_gpio_output(struct gpio_chip *gc, unsigned offset,
/* Set the initial value */
tps65910_gpio_set(gc, offset, value);

- return tps65910_set_bits(tps65910, TPS65910_GPIO0 + offset,
+ return tps65910_reg_set_bits(tps65910, TPS65910_GPIO0 + offset,
GPIO_CFG_MASK);
}

@@ -62,7 +62,7 @@ static int tps65910_gpio_input(struct gpio_chip *gc, unsigned offset)
{
struct tps65910 *tps65910 = container_of(gc, struct tps65910, gpio);

- return tps65910_clear_bits(tps65910, TPS65910_GPIO0 + offset,
+ return tps65910_reg_clear_bits(tps65910, TPS65910_GPIO0 + offset,
GPIO_CFG_MASK);
}

@@ -102,7 +102,7 @@ void tps65910_gpio_init(struct tps65910 *tps65910, int gpio_base)
int i;
for (i = 0; i < tps65910->gpio.ngpio; ++i) {
if (board_data->en_gpio_sleep[i]) {
- ret = tps65910_set_bits(tps65910,
+ ret = tps65910_reg_set_bits(tps65910,
TPS65910_GPIO0 + i, GPIO_SLEEP_MASK);
if (ret < 0)
dev_warn(tps65910->dev,
diff --git a/drivers/mfd/tps65910-irq.c b/drivers/mfd/tps65910-irq.c
index c9ed5c0..0f1ff7f 100644
--- a/drivers/mfd/tps65910-irq.c
+++ b/drivers/mfd/tps65910-irq.c
@@ -41,28 +41,28 @@ static inline int irq_to_tps65910_irq(struct tps65910 *tps65910,
static irqreturn_t tps65910_irq(int irq, void *irq_data)
{
struct tps65910 *tps65910 = irq_data;
+ unsigned int reg;
u32 irq_sts;
u32 irq_mask;
- u8 reg;
int i;

- tps65910->read(tps65910, TPS65910_INT_STS, 1, &reg);
+ tps65910_reg_read(tps65910, TPS65910_INT_STS, &reg);
irq_sts = reg;
- tps65910->read(tps65910, TPS65910_INT_STS2, 1, &reg);
+ tps65910_reg_read(tps65910, TPS65910_INT_STS2, &reg);
irq_sts |= reg << 8;
switch (tps65910_chip_id(tps65910)) {
case TPS65911:
- tps65910->read(tps65910, TPS65910_INT_STS3, 1, &reg);
+ tps65910_reg_read(tps65910, TPS65910_INT_STS3, &reg);
irq_sts |= reg << 16;
}

- tps65910->read(tps65910, TPS65910_INT_MSK, 1, &reg);
+ tps65910_reg_read(tps65910, TPS65910_INT_MSK, &reg);
irq_mask = reg;
- tps65910->read(tps65910, TPS65910_INT_MSK2, 1, &reg);
+ tps65910_reg_read(tps65910, TPS65910_INT_MSK2, &reg);
irq_mask |= reg << 8;
switch (tps65910_chip_id(tps65910)) {
case TPS65911:
- tps65910->read(tps65910, TPS65910_INT_MSK3, 1, &reg);
+ tps65910_reg_read(tps65910, TPS65910_INT_MSK3, &reg);
irq_mask |= reg << 16;
}

@@ -82,13 +82,13 @@ static irqreturn_t tps65910_irq(int irq, void *irq_data)
/* Write the STS register back to clear IRQs we handled */
reg = irq_sts & 0xFF;
irq_sts >>= 8;
- tps65910->write(tps65910, TPS65910_INT_STS, 1, &reg);
+ tps65910_reg_write(tps65910, TPS65910_INT_STS, reg);
reg = irq_sts & 0xFF;
- tps65910->write(tps65910, TPS65910_INT_STS2, 1, &reg);
+ tps65910_reg_write(tps65910, TPS65910_INT_STS2, reg);
switch (tps65910_chip_id(tps65910)) {
case TPS65911:
reg = irq_sts >> 8;
- tps65910->write(tps65910, TPS65910_INT_STS3, 1, &reg);
+ tps65910_reg_write(tps65910, TPS65910_INT_STS3, reg);
}

return IRQ_HANDLED;
@@ -105,27 +105,27 @@ static void tps65910_irq_sync_unlock(struct irq_data *data)
{
struct tps65910 *tps65910 = irq_data_get_irq_chip_data(data);
u32 reg_mask;
- u8 reg;
+ unsigned int reg;

- tps65910->read(tps65910, TPS65910_INT_MSK, 1, &reg);
+ tps65910_reg_read(tps65910, TPS65910_INT_MSK, &reg);
reg_mask = reg;
- tps65910->read(tps65910, TPS65910_INT_MSK2, 1, &reg);
+ tps65910_reg_read(tps65910, TPS65910_INT_MSK2, &reg);
reg_mask |= reg << 8;
switch (tps65910_chip_id(tps65910)) {
case TPS65911:
- tps65910->read(tps65910, TPS65910_INT_MSK3, 1, &reg);
+ tps65910_reg_read(tps65910, TPS65910_INT_MSK3, &reg);
reg_mask |= reg << 16;
}

if (tps65910->irq_mask != reg_mask) {
reg = tps65910->irq_mask & 0xFF;
- tps65910->write(tps65910, TPS65910_INT_MSK, 1, &reg);
+ tps65910_reg_write(tps65910, TPS65910_INT_MSK, reg);
reg = tps65910->irq_mask >> 8 & 0xFF;
- tps65910->write(tps65910, TPS65910_INT_MSK2, 1, &reg);
+ tps65910_reg_write(tps65910, TPS65910_INT_MSK2, reg);
switch (tps65910_chip_id(tps65910)) {
case TPS65911:
reg = tps65910->irq_mask >> 16;
- tps65910->write(tps65910, TPS65910_INT_MSK3, 1, &reg);
+ tps65910_reg_write(tps65910, TPS65910_INT_MSK3, reg);
}
}
mutex_unlock(&tps65910->irq_lock);
diff --git a/drivers/mfd/tps65910.c b/drivers/mfd/tps65910.c
index ae7f47b..f221153 100644
--- a/drivers/mfd/tps65910.c
+++ b/drivers/mfd/tps65910.c
@@ -37,30 +37,6 @@ static struct mfd_cell tps65910s[] = {
};


-static int tps65910_i2c_read(struct tps65910 *tps65910, u8 reg,
- int bytes, void *dest)
-{
- return regmap_bulk_read(tps65910->regmap, reg, dest, bytes);
-}
-
-static int tps65910_i2c_write(struct tps65910 *tps65910, u8 reg,
- int bytes, void *src)
-{
- return regmap_bulk_write(tps65910->regmap, reg, src, bytes);
-}
-
-int tps65910_set_bits(struct tps65910 *tps65910, u8 reg, u8 mask)
-{
- return regmap_update_bits(tps65910->regmap, reg, mask, mask);
-}
-EXPORT_SYMBOL_GPL(tps65910_set_bits);
-
-int tps65910_clear_bits(struct tps65910 *tps65910, u8 reg, u8 mask)
-{
- return regmap_update_bits(tps65910->regmap, reg, mask, 0);
-}
-EXPORT_SYMBOL_GPL(tps65910_clear_bits);
-
static bool is_volatile_reg(struct device *dev, unsigned int reg)
{
struct tps65910 *tps65910 = dev_get_drvdata(dev);
@@ -102,7 +78,7 @@ static int __init tps65910_sleepinit(struct tps65910 *tps65910,
return 0;

/* enabling SLEEP device state */
- ret = tps65910_set_bits(tps65910, TPS65910_DEVCTRL,
+ ret = tps65910_reg_set_bits(tps65910, TPS65910_DEVCTRL,
DEVCTRL_DEV_SLP_MASK);
if (ret < 0) {
dev_err(dev, "set dev_slp failed: %d\n", ret);
@@ -114,7 +90,8 @@ static int __init tps65910_sleepinit(struct tps65910 *tps65910,
return 0;

if (pmic_pdata->slp_keepon->therm_keepon) {
- ret = tps65910_set_bits(tps65910, TPS65910_SLEEP_KEEP_RES_ON,
+ ret = tps65910_reg_set_bits(tps65910,
+ TPS65910_SLEEP_KEEP_RES_ON,
SLEEP_KEEP_RES_ON_THERM_KEEPON_MASK);
if (ret < 0) {
dev_err(dev, "set therm_keepon failed: %d\n", ret);
@@ -123,7 +100,8 @@ static int __init tps65910_sleepinit(struct tps65910 *tps65910,
}

if (pmic_pdata->slp_keepon->clkout32k_keepon) {
- ret = tps65910_set_bits(tps65910, TPS65910_SLEEP_KEEP_RES_ON,
+ ret = tps65910_reg_set_bits(tps65910,
+ TPS65910_SLEEP_KEEP_RES_ON,
SLEEP_KEEP_RES_ON_CLKOUT32K_KEEPON_MASK);
if (ret < 0) {
dev_err(dev, "set clkout32k_keepon failed: %d\n", ret);
@@ -132,7 +110,8 @@ static int __init tps65910_sleepinit(struct tps65910 *tps65910,
}

if (pmic_pdata->slp_keepon->i2chs_keepon) {
- ret = tps65910_set_bits(tps65910, TPS65910_SLEEP_KEEP_RES_ON,
+ ret = tps65910_reg_set_bits(tps65910,
+ TPS65910_SLEEP_KEEP_RES_ON,
SLEEP_KEEP_RES_ON_I2CHS_KEEPON_MASK);
if (ret < 0) {
dev_err(dev, "set i2chs_keepon failed: %d\n", ret);
@@ -143,7 +122,8 @@ static int __init tps65910_sleepinit(struct tps65910 *tps65910,
return 0;

disable_dev_slp:
- tps65910_clear_bits(tps65910, TPS65910_DEVCTRL, DEVCTRL_DEV_SLP_MASK);
+ tps65910_reg_clear_bits(tps65910, TPS65910_DEVCTRL,
+ DEVCTRL_DEV_SLP_MASK);

err_sleep_init:
return ret;
@@ -176,8 +156,6 @@ static int tps65910_i2c_probe(struct i2c_client *i2c,
tps65910->dev = &i2c->dev;
tps65910->i2c_client = i2c;
tps65910->id = id->driver_data;
- tps65910->read = tps65910_i2c_read;
- tps65910->write = tps65910_i2c_write;
mutex_init(&tps65910->io_mutex);

tps65910->regmap = regmap_init_i2c(i2c, &tps65910_regmap_config);
diff --git a/drivers/regulator/tps65910-regulator.c b/drivers/regulator/tps65910-regulator.c
index 2deeb7c..09fb5cd 100644
--- a/drivers/regulator/tps65910-regulator.c
+++ b/drivers/regulator/tps65910-regulator.c
@@ -330,21 +330,16 @@ struct tps65910_reg {

static inline int tps65910_read(struct tps65910_reg *pmic, u8 reg)
{
- u8 val;
+ unsigned int val;
int err;

- err = pmic->mfd->read(pmic->mfd, reg, 1, &val);
+ err = tps65910_reg_read(pmic->mfd, reg, &val);
if (err)
return err;

return val;
}

-static inline int tps65910_write(struct tps65910_reg *pmic, u8 reg, u8 val)
-{
- return pmic->mfd->write(pmic->mfd, reg, 1, &val);
-}
-
static int tps65910_modify_bits(struct tps65910_reg *pmic, u8 reg,
u8 set_mask, u8 clear_mask)
{
@@ -361,7 +356,7 @@ static int tps65910_modify_bits(struct tps65910_reg *pmic, u8 reg,

data &= ~clear_mask;
data |= set_mask;
- err = tps65910_write(pmic, reg, data);
+ err = tps65910_reg_write(pmic->mfd, reg, data);
if (err)
dev_err(pmic->mfd->dev, "Write for reg 0x%x failed\n", reg);

@@ -370,7 +365,7 @@ out:
return err;
}

-static int tps65910_reg_read(struct tps65910_reg *pmic, u8 reg)
+static int tps65910_reg_read_locked(struct tps65910_reg *pmic, u8 reg)
{
int data;

@@ -384,13 +379,13 @@ static int tps65910_reg_read(struct tps65910_reg *pmic, u8 reg)
return data;
}

-static int tps65910_reg_write(struct tps65910_reg *pmic, u8 reg, u8 val)
+static int tps65910_reg_write_locked(struct tps65910_reg *pmic, u8 reg, u8 val)
{
int err;

mutex_lock(&pmic->mutex);

- err = tps65910_write(pmic, reg, val);
+ err = tps65910_reg_write(pmic->mfd, reg, val);
if (err < 0)
dev_err(pmic->mfd->dev, "Write for reg 0x%x failed\n", reg);

@@ -489,9 +484,9 @@ static int tps65910_set_mode(struct regulator_dev *dev, unsigned int mode)
LDO_ST_MODE_BIT);
case REGULATOR_MODE_IDLE:
value = LDO_ST_ON_BIT | LDO_ST_MODE_BIT;
- return tps65910_set_bits(mfd, reg, value);
+ return tps65910_reg_set_bits(mfd, reg, value);
case REGULATOR_MODE_STANDBY:
- return tps65910_clear_bits(mfd, reg, LDO_ST_ON_BIT);
+ return tps65910_reg_clear_bits(mfd, reg, LDO_ST_ON_BIT);
}

return -EINVAL;
@@ -506,7 +501,7 @@ static unsigned int tps65910_get_mode(struct regulator_dev *dev)
if (reg < 0)
return reg;

- value = tps65910_reg_read(pmic, reg);
+ value = tps65910_reg_read_locked(pmic, reg);
if (value < 0)
return value;

@@ -526,28 +521,28 @@ static int tps65910_get_voltage_dcdc_sel(struct regulator_dev *dev)

switch (id) {
case TPS65910_REG_VDD1:
- opvsel = tps65910_reg_read(pmic, TPS65910_VDD1_OP);
- mult = tps65910_reg_read(pmic, TPS65910_VDD1);
+ opvsel = tps65910_reg_read_locked(pmic, TPS65910_VDD1_OP);
+ mult = tps65910_reg_read_locked(pmic, TPS65910_VDD1);
mult = (mult & VDD1_VGAIN_SEL_MASK) >> VDD1_VGAIN_SEL_SHIFT;
- srvsel = tps65910_reg_read(pmic, TPS65910_VDD1_SR);
+ srvsel = tps65910_reg_read_locked(pmic, TPS65910_VDD1_SR);
sr = opvsel & VDD1_OP_CMD_MASK;
opvsel &= VDD1_OP_SEL_MASK;
srvsel &= VDD1_SR_SEL_MASK;
vselmax = 75;
break;
case TPS65910_REG_VDD2:
- opvsel = tps65910_reg_read(pmic, TPS65910_VDD2_OP);
- mult = tps65910_reg_read(pmic, TPS65910_VDD2);
+ opvsel = tps65910_reg_read_locked(pmic, TPS65910_VDD2_OP);
+ mult = tps65910_reg_read_locked(pmic, TPS65910_VDD2);
mult = (mult & VDD2_VGAIN_SEL_MASK) >> VDD2_VGAIN_SEL_SHIFT;
- srvsel = tps65910_reg_read(pmic, TPS65910_VDD2_SR);
+ srvsel = tps65910_reg_read_locked(pmic, TPS65910_VDD2_SR);
sr = opvsel & VDD2_OP_CMD_MASK;
opvsel &= VDD2_OP_SEL_MASK;
srvsel &= VDD2_SR_SEL_MASK;
vselmax = 75;
break;
case TPS65911_REG_VDDCTRL:
- opvsel = tps65910_reg_read(pmic, TPS65911_VDDCTRL_OP);
- srvsel = tps65910_reg_read(pmic, TPS65911_VDDCTRL_SR);
+ opvsel = tps65910_reg_read_locked(pmic, TPS65911_VDDCTRL_OP);
+ srvsel = tps65910_reg_read_locked(pmic, TPS65911_VDDCTRL_SR);
sr = opvsel & VDDCTRL_OP_CMD_MASK;
opvsel &= VDDCTRL_OP_SEL_MASK;
srvsel &= VDDCTRL_SR_SEL_MASK;
@@ -587,7 +582,7 @@ static int tps65910_get_voltage(struct regulator_dev *dev)
if (reg < 0)
return reg;

- value = tps65910_reg_read(pmic, reg);
+ value = tps65910_reg_read_locked(pmic, reg);
if (value < 0)
return value;

@@ -626,7 +621,7 @@ static int tps65911_get_voltage(struct regulator_dev *dev)

reg = pmic->get_ctrl_reg(id);

- value = tps65910_reg_read(pmic, reg);
+ value = tps65910_reg_read_locked(pmic, reg);

switch (id) {
case TPS65911_REG_LDO1:
@@ -685,7 +680,7 @@ static int tps65910_set_voltage_dcdc_sel(struct regulator_dev *dev,
tps65910_modify_bits(pmic, TPS65910_VDD1,
(dcdc_mult << VDD1_VGAIN_SEL_SHIFT),
VDD1_VGAIN_SEL_MASK);
- tps65910_reg_write(pmic, TPS65910_VDD1_OP, vsel);
+ tps65910_reg_write_locked(pmic, TPS65910_VDD1_OP, vsel);
break;
case TPS65910_REG_VDD2:
dcdc_mult = (selector / VDD1_2_NUM_VOLT_FINE) + 1;
@@ -696,11 +691,11 @@ static int tps65910_set_voltage_dcdc_sel(struct regulator_dev *dev,
tps65910_modify_bits(pmic, TPS65910_VDD2,
(dcdc_mult << VDD2_VGAIN_SEL_SHIFT),
VDD1_VGAIN_SEL_MASK);
- tps65910_reg_write(pmic, TPS65910_VDD2_OP, vsel);
+ tps65910_reg_write_locked(pmic, TPS65910_VDD2_OP, vsel);
break;
case TPS65911_REG_VDDCTRL:
vsel = selector + 3;
- tps65910_reg_write(pmic, TPS65911_VDDCTRL_OP, vsel);
+ tps65910_reg_write_locked(pmic, TPS65911_VDDCTRL_OP, vsel);
}

return 0;
@@ -951,10 +946,10 @@ static int tps65910_set_ext_sleep_config(struct tps65910_reg *pmic,

/* External EN1 control */
if (ext_sleep_config & TPS65910_SLEEP_CONTROL_EXT_INPUT_EN1)
- ret = tps65910_set_bits(mfd,
+ ret = tps65910_reg_set_bits(mfd,
TPS65910_EN1_LDO_ASS + regoffs, bit_pos);
else
- ret = tps65910_clear_bits(mfd,
+ ret = tps65910_reg_clear_bits(mfd,
TPS65910_EN1_LDO_ASS + regoffs, bit_pos);
if (ret < 0) {
dev_err(mfd->dev,
@@ -964,10 +959,10 @@ static int tps65910_set_ext_sleep_config(struct tps65910_reg *pmic,

/* External EN2 control */
if (ext_sleep_config & TPS65910_SLEEP_CONTROL_EXT_INPUT_EN2)
- ret = tps65910_set_bits(mfd,
+ ret = tps65910_reg_set_bits(mfd,
TPS65910_EN2_LDO_ASS + regoffs, bit_pos);
else
- ret = tps65910_clear_bits(mfd,
+ ret = tps65910_reg_clear_bits(mfd,
TPS65910_EN2_LDO_ASS + regoffs, bit_pos);
if (ret < 0) {
dev_err(mfd->dev,
@@ -979,10 +974,10 @@ static int tps65910_set_ext_sleep_config(struct tps65910_reg *pmic,
if ((tps65910_chip_id(mfd) == TPS65910) &&
(id >= TPS65910_REG_VDIG1)) {
if (ext_sleep_config & TPS65910_SLEEP_CONTROL_EXT_INPUT_EN3)
- ret = tps65910_set_bits(mfd,
+ ret = tps65910_reg_set_bits(mfd,
TPS65910_EN3_LDO_ASS + regoffs, bit_pos);
else
- ret = tps65910_clear_bits(mfd,
+ ret = tps65910_reg_clear_bits(mfd,
TPS65910_EN3_LDO_ASS + regoffs, bit_pos);
if (ret < 0) {
dev_err(mfd->dev,
@@ -994,10 +989,10 @@ static int tps65910_set_ext_sleep_config(struct tps65910_reg *pmic,
/* Return if no external control is selected */
if (!(ext_sleep_config & EXT_SLEEP_CONTROL)) {
/* Clear all sleep controls */
- ret = tps65910_clear_bits(mfd,
+ ret = tps65910_reg_clear_bits(mfd,
TPS65910_SLEEP_KEEP_LDO_ON + regoffs, bit_pos);
if (!ret)
- ret = tps65910_clear_bits(mfd,
+ ret = tps65910_reg_clear_bits(mfd,
TPS65910_SLEEP_SET_LDO_OFF + regoffs, bit_pos);
if (ret < 0)
dev_err(mfd->dev,
@@ -1016,32 +1011,33 @@ static int tps65910_set_ext_sleep_config(struct tps65910_reg *pmic,
(tps65910_chip_id(mfd) == TPS65911))) {
int op_reg_add = pmic->get_ctrl_reg(id) + 1;
int sr_reg_add = pmic->get_ctrl_reg(id) + 2;
- int opvsel = tps65910_reg_read(pmic, op_reg_add);
- int srvsel = tps65910_reg_read(pmic, sr_reg_add);
+ int opvsel = tps65910_reg_read_locked(pmic, op_reg_add);
+ int srvsel = tps65910_reg_read_locked(pmic, sr_reg_add);
if (opvsel & VDD1_OP_CMD_MASK) {
u8 reg_val = srvsel & VDD1_OP_SEL_MASK;
- ret = tps65910_reg_write(pmic, op_reg_add, reg_val);
+ ret = tps65910_reg_write_locked(pmic, op_reg_add,
+ reg_val);
if (ret < 0) {
dev_err(mfd->dev,
"Error in configuring op register\n");
return ret;
}
}
- ret = tps65910_reg_write(pmic, sr_reg_add, 0);
+ ret = tps65910_reg_write_locked(pmic, sr_reg_add, 0);
if (ret < 0) {
dev_err(mfd->dev, "Error in settting sr register\n");
return ret;
}
}

- ret = tps65910_clear_bits(mfd,
+ ret = tps65910_reg_clear_bits(mfd,
TPS65910_SLEEP_KEEP_LDO_ON + regoffs, bit_pos);
if (!ret) {
if (ext_sleep_config & TPS65911_SLEEP_CONTROL_EXT_INPUT_SLEEP)
- ret = tps65910_set_bits(mfd,
+ ret = tps65910_reg_set_bits(mfd,
TPS65910_SLEEP_SET_LDO_OFF + regoffs, bit_pos);
else
- ret = tps65910_clear_bits(mfd,
+ ret = tps65910_reg_clear_bits(mfd,
TPS65910_SLEEP_SET_LDO_OFF + regoffs, bit_pos);
}
if (ret < 0)
@@ -1075,7 +1071,7 @@ static __devinit int tps65910_probe(struct platform_device *pdev)
platform_set_drvdata(pdev, pmic);

/* Give control of all register to control port */
- tps65910_set_bits(pmic->mfd, TPS65910_DEVCTRL,
+ tps65910_reg_set_bits(pmic->mfd, TPS65910_DEVCTRL,
DEVCTRL_SR_CTL_I2C_SEL_MASK);

switch(tps65910_chip_id(tps65910)) {
diff --git a/include/linux/mfd/tps65910.h b/include/linux/mfd/tps65910.h
index 56903ad..949f1da 100644
--- a/include/linux/mfd/tps65910.h
+++ b/include/linux/mfd/tps65910.h
@@ -18,6 +18,7 @@
#define __LINUX_MFD_TPS65910_H

#include <linux/gpio.h>
+#include <linux/regmap.h>

/* TPS chip id list */
#define TPS65910 0
@@ -823,8 +824,6 @@ struct tps65910 {
struct regmap *regmap;
struct mutex io_mutex;
unsigned int id;
- int (*read)(struct tps65910 *tps65910, u8 reg, int size, void *dest);
- int (*write)(struct tps65910 *tps65910, u8 reg, int size, void *src);

/* Client devices */
struct tps65910_pmic *pmic;
@@ -847,8 +846,6 @@ struct tps65910_platform_data {
int irq_base;
};

-int tps65910_set_bits(struct tps65910 *tps65910, u8 reg, u8 mask);
-int tps65910_clear_bits(struct tps65910 *tps65910, u8 reg, u8 mask);
void tps65910_gpio_init(struct tps65910 *tps65910, int gpio_base);
int tps65910_irq_init(struct tps65910 *tps65910, int irq,
struct tps65910_platform_data *pdata);
@@ -859,4 +856,28 @@ static inline int tps65910_chip_id(struct tps65910 *tps65910)
return tps65910->id;
}

+static inline int tps65910_reg_read(struct tps65910 *tps65910, u8 reg,
+ unsigned int *val)
+{
+ return regmap_read(tps65910->regmap, reg, val);
+}
+
+static inline int tps65910_reg_write(struct tps65910 *tps65910, u8 reg,
+ unsigned int val)
+{
+ return regmap_write(tps65910->regmap, reg, val);
+}
+
+static inline int tps65910_reg_set_bits(struct tps65910 *tps65910, u8 reg,
+ u8 mask)
+{
+ return regmap_update_bits(tps65910->regmap, reg, mask, mask);
+}
+
+static inline int tps65910_reg_clear_bits(struct tps65910 *tps65910, u8 reg,
+ u8 mask)
+{
+ return regmap_update_bits(tps65910->regmap, reg, mask, 0);
+}
+
#endif /* __LINUX_MFD_TPS65910_H */
--
1.7.0.4

2012-05-08 15:48:28

by Mark Brown

[permalink] [raw]
Subject: Re: [PATCH 5/6 v3] mfd: tps65910-irq: Add devicetree init support

On Tue, May 08, 2012 at 11:42:42AM -0700, Rhyland Klein wrote:

> + if (pdata->irq_base <= 0)
> + pdata->irq_base = irq_alloc_descs(-1, 0, tps65910->irq_num, -1);
> +
> + if (pdata->irq_base <= 0) {
> + dev_err(tps65910->dev, "Failed to allocate irq descs: %d\n",
> + pdata->irq_base);
> + return pdata->irq_base;
> + }

I'd expect the driver to always call irq_alloc_descs() but to use the
value specified in platform data if there is any. This is the normal
way of doing things, anyway.


Attachments:
(No filename) (495.00 B)
signature.asc (836.00 B)
Digital signature
Download all attachments

2012-05-08 15:56:07

by Stephen Warren

[permalink] [raw]
Subject: Re: [PATCH 6/6 v3] ARM: Tegra: Add support for TPS65910 PMIC

On 05/08/2012 12:42 PM, Rhyland Klein wrote:
> Add support for the tps65910 pmic on cardhu.

Lets ignore this one patch for now; it's not clear whether Cardhu should
instantiate the TPS65910 or TPS62360 PMU; apparently different Cardhu
variants have one, the other, or even both. I'm trying to find out which
variant(s) is/are useful to support.

But this has no effect on the other patches in this series.

2012-05-08 15:58:53

by Mark Brown

[permalink] [raw]
Subject: Re: [PATCH 6/6 v3] ARM: Tegra: Add support for TPS65910 PMIC

On Tue, May 08, 2012 at 09:56:02AM -0600, Stephen Warren wrote:
> On 05/08/2012 12:42 PM, Rhyland Klein wrote:
> > Add support for the tps65910 pmic on cardhu.

> Lets ignore this one patch for now; it's not clear whether Cardhu should
> instantiate the TPS65910 or TPS62360 PMU; apparently different Cardhu
> variants have one, the other, or even both. I'm trying to find out which
> variant(s) is/are useful to support.

IIRC it's the same as Whistler was and even more fun that that - you
might have random PMIC boards appearing from random places and can't
rely on what's on the system at all! See the thread I started about
plugin modules the other day :)


Attachments:
(No filename) (662.00 B)
signature.asc (836.00 B)
Digital signature
Download all attachments

2012-05-08 16:11:52

by Rhyland Klein

[permalink] [raw]
Subject: Re: [PATCH 5/6 v3] mfd: tps65910-irq: Add devicetree init support

On Tue, 2012-05-08 at 08:48 -0700, Mark Brown wrote:
> * PGP Signed by an unknown key
>
> On Tue, May 08, 2012 at 11:42:42AM -0700, Rhyland Klein wrote:
>
> > + if (pdata->irq_base <= 0)
> > + pdata->irq_base = irq_alloc_descs(-1, 0, tps65910->irq_num, -1);
> > +
> > + if (pdata->irq_base <= 0) {
> > + dev_err(tps65910->dev, "Failed to allocate irq descs: %d\n",
> > + pdata->irq_base);
> > + return pdata->irq_base;
> > + }
>
> I'd expect the driver to always call irq_alloc_descs() but to use the
> value specified in platform data if there is any. This is the normal
> way of doing things, anyway.
>

Is this more what you would expect? If the dt code initialized the
irq_base to 0 instead of -1 then this should also work.

pdata->irq_base = irq_alloc_descs(-1, pdata->irq_base,
tps65910->irq_num, -1);

if (pdata->irq_base <= 0) {
dev_err(tps65910->dev, "Failed to allocate irq descs: %d\n",
pdata->irq_base);
return pdata->irq_base;
}

> * Unknown Key
> * 0x6E30FDDD

--
nvpublic

2012-05-08 16:15:56

by Stephen Warren

[permalink] [raw]
Subject: Re: [PATCH 6/6 v3] ARM: Tegra: Add support for TPS65910 PMIC

On 05/08/2012 09:58 AM, Mark Brown wrote:
> On Tue, May 08, 2012 at 09:56:02AM -0600, Stephen Warren wrote:
>> On 05/08/2012 12:42 PM, Rhyland Klein wrote:
>>> Add support for the tps65910 pmic on cardhu.
>
>> Lets ignore this one patch for now; it's not clear whether Cardhu
>> should instantiate the TPS65910 or TPS62360 PMU; apparently
>> different Cardhu variants have one, the other, or even both. I'm
>> trying to find out which variant(s) is/are useful to support.
>
> IIRC it's the same as Whistler was and even more fun that that -
> you might have random PMIC boards appearing from random places and
> can't rely on what's on the system at all! See the thread I
> started about plugin modules the other day :)

I think you're thinking of some other board. Cardhu is a tablet
reference platform without pluggable modules, and I believe the PMIC
mess is just due to some rework in later board revisions to solve some
under-power issues in the earlier variants. But I believe there is
some Tegra30 reference board equivalent to Whistler with all kinds of
pluggable modules, although I haven't touched one yet, and don't
recall its name.

2012-05-08 16:29:08

by Mark Brown

[permalink] [raw]
Subject: Re: [PATCH 5/6 v3] mfd: tps65910-irq: Add devicetree init support

On Tue, May 08, 2012 at 12:11:31PM -0700, Rhyland Klein wrote:

> Is this more what you would expect? If the dt code initialized the
> irq_base to 0 instead of -1 then this should also work.

> pdata->irq_base = irq_alloc_descs(-1, pdata->irq_base,
> tps65910->irq_num, -1);
> if (pdata->irq_base <= 0) {
> dev_err(tps65910->dev, "Failed to allocate irq descs: %d\n",
> pdata->irq_base);
> return pdata->irq_base;
> }

More like:

if (pdata->irq_base)
base = pdata->irq_base;
else
base = 0;
pdata->irq_base = irq_alloc_descs(base, 0,


Attachments:
(No filename) (548.00 B)
signature.asc (836.00 B)
Digital signature
Download all attachments

2012-05-08 16:36:43

by Rhyland Klein

[permalink] [raw]
Subject: Re: [PATCH 5/6 v3] mfd: tps65910-irq: Add devicetree init support

On Tue, 2012-05-08 at 09:29 -0700, Mark Brown wrote:
> * PGP Signed by an unknown key
>
> On Tue, May 08, 2012 at 12:11:31PM -0700, Rhyland Klein wrote:
>
> > Is this more what you would expect? If the dt code initialized the
> > irq_base to 0 instead of -1 then this should also work.
>
> > pdata->irq_base = irq_alloc_descs(-1, pdata->irq_base,
> > tps65910->irq_num, -1);
> > if (pdata->irq_base <= 0) {
> > dev_err(tps65910->dev, "Failed to allocate irq descs: %d\n",
> > pdata->irq_base);
> > return pdata->irq_base;
> > }
>
> More like:
>
> if (pdata->irq_base)
> base = pdata->irq_base;
> else
> base = 0;
> pdata->irq_base = irq_alloc_descs(base, 0,

Alright, will do in next round of changes.

>
> * Unknown Key
> * 0x6E30FDDD

--
nvpublic

2012-05-08 16:48:50

by Mark Brown

[permalink] [raw]
Subject: Re: [PATCH 0/6 v3] Update TPS65910 to boot using devicetree

On Tue, May 08, 2012 at 11:42:37AM -0700, Rhyland Klein wrote:

> mfd: tps65910-irq: Add devicetree init support

Actually, it's not really relevant to this patch series but looking at
the code for the interrupt controller here it looks like it could be
done in terms of regmap-irq.


Attachments:
(No filename) (285.00 B)
signature.asc (836.00 B)
Digital signature
Download all attachments

2012-05-11 09:29:05

by Samuel Ortiz

[permalink] [raw]
Subject: Re: [PATCH 0/6 v3] Update TPS65910 to boot using devicetree

Hi Rhyland,

On Tue, May 08, 2012 at 11:42:37AM -0700, Rhyland Klein wrote:
> This patch set updates the tps65910 driver to boot using devicetree.
>
> This patch set now uses the new addition to the of_regulator code,
> of_regulator_match to find all the regulator init data for the device.
>
> Rhyland Klein (6):
> mfd: tps65910: Commonize regmap access through header
> regulator: tps65910: Add device tree bindings
> mfd: tps65910: Add device-tree support
> regulator: tps65910 regulator: add device tree support
> mfd: tps65910-irq: Add devicetree init support
> ARM: Tegra: Add support for TPS65910 PMIC
I applied patches 1,2 and 3 to my for-next branch.
Patch 4 is a regulator one, I assume Mark is going to take it. And patch 5
needs some additional work.

Cheers,
Samuel.

--
Intel Open Source Technology Centre
http://oss.intel.com/

2012-05-11 18:45:37

by Grant Likely

[permalink] [raw]
Subject: Re: [PATCH 5/6 v3] mfd: tps65910-irq: Add devicetree init support

On Tue, 8 May 2012 16:48:21 +0100, Mark Brown <[email protected]> wrote:
> On Tue, May 08, 2012 at 11:42:42AM -0700, Rhyland Klein wrote:
>
> > + if (pdata->irq_base <= 0)
> > + pdata->irq_base = irq_alloc_descs(-1, 0, tps65910->irq_num, -1);
> > +
> > + if (pdata->irq_base <= 0) {
> > + dev_err(tps65910->dev, "Failed to allocate irq descs: %d\n",
> > + pdata->irq_base);
> > + return pdata->irq_base;
> > + }
>
> I'd expect the driver to always call irq_alloc_descs() but to use the
> value specified in platform data if there is any. This is the normal
> way of doing things, anyway.

More importantly, the driver should be converted to use an irq_domain
and remove the custom irq_to_tps65910_irq function.
irq_find_mapping() handles the hwirq --> irq lookup and
irq_data->hwirq will always give you the controller's irq number.

When irq_base is -1, then use irq_domain_add_linear() which will also
manage irq_desc allocations for you. If the driver requires a
specific irq_base then the descs still need to be manually allocated
and use an irq_domain_add_legacy() to set it up. (this is sub-optimal
though; I hope to have a better solution for legacy that also manages
desc allocations just like the other mappings; maybe in the 3.6
timeframe).

g.

--
Grant Likely, B.Sc, P.Eng.
Secret Lab Technologies, Ltd.

2012-05-11 19:22:08

by Rhyland Klein

[permalink] [raw]
Subject: Re: [PATCH 0/6 v3] Update TPS65910 to boot using devicetree

On Fri, 2012-05-11 at 02:38 -0700, Samuel Ortiz wrote:
> Hi Rhyland,
>
> On Tue, May 08, 2012 at 11:42:37AM -0700, Rhyland Klein wrote:
> > This patch set updates the tps65910 driver to boot using devicetree.
> >
> > This patch set now uses the new addition to the of_regulator code,
> > of_regulator_match to find all the regulator init data for the device.
> >
> > Rhyland Klein (6):
> > mfd: tps65910: Commonize regmap access through header
> > regulator: tps65910: Add device tree bindings
> > mfd: tps65910: Add device-tree support
> > regulator: tps65910 regulator: add device tree support
> > mfd: tps65910-irq: Add devicetree init support
> > ARM: Tegra: Add support for TPS65910 PMIC
> I applied patches 1,2 and 3 to my for-next branch.
> Patch 4 is a regulator one, I assume Mark is going to take it. And patch 5
> needs some additional work.
>
> Cheers,
> Samuel.
>
Thank! I will resend the change for patch 5.

-rhyland

--
nvpublic

2012-05-12 10:17:38

by Mark Brown

[permalink] [raw]
Subject: Re: [PATCH 6/6 v3] ARM: Tegra: Add support for TPS65910 PMIC

On Tue, May 08, 2012 at 10:15:51AM -0600, Stephen Warren wrote:

> under-power issues in the earlier variants. But I believe there is
> some Tegra30 reference board equivalent to Whistler with all kinds of
> pluggable modules, although I haven't touched one yet, and don't
> recall its name.

It's called Verbier I believe.


Attachments:
(No filename) (324.00 B)
signature.asc (836.00 B)
Digital signature
Download all attachments

2012-05-12 10:19:11

by Mark Brown

[permalink] [raw]
Subject: Re: [PATCH 4/6 v3] regulator: tps65910 regulator: add device tree support

On Tue, May 08, 2012 at 11:42:41AM -0700, Rhyland Klein wrote:
> Add devicetree based initialization support for TI tps65910 regulators.

Applied, thanks.


Attachments:
(No filename) (155.00 B)
signature.asc (836.00 B)
Digital signature
Download all attachments