[PATCH] regulator: Add support samsung power domain

[PATCH] regulator: Add support samsung power domain

[Kukjin Kim]

From: Changhwan Youn <chaos.youn@samsung.com>

This patch adds common regulator driver for samsung power domain.
A consumer of controlling power domain uses regulator framework API,
So new samsung pd driver is inserted into regulator directory.

Signed-off-by: Changhwan Youn <chaos.youn@samsung.com>
Signed-off-by: Jeongbae Seo <jeongbae.seo@samsung.com>
Signed-off-by: Kukjin Kim <kgene.kim@samsung.com>
---
 drivers/regulator/Kconfig            |    5 +
 drivers/regulator/Makefile           |    1 +
 drivers/regulator/samsung_pd.c       |  169 ++++++++++++++++++++++++++++++++++
 include/linux/regulator/samsung_pd.h |   46 +++++++++
 4 files changed, 221 insertions(+), 0 deletions(-)
 create mode 100644 drivers/regulator/samsung_pd.c
 create mode 100644 include/linux/regulator/samsung_pd.h

diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig
index 172951b..1ef8efe 100644
--- a/drivers/regulator/Kconfig
+++ b/drivers/regulator/Kconfig
@@ -235,5 +235,10 @@ config REGULATOR_TPS6586X
 	help
 	  This driver supports TPS6586X voltage regulator chips.
 
+config REGULATOR_SAMSUNG_POWER_DOMAIN
+	tristate "Samsung power domain support"
+	help
+	  This driver provides support for samsung power domain.
+
 endif
 
diff --git a/drivers/regulator/Makefile b/drivers/regulator/Makefile
index 8285fd8..30ae640 100644
--- a/drivers/regulator/Makefile
+++ b/drivers/regulator/Makefile
@@ -36,5 +36,6 @@ obj-$(CONFIG_REGULATOR_TPS6507X) += tps6507x-regulator.o
 obj-$(CONFIG_REGULATOR_88PM8607) += 88pm8607.o
 obj-$(CONFIG_REGULATOR_ISL6271A) += isl6271a-regulator.o
 obj-$(CONFIG_REGULATOR_AB8500)	+= ab8500.o
+obj-$(CONFIG_REGULATOR_SAMSUNG_POWER_DOMAIN) += samsung_pd.o
 
 ccflags-$(CONFIG_REGULATOR_DEBUG) += -DDEBUG
