regulator: Rename s5m8767_convert_voltage to s5m8767_get_proper_voltage_sel
PATCH 1/1 regulator: Rename s5m8767_convert_voltage to s5m8767_get_proper_voltage_sel
This function finds the smallest voltage that falls within the specified range, and then returns the selector. This rename makes the intention more clear.
Also remove unneeded local variables min_vol and max_vol in s5m8767_set_voltage and s5m8767_set_voltage_buck.
---
I was confused by the function name, s5m8767_convert_voltage() converts what to what?
Axel
drivers/regulator/s5m8767.c | 22 ++++++++++------------ 1 files changed, 10 insertions(+), 12 deletions(-)
drivers/regulator/s5m8767.c +10 -12
@@ -294,11 +294,11 @@ static int s5m8767_get_voltage_sel(struct regulator_dev *rdev)return val; } -static inline int s5m8767_convert_voltage ( +static int s5m8767_get_proper_voltage_sel (const struct s5m_voltage_desc *desc,int min_vol,int max_vol) { -int out_vol =0 ; +int selector =0 ;if (desc ==NULL )return -EINVAL ;
@@ -306,19 +306,18 @@ static inline int s5m8767_convert_voltage(if (max_vol < desc->min || min_vol > desc->max)return -EINVAL ; - out_vol = (min_vol - desc->min) / desc->step; + selector = (min_vol - desc->min) / desc->step; -if (desc->min + desc->step * out_vol > max_vol) +if (desc->min + desc->step * selector > max_vol)return -EINVAL ; -return out_vol; +return selector; }static int s5m8767_set_voltage (struct regulator_dev *rdev,int min_uV,int max_uV,unsigned *selector) {struct s5m8767_info *s5m8767 =rdev_get_drvdata (rdev); -int min_vol = min_uV, max_vol = max_uV;const struct s5m_voltage_desc *desc;int reg_id =rdev_get_id (rdev);int reg, mask, ret;
@@ -343,7 +342,7 @@ static int s5m8767_set_voltage(struct regulator_dev *rdev, desc = reg_voltage_map[reg_id]; - i =s5m8767_convert_voltage (desc, min_vol, max_vol); + i =s5m8767_get_proper_voltage_sel (desc, min_uV, max_uV);if (i <0 )return i;
@@ -385,7 +384,6 @@ static int s5m8767_set_voltage_buck(struct regulator_dev *rdev,int reg_id =rdev_get_id (rdev);const struct s5m_voltage_desc *desc;int new_val, old_val, i =0 ; -int min_vol = min_uV, max_vol = max_uV;if (reg_id <S5M8767_BUCK1 || reg_id >S5M8767_BUCK6 )return -EINVAL ;
@@ -402,7 +400,7 @@ static int s5m8767_set_voltage_buck(struct regulator_dev *rdev, } desc = reg_voltage_map[reg_id]; - new_val =s5m8767_convert_voltage (desc, min_vol, max_vol); + new_val =s5m8767_get_proper_voltage_sel (desc, min_uV, max_uV);if (new_val <0 )return new_val;
@@ -580,7 +578,7 @@ static __devinit int s5m8767_pmic_probe(struct platform_device *pdev)for (i =0 ; i <8 ; i++) {if (s5m8767->buck2_gpiodvs) { s5m8767->buck2_vol[i] = - s5m8767_convert_voltage( + s5m8767_get_proper_voltage_sel( &buck_voltage_val2, pdata->buck2_voltage[i], pdata->buck2_voltage[i] +
@@ -589,7 +587,7 @@ static __devinit int s5m8767_pmic_probe(struct platform_device *pdev)if (s5m8767->buck3_gpiodvs) { s5m8767->buck3_vol[i] = - s5m8767_convert_voltage( + s5m8767_get_proper_voltage_sel( &buck_voltage_val2, pdata->buck3_voltage[i], pdata->buck3_voltage[i] +
@@ -598,7 +596,7 @@ static __devinit int s5m8767_pmic_probe(struct platform_device *pdev)if (s5m8767->buck4_gpiodvs) { s5m8767->buck4_vol[i] = - s5m8767_convert_voltage( + s5m8767_get_proper_voltage_sel( &buck_voltage_val2, pdata->buck4_voltage[i], pdata->buck4_voltage[i] +
Signature · 1 line
1.7.5.4
2012/3/7 Axel Lin <[email protected]>:
Quoted 10 lines
This function finds the smallest voltage that falls within the specified range, and then returns the selector. This rename makes the intention more clear.
Also remove unneeded local variables
min_volandmax_volins5m8767_set_voltageands5m8767_set_voltage_buck.Signed-off-by Axel Lin <[email protected]>---
I was confused by the function name,s5m8767_convert_voltage()converts what to what?
BTW, I'm also confused by s5m8767_set_high and s5m8767_set_low functions. Both function sets the same gpios to the same state. ( by different order ) Maybe we need comments (or rename) for both functions.
Regards,
Axel
Hi,
Quoted 2 lines
Quoted 1 line
I was confused by the function name,
s5m8767_convert_voltage()convertswhat to what?
Convert means that Normal Voltage unit to 8767 internal voltage register unit.
Quoted 2 lines
BTW, I'm also confused by
s5m8767_set_highands5m8767_set_lowfunctions. Both function sets the same gpios to the same state. ( by different order
)
Quoted 1 line
Maybe we need comments (or rename) for both functions.
S5M8767A can be controlled with 3 GPIO.
To control voltage with GPIO, Order is very important. Because Voltage inversion should not be happened. If DVFS table is like a below,
1.5Ghz 1.5V 1.0Ghz 1.0V 0.5Ghz 0.5V
If System try to change 1.0Ghz(1.0V) -> 1.5Ghz(1.5V) Absolutely 0.5V should not be set.
Thanks,
Sangbeom.
Quoted 2 lines · Axel Lin
This function finds the smallest voltage that falls within the specified range, and then returns the selector. This rename makes the intention more clear.
Sangbeom, are you OK with this change?
Hi!
drivers/regulator/s5m8767.c static int s5m8767_get_voltage_sel(struct regulator_dev *rdev) · line 297
Quoted 2 lines · Axel Lin
drivers/regulator/s5m8767.c +1
-1-static inline int s5m8767_convert_voltage( +static int s5m8767_get_proper_voltage_sel(
s5m8767_get_voltage_sel and s5m8767_get_proper_voltage_sel have confusion too.
Except that, Looks OK.
Thanks,
Hi Sangbeom,
2012/3/9 Sangbeom Kim <[email protected]>:
Quoted 8 lines
Hi!
drivers/regulator/s5m8767.c static int s5m8767_get_voltage_sel(struct regulator_dev *rdev) · line 297
Quoted 2 lines · Axel Lin
drivers/regulator/s5m8767.c +1
-1-static inline int s5m8767_convert_voltage( +static int s5m8767_get_proper_voltage_sel(
s5m8767_get_voltage_selands5m8767_get_proper_voltage_selhave confusion too.
How about s5m8767_convert_voltage_to_proper_sel ?
If it is ok for you, I'll send the patch right away. :-)
Regards,
Axel
Hi Axel,
Quoted 1 line · Axel Lin
How about
s5m8767_convert_voltage_to_proper_sel?
It looks too long.
How about s5m8767_convert_voltage_to_sel ?
Thanks,
Sangbeom.