[lm-sensors] [PATCH] i2c: add support for MAX1618 in MAX1619 driver
khali at linux-fr.org
Sun Sep 28 18:02:40 CEST 2008
On Wed, 17 Sep 2008 15:08:58 -0700, Andrew Morton wrote:
> On Tue, 16 Sep 2008 15:35:42 +0200
> Sebastian Siewior <bigeasy at linutronix.de> wrote:
> > + return sprintf(buf, "%d\n", TEMP_FROM_REG((data->kind == max1618) ?
> > + data->remote : data->local));
> oh dear.
> #define TEMP_FROM_REG(val) ((val & 0x80 ? val-0x100 : val) * 1000)
> Imagine how truly awful the code generation must be here.
> --- a/drivers/hwmon/max1619.c~a
> +++ a/drivers/hwmon/max1619.c
> @@ -75,8 +75,15 @@ I2C_CLIENT_INSMOD_2(max1619, max1618);
> * Conversions and various macros
> -#define TEMP_FROM_REG(val) ((val & 0x80 ? val-0x100 : val) * 1000)
> -#define TEMP_TO_REG(val) ((val < 0 ? val+0x100*1000 : val) / 1000)
> +static int TEMP_FROM_REG(int val)
> + return ((val & 0x80 ? val-0x100 : val) * 1000);
> +static int TEMP_TO_REG(int val)
> + return (val < 0 ? val+0x100*1000 : val) / 1000;
> * Functions declaration
> text data bss dec hex filename
> before: 3927 1148 28 5103 13ef drivers/hwmon/max1619.o
> after: 3743 1148 28 4919 1337 drivers/hwmon/max1619.o
> That's a 6% reduction in the number of instructions in the whole driver!
> Not only that, it generates nicer-to-read code and it fixes the bugs
> which will occur if someone calls one of these macros with an
> expression which has side-effects.
> Macros suck, suck, suck, suck and the kernel is just littered with the
> stupid things in places where they were completely unnecessary.
I totally support this change. We are trying to get rid of these macros
in all hwmon drivers but there are still a number drivers that had not
been cleaned up. One down, thanks.
I'm taking this patch in my hwmon tree. I've added a proper summary and
fixed checkpatch warnings.
More information about the lm-sensors