linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [RESEND 00/10] regulator: pwm-regulator: Introduce continuous-mode
@ 2015-07-06  8:58 Lee Jones
  2015-07-06  8:58 ` [RESEND 01/10] ARM: multi_v7_defconfig: Enable ST's PWM driver Lee Jones
                   ` (10 more replies)
  0 siblings, 11 replies; 23+ messages in thread
From: Lee Jones @ 2015-07-06  8:58 UTC (permalink / raw)
  To: linux-arm-kernel, linux-kernel; +Cc: kernel, broonie, lgirdwood, Lee Jones

This patch-set has been rebased on to v4.2-rc1.
 
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 (10):
  ARM: multi_v7_defconfig: Enable ST's PWM driver
  ARM: multi_v7_defconfig: Enable ST's Power Reset driver
  ARM: multi_v7_defconfig: Enable support for PWM Regulators
  ARM: STi: STiH407: Move PWM nodes STiH407 => STiH407-family
  ARM: STi: STiH407: Add PWM Regulator node
  dt: regulator: pwm-regulator: Re-write bindings
  regulator: pwm-regulator: Separate voltage-table initialisation
  regulator: pwm-regulator: Add support for continuous-voltage
  regulator: pwm-regulator: Simplify voltage to duty-cycle call
  regulator: pwm-regulator: Don't assign structure attributes right away

 .../bindings/regulator/pwm-regulator.txt           |  66 ++++++--
 arch/arm/boot/dts/stih407-family.dtsi              |  41 +++++
 arch/arm/boot/dts/stih407.dtsi                     |  28 ----
 arch/arm/configs/multi_v7_defconfig                |   3 +
 drivers/regulator/pwm-regulator.c                  | 172 ++++++++++++++++-----
 5 files changed, 234 insertions(+), 76 deletions(-)

-- 
1.9.1


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

* [RESEND 01/10] ARM: multi_v7_defconfig: Enable ST's PWM driver
  2015-07-06  8:58 [RESEND 00/10] regulator: pwm-regulator: Introduce continuous-mode Lee Jones
@ 2015-07-06  8:58 ` Lee Jones
  2015-07-06  8:58 ` [RESEND 02/10] ARM: multi_v7_defconfig: Enable ST's Power Reset driver Lee Jones
                   ` (9 subsequent siblings)
  10 siblings, 0 replies; 23+ messages in thread
From: Lee Jones @ 2015-07-06  8:58 UTC (permalink / raw)
  To: linux-arm-kernel, linux-kernel; +Cc: kernel, broonie, lgirdwood, Lee Jones

Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 arch/arm/configs/multi_v7_defconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/configs/multi_v7_defconfig b/arch/arm/configs/multi_v7_defconfig
index 6d83a1b..21c9e12 100644
--- a/arch/arm/configs/multi_v7_defconfig
+++ b/arch/arm/configs/multi_v7_defconfig
@@ -626,6 +626,7 @@ CONFIG_PWM_SAMSUNG=m
 CONFIG_PWM_TEGRA=y
 CONFIG_PWM_VT8500=y
 CONFIG_PHY_HIX5HD2_SATA=y
+CONFIG_PWM_STI=m
 CONFIG_OMAP_USB2=y
 CONFIG_TI_PIPE3=y
 CONFIG_PHY_MIPHY28LP=y
-- 
1.9.1


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

* [RESEND 02/10] ARM: multi_v7_defconfig: Enable ST's Power Reset driver
  2015-07-06  8:58 [RESEND 00/10] regulator: pwm-regulator: Introduce continuous-mode Lee Jones
  2015-07-06  8:58 ` [RESEND 01/10] ARM: multi_v7_defconfig: Enable ST's PWM driver Lee Jones
