On Wed, 9 Jan 2013 08:59:07 -0800, Guenter Roeck wrote:
> SENSORS_LIMIT and the generic clamp_val have the same functionality,
> and clamp_val is more efficient.
>
> This patch reduces text size by 9052 bytes and bss size by 11624 bytes
> for x86_64 builds.
Wow, I like it a lot! Well done!
>
> Signed-off-by: Guenter Roeck <linux@xxxxxxxxxxxx>
Acked-by: Jean Delvare <khali@xxxxxxxxxxxx>
Just one thing:
> (...)
> diff --git a/drivers/hwmon/lm93.c b/drivers/hwmon/lm93.c
> index 1a003f7..81449a2 100644
> --- a/drivers/hwmon/lm93.c
> +++ b/drivers/hwmon/lm93.c
> @@ -371,22 +371,22 @@ static unsigned LM93_IN_FROM_REG(int nr, u8 reg)
> static u8 LM93_IN_TO_REG(int nr, unsigned val)
> {
> /* range limit */
> - const long mV = SENSORS_LIMIT(val,
> - lm93_vin_val_min[nr], lm93_vin_val_max[nr]);
> + const long mv = clamp_val(val,
> + lm93_vin_val_min[nr], lm93_vin_val_max[nr]);
>
> /* try not to lose too much precision here */
> - const long uV = mV * 1000;
> - const long uV_max = lm93_vin_val_max[nr] * 1000;
> - const long uV_min = lm93_vin_val_min[nr] * 1000;
> + const long uv = mv * 1000;
> + const long uv_max = lm93_vin_val_max[nr] * 1000;
> + const long uv_min = lm93_vin_val_min[nr] * 1000;
>
> /* convert */
> - const long slope = (uV_max - uV_min) /
> + const long slope = (uv_max - uv_min) /
> (lm93_vin_reg_max[nr] - lm93_vin_reg_min[nr]);
> - const long intercept = uV_min - slope * lm93_vin_reg_min[nr];
> + const long intercept = uv_min - slope * lm93_vin_reg_min[nr];
>
> - u8 result = ((uV - intercept + (slope/2)) / slope);
> - result = SENSORS_LIMIT(result,
> - lm93_vin_reg_min[nr], lm93_vin_reg_max[nr]);
> + u8 result = ((uv - intercept + (slope/2)) / slope);
All these case changes in variable name seem a little off-topic here,
especially for a subsystem-wide patch.
> + result = clamp_val(result,
> + lm93_vin_reg_min[nr], lm93_vin_reg_max[nr]);
> return result;
> }
>
> @@ -409,13 +409,13 @@ static unsigned LM93_IN_REL_FROM_REG(u8 reg, int upper, int vid)
> */
> static u8 LM93_IN_REL_TO_REG(unsigned val, int upper, int vid)
> {
> - long uV_offset = vid * 1000 - val * 10000;
> + long uv_offset = vid * 1000 - val * 10000;
> if (upper) {
> - uV_offset = SENSORS_LIMIT(uV_offset, 12500, 200000);
> - return (u8)((uV_offset / 12500 - 1) << 4);
> + uv_offset = clamp_val(uv_offset, 12500, 200000);
> + return (u8)((uv_offset / 12500 - 1) << 4);
> } else {
> - uV_offset = SENSORS_LIMIT(uV_offset, -400000, -25000);
> - return (u8)((uV_offset / -25000 - 1) << 0);
> + uv_offset = clamp_val(uv_offset, -400000, -25000);
> + return (u8)((uv_offset / -25000 - 1) << 0);
Same here.
> }
> }
> (...)
> @@ -2052,9 +2052,9 @@ static ssize_t store_pwm_auto_channels(struct device *dev,
> return err;
>
> mutex_lock(&data->update_lock);
> - data->block9[nr][LM93_PWM_CTL1] = SENSORS_LIMIT(val, 0, 255);
> + data->block9[nr][LM93_PWM_CTL1] = clamp_val(val, 0, 255);
> lm93_write_byte(client, LM93_REG_PWM_CTL(nr, LM93_PWM_CTL1),
> - data->block9[nr][LM93_PWM_CTL1]);
> + data->block9[nr][LM93_PWM_CTL1]);
This one, while correct, also doesn't belong here IMHO.
> mutex_unlock(&data->update_lock);
> return count;
> }
--
Jean Delvare
_______________________________________________
lm-sensors mailing list
lm-sensors@xxxxxxxxxxxxxx
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