diff --git a/drivers/regulator/samsung_pd.c b/drivers/regulator/samsung_pd.c
new file mode 100644
index 0000000..ef48f16
--- /dev/null
+++ b/drivers/regulator/samsung_pd.c
@@ -0,0 +1,169 @@
+/* linux/driver/regulator/samsung_pd.c
+ *
+ * Copyright (c) 2010 Samsung Electronics Co., Ltd.
+ *		http://www.samsung.com
+ *
+ * Based on linux/driver/regulator/fixed.c
+ *
+ * Samsung power domain support
+ *
+ * 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.
+*/
+
+#include <linux/err.h>
+#include <linux/io.h>
+#include <linux/mutex.h>
+#include <linux/platform_device.h>
+#include <linux/regulator/driver.h>
+#include <linux/regulator/samsung_pd.h>
+#include <linux/delay.h>
+#include <linux/slab.h>
+#include <linux/clk.h>
+
+struct samsung_pd_data {
+	struct regulator_desc desc;
+	struct regulator_dev *dev;
+	unsigned startup_delay;
+	bool is_enabled;
+	int (*enable)(void);
+	int (*disable)(void);
+};
+
+static int samsung_pd_is_enabled(struct regulator_dev *dev)
+{
+	struct samsung_pd_data *data = rdev_get_drvdata(dev);
+
+	return data->is_enabled;
+}
+
+static int samsung_pd_enable(struct regulator_dev *dev)
+{
+	struct samsung_pd_data *data = rdev_get_drvdata(dev);
+	int ret;
+
+	ret = data->enable();
+	if (!ret)
+		data->is_enabled = true;
+
+	return ret;
+}
+
+static int samsung_pd_disable(struct regulator_dev *dev)
+{
+	struct samsung_pd_data *data = rdev_get_drvdata(dev);
+	int ret;
+
+	ret = data->disable();
+	if (!ret)
+		data->is_enabled = false;
+
+	return ret;
+}
+
+static int samsung_pd_enable_time(struct regulator_dev *dev)
+{
+	struct samsung_pd_data *data = rdev_get_drvdata(dev);
+
+	return data->startup_delay;
+}
+
+static struct regulator_ops samsung_pd_ops = {
+	.is_enabled	= samsung_pd_is_enabled,
+	.enable		= samsung_pd_enable,
+	.disable	= samsung_pd_disable,
+	.enable_time	= samsung_pd_enable_time,
+};
+
+static int __devinit reg_samsung_pd_probe(struct platform_device *pdev)
+{
+	struct samsung_pd_config *config = pdev->dev.platform_data;
+	struct samsung_pd_data *drvdata;
+	int ret;
+
+	drvdata = kzalloc(sizeof(struct samsung_pd_data), GFP_KERNEL);
+	if (drvdata == NULL) {
+		dev_err(&pdev->dev, "Failed to allocate device data\n");
+		ret = -ENOMEM;
+		goto err;
+	}
+
+	drvdata->desc.name = kstrdup(config->supply_name, GFP_KERNEL);
+	if (drvdata->desc.name == NULL) {
+		dev_err(&pdev->dev, "Failed to allocate supply name\n");
+		ret = -ENOMEM;
+		goto err;
+	}
+
+	drvdata->desc.type = REGULATOR_VOLTAGE;
+	drvdata->desc.owner = THIS_MODULE;
+	drvdata->desc.ops = &samsung_pd_ops;
+	drvdata->desc.n_voltages = 1;
+
+	drvdata->startup_delay = config->startup_delay;
+	drvdata->is_enabled = config->enabled_at_boot;
+	drvdata->enable = config->enable;
+	drvdata->disable = config->disable;
+
+	if (drvdata->is_enabled)
+		drvdata->enable();
+
+	drvdata->dev = regulator_register(&drvdata->desc, &pdev->dev,
+					  config->init_data, drvdata);
+	if (IS_ERR(drvdata->dev)) {
+		ret = PTR_ERR(drvdata->dev);
+		dev_err(&pdev->dev, "Failed to register regulator: %d\n", ret);
+		goto err_name;
+	}
+
+	platform_set_drvdata(pdev, drvdata);
+
+	dev_dbg(&pdev->dev, "%s registerd\n", drvdata->desc.name);
+
+	return 0;
+
+err_name:
+	kfree(drvdata->desc.name);
+err:
+	kfree(drvdata);
+	return ret;
+}
+
+static int __devexit reg_samsung_pd_remove(struct platform_device *pdev)
+{
+	struct samsung_pd_data *drvdata = platform_get_drvdata(pdev);
+
+	regulator_unregister(drvdata->dev);
+	kfree(drvdata->desc.name);
+	kfree(drvdata);
+
+	return 0;
+}
+
+static struct platform_driver regulator_samsung_pd_driver = {
+	.probe		= reg_samsung_pd_probe,
+	.remove		= __devexit_p(reg_samsung_pd_remove),
+	.driver		= {
+		.name		= "reg-samsung-pd",
+		.owner		= THIS_MODULE,
+	},
+};
+
+static int __init regulator_samsung_pd_init(void)
+{
+	return platform_driver_register(&regulator_samsung_pd_driver);
+}
+subsys_initcall(regulator_samsung_pd_init);
+
+static void __exit regulator_samsung_pd_exit(void)
+{
+	platform_driver_unregister(&regulator_samsung_pd_driver);
+}
+module_exit(regulator_samsung_pd_exit);
+
+MODULE_AUTHOR("Changhwan Youn <chaos@samsung.com>");
+MODULE_AUTHOR("Jeongbae Seo <jeongbae.seo@samsung.com>");
+MODULE_DESCRIPTION("Samsung power domain regulator");
+MODULE_LICENSE("GPL");
+MODULE_ALIAS("platform:reg-samsung-pd");
diff --git a/include/linux/regulator/samsung_pd.h b/include/linux/regulator/samsung_pd.h
new file mode 100644
index 0000000..505b458
--- /dev/null
+++ b/include/linux/regulator/samsung_pd.h
@@ -0,0 +1,46 @@
+/* linux/include/linux/regulator/samsung_pd.h
+ *
+ * Copyright (c) 2010 Samsung Electronics Co., Ltd.
+ *		http://www.samsung.com
+ *
+ * Based on linux/include/linux/regulator/fixed.h
+ *
+ * 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.
+*/
+
+#ifndef __REGULATOR_SAMSUNG_PD_H
+#define __REGULATOR_SAMSUNG_PD_H __FILE__
+
+struct regulator_init_data;
+
+/*
+ * struct samsung_pd_config - samsung_pd_config structure
+ * @supply_name:	Name of the regulator supply
+ * @microvolts:		Output voltage of regulator
+ * @startup_delay:	Start-up time in microseconds
+ * @enabled_at_boot:	Whether regulator has been enabled at
+ *			boot or not. 1 = Yes, 0 = No
+ *			This is used to keep the regulator at
+ *			the default state
+ * @init_data:		regulator_init_data
+ * @enable:		regulator enable function
+ * @disable:		regulator disable function
+ *
+ * This structure contains samsung power domain regulator configuration
+ * information that must be passed by platform code to the samsung
+ * power domain regulator driver.
+ */
+
+struct samsung_pd_config {
+	const char *supply_name;
+	int microvolts;
+	unsigned startup_delay;
+	unsigned enabled_at_boot:1;
+	struct regulator_init_data *init_data;
+	int (*enable)(void);
+	int (*disable)(void);
+};
+
+#endif /* __REGULATOR_SAMSUNG_PD_H */
-- 
1.6.2.5

