Fix some bugs in the EFI support

The ALLOC_CACHE_ALIGN_BUFFER macro allocates a pointer to a buffer, and the address of the pointer and not the buffer was being passed to a function that expected the other. This change fixes a few places that bug crops up. Also, it provides a default definition for the CONFIG_SYS_CACHELINE_SIZE macro to avoid compiler errors.

Signed-off-by Gabe Black <[email protected]>

---
 disk/part_efi.c |    8 ++++++--
 1 files changed, 6 insertions(+), 2 deletions(-)

disk/part_efi.c +5 -1

index e7f2714..c94d808 100644
@@ -37,6 +37,10 @@
 #include "part_efi.h"
 #include <linux/ctype.h>
 
+#ifndef CONFIG_SYS_CACHELINE_SIZE
+#define CONFIG_SYS_CACHELINE_SIZE __BIGGEST_ALIGNMENT__
+#endif
+
 #if defined(CONFIG_CMD_IDE) || \
     defined(CONFIG_CMD_MG_DISK) || \
     defined(CONFIG_CMD_SATA) || \

@@ -130,7 +134,7 @@ void print_part_efi(block_dev_desc_t * dev_desc)
 	}
 	/* This function validates AND fills in the GPT header and PTE */
 	if (is_gpt_valid(dev_desc, GPT_PRIMARY_PARTITION_TABLE_LBA,
-			 &(gpt_head), &gpt_pte) != 1) {
+			 gpt_head, &gpt_pte) != 1) {
 		printf("%s: *** ERROR: Invalid GPT ***\n", __func__);
 		return;
 	}

Hi Anton,

Quoted 37 lines · Gabe Black

The ALLOC_CACHE_ALIGN_BUFFER macro allocates a pointer to a buffer, and the address of the pointer and not the buffer was being passed to a function that expected the other. This change fixes a few places that bug crops up. Also, it provides a default definition for the CONFIG_SYS_CACHELINE_SIZE macro to avoid compiler errors.

Signed-off-by Gabe Black <[email protected]>

---
?disk/part_efi.c | ? ?8 ++++++--
?1 files changed, 6 insertions(+), 2 deletions(-)

disk/part_efi.c +0 -0

index e7f2714..c94d808 100644
@@ -37,6 +37,10 @@

?#include "part_efi.h"
?#include <linux/ctype.h>

disk/part_efi.c +4 -0

+#ifndef CONFIG_SYS_CACHELINE_SIZE
+#define CONFIG_SYS_CACHELINE_SIZE __BIGGEST_ALIGNMENT__
+#endif
+

?#if defined(CONFIG_CMD_IDE) || \
? ? ?defined(CONFIG_CMD_MG_DISK) || \
? ? ?defined(CONFIG_CMD_SATA) || \

disk/part_efi.c +0 -0

@@ -130,7 +134,7 @@ void print_part_efi(block_dev_desc_t * dev_desc)
 ? ? ? }

? ? ? /* This function validates AND fills in the GPT header and PTE */ ? ? ? if (is_gpt_valid(dev_desc, GPT_PRIMARY_PARTITION_TABLE_LBA,
- ? ? ? ? ? ? ? ? ? ? ?&(gpt_head), &gpt_pte) != 1) {
+ ? ? ? ? ? ? ? ? ? ? ?gpt_head, &gpt_pte) != 1) {
? ? ? ? ? ? ? printf("%s: *** ERROR: Invalid GPT ***\n", __func__); ? ? ? ? ? ? ? return;
? ? ? }

disk/part_efi.c +0 -0

@@ -169,7 +173,7 @@ int get_partition_info_efi(block_dev_desc_t * dev_desc, int part,
 

Quoted 7 lines

? ? ? /* This function validates AND fills in the GPT header and PTE */ ? ? ? if (is_gpt_valid(dev_desc, GPT_PRIMARY_PARTITION_TABLE_LBA,
- ? ? ? ? ? ? ? ? ? ? &(gpt_head), &gpt_pte) != 1) {
+ ? ? ? ? ? ? ? ? ? ? gpt_head, &gpt_pte) != 1) {
? ? ? ? ? ? ? printf("%s: *** ERROR: Invalid GPT ***\n", __func__); ? ? ? ? ? ? ? return -1;
? ? ? }

This appears to be a fix for your "part_efi: dcache: allocate cacheline aligned buffers" commit - Can you have a look at it please? If you ack, I will apply to staging

Regards,

Graeme

Hi Graeme,

Quoted 40 lines · Graeme Russ

Hi Anton,

Quoted 37 lines · Gabe Black

The ALLOC_CACHE_ALIGN_BUFFER macro allocates a pointer to a buffer, and the address of the pointer and not the buffer was being passed to a function that expected the other. This change fixes a few places that bug crops up. Also, it provides a default definition for the CONFIG_SYS_CACHELINE_SIZE macro to avoid compiler errors.

