[PATCH 0/5] regulator: pwm: Add supports for multiple instance and voltage linear steps

[PATCH 0/5] regulator: pwm: Add supports for multiple instance and voltage linear steps

[Laxman Dewangan]

This series fix the equation for calculating the ducty cycle for PWM,
add supports for multiple instances, prints the error numbers if it fails,
and add Linear Equal Step mode for voltage setting.

For the context: NVIDIA's Tegra210 platform Jetson-TX1 uses the two
Open Voltage Regulators (OVR i.e. uP1665P) regulators for CPU and
GPU rails. These regulators are controlled by the PWM controller.

Laxman Dewangan (5):
  regulator: pwm: Fix calculation of voltage-to-duty cycle
  regulator: pwm: Add support to have multiple instance of pwm regulator
  regulator: pwm: Prints error number when it fails
  regulator: pwm: Add support for voltage linear steps
  regulator: pwm: Add DT binding details for Linear Equal Step Mode

 .../bindings/regulator/pwm-regulator.txt           |  32 +++++-
 drivers/regulator/pwm-regulator.c                  | 109 +++++++++++++++++----
 2 files changed, 120 insertions(+), 21 deletions(-)

-- 
2.1.4

[PATCH 1/5] regulator: pwm: Fix calculation of voltage-to-duty cycle

[Laxman Dewangan]

With following equation for calculating
voltage_to_duty_cycle_percentage
	100 - (((req_uV * 100) - (min_uV * 100)) / diff);

we get 0% for max_uV and 100% for min_uV.

Correcting this to
	((req_uV * 100) - (min_uV * 100)) / diff;
 to get proper duty cycle.

Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
Cc: Lee Jones <lee.jones@linaro.org>
---
 drivers/regulator/pwm-regulator.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/regulator/pwm-regulator.c b/drivers/regulator/pwm-regulator.c
index 3aca067..4d8eb35 100644
--- a/drivers/regulator/pwm-regulator.c
+++ b/drivers/regulator/pwm-regulator.c
@@ -115,7 +115,7 @@ static int pwm_voltage_to_duty_cycle_percentage(struct regulator_dev *rdev, int
 	int max_uV = rdev->constraints->max_uV;
 	int diff = max_uV - min_uV;
 
-	return 100 - (((req_uV * 100) - (min_uV * 100)) / diff);
+	return ((req_uV * 100) - (min_uV * 100)) / diff;
 }
 
 static int pwm_regulator_get_voltage(struct regulator_dev *rdev)
-- 
2.1.4

[PATCH 2/5] regulator: pwm: Add support to have multiple instance of pwm regulator

[Laxman Dewangan]

Some of platforms like Nvidia's Tegra210 Jetson-TX1 platform has
multiple PMW based regulators. Add support to have multiple instances
of the driver by not changing any global data of pwm regulator and
if required, making instance specific copy and then making changes.

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

diff --git a/drivers/regulator/pwm-regulator.c b/drivers/regulator/pwm-regulator.c
index 4d8eb35..4689d62 100644
--- a/drivers/regulator/pwm-regulator.c
+++ b/drivers/regulator/pwm-regulator.c
@@ -27,6 +27,13 @@ struct pwm_regulator_data {
 
 	/* Voltage table */
 	struct pwm_voltages *duty_cycle_table;
+
+	/* regulator descriptor */
+	struct regulator_desc desc;
+
+	/* Regulator ops */
+	struct regulator_ops ops;
+
 	int state;
 
 	/* Continuous voltage */
@@ -212,8 +219,10 @@ static int pwm_regulator_init_table(struct platform_device *pdev,
 	}
 
 	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);
+	memcpy(&drvdata->ops, &pwm_regulator_voltage_table_ops,
+	       sizeof(drvdata->ops));
+	drvdata->desc.ops = &drvdata->ops;
+	drvdata->desc.n_voltages	= length / sizeof(*duty_cycle_table);
 
 	return 0;
 }