@ 2015-07-06  8:58 ` Lee Jones
  2015-07-06  8:58 ` [RESEND 03/10] ARM: multi_v7_defconfig: Enable support for PWM Regulators Lee Jones
                   ` (8 subsequent siblings)
  10 siblings, 0 replies; 23+ messages in thread
From: Lee Jones @ 2015-07-06  8:58 UTC (permalink / raw)
  To: linux-arm-kernel, linux-kernel; +Cc: kernel, broonie, lgirdwood, Lee Jones

Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 arch/arm/configs/multi_v7_defconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/configs/multi_v7_defconfig b/arch/arm/configs/multi_v7_defconfig
index 21c9e12..4144fa4 100644
--- a/arch/arm/configs/multi_v7_defconfig
+++ b/arch/arm/configs/multi_v7_defconfig
@@ -355,6 +355,7 @@ CONFIG_POWER_RESET_GPIO_RESTART=y
 CONFIG_POWER_RESET_KEYSTONE=y
 CONFIG_POWER_RESET_SUN6I=y
 CONFIG_POWER_RESET_RMOBILE=y
+CONFIG_POWER_RESET_ST=m
 CONFIG_SENSORS_LM90=y
 CONFIG_SENSORS_LM95245=y
 CONFIG_THERMAL=y
-- 
1.9.1


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

* [RESEND 03/10] ARM: multi_v7_defconfig: Enable support for PWM Regulators
  2015-07-06  8:58 [RESEND 00/10] regulator: pwm-regulator: Introduce continuous-mode Lee Jones
  2015-07-06  8:58 ` [RESEND 01/10] ARM: multi_v7_defconfig: Enable ST's PWM driver Lee Jones
  2015-07-06  8:58 ` [RESEND 02/10] ARM: multi_v7_defconfig: Enable ST's Power Reset driver Lee Jones
@ 2015-07-06  8:58 ` Lee Jones
  2015-07-06  8:58 ` [RESEND 04/10] ARM: STi: STiH407: Move PWM nodes STiH407 => STiH407-family Lee Jones
                   ` (7 subsequent siblings)
  10 siblings, 0 replies; 23+ messages in thread
From: Lee Jones @ 2015-07-06  8:58 UTC (permalink / raw)
  To: linux-arm-kernel, linux-kernel; +Cc: kernel, broonie, lgirdwood, Lee Jones

Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 arch/arm/configs/multi_v7_defconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/configs/multi_v7_defconfig b/arch/arm/configs/multi_v7_defconfig
index 4144fa4..4629bed 100644
--- a/arch/arm/configs/multi_v7_defconfig
+++ b/arch/arm/configs/multi_v7_defconfig
@@ -405,6 +405,7 @@ CONFIG_REGULATOR_MAX8973=y
 CONFIG_REGULATOR_MAX77686=y
 CONFIG_REGULATOR_MAX77693=m
 CONFIG_REGULATOR_PALMAS=y
+CONFIG_REGULATOR_PWM=m
 CONFIG_REGULATOR_S2MPS11=y
 CONFIG_REGULATOR_S5M8767=y
 CONFIG_REGULATOR_TPS51632=y
-- 
1.9.1


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

* [RESEND 04/10] ARM: STi: STiH407: Move PWM nodes STiH407 => STiH407-family
  2015-07-06  8:58 [RESEND 00/10] regulator: pwm-regulator: Introduce continuous-mode Lee Jones
                   ` (2 preceding siblings ...)
  2015-07-06  8:58 ` [RESEND 03/10] ARM: multi_v7_defconfig: Enable support for PWM Regulators Lee Jones
