regulator: MAX8998: set_voltage bugfix. ramp_up delay and min/max voltage
Subsystems (2)
PATCH 1/1 regulator: MAX8998: set_voltage bugfix. ramp_up delay and min/max voltage
Two issues are addressed for max8998_set_voltage function.
1. Min/Max Voltage.
max8998_set_voltage had been using the voltage value of
min ( voltage[i] >= max_vol , i )
This is corrected to use:
min ( voltage[i] >= min_vol , i )
2. Ramp Up Delay.
max8998_set_voltage should provide delay for BUCK1/2
if ENRAMP is on. It reads RAMP value from ONOFF4 register to determine RAMP delay length. However, when max8998_set_voltage's new voltage is lower than the previous, we don't care because it does not deteriorate the stability.
--
I've omitted some CC in the previous email, adding them here. Sorry for the duplicated submission.
--- drivers/regulator/max8998.c | 30 ++++++++++++++++++++++++++++-- 1 files changed, 28 insertions(+), 2 deletions(-)
drivers/regulator/max8998.c +28 -2
@@ -28,6 +28,7 @@#include <linux/slab.h> #include <linux/interrupt.h> #include <linux/mutex.h> +#include <linux/delay.h> #include <linux/platform_device.h> #include <linux/regulator/driver.h> #include <linux/mfd/max8998.h>
@@ -302,10 +303,13 @@ static int max8998_set_voltage(struct regulator_dev *rdev,struct max8998_data *max8998 =rdev_get_drvdata (rdev);struct i2c_client *i2c = max8998->iodev->i2c;int min_vol = min_uV /1000 , max_vol = max_uV /1000 ; +int previous_vol =0 ;const struct voltage_map_desc *desc;int ldo =max8998_get_ldo (rdev);int reg, shift =0 , mask, ret;int i =0 ; +u8 val; +bool en_ramp = false;if (ldo >ARRAY_SIZE (ldo_voltage_map))return -EINVAL ;
@@ -317,15 +321,37 @@ static int max8998_set_voltage(struct regulator_dev *rdev,if (max_vol < desc->min || min_vol > desc->max)return -EINVAL ; -while (desc->min + desc->step*i < max_vol && +while (desc->min + desc->step*i < min_vol && desc->min + desc->step*i < desc->max) i++; +if (desc->min + desc->step*i > max_vol) +return -EINVAL ; + ret =max8998_get_voltage_register (rdev, ®, &shift, &mask);if (ret)return ret; -return max8998_update_reg (i2c, reg, i<<shift, mask<<shift); +/* wait for RAMP_UP_DELAY if rdev is BUCK1/2 and +* ENRAMP is ON */ +if (ldo ==MAX8998_BUCK1 || ldo ==MAX8998_BUCK2 ) { +max8998_read_reg (i2c,MAX8998_REG_ONOFF4 , &val); +if (val & (1 <<4 )) { + en_ramp = true; + previous_vol =max8998_get_voltage (rdev); + } + } + + ret =max8998_update_reg (i2c, reg, i<<shift, mask<<shift); + +if (en_ramp == true) { +int difference = desc->min + desc->step * i + - previous_vol /1000 ; +if (difference >0 ) +udelay (difference / ((val &0x0f ) +1 )); + } + +return ret; }static struct regulator_ops max8998_ldo_ops = {
Signature · 1 line
1.6.3.3
PATCH 1/1 regulator: MAX8998: set_voltage bugfix. ramp_up delay and min/max voltage
Quoted 1 line · MyungJoo Ham
Two issues are addressed for
max8998_set_voltagefunction.
Ideally you should've submitted this as two separate patches.
PATCH 1/1 regulator: MAX8998: set_voltage bugfix. ramp_up delay and min/max voltage
In reply to MyungJoo HamHello,
Quoted 14 lines · MyungJoo Ham
Two issues are addressed for
max8998_set_voltagefunction.1. Min/Max Voltage.
max8998_set_voltagehad been using the voltage value ofmin ( voltage[i] >=
max_vol, i )This is corrected to use:
min ( voltage[i] >=
min_vol, i )2. Ramp Up Delay.
Quoted 8 lines
max8998_set_voltageshould provide delay for BUCK1/2
if ENRAMP is on. It reads RAMP value from ONOFF4 register to determine RAMP delay length. However, when max8998_set_voltage's new voltage is lower than the previous, we don't care because it does not deteriorate the stability.Signed-off-by MyungJoo Ham <[email protected]>Signed-off-by Kyungmin Park <[email protected]>
Quoted 74 lines
--
I've omitted some CC in the previous email, adding them here. Sorry for the duplicated submission.--- drivers/regulator/max8998.c | 30 ++++++++++++++++++++++++++++-- 1 files changed, 28 insertions(+), 2 deletions(-)drivers/regulator/max8998.c +4
-0@@ -28,6 +28,7 @@#include <linux/slab.h> #include <linux/interrupt.h> #include <linux/mutex.h> +#include <linux/delay.h> #include <linux/platform_device.h> #include <linux/regulator/driver.h> #include <linux/mfd/max8998.h> @@ -302,10 +303,13 @@ static int max8998_set_voltage(struct regulator_dev *rdev, struct max8998_data *max8998 = rdev_get_drvdata(rdev);struct i2c_client *i2c = max8998->iodev->i2c;int min_vol = min_uV /1000 , max_vol = max_uV /1000 ; +int previous_vol =0 ;const struct voltage_map_desc *desc;int ldo =max8998_get_ldo (rdev);int reg, shift =0 , mask, ret;int i =0 ; +u8 val; +bool en_ramp = false;if (ldo >ARRAY_SIZE (ldo_voltage_map))drivers/regulator/max8998.c +24
-2return -EINVAL ;@@ -317,15 +321,37 @@ static int max8998_set_voltage(struct regulator_dev *rdev, if (max_vol < desc->min || min_vol > desc->max) return -EINVAL; -while (desc->min + desc->step*i < max_vol && +while (desc->min + desc->step*i < min_vol && desc->min + desc->step*i < desc->max) i++; +if (desc->min + desc->step*i > max_vol) +return -EINVAL ; + ret =max8998_get_voltage_register (rdev, ®, &shift, &mask);if (ret)return ret; -return max8998_update_reg (i2c, reg, i<<shift, mask<<shift); +/* wait for RAMP_UP_DELAY if rdev is BUCK1/2 and +* ENRAMP is ON */ +if (ldo ==MAX8998_BUCK1 || ldo ==MAX8998_BUCK2 ) { +max8998_read_reg (i2c,MAX8998_REG_ONOFF4 , &val); +if (val & (1 <<4 )) { + en_ramp = true; + previous_vol =max8998_get_voltage (rdev); + } + } + + ret =max8998_update_reg (i2c, reg, i<<shift, mask<<shift); + +if (en_ramp == true) { +int difference = desc->min + desc->step * i + - previous_vol /1000 ; +if (difference >0 ) +udelay (difference / ((val &0x0f ) +1 )); + } + +return ret; }
Quoted 3 lines
static struct
regulator_opsmax8998_ldo_ops= {
--
1.6.3.3
Best regards
--
Marek Szyprowski
Samsung Poland R&D Center
PATCH 1/1 regulator: MAX8998: set_voltage bugfix. ramp_up delay and min/max voltage
In reply to MyungJoo HamQuoted 20 lines · MyungJoo Ham
Two issues are addressed for
max8998_set_voltagefunction.1. Min/Max Voltage.
max8998_set_voltagehad been using the voltage value ofmin ( voltage[i] >=
max_vol, i )This is corrected to use:
min ( voltage[i] >=
min_vol, i )2. Ramp Up Delay.
max8998_set_voltageshould provide delay for BUCK1/2
if ENRAMP is on. It reads RAMP value from ONOFF4 register to determine RAMP delay length. However, when max8998_set_voltage's new voltage is lower than the previous, we don't care because it does not deteriorate the stability.
Quoted 4 lines
Signed-off-by MyungJoo Ham <[email protected]>Signed-off-by Kyungmin Park <[email protected]>--
I've omitted some CC in the previous email, adding them here. Sorry for the duplicated submission.
This doesn't apply. Can you re-create against my regulator for-next branch.
git://git.kernel.org/pub/scm/linux/kernel/git/lrg/voltage-2.6.git for-next
Thanks
Liam
Signature · 3 lines
Freelance Developer, SlimLogic Ltd ASoC and Voltage Regulator Maintainer. http://www.slimlogic.co.uk
PATCH v2 1/1 regulator: MAX8998: set_voltage bugfix. ramp_up delay and min/max voltage
From: MyungJoo Ham <[email protected]>
Two issues are addressed for max8998_set_voltage function.
1. Min/Max Voltage.
max8998_set_voltage had been using the voltage value of
min ( voltage[i] >= max_vol , i )
This is corrected to use:
min ( voltage[i] >= min_vol , i )
2. Ramp Up Delay.
max8998_set_voltage should provide delay for BUCK1/2
if ENRAMP is on. It reads RAMP value from ONOFF4 register to determine RAMP delay length. However, when max8998_set_voltage's new voltage is lower than the previous, we don't care because it does not deteriorate the stability.
Changes since v1:
- rebased onto latest regulator-for-next tree
--- drivers/regulator/max8998.c | 29 +++++++++++++++++++++++++++-- 1 files changed, 27 insertions(+), 2 deletions(-)
drivers/regulator/max8998.c +27 -2
@@ -28,6 +28,7 @@#include <linux/slab.h> #include <linux/interrupt.h> #include <linux/mutex.h> +#include <linux/delay.h> #include <linux/platform_device.h> #include <linux/regulator/driver.h> #include <linux/mfd/max8998.h>
@@ -297,10 +298,13 @@ static int max8998_set_voltage(struct regulator_dev *rdev, {struct max8998_data *max8998 =rdev_get_drvdata (rdev);int min_vol = min_uV /1000 , max_vol = max_uV /1000 ; +int previous_vol =0 ;const struct voltage_map_desc *desc;int ldo =max8998_get_ldo (rdev);int reg, shift =0 , mask, ret;int i =0 ; +u8 val; +bool en_ramp = false;if (ldo >ARRAY_SIZE (ldo_voltage_map))return -EINVAL ;
@@ -312,15 +316,36 @@ static int max8998_set_voltage(struct regulator_dev *rdev,if (max_vol < desc->min || min_vol > desc->max)return -EINVAL ; -while (desc->min + desc->step*i < max_vol && +while (desc->min + desc->step*i < min_vol && desc->min + desc->step*i < desc->max) i++; +if (desc->min + desc->step*i > max_vol) +return -EINVAL ; + ret =max8998_get_voltage_register (rdev, ®, &shift, &mask);if (ret)return ret; -return max8998_update_reg (max8998->iodev, reg, i<<shift, mask<<shift); +/* wait for RAMP_UP_DELAY if rdev is BUCK1/2 and +* ENRAMP is ON */ +if (ldo ==MAX8998_BUCK1 || ldo ==MAX8998_BUCK2 ) { +max8998_read_reg (max8998->iodev,MAX8998_REG_ONOFF4 , &val); +if (val & (1 <<4 )) { + en_ramp = true; + previous_vol =max8998_get_voltage (rdev); + } + } + + ret =max8998_update_reg (max8998->iodev, reg, i<<shift, mask<<shift); + +if (en_ramp == true) { +int difference = desc->min + desc->step*i - previous_vol/1000 ; +if (difference >0 ) +udelay (difference / ((val &0x0f ) +1 )); + } + +return ret; }static struct regulator_ops max8998_ldo_ops = {
Signature · 1 line
1.7.1.569.g6f426
PATCH v2 1/1 regulator: MAX8998: set_voltage bugfix. ramp_up delay and min/max voltage
Quoted 3 lines · Marek Szyprowski
From: MyungJoo Ham <[email protected]>
Two issues are addressed for
max8998_set_voltagefunction.
but as IIRC I said last time it would be nicer to fix one issue per patch.
Liam Girdwood
AppliedPATCH v2 1/1 regulator: MAX8998: set_voltage bugfix. ramp_up delay and min/max voltage
In reply to Marek SzyprowskiQuoted 16 lines · Marek Szyprowski
From: MyungJoo Ham <[email protected]>
Two issues are addressed for
max8998_set_voltagefunction.1. Min/Max Voltage.
max8998_set_voltagehad been using the voltage value ofmin ( voltage[i] >=
max_vol, i )This is corrected to use:
min ( voltage[i] >=
min_vol, i )2. Ramp Up Delay.
Quoted 8 lines
max8998_set_voltageshould provide delay for BUCK1/2
if ENRAMP is on. It reads RAMP value from ONOFF4 register to determine RAMP delay length. However, when max8998_set_voltage's new voltage is lower than the previous, we don't care because it does not deteriorate the stability.Changes since v1:
- rebased onto latest regulator-for-next tree
Applied. Btw, please run checkpatch and/or cleanpatch before submitting next time as this patch contained some trailing white space.
Thanks
Liam
Signature · 3 lines
Freelance Developer, SlimLogic Ltd ASoC and Voltage Regulator Maintainer. http://www.slimlogic.co.uk