[U-Boot] U-Boot on Nokia RX-51 (aka N900)

From: Wolfgang Denk <wd@denx.de>
To: u-boot@lists.denx.de
Subject: [U-Boot] U-Boot on Nokia RX-51 (aka N900)
Date: Sun, 18 Dec 2011 13:42:05 +0100	[thread overview]
Message-ID: <20111218124205.963D9138EF0A@gemini.denx.de> (raw)
In-Reply-To: <17199828.H8TsXDWnq5@pali-elitebook>

Dear Pali =?ISO-8859-1?Q?Roh=E1r?=,

please do not full-quote.  Quote only as much of the mail you are
replying to as is needed to give enough context.  It makes no sense to
quoted hundrets of lines which you are not referring to in your reply.

Thanks.

In message <17199828.H8TsXDWnq5@pali-elitebook> you wrote:
> 
> > WARNING: space prohibited between function name and open parenthesis '('
> > #130: FILE: arch/arm/lib/bootm.c:128:
> > +	s = getenv ("atagaddr");
>
> see file arch/arm/lib/bootm.c - it has this code styling. so this is not error 
> unless you want to mix more code styling in one file.

If you find broken code, then please help fixing it.

Never try to use this as an excuse to add even more broken code.

> > WARNING: consider using kstrto* in preference to simple_strtoul
...
> really? I was not able to find kstrtoul function

False positive.

> > WARNING: space prohibited between function name and open parenthesis '('
...
> > WARNING: please, no spaces at the start of a line
...
> same - see my first comment

See my comment.  All these must be fixed.

> see commit message - this patch is from linux upstream and uf u-boot has same 
> styling this should be ok too.

Linux is not always as strict as we tend to be.

Also, if this code has been copied from Linux, then youi missed to
properly reference it.  See
http://www.denx.de/wiki/view/U-Boot/Patches#Attributing_Code_Copyrights_Sign

> > WARNING: line over 80 characters
> > #117: FILE: common/main.c:1028:
> > +		case '\e':				/* ANSI escape char	*/
>
> again - same formating in file common/main.c

Fix it!

> > WARNING: line over 80 characters
...
> If you want I remove above comments /* ... */

If these comments are considered useful, then they should be kept.

In any case, the line length must not be exceeded.

> > ERROR: do not initialise statics to 0 or NULL
> > #113: FILE: drivers/video/cfb_console.c:382:
> > +static int ansi_colors_need_revert = 0;
>
> what is problem with initialising to 0?

It's redundant, we don't like it, and it triggers a checkpatch
warning :-)

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
When it is incorrect, it is, at least *authoritatively* incorrect.
                                    - Hitchiker's Guide To The Galaxy