[PATCH] regulator: Add support samsung power domain

[Mark Brown]

On Fri, Sep 17, 2010 at 06:58:52PM +0900, Kukjin Kim wrote:

> This patch adds common regulator driver for samsung power domain.
> A consumer of controlling power domain uses regulator framework API,
> So new samsung pd driver is inserted into regulator directory.

This looks conceptually OK for the regulator API - there's no operating
points, it's just straight enable/disable stuff - but I do have a few
concerns about this approach.

> +config REGULATOR_SAMSUNG_POWER_DOMAIN
> +	tristate "Samsung power domain support"
> +	help
> +	  This driver provides support for samsung power domain.
> +

This really ought to have a dependency on PLAT_SAMSUNG or something.
However, see below.

> +static struct regulator_ops samsung_pd_ops = {
> +	.is_enabled	= samsung_pd_is_enabled,
> +	.enable		= samsung_pd_enable,
> +	.disable	= samsung_pd_disable,
> +	.enable_time	= samsung_pd_enable_time,
> +};

You've got a microvolts in the platform data but no voltage stuff here.
Equally well given that this is for processor internal stuff probably
the change you need is to remove the voltage from the config.

> +	dev_dbg(&pdev->dev, "%s registerd\n", drvdata->desc.name);

Spelling mistake here.  The regulator API is fairly chatty so I'd have
thought this was a bit redundant?

> +struct samsung_pd_config {
> +	const char *supply_name;
> +	int microvolts;
> +	unsigned startup_delay;
> +	unsigned enabled_at_boot:1;
> +	struct regulator_init_data *init_data;
> +	int (*enable)(void);
> +	int (*disable)(void);
> +};

So, this driver is essentially just a mapping of this ops structure into
the regulator API.  This is not at all Samsung specific so if it did get
included it shouldn't have any Samsung references (other than the author
and copyright ones) but I'm not convinced that this is the best approach.

What I'd have expected to see is either a regulator driver that at least
knows something about updating registers in the CPU (or whatever else is
needed to configure the power domains) or something more generic (along
the lines of the existing fixed voltage regulator, possibly just some
new features for it).  I'm tending more towards the former approach
since if you're having to provide enable and disable operations you're
getting very close to just implementing a subset regulator API.

Another option to consider here is using runtime PM - other platforms
seem to be going down that route, and are using it to also factor clock
management for the IP blocks out (so that the block's clocks get enabled
and disabled automatically when the block is active without needing any
code in the driver).

[PATCH] regulator: Add support samsung power domain

[Marek Szyprowski]

Hello,

On Friday, September 17, 2010 1:31 PM Mark Brown wrote:

> Another option to consider here is using runtime PM - other platforms
> seem to be going down that route, and are using it to also factor clock
> management for the IP blocks out (so that the block's clocks get enabled
> and disabled automatically when the block is active without needing any
> code in the driver).

The approach with merging power domains with clocks have some disadvantages.
In some cases the clock gating and power gating should be distinguished.
Just assume an IP like a video codec. It has it's own internal state machine.
It gets reset when the ip is power gated, but it is preserved during clock
gating. The driver might want to do a clock gating when it is waiting for a
new frame to decode, but should do power gating only when the device has
been closed. Having a set of fake clocks just for power gating imho doesn't
look good.

Best regards
--
Marek Szyprowski
Samsung Poland R&D Center

[PATCH] regulator: Add support samsung power domain

[Mark Brown]

On Fri, Sep 17, 2010 at 01:45:36PM +0200, Marek Szyprowski wrote:

> The approach with merging power domains with clocks have some disadvantages.
> In some cases the clock gating and power gating should be distinguished.

This is not what I suggested.  I suggested using runtime PM instead of
the regulator API to manage blocks.

> Just assume an IP like a video codec. It has it's own internal state machine.
> It gets reset when the ip is power gated, but it is preserved during clock
> gating. The driver might want to do a clock gating when it is waiting for a
> new frame to decode, but should do power gating only when the device has

There's nothing stopping the drivers enabling and disabling the clocks
for themselves while they're active, all the rumtime PM stuff I've seen
for this does is ensure that the clocks are enabled and disabled when
the device is enabled and disabled.  This matches what you're saying
above - clearly, if the device is power gated there's no need to provide
a clock for it - and with aggressive runtime PM it's pretty much exactly
what a lot of devices end up needing.

> been closed. Having a set of fake clocks just for power gating imho doesn't
> look good.

