[U-Boot-Users] i.MX support for scb9328 cleaned up
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
Dear Konstantin,
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?
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.
It contains the code unlocking intel k3 devices.
This is another reason for rejecting it. See the rpevious discussion about this issue.
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
On Sun, Oct 09, 2005 at 01:17:18AM +0200, Wolfgang Denk wrote:
Dear Konstantin,
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
Hello,
in message 20051011074941.GA8818@herry.saufen 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
On Tue, Oct 11, 2005 at 10:11:47AM +0200, Wolfgang Denk wrote:
Hello,
in message 20051011074941.GA8818@herry.saufen 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
At Tue, 11 Oct 2005 10:11:47 +0200, Wolfgang Denk wd@denx.de wrote:
Konstantin, is this OK with you?
Yes.
kletschke@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
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
In message 87ll10jjpf.wl%kletschke@synertronixx.de 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
At Tue, 11 Oct 2005 15:38:24 +0200, Wolfgang Denk wd@denx.de wrote:
in the git repository (or the top of tree in CVS).
Here we go:
http://www.ludenkalle.de/u-boot/u-boot-git-20051011/
Compiles and runs well.
Regards Konsti
In message 87hdbojbn1.wl%kletschke@synertronixx.de 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
Oops, sorry, I overlooked this e-mail!
At Sun, 09 Oct 2005 01:17:18 +0200, Wolfgang Denk wd@denx.de wrote:
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.
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?
scb9328.h file don't look clean to me. Your definition of CONFIG_COMMANDS is error prone.
Ok, how should I do it right?
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).
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.
Please clean up and submit as patch here on the list.
No Problem.
Konsti
In message 87achfykq2.wl%konsti@ku-gbr.de 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?
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.
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".
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
At Tue, 11 Oct 2005 22:34:24 +0200, Wolfgang Denk wd@denx.de wrote:
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).
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?
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
participants (3)
-
Konstantin Kletschke -
Sascha Hauer -
Wolfgang Denk