[U-Boot] [PATCH] Davinci I2C code cleanup

[ksi at koi8.net]

On Sun, 22 Feb 2009, Wolfgang Denk wrote:

> Dear Jean-Christophe PLAGNIOL-VILLARD,
> 
> In message <20090222124436.GA9867 at game.jcrosoft.org> you wrote:
> > On 13:52 Thu 19 Feb     , ksi at koi8.net wrote:
> > > Removed CHECK_NACK macro from Davinci I2C driver for code cleanup.
> > > 
> > > Signed-off-by: Sergey Kubushyn <ksi at 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 at home    KOI8 Net  < >  The impossible we do immediately.  *
*  Las Vegas   NV, USA   < >  Miracles require 24-hour notice.   *
******************************************************************