From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753322AbbBYK7x (ORCPT ); Wed, 25 Feb 2015 05:59:53 -0500 Received: from mail-ig0-f182.google.com ([209.85.213.182]:37755 "EHLO mail-ig0-f182.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753041AbbBYK7v (ORCPT ); Wed, 25 Feb 2015 05:59:51 -0500 MIME-Version: 1.0 In-Reply-To: <20150224205125.GB18025@roeck-us.net> References: <1424799734-2170-1-git-send-email-bgolaszewski@baylibre.com> <1424799734-2170-2-git-send-email-bgolaszewski@baylibre.com> <20150224205125.GB18025@roeck-us.net> Date: Wed, 25 Feb 2015 11:59:51 +0100 Message-ID: Subject: Re: [lm-sensors] [PATCH 1/4] kernel.h: add find_closest() macro From: Bartosz Golaszewski To: Guenter Roeck Cc: Phil Pokorny , LKML , lm-sensors Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 2015-02-24 21:51 GMT+01:00 Guenter Roeck : > 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