On Sun, 22 Feb 2009, Wolfgang Denk wrote:
Dear Jean-Christophe PLAGNIOL-VILLARD,
In message 20090222124436.GA9867@game.jcrosoft.org you wrote:
On 13:52 Thu 19 Feb , ksi@koi8.net wrote:
Removed CHECK_NACK macro from Davinci I2C driver for code cleanup.
Signed-off-by: Sergey Kubushyn ksi@koi8.net
cpu/arm926ejs/davinci/i2c.c | 62 +++++++++++++++++++++++++--------- 1 files changed, 45 insertions(+), 17 deletions(-)
NACK
please explain why
Please see previous discussion about multibus I2C support.
IMHO duplicate code is really wrong
I agree. The macro should indeed NOT be deleted, but it needs fixing. It is magically accessing the local variable "tmp" which [Quoting the CodingStyle] "...might look like a good thing, but it's confusing as hell when one reads the code and it's prone to breakage from seemingly innocent changes."
I suggest that "tmp" gets passed as argument to the macro.
It is _NOT_ a macro working on some variable. It is simply repeating _LITERAL_ text that not only accesses a local variable tmp but even performs a function return with an error if condition is not met. That is, why, BTW, it should not be made into an inline function because it would require additional checking of its return code where it is used (lot of places in Davinci I2C driver) that, in turn, would've added a lot of unnecessary overhead for local variable, register loads, second conditional checking etc.
It is _NOT_ supposed to be any function-like macro. It is just a fragment of _LITERAL_ text.
I don't think it is a right thing to blindly apply "Coding Style" to everything no matter if it applies to it or not.
I also NACK this :)
--- ****************************************************************** * KSI@home KOI8 Net < > The impossible we do immediately. * * Las Vegas NV, USA < > Miracles require 24-hour notice. * ******************************************************************