Dear Stefan Roese,
In message 200810211435.39059.sr@denx.de you wrote:
That's directly imported from the Linux source. Most of your comments below also deal with this Linux code copying. I personally think it is a good idea to clone the code from Linux instead of writing a U-Boot specific UBI driver. We have done this before on other occasions and it makes perfect sense here too from my point of view. It makes porting easier and faster and less error prone and even more important, it makes porting of new versions, fixes etc easier.
We agree on that. So maybe we should send coding style cleanup patches for the Linux kenrel?
- Core registration and callback routines for MTD
- drivers and users.
- */
GPL header missing - here and in many other files.
This *is* a serious issue. And I don;t care wether the code comes from Linux or not, but I will not accept any code without absolutley clear licensing conditions.
Need "{...}" for multi-line statements.
Again from the Linux original source. When we start changing this code because of coding-style issue, it will get very hard to compare those versions and add fixes in the future. So I vote for keeping the code as is. It's not that ugly, at least from my understanding.
It is a clear violation of Linux Coding Style requirements.
If we don;t want to fix it here, I suggest to fixit at the origin, i.e. in Linux.
+++ b/drivers/mtd/mtdpart.c @@ -0,0 +1,531 @@ +/*
- Simple MTD partitioning layer
Hm... We were talking about UBI support.
Do we really have to pull in the whole MTD layer from Linux for this?
...
Please do not add dead code.
Here I'm not sure which version I prefer. The one Kyungmin used, disabling > the not needed code via #if 0 (or something else, like #if UBI_LINUX), or completely removing it. Both has some advantages. But again for comparison> with the original Linux source it's perhaps better to just disable the code> . An "#if UBI_LINIX" is probably better than on "#if 0" though.
There is two things I am concerned about:
1) Do we really need all these files? 2) Do we really need all of this code?
Lots of the code that was left in was obviously dealing with features we don;t have in U-Boot, and will not have: concepts like user IDs, file permissions, major and minor numbers, etc.
Of course we haven't. But it's hard to draw the line *if* you decide to include the Linux source. Most OS functions (like spin_lock()...) are defin> ed to no-ops in ubi_uboot.h. So it doesn't really increase the code size for U-Boot. It just keeps the source in-line with the Linux version.
On the other hand it makes it semi-impossible to review the code in U-Boot context, or even to get an understanding of flow of control etc.
Best regards,
Wolfgang Denk