@ 2015-07-06  8:58 ` Lee Jones
  2015-07-06  8:58 ` [RESEND 05/10] ARM: STi: STiH407: Add PWM Regulator node Lee Jones
                   ` (6 subsequent siblings)
  10 siblings, 0 replies; 23+ messages in thread
From: Lee Jones @ 2015-07-06  8:58 UTC (permalink / raw)
  To: linux-arm-kernel, linux-kernel; +Cc: kernel, broonie, lgirdwood, Lee Jones

This also incorporates the STiH410.

Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 arch/arm/boot/dts/stih407-family.dtsi | 30 ++++++++++++++++++++++++++++++
 arch/arm/boot/dts/stih407.dtsi        | 28 ----------------------------
 2 files changed, 30 insertions(+), 28 deletions(-)

diff --git a/arch/arm/boot/dts/stih407-family.dtsi b/arch/arm/boot/dts/stih407-family.dtsi
index 838b812..64a1a26 100644
--- a/arch/arm/boot/dts/stih407-family.dtsi
+++ b/arch/arm/boot/dts/stih407-family.dtsi
@@ -539,6 +539,7 @@
 			status = "disabled";
 		};
 
+
 		st_dwc3: dwc3@8f94000 {
 			compatible	= "st,stih407-dwc3";
 			reg		= <0x08f94000 0x1000>, <0x110 0x4>;
@@ -565,5 +566,34 @@
 						  <&phy_port2 PHY_TYPE_USB3>;
 			};
 		};
+
+		/* COMMS PWM Module */
+		pwm0: pwm@9810000 {
+			compatible	= "st,sti-pwm";
+			status		= "okay";
+			#pwm-cells	= <2>;
+			reg		= <0x9810000 0x68>;
+			pinctrl-names	= "default";
+			pinctrl-0	= <&pinctrl_pwm0_chan0_default>;
+			clock-names	= "pwm";
+			clocks		= <&clk_sysin>;
+			st,pwm-num-chan = <1>;
+		};
+
+		/* SBC PWM Module */
+		pwm1: pwm@9510000 {
+			compatible	= "st,sti-pwm";
+			status		= "okay";
+			#pwm-cells	= <2>;
+			reg		= <0x9510000 0x68>;
+			pinctrl-names	= "default";
+			pinctrl-0	= <&pinctrl_pwm1_chan0_default
+					&pinctrl_pwm1_chan1_default
+					&pinctrl_pwm1_chan2_default
+					&pinctrl_pwm1_chan3_default>;
+			clock-names	= "pwm";
+			clocks		= <&clk_sysin>;
+			st,pwm-num-chan = <4>;
+		};
 	};
 };
diff --git a/arch/arm/boot/dts/stih407.dtsi b/arch/arm/boot/dts/stih407.dtsi
index 2c560fc3..3efa3b2 100644
--- a/arch/arm/boot/dts/stih407.dtsi
+++ b/arch/arm/boot/dts/stih407.dtsi
@@ -147,33 +147,5 @@
 				};
 			};
 		};
-
-		/* COMMS PWM Module */
-		pwm0: pwm@9810000 {
-			compatible	= "st,sti-pwm";
-			status		= "disabled";
-			#pwm-cells	= <2>;
-			reg		= <0x9810000 0x68>;
-			pinctrl-names	= "default";
-			pinctrl-0	= <&pinctrl_pwm0_chan0_default>;
-			clock-names	= "pwm";
-			clocks		= <&clk_sysin>;
-		};
-
-		/* SBC PWM Module */
-		pwm1: pwm@9510000 {
-			compatible	= "st,sti-pwm";
-			status		= "disabled";
-			#pwm-cells	= <2>;
-			reg		= <0x9510000 0x68>;
-			pinctrl-names	= "default";
-			pinctrl-0	= <&pinctrl_pwm1_chan0_default
-					&pinctrl_pwm1_chan1_default
-					&pinctrl_pwm1_chan2_default
-					&pinctrl_pwm1_chan3_default>;
-			clock-names	= "pwm";
-			clocks		= <&clk_sysin>;
-			st,pwm-num-chan = <4>;
-		};
 	};
 };
-- 
1.9.1


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

* [RESEND 05/10] ARM: STi: STiH407: Add PWM Regulator node
  2015-07-06  8:58 [RESEND 00/10] regulator: pwm-regulator: Introduce continuous-mode Lee Jones
                   ` (3 preceding siblings ...)
  2015-07-06  8:58 ` [RESEND 04/10] ARM: STi: STiH407: Move PWM nodes STiH407 => STiH407-family Lee Jones
@ 2015-07-06  8:58 ` Lee Jones
  2015-07-06  8:58 ` [RESEND 06/10] dt: regulator: pwm-regulator: Re-write bindings Lee Jones
                   ` (5 subsequent siblings)
  10 siblings, 0 replies; 23+ messages in thread
From: Lee Jones @ 2015-07-06  8:58 UTC (permalink / raw)
  To: linux-arm-kernel, linux-kernel; +Cc: kernel, broonie, lgirdwood, Lee Jones

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 64a1a26..3c1bd5d 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] 23+ messages in thread

* [RESEND 06/10] dt: regulator: pwm-regulator: Re-write bindings
  2015-07-06  8:58 [RESEND 00/10] regulator: pwm-regulator: Introduce continuous-mode Lee Jones
                   ` (4 preceding siblings ...)
  2015-07-06  8:58 ` [RESEND 05/10] ARM: STi: STiH407: Add PWM Regulator node Lee Jones
@ 2015-07-06  8:58 ` Lee Jones
  2015-07-07 13:38   ` Mark Brown
  2015-07-06  8:58 ` [RESEND 07/10] regulator: pwm-regulator: Separate voltage-table initialisation Lee Jones
                   ` (4 subsequent siblings)
  10 siblings, 1 reply; 23+ messages in thread
