[PATCH] regulator: pwm: Try to avoid voltage error in duty cycle calculation

[PATCH] regulator: pwm: Try to avoid voltage error in duty cycle calculation

[Laxman Dewangan]

In continuous mode of the PWM regulators, the requested voltage
PWM duty cycle is calculated in terms of 100% scale where entire
range denotes 100%. The calculation for PWM pulse ON time(duty_pulse)
is done as:

	duty_cycle = ((requested - minimum) * 100) / voltage_range.

 duty pulse is calculated as
	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: Consider 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.

	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.

Proposing the solution in which if requested voltage makes the accurate
duty pulse then there will not be any error. On this case, if
(req_uV - min_uV) * pwm_period is perfect dividable by voltage_range
then get the duty pulse time directly.

	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,

Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
Cc: Lee Jones <lee.jones@linaro.org>
---
This is the rework on patch based on discussion in patch
Re: [PATCH 4/5] regulator: pwm: Add support for voltage linear equal steps

In the older patch, instead of creating new property for linear steps,
based on pwm period and ranges, calculate the duty cycle without any
voltage calcualtion loss.

 drivers/regulator/pwm-regulator.c | 27 ++++++++++++---------------
 1 file changed, 12 insertions(+), 15 deletions(-)

diff --git a/drivers/regulator/pwm-regulator.c b/drivers/regulator/pwm-regulator.c
index f99a697..8d980701 100644
--- a/drivers/regulator/pwm-regulator.c
+++ b/drivers/regulator/pwm-regulator.c
@@ -113,18 +113,6 @@ static int pwm_regulator_is_enabled(struct regulator_dev *dev)
 	return pwm_is_enabled(drvdata->pwm);
 }
 
