> Values in OFFSET_[XYZ] needs to be 5-bits two's complement. Fix the
> macro to handle negative values properly and apply the proper mask to
> prevent overflow.

Sounds good.
But:

> -#define OFFSET_X(x)     (x)
> +#define OFFSET_X(x)     (((x) >= 0) ? ((x) & 0x0f) : (((-(x)-1) & 0x0f) ^ 0x1f))

Shouldn't

#define OFFSET_X(x)     ((x) & 0x1f)		/* 5 bit signed two's complement -16..15 */

be enough? Overflow cannot be handled at this level (the offset would be
wrong anyway). C doesn't state how negative integer values are handled
(actually it does, but in one of three ways: sign+magnitude, two's
complement, and one's complement), but I don't think this driver will ever
work on architectures that do not use two's complement as native form.

I know this is also a question of style, and the compiler will probably
optimize your code to the same instructions. I don't the right answer.

Matthias

-- 
Matthias Hopf <mhopf@suse.de>      __        __   __
Maxfeldstr. 5 / 90409 Nuernberg   (_   | |  (_   |__          mat@mshopf.de
Phone +49-911-74053-715           __)  |_|  __)  |__  R & D   www.mshopf.de
-- 
To unsubscribe, e-mail: radeonhd+unsubscribe@opensuse.org
For additional commands, e-mail: radeonhd+help@opensuse.org