linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [REBASED 0/5] regulator: pwm-regulator: Introduce continuous-mode
@ 2015-06-10  7:57 Lee Jones
  2015-06-10  7:57 ` [REBASED 1/5] ARM: STi: STiH407: Add PWM Regulator node Lee Jones
                   ` (4 more replies)
  0 siblings, 5 replies; 8+ messages in thread
From: Lee Jones @ 2015-06-10  7:57 UTC (permalink / raw)
  To: linux-arm-kernel

Continuous mode uses the PWM regulator's maximum and minimum supplied
voltages specified in the regulator-{min,max}-microvolt properties to
calculate appropriate duty-cycle values.  This allows for a much more
fine grained solution when compared with voltage-table mode, which
this driver already supports.  This solution does make an assumption
that a %50 duty-cycle value will cause the regulator voltage to run
at half way between the supplied max_uV and min_uV values.

Lee Jones (5):
  ARM: STi: STiH407: Add PWM Regulator node
  regulator: pwm-regulator: Separate voltage-table initialisation
  regulator: pwm-regulator: Add support for continuous-voltage
  dt: regulator: pwm-regulator: Re-write bindings
  regulator: pwm-regulator: Don't assign structure attributes right away

 .../bindings/regulator/pwm-regulator.txt           |  66 ++++++--
 arch/arm/boot/dts/stih407-family.dtsi              |  11 ++
 drivers/mfd/mfd-child.c                            |  47 ++++++
 drivers/regulator/pwm-regulator.c                  | 185 +++++++++++++++++----
 4 files changed, 261 insertions(+), 48 deletions(-)
 create mode 100644 drivers/mfd/mfd-child.c

-- 
1.9.1

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

* [REBASED 1/5] ARM: STi: STiH407: Add PWM Regulator node
  2015-06-10  7:57 [REBASED 0/5] regulator: pwm-regulator: Introduce continuous-mode Lee Jones
@ 2015-06-10  7:57 ` Lee Jones
  2015-06-10  7:57 ` [REBASED 2/5] regulator: pwm-regulator: Separate voltage-table initialisation Lee Jones
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 8+ messages in thread
From: Lee Jones @ 2015-06-10  7:57 UTC (permalink / raw)
  To: linux-arm-kernel

Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 arch/arm/boot/dts/stih407-family.dtsi | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/arch/arm/boot/dts/stih407-family.dtsi b/arch/arm/boot/dts/stih407-family.dtsi
index e096105..be201aa 100644
--- a/arch/arm/boot/dts/stih407-family.dtsi
+++ b/arch/arm/boot/dts/stih407-family.dtsi
@@ -65,6 +65,17 @@
 		interrupts = <GIC_PPI 15 IRQ_TYPE_LEVEL_HIGH>;
 	};
 
+	pwm_regulator: pwm-regulator {
+		compatible = "pwm-regulator";
+		pwms = <&pwm1 3 8448>;
+		regulator-name = "CPU_1V0_AVS";
+		regulator-min-microvolt = <784000>;
+		regulator-max-microvolt = <1299000>;
+		regulator-always-on;
+		max-duty-cycle = <255>;
+		status = "okay";
+	};
+
 	soc {
 		#address-cells = <1>;
 		#size-cells = <1>;
-- 
1.9.1

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

* [REBASED 2/5] regulator: pwm-regulator: Separate voltage-table initialisation
  2015-06-10  7:57 [REBASED 0/5] regulator: pwm-regulator: Introduce continuous-mode Lee Jones
  2015-06-10  7:57 ` [REBASED 1/5] ARM: STi: STiH407: Add PWM Regulator node Lee Jones
@ 2015-06-10  7:57 ` Lee Jones
  2015-06-10  9:02   ` Lothar Waßmann
  2015-06-10  7:57 ` [REBASED 3/5] regulator: pwm-regulator: Add support for continuous-voltage Lee Jones
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 8+ messages in thread
From: Lee Jones @ 2015-06-10  7:57 UTC (permalink / raw)
  To: linux-arm-kernel

Take this out of the main .probe() routine in order to facilitate the
introduction of different ways to obtain 'duty cycle' information.

Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 drivers/mfd/mfd-child.c           | 47 ++++++++++++++++++++++++
 drivers/regulator/pwm-regulator.c | 77 +++++++++++++++++++++++----------------
 2 files changed, 92 insertions(+), 32 deletions(-)
 create mode 100644 drivers/mfd/mfd-child.c

diff --git a/drivers/mfd/mfd-child.c b/drivers/mfd/mfd-child.c
new file mode 100644
index 0000000..f233add
--- /dev/null
+++ b/drivers/mfd/mfd-child.c
@@ -0,0 +1,47 @@
+#include <linux/interrupt.h>
+#include <linux/module.h>
+#include <linux/of_device.h>
+#include <linux/of_irq.h>
+
+static int simple_mfd_child_probe(struct platform_device *pdev)
+{
+	struct resource *res;
+	void __iomem *base;
+	int irq;
+
+	printk("LEE: %s %s()[%d]: Enter\n", __FILE__, __func__, __LINE__);
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	dev_err(&pdev->dev, "Phys: %x: %x\n", res->start, resource_size(res));
+
+	base = devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(base))
+		return PTR_ERR(base);
+
+	dev_err(&pdev->dev, "Virt: %p\n", base);
+
+	irq = platform_get_irq(pdev, 0);
+	if (irq < 0) {
+		dev_err(&pdev->dev, "failed to get IRQ: %d\n", irq);
+		return irq;
+	}
+
+	dev_err(&pdev->dev, "IRQ: %d\n", irq);
+
+	return 0;
+}
+
+static const struct of_device_id simple_mfd_child_of_match[] = {
+	{ .compatible = "simple-mfd-child" },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, simple_mfd_child_of_match);
+
+static struct platform_driver simple_mfd_child_driver = {
+	.probe = simple_mfd_child_probe,
+	.driver = {
+		.name = "simple-mfd-child",
+		.of_match_table = of_match_ptr(simple_mfd_child_of_match),
+	},
+};
+module_platform_driver(simple_mfd_child_driver);
diff --git a/drivers/regulator/pwm-regulator.c b/drivers/regulator/pwm-regulator.c
index ffa9612..25560fc 100644
--- a/drivers/regulator/pwm-regulator.c
+++ b/drivers/regulator/pwm-regulator.c
@@ -78,8 +78,7 @@ static int pwm_regulator_list_voltage(struct regulator_dev *rdev,
 
 	return drvdata->duty_cycle_table[selector].uV;
 }
