public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH 2/2] qcom: ipq4019: Add LDO regulator driver for SDHC controller
@ 2016-04-04 21:08 Sreedhar Sambangi
  2016-04-05  5:53 ` Andy Gross
  0 siblings, 1 reply; 4+ messages in thread
From: Sreedhar Sambangi @ 2016-04-04 21:08 UTC (permalink / raw)
  To: andy.gross, linux-arm-msm
  Cc: qca-upstream.external, lgirdwood, broonie, linux-kernel

From: Kirthik Srinivasan <kirthik@codeaurora.org>

Add LDO regulator driver to enable SD /MMC  card to
switch between 3.0 volts and 1.8 volts

Change-Id: I66f770878570b1f5b1db044ba626e0f6989acc3f
Signed-off-by: Kirthik Srinivasan <kirthik@codeaurora.org>
Signed-off-by: Rajith Cherian <rajith@codeaurora.org>
Signed-off-by: Sreedhar Sambangi <ssambang@codeaurora.org>
---
 drivers/regulator/Kconfig             |   7 +
 drivers/regulator/Makefile            |   1 +
 drivers/regulator/ipq4019-regulator.c | 275 ++++++++++++++++++++++++++++++++++
 3 files changed, 283 insertions(+)
 create mode 100644 drivers/regulator/ipq4019-regulator.c

diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig
index c77dc08..bb44873 100644
--- a/drivers/regulator/Kconfig
+++ b/drivers/regulator/Kconfig
@@ -843,5 +843,12 @@ config REGULATOR_WM8994
 	  This driver provides support for the voltage regulators on the
 	  WM8994 CODEC.
 
+config REGULATOR_IPQ4019
+	tristate "IPQ4019 regulator support"
+	depends on ARCH_QCOM
+	help
+	  This driver provides support for the voltage regulators of the
+	  IPQ40xx devices.
+
 endif
 
diff --git a/drivers/regulator/Makefile b/drivers/regulator/Makefile
index 61bfbb9..3647e8e 100644
--- a/drivers/regulator/Makefile
+++ b/drivers/regulator/Makefile
@@ -108,6 +108,7 @@ obj-$(CONFIG_REGULATOR_WM831X) += wm831x-ldo.o
 obj-$(CONFIG_REGULATOR_WM8350) += wm8350-regulator.o
 obj-$(CONFIG_REGULATOR_WM8400) += wm8400-regulator.o
 obj-$(CONFIG_REGULATOR_WM8994) += wm8994-regulator.o
+obj-$(CONFIG_REGULATOR_IPQ4019) += ipq4019-regulator.o
 
 
 ccflags-$(CONFIG_REGULATOR_DEBUG) += -DDEBUG
diff --git a/drivers/regulator/ipq4019-regulator.c b/drivers/regulator/ipq4019-regulator.c
new file mode 100644
index 0000000..c4903aa
--- /dev/null
+++ b/drivers/regulator/ipq4019-regulator.c
@@ -0,0 +1,275 @@
+/* Copyright (c) 2015, The Linux Foundation. All rights reserved.
+ *
+ * Permission to use, copy, modify, and/or distribute this software for any
+ * purpose with or without fee is hereby granted, provided that the above
+ * copyright notice and this permission notice appear in all copies.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
+ * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
+ * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
+ * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
+ * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
+ * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
+ * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+ *
+ */
+
+#include <linux/err.h>
+#include <linux/mutex.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/regulator/driver.h>
+#include <linux/regulator/machine.h>
+#include <linux/regulator/of_regulator.h>
+#include <linux/slab.h>
+#include <linux/of.h>
+#include <linux/io.h>
+
+struct ipq4019_regulator_state {
+	int range;
+	int value;
+};
+
+struct ipq4019_regulator_config {
+	const char *supply_name;
+	struct ipq4019_regulator_state *states;
+	int nr_states;
+	int mask;
+	enum regulator_type type;
+	struct regulator_init_data *init_data;
+	void __iomem *base;
+};
+
+struct ipq4019_regulator_data {
+	struct regulator_desc desc;
+	struct regulator_dev *dev;
+	struct ipq4019_regulator_config *config;
+	int range;
+};
+
+static int ipq4019_regulator_get_voltage(struct regulator_dev *dev)
+{
+	struct ipq4019_regulator_data *data = rdev_get_drvdata(dev);
+	struct ipq4019_regulator_config *cfg = data->config;
+	int ptr;
+
+	for (ptr = 0; ptr < cfg->nr_states; ptr++)
+		if (cfg->states[ptr].range == data->range)
+			return cfg->states[ptr].range;
+
+	return -EINVAL;
+}
+
+static int ipq4019_regulator_set_voltage(struct regulator_dev *dev,
+					int min_uV, int max_uV,
+					unsigned *selector)
+{
+	struct ipq4019_regulator_data *data = rdev_get_drvdata(dev);
+	struct ipq4019_regulator_config *cfg = data->config;
+
+	int ptr, best_val = INT_MAX, val;
+
+	for (ptr = 0; ptr < cfg->nr_states; ptr++)
+		if (cfg->states[ptr].range < best_val &&
+		    cfg->states[ptr].range >= min_uV &&
+		    cfg->states[ptr].range <= max_uV) {
+			best_val = cfg->states[ptr].value;
+			if (selector)
+				*selector = ptr;
+		}
+
+	if (best_val == INT_MAX)
+		return -EINVAL;
+
+	val = readl(cfg->base);
+	val  = val & (~(cfg->mask));
+	writel((val | best_val), cfg->base);
+
+	data->range = cfg->states[ptr].range;
+
+	return 0;
+}
+
+static int ipq4019_regulator_list_voltage(struct regulator_dev *dev,
+				      unsigned selector)
+{
+	struct ipq4019_regulator_data *data = rdev_get_drvdata(dev);
+	struct ipq4019_regulator_config *cfg = data->config;
+
+	if (selector >= cfg->nr_states)
+		return -EINVAL;
+
+	return cfg->states[selector].range;
+}
+
+static struct regulator_ops ipq4019_regulator_voltage_ops = {
+	.get_voltage = ipq4019_regulator_get_voltage,
+	.set_voltage = ipq4019_regulator_set_voltage,
+	.list_voltage = ipq4019_regulator_list_voltage,
+};
+
+static struct ipq4019_regulator_config *
+of_get_ipq4019_regulator_data(struct device *dev, struct device_node *np, const struct regulator_desc *desc)
+{
+	struct ipq4019_regulator_config *config;
+	struct property *prop;
+	int proplen, i;
+
+	config = devm_kzalloc(dev,
+			sizeof(struct ipq4019_regulator_config),
+			GFP_KERNEL);
+	if (!config)
+		return ERR_PTR(-ENOMEM);
+
+	config->init_data = of_get_regulator_init_data(dev, np, desc);
+	if (!config->init_data)
+		return ERR_PTR(-EINVAL);
+
+	config->supply_name = config->init_data->constraints.name;
+
+
+	/* Fetch states. */
+	prop = of_find_property(np, "states", NULL);
+	if (!prop) {
+		dev_err(dev, "No 'states' property found\n");
+		return ERR_PTR(-EINVAL);
+	}
+
+	proplen = prop->length / sizeof(int);
+
+	config->states = devm_kzalloc(dev,
+				sizeof(struct ipq4019_regulator_state)
+				* (proplen / 2),
+				GFP_KERNEL);
+	if (!config->states)
+		return ERR_PTR(-ENOMEM);
+
+	for (i = 0; i < proplen / 2; i++) {
+		config->states[i].range =
+			be32_to_cpup((int *)prop->value + (i * 2));
+		config->states[i].value =
+			be32_to_cpup((int *)prop->value + (i * 2 + 1));
+	}
+	config->nr_states = i;
+
+	prop = of_find_property(np, "mask", NULL);
+	if (!prop) {
+		dev_err(dev, "No 'states' property found\n");
+		return ERR_PTR(-EINVAL);
+	}
+
+	config->mask = be32_to_cpup((int *)prop->value);
+	config->type = REGULATOR_VOLTAGE;
+
+	return config;
+}
+
+static int ipq4019_regulator_probe(struct platform_device *pdev)
+{
+	struct ipq4019_regulator_config *config = dev_get_platdata(&pdev->dev);
+	struct device_node *np = pdev->dev.of_node;
+	struct ipq4019_regulator_data *drvdata;
+	struct regulator_config cfg = { };
+	int ret;
+	struct resource *res;
+
+	drvdata = devm_kzalloc(&pdev->dev,
+				sizeof(struct ipq4019_regulator_data),
+				GFP_KERNEL);
+	if (drvdata == NULL) {
+		dev_err(&pdev->dev, "Failed to allocate device data\n");
+		return -ENOMEM;
+	}
+
+	if (np) {
+		config = of_get_ipq4019_regulator_data(&pdev->dev, np, &drvdata->desc);
+		if (IS_ERR(config))
+			return PTR_ERR(config);
+	}
+
+	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->config = config;
+
+	drvdata->desc.type = REGULATOR_VOLTAGE;
+	drvdata->desc.ops = &ipq4019_regulator_voltage_ops;
+	drvdata->desc.n_voltages = config->nr_states;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	drvdata->config->base = devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(drvdata->config->base)) {
+		ret = PTR_ERR(drvdata->config->base);
+		goto err_state;
+	}
+
+	cfg.dev = &pdev->dev;
+	cfg.init_data = config->init_data;
+	cfg.driver_data = drvdata;
+	cfg.of_node = np;
+
+	drvdata->dev = regulator_register(&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_state;
+	}
+
+	platform_set_drvdata(pdev, drvdata);
+
+	return 0;
+
+err_state:
+	kfree(drvdata->config->states);
+	kfree(drvdata->config);
+	kfree(drvdata->desc.name);
+err:
+	return ret;
+}
+
+static int ipq4019_regulator_remove(struct platform_device *pdev)
+{
+	struct ipq4019_regulator_data *drvdata = platform_get_drvdata(pdev);
+
+	regulator_unregister(drvdata->dev);
+
+	kfree(drvdata->config->states);
+	kfree(drvdata->config);
+	kfree(drvdata->desc.name);
+
+	return 0;
+}
+
+#if defined(CONFIG_OF)
+static const struct of_device_id regulator_ipq4019_of_match[] = {
+	{ .compatible = "qcom,regulator-ipq4019", },
+	{},
+};
+#endif
+
+static struct platform_driver ipq4019_regulator_driver = {
+	.probe		= ipq4019_regulator_probe,
+	.remove		= ipq4019_regulator_remove,
+	.driver		= {
+		.name		= "regulator-ipq4019",
+		.of_match_table = of_match_ptr(regulator_ipq4019_of_match),
+	},
+};
+
+static int __init ipq4019_regulator_init(void)
+{
+	return platform_driver_register(&ipq4019_regulator_driver);
+}
+subsys_initcall(ipq4019_regulator_init);
+
+static void __exit ipq4019_regulator_exit(void)
+{
+	platform_driver_unregister(&ipq4019_regulator_driver);
+}
+module_exit(ipq4019_regulator_exit);
+
+MODULE_DESCRIPTION("ipq4019 voltage regulator");
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

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

* Re: [PATCH 2/2] qcom: ipq4019: Add LDO regulator driver for SDHC controller
  2016-04-04 21:08 [PATCH 2/2] qcom: ipq4019: Add LDO regulator driver for SDHC controller Sreedhar Sambangi
@ 2016-04-05  5:53 ` Andy Gross
  2016-04-06  5:02   ` Sreedhar Sambangi
  0 siblings, 1 reply; 4+ messages in thread
From: Andy Gross @ 2016-04-05  5:53 UTC (permalink / raw)
  To: Sreedhar Sambangi
  Cc: linux-arm-msm, qca-upstream.external, lgirdwood, broonie,
	linux-kernel

On Mon, Apr 04, 2016 at 02:08:24PM -0700, Sreedhar Sambangi wrote:
> From: Kirthik Srinivasan <kirthik@codeaurora.org>
> 
> Add LDO regulator driver to enable SD /MMC  card to
> switch between 3.0 volts and 1.8 volts
> 
> Change-Id: I66f770878570b1f5b1db044ba626e0f6989acc3f
> Signed-off-by: Kirthik Srinivasan <kirthik@codeaurora.org>
> Signed-off-by: Rajith Cherian <rajith@codeaurora.org>
> Signed-off-by: Sreedhar Sambangi <ssambang@codeaurora.org>
> ---
>  drivers/regulator/Kconfig             |   7 +
>  drivers/regulator/Makefile            |   1 +
>  drivers/regulator/ipq4019-regulator.c | 275 ++++++++++++++++++++++++++++++++++

It'd be good to have the file name have qcom prepended.  See the other qcom
regulators as an example.

>  3 files changed, 283 insertions(+)
>  create mode 100644 drivers/regulator/ipq4019-regulator.c
> 
> diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig
> index c77dc08..bb44873 100644
> --- a/drivers/regulator/Kconfig
> +++ b/drivers/regulator/Kconfig
> @@ -843,5 +843,12 @@ config REGULATOR_WM8994
>  	  This driver provides support for the voltage regulators on the
>  	  WM8994 CODEC.
>  
> +config REGULATOR_IPQ4019

How bout REGULATOR_QCOM_IPQ4019.
> +	tristate "IPQ4019 regulator support"
> +	depends on ARCH_QCOM
> +	help
> +	  This driver provides support for the voltage regulators of the
> +	  IPQ40xx devices.
> +
>  endif
>  

<snip>

> +static int ipq4019_regulator_set_voltage(struct regulator_dev *dev,
> +					int min_uV, int max_uV,
> +					unsigned *selector)
> +{
> +	struct ipq4019_regulator_data *data = rdev_get_drvdata(dev);
> +	struct ipq4019_regulator_config *cfg = data->config;
> +
> +	int ptr, best_val = INT_MAX, val;
> +
> +	for (ptr = 0; ptr < cfg->nr_states; ptr++)
> +		if (cfg->states[ptr].range < best_val &&
> +		    cfg->states[ptr].range >= min_uV &&
> +		    cfg->states[ptr].range <= max_uV) {
> +			best_val = cfg->states[ptr].value;
> +			if (selector)
> +				*selector = ptr;
> +		}
> +
> +	if (best_val == INT_MAX)
> +		return -EINVAL;
> +
> +	val = readl(cfg->base);
> +	val  = val & (~(cfg->mask));

val &= mask

> +	writel((val | best_val), cfg->base);
> +
> +	data->range = cfg->states[ptr].range;
> +
> +	return 0;
> +}
> +
> +static int ipq4019_regulator_list_voltage(struct regulator_dev *dev,
> +				      unsigned selector)
> +{
> +	struct ipq4019_regulator_data *data = rdev_get_drvdata(dev);
> +	struct ipq4019_regulator_config *cfg = data->config;
> +
> +	if (selector >= cfg->nr_states)
> +		return -EINVAL;
> +
> +	return cfg->states[selector].range;

If you can define your ranges using LINEAR_RANGE, you can just use the
regulator_list_voltage_linear_range.

> +}
> +
> +static struct regulator_ops ipq4019_regulator_voltage_ops = {
> +	.get_voltage = ipq4019_regulator_get_voltage,
> +	.set_voltage = ipq4019_regulator_set_voltage,
> +	.list_voltage = ipq4019_regulator_list_voltage,

No enable and disable?

> +};
> +
> +static struct ipq4019_regulator_config *
> +of_get_ipq4019_regulator_data(struct device *dev, struct device_node *np, const struct regulator_desc *desc)
> +{
> +	struct ipq4019_regulator_config *config;
> +	struct property *prop;
> +	int proplen, i;
> +
> +	config = devm_kzalloc(dev,
> +			sizeof(struct ipq4019_regulator_config),
> +			GFP_KERNEL);
> +	if (!config)
> +		return ERR_PTR(-ENOMEM);
> +
> +	config->init_data = of_get_regulator_init_data(dev, np, desc);
> +	if (!config->init_data)
> +		return ERR_PTR(-EINVAL);
> +
> +	config->supply_name = config->init_data->constraints.name;
> +
> +
> +	/* Fetch states. */
> +	prop = of_find_property(np, "states", NULL);
> +	if (!prop) {
> +		dev_err(dev, "No 'states' property found\n");
> +		return ERR_PTR(-EINVAL);
> +	}
> +
> +	proplen = prop->length / sizeof(int);
> +
> +	config->states = devm_kzalloc(dev,
> +				sizeof(struct ipq4019_regulator_state)
> +				* (proplen / 2),
> +				GFP_KERNEL);
> +	if (!config->states)
> +		return ERR_PTR(-ENOMEM);
> +
> +	for (i = 0; i < proplen / 2; i++) {
> +		config->states[i].range =
> +			be32_to_cpup((int *)prop->value + (i * 2));
> +		config->states[i].value =
> +			be32_to_cpup((int *)prop->value + (i * 2 + 1));
> +	}
> +	config->nr_states = i;

Is it necessary to encode all of this data in the DT?  Is this varied between
boards using the IPQ4019?  Or are these values fixed for this chip?  If they are
fixed, it'd be better to put the data in a static structure or see if the
REGULATOR_LINEAR_RANGE would work for you.

> +
> +	prop = of_find_property(np, "mask", NULL);
> +	if (!prop) {
> +		dev_err(dev, "No 'states' property found\n");
> +		return ERR_PTR(-EINVAL);
> +	}
> +
> +	config->mask = be32_to_cpup((int *)prop->value);
> +	config->type = REGULATOR_VOLTAGE;
> +
> +	return config;
> +}
> +

<snip>

How many of these LDOs are being provided?

Regards,

Andy Gross

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

* Re: [PATCH 2/2] qcom: ipq4019: Add LDO regulator driver for SDHC controller
  2016-04-05  5:53 ` Andy Gross
