diff mbox

[RFC,v3,3/8] regulator: add pbias regulator support

Message ID 1385043627-30439-4-git-send-email-balajitk@ti.com
State Superseded, archived
Headers show

Commit Message

balajitk@ti.com Nov. 21, 2013, 2:20 p.m. UTC 
pbias register controls internal power supply to sd card i/o pads
in most OMAPs (OMAP2-5, DRA7).
Control bits for selecting voltage level and
enabling/disabling are in the same PBIAS register.

Signed-off-by: Balaji T K <balajitk@ti.com>
---
 .../bindings/regulator/pbias-regulator.txt         |   21 ++
 drivers/regulator/Kconfig                          |    7 +
 drivers/regulator/Makefile                         |    1 +
 drivers/regulator/pbias-regulator.c                |  264 ++++++++++++++++++++
 4 files changed, 293 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/regulator/pbias-regulator.txt
 create mode 100644 drivers/regulator/pbias-regulator.c

Comments

Mark Brown Nov. 21, 2013, 2:46 p.m. UTC | #1 
On Thu, Nov 21, 2013 at 07:50:22PM +0530, Balaji T K wrote:

> +static int pbias_regulator_set_voltage(struct regulator_dev *dev,
> +			int min_uV, int max_uV, unsigned *selector)
> +{
> +	struct pbias_regulator_data *data = rdev_get_drvdata(dev);
> +	const struct pbias_bit_map *bmap = data->bmap;
> +	int ret, vmode;
> +
> +	if (min_uV <= 1800000)
> +		vmode = 0;
> +	else if (min_uV > 1800000)
> +		vmode = bmap->vmode;
> +
> +	ret = regmap_update_bits(data->syscon, data->pbias_reg,
> +						bmap->vmode, vmode);
> +	data->voltage = min_uV;

> +static int pbias_regulator_get_voltage(struct regulator_dev *rdev)
> +{
> +	struct pbias_regulator_data *data = rdev_get_drvdata(rdev);
> +
> +	return data->voltage;
> +}

These don't match up with each other - the get and set voltage calls
should reflect what the hardware state is, not what was requested by the
caller.  You should be able to use the regmap helpers I think.

> +static int pbias_regulator_enable(struct regulator_dev *rdev)
> +{
> +	struct pbias_regulator_data *data = rdev_get_drvdata(rdev);
> +	const struct pbias_bit_map *bmap = data->bmap;
> +	int ret;
> +
> +	ret = regmap_update_bits(data->syscon, data->pbias_reg,
> +					bmap->enable_mask, bmap->enable);

regulator_enable_regmap() and similarly for disable() and is_enabled().

> +	supply_name = initdata->constraints.name;
> +
> +	of_property_read_u32(np, "startup-delay-us", &startup_delay);
> +	ret = of_property_read_u32(np, "pbias-reg-offset",
> +				   &drvdata->pbias_reg);
> +	if (ret) {
> +		dev_err(&pdev->dev, "no pbias-reg-offset property set\n");
> +		return ret;
> +	}

This looks like it should be added as a standard property for overridig
the regulator delay if it can't be set based on the compatible string
alone due to board dependencies.  Do something like what's done for
regulator-ramp-delay.

> +err_regulator:
> +	kfree(drvdata->desc.name);
> +	return ret;

devm_kzalloc().

> +static int __init pbias_regulator_init(void)
> +{
> +	return platform_driver_register(&pbias_regulator_driver);
> +}
> +subsys_initcall(pbias_regulator_init);

module_platform_driver().
balajitk@ti.com Dec. 3, 2013, 3:54 p.m. UTC | #2 
On Thursday 21 November 2013 08:16 PM, Mark Brown wrote:
> On Thu, Nov 21, 2013 at 07:50:22PM +0530, Balaji T K wrote:
>
>> +static int pbias_regulator_set_voltage(struct regulator_dev *dev,
>> +			int min_uV, int max_uV, unsigned *selector)
>> +{
>> +	struct pbias_regulator_data *data = rdev_get_drvdata(dev);
>> +	const struct pbias_bit_map *bmap = data->bmap;
>> +	int ret, vmode;
>> +
>> +	if (min_uV <= 1800000)
>> +		vmode = 0;
>> +	else if (min_uV > 1800000)
>> +		vmode = bmap->vmode;
>> +
>> +	ret = regmap_update_bits(data->syscon, data->pbias_reg,
>> +						bmap->vmode, vmode);
>> +	data->voltage = min_uV;
>
>> +static int pbias_regulator_get_voltage(struct regulator_dev *rdev)
>> +{
>> +	struct pbias_regulator_data *data = rdev_get_drvdata(rdev);
>> +
>> +	return data->voltage;
>> +}
>
> These don't match up with each other - the get and set voltage calls
> should reflect what the hardware state is, not what was requested by the
> caller.  You should be able to use the regmap helpers I think.
>
Hi,

get_voltage returns the min_uV, saved in set_voltage since pbias
only need to programmed if the voltage supply in for pbias cell is less than
or greater than 1.8V, so vmode bit will be set for 3V and also for 3.3V too.

>> +static int pbias_regulator_enable(struct regulator_dev *rdev)
>> +{
>> +	struct pbias_regulator_data *data = rdev_get_drvdata(rdev);
>> +	const struct pbias_bit_map *bmap = data->bmap;
>> +	int ret;
>> +
>> +	ret = regmap_update_bits(data->syscon, data->pbias_reg,
>> +					bmap->enable_mask, bmap->enable);
>
> regulator_enable_regmap() and similarly for disable() and is_enabled().
>

I don't think regmap helper can be used here, to enable pbias cells
I need reset a bit field (to bring pbias out of high impedence mode)
and also set a bit (to bring it out of power down mode)

>> +	supply_name = initdata->constraints.name;
>> +
>> +	of_property_read_u32(np, "startup-delay-us", &startup_delay);
>> +	ret = of_property_read_u32(np, "pbias-reg-offset",
>> +				   &drvdata->pbias_reg);
>> +	if (ret) {
>> +		dev_err(&pdev->dev, "no pbias-reg-offset property set\n");
>> +		return ret;
>> +	}
>
> This looks like it should be added as a standard property for overridig
> the regulator delay if it can't be set based on the compatible string
> alone due to board dependencies.  Do something like what's done for
> regulator-ramp-delay.
>
>> +err_regulator:
>> +	kfree(drvdata->desc.name);
>> +	return ret;
>
> devm_kzalloc().
>

Will assign desc name statically

>> +static int __init pbias_regulator_init(void)
>> +{
>> +	return platform_driver_register(&pbias_regulator_driver);
>> +}
>> +subsys_initcall(pbias_regulator_init);
>
> module_platform_driver().

OK

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Mark Brown Dec. 3, 2013, 4:06 p.m. UTC | #3 
On Tue, Dec 03, 2013 at 09:24:15PM +0530, Balaji T K wrote:
> On Thursday 21 November 2013 08:16 PM, Mark Brown wrote:

> >These don't match up with each other - the get and set voltage calls
> >should reflect what the hardware state is, not what was requested by the
> >caller.  You should be able to use the regmap helpers I think.

> get_voltage returns the min_uV, saved in set_voltage since pbias
> only need to programmed if the voltage supply in for pbias cell is less than
> or greater than 1.8V, so vmode bit will be set for 3V and also for 3.3V too.

That's the problem...
diff mbox

Patch

diff --git a/Documentation/devicetree/bindings/regulator/pbias-regulator.txt b/Documentation/devicetree/bindings/regulator/pbias-regulator.txt
new file mode 100644
index 0000000..712fcd8
--- /dev/null
+++ b/Documentation/devicetree/bindings/regulator/pbias-regulator.txt
@@ -0,0 +1,21 @@ 
+PBIAS internal regulator for SD card i/o pads on OMAP SoCs.
+
+Required properties:
+- compatible:
+  - "regulator-pbias-omap2" for OMAP2
+  - "regulator-pbias-omap3" for OMAP3
+  - "regulator-pbias-omap4" for OMAP4
+  - "regulator-pbias-omap5" for OMAP5, DRA7
+- pbias-reg-offset: PBIAS control register offset from syscon base address
+
+Optional properties:
+- Any optional property defined in bindings/regulator/regulator.txt
+
+Example:
+
+		pbias_regulator: pbias_regulator {
+			pbias-reg-offset = <0>;
+			regulator-name = "pbias_regulator";
+			regulator-min-microvolt = <1800000>;
+			regulator-max-microvolt = <3000000>;
+	};
diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig
index ce785f4..3177f45 100644
--- a/drivers/regulator/Kconfig
+++ b/drivers/regulator/Kconfig
@@ -369,6 +369,13 @@  config REGULATOR_PALMAS
 	  on the muxing. This is handled automatically in the driver by
 	  reading the mux info from OTP.
 
+config REGULATOR_PBIAS
+	tristate "PBIAS OMAP regulator driver"
+	depends on ARCH_OMAP && MFD_SYSCON
+	help
+	 This driver provides support for OMAP pbias modelled
+	 regulators.
+
 config REGULATOR_PCAP
 	tristate "Motorola PCAP2 regulator driver"
 	depends on EZX_PCAP
diff --git a/drivers/regulator/Makefile b/drivers/regulator/Makefile
index 01c597e..16000fa 100644
--- a/drivers/regulator/Makefile
+++ b/drivers/regulator/Makefile
@@ -52,6 +52,7 @@  obj-$(CONFIG_REGULATOR_MC13XXX_CORE) +=  mc13xxx-regulator-core.o
 obj-$(CONFIG_REGULATOR_PALMAS) += palmas-regulator.o
 obj-$(CONFIG_REGULATOR_PFUZE100) += pfuze100-regulator.o
 obj-$(CONFIG_REGULATOR_TPS51632) += tps51632-regulator.o
+obj-$(CONFIG_REGULATOR_PBIAS) += pbias-regulator.o
 obj-$(CONFIG_REGULATOR_PCAP) += pcap-regulator.o
 obj-$(CONFIG_REGULATOR_PCF50633) += pcf50633-regulator.o
 obj-$(CONFIG_REGULATOR_RC5T583)  += rc5t583-regulator.o
diff --git a/drivers/regulator/pbias-regulator.c b/drivers/regulator/pbias-regulator.c
new file mode 100644
index 0000000..48fda88
--- /dev/null
+++ b/drivers/regulator/pbias-regulator.c
@@ -0,0 +1,264 @@ 
+/*
+ * pbias-regulator.c
+ *
+ * Copyright (C) 2011 Texas Instruments Incorporated - http://www.ti.com/
+ * Author: Balaji T K <balajitk@ti.com>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation version 2.
+ *
+ * This program is distributed "as is" WITHOUT ANY WARRANTY of any
+ * kind, whether express or implied; without even the implied warranty
+ * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/err.h>
+#include <linux/io.h>
+#include <linux/delay.h>
+#include <linux/mutex.h>
+#include <linux/module.h>
+#include <linux/mfd/syscon.h>
+#include <linux/platform_device.h>
+#include <linux/regulator/driver.h>
+#include <linux/regulator/machine.h>
+#include <linux/regulator/of_regulator.h>
+#include <linux/regmap.h>
+#include <linux/slab.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+
+struct pbias_bit_map {
+	u32 enable;
+	u32 enable_mask;
+	u32 vmode;
+};
+
+struct pbias_regulator_data {
+	struct regulator_desc desc;
+	void __iomem *pbias_addr;
+	unsigned int pbias_reg;
+	struct regulator_dev *dev;
+	struct regmap *syscon;
+
+
+	const struct pbias_bit_map *bmap;
+	int voltage;
+};
+
+static int pbias_regulator_set_voltage(struct regulator_dev *dev,
+			int min_uV, int max_uV, unsigned *selector)
+{
+	struct pbias_regulator_data *data = rdev_get_drvdata(dev);
+	const struct pbias_bit_map *bmap = data->bmap;
+	int ret, vmode;
+
+	if (min_uV <= 1800000)
+		vmode = 0;
+	else if (min_uV > 1800000)
+		vmode = bmap->vmode;
+
+	ret = regmap_update_bits(data->syscon, data->pbias_reg,
+						bmap->vmode, vmode);
+	data->voltage = min_uV;
+
+	return ret;
+}
+
+static int pbias_regulator_get_voltage(struct regulator_dev *rdev)
+{
+	struct pbias_regulator_data *data = rdev_get_drvdata(rdev);
+
+	return data->voltage;
+}
+
+static int pbias_regulator_enable(struct regulator_dev *rdev)
+{
+	struct pbias_regulator_data *data = rdev_get_drvdata(rdev);
+	const struct pbias_bit_map *bmap = data->bmap;
+	int ret;
+
+	ret = regmap_update_bits(data->syscon, data->pbias_reg,
+					bmap->enable_mask, bmap->enable);
+
+	return ret;
+}
+
+static int pbias_regulator_disable(struct regulator_dev *rdev)
+{
+	struct pbias_regulator_data *data = rdev_get_drvdata(rdev);
+	const struct pbias_bit_map *bmap = data->bmap;
+	int ret;
+
+	ret = regmap_update_bits(data->syscon, data->pbias_reg,
+						bmap->enable_mask, 0);
+	return ret;
+}
+
+static int pbias_regulator_is_enable(struct regulator_dev *rdev)
+{
+	struct pbias_regulator_data *data = rdev_get_drvdata(rdev);
+	const struct pbias_bit_map *bmap = data->bmap;
+	int value;
+
+	regmap_read(data->syscon, data->pbias_reg, &value);
+
+	return value & bmap->enable_mask;
+}
+
+static struct regulator_ops pbias_regulator_voltage_ops = {
+	.set_voltage	= pbias_regulator_set_voltage,
+	.get_voltage	= pbias_regulator_get_voltage,
+	.enable		= pbias_regulator_enable,
+	.disable	= pbias_regulator_disable,
+	.is_enabled	= pbias_regulator_is_enable,
+};
+
+#if CONFIG_OF
+static const struct pbias_bit_map pbias_omap3 = {
+	.enable = BIT(1),
+	.enable_mask = BIT(1),
+	.vmode = BIT(0),
+};
+
+static const struct pbias_bit_map pbias_omap4 = {
+	.enable = BIT(26) | BIT(22),
+	.enable_mask = BIT(26) | BIT(25) | BIT(22),
+	.vmode = BIT(21),
+};
+
+static const struct pbias_bit_map pbias_omap5 = {
+	.enable = BIT(27) | BIT(26),
+	.enable_mask = BIT(27) | BIT(25) | BIT(26),
+	.vmode = BIT(21),
+};
+
+static const struct of_device_id pbias_of_match[] = {
+	{ .compatible = "regulator-pbias-omap3", .data = &pbias_omap3},
+	{ .compatible = "regulator-pbias-omap4", .data = &pbias_omap4},
+	{ .compatible = "regulator-pbias-omap5", .data = &pbias_omap5},
+	{},
+};
+MODULE_DEVICE_TABLE(of, pbias_of_match);
+#endif
+
+static int pbias_regulator_probe(struct platform_device *pdev)
+{
+	struct device_node *np = pdev->dev.of_node;
+	struct device_node *syscon_np;
+	struct pbias_regulator_data *drvdata;
+	const struct of_device_id *id;
+	const char *supply_name;
+	struct regulator_init_data *initdata;
+	struct regulator_config cfg = { };
+	unsigned startup_delay;
+	int ret;
+
+	drvdata = devm_kzalloc(&pdev->dev, sizeof(struct pbias_regulator_data),
+			       GFP_KERNEL);
+	if (drvdata == NULL) {
+		dev_err(&pdev->dev, "Failed to allocate device data\n");
+		return -ENOMEM;
+	}
+
+	id = of_match_device(of_match_ptr(pbias_of_match), &pdev->dev);
+	if (!id)
+		return -ENODEV;
+
+	drvdata->bmap = id->data;
+	if (!drvdata->bmap)
+		return -ENODEV;
+
+	initdata = of_get_regulator_init_data(&pdev->dev, np);
+	if (!initdata)
+		return -EINVAL;
+
+	supply_name = initdata->constraints.name;
+
+	of_property_read_u32(np, "startup-delay-us", &startup_delay);
+	ret = of_property_read_u32(np, "pbias-reg-offset",
+				   &drvdata->pbias_reg);
+	if (ret) {
+		dev_err(&pdev->dev, "no pbias-reg-offset property set\n");
+		return ret;
+	}
+
+	syscon_np = of_get_parent(np);
+	if (!syscon_np)
+		return -ENODEV;
+
+	drvdata->syscon = syscon_node_to_regmap(syscon_np);
+	of_node_put(syscon_np);
+	if (IS_ERR(drvdata->syscon))
+		return PTR_ERR(drvdata->syscon);
+
+	drvdata->desc.name = kstrdup(supply_name, GFP_KERNEL);
+	if (drvdata->desc.name == NULL) {
+		dev_err(&pdev->dev, "Failed to allocate supply name\n");
+		return -ENOMEM;
+	}
+
+	drvdata->desc.owner = THIS_MODULE;
+	drvdata->desc.enable_time = startup_delay;
+	drvdata->desc.type = REGULATOR_VOLTAGE;
+	drvdata->desc.ops = &pbias_regulator_voltage_ops;
+	drvdata->desc.n_voltages = 3;
+
+	cfg.dev = &pdev->dev;
+	cfg.init_data = initdata;
+	cfg.driver_data = drvdata;
+	cfg.of_node = np;
+
+	drvdata->dev = devm_regulator_register(&pdev->dev, &drvdata->desc,
+									&cfg);
+	if (IS_ERR(drvdata->dev)) {
+		ret = PTR_ERR(drvdata->dev);
+		dev_err(&pdev->dev, "Failed to register regulator: %d\n", ret);
+		goto err_regulator;
+	}
+
+	platform_set_drvdata(pdev, drvdata);
+
+	return 0;
+
+err_regulator:
+	kfree(drvdata->desc.name);
+	return ret;
+}
+
+static int pbias_regulator_remove(struct platform_device *pdev)
+{
+	struct pbias_regulator_data *drvdata = platform_get_drvdata(pdev);
+
+	kfree(drvdata->desc.name);
+
+	return 0;
+}
+
+static struct platform_driver pbias_regulator_driver = {
+	.probe		= pbias_regulator_probe,
+	.remove		= pbias_regulator_remove,
+	.driver		= {
+		.name		= "pbias-regulator",
+		.owner		= THIS_MODULE,
+		.of_match_table = of_match_ptr(pbias_of_match),
+	},
+};
+
+static int __init pbias_regulator_init(void)
+{
+	return platform_driver_register(&pbias_regulator_driver);
+}
+subsys_initcall(pbias_regulator_init);
+
+static void __exit pbias_regulator_exit(void)
+{
+	platform_driver_unregister(&pbias_regulator_driver);
+}
+module_exit(pbias_regulator_exit);
+
+MODULE_AUTHOR("Balaji T K <balajitk@ti.com>");
+MODULE_DESCRIPTION("pbias voltage regulator");
+MODULE_LICENSE("GPL");
+MODULE_ALIAS("platform:pbias-regulator");