-
-static struct regulator_ops pwm_regulator_voltage_ops = {
+static struct regulator_ops pwm_regulator_voltage_table_ops = {
 	.set_voltage_sel = pwm_regulator_set_voltage_sel,
 	.get_voltage_sel = pwm_regulator_get_voltage_sel,
 	.list_voltage    = pwm_regulator_list_voltage,
@@ -88,20 +87,55 @@ static struct regulator_ops pwm_regulator_voltage_ops = {
 
 static struct regulator_desc pwm_regulator_desc = {
 	.name		= "pwm-regulator",
-	.ops		= &pwm_regulator_voltage_ops,
 	.type		= REGULATOR_VOLTAGE,
 	.owner		= THIS_MODULE,
 	.supply_name    = "pwm",
 };
 
+static int pwm_regulator_init_table(struct platform_device *pdev,
+				    struct pwm_regulator_data *drvdata)
+{
+	struct device_node *np = pdev->dev.of_node;
+	struct pwm_voltages *duty_cycle_table;
+	int length;
+	int ret;
+
+	of_find_property(np, "voltage-table", &length);
+
+	if ((length < sizeof(*duty_cycle_table)) ||
+	    (length % sizeof(*duty_cycle_table))) {
+		dev_err(&pdev->dev,
+			"voltage-table length(%d) is invalid\n",
+			length);
+		return -EINVAL;
+	}
+
+	duty_cycle_table = devm_kzalloc(&pdev->dev, length, GFP_KERNEL);
+	if (!duty_cycle_table)
+		return -ENOMEM;
+
+	ret = of_property_read_u32_array(np, "voltage-table",
+					 (u32 *)duty_cycle_table,
+					 length / sizeof(u32));
+	if (ret) {
+		dev_err(&pdev->dev, "Failed to read voltage-table\n");
+		return ret;
+	}
+
+	drvdata->duty_cycle_table	= duty_cycle_table;
+	pwm_regulator_desc.ops		= &pwm_regulator_voltage_table_ops;
+	pwm_regulator_desc.n_voltages	= length / sizeof(*duty_cycle_table);
+
+	return 0;
+}
+
 static int pwm_regulator_probe(struct platform_device *pdev)
 {
 	struct pwm_regulator_data *drvdata;
-	struct property *prop;
 	struct regulator_dev *regulator;
 	struct regulator_config config = { };
 	struct device_node *np = pdev->dev.of_node;
-	int length, ret;
+	int ret;
 
 	if (!np) {
 		dev_err(&pdev->dev, "Device Tree node missing\n");
@@ -112,36 +146,15 @@ static int pwm_regulator_probe(struct platform_device *pdev)
 	if (!drvdata)
 		return -ENOMEM;
 
-	/* determine the number of voltage-table */
-	prop = of_find_property(np, "voltage-table", &length);
-	if (!prop) {
-		dev_err(&pdev->dev, "No voltage-table\n");
-		return -EINVAL;
-	}
-
-	if ((length < sizeof(*drvdata->duty_cycle_table)) ||
-	    (length % sizeof(*drvdata->duty_cycle_table))) {
-		dev_err(&pdev->dev, "voltage-table length(%d) is invalid\n",
-			length);
+	if (of_find_property(np, "voltage-table", NULL)) {
+		ret = pwm_regulator_init_table(pdev, drvdata);
+		if (ret)
+			return ret;
+	} else {
+		dev_err(&pdev->dev, "No \"voltage-table\" supplied\n");
 		return -EINVAL;
 	}
 
-	pwm_regulator_desc.n_voltages = length / sizeof(*drvdata->duty_cycle_table);
-
-	drvdata->duty_cycle_table = devm_kzalloc(&pdev->dev,
-						 length, GFP_KERNEL);
-	if (!drvdata->duty_cycle_table)
-		return -ENOMEM;
-
-	/* read voltage table from DT property */
-	ret = of_property_read_u32_array(np, "voltage-table",
-					 (u32 *)drvdata->duty_cycle_table,
-					 length / sizeof(u32));
-	if (ret < 0) {
-		dev_err(&pdev->dev, "read voltage-table failed\n");
-		return ret;
-	}
-
 	config.init_data = of_get_regulator_init_data(&pdev->dev, np,
 						      &pwm_regulator_desc);
 	if (!config.init_data)
-- 
1.9.1

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

* [REBASED 3/5] regulator: pwm-regulator: Add support for continuous-voltage
  2015-06-10  7:57 [REBASED 0/5] regulator: pwm-regulator: Introduce continuous-mode Lee Jones
  2015-06-10  7:57 ` [REBASED 1/5] ARM: STi: STiH407: Add PWM Regulator node Lee Jones
  2015-06-10  7:57 ` [REBASED 2/5] regulator: pwm-regulator: Separate voltage-table initialisation Lee Jones
@ 2015-06-10  7:57 ` Lee Jones
  2015-06-10  7:57 ` [REBASED 4/5] dt: regulator: pwm-regulator: Re-write bindings Lee Jones
  2015-06-10  7:57 ` [REBASED 5/5] regulator: pwm-regulator: Don't assign structure attributes right away Lee Jones
  4 siblings, 0 replies; 8+ messages in thread
From: Lee Jones @ 2015-06-10  7:57 UTC (permalink / raw)
  To: linux-arm-kernel

The current version of PWM regulator only supports a static table
approach, where pre-calculated values are supplied by the vendor and
obtained via DT.  The continuous-voltage method takes min_uV and
max_uV, and divides the difference between them up into a number of
slices.  The number of slices depend on how large the duty cycle
register is.  This information is provided by a DT property.

As the name alludes, this provides values for a continuous voltage
range between min_uV and max_uV, which has obvious benefits over
either limited voltage possibilities, or the requirement to provide
a large voltage-table.

Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 drivers/regulator/pwm-regulator.c | 114 +++++++++++++++++++++++++++++++++++---
 1 file changed, 106 insertions(+), 8 deletions(-)

diff --git a/drivers/regulator/pwm-regulator.c b/drivers/regulator/pwm-regulator.c
index 25560fc..b37b616 100644
--- a/drivers/regulator/pwm-regulator.c
+++ b/drivers/regulator/pwm-regulator.c
@@ -10,6 +10,7 @@
  * published by the Free Software Foundation.
  */
 
+#include <linux/delay.h>
 #include <linux/module.h>
 #include <linux/init.h>
 #include <linux/err.h>
@@ -21,9 +22,16 @@
 #include <linux/pwm.h>
 
 struct pwm_regulator_data {
-	struct pwm_voltages *duty_cycle_table;
+	/*  Shared */
 	struct pwm_device *pwm;
+
+	/* Voltage table */
+	struct pwm_voltages *duty_cycle_table;
 	int state;
+
+	/* Continuous voltage */
+	u32 max_duty_cycle;
+	int volt_uV;
 };
 
 struct pwm_voltages {
@@ -31,6 +39,9 @@ struct pwm_voltages {
 	unsigned int dutycycle;
 };
 
+/**
+ * Voltage table call-backs
+ */
 static int pwm_regulator_get_voltage_sel(struct regulator_dev *rdev)
 {
 	struct pwm_regulator_data *drvdata = rdev_get_drvdata(rdev);
@@ -78,6 +89,71 @@ static int pwm_regulator_list_voltage(struct regulator_dev *rdev,
 
 	return drvdata->duty_cycle_table[selector].uV;
 }
+
+/**
+ * Continuous voltage call-backs
+ */
+static int pwm_voltage_to_duty_cycle(struct regulator_dev *rdev,
+					int volt_mV)
+{
+	struct pwm_regulator_data *drvdata = rdev_get_drvdata(rdev);
+	int min_mV = rdev->constraints->min_uV / 1000;
+	int max_mV = rdev->constraints->max_uV / 1000;
+	int max_duty_cycle = drvdata->max_duty_cycle;
+	int vdiff = min_mV - max_mV;
+	int pwm_code;
+	int tmp;
+
+	tmp = max_duty_cycle - min_mV * max_duty_cycle / vdiff;
+	pwm_code = tmp + max_duty_cycle * volt_mV / vdiff;
+
+	if (pwm_code < 0)
+		pwm_code = 0;
+	if (pwm_code > max_duty_cycle)
+		pwm_code = max_duty_cycle;
+
+	return pwm_code * 100 / max_duty_cycle;
+}
+
+static int pwm_regulator_get_voltage(struct regulator_dev *rdev)
+{
+	struct pwm_regulator_data *drvdata = rdev_get_drvdata(rdev);
+
+	return drvdata->volt_uV;
+}
+
+static int pwm_regulator_set_voltage(struct regulator_dev *rdev,
+					int min_uV, int max_uV,
+					unsigned *selector)
+{
+	struct pwm_regulator_data *drvdata = rdev_get_drvdata(rdev);
+	unsigned int ramp_delay = rdev->constraints->ramp_delay;
+	int duty_cycle;
+	int ret;
+
+	duty_cycle = pwm_voltage_to_duty_cycle(rdev, min_uV / 1000);
+
+	ret = pwm_config(drvdata->pwm,
+			 (drvdata->pwm->period / 100) * duty_cycle,
+			 drvdata->pwm->period);
+	if (ret) {
+		dev_err(&rdev->dev, "Failed to configure PWM\n");
+		return ret;
+	}
+
+	ret = pwm_enable(drvdata->pwm);
+	if (ret) {
+		dev_err(&rdev->dev, "Failed to enable PWM\n");
+		return ret;
+	}
+	drvdata->volt_uV = min_uV;
+
+	/* Delay required by PWM regulator to settle to the new voltage */
+	usleep_range(ramp_delay, ramp_delay + 1000);
+
+	return 0;
+}
+
 static struct regulator_ops pwm_regulator_voltage_table_ops = {
 	.set_voltage_sel = pwm_regulator_set_voltage_sel,
 	.get_voltage_sel = pwm_regulator_get_voltage_sel,
@@ -85,6 +161,11 @@ static struct regulator_ops pwm_regulator_voltage_table_ops = {
 	.map_voltage     = regulator_map_voltage_iterate,
 };
 
+static struct regulator_ops pwm_regulator_voltage_continuous_ops = {
+	.get_voltage = pwm_regulator_get_voltage,
+	.set_voltage = pwm_regulator_set_voltage,
+};
+
 static struct regulator_desc pwm_regulator_desc = {
 	.name		= "pwm-regulator",
 	.type		= REGULATOR_VOLTAGE,
@@ -129,6 +210,25 @@ static int pwm_regulator_init_table(struct platform_device *pdev,
 	return 0;
 }
 
+static int pwm_regulator_init_continuous(struct platform_device *pdev,
+					 struct pwm_regulator_data *drvdata)
+{
+	struct device_node *np = pdev->dev.of_node;
+	int ret;
+
+	ret = of_property_read_u32(np, "max-duty-cycle",
+				   &drvdata->max_duty_cycle);
+	if (ret) {
+		dev_err(&pdev->dev, "Failed to read \"pwm-max-value\"\n");
+		return ret;
+	}
+
+	pwm_regulator_desc.ops = &pwm_regulator_voltage_continuous_ops;
+	pwm_regulator_desc.continuous_voltage_range = true;
+
+	return 0;
+}
+
 static int pwm_regulator_probe(struct platform_device *pdev)
 {
 	struct pwm_regulator_data *drvdata;
@@ -146,14 +246,12 @@ static int pwm_regulator_probe(struct platform_device *pdev)
 	if (!drvdata)
 		return -ENOMEM;
 
-	if (of_find_property(np, "voltage-table", NULL)) {
+	if (of_find_property(np, "voltage-table", NULL))
 		ret = pwm_regulator_init_table(pdev, drvdata);
-		if (ret)
-			return ret;
-	} else {
-		dev_err(&pdev->dev, "No \"voltage-table\" supplied\n");
-		return -EINVAL;
-	}
+	else
+		ret = pwm_regulator_init_continuous(pdev, drvdata);
+	if (ret)
+		return ret;
 
 	config.init_data = of_get_regulator_init_data(&pdev->dev, np,
 						      &pwm_regulator_desc);
-- 
1.9.1

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

* [REBASED 4/5] dt: regulator: pwm-regulator: Re-write bindings
  2015-06-10  7:57 [REBASED 0/5] regulator: pwm-regulator: Introduce continuous-mode Lee Jones
                   ` (2 preceding siblings ...)
  2015-06-10  7:57 ` [REBASED 3/5] regulator: pwm-regulator: Add support for continuous-voltage Lee Jones
@ 2015-06-10  7:57 ` Lee Jones
  2015-06-10  7:57 ` [REBASED 5/5] regulator: pwm-regulator: Don't assign structure attributes right away Lee Jones
  4 siblings, 0 replies; 8+ messages in thread
From: Lee Jones @ 2015-06-10  7:57 UTC (permalink / raw)
  To: linux-arm-kernel

* Add support for continuous-voltage mode
* Put more meat on the bones with regards to voltage-table mode
* Sort out formatting for ease of consumption

Cc: devicetree at vger.kernel.org
Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 .../bindings/regulator/pwm-regulator.txt           | 66 ++++++++++++++++++----
 1 file changed, 54 insertions(+), 12 deletions(-)

diff --git a/Documentation/devicetree/bindings/regulator/pwm-regulator.txt b/Documentation/devicetree/bindings/regulator/pwm-regulator.txt
index ce91f61..0ae7895 100644
--- a/Documentation/devicetree/bindings/regulator/pwm-regulator.txt
+++ b/Documentation/devicetree/bindings/regulator/pwm-regulator.txt
@@ -1,27 +1,69 @@
-pwm regulator bindings
+Bindings for the Generic PWM Regulator
+======================================
+
+Currently supports 2 modes of operation:
+
+voltage-table:		When in this mode, a voltage table (See below) of predefined
+			voltage <=> duty-cycle values must be provided via DT.
+			Limitations are that the regulator can only operate at the
+			voltages supplied in the table.  Intermediary duty-cycle
+			values which would normally allow finer grained voltage
+			selection are ignored and rendered useless.  Although more
+			control is given to the user if the assumptions made in
+			continuous-voltage mode do not reign true.
+
+continuous-voltage:	This mode uses the regulator's maximum and minimum supplied
+			voltages specified in the regulator-{min,max}-microvolt
+			properties to calculate appropriate duty-cycle values.  This
+			allows for a much more fine grained solution when compared
+			with voltage-table mode above.  This solution does make an
+			assumption that a %50 duty-cycle value will cause the
+			regulator voltage to run at half way between the supplied
+			max_uV and min_uV values.
 
 Required properties:
-- compatible: Should be "pwm-regulator"
-- pwms: OF device-tree PWM specification (see PWM binding pwm.txt)
-- voltage-table: voltage and duty table, include 2 members in each set of
-  brackets, first one is voltage(unit: uv), the next is duty(unit: percent)
+--------------------
+- compatible:		Should be "pwm-regulator"
+
+- pwms:			PWM specification (See: ../pwm/pwm.txt)
+
+One of these must be provided:
+- voltage-table: 	Voltage and Duty-Cycle table consisting of 2 cells
+			    First cell is voltage in microvolts (uV)
+			    Second cell is duty-cycle in percent (%)
+
+- max-duty-cycle:	Maximum Duty-Cycle value -- this will normally be 255 (0xff)
+			for an 8 bit PWM device
 
-Any property defined as part of the core regulator binding defined in
-regulator.txt can also be used.
+If both are provided, the current default is voltage-table mode.
 
-Example:
+Any property defined as part of the core regulator binding can also be used.
+(See: ../regulator/regulator.txt)
+
+Continuous Voltage Example:
 	pwm_regulator {
 		compatible = "pwm-regulator;
 		pwms = <&pwm1 0 8448 0>;
+		regulator-min-microvolt = <1016000>;
+		regulator-max-microvolt = <1114000>;
+		regulator-name = "vdd_logic";
+
+		max-duty-cycle = <255>; /* 8bit PWM */
+	};
 
+Voltage Table Example:
+	pwm_regulator {
+		compatible = "pwm-regulator;
+		pwms = <&pwm1 0 8448 0>;
+		regulator-min-microvolt = <1016000>;
+		regulator-max-microvolt = <1114000>;
+		regulator-name = "vdd_logic";
+
+			      /* Voltage Duty-Cycle */
 		voltage-table = <1114000 0>,
 				<1095000 10>,
 				<1076000 20>,
 				<1056000 30>,
 				<1036000 40>,
 				<1016000 50>;
-
-		regulator-min-microvolt = <1016000>;
-		regulator-max-microvolt = <1114000>;
-		regulator-name = "vdd_logic";
 	};
-- 
1.9.1

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

* [REBASED 5/5] regulator: pwm-regulator: Don't assign structure attributes right away
  2015-06-10  7:57 [REBASED 0/5] regulator: pwm-regulator: Introduce continuous-mode Lee Jones
                   ` (3 preceding siblings ...)
  2015-06-10  7:57 ` [REBASED 4/5] dt: regulator: pwm-regulator: Re-write bindings Lee Jones
@ 2015-06-10  7:57 ` Lee Jones
  4 siblings, 0 replies; 8+ messages in thread
From: Lee Jones @ 2015-06-10  7:57 UTC (permalink / raw)
  To: linux-arm-kernel

Perhaps this is just personal preference, but ...

This patch introduces a new local variable to receive and test regulator
initialisation data.  It simplifies and cleans up the code making it
that little bit easier to read and maintain.  The local value is assigned
to the structure attribute when all the others are.  This is the way we
usually do things.

Prevents this kind of nonsense:

	this->is->just.silly = fetch_silly_value(&pointer);
	if (!this->is->just.silly) {
		printk("Silly value failed: %d\n", this->is->just.silly);
		return this->is->just.silly;
	}

Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 drivers/regulator/pwm-regulator.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/regulator/pwm-regulator.c b/drivers/regulator/pwm-regulator.c
index b37b616..2bfcb52 100644
--- a/drivers/regulator/pwm-regulator.c
+++ b/drivers/regulator/pwm-regulator.c
@@ -231,6 +231,7 @@ static int pwm_regulator_init_continuous(struct platform_device *pdev,
 
 static int pwm_regulator_probe(struct platform_device *pdev)
 {
+	const struct regulator_init_data *init_data;
 	struct pwm_regulator_data *drvdata;
 	struct regulator_dev *regulator;
 	struct regulator_config config = { };
@@ -253,14 +254,15 @@ static int pwm_regulator_probe(struct platform_device *pdev)
 	if (ret)
 		return ret;
 
-	config.init_data = of_get_regulator_init_data(&pdev->dev, np,
-						      &pwm_regulator_desc);
-	if (!config.init_data)
+	init_data = of_get_regulator_init_data(&pdev->dev, np,
+					       &pwm_regulator_desc);
+	if (!init_data)
 		return -ENOMEM;
 
 	config.of_node = np;
 	config.dev = &pdev->dev;
 	config.driver_data = drvdata;
+	config.init_data = init_data;
 
 	drvdata->pwm = devm_pwm_get(&pdev->dev, NULL);
 	if (IS_ERR(drvdata->pwm)) {
-- 
1.9.1

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

* [REBASED 2/5] regulator: pwm-regulator: Separate voltage-table initialisation
  2015-06-10  7:57 ` [REBASED 2/5] regulator: pwm-regulator: Separate voltage-table initialisation Lee Jones
@ 2015-06-10  9:02   ` Lothar Waßmann
  2015-06-10  9:10     ` Lee Jones
  0 siblings, 1 reply; 8+ messages in thread
From: Lothar Waßmann @ 2015-06-10  9:02 UTC (permalink / raw)
  To: linux-arm-kernel

Hi,

> Take this out of the main .probe() routine in order to facilitate the
> introduction of different ways to obtain 'duty cycle' information.
> 
> Signed-off-by: Lee Jones <lee.jones@linaro.org>
> ---
>  drivers/mfd/mfd-child.c           | 47 ++++++++++++++++++++++++
>  drivers/regulator/pwm-regulator.c | 77 +++++++++++++++++++++++----------------
>  2 files changed, 92 insertions(+), 32 deletions(-)
>  create mode 100644 drivers/mfd/mfd-child.c
> 
> diff --git a/drivers/mfd/mfd-child.c b/drivers/mfd/mfd-child.c
> new file mode 100644
> index 0000000..f233add
> --- /dev/null
> +++ b/drivers/mfd/mfd-child.c
> @@ -0,0 +1,47 @@
> +#include <linux/interrupt.h>
> +#include <linux/module.h>
> +#include <linux/of_device.h>
> +#include <linux/of_irq.h>
> +
> +static int simple_mfd_child_probe(struct platform_device *pdev)
> +{
> +	struct resource *res;
> +	void __iomem *base;
> +	int irq;
> +
> +	printk("LEE: %s %s()[%d]: Enter\n", __FILE__, __func__, __LINE__);
> +
Debugging remnant?

> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	dev_err(&pdev->dev, "Phys: %x: %x\n", res->start, resource_size(res));
> +
That's not an error message and thus shouldn't be printed with
dev_err(). dev_dbg() would be more appropriate here.

> +	base = devm_ioremap_resource(&pdev->dev, res);
> +	if (IS_ERR(base))
> +		return PTR_ERR(base);
> +
> +	dev_err(&pdev->dev, "Virt: %p\n", base);
dto.

> +
> +	irq = platform_get_irq(pdev, 0);
> +	if (irq < 0) {
> +		dev_err(&pdev->dev, "failed to get IRQ: %d\n", irq);
> +		return irq;
> +	}
> +
> +	dev_err(&pdev->dev, "IRQ: %d\n", irq);
> +
dto.
> +	return 0;
> +}
> +
> +static const struct of_device_id simple_mfd_child_of_match[] = {
> +	{ .compatible = "simple-mfd-child" },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(of, simple_mfd_child_of_match);
> +
> +static struct platform_driver simple_mfd_child_driver = {
> +	.probe = simple_mfd_child_probe,
> +	.driver = {
> +		.name = "simple-mfd-child",
> +		.of_match_table = of_match_ptr(simple_mfd_child_of_match),
> +	},
> +};
> +module_platform_driver(simple_mfd_child_driver);
> diff --git a/drivers/regulator/pwm-regulator.c b/drivers/regulator/pwm-regulator.c
> index ffa9612..25560fc 100644
> --- a/drivers/regulator/pwm-regulator.c
> +++ b/drivers/regulator/pwm-regulator.c
> @@ -78,8 +78,7 @@ static int pwm_regulator_list_voltage(struct regulator_dev *rdev,
>  
>  	return drvdata->duty_cycle_table[selector].uV;
>  }
> -
> -static struct regulator_ops pwm_regulator_voltage_ops = {
> +static struct regulator_ops pwm_regulator_voltage_table_ops = {
>  	.set_voltage_sel = pwm_regulator_set_voltage_sel,
>  	.get_voltage_sel = pwm_regulator_get_voltage_sel,
>  	.list_voltage    = pwm_regulator_list_voltage,
> @@ -88,20 +87,55 @@ static struct regulator_ops pwm_regulator_voltage_ops = {
>  
>  static struct regulator_desc pwm_regulator_desc = {
>  	.name		= "pwm-regulator",
> -	.ops		= &pwm_regulator_voltage_ops,
>  	.type		= REGULATOR_VOLTAGE,
>  	.owner		= THIS_MODULE,
>  	.supply_name    = "pwm",
>  };
>  
> +static int pwm_regulator_init_table(struct platform_device *pdev,
> +				    struct pwm_regulator_data *drvdata)
> +{
> +	struct device_node *np = pdev->dev.of_node;
> +	struct pwm_voltages *duty_cycle_table;
> +	int length;
> +	int ret;
> +
> +	of_find_property(np, "voltage-table", &length);
> +
> +	if ((length < sizeof(*duty_cycle_table)) ||
> +	    (length % sizeof(*duty_cycle_table))) {
> +		dev_err(&pdev->dev,
> +			"voltage-table length(%d) is invalid\n",
> +			length);
> +		return -EINVAL;
> +	}
> +
> +	duty_cycle_table = devm_kzalloc(&pdev->dev, length, GFP_KERNEL);
> +	if (!duty_cycle_table)
> +		return -ENOMEM;
> +
> +	ret = of_property_read_u32_array(np, "voltage-table",
> +					 (u32 *)duty_cycle_table,
> +					 length / sizeof(u32));
> +	if (ret) {
> +		dev_err(&pdev->dev, "Failed to read voltage-table\n");
> +		return ret;
> +	}
> +
> +	drvdata->duty_cycle_table	= duty_cycle_table;
> +	pwm_regulator_desc.ops		= &pwm_regulator_voltage_table_ops;
> +	pwm_regulator_desc.n_voltages	= length / sizeof(*duty_cycle_table);
> +
> +	return 0;
> +}
> +
>  static int pwm_regulator_probe(struct platform_device *pdev)
>  {
>  	struct pwm_regulator_data *drvdata;
> -	struct property *prop;
>  	struct regulator_dev *regulator;
>  	struct regulator_config config = { };
>  	struct device_node *np = pdev->dev.of_node;
> -	int length, ret;
> +	int ret;
>  
>  	if (!np) {
>  		dev_err(&pdev->dev, "Device Tree node missing\n");
> @@ -112,36 +146,15 @@ static int pwm_regulator_probe(struct platform_device *pdev)
>  	if (!drvdata)
>  		return -ENOMEM;
>  
> -	/* determine the number of voltage-table */
> -	prop = of_find_property(np, "voltage-table", &length);
> -	if (!prop) {
> -		dev_err(&pdev->dev, "No voltage-table\n");
> -		return -EINVAL;
> -	}
> -
> -	if ((length < sizeof(*drvdata->duty_cycle_table)) ||
> -	    (length % sizeof(*drvdata->duty_cycle_table))) {
> -		dev_err(&pdev->dev, "voltage-table length(%d) is invalid\n",
> -			length);
> +	if (of_find_property(np, "voltage-table", NULL)) {
> +		ret = pwm_regulator_init_table(pdev, drvdata);
> +		if (ret)
> +			return ret;
> +	} else {
> +		dev_err(&pdev->dev, "No \"voltage-table\" supplied\n");
>  		return -EINVAL;
>  	}
>  
> -	pwm_regulator_desc.n_voltages = length / sizeof(*drvdata->duty_cycle_table);
> -
> -	drvdata->duty_cycle_table = devm_kzalloc(&pdev->dev,
> -						 length, GFP_KERNEL);
> -	if (!drvdata->duty_cycle_table)
> -		return -ENOMEM;
> -
> -	/* read voltage table from DT property */
> -	ret = of_property_read_u32_array(np, "voltage-table",
> -					 (u32 *)drvdata->duty_cycle_table,
> -					 length / sizeof(u32));
> -	if (ret < 0) {
> -		dev_err(&pdev->dev, "read voltage-table failed\n");
> -		return ret;
> -	}
> -
>  	config.init_data = of_get_regulator_init_data(&pdev->dev, np,
>  						      &pwm_regulator_desc);
>  	if (!config.init_data)
> -- 
> 1.9.1
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel



-- 
___________________________________________________________

Ka-Ro electronics GmbH | Pascalstra?e 22 | D - 52076 Aachen
Phone: +49 2408 1402-0 | Fax: +49 2408 1402-10
Gesch?ftsf?hrer: Matthias Kaussen
Handelsregistereintrag: Amtsgericht Aachen, HRB 4996

www.karo-electronics.de | info at karo-electronics.de
___________________________________________________________

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

* [REBASED 2/5] regulator: pwm-regulator: Separate voltage-table initialisation
  2015-06-10  9:02   ` Lothar Waßmann
@ 2015-06-10  9:10     ` Lee Jones
  0 siblings, 0 replies; 8+ messages in thread
From: Lee Jones @ 2015-06-10  9:10 UTC (permalink / raw)
  To: linux-arm-kernel

Oh my ...

On Wed, 10 Jun 2015, Lothar Wa?mann wrote:
> > Take this out of the main .probe() routine in order to facilitate the
> > introduction of different ways to obtain 'duty cycle' information.
> > 
> > Signed-off-by: Lee Jones <lee.jones@linaro.org>
> > ---
> >  drivers/mfd/mfd-child.c           | 47 ++++++++++++++++++++++++
> >  drivers/regulator/pwm-regulator.c | 77 +++++++++++++++++++++++----------------
> >  2 files changed, 92 insertions(+), 32 deletions(-)
> >  create mode 100644 drivers/mfd/mfd-child.c
> > 
> > diff --git a/drivers/mfd/mfd-child.c b/drivers/mfd/mfd-child.c
> > new file mode 100644
> > index 0000000..f233add
> > --- /dev/null
> > +++ b/drivers/mfd/mfd-child.c
> > @@ -0,0 +1,47 @@
> > +#include <linux/interrupt.h>
> > +#include <linux/module.h>
> > +#include <linux/of_device.h>
> > +#include <linux/of_irq.h>
> > +
> > +static int simple_mfd_child_probe(struct platform_device *pdev)
> > +{
> > +	struct resource *res;
> > +	void __iomem *base;
> > +	int irq;
> > +
> > +	printk("LEE: %s %s()[%d]: Enter\n", __FILE__, __func__, __LINE__);
> > +
> Debugging remnant?
> 
> > +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > +	dev_err(&pdev->dev, "Phys: %x: %x\n", res->start, resource_size(res));
> > +
> That's not an error message and thus shouldn't be printed with
> dev_err(). dev_dbg() would be more appropriate here.

This entire file is cruft and has nothing to do with this set.

It was knocked it up quickly to test something unrelated.

I will resubmit this patch with it removed.

Sorry for the noise.

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

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

end of thread, other threads:[~2015-06-10  9:10 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-06-10  7:57 [REBASED 0/5] regulator: pwm-regulator: Introduce continuous-mode Lee Jones
2015-06-10  7:57 ` [REBASED 1/5] ARM: STi: STiH407: Add PWM Regulator node Lee Jones
2015-06-10  7:57 ` [REBASED 2/5] regulator: pwm-regulator: Separate voltage-table initialisation Lee Jones
2015-06-10  9:02   ` Lothar Waßmann
2015-06-10  9:10     ` Lee Jones
2015-06-10  7:57 ` [REBASED 3/5] regulator: pwm-regulator: Add support for continuous-voltage Lee Jones
2015-06-10  7:57 ` [REBASED 4/5] dt: regulator: pwm-regulator: Re-write bindings Lee Jones
2015-06-10  7:57 ` [REBASED 5/5] regulator: pwm-regulator: Don't assign structure attributes right away Lee Jones

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).