@@ -221,8 +230,10 @@ static int pwm_regulator_init_table(struct platform_device *pdev,
 static int pwm_regulator_init_continuous(struct platform_device *pdev,
 					 struct pwm_regulator_data *drvdata)
 {
-	pwm_regulator_desc.ops = &pwm_regulator_voltage_continuous_ops;
-	pwm_regulator_desc.continuous_voltage_range = true;
+	memcpy(&drvdata->ops, &pwm_regulator_voltage_continuous_ops,
+	       sizeof(drvdata->ops));
+	drvdata->desc.ops = &drvdata->ops;
+	drvdata->desc.continuous_voltage_range = true;
 
 	return 0;
 }
@@ -245,6 +256,8 @@ static int pwm_regulator_probe(struct platform_device *pdev)
 	if (!drvdata)
 		return -ENOMEM;
 
+	memcpy(&drvdata->desc, &pwm_regulator_desc, sizeof(drvdata->desc));
+
 	if (of_find_property(np, "voltage-table", NULL))
 		ret = pwm_regulator_init_table(pdev, drvdata);
 	else
@@ -253,7 +266,7 @@ static int pwm_regulator_probe(struct platform_device *pdev)
 		return ret;
 
 	init_data = of_get_regulator_init_data(&pdev->dev, np,
-					       &pwm_regulator_desc);
+					       &drvdata->desc);
 	if (!init_data)
 		return -ENOMEM;
 
@@ -269,10 +282,10 @@ static int pwm_regulator_probe(struct platform_device *pdev)
 	}
 
 	regulator = devm_regulator_register(&pdev->dev,
-					    &pwm_regulator_desc, &config);
+					    &drvdata->desc, &config);
 	if (IS_ERR(regulator)) {
 		dev_err(&pdev->dev, "Failed to register regulator %s\n",
-			pwm_regulator_desc.name);
+			drvdata->desc.name);
 		return PTR_ERR(regulator);
 	}
 
-- 
2.1.4

[PATCH 3/5] regulator: pwm: Prints error number when it fails

[Laxman Dewangan]

Prints the error number along with error message when any
error occurs. This helps on getting the reason of failure
quickly from log without any code instrument.

Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
Cc: Lee Jones <lee.jones@linaro.org>
---
 drivers/regulator/pwm-regulator.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/drivers/regulator/pwm-regulator.c b/drivers/regulator/pwm-regulator.c
index 4689d62..9db67a1 100644
--- a/drivers/regulator/pwm-regulator.c
+++ b/drivers/regulator/pwm-regulator.c
@@ -277,16 +277,18 @@ static int pwm_regulator_probe(struct platform_device *pdev)
 
 	drvdata->pwm = devm_pwm_get(&pdev->dev, NULL);
 	if (IS_ERR(drvdata->pwm)) {
-		dev_err(&pdev->dev, "Failed to get PWM\n");
-		return PTR_ERR(drvdata->pwm);
+		ret = PTR_ERR(drvdata->pwm);
+		dev_err(&pdev->dev, "Failed to get PWM, %d\n", ret);
+		return ret;
 	}
 
 	regulator = devm_regulator_register(&pdev->dev,
 					    &drvdata->desc, &config);
 	if (IS_ERR(regulator)) {
-		dev_err(&pdev->dev, "Failed to register regulator %s\n",
-			drvdata->desc.name);
-		return PTR_ERR(regulator);
+		ret = PTR_ERR(regulator);
+		dev_err(&pdev->dev, "Failed to register regulator %s, %d\n",
+			drvdata->desc.name, ret);
+		return ret;
 	}
 
 	return 0;
-- 
2.1.4

Re: [PATCH 3/5] regulator: pwm: Prints error number when it fails

[Mark Brown]

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

On Tue, Mar 08, 2016 at 04:23:23PM +0530, Laxman Dewangan wrote:

> +		dev_err(&pdev->dev, "Failed to get PWM, %d\n", ret);

