[U-Boot-Users] [PATCH] Add S3C2410 MMC/SD support

[Wolfgang Denk]

In message <20070217111634.GC11991 at prithivi.gnumonks.org> you wrote:
>
> > C++ style comment; bad!  :-)
> 
> well, I was a long-time proponent of this, too.  But it's now officially
> part of the C99 standard.  So there's not really much [technical]
> argument you can still make about this :(

Not technical, but stylish. It's forbidden in U-Boot code, so  please
fix it.

> > Seems a little verbose; what about something like this?
> > for ( ; fifo && len > 0; fifo--, len += 4)
> >    *(dst_u32++) = sdi->SDIDAT;
> 
> I guess this si a question of taste/preferecne.  I personally think the
> explicit variant (minus the //indented DEBUGP statement) is more
> readable than a for loop with no initial statement and ',' style
> multiple commands in each loop.

IMO the shorter version is much easier to read, so please change this,
too.

> > Can/should this ever time out?
> 
> probably yes, if you do something strange to the hardware...
> 
> > Should (1 << 4) be a #defined constant?
> 
> probably, too.

Then please change these, too.

> > This long debug statement appears a lot; should it be a macro?
> 
> that's (like much of the stuff) just a copy of something that is already
> in u-boot.  So one of the things while adopting it from pxa to s3c2410
> was: keep it as close as possible to the original.  It seems like that
> one was a bad example ;)

If you decide to submit a second patch to also clean up the pxa  code
this  is  just  the  more appreciated. If you see ways to improve the
code, please always do. Do not follow bad examples just because there
are so many of them around.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH, HRB 165235 Munich, CEO: Wolfgang Denk
Office:  Kirchenstr. 5,       D-82194 Groebenzell,            Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
They're usually so busy thinking about what  happens  next  that  the
only  time they ever find out what is happening now is when they come
to look back on it.                 - Terry Pratchett, _Wyrd Sisters_