From: Lee Jones @ 2015-07-06  8:58 UTC (permalink / raw)
  To: linux-arm-kernel, linux-kernel
  Cc: kernel, broonie, lgirdwood, Lee Jones, devicetree

* 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@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] 23+ messages in thread

* [RESEND 07/10] regulator: pwm-regulator: Separate voltage-table initialisation
  2015-07-06  8:58 [RESEND 00/10] regulator: pwm-regulator: Introduce continuous-mode Lee Jones
                   ` (5 preceding siblings ...)
  2015-07-06  8:58 ` [RESEND 06/10] dt: regulator: pwm-regulator: Re-write bindings Lee Jones
@ 2015-07-06  8:58 ` Lee Jones
  2015-07-06  8:58 ` [RESEND 08/10] regulator: pwm-regulator: Add support for continuous-voltage Lee Jones
                   ` (3 subsequent siblings)
  10 siblings, 0 replies; 23+ messages in thread
From: Lee Jones @ 2015-07-06  8:58 UTC (permalink / raw)
  To: linux-arm-kernel, linux-kernel; +Cc: kernel, broonie, lgirdwood, Lee Jones

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/regulator/pwm-regulator.c | 77 +++++++++++++++++++++++----------------
 1 file changed, 45 insertions(+), 32 deletions(-)

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] 23+ messages in thread

* [RESEND 08/10] regulator: pwm-regulator: Add support for continuous-voltage
  2015-07-06  8:58 [RESEND 00/10] regulator: pwm-regulator: Introduce continuous-mode Lee Jones
                   ` (6 preceding siblings ...)
  2015-07-06  8:58 ` [RESEND 07/10] regulator: pwm-regulator: Separate voltage-table initialisation Lee Jones
@ 2015-07-06  8:58 ` Lee Jones
  2015-07-07 13:31   ` Mark Brown
  2015-07-06  8:58 ` [RESEND 09/10] regulator: pwm-regulator: Simplify voltage to duty-cycle call Lee Jones
                   ` (2 subsequent siblings)
  10 siblings, 1 reply; 23+ messages in thread
From: Lee Jones @ 2015-07-06  8:58 UTC (permalink / raw)
  To: linux-arm-kernel, linux-kernel; +Cc: kernel, broonie, lgirdwood, Lee Jones

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] 23+ messages in thread

* [RESEND 09/10] regulator: pwm-regulator: Simplify voltage to duty-cycle call
  2015-07-06  8:58 [RESEND 00/10] regulator: pwm-regulator: Introduce continuous-mode Lee Jones
                   ` (7 preceding siblings ...)
  2015-07-06  8:58 ` [RESEND 08/10] regulator: pwm-regulator: Add support for continuous-voltage Lee Jones
@ 2015-07-06  8:58 ` Lee Jones
  2015-07-07 13:32   ` Mark Brown
  2015-07-06  8:58 ` [RESEND 10/10] regulator: pwm-regulator: Don't assign structure attributes right away Lee Jones
  2015-07-06 20:48 ` [RESEND 00/10] regulator: pwm-regulator: Introduce continuous-mode Mark Brown
  10 siblings, 1 reply; 23+ messages in thread
From: Lee Jones @ 2015-07-06  8:58 UTC (permalink / raw)
  To: linux-arm-kernel, linux-kernel; +Cc: kernel, broonie, lgirdwood, Lee Jones

If we reverse some of the logic and change the formula used,
we can simplify the function greatly.

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

diff --git a/drivers/regulator/pwm-regulator.c b/drivers/regulator/pwm-regulator.c
index b37b616..d5cb267 100644
--- a/drivers/regulator/pwm-regulator.c
+++ b/drivers/regulator/pwm-regulator.c
@@ -93,26 +93,13 @@ static int pwm_regulator_list_voltage(struct regulator_dev *rdev,
 /**
  * Continuous voltage call-backs
  */
-static int pwm_voltage_to_duty_cycle(struct regulator_dev *rdev,
-					int volt_mV)
+static int pwm_voltage_to_duty_cycle(struct regulator_dev *rdev, int req_uV)
 {
-	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;
+	int min_uV = rdev->constraints->min_uV;
+	int max_uV = rdev->constraints->max_uV;
+	int diff = max_uV - min_uV;
+
+	return 100 - ((((req_uV * 100) - (min_uV * 100)) / diff));
 }
 
 static int pwm_regulator_get_voltage(struct regulator_dev *rdev)
@@ -131,7 +118,7 @@ static int pwm_regulator_set_voltage(struct regulator_dev *rdev,
 	int duty_cycle;
 	int ret;
 
-	duty_cycle = pwm_voltage_to_duty_cycle(rdev, min_uV / 1000);
+	duty_cycle = pwm_voltage_to_duty_cycle(rdev, min_uV);
 
 	ret = pwm_config(drvdata->pwm,
 			 (drvdata->pwm->period / 100) * duty_cycle,
-- 
1.9.1


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

* [RESEND 10/10] regulator: pwm-regulator: Don't assign structure attributes right away
  2015-07-06  8:58 [RESEND 00/10] regulator: pwm-regulator: Introduce continuous-mode Lee Jones
                   ` (8 preceding siblings ...)
  2015-07-06  8:58 ` [RESEND 09/10] regulator: pwm-regulator: Simplify voltage to duty-cycle call Lee Jones
@ 2015-07-06  8:58 ` Lee Jones
  2015-07-06 20:48 ` [RESEND 00/10] regulator: pwm-regulator: Introduce continuous-mode Mark Brown
  10 siblings, 0 replies; 23+ messages in thread
From: Lee Jones @ 2015-07-06  8:58 UTC (permalink / raw)
  To: linux-arm-kernel, linux-kernel; +Cc: kernel, broonie, lgirdwood, Lee Jones

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.

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 d5cb267..cb48208 100644
--- a/drivers/regulator/pwm-regulator.c
+++ b/drivers/regulator/pwm-regulator.c
@@ -218,6 +218,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 = { };
@@ -240,14 +241,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] 23+ messages in thread

* Re: [RESEND 00/10] regulator: pwm-regulator: Introduce continuous-mode
  2015-07-06  8:58 [RESEND 00/10] regulator: pwm-regulator: Introduce continuous-mode Lee Jones
                   ` (9 preceding siblings ...)
  2015-07-06  8:58 ` [RESEND 10/10] regulator: pwm-regulator: Don't assign structure attributes right away Lee Jones
@ 2015-07-06 20:48 ` Mark Brown
  2015-07-07  6:52   ` Lee Jones
  10 siblings, 1 reply; 23+ messages in thread
From: Mark Brown @ 2015-07-06 20:48 UTC (permalink / raw)
  To: Lee Jones; +Cc: linux-arm-kernel, linux-kernel, kernel, lgirdwood

[-- Attachment #1: Type: text/plain, Size: 244 bytes --]

On Mon, Jul 06, 2015 at 09:58:22AM +0100, Lee Jones wrote:
> This patch-set has been rebased on to v4.2-rc1.

Is this just a rebase or has the code been changed as well - IIRC there
were some bits in there that weren't supposed to be be there?

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

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

* Re: [RESEND 00/10] regulator: pwm-regulator: Introduce continuous-mode
  2015-07-06 20:48 ` [RESEND 00/10] regulator: pwm-regulator: Introduce continuous-mode Mark Brown