Pretty much everywhere else in the kernel we use "foo: %d\n" - please
use a consistent style.

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

Re: [PATCH 3/5] regulator: pwm: Prints error number when it fails

[Laxman Dewangan]

On Saturday 12 March 2016 11:35 AM, Mark Brown wrote:
> * PGP Signed by an unknown key
>
> On Tue, Mar 08, 2016 at 04:23:23PM +0530, Laxman Dewangan wrote:
>
>> +		dev_err(&pdev->dev, "Failed to get PWM, %d\n", ret);
> Pretty much everywhere else in the kernel we use "foo: %d\n" - please
> use a consistent style.

I saw mixed approach and so always thought what would be better.
Great to know that it is foo: %d style is far better than other way.

Will correct and post the another patch.

[PATCH 4/5] regulator: pwm: Add support for voltage linear equal steps

[Laxman Dewangan]

Currently PWM regulator support the multiple voltage by
two different mechanism: Voltage table based and continuous
steps which divides duty cycle into 100 parts.

There is a use cases where entire voltage ranges from minimum
to maximum is divided into n equal steps and just providing the
steps count, the voltage table with duty cycles is linearly
calculated.

Add support to have specified linear n equal steps for specified
regulator's minimum and maximum voltage range for PWM regulators.

Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
Cc: Lee Jones <lee.jones@linaro.org>
---
 drivers/regulator/pwm-regulator.c | 76 ++++++++++++++++++++++++++++++++++-----
 1 file changed, 68 insertions(+), 8 deletions(-)