-/**
- * Continuous voltage call-backs
- */
-static int pwm_voltage_to_duty_cycle_percentage(struct regulator_dev *rdev, int req_uV)
-{
-	int min_uV = rdev->constraints->min_uV;
-	int max_uV = rdev->constraints->max_uV;
-	int diff = max_uV - min_uV;
-
-	return ((req_uV * 100) - (min_uV * 100)) / diff;
-}
-
 static int pwm_regulator_get_voltage(struct regulator_dev *rdev)
 {
 	struct pwm_regulator_data *drvdata = rdev_get_drvdata(rdev);
@@ -139,12 +127,21 @@ static int pwm_regulator_set_voltage(struct regulator_dev *rdev,
 	struct pwm_regulator_data *drvdata = rdev_get_drvdata(rdev);
 	unsigned int ramp_delay = rdev->constraints->ramp_delay;
 	unsigned int period = pwm_get_period(drvdata->pwm);
-	int duty_cycle;
+	unsigned int req_diff = min_uV - rdev->constraints->min_uV;
+	unsigned int diff;
+	unsigned int duty_pulse;
+	u64 req_period;
 	int ret;
 
-	duty_cycle = pwm_voltage_to_duty_cycle_percentage(rdev, min_uV);
+	diff = rdev->constraints->max_uV - rdev->constraints->min_uV;
+	req_period = req_diff * period;
+
+	if (req_period % diff == 0)
+		duty_pulse = req_period / diff;
+	else
+		duty_pulse = (period / 100) * ((req_diff * 100) / diff);
 
-	ret = pwm_config(drvdata->pwm, (period / 100) * duty_cycle, period);
+	ret = pwm_config(drvdata->pwm, duty_pulse, period);
 	if (ret) {
 		dev_err(&rdev->dev, "Failed to configure PWM: %d\n", ret);
 		return ret;
-- 
2.1.4

Re: [PATCH] regulator: pwm: Try to avoid voltage error in duty cycle calculation

[kbuild test robot]

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

Hi Laxman,

[auto build test ERROR on regulator/for-next]
[also build test ERROR on v4.6-rc1 next-20160401]
[if your patch is applied to the wrong git tree, please drop us a note to help improving the system]

url:    https://github.com/0day-ci/linux/commits/Laxman-Dewangan/regulator-pwm-Try-to-avoid-voltage-error-in-duty-cycle-calculation/20160331-220703
base:   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git for-next
config: arm-multi_v7_defconfig (attached as .config)
reproduce:
        wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=arm 

All errors (new ones prefixed by >>):

>> ERROR: "__aeabi_uldivmod" [drivers/regulator/pwm-regulator.ko] undefined!

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 37213 bytes --]

Re: [PATCH] regulator: pwm: Try to avoid voltage error in duty cycle calculation

[kbuild test robot]

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

Hi Laxman,

[auto build test ERROR on regulator/for-next]
[also build test ERROR on v4.6-rc1 next-20160401]
[if your patch is applied to the wrong git tree, please drop us a note to help improving the system]

url:    https://github.com/0day-ci/linux/commits/Laxman-Dewangan/regulator-pwm-Try-to-avoid-voltage-error-in-duty-cycle-calculation/20160331-220703
base:   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git for-next
config: i386-randconfig-a0-04010940 (attached as .config)
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All errors (new ones prefixed by >>):

   drivers/built-in.o: In function `pwm_regulator_set_voltage':
>> pwm-regulator.c:(.text+0x1728d4): undefined reference to `__umoddi3'
>> pwm-regulator.c:(.text+0x1728fd): undefined reference to `__udivdi3'

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 22214 bytes --]

Re: [PATCH] regulator: pwm: Try to avoid voltage error in duty cycle calculation

[Laxman Dewangan]

On Friday 01 April 2016 11:55 AM, kbuild test robot wrote:
> Hi Laxman,
>
> [auto build test ERROR on regulator/for-next]
> [also build test ERROR on v4.6-rc1 next-20160401]
> [if your patch is applied to the wrong git tree, please drop us a note to help improving the system]
>
> url:    https://github.com/0day-ci/linux/commits/Laxman-Dewangan/regulator-pwm-Try-to-avoid-voltage-error-in-duty-cycle-calculation/20160331-220703
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git for-next
> config: i386-randconfig-a0-04010940 (attached as .config)
> reproduce:
>          # save the attached .config to linux build tree
>          make ARCH=i386
>
> All errors (new ones prefixed by >>):
>
>     drivers/built-in.o: In function `pwm_regulator_set_voltage':
>>> pwm-regulator.c:(.text+0x1728d4): undefined reference to `__umoddi3'
>>> pwm-regulator.c:(.text+0x1728fd): undefined reference to `__udivdi3'
>


Seems build error is coming from logic

u64 req_period
unsigned int diff;
unsigned int period
unsigned int req_diff


req_period = req_diff * period;
if (req_period % diff == 0)


used u64 to avoid overflow in multiplication.

Re: [PATCH] regulator: pwm: Try to avoid voltage error in duty cycle calculation

[Mark Brown]

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

On Fri, Apr 01, 2016 at 12:02:19PM +0530, Laxman Dewangan wrote:
> On Friday 01 April 2016 11:55 AM, kbuild test robot wrote:

> >>>pwm-regulator.c:(.text+0x1728d4): undefined reference to `__umoddi3'
> >>>pwm-regulator.c:(.text+0x1728fd): undefined reference to `__udivdi3'

> Seems build error is coming from logic

> u64 req_period
> unsigned int diff;
> unsigned int period
> unsigned int req_diff

> req_period = req_diff * period;
> if (req_period % diff == 0)

> used u64 to avoid overflow in multiplication.

Yes, you're doing a 64 bit divide so that's kind of expected...  

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

Re: [PATCH] regulator: pwm: Try to avoid voltage error in duty cycle calculation

[Laxman Dewangan]

On Saturday 02 April 2016 10:23 PM, Mark Brown wrote:
> * PGP Signed by an unknown key
>
> On Fri, Apr 01, 2016 at 12:02:19PM +0530, Laxman Dewangan wrote:
>> On Friday 01 April 2016 11:55 AM, kbuild test robot wrote:
>>>>> pwm-regulator.c:(.text+0x1728d4): undefined reference to `__umoddi3'
>>>>> pwm-regulator.c:(.text+0x1728fd): undefined reference to `__udivdi3'
>> Seems build error is coming from logic
>> u64 req_period
>> unsigned int diff;
>> unsigned int period
>> unsigned int req_diff
>> req_period = req_diff * period;
>> if (req_period % diff == 0)
>> used u64 to avoid overflow in multiplication.
> Yes, you're doing a 64 bit divide so that's kind of expected...
>

Is there any way to resolve this issue?

Otherwise, need to divide by 100 first for the req_diff to get 10x of mV 
(instead of uV) and then do calculation with unsigned int only. That way 
I can restrict the number to unsigned int.

Re: [PATCH] regulator: pwm: Try to avoid voltage error in duty cycle calculation

[Mark Brown]

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

On Sun, Apr 03, 2016 at 04:37:18PM +0530, Laxman Dewangan wrote:
> On Saturday 02 April 2016 10:23 PM, Mark Brown wrote:

> >Yes, you're doing a 64 bit divide so that's kind of expected...

> Is there any way to resolve this issue?

You need to reorganize the maths so you're not doing a 64 bit divide
operation or use do_div().

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