@ 2015-07-07  6:52   ` Lee Jones
  2015-07-07 12:03     ` Mark Brown
  0 siblings, 1 reply; 23+ messages in thread
From: Lee Jones @ 2015-07-07  6:52 UTC (permalink / raw)
  To: Mark Brown; +Cc: linux-arm-kernel, linux-kernel, kernel, lgirdwood

On Mon, 06 Jul 2015, Mark Brown wrote:

> On Mon, Jul 06, 2015 at 09:58:22AM +0100, Lee Jones wrote:
> > This patch-set has been rebased on to v4.2-rc1.
> 
> Is this just a rebase or has the code been changed as well - IIRC there
> were some bits in there that weren't supposed to be be there?

This is a rebased resend with those couple of lines of debug removed.

-- 
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] 23+ messages in thread

* Re: [RESEND 00/10] regulator: pwm-regulator: Introduce continuous-mode
  2015-07-07  6:52   ` Lee Jones
@ 2015-07-07 12:03     ` Mark Brown
  2015-07-07 12:52       ` Lee Jones
  0 siblings, 1 reply; 23+ messages in thread
From: Mark Brown @ 2015-07-07 12:03 UTC (permalink / raw)
  To: Lee Jones; +Cc: linux-arm-kernel, linux-kernel, kernel, lgirdwood

[-- Attachment #1: Type: text/plain, Size: 472 bytes --]

On Tue, Jul 07, 2015 at 07:52:50AM +0100, Lee Jones wrote:
> On Mon, 06 Jul 2015, Mark Brown wrote:
> > On Mon, Jul 06, 2015 at 09:58:22AM +0100, Lee Jones wrote:

> > > This patch-set has been rebased on to v4.2-rc1.

> > Is this just a rebase or has the code been changed as well - IIRC there
> > were some bits in there that weren't supposed to be be there?

> This is a rebased resend with those couple of lines of debug removed.

OK, so not actually a resend then :(

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

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

* Re: [RESEND 00/10] regulator: pwm-regulator: Introduce continuous-mode
  2015-07-07 12:03     ` Mark Brown
