Skip to contentSkip to conversation

i.MX support for scb9328 cleaned up

U-Boot-Users Konstantin Kletschke 3 participants 13 messages May 12, 2005 – Oct 12, 2005

lore
Share & tools
lore

Konstantin Kletschke

author
#0 lore raw

I cleaned up board support for the scb9328 board.
I did changes to the scb9328.h, it is very much more readable more. Many timings have been tuned, the system is much faster now.

Also the board/scb9328/flash.c is removed, the patch will delete it! The cfi_flash.c is used now.

If anybody is interested, it is at
http://www.ludenkalle.de/u-boot-1.1.2-imx1/u-boot-1.1.2-imx1.diff

It contains the code unlocking intel k3 devices. splitdiff output is at
http://www.ludenkalle.de/u-boot-1.1.2-imx1/u-boot-1.1.2_*
(directory viewing on http://www.ludenkalle.de/u-boot-1.1.2-imx1/ is allowed).

May be I could get advice what still looks odd so I can fix it, so the stuff could be apllied. Or it can be applied as is...

Regards, Konsti

Signature · 2 lines
GPG KeyID EF62FCEF
Fingerprint: 13C9 B16B 9844 EC15 CC2E  A080 1E69 3FDA EF62 FCEF
#1 lore raw

Dear Konstantin,

Quoted 3 lines · in message <20050512143238.GB4333@synertronixx3> you wrote:

I cleaned up board support for the scb9328 board.
I did changes to the scb9328.h, it is very much more readable more. Many timings have been tuned, the system is much faster now.

Ummm... we don't have an entry in the official <MAINTAINERS file, but it seems Sascha Hauer is "responsible" for the scb9328 board. Did you nbegotiate your patch with him?

Quoted 5 lines

Also the board/scb9328/flash.c is removed, the patch will delete it! The cfi_flash.c is used now.

If anybody is interested, it is at
http://www.ludenkalle.de/u-boot-1.1.2-imx1/u-boot-1.1.2-imx1.diff

I reject this patch. It violates the coding style (trailing white space, indenation not by TABs, etc.), and some of your changes to the
scb9328.h file don't look clean to me. Your definition of
CONFIG_COMMANDS is error prone. Go figure what happens when somebody add's a new command which is not supported by your board. He will probably not bother to update your config file.

Quoted 1 line

It contains the code unlocking intel k3 devices.

This is another reason for rejecting it. See the rpevious discussion about this issue.

Quoted 2 lines

May be I could get advice what still looks odd so I can fix it, so the stuff could be apllied. Or it can be applied as is...

Please clean up and submit as patch here on the list.

Best regards,

Wolfgang Denk

Signature · 5 lines
Software Engineering:  Embedded and Realtime Systems,  Embedded Linux
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
"Data is a lot like humans: It is  born.  Matures.  Gets  married  to
other  data, divorced. Gets old. One thing that it doesn't do is die.
It has to be killed."                                 - Arthur Miller
#2 lore raw
Quoted 10 lines · Wolfgang Denk

Dear Konstantin,

Quoted 3 lines · in message <20050512143238.GB4333@synertronixx3> you wrote:

I cleaned up board support for the scb9328 board.
I did changes to the scb9328.h, it is very much more readable more. Many timings have been tuned, the system is much faster now.

Ummm... we don't have an entry in the official <MAINTAINERS file, but it seems Sascha Hauer is "responsible" for the scb9328 board. Did you nbegotiate your patch with him?

No, he did not. But since he actively develops with this board and I only occasionally use it, it is ok.

Sascha Hauer

#3 lore raw

Hello,

Quoted 3 lines · in message <[email protected]> you wrote:

No, he did not. But since he actively develops with this board and I only occasionally use it, it is ok.

So should I add Konstantin as Maintainer?

Konstantin, is this OK with you?

Best regards,

Wolfgang Denk

Signature · 4 lines
Software Engineering:  Embedded and Realtime Systems,  Embedded Linux
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Leave bigotry in your quarters; there's no room for it on the bridge.
	-- Kirk, "Balance of Terror", stardate 1709.2
#4 lore raw
Quoted 8 lines · Wolfgang Denk

Hello,

Quoted 3 lines · in message <[email protected]> you wrote:

No, he did not. But since he actively develops with this board and I only occasionally use it, it is ok.

So should I add Konstantin as Maintainer?

Yes, he has more knowledge about this board then me.

regards,
Sascha

author
#5 lore raw
In reply to Wolfgang Denk

At Tue, 11 Oct 2005 10:11:47 +0200,

Quoted 1 line · Wolfgang Denk

Konstantin, is this OK with you?

Yes.

kletschke at synertronixx.de

What a funny coincidence! Actually I am preparing a new patch with some small new features and a cleaned up flash support now. The port uses cfi now and not board/scb9328/flash.c anymore. This is lightning fast compared to the old way due to buffered writing.

Konstantin Kletschke

Signature · 2 lines
GPG KeyID EF62FCEF
Fingerprint: 13C9 B16B 9844 EC15 CC2E  A080 1E69 3FDA EF62 FCEF
author
#6 lore raw
In reply to Wolfgang Denk

As I see 1.1.3 is released so actual code improvements should be done in there, is that right?

At
http://www.ludenkalle.de/u-boot/u-boot-1.1.2-imx2/
is the revised scb9328 i.MX BS apllying against 1.1.2.

If anybody is interested, he could take a look into it and advise in my coding while I adapt the stuff to 1.1.3.

@Sascha: This patch actually contains cmd_burst.c and should compile now :-)

Regards, Konsti