Signed-off-by Gabe Black <[email protected]>

---
?disk/part_efi.c | ? ?8 ++++++--
?1 files changed, 6 insertions(+), 2 deletions(-)

disk/part_efi.c +0 -0

index e7f2714..c94d808 100644
@@ -37,6 +37,10 @@

?#include "part_efi.h"
?#include <linux/ctype.h>

disk/part_efi.c +4 -0

+#ifndef CONFIG_SYS_CACHELINE_SIZE
+#define CONFIG_SYS_CACHELINE_SIZE __BIGGEST_ALIGNMENT__
+#endif
+

?#if defined(CONFIG_CMD_IDE) || \
? ? ?defined(CONFIG_CMD_MG_DISK) || \
? ? ?defined(CONFIG_CMD_SATA) || \

disk/part_efi.c +0 -0

@@ -130,7 +134,7 @@ void print_part_efi(block_dev_desc_t * dev_desc)
 ? ? ? }

? ? ? /* This function validates AND fills in the GPT header and PTE */ ? ? ? if (is_gpt_valid(dev_desc, GPT_PRIMARY_PARTITION_TABLE_LBA,
- ? ? ? ? ? ? ? ? ? ? ?&(gpt_head), &gpt_pte) != 1) {
+ ? ? ? ? ? ? ? ? ? ? ?gpt_head, &gpt_pte) != 1) {
? ? ? ? ? ? ? printf("%s: *** ERROR: Invalid GPT ***\n", __func__); ? ? ? ? ? ? ? return;
? ? ? }

disk/part_efi.c +0 -0

@@ -169,7 +173,7 @@ int get_partition_info_efi(block_dev_desc_t * dev_desc, int part,
 

Quoted 11 lines

Quoted 7 lines

? ? ? /* This function validates AND fills in the GPT header and PTE */ ? ? ? if (is_gpt_valid(dev_desc, GPT_PRIMARY_PARTITION_TABLE_LBA,
- ? ? ? ? ? ? ? ? ? ? &(gpt_head), &gpt_pte) != 1) {
+ ? ? ? ? ? ? ? ? ? ? gpt_head, &gpt_pte) != 1) {
? ? ? ? ? ? ? printf("%s: *** ERROR: Invalid GPT ***\n", __func__); ? ? ? ? ? ? ? return -1;
? ? ? }

This appears to be a fix for your "part_efi: dcache: allocate cacheline aligned buffers" commit - Can you have a look at it please? If you ack, I will apply to staging

A fix for this is apparently already in the ARM tree. Perhaps it could be applied so everyone can see it?

Regards,
Simon

Quoted 6 lines

Regards,

Graeme

Whimper whimper. This is silly, what happened was that two different patches collided when committed. Mine had the correct change to this file. Doug Andersons also touched this file but reverted the change. So when both changes went in the result was that these lines didn't change. Since then there have been no fewer than four attempts to get this simple fix committed. I have acked and pinged at least two of them. One finally made it into the ARM tree. Someone should just put this into the main tree because that is where my changes to add the MACRO went in initially. Oh, and we really don't want to have a default for CONFIG_SYS_CACHELINE_SIZE, especially not in this file.

Whimper whimper. This is silly, what happened was that two different patches collided when committed. Mine had the correct change to this file. Doug Andersons also touched this file but reverted the change. So when both changes went in the result was that these lines didn't change. Since then there have been no fewer than four attempts to get this simple fix committed. I have acked and pinged at least two of them. One finally made it into the ARM tree. Someone should just put this into the main tree because that is where my changes to add the MACRO went in initially. Oh, and we really don't want to have a default for CONFIG_SYS_CACHELINE_SIZE, especially not in this file.

Dear Anton Staaf,

Quoted 11 lines · In message <CAF6FioWeoF1RWckrfWUsoBcybwU4g4xp84pROirb5HcbfJKEZQ@mail.gmail.com>…

Whimper whimper. ?This is silly, what happened was that two different patches collided when committed. ?Mine had the correct change to this file. ?Doug Andersons also touched this file but reverted the change. So when both changes went in the result was that these lines didn't change. ?Since then there have been no fewer than four attempts to get this simple fix committed. ?I have acked and pinged at least two of them. ?One finally made it into the ARM tree. ?Someone should just put this into the main tree because that is where my changes to add the MACRO went in initially. ?Oh, and we really don't want to have a default for CONFIG_SYS_CACHELINE_SIZE, especially not in this file.

What exactly is wrong - in your opinion - in the current master?

Best regards,

Wolfgang Denk

--
DENX Software Engineering GmbH, ? ? MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de SW engineering is a race between programmers trying ?to ?make ?better idiot-proof programs and the universe producing greater idiots.