public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* RE: [PATCH 3/3] regulator: aat2870: Add AAT2870 regulator driver
@ 2011-07-04  8:43 Jin Park
  2011-07-04 17:23 ` Mark Brown
  0 siblings, 1 reply; 4+ messages in thread
From: Jin Park @ 2011-07-04  8:43 UTC (permalink / raw)
  To: Mark Brown
  Cc: Samuel Ortiz, Richard Purdie, Liam Girdwood,
	linux-kernel@vger.kernel.org, Jin Park

Mark,

Thanks for your review.

>> +config REGULATOR_AAT2870
>> +	bool "AnalogicTech AAT2870 Regulators"
> 
> Why bool?
There is no specific reason, so I have changed to tristate.

> This should be a set_voltage_sel(), the core will do all the above for
> you and the driver only needs to worry about the register writes.  Note
> also that the first test isn't well formed - so long as we can select a
> voltage between min_uV and max_uV we're fine even if one or both of
> those limits is outside the valid range.

> Again, get_voltage_sel().
I have changed set_voltage() and get_voltage() to set_voltage_sel() and 
get_voltage_sel(), according to your mention.
Please review below codes.

Thanks,
Jin.



Add regulator driver for AnalogicTech AAT2870.

Signed-off-by: Jin Park <jinyoungp@nvidia.com>
---
 drivers/regulator/Kconfig             |    7 +
 drivers/regulator/Makefile            |    1 +
 drivers/regulator/aat2870-regulator.c |  232 +++++++++++++++++++++++++++++++++
 3 files changed, 240 insertions(+), 0 deletions(-)
 create mode 100644 drivers/regulator/aat2870-regulator.c

diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig
index a1dffd9..065892a 100644
--- a/drivers/regulator/Kconfig
+++ b/drivers/regulator/Kconfig
@@ -309,5 +309,12 @@ config REGULATOR_TPS65910
 	help
 	  This driver supports TPS65910 voltage regulator chips.
 
+config REGULATOR_AAT2870
+	tristate "AnalogicTech AAT2870 Regulators"
+	depends on MFD_AAT2870_CORE
+	help
+	  If you have a AnalogicTech AAT2870 say Y to enable the
+	  regulator driver.
+
 endif
 
diff --git a/drivers/regulator/Makefile b/drivers/regulator/Makefile
index 87ba2fd..040d5aa 100644
--- a/drivers/regulator/Makefile
+++ b/drivers/regulator/Makefile
@@ -44,5 +44,6 @@ obj-$(CONFIG_REGULATOR_ISL6271A) += isl6271a-regulator.o
 obj-$(CONFIG_REGULATOR_AB8500)	+= ab8500.o
 obj-$(CONFIG_REGULATOR_DB8500_PRCMU) += db8500-prcmu.o
 obj-$(CONFIG_REGULATOR_TPS65910) += tps65910-regulator.o
+obj-$(CONFIG_REGULATOR_AAT2870) += aat2870-regulator.o
 
 ccflags-$(CONFIG_REGULATOR_DEBUG) += -DDEBUG
diff --git a/drivers/regulator/aat2870-regulator.c b/drivers/regulator/aat2870-regulator.c
new file mode 100644
index 0000000..cd41045
--- /dev/null
+++ b/drivers/regulator/aat2870-regulator.c
@@ -0,0 +1,232 @@
+/*
+ * linux/drivers/regulator/aat2870-regulator.c
+ *
+ * Copyright (c) 2011, NVIDIA Corporation.
+ * Author: Jin Park <jinyoungp@nvidia.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 in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA
+ * 02110-1301 USA
+ */
+
+#include <linux/kernel.h>
+#include <linux/init.h>
+#include <linux/err.h>
+#include <linux/slab.h>
+#include <linux/delay.h>
+#include <linux/platform_device.h>
+#include <linux/regulator/driver.h>
+#include <linux/regulator/machine.h>
+#include <linux/mfd/aat2870.h>
+
+struct aat2870_regulator {
+	struct platform_device *pdev;
+	struct regulator_desc desc;
+
+	const int *voltages; /* uV */
+
+	int min_uV;
+	int max_uV;
+
+	u8 enable_addr;
+	u8 enable_shift;
+	u8 enable_mask;
+
+	u8 voltage_addr;
+	u8 voltage_shift;
+	u8 voltage_mask;
+};
+
+static int aat2870_ldo_list_voltage(struct regulator_dev *rdev,
+				    unsigned selector)
+{
+	struct aat2870_regulator *ri = rdev_get_drvdata(rdev);
+
+	return ri->voltages[selector];
+}
+
+static int aat2870_ldo_set_voltage_sel(struct regulator_dev *rdev,
+				       unsigned selector)
+{
+	struct aat2870_regulator *ri = rdev_get_drvdata(rdev);
+	struct aat2870_data *aat2870 = dev_get_drvdata(ri->pdev->dev.parent);
+
+	return aat2870->update(aat2870, ri->voltage_addr, ri->voltage_mask,
+			(selector << ri->voltage_shift) & ri->voltage_mask);
+}
+
+static int aat2870_ldo_get_voltage_sel(struct regulator_dev *rdev)
+{
+	struct aat2870_regulator *ri = rdev_get_drvdata(rdev);
+	struct aat2870_data *aat2870 = dev_get_drvdata(ri->pdev->dev.parent);
+	u8 val;
+	int ret;
+
+	ret = aat2870->read(aat2870, ri->voltage_addr, &val);
+	if (ret)
+		return ret;
+
+	return (val & ri->voltage_mask) >> ri->voltage_shift;
+}
+
+static int aat2870_ldo_enable(struct regulator_dev *rdev)
+{
+	struct aat2870_regulator *ri = rdev_get_drvdata(rdev);
+	struct aat2870_data *aat2870 = dev_get_drvdata(ri->pdev->dev.parent);
+
+	return aat2870->update(aat2870, ri->enable_addr, ri->enable_mask,
+			       ri->enable_mask);
+}
+
+static int aat2870_ldo_disable(struct regulator_dev *rdev)
+{
+	struct aat2870_regulator *ri = rdev_get_drvdata(rdev);
+	struct aat2870_data *aat2870 = dev_get_drvdata(ri->pdev->dev.parent);
+
+	return aat2870->update(aat2870, ri->enable_addr, ri->enable_mask, 0);
+}
+
+static int aat2870_ldo_is_enabled(struct regulator_dev *rdev)
+{
+	struct aat2870_regulator *ri = rdev_get_drvdata(rdev);
+	struct aat2870_data *aat2870 = dev_get_drvdata(ri->pdev->dev.parent);
+	u8 val;
+	int ret;
+
+	ret = aat2870->read(aat2870, ri->enable_addr, &val);
+	if (ret)
+		return ret;
+
+	return val & ri->enable_mask ? 1 : 0;
+}
+
+static struct regulator_ops aat2870_ldo_ops = {
+	.list_voltage = aat2870_ldo_list_voltage,
+	.set_voltage_sel = aat2870_ldo_set_voltage_sel,
+	.get_voltage_sel = aat2870_ldo_get_voltage_sel,
+	.enable = aat2870_ldo_enable,
+	.disable = aat2870_ldo_disable,
+	.is_enabled = aat2870_ldo_is_enabled,
+};
+
+static const int aat2870_ldo_voltages[] = {
+	1200000, 1300000, 1500000, 1600000,
+	1800000, 2000000, 2200000, 2500000,
+	2600000, 2700000, 2800000, 2900000,
+	3000000, 3100000, 3200000, 3300000,
+};
+
+#define AAT2870_LDO(ids)				\
+	{						\
+		.desc = {				\
+			.name = #ids,			\
+			.id = AAT2870_ID_##ids,		\
+			.n_voltages = ARRAY_SIZE(aat2870_ldo_voltages),	\
+			.ops = &aat2870_ldo_ops,	\
+			.type = REGULATOR_VOLTAGE,	\
+			.owner = THIS_MODULE,		\
+		},					\
+		.voltages = aat2870_ldo_voltages,	\
+		.min_uV = 1200000,			\
+		.max_uV = 3300000,			\
+	}
+
+static struct aat2870_regulator aat2870_regulators[] = {
+	AAT2870_LDO(LDOA),
+	AAT2870_LDO(LDOB),
+	AAT2870_LDO(LDOC),
+	AAT2870_LDO(LDOD),
+};
+
+static struct aat2870_regulator *aat2870_get_regulator(int id)
+{
+	struct aat2870_regulator *ri = NULL;
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(aat2870_regulators); i++) {
+		ri = &aat2870_regulators[i];
+		if (ri->desc.id == id)
+			break;
+	}
+
+	if (!ri)
+		return NULL;
+
+	ri->enable_addr = AAT2870_LDO_EN;
+	ri->enable_shift = id - AAT2870_ID_LDOA;
+	ri->enable_mask = 0x1 << ri->enable_shift;
+
+	ri->voltage_addr = (id - AAT2870_ID_LDOA) / 2 ?
+			   AAT2870_LDO_CD : AAT2870_LDO_AB;
+	ri->voltage_shift = (id - AAT2870_ID_LDOA) % 2 ? 0 : 4;
+	ri->voltage_mask = 0xF << ri->voltage_shift;
+
+	return ri;
+}
+
+static int aat2870_regulator_probe(struct platform_device *pdev)
+{
+	struct aat2870_regulator *ri;
+	struct regulator_dev *rdev;
+
+	ri = aat2870_get_regulator(pdev->id);
+	if (!ri) {
+		dev_err(&pdev->dev, "Invalid device ID, %d\n", pdev->id);
+		return -EINVAL;
+	}
+	ri->pdev = pdev;
+
+	rdev = regulator_register(&ri->desc, &pdev->dev,
+				  pdev->dev.platform_data, ri);
+	if (IS_ERR(rdev)) {
+		dev_err(&pdev->dev, "Failed to register regulator %s\n",
+			ri->desc.name);
+		return PTR_ERR(rdev);
+	}
+	platform_set_drvdata(pdev, rdev);
+
+	return 0;
+}
+
+static int __devexit aat2870_regulator_remove(struct platform_device *pdev)
+{
+	struct regulator_dev *rdev = platform_get_drvdata(pdev);
+
+	regulator_unregister(rdev);
+	return 0;
+}
+
+static struct platform_driver aat2870_regulator_driver = {
+	.driver = {
+		.name	= "aat2870-regulator",
+		.owner	= THIS_MODULE,
+	},
+	.probe	= aat2870_regulator_probe,
+	.remove	= __devexit_p(aat2870_regulator_remove),
+};
+
+static int __init aat2870_regulator_init(void)
+{
+	return platform_driver_register(&aat2870_regulator_driver);
+}
+subsys_initcall(aat2870_regulator_init);
+
+static void __exit aat2870_regulator_exit(void)
+{
+	platform_driver_unregister(&aat2870_regulator_driver);
+}
+module_exit(aat2870_regulator_exit);
+
+MODULE_DESCRIPTION("AnalogicTech AAT2870 Regulator");
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Jin Park <jinyoungp@nvidia.com>");
-- 
1.7.1


^ permalink raw reply related	[flat|nested] 4+ messages in thread
* Re: [PATCH 0/3] aat2870: Adding mfd, backlight and regulator drivers
@ 2011-06-29 14:06 Jin Park
  2011-06-29 14:06 ` [PATCH 3/3] regulator: aat2870: Add AAT2870 regulator driver Jin Park
  0 siblings, 1 reply; 4+ messages in thread
