* [U-Boot] [PATCH] Fix some bugs in the EFI support @ 2011-11-29 23:39 Gabe Black 2011-12-02 21:28 ` Graeme Russ 0 siblings, 1 reply; 7+ messages in thread From: Gabe Black @ 2011-11-29 23:39 UTC (permalink / raw) To: u-boot 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 <gabeblack@chromium.org> --- disk/part_efi.c | 8 ++++++-- 1 files changed, 6 insertions(+), 2 deletions(-) diff --git a/disk/part_efi.c b/disk/part_efi.c index e7f2714..c94d808 100644 --- a/disk/part_efi.c +++ b/disk/part_efi.c @@ -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; } @@ -169,7 +173,7 @@ int get_partition_info_efi(block_dev_desc_t * dev_desc, int part, /* 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; } -- 1.7.3.1 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [U-Boot] [PATCH] Fix some bugs in the EFI support 2011-11-29 23:39 [U-Boot] [PATCH] Fix some bugs in the EFI support Gabe Black @ 2011-12-02 21:28 ` Graeme Russ 2011-12-02 21:59 ` Simon Glass 0 siblings, 1 reply; 7+ messages in thread From: Graeme Russ @ 2011-12-02 21:28 UTC (permalink / raw) To: u-boot Hi Anton, On 30/11/11 10:39, Gabe Black wrote: > 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 <gabeblack@chromium.org> > --- > disk/part_efi.c | 8 ++++++-- > 1 files changed, 6 insertions(+), 2 deletions(-) > > diff --git a/disk/part_efi.c b/disk/part_efi.c > index e7f2714..c94d808 100644 > --- a/disk/part_efi.c > +++ b/disk/part_efi.c > @@ -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; > } > @@ -169,7 +173,7 @@ int get_partition_info_efi(block_dev_desc_t * dev_desc, int part, > > /* 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
* [U-Boot] [PATCH] Fix some bugs in the EFI support 2011-12-02 21:28 ` Graeme Russ @ 2011-12-02 21:59 ` Simon Glass 2011-12-06 18:50 ` Anton Staaf 0 siblings, 1 reply; 7+ messages in thread From: Simon Glass @ 2011-12-02 21:59 UTC (permalink / raw) To: u-boot Hi Graeme, On Fri, Dec 2, 2011 at 1:28 PM, Graeme Russ <graeme.russ@gmail.com> wrote: > Hi Anton, > > On 30/11/11 10:39, Gabe Black wrote: >> 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 <gabeblack@chromium.org> >> --- >> ?disk/part_efi.c | ? ?8 ++++++-- >> ?1 files changed, 6 insertions(+), 2 deletions(-) >> >> diff --git a/disk/part_efi.c b/disk/part_efi.c >> index e7f2714..c94d808 100644 >> --- a/disk/part_efi.c >> +++ b/disk/part_efi.c >> @@ -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; >> ? ? ? } >> @@ -169,7 +173,7 @@ int get_partition_info_efi(block_dev_desc_t * dev_desc, int part, >> >> ? ? ? /* 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 > > Regards, > > Graeme > > ^ permalink raw reply [flat|nested] 7+ messages in thread
* [U-Boot] [PATCH] Fix some bugs in the EFI support 2011-12-02 21:59 ` Simon Glass @ 2011-12-06 18:50 ` Anton Staaf 2011-12-06 19:22 ` Anatolij Gustschin 2011-12-06 19:35 ` Wolfgang Denk 0 siblings, 2 replies; 7+ messages in thread From: Anton Staaf @ 2011-12-06 18:50 UTC (permalink / raw) To: u-boot On Fri, Dec 2, 2011 at 1:59 PM, Simon Glass <sjg@chromium.org> wrote: > Hi Graeme, > > On Fri, Dec 2, 2011 at 1:28 PM, Graeme Russ <graeme.russ@gmail.com> wrote: >> Hi Anton, >> >> On 30/11/11 10:39, Gabe Black wrote: >>> 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 <gabeblack@chromium.org> >>> --- >>> ?disk/part_efi.c | ? ?8 ++++++-- >>> ?1 files changed, 6 insertions(+), 2 deletions(-) >>> >>> diff --git a/disk/part_efi.c b/disk/part_efi.c >>> index e7f2714..c94d808 100644 >>> --- a/disk/part_efi.c >>> +++ b/disk/part_efi.c >>> @@ -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; >>> ? ? ? } >>> @@ -169,7 +173,7 @@ int get_partition_info_efi(block_dev_desc_t * dev_desc, int part, >>> >>> ? ? ? /* 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 > > >> >> 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. Thanks, Anton ^ permalink raw reply [flat|nested] 7+ messages in thread
* [U-Boot] [PATCH] Fix some bugs in the EFI support 2011-12-06 18:50 ` Anton Staaf @ 2011-12-06 19:22 ` Anatolij Gustschin 2011-12-06 19:35 ` Wolfgang Denk 1 sibling, 0 replies; 7+ messages in thread From: Anatolij Gustschin @ 2011-12-06 19:22 UTC (permalink / raw) To: u-boot Hi Anton, On Tue, 6 Dec 2011 10:50:14 -0800 Anton Staaf <robotboy@chromium.org> wrote: ... > 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. One of the acked patches [1] is in mainline u-boot.git/master now [2]. We will have to double-check after Wolfgang pulls from Albert's ARM tree since there is another patch [3] addressing this and it additionally changes CONFIG_SYS_CACHELINE_SIZE to ARCH_DMA_MINALIGN. Thanks, Anatolij [1] http://patchwork.ozlabs.org/patch/122485/ [2] http://git.denx.de/?p=u-boot.git;a=commit;h=4715a81136049167160230efd28eb9d48eeded1f [3] http://git.denx.de/?p=u-boot/u-boot-arm.git;a=commit;h=46ccae75e9f9c497a3e0e3c55a3173c2801db41d ^ permalink raw reply [flat|nested] 7+ messages in thread
* [U-Boot] [PATCH] Fix some bugs in the EFI support 2011-12-06 18:50 ` Anton Staaf 2011-12-06 19:22 ` Anatolij Gustschin @ 2011-12-06 19:35 ` Wolfgang Denk 2011-12-09 23:20 ` Anton Staaf 1 sibling, 1 reply; 7+ messages in thread From: Wolfgang Denk @ 2011-12-06 19:35 UTC (permalink / raw) To: u-boot Dear Anton Staaf, In message <CAF6FioWeoF1RWckrfWUsoBcybwU4g4xp84pROirb5HcbfJKEZQ@mail.gmail.com> you wrote: > > 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. ^ permalink raw reply [flat|nested] 7+ messages in thread
* [U-Boot] [PATCH] Fix some bugs in the EFI support 2011-12-06 19:35 ` Wolfgang Denk @ 2011-12-09 23:20 ` Anton Staaf 0 siblings, 0 replies; 7+ messages in thread From: Anton Staaf @ 2011-12-09 23:20 UTC (permalink / raw) To: u-boot On Tue, Dec 6, 2011 at 11:35 AM, Wolfgang Denk <wd@denx.de> wrote: > Dear Anton Staaf, > > In message <CAF6FioWeoF1RWckrfWUsoBcybwU4g4xp84pROirb5HcbfJKEZQ@mail.gmail.com> you wrote: >> >> 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? I don't believe that there is anything wrong right now. My whimpering was more about the sheer number of messages have been generated by this patch collision. Not a big deal since as far as I can tell master is OK. Thanks, Anton > 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. ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2011-12-09 23:20 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2011-11-29 23:39 [U-Boot] [PATCH] Fix some bugs in the EFI support Gabe Black 2011-12-02 21:28 ` Graeme Russ 2011-12-02 21:59 ` Simon Glass 2011-12-06 18:50 ` Anton Staaf 2011-12-06 19:22 ` Anatolij Gustschin 2011-12-06 19:35 ` Wolfgang Denk 2011-12-09 23:20 ` Anton Staaf
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox