[U-Boot] [PATCH] arm926ejs: add NXP LPC32x0 cpu series support

[Wolfgang Denk]

Dear Vladimir Zapolskiy,

In message <4E9B1B16.4030407 at mleia.com> you wrote:
> 
> > Do you need interrupts to get udelay() working?
> >
> Good point, potentially udelay() can be implemented reading values from
> Timer Counter register. I'll refine this part.

s/can/must/

> >> +#define SBF(s, v) (((unsigned long)(v))<< (s))
> >> +#define BIT(b) SBF(b, 0x01)
> >
> > Those names are not really self-explanatory.
> BIT is a one-bit field, and SBF stands for shifted bit field.
>
> These macro are quite commonly used in the Linux kernel, so I had a hope
> that it could be appropriate to have them in U-boot.

Sorry, but we don't accept such macros.

> > Besides, I don't think a macro is justified for just shifting values --
> > I prefer the shift to be explicit, or even to be precomputed.
> >
> In my humble opinion two functions of macro definitions are to save code
> space in bytes and to improve code readability, and both of them there
> are fulfilled.

No.

The code size actually remains the same. We're talking about the size
of the compiled code, aren't we?

Regarding readibility: This is, at least to some extend, a matter of
taste.  In our opinion these macros are bad, last but not least
because they hide what the code is doing, and because they are
inherently unportable.

> For driver developers or additional functionality writers there are 
> provided human-readable macro, and these two macro are merely defined
> for supplementary purposes. Maybe I'd be happy to find them defined in
> include/common.h one day, but for a moment I would like to define and 
> expoit them in the code, which I'm going to maintain.

No chance.  Please understand that such macros simply don't work well
across architectures.

> > Isn't this a 16550? If so, you should ise the existing 16550 driver.
> >
> According to the SoC specification LPC32xx has 4 standard UARTs, which
> are compatible with INS16Cx50 and in the Linux kernel it is mentioned
> that these UARTs are compatible with standard 8250/16550.
>
>  From my side I've already prepared a simple LPC32xx serial driver
> realization, but I'll review it from the perspective of your hint and
> probably some parts related to UART can be removed in the next patch
> version.

Actually the whole custom driver should be removed then.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
In the pitiful, multipage, connection-boxed form to which  the  flow-
chart  has  today  been  elaborated, it has proved to be useless as a
design tool -- programmers draw flowcharts after, not before, writing
the programs they describe.                        - Fred Brooks, Jr.