From: Jin Park @ 2011-06-29 14:06 UTC (permalink / raw)
  To: Samuel Ortiz; +Cc: Richard Purdie, Liam Girdwood, linux-kernel, Jin Park

Changing subject because the patch was separated.
Adding relevant maintainers for backlight and regulator.

Hi Samuel,

> Before reviewing this patch, could you please do the following:
> 
> 1) Split it into 3 actual patches: the MFD one, the regulator one and the
> backlight one.
I have separated the patch to mfd, backlight and regulator.

> 2) Add the relevant maintainers (See MAINTAINERS) if you want to get a proper
> regulator and backlight driver review.
I submitting the patches with relevant maintainers.

> You also need to take the io_lock mutex here, to prevent someone else to write
> a different value to your register between your read and write.
> So what you typically want is an unlocked version of aat2870_[read|write]
> (let's say we call it __aat2870_[read|write]). Then your aat2870_[read|write]
> become wrappers around the __aat2870_[read|write] with the lock taken. And
> your update_bits routine can use __aat2870_[read|write] with the lock taken
> from the beginning.
I have modified mfd driver according to your comment.

Thanks,
Jin.

Jin Park (3):
  mfd: aat2870: Add AAT2870 mfd driver
  backlight: aat2870: Add AAT2870 backlight driver
  regulator: aat2870: Add AAT2870 regulator driver

 drivers/mfd/Kconfig                   |   10 +
 drivers/mfd/Makefile                  |    1 +
 drivers/mfd/aat2870-core.c            |  534 +++++++++++++++++++++++++++++++++
 drivers/regulator/Kconfig             |    7 +
 drivers/regulator/Makefile            |    1 +
 drivers/regulator/aat2870-regulator.c |  264 ++++++++++++++++
 drivers/video/backlight/Kconfig       |    7 +
 drivers/video/backlight/Makefile      |    1 +
 drivers/video/backlight/aat2870_bl.c  |  246 +++++++++++++++
 include/linux/mfd/aat2870.h           |  181 +++++++++++
 10 files changed, 1252 insertions(+), 0 deletions(-)
 create mode 100644 drivers/mfd/aat2870-core.c
 create mode 100644 drivers/regulator/aat2870-regulator.c
 create mode 100644 drivers/video/backlight/aat2870_bl.c
 create mode 100644 include/linux/mfd/aat2870.h


^ permalink raw reply	[flat|nested] 4+ messages in thread
end of thread, other threads:[~2011-07-04 17:23 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-07-04  8:43 [PATCH 3/3] regulator: aat2870: Add AAT2870 regulator driver Jin Park
2011-07-04 17:23 ` Mark Brown
  -- strict thread matches above, loose matches on Subject: below --
2011-06-29 14:06 [PATCH 0/3] aat2870: Adding mfd, backlight and regulator drivers Jin Park
2011-06-29 14:06 ` [PATCH 3/3] regulator: aat2870: Add AAT2870 regulator driver Jin Park
2011-07-01 16:39   ` Mark Brown

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox