Re: [lm-sensors] [PATCH 1/4] kernel.h: add find_closest() macro

[Bartosz Golaszewski <bgolaszewski@baylibre.com>]

2015-02-24 21:51 GMT+01:00 Guenter Roeck <linux@roeck-us.net>:
> I think the lm85 conversion actually introduces a bug with such an
> off-by-one mistake. And if it doesn't, there is still a unexplained
> and not easy to understand '-1' in one of the calls to find_closest().
>
> So the question is if the new code really improves the situation in that
> respect.

Yes, it's a mistake. I couldn't test this one and missed this '-1'.
Sorry for that.

As for the size comparisons - at first glance it seems as if it adds bloat:

ina2xx:
add/remove: 0/0 grow/shrink: 1/0 up/down: 24/0 (24)
function                                     old     new   delta
ina226_set_interval                          296     320     +24

lm85:
add/remove: 0/0 grow/shrink: 3/0 up/down: 72/0 (72)
function                                     old     new   delta
set_temp_auto_temp_min                       364     388     +24
set_temp_auto_temp_max                       336     360     +24
set_pwm_freq                                 284     308     +24

w83795:
add/remove: 0/0 grow/shrink: 1/0 up/down: 16/0 (16)
function                                     old     new   delta
store_pwm                                    496     512     +16

But this actually comes from DIV_ROUND_CLOSEST() since replacing it
with a simple '/ 2' gives different results:

ina2xx.ko:
add/remove: 0/0 grow/shrink: 1/0 up/down: 6/0 (6)
function                                     old     new   delta
__UNIQUE_ID_vermagic0                         73      79      +6

lm85.ko:
add/remove: 0/0 grow/shrink: 1/0 up/down: 6/0 (6)
function                                     old     new   delta
__UNIQUE_ID_vermagic0                         73      79      +6

w83795.ko:
add/remove: 0/0 grow/shrink: 2/0 up/down: 14/0 (14)
function                                     old     new   delta
store_pwm                                    496     504      +8
__UNIQUE_ID_vermagic0                         73      79      +6

The idea however was to remove duplicated operations from source files
and prevent off-by-one bugs (how ironic the lm85 patch...), not really
to reduce size.

Bart