Who said use the clock API?

[PATCH] regulator: Add support samsung power domain

[Kukjin Kim]

Mark Brown wrote:
> 
Hi :-)

> On Fri, Sep 17, 2010 at 06:58:52PM +0900, Kukjin Kim wrote:
> 
> > This patch adds common regulator driver for samsung power domain.
> > A consumer of controlling power domain uses regulator framework API,
> > So new samsung pd driver is inserted into regulator directory.
> 
> This looks conceptually OK for the regulator API - there's no operating
> points, it's just straight enable/disable stuff - but I do have a few
> concerns about this approach.
> 
Hmm..ok ;-)

> > +config REGULATOR_SAMSUNG_POWER_DOMAIN
> > +	tristate "Samsung power domain support"
> > +	help
> > +	  This driver provides support for samsung power domain.
> > +
> 
> This really ought to have a dependency on PLAT_SAMSUNG or something.

Ok..you're right. your suggestion is better.

> However, see below.
> 
Ok..

> > +static struct regulator_ops samsung_pd_ops = {
> > +	.is_enabled	= samsung_pd_is_enabled,
> > +	.enable		= samsung_pd_enable,
> > +	.disable	= samsung_pd_disable,
> > +	.enable_time	= samsung_pd_enable_time,
> > +};
> 
> You've got a microvolts in the platform data but no voltage stuff here.
> Equally well given that this is for processor internal stuff probably
> the change you need is to remove the voltage from the config.
> 
Yeah...will be removed useless stuff.

> > +	dev_dbg(&pdev->dev, "%s registerd\n", drvdata->desc.name);
> 
> Spelling mistake here.  The regulator API is fairly chatty so I'd have
> thought this was a bit redundant?
> 
Oh...thanks.

> > +struct samsung_pd_config {
> > +	const char *supply_name;
> > +	int microvolts;
> > +	unsigned startup_delay;
> > +	unsigned enabled_at_boot:1;
> > +	struct regulator_init_data *init_data;
> > +	int (*enable)(void);
> > +	int (*disable)(void);
> > +};
> 
> So, this driver is essentially just a mapping of this ops structure into
> the regulator API.  This is not at all Samsung specific so if it did get
> included it shouldn't have any Samsung references (other than the author
> and copyright ones) but I'm not convinced that this is the best approach.
> 
> What I'd have expected to see is either a regulator driver that at least
> knows something about updating registers in the CPU (or whatever else is
> needed to configure the power domains) or something more generic (along
> the lines of the existing fixed voltage regulator, possibly just some
> new features for it).  I'm tending more towards the former approach
> since if you're having to provide enable and disable operations you're
> getting very close to just implementing a subset regulator API.
> 
As you expected, we've already implemented a regulator API for S5PV210 and
S5PV310 in the machine-specific code.
According to your comments, it would be better to move those implementations
to the regulator driver code and we agree with you.
The reason the implementation is located in machine-specific code is that
the way to handle power domain in S5PV210 and S5PV310 is different but we
can handle this using platform_device_id.

> Another option to consider here is using runtime PM - other platforms
> seem to be going down that route, and are using it to also factor clock
> management for the IP blocks out (so that the block's clocks get enabled
> and disabled automatically when the block is active without needing any
> code in the driver).

To use runtime PM cannot be a substitute for using regulator API.
It's related to when to control the power domain whether using regulator API
is to how to control the power domain.

We have a plan to support runtime PM using regulator API.
Am I on the right track ?

Thank you for your comments. :-)

Thanks.

Best regards,
Kgene.
--
Kukjin Kim <kgene.kim@samsung.com>, Senior Engineer,
SW Solution Development Team, Samsung Electronics Co., Ltd.

[PATCH] regulator: Add support samsung power domain

[Mark Brown]

On Mon, Sep 20, 2010 at 03:12:34PM +0900, Kukjin Kim wrote:

> The reason the implementation is located in machine-specific code is that
> the way to handle power domain in S5PV210 and S5PV310 is different but we
> can handle this using platform_device_id.

Yes, I think it's reasonable to implement stuff under arch/arm - my only
issue with what you'd done was that I didn't feel the glue layer was
adding anything.

> > Another option to consider here is using runtime PM - other platforms
> > seem to be going down that route, and are using it to also factor clock
> > management for the IP blocks out (so that the block's clocks get enabled
> > and disabled automatically when the block is active without needing any
> > code in the driver).

> To use runtime PM cannot be a substitute for using regulator API.
> It's related to when to control the power domain whether using regulator API
> is to how to control the power domain.

The runtime PM API will give you very similar reference counting to the
regulator API, for all you're using the regulator API for here there's
not a massive difference.  If you started using voltage scaling then
there'd be more win.

> We have a plan to support runtime PM using regulator API.
> Am I on the right track ?

That's doable.