@ 2015-07-07 12:52       ` Lee Jones
  0 siblings, 0 replies; 23+ messages in thread
From: Lee Jones @ 2015-07-07 12:52 UTC (permalink / raw)
  To: Mark Brown; +Cc: linux-arm-kernel, linux-kernel, kernel, lgirdwood

On Tue, 07 Jul 2015, Mark Brown wrote:
> On Tue, Jul 07, 2015 at 07:52:50AM +0100, Lee Jones wrote:
> > On Mon, 06 Jul 2015, Mark Brown wrote:
> > > On Mon, Jul 06, 2015 at 09:58:22AM +0100, Lee Jones wrote:
> 
> > > > This patch-set has been rebased on to v4.2-rc1.
> 
> > > Is this just a rebase or has the code been changed as well - IIRC there
> > > were some bits in there that weren't supposed to be be there?
> 
> > This is a rebased resend with those couple of lines of debug removed.
> 
> OK, so not actually a resend then :(

The changes are very trivial.  Literally just 4 debug prints removed.

-- 
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] 23+ messages in thread

* Re: [RESEND 08/10] regulator: pwm-regulator: Add support for continuous-voltage
  2015-07-06  8:58 ` [RESEND 08/10] regulator: pwm-regulator: Add support for continuous-voltage Lee Jones
@ 2015-07-07 13:31   ` Mark Brown
  2015-07-07 13:45     ` Lee Jones
  0 siblings, 1 reply; 23+ messages in thread
From: Mark Brown @ 2015-07-07 13:31 UTC (permalink / raw)
  To: Lee Jones; +Cc: linux-arm-kernel, linux-kernel, kernel, lgirdwood

