Re: [U-Boot] [PATCH 2/6] gpt: Replace the leXX_to_int() calls with ones defined at compiler.h
Hi Chang Hyun, Hello Lukasz, Stephen, I see there has been quite a few mails going back and forth on the mailing list on the gpt: Replace the leXX_to_int() calls with ones defined at compiler.hI haven't been receiving the e-mails because my samsung e-mail account has expired.(Internship expired)(I just happened to run into the mails from a google search) Is there anything I should catch up on? Anything I should be doing?And on which fork is this commit being pushed onto? Thank you. I've changed you as an author of the commit. I will also add your private e-mail to CC when I send gpt v2, so stay tunned :-) -- Best regards, Lukasz Majewski Samsung Poland RD Center | Linux Platform Group ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 2/6] gpt: Replace the leXX_to_int() calls with ones defined at compiler.h
Hi Stephen,
On 08/24/2012 02:13 AM, Lukasz Majewski wrote:
Custom definitions of le_XX_to_int functions have been replaced with
standard ones, defined at compiler.h
diff --git a/disk/part_efi.c b/disk/part_efi.c
-/* Convert char[8] in little endian format to the host format
integer
- */
-static inline unsigned long long le64_to_int(unsigned char *le64)
-{
So this original function takes a pointer to the value ...
/* Check the first_usable_lba and last_usable_lba are
within the disk. */ lastlba = (unsigned long long)dev_desc-lba;
- if (le64_to_int(pgpt_head-first_usable_lba) lastlba) {
+ if (le64_to_cpu(pgpt_head-first_usable_lba) lastlba) {
At this point in the series, first_usable_lba is a char[8], so this is
passing the address of the first byte to both the original function
le64_to_int(), and the replacement function le64_to_cpu(). However,
le64_to_cpu() expects to receive the 64-bit value to swap, not a
pointer to it - from compiler.h:
#define _uswap_64(x, sfx) \
x) 0xff00##sfx) 56) | \
...
# define uswap_64(x) _uswap_64(x, ull)
le:
# define le64_to_cpu(x) (x)
be:
# define le64_to_cpu(x) uswap_64(x)
So I think this patch breaks the code, and then the next patch fixes
it, since it changes the type of first_usable_lba from char[8] to
__le64?
First of all, thanks for very thorough review.
You are right, my fault. Patch 2/6 and 3/6 shall be squashed together.
When squashed, no errors and warnings appear.
I've unnecessary separated them to show this two step change. For the
sake of bisecting - those shall be squashed.
--
Best regards,
Lukasz Majewski
Samsung Poland RD Center | Linux Platform Group
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 2/6] gpt: Replace the leXX_to_int() calls with ones defined at compiler.h
Hi Stephen, On 08/24/2012 02:13 AM, Lukasz Majewski wrote: Custom definitions of le_XX_to_int functions have been replaced with standard ones, defined at compiler.h Signed-off-by: Chang Hyun Park chchch.p...@samsung.com Signed-off-by: Lukasz Majewski l.majew...@samsung.com Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com Did Chang Hyun Park write this? If so, shouldn't git have generated a From header for him, to reflect his authorship upstream. I'm not sure how Kyungmin Park's S-o-b plays into this, since he's not sending the patch. My work (for those patches) was based on Mr. Chang Hyum Park patches. However I had to modify his work for patches submission. Therefore I've added Mr. Chang in a first place to credit his effort to this patch series creation. Shall I do it in a different way? -- Best regards, Lukasz Majewski Samsung Poland RD Center | Linux Platform Group ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 2/6] gpt: Replace the leXX_to_int() calls with ones defined at compiler.h
On 09/06/2012 04:20 AM, Lukasz Majewski wrote: Hi Stephen, On 08/24/2012 02:13 AM, Lukasz Majewski wrote: Custom definitions of le_XX_to_int functions have been replaced with standard ones, defined at compiler.h Signed-off-by: Chang Hyun Park chchch.p...@samsung.com Signed-off-by: Lukasz Majewski l.majew...@samsung.com Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com Did Chang Hyun Park write this? If so, shouldn't git have generated a From header for him, to reflect his authorship upstream. I'm not sure how Kyungmin Park's S-o-b plays into this, since he's not sending the patch. My work (for those patches) was based on Mr. Chang Hyum Park patches. However I had to modify his work for patches submission. Therefore I've added Mr. Chang in a first place to credit his effort to this patch series creation. Shall I do it in a different way? To be honest, I'm not really sure. When I've done this, I've kept the original author in the git author field to show this, and then listed what I changed in the commit description. Whether that's the right way to go perhaps depends on the size of your changes. If your changes are so large that it doesn't make sense to do that, perhaps his S-o-b should be removed and replaced with a simple note based on work by Chang Hyun Park chchch.p...@samsung.com? ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 2/6] gpt: Replace the leXX_to_int() calls with ones defined at compiler.h
On 08/24/2012 02:13 AM, Lukasz Majewski wrote: Custom definitions of le_XX_to_int functions have been replaced with standard ones, defined at compiler.h Signed-off-by: Chang Hyun Park chchch.p...@samsung.com Signed-off-by: Lukasz Majewski l.majew...@samsung.com Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com Did Chang Hyun Park write this? If so, shouldn't git have generated a From header for him, to reflect his authorship upstream. I'm not sure how Kyungmin Park's S-o-b plays into this, since he's not sending the patch. ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 2/6] gpt: Replace the leXX_to_int() calls with ones defined at compiler.h
On 08/24/2012 02:13 AM, Lukasz Majewski wrote:
Custom definitions of le_XX_to_int functions have been replaced with
standard ones, defined at compiler.h
diff --git a/disk/part_efi.c b/disk/part_efi.c
-/* Convert char[8] in little endian format to the host format integer
- */
-static inline unsigned long long le64_to_int(unsigned char *le64)
-{
So this original function takes a pointer to the value ...
/* Check the first_usable_lba and last_usable_lba are within the disk.
*/
lastlba = (unsigned long long)dev_desc-lba;
- if (le64_to_int(pgpt_head-first_usable_lba) lastlba) {
+ if (le64_to_cpu(pgpt_head-first_usable_lba) lastlba) {
At this point in the series, first_usable_lba is a char[8], so this is
passing the address of the first byte to both the original function
le64_to_int(), and the replacement function le64_to_cpu(). However,
le64_to_cpu() expects to receive the 64-bit value to swap, not a pointer
to it - from compiler.h:
#define _uswap_64(x, sfx) \
x) 0xff00##sfx) 56) | \
...
# define uswap_64(x) _uswap_64(x, ull)
le:
# define le64_to_cpu(x) (x)
be:
# define le64_to_cpu(x) uswap_64(x)
So I think this patch breaks the code, and then the next patch fixes it,
since it changes the type of first_usable_lba from char[8] to __le64?
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [PATCH 2/6] gpt: Replace the leXX_to_int() calls with ones defined at compiler.h
Custom definitions of le_XX_to_int functions have been replaced with
standard ones, defined at compiler.h
Signed-off-by: Chang Hyun Park chchch.p...@samsung.com
Signed-off-by: Lukasz Majewski l.majew...@samsung.com
Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com
---
disk/part_efi.c | 109 ---
1 files changed, 40 insertions(+), 69 deletions(-)
diff --git a/disk/part_efi.c b/disk/part_efi.c
index 02927a0..86e7f33 100644
--- a/disk/part_efi.c
+++ b/disk/part_efi.c
@@ -44,34 +44,6 @@
defined(CONFIG_MMC) || \
defined(CONFIG_SYSTEMACE)
-/* Convert char[2] in little endian format to the host format integer
- */
-static inline unsigned short le16_to_int(unsigned char *le16)
-{
- return ((le16[1] 8) + le16[0]);
-}
-
-/* Convert char[4] in little endian format to the host format integer
- */
-static inline unsigned long le32_to_int(unsigned char *le32)
-{
- return ((le32[3] 24) + (le32[2] 16) + (le32[1] 8) + le32[0]);
-}
-
-/* Convert char[8] in little endian format to the host format integer
- */
-static inline unsigned long long le64_to_int(unsigned char *le64)
-{
- return (((unsigned long long)le64[7] 56) +
- ((unsigned long long)le64[6] 48) +
- ((unsigned long long)le64[5] 40) +
- ((unsigned long long)le64[4] 32) +
- ((unsigned long long)le64[3] 24) +
- ((unsigned long long)le64[2] 16) +
- ((unsigned long long)le64[1] 8) +
- (unsigned long long)le64[0]);
-}
-
/**
* efi_crc32() - EFI version of crc32 function
* @buf: buffer to calculate crc32 of
@@ -79,7 +51,7 @@ static inline unsigned long long le64_to_int(unsigned char
*le64)
*
* Description: Returns EFI-style CRC32 value for @buf
*/
-static inline unsigned long efi_crc32(const void *buf, unsigned long len)
+static inline u32 efi_crc32(const void *buf, u32 len)
{
return crc32(0, buf, len);
}
@@ -137,13 +109,13 @@ void print_part_efi(block_dev_desc_t * dev_desc)
debug(%s: gpt-entry at %p\n, __func__, gpt_pte);
printf(Part\tName\t\t\tStart LBA\tEnd LBA\n);
- for (i = 0; i le32_to_int(gpt_head-num_partition_entries); i++) {
+ for (i = 0; i le32_to_cpu(gpt_head-num_partition_entries); i++) {
if (is_pte_valid(gpt_pte[i])) {
printf(%3d\t%-18s\t0x%08llX\t0x%08llX\n, (i + 1),
print_efiname(gpt_pte[i]),
- le64_to_int(gpt_pte[i].starting_lba),
- le64_to_int(gpt_pte[i].ending_lba));
+ (u64) le64_to_cpu(gpt_pte[i].starting_lba),
+ (u64) le64_to_cpu(gpt_pte[i].ending_lba));
} else {
break; /* Stop at the first non valid PTE */
}
@@ -174,9 +146,9 @@ int get_partition_info_efi(block_dev_desc_t * dev_desc, int
part,
}
/* The ulong casting limits the maximum disk size to 2 TB */
- info-start = (ulong) le64_to_int(gpt_pte[part - 1].starting_lba);
+ info-start = (u64) le64_to_cpu(gpt_pte[part - 1].starting_lba);
/* The ending LBA is inclusive, to calculate size, add 1 to it */
- info-size = ((ulong)le64_to_int(gpt_pte[part - 1].ending_lba) + 1)
+ info-size = ((u64)le64_to_cpu(gpt_pte[part - 1].ending_lba) + 1)
- info-start;
info-blksz = GPT_BLOCK_SIZE;
@@ -215,7 +187,7 @@ int test_part_efi(block_dev_desc_t * dev_desc)
static int pmbr_part_valid(struct partition *part)
{
if (part-sys_ind == EFI_PMBR_OSTYPE_EFI_GPT
- le32_to_int(part-start_sect) == 1UL) {
+ le32_to_cpu(part-start_sect) == 1UL) {
return 1;
}
@@ -234,9 +206,8 @@ static int is_pmbr_valid(legacy_mbr * mbr)
{
int i = 0;
- if (!mbr || le16_to_int(mbr-signature) != MSDOS_MBR_SIGNATURE) {
+ if (!mbr || le16_to_cpu(mbr-signature) != MSDOS_MBR_SIGNATURE)
return 0;
- }
for (i = 0; i 4; i++) {
if (pmbr_part_valid(mbr-partition_record[i])) {
@@ -259,8 +230,8 @@ static int is_pmbr_valid(legacy_mbr * mbr)
static int is_gpt_valid(block_dev_desc_t * dev_desc, unsigned long long lba,
gpt_header * pgpt_head, gpt_entry ** pgpt_pte)
{
- unsigned char crc32_backup[4] = { 0 };
- unsigned long calc_crc32;
+ u32 crc32_backup = 0;
+ u32 calc_crc32;
unsigned long long lastlba;
if (!dev_desc || !pgpt_head) {
@@ -275,54 +246,54 @@ static int is_gpt_valid(block_dev_desc_t * dev_desc,
unsigned long long lba,
}
/* Check the GPT header signature */
- if (le64_to_int(pgpt_head-signature) != GPT_HEADER_SIGNATURE) {
+ if (le64_to_cpu(pgpt_head-signature) != GPT_HEADER_SIGNATURE) {
printf(GUID Partition Table Header signature