@ 2016-04-06  5:02   ` Sreedhar Sambangi
  2016-04-12 16:37     ` Matthew McClintock
  0 siblings, 1 reply; 4+ messages in thread
From: Sreedhar Sambangi @ 2016-04-06  5:02 UTC (permalink / raw)
  To: Andy Gross
  Cc: linux-arm-msm, qca-upstream.external, lgirdwood, broonie,
	linux-kernel

On 2016-04-04 22:53, Andy Gross wrote:
> On Mon, Apr 04, 2016 at 02:08:24PM -0700, Sreedhar Sambangi wrote:
>> From: Kirthik Srinivasan <kirthik@codeaurora.org>
>> 
>> Add LDO regulator driver to enable SD /MMC  card to
>> switch between 3.0 volts and 1.8 volts
>> 
>> Change-Id: I66f770878570b1f5b1db044ba626e0f6989acc3f
>> Signed-off-by: Kirthik Srinivasan <kirthik@codeaurora.org>
>> Signed-off-by: Rajith Cherian <rajith@codeaurora.org>
>> Signed-off-by: Sreedhar Sambangi <ssambang@codeaurora.org>
>> ---
>>  drivers/regulator/Kconfig             |   7 +
>>  drivers/regulator/Makefile            |   1 +
>>  drivers/regulator/ipq4019-regulator.c | 275 
>> ++++++++++++++++++++++++++++++++++
> 
> It'd be good to have the file name have qcom prepended.  See the other 
> qcom
> regulators as an example.