diff --git a/drivers/regulator/pwm-regulator.c b/drivers/regulator/pwm-regulator.c
index 9db67a1..060e5cf 100644
--- a/drivers/regulator/pwm-regulator.c
+++ b/drivers/regulator/pwm-regulator.c
@@ -38,6 +38,9 @@ struct pwm_regulator_data {
 
 	/* Continuous voltage */
 	int volt_uV;
+
+	/* Regulator n linear steps */
+	unsigned int regulator_n_steps;
 };
 
 struct pwm_voltages {
@@ -65,7 +68,11 @@ static int pwm_regulator_set_voltage_sel(struct regulator_dev *rdev,
 
 	pwm_reg_period = pwm_get_period(drvdata->pwm);
 
-	dutycycle = (pwm_reg_period *
+	if (drvdata->regulator_n_steps)
+		dutycycle = (pwm_reg_period * selector) /
+					(drvdata->regulator_n_steps - 1);
+	else
+		dutycycle = (pwm_reg_period *
 		    drvdata->duty_cycle_table[selector].dutycycle) / 100;
 
 	ret = pwm_config(drvdata->pwm, dutycycle, pwm_reg_period);
@@ -227,6 +234,41 @@ static int pwm_regulator_init_table(struct platform_device *pdev,
 	return 0;
 }
 
+static int pwm_regulator_init_linear_steps(struct platform_device *pdev,
+					   struct pwm_regulator_data *drvdata)
+{
+	struct device_node *np = pdev->dev.of_node;
+	unsigned int period;
+	u32 uval;
+	int ret;
+
+	ret = of_property_read_u32(np, "regulator-n-voltages", &uval);
+	if (ret < 0)
+		return ret;
+	if (uval < 2) {
+		dev_err(&pdev->dev, "Invalid number of voltage steps\n");
+		return -EINVAL;
+	}
+
+	period = pwm_get_period(drvdata->pwm);
+	if (period % (uval - 1)) {
+		dev_err(&pdev->dev, "PWM Period must multiple of n_voltages\n");
+		return -EINVAL;
+	}
+
+	memcpy(&drvdata->ops, &pwm_regulator_voltage_table_ops,
+	       sizeof(drvdata->ops));
+	drvdata->ops.list_voltage = regulator_list_voltage_linear;
+	drvdata->ops.map_voltage = regulator_map_voltage_linear;
+
+	drvdata->regulator_n_steps = uval;
+	drvdata->desc.ops = &drvdata->ops;
+	drvdata->desc.linear_min_sel = 0;
+	drvdata->desc.n_voltages = drvdata->regulator_n_steps;
+
+	return 0;
+}
+
 static int pwm_regulator_init_continuous(struct platform_device *pdev,
 					 struct pwm_regulator_data *drvdata)
 {
@@ -256,10 +298,19 @@ static int pwm_regulator_probe(struct platform_device *pdev)
 	if (!drvdata)
 		return -ENOMEM;
 
+	drvdata->pwm = devm_pwm_get(&pdev->dev, NULL);
+	if (IS_ERR(drvdata->pwm)) {
+		ret = PTR_ERR(drvdata->pwm);
+		dev_err(&pdev->dev, "Failed to get PWM, %d\n", ret);
+		return ret;
+	}
+
 	memcpy(&drvdata->desc, &pwm_regulator_desc, sizeof(drvdata->desc));
 
 	if (of_find_property(np, "voltage-table", NULL))
 		ret = pwm_regulator_init_table(pdev, drvdata);
+	else if (of_find_property(np, "regulator-n-voltages", NULL))
+		ret = pwm_regulator_init_linear_steps(pdev, drvdata);
 	else
 		ret = pwm_regulator_init_continuous(pdev, drvdata);
 	if (ret)
@@ -270,18 +321,27 @@ static int pwm_regulator_probe(struct platform_device *pdev)
 	if (!init_data)
 		return -ENOMEM;
 
+	if (drvdata->regulator_n_steps) {
+		int min_uV = init_data->constraints.min_uV;
+		int max_uV = init_data->constraints.max_uV;
+		int step_uV;
+
+		if ((max_uV - min_uV) % (drvdata->regulator_n_steps - 1)) {
+			dev_err(&pdev->dev,
+				"Min/Max is not proper to get step voltage\n");
+			return -EINVAL;
+		}
+
+		step_uV = (max_uV - min_uV) / (drvdata->regulator_n_steps - 1);
+		drvdata->desc.min_uV = min_uV;
+		drvdata->desc.uV_step = step_uV;
+	}
+
 	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)) {
-		ret = PTR_ERR(drvdata->pwm);
-		dev_err(&pdev->dev, "Failed to get PWM, %d\n", ret);
-		return ret;
-	}
-
 	regulator = devm_regulator_register(&pdev->dev,
 					    &drvdata->desc, &config);
 	if (IS_ERR(regulator)) {
-- 
2.1.4

Re: [PATCH 4/5] regulator: pwm: Add support for voltage linear equal steps

[Mark Brown]

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

On Tue, Mar 08, 2016 at 04:23:24PM +0530, Laxman Dewangan wrote:

> There is a use cases where entire voltage ranges from minimum
> to maximum is divided into n equal steps and just providing the
> steps count, the voltage table with duty cycles is linearly
> calculated.

I can't see any reason why this would ever be preferable to just using
the flat linear range (you certainly haven't articulated one, you're
just stating it).  This seems like you are bodging around a limited
consumer driver, you should fix the consumer to cope with regulators
with lots of voltages - PWM regulators aren't the only ones with high
resolution steps.

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

Re: [PATCH 4/5] regulator: pwm: Add support for voltage linear equal steps

[Laxman Dewangan]

On Saturday 12 March 2016 11:39 AM, Mark Brown wrote:
> * PGP Signed by an unknown key
>
> On Tue, Mar 08, 2016 at 04:23:24PM +0530, Laxman Dewangan wrote:
>
>> There is a use cases where entire voltage ranges from minimum
>> to maximum is divided into n equal steps and just providing the
>> steps count, the voltage table with duty cycles is linearly
>> calculated.
> I can't see any reason why this would ever be preferable to just using
> the flat linear range (you certainly haven't articulated one, you're
> just stating it).  This seems like you are bodging around a limited
> consumer driver, you should fix the consumer to cope with regulators
> with lots of voltages - PWM regulators aren't the only ones with high
> resolution steps.

The requirement is to have perfect linear steps interms of the 
period/pulse time of PWM without loosing any voltage.
Continuous mode is pretty much near to what you said but here we are 
loosing the perfect step as this divides the periods to 100 parts and 
then set voltage.

If new mode is not accpetable then need to enhance the existing 
continuous mode like before scaling for 100% of period, first look if we 
get the perfect pulse time of of PWM period and if it is there then use 
this direct instead of converting required voltage to 100% scale and 
then back calculating duty time.

Re: [PATCH 4/5] regulator: pwm: Add support for voltage linear equal steps

[Mark Brown]

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

On Sun, Mar 13, 2016 at 06:36:06PM +0530, Laxman Dewangan wrote:
> On Saturday 12 March 2016 11:39 AM, Mark Brown wrote:

> >I can't see any reason why this would ever be preferable to just using
> >the flat linear range (you certainly haven't articulated one, you're
> >just stating it).  This seems like you are bodging around a limited
> >consumer driver, you should fix the consumer to cope with regulators
> >with lots of voltages - PWM regulators aren't the only ones with high
> >resolution steps.

> The requirement is to have perfect linear steps interms of the period/pulse
> time of PWM without loosing any voltage.
> Continuous mode is pretty much near to what you said but here we are loosing
> the perfect step as this divides the periods to 100 parts and then set
> voltage.

Could you be more specific about what the issue is?  We've hopefully got
errors of less than 1% in the values here...

> If new mode is not accpetable then need to enhance the existing continuous
> mode like before scaling for 100% of period, first look if we get the
> perfect pulse time of of PWM period and if it is there then use this direct
> instead of converting required voltage to 100% scale and then back
> calculating duty time.

That seems a lot better, what you're proposing is changing the ABI to
fudge things for a client specific requirement where both the client
requirement and the change you're proposing are specific to the current
Linux implementation.  That doesn't sound like a DT thing.

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

Re: [PATCH 4/5] regulator: pwm: Add support for voltage linear equal steps

[Laxman Dewangan]

On Monday 14 March 2016 09:58 PM, Mark Brown wrote:
> * PGP Signed by an unknown key
>
> On Sun, Mar 13, 2016 at 06:36:06PM +0530, Laxman Dewangan wrote:
>> On Saturday 12 March 2016 11:39 AM, Mark Brown wrote:
>>> I can't see any reason why this would ever be preferable to just using
>>> the flat linear range (you certainly haven't articulated one, you're
>>> just stating it).  This seems like you are bodging around a limited
>>> consumer driver, you should fix the consumer to cope with regulators
>>> with lots of voltages - PWM regulators aren't the only ones with high
>>> resolution steps.
>> The requirement is to have perfect linear steps interms of the period/pulse
>> time of PWM without loosing any voltage.
>> Continuous mode is pretty much near to what you said but here we are loosing
>> the perfect step as this divides the periods to 100 parts and then set
>> voltage.
> Could you be more specific about what the issue is?  We've hopefully got
> errors of less than 1% in the values here...
>
If I use the continuous mode of PWM regulator then the calculation for 
PWM pulse ON time(duty_pulse)
done as:
         duty_cycle = ((requested - minimum) * 100) / voltage_range.

         duty_pulse = (pwm_period/100) * duty_cycle

This leads to the calculation error if we have the requested voltage 
where accurate pulse time is possible. For example: Let's have following 
case
         voltage range is 800000uV to 1350000uV.
         pwm-period = 1550ns (1ns time is 1mV).
         Requested 900000uV.

         duty_cycle = ((900000uV - 800000uV) * 100)/ 1550000
                    = 6.45 but we will get 6 due to integer division.

         duty_pulse = (1550/100) * 6 = 90 pulse time.

     90 pulse time is equivalent to 90mV and this gives us pulse time 
equivalent
     to 890000uV instead of 900000uV.


>> If new mode is not accpetable then need to enhance the existing continuous
>> mode like before scaling for 100% of period, first look if we get the
>> perfect pulse time of of PWM period and if it is there then use this direct
>> instead of converting required voltage to 100% scale and then back
>> calculating duty time.
> That seems a lot better,

You mean using the continuous mode only.

If I add following logic then also it resolve the issue:
if (((req_uV - min_uV) * pwm_period) % voltage_range == 0)
     duty_pulse =  ((req_uV - min_uV) * pwm_period) / voltage_range;
else
     existing_continuous mode calculation.

So on above example:
     duty_pulse  = ((900000uV - 800000uV) * 1550)/1550000)
                        = 100

and this is equivalent to 100mV and so final voltage is
     (800000 + 100000) = 900000uV which is same as requested,

[PATCH 5/5] regulator: pwm: Add DT binding details for Linear Equal Step Mode

[Laxman Dewangan]

Add support for Linear Equal Step mode  in pwm regulators on which
specified regulator's minimum and maximum voltages are divided into
specified equal steps. The number of steps must divided the range of
minimum to maximum as well as PWM periods in equal parts.

Add DT property to specify number of steps and its details.

Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
Cc: Lee Jones <lee.jones@linaro.org>
---
 .../bindings/regulator/pwm-regulator.txt           | 32 +++++++++++++++++++---
 1 file changed, 28 insertions(+), 4 deletions(-)

diff --git a/Documentation/devicetree/bindings/regulator/pwm-regulator.txt b/Documentation/devicetree/bindings/regulator/pwm-regulator.txt
index ed936f0..b61ff59 100644
--- a/Documentation/devicetree/bindings/regulator/pwm-regulator.txt
+++ b/Documentation/devicetree/bindings/regulator/pwm-regulator.txt
@@ -1,7 +1,7 @@
 Bindings for the Generic PWM Regulator
 ======================================
 
-Currently supports 2 modes of operation:
+Currently supports following modes of operation:
 
 Voltage Table:		When in this mode, a voltage table (See below) of
 			predefined voltage <=> duty-cycle values must be
@@ -23,20 +23,34 @@ Continuous Voltage:	This mode uses the regulator's maximum and minimum
 			regulator voltage to run at half way between the
 			supplied max_uV and min_uV values.
 
+Linear Equal Step:	This modes divides the regulator's maximum and minimum
+			supplied voltages into specified equal steps. The
+			number of steps must equally divides the PWM periods.
+			On this case, step is translated into number of pulse
+			in PWM periods and then multiplied by voltage
+			selectors number to get the duty cycles of PWM.
+
 Required properties:
 --------------------
 - compatible:		Should be "pwm-regulator"
 
 - pwms:			PWM specification (See: ../pwm/pwm.txt)
 
-Only required for Voltage Table Mode:
+Optional properties:
+--------------------
+For Voltage Table Mode, following properties must be supplied:
 - 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 (%)
 
+For Linear Equal Steps, following properties must be supplied:
+- regulator-n-voltages:	Number of voltage steps between regulator's minimum
+			and maximum.
+
 NB: To be clear, if voltage-table is provided, then the device will be used
-in Voltage Table Mode.  If no voltage-table is provided, then the device will
-be used in Continuous Voltage Mode.
+in Voltage Table Mode.  If regulator-n-voltages is provided then the device
+will be used in Linear Equal Step Mode. If both properties are not provided,
+then the device will be used in Continuous Voltage Mode.
 
 Any property defined as part of the core regulator binding can also be used.
 (See: ../regulator/regulator.txt)
@@ -66,3 +80,13 @@ Voltage Table Example:
 				<1036000 40>,
 				<1016000 50>;
 	};
+
+Linear Equal Step Mode Example:
+	pwm-regulator@1 {
+		compatible = "pwm-regulator";
+		pwms = <&pwm 1 4880>;
+		regulator-name = "vdd-gpu";
+		regulator-min-microvolt = <710000>;
+		regulator-max-microvolt = <1320000>;
+		regulator-n-voltages = <62>;
+	};
-- 
2.1.4

Re: [PATCH 5/5] regulator: pwm: Add DT binding details for Linear Equal Step Mode

[Rob Herring]

On Tue, Mar 08, 2016 at 04:23:25PM +0530, Laxman Dewangan wrote:
> Add support for Linear Equal Step mode  in pwm regulators on which
> specified regulator's minimum and maximum voltages are divided into
> specified equal steps. The number of steps must divided the range of
> minimum to maximum as well as PWM periods in equal parts.
> 
> Add DT property to specify number of steps and its details.
> 
> Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
> Cc: Lee Jones <lee.jones@linaro.org>
> ---
>  .../bindings/regulator/pwm-regulator.txt           | 32 +++++++++++++++++++---
>  1 file changed, 28 insertions(+), 4 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/regulator/pwm-regulator.txt b/Documentation/devicetree/bindings/regulator/pwm-regulator.txt
> index ed936f0..b61ff59 100644
> --- a/Documentation/devicetree/bindings/regulator/pwm-regulator.txt
> +++ b/Documentation/devicetree/bindings/regulator/pwm-regulator.txt
> @@ -1,7 +1,7 @@
>  Bindings for the Generic PWM Regulator
>  ======================================
>  
> -Currently supports 2 modes of operation:
> +Currently supports following modes of operation:
>  
>  Voltage Table:		When in this mode, a voltage table (See below) of
>  			predefined voltage <=> duty-cycle values must be
> @@ -23,20 +23,34 @@ Continuous Voltage:	This mode uses the regulator's maximum and minimum
>  			regulator voltage to run at half way between the
>  			supplied max_uV and min_uV values.
>  
> +Linear Equal Step:	This modes divides the regulator's maximum and minimum
> +			supplied voltages into specified equal steps. The
> +			number of steps must equally divides the PWM periods.

s/divides/divide/

> +			On this case, step is translated into number of pulse

In this case...

s/pulse/pulses/

> +			in PWM periods and then multiplied by voltage
> +			selectors number to get the duty cycles of PWM.

This description is not so clear to me. This equates to Vmax-Vmin/steps 
== step voltage?

> +
>  Required properties:
>  --------------------
>  - compatible:		Should be "pwm-regulator"
>  
>  - pwms:			PWM specification (See: ../pwm/pwm.txt)
>  
> -Only required for Voltage Table Mode:
> +Optional properties:
> +--------------------
> +For Voltage Table Mode, following properties must be supplied:
>  - 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 (%)
>  
> +For Linear Equal Steps, following properties must be supplied:
> +- regulator-n-voltages:	Number of voltage steps between regulator's minimum
> +			and maximum.
> +
>  NB: To be clear, if voltage-table is provided, then the device will be used
> -in Voltage Table Mode.  If no voltage-table is provided, then the device will
> -be used in Continuous Voltage Mode.
> +in Voltage Table Mode.  If regulator-n-voltages is provided then the device
> +will be used in Linear Equal Step Mode. If both properties are not provided,

"both properties" provided would be invalid. "If neither property is 
provided" would be a bit more clear.

> +then the device will be used in Continuous Voltage Mode.
>  
>  Any property defined as part of the core regulator binding can also be used.
>  (See: ../regulator/regulator.txt)
> @@ -66,3 +80,13 @@ Voltage Table Example:
>  				<1036000 40>,
>  				<1016000 50>;
>  	};
> +
> +Linear Equal Step Mode Example:
> +	pwm-regulator@1 {

Drop the unit address. No reg property.

> +		compatible = "pwm-regulator";
> +		pwms = <&pwm 1 4880>;
> +		regulator-name = "vdd-gpu";
> +		regulator-min-microvolt = <710000>;
> +		regulator-max-microvolt = <1320000>;
> +		regulator-n-voltages = <62>;
> +	};
> -- 
> 2.1.4
>