Signature · 2 lines
GPG KeyID EF62FCEF
Fingerprint: 13C9 B16B 9844 EC15 CC2E  A080 1E69 3FDA EF62 FCEF
#7 lore raw
Quoted 3 lines · In message <87ll10jjpf.wl%[email protected]> you wrote:

As I see 1.1.3 is released so actual code improvements should be done in there, is that right?

No. Code improvements should be always done against the HEAD version in the git repository (or the top of tree in CVS).

Best regards,

Wolfgang Denk

Signature · 5 lines
Software Engineering:  Embedded and Realtime Systems,  Embedded Linux
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
The most exciting phrase to hear in science, the one that heralds new
discoveries, is not "Eureka!" (I found it!) but "That's funny ..."
                                                      -- Isaac Asimov
#9 lore raw
Quoted 6 lines · In message <87hdbojbn1.wl%[email protected]> you wrote:

Here we go:

http://www.ludenkalle.de/u-boot/u-boot-git-20051011/

Compiles and runs well.

Ummm.... there are 15 files with somewhat funny, but non-desriptive names in this directory. What am I to do with this?

Best regards,

Wolfgang Denk

Signature · 4 lines
Software Engineering:  Embedded and Realtime Systems,  Embedded Linux
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
It is dangerous to be right on a subject  on  which  the  established
authorities are wrong.                                    -- Voltaire
author
#10 lore raw
In reply to Wolfgang Denk

Oops, sorry, I overlooked this e-mail!

At Sun, 09 Oct 2005 01:17:18 +0200,

Quoted 3 lines · Wolfgang Denk

Ummm... we don't have an entry in the official <MAINTAINERS file, but it seems Sascha Hauer is "responsible" for the scb9328 board. Did you nbegotiate your patch with him?

Hm, he did the initial port. Now I tune this and that and Sascha is testing my images or patches from time to time.
The scb9328 is developed sold in our company whereas Sascha works somewhere else now.

Quoted 2 lines

I reject this patch. It violates the coding style (trailing white space, indenation not by TABs, etc.), and some of your changes to

Ouch! I will fix this. I even found linebreaks replaced by TAB+linebreak.
Do you have seen this by accident or do you search for such things with your editor?

Quoted 2 lines

scb9328.h file don't look clean to me. Your definition of
CONFIG_COMMANDS is error prone.

Ok, how should I do it right?

Quoted 3 lines

Go figure what happens when somebody
add's a new command which is not supported by your board. He will probably not bother to update your config file.

I don't get it. May be I have blinders on now. But how breaks a new config command my setup (and not the others, I saw this construction somewhere else).

Quoted 4 lines
Quoted 1 line

It contains the code unlocking intel k3 devices.

This is another reason for rejecting it. See the rpevious discussion about this issue.

This stuff I removed again in the git diff (not fully I realized now, two defines...).
I use "protect off" and for normal use
userspace tools does this from linux-mtd.

Quoted 1 line

Please clean up and submit as patch here on the list.

No Problem.

Konsti

Signature · 2 lines
GPG KeyID EF62FCEF
Fingerprint: 13C9 B16B 9844 EC15 CC2E  A080 1E69 3FDA EF62 FCEF
#11 lore raw
Quoted 3 lines · In message <87achfykq2.wl%[email protected]> you wrote:

Ouch! I will fix this. I even found linebreaks replaced by TAB+linebreak.

Does that mean that I should ignore the link you posted earlier today?

Quoted 2 lines

Do you have seen this by accident or do you search for such things with your editor?

I scan most patches, escpecially from new or untrusted posters, and I regularly scan (and cleanup) the whole code tree.

Quoted 4 lines
Quoted 2 lines

scb9328.h file don't look clean to me. Your definition of
CONFIG_COMMANDS is error prone.

Ok, how should I do it right?

Make sure to include or exactly the needed features only. "all but X" is asking for problems when a new feature Y gets added which is not supported by your board. Rather use "default - M - N + X + Y".

Quoted 3 lines

I don't get it. May be I have blinders on now. But how breaks a new config command my setup (and not the others, I saw this construction somewhere else).

Yes, there are several of these stupidly configured boards which I have to cleanup nearly each time a new command gets added. See above.

Best regards,

Wolfgang Denk

Signature · 4 lines
Software Engineering:  Embedded and Realtime Systems,  Embedded Linux
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Applying computer technology is simply finding the  right  wrench  to
pound in the correct screw.
author
#12 lore raw

At Tue, 11 Oct 2005 22:34:24 +0200,

Quoted 2 lines · Wolfgang Denk

Does that mean that I should ignore the link you posted earlier today?

I cleared up the structure now.

http://www.ludenkalle.de/u-boot/

Has my last clean up effort and the old stuff in deprecated. The u-boot-git-20051012.diff is a diff between yesterdays git checkout and my actual modifications. the directory content of u-boot-git-20051012/ is the same splittet with splitdiff vor viewing convenience on a per file basis (so pretty redundant).

Quoted 3 lines

Make sure to include or exactly the needed features only. "all but X" is asking for problems when a new feature Y gets added which is not supported by your board. Rather use "default - M - N + X + Y".

But is

+#define CONFIG_COMMANDS \
	 (CONFIG_CMD_DFL\
	& ~CFG_CMD_NET\
	& ~CFG_CMD_IMLS\

[...]

	& ~CFG_CMD_MISC\
	| CFG_CMD_CACHE)

not your described way to do this?

Quoted 2 lines

Yes, there are several of these stupidly configured boards which I have to cleanup nearly each time a new command gets added. See

Do you have an example in mind where I should have a look at?

Regards, Konsti

Signature · 2 lines
GPG KeyID EF62FCEF
Fingerprint: 13C9 B16B 9844 EC15 CC2E  A080 1E69 3FDA EF62 FCEF