[U-Boot] [PATCH] EXT4
Slowly getting the hang of github and u-boot. I took a fork of u-boot 20120520, and merged in the EXT4 patched.
In addition to Samsung's work;
* Fix minor string ext2->ext4 * 'unsigned' sectors for volumes over 1TB * changed to fit with current u-boot compile env
https://github.com/lundman/u-boot/compare/upstream...upstream-ext4
There is a new limit of 2TB with 512 byte blocks. To go beyond that would require "disk_partition_t" to go bigger than 32bit. I think that is outside of my access permissions.
Hi Jorgen
On Tue, May 22, 2012 at 1:31 PM, Jorgen Lundman lundman@lundman.net wrote:
Slowly getting the hang of github and u-boot. I took a fork of u-boot 20120520, and merged in the EXT4 patched.
In addition to Samsung's work;
* Fix minor string ext2->ext4 * 'unsigned' sectors for volumes over 1TB * changed to fit with current u-boot compile env
https://github.com/lundman/u-boot/compare/upstream...upstream-ext4
These patches will likely not get reviewed and definitely will not get merged unless that are posted to the mailing list
There is a new limit of 2TB with 512 byte blocks. To go beyond that would require "disk_partition_t" to go bigger than 32bit. I think that is outside of my access permissions.
What do you mean by this?
Regards,
Graeme
These patches will likely not get reviewed and definitely will not get merged unless that are posted to the mailing list
Ok, is it better if I use "git format-patch" and send them on the list?
There is a new limit of 2TB with 512 byte blocks. To go beyond that would require "disk_partition_t" to go bigger than 32bit. I think that is outside of my access permissions.
What do you mean by this?
I just assumed that I, new to the use of u-boot, should not tinker with core structs, and core API functionality. I merely wish to mention the issue in case it has already been addressed by the gurus.
It could very well be that u-boot developers don't want to go larger. (presumably, majority of u-boot devices aren't booting from > 2TB devices..)
Or, it has already been discussed and I just missed it.
Hi Jorgen,
On Tue, May 22, 2012 at 1:45 PM, Jorgen Lundman lundman@lundman.net wrote:
These patches will likely not get reviewed and definitely will not get merged unless that are posted to the mailing list
Ok, is it better if I use "git format-patch" and send them on the list?
Yes, that is exactly what you should do.
But before you post them, make sure you run them through checkpatch.pl first and resolve/explain any errors or warnings
There is a new limit of 2TB with 512 byte blocks. To go beyond that would require "disk_partition_t" to go bigger than 32bit. I think that is outside of my access permissions.
What do you mean by this?
I just assumed that I, new to the use of u-boot, should not tinker with core structs, and core API functionality. I merely wish to mention the issue in case it has already been addressed by the gurus.
Anyone can post patches to the mailing list that tinker with core structs, APIs, functionality etc. There are no 'permission' levels here :)
Whatever you post will get reviewed by the community at large and if the feeling is that you proposed 'tinkering' is not suitable for inclusion in U-Boot then comments to that effect will be made. You will then have the opportunity to either a) argue your position or b) submit revised patches which address the comments made
It could very well be that u-boot developers don't want to go larger. (presumably, majority of u-boot devices aren't booting from > 2TB devices..)
Well you could port U-Boot to a NAS with >> 2TB storage :)
Or, it has already been discussed and I just missed it.
Not that I am aware
Regards,
Graeme
Yes, that is exactly what you should do.
But before you post them, make sure you run them through checkpatch.pl first and resolve/explain any errors or warnings
Wow, ohhweee this will take a little while.
How set in stone is the output of checkpatch.pl ? Specifically;
ERROR: do not initialise globals to 0 or NULL #596: FILE: fs/zfs/zfs.c:33: +block_dev_desc_t *zfs_dev_desc = NULL;
That strikes me as dangerous. One lets you fail gracefully (Sorry, X has not been initialised) and the other is just a plain crash. I find crashes to be very ugly, even if it is only reachable by other developers.
WARNING: do not add new typedefs #728: FILE: fs/zfs/zfs.c:165: +typedef struct decomp_entry
I'm seriously not allowed to make new typedefs? ouch.
So yeah, should it always pass without a single problem, or may I employ some measure of moderation?
On Tue, May 22, 2012 at 02:55:57PM +0900, Jorgen Lundman wrote:
ERROR: do not initialise globals to 0 or NULL #596: FILE: fs/zfs/zfs.c:33: +block_dev_desc_t *zfs_dev_desc = NULL;
That strikes me as dangerous. One lets you fail gracefully (Sorry, X has not been initialised) and the other is just a plain crash. I find crashes to be very ugly, even if it is only reachable by other developers.
Globals are per se initialised to 0, so there is no need to explicitly initialise them. As a consequence, it is neither dangerous to omit the initialisation.
Stefan
Hi Jorgen,
On Tue, May 22, 2012 at 3:55 PM, Jorgen Lundman lundman@lundman.net wrote:
Yes, that is exactly what you should do.
But before you post them, make sure you run them through checkpatch.pl first and resolve/explain any errors or warnings
Wow, ohhweee this will take a little while.
How set in stone is the output of checkpatch.pl ? Specifically;
ERROR: do not initialise globals to 0 or NULL #596: FILE: fs/zfs/zfs.c:33: +block_dev_desc_t *zfs_dev_desc = NULL;
That strikes me as dangerous. One lets you fail gracefully (Sorry, X has not been initialised) and the other is just a plain crash. I find crashes to be very ugly, even if it is only reachable by other developers.
Uninitialized global and static variables reside in .bss and are set to zero during relocation. Initialised globals and static variables go into .data
Does a global/static initialised to zero belong in .bss or .data?
By not initialising them, the compiler/linker is forced into putting them into .bss where they are guaranteed to be zero when first accessed
WARNING: do not add new typedefs #728: FILE: fs/zfs/zfs.c:165: +typedef struct decomp_entry
I'm seriously not allowed to make new typedefs? ouch.
If the code is copied verbatim from an existing external repository, the rules can be relaxed - Just make sure you provide a clear and concise reference (like a git commit ID or tag)
Oh and remember, just because you can find a prior art in the U-Boot code does not mean it will be allowed to be used as a backing argument ;)
So yeah, should it always pass without a single problem, or may I employ some measure of moderation?
Only one way to find out ;) But try to make it as clean as you believe reasonable and explain what's left
Regards,
Graeme
Uninitialized global and static variables reside in .bss and are set to zero during relocation. Initialised globals and static variables go into .data
Alas, I know from experience that Microsoft's C compiler does not initialise global variables (to make it faster one assumes) which has led to hours of debugging.
If u-boot has decided that going without Microsoft compiling support is A-OK, then that is A-OK with me too. :)
Oh and remember, just because you can find a prior art in the U-Boot code does not mean it will be allowed to be used as a backing argument ;)
This I understand. Even if what came in from legacy has yet to be cleaned, there is no reason to allow more filth in :)
Only one way to find out ;) But try to make it as clean as you believe reasonable and explain what's left
Understood, clean enough to eat of is the goal.
Lund
Hi Jorgen,
On Tue, May 22, 2012 at 4:17 PM, Jorgen Lundman lundman@lundman.net wrote:
Uninitialized global and static variables reside in .bss and are set to zero during relocation. Initialised globals and static variables go into .data
Alas, I know from experience that Microsoft's C compiler does not initialise global variables (to make it faster one assumes) which has led to hours of debugging.
Well actually, it's not the compiler/linker that initialised .bss - it's the dynamic loader. In the case of U-Boot, it's the Flash->RAM relocation code.
If u-boot has decided that going without Microsoft compiling support is A-OK, then that is A-OK with me too. :)
I don't know of anyone that has even tried, let alone succeeded in compiling U-Boot with a Microsoft compiler. I've heard of cygwin attempts (that from memory have also failed miserably)
Generally speaking, gnu based tools are your best bet. Speaking of which, we really should have a list of 'supported tools' which are known to successfully build U-Boot...
Regards,
Graeme
Dear Jorgen Lundman,
In message 4FBB2F6F.5000907@lundman.net you wrote:
Alas, I know from experience that Microsoft's C compiler does not initialise global variables (to make it faster one assumes) which has led to hours of debugging.
That would be a violation of existing C standards.
If u-boot has decided that going without Microsoft compiling support is A-OK, then that is A-OK with me too. :)
We really don't care about broken, standard-violating crap.
Best regards,
Wolfgang Denk
Dear Jorgen Lundman,
In message 4FBB0BBE.6050103@lundman.net you wrote:
These patches will likely not get reviewed and definitely will not get merged unless that are posted to the mailing list
Ok, is it better if I use "git format-patch" and send them on the list?
Please read the docs. Start at http://www.denx.de/wiki/U-Boot/Patches
Best regards,
Wolfgang Denk
participants (4)
-
Graeme Russ -
Jorgen Lundman -
Stefan Kristiansson -
Wolfgang Denk