Sure, but is it reasonable to say qcom_ipq4019-regulator ? Since as of 
now ,this regulator driver is supporting only ipq4019 SOC.

> 
>>  3 files changed, 283 insertions(+)
>>  create mode 100644 drivers/regulator/ipq4019-regulator.c
>> 
>> diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig
>> index c77dc08..bb44873 100644
>> --- a/drivers/regulator/Kconfig
>> +++ b/drivers/regulator/Kconfig
>> @@ -843,5 +843,12 @@ config REGULATOR_WM8994
>>  	  This driver provides support for the voltage regulators on the
>>  	  WM8994 CODEC.
>> 
>> +config REGULATOR_IPQ4019
> 
> How bout REGULATOR_QCOM_IPQ4019.

Sounds good, Will update in V2

>> +	tristate "IPQ4019 regulator support"
>> +	depends on ARCH_QCOM
>> +	help
>> +	  This driver provides support for the voltage regulators of the
>> +	  IPQ40xx devices.
>> +
>>  endif
>> 
> 
> <snip>
> 
>> +static int ipq4019_regulator_set_voltage(struct regulator_dev *dev,
>> +					int min_uV, int max_uV,
>> +					unsigned *selector)
>> +{
>> +	struct ipq4019_regulator_data *data = rdev_get_drvdata(dev);
>> +	struct ipq4019_regulator_config *cfg = data->config;
>> +
>> +	int ptr, best_val = INT_MAX, val;
>> +
>> +	for (ptr = 0; ptr < cfg->nr_states; ptr++)
>> +		if (cfg->states[ptr].range < best_val &&
>> +		    cfg->states[ptr].range >= min_uV &&
>> +		    cfg->states[ptr].range <= max_uV) {
>> +			best_val = cfg->states[ptr].value;
>> +			if (selector)
>> +				*selector = ptr;
>> +		}
>> +
>> +	if (best_val == INT_MAX)
>> +		return -EINVAL;
>> +
>> +	val = readl(cfg->base);
>> +	val  = val & (~(cfg->mask));
> 
> val &= mask

Thank you, Will fix this

> 
>> +	writel((val | best_val), cfg->base);
>> +
>> +	data->range = cfg->states[ptr].range;
>> +
>> +	return 0;
>> +}
>> +
>> +static int ipq4019_regulator_list_voltage(struct regulator_dev *dev,
>> +				      unsigned selector)
>> +{
>> +	struct ipq4019_regulator_data *data = rdev_get_drvdata(dev);
>> +	struct ipq4019_regulator_config *cfg = data->config;
>> +
>> +	if (selector >= cfg->nr_states)
>> +		return -EINVAL;
>> +
>> +	return cfg->states[selector].range;
> 
> If you can define your ranges using LINEAR_RANGE, you can just use the
> regulator_list_voltage_linear_range.
Since we are supporting only two states , haven't gone through the 
approach of LINEAR_RANGE.but sure will look in to it.

> 
>> +}
>> +
>> +static struct regulator_ops ipq4019_regulator_voltage_ops = {
>> +	.get_voltage = ipq4019_regulator_get_voltage,
>> +	.set_voltage = ipq4019_regulator_set_voltage,
>> +	.list_voltage = ipq4019_regulator_list_voltage,
> 
> No enable and disable?

There is no such states like enable and disable.
> 
>> +};
>> +
>> +static struct ipq4019_regulator_config *
>> +of_get_ipq4019_regulator_data(struct device *dev, struct device_node 
>> *np, const struct regulator_desc *desc)
>> +{
>> +	struct ipq4019_regulator_config *config;
>> +	struct property *prop;
>> +	int proplen, i;
>> +
>> +	config = devm_kzalloc(dev,
>> +			sizeof(struct ipq4019_regulator_config),
>> +			GFP_KERNEL);
>> +	if (!config)
>> +		return ERR_PTR(-ENOMEM);
>> +
>> +	config->init_data = of_get_regulator_init_data(dev, np, desc);
>> +	if (!config->init_data)
>> +		return ERR_PTR(-EINVAL);
>> +
>> +	config->supply_name = config->init_data->constraints.name;
>> +
>> +
>> +	/* Fetch states. */
>> +	prop = of_find_property(np, "states", NULL);
>> +	if (!prop) {
>> +		dev_err(dev, "No 'states' property found\n");
>> +		return ERR_PTR(-EINVAL);
>> +	}
>> +
>> +	proplen = prop->length / sizeof(int);
>> +
>> +	config->states = devm_kzalloc(dev,
>> +				sizeof(struct ipq4019_regulator_state)
>> +				* (proplen / 2),
>> +				GFP_KERNEL);
>> +	if (!config->states)
>> +		return ERR_PTR(-ENOMEM);
>> +
>> +	for (i = 0; i < proplen / 2; i++) {
>> +		config->states[i].range =
>> +			be32_to_cpup((int *)prop->value + (i * 2));
>> +		config->states[i].value =
>> +			be32_to_cpup((int *)prop->value + (i * 2 + 1));
>> +	}
>> +	config->nr_states = i;
> 
> Is it necessary to encode all of this data in the DT?  Is this varied 
> between
> boards using the IPQ4019?  Or are these values fixed for this chip?  If 
> they are
> fixed, it'd be better to put the data in a static structure or see if 
> the
> REGULATOR_LINEAR_RANGE would work for you.


> 
>> +
>> +	prop = of_find_property(np, "mask", NULL);
>> +	if (!prop) {
>> +		dev_err(dev, "No 'states' property found\n");
>> +		return ERR_PTR(-EINVAL);
>> +	}
>> +
>> +	config->mask = be32_to_cpup((int *)prop->value);
>> +	config->type = REGULATOR_VOLTAGE;
>> +
>> +	return config;
>> +}
>> +
> 
> <snip>
> 
> How many of these LDOs are being provided?
Only one sw configurabale LDO
> 
> Regards,
> 
> Andy Gross