[-- Attachment #1: Type: text/plain, Size: 235 bytes --]

On Mon, Jul 06, 2015 at 09:58:30AM +0100, Lee Jones wrote:

> +	tmp = max_duty_cycle - min_mV * max_duty_cycle / vdiff;
> +	pwm_code = tmp + max_duty_cycle * volt_mV / vdiff;

These need some brackets to be clear that they're correct.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

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

* Re: [RESEND 09/10] regulator: pwm-regulator: Simplify voltage to duty-cycle call
  2015-07-06  8:58 ` [RESEND 09/10] regulator: pwm-regulator: Simplify voltage to duty-cycle call Lee Jones
@ 2015-07-07 13:32   ` Mark Brown
  2015-07-07 13:45     ` Lee Jones
  0 siblings, 1 reply; 23+ messages in thread
From: Mark Brown @ 2015-07-07 13:32 UTC (permalink / raw)
  To: Lee Jones; +Cc: linux-arm-kernel, linux-kernel, kernel, lgirdwood

[-- Attachment #1: Type: text/plain, Size: 366 bytes --]

On Mon, Jul 06, 2015 at 09:58:31AM +0100, Lee Jones wrote:
> If we reverse some of the logic and change the formula used,
> we can simplify the function greatly.

> +static int pwm_voltage_to_duty_cycle(struct regulator_dev *rdev, int req_uV)
>  {
> -	struct pwm_regulator_data *drvdata = rdev_get_drvdata(rdev);

You just added this function in the previous patch?

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

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

* Re: [RESEND 06/10] dt: regulator: pwm-regulator: Re-write bindings
  2015-07-06  8:58 ` [RESEND 06/10] dt: regulator: pwm-regulator: Re-write bindings Lee Jones
@ 2015-07-07 13:38   ` Mark Brown
  0 siblings, 0 replies; 23+ messages in thread
From: Mark Brown @ 2015-07-07 13:38 UTC (permalink / raw)
  To: Lee Jones; +Cc: linux-arm-kernel, linux-kernel, kernel, lgirdwood, devicetree

[-- Attachment #1: Type: text/plain, Size: 364 bytes --]

On Mon, Jul 06, 2015 at 09:58:28AM +0100, Lee Jones wrote:
> * 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

Please when posting again use a subject line reflecting the style for
the subsystem and try to reflow the documentation to fit within 80
columns.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

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

* Re: [RESEND 09/10] regulator: pwm-regulator: Simplify voltage to duty-cycle call
  2015-07-07 13:32   ` Mark Brown
@ 2015-07-07 13:45     ` Lee Jones
  2015-07-07 13:52       ` Mark Brown
  0 siblings, 1 reply; 23+ messages in thread
From: Lee Jones @ 2015-07-07 13:45 UTC (permalink / raw)
  To: Mark Brown; +Cc: linux-arm-kernel, linux-kernel, kernel, lgirdwood

On Tue, 07 Jul 2015, Mark Brown wrote:

> On Mon, Jul 06, 2015 at 09:58:31AM +0100, Lee Jones wrote:
> > If we reverse some of the logic and change the formula used,
> > we can simplify the function greatly.
> 
> > +static int pwm_voltage_to_duty_cycle(struct regulator_dev *rdev, int req_uV)
> >  {
> > -	struct pwm_regulator_data *drvdata = rdev_get_drvdata(rdev);
> 
> You just added this function in the previous patch?

You're right, it does look a little weird contained in a single
patch-set.  The submission in the previous patch is the tried and
tested (i.e. in real releases) method written by ST.  This patch
contains a simplification provided by me.  IMO it looks and performs
better, but doesn't have the same time-under-test that the original
method does.  I'm merely ensuring we keep some history in order so
provide and easy way back i.e. revert.

If I have any say at all, I'd really like to keep this piece of
history.

-- 
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] 23+ messages in thread

* Re: [RESEND 08/10] regulator: pwm-regulator: Add support for continuous-voltage
  2015-07-07 13:31   ` Mark Brown
@ 2015-07-07 13:45     ` Lee Jones
  2015-07-07 13:53       ` Mark Brown
  0 siblings, 1 reply; 23+ messages in thread
From: Lee Jones @ 2015-07-07 13:45 UTC (permalink / raw)
  To: Mark Brown; +Cc: linux-arm-kernel, linux-kernel, kernel, lgirdwood

On Tue, 07 Jul 2015, Mark Brown wrote:

> On Mon, Jul 06, 2015 at 09:58:30AM +0100, Lee Jones wrote:
> 
> > +	tmp = max_duty_cycle - min_mV * max_duty_cycle / vdiff;
> > +	pwm_code = tmp + max_duty_cycle * volt_mV / vdiff;
> 
> These need some brackets to be clear that they're correct.

As you rightly pointed out, this stuff gets over-written in the next
patch, although I have no problem with providing some bracketing if
required.

-- 
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] 23+ messages in thread

* Re: [RESEND 09/10] regulator: pwm-regulator: Simplify voltage to duty-cycle call
  2015-07-07 13:45     ` Lee Jones
@ 2015-07-07 13:52       ` Mark Brown
  2015-07-07 14:29         ` Lee Jones
  0 siblings, 1 reply; 23+ messages in thread
From: Mark Brown @ 2015-07-07 13:52 UTC (permalink / raw)
  To: Lee Jones; +Cc: linux-arm-kernel, linux-kernel, kernel, lgirdwood

[-- Attachment #1: Type: text/plain, Size: 764 bytes --]

On Tue, Jul 07, 2015 at 02:45:03PM +0100, Lee Jones wrote:
> On Tue, 07 Jul 2015, Mark Brown wrote:

> > You just added this function in the previous patch?

> You're right, it does look a little weird contained in a single
> patch-set.  The submission in the previous patch is the tried and
> tested (i.e. in real releases) method written by ST.  This patch
> contains a simplification provided by me.  IMO it looks and performs
> better, but doesn't have the same time-under-test that the original
> method does.  I'm merely ensuring we keep some history in order so
> provide and easy way back i.e. revert.

> If I have any say at all, I'd really like to keep this piece of
> history.

OK, that makes sense - can you put the above in the commit message
please?

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

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

* Re: [RESEND 08/10] regulator: pwm-regulator: Add support for continuous-voltage
  2015-07-07 13:45     ` Lee Jones
@ 2015-07-07 13:53       ` Mark Brown
  0 siblings, 0 replies; 23+ messages in thread
From: Mark Brown @ 2015-07-07 13:53 UTC (permalink / raw)
  To: Lee Jones; +Cc: linux-arm-kernel, linux-kernel, kernel, lgirdwood

[-- Attachment #1: Type: text/plain, Size: 519 bytes --]

On Tue, Jul 07, 2015 at 02:45:45PM +0100, Lee Jones wrote:
> On Tue, 07 Jul 2015, Mark Brown wrote:
> > On Mon, Jul 06, 2015 at 09:58:30AM +0100, Lee Jones wrote:

> > > +	tmp = max_duty_cycle - min_mV * max_duty_cycle / vdiff;
> > > +	pwm_code = tmp + max_duty_cycle * volt_mV / vdiff;

> > These need some brackets to be clear that they're correct.

> As you rightly pointed out, this stuff gets over-written in the next
> patch, although I have no problem with providing some bracketing if
> required.

Yes, please.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

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

* Re: [RESEND 09/10] regulator: pwm-regulator: Simplify voltage to duty-cycle call
  2015-07-07 13:52       ` Mark Brown
@ 2015-07-07 14:29         ` Lee Jones
  0 siblings, 0 replies; 23+ messages in thread
From: Lee Jones @ 2015-07-07 14:29 UTC (permalink / raw)
  To: Mark Brown; +Cc: linux-arm-kernel, linux-kernel, kernel, lgirdwood

On Tue, 07 Jul 2015, Mark Brown wrote:

> On Tue, Jul 07, 2015 at 02:45:03PM +0100, Lee Jones wrote:
> > On Tue, 07 Jul 2015, Mark Brown wrote:
> 
> > > You just added this function in the previous patch?
> 
> > You're right, it does look a little weird contained in a single
> > patch-set.  The submission in the previous patch is the tried and
> > tested (i.e. in real releases) method written by ST.  This patch
> > contains a simplification provided by me.  IMO it looks and performs
> > better, but doesn't have the same time-under-test that the original
> > method does.  I'm merely ensuring we keep some history in order so
> > provide and easy way back i.e. revert.
> 
> > If I have any say at all, I'd really like to keep this piece of
> > history.
> 
> OK, that makes sense - can you put the above in the commit message
> please?

Sure.

-- 
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] 23+ messages in thread

end of thread, other threads:[~2015-07-07 14:29 UTC | newest]

Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-07-06  8:58 [RESEND 00/10] regulator: pwm-regulator: Introduce continuous-mode Lee Jones
2015-07-06  8:58 ` [RESEND 01/10] ARM: multi_v7_defconfig: Enable ST's PWM driver Lee Jones
2015-07-06  8:58 ` [RESEND 02/10] ARM: multi_v7_defconfig: Enable ST's Power Reset driver Lee Jones
2015-07-06  8:58 ` [RESEND 03/10] ARM: multi_v7_defconfig: Enable support for PWM Regulators Lee Jones
2015-07-06  8:58 ` [RESEND 04/10] ARM: STi: STiH407: Move PWM nodes STiH407 => STiH407-family Lee Jones
2015-07-06  8:58 ` [RESEND 05/10] ARM: STi: STiH407: Add PWM Regulator node Lee Jones
2015-07-06  8:58 ` [RESEND 06/10] dt: regulator: pwm-regulator: Re-write bindings Lee Jones
2015-07-07 13:38   ` Mark Brown
2015-07-06  8:58 ` [RESEND 07/10] regulator: pwm-regulator: Separate voltage-table initialisation Lee Jones
2015-07-06  8:58 ` [RESEND 08/10] regulator: pwm-regulator: Add support for continuous-voltage Lee Jones
2015-07-07 13:31   ` Mark Brown
2015-07-07 13:45     ` Lee Jones
2015-07-07 13:53       ` Mark Brown
2015-07-06  8:58 ` [RESEND 09/10] regulator: pwm-regulator: Simplify voltage to duty-cycle call Lee Jones
2015-07-07 13:32   ` Mark Brown
2015-07-07 13:45     ` Lee Jones
2015-07-07 13:52       ` Mark Brown
2015-07-07 14:29         ` Lee Jones
2015-07-06  8:58 ` [RESEND 10/10] regulator: pwm-regulator: Don't assign structure attributes right away Lee Jones
2015-07-06 20:48 ` [RESEND 00/10] regulator: pwm-regulator: Introduce continuous-mode Mark Brown
2015-07-07  6:52   ` Lee Jones
2015-07-07 12:03     ` Mark Brown
2015-07-07 12:52       ` 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).