-- 
-Sree

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

* Re: [PATCH 2/2] qcom: ipq4019: Add LDO regulator driver for SDHC controller
  2016-04-06  5:02   ` Sreedhar Sambangi
@ 2016-04-12 16:37     ` Matthew McClintock
  0 siblings, 0 replies; 4+ messages in thread
From: Matthew McClintock @ 2016-04-12 16:37 UTC (permalink / raw)
  To: Sreedhar Sambangi
  Cc: Andy Gross, linux-arm-msm, qca-upstream.external, lgirdwood,
	broonie, linux-kernel

On Apr 6, 2016, at 12:02 AM, Sreedhar Sambangi <ssambang@codeaurora.org> wrote:
> 
>>> +config REGULATOR_IPQ4019
>> How bout REGULATOR_QCOM_IPQ4019.
> 
> Sounds good, Will update in V2

Also prefix the name with “Qualcomm” and insert it in the list in order.

-M

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

end of thread, other threads:[~2016-04-12 16:37 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-04-04 21:08 [PATCH 2/2] qcom: ipq4019: Add LDO regulator driver for SDHC controller Sreedhar Sambangi
2016-04-05  5:53 ` Andy Gross
2016-04-06  5:02   ` Sreedhar Sambangi
2016-04-12 16:37     ` Matthew McClintock

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