* [U-Boot] [PATCH 1/2] powerpc/85xx: CONFIG_ENABLE_36BIT_PHYS does not depend on CONFIG_PHYS_64BIT @ 2011-08-31 22:15 Timur Tabi 2011-08-31 22:15 ` [U-Boot] [PATCH 2/2] powerpc/85xx: standardize display of address map size (32-bit vs. 36-bit) Timur Tabi 0 siblings, 1 reply; 18+ messages in thread From: Timur Tabi @ 2011-08-31 22:15 UTC (permalink / raw) To: u-boot The macro CONFIG_ENABLE_36BIT_PHYS is used to indicate that the given SOC is capable of 36-bit physical addresses, even if such large addresses are not used. On two boards, this macro was enabled only when building a 36-bit image. Signed-off-by: Timur Tabi <timur@freescale.com> --- include/configs/P1022DS.h | 3 ++- include/configs/p1_p2_rdb_pc.h | 3 +-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/include/configs/P1022DS.h b/include/configs/P1022DS.h index a3cccf4..28848bd 100644 --- a/include/configs/P1022DS.h +++ b/include/configs/P1022DS.h @@ -43,8 +43,9 @@ #define CONFIG_FSL_PCIE_RESET /* need PCIe reset errata */ #define CONFIG_SYS_PCI_64BIT /* enable 64-bit PCI resources */ -#ifdef CONFIG_PHYS_64BIT #define CONFIG_ENABLE_36BIT_PHYS + +#ifdef CONFIG_PHYS_64BIT #define CONFIG_ADDR_MAP #define CONFIG_SYS_NUM_ADDR_MAP 16 /* number of TLB1 entries */ #endif diff --git a/include/configs/p1_p2_rdb_pc.h b/include/configs/p1_p2_rdb_pc.h index b9b89cf..df1925f 100644 --- a/include/configs/p1_p2_rdb_pc.h +++ b/include/configs/p1_p2_rdb_pc.h @@ -204,9 +204,8 @@ #define CONFIG_BTB #define CONFIG_BOARD_EARLY_INIT_F /* Call board_pre_init */ -#ifdef CONFIG_PHYS_64BIT + #define CONFIG_ENABLE_36BIT_PHYS -#endif #ifdef CONFIG_PHYS_64BIT #define CONFIG_ADDR_MAP 1 -- 1.7.3.4 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* [U-Boot] [PATCH 2/2] powerpc/85xx: standardize display of address map size (32-bit vs. 36-bit) 2011-08-31 22:15 [U-Boot] [PATCH 1/2] powerpc/85xx: CONFIG_ENABLE_36BIT_PHYS does not depend on CONFIG_PHYS_64BIT Timur Tabi @ 2011-08-31 22:15 ` Timur Tabi 2011-09-01 14:07 ` Kumar Gala 0 siblings, 1 reply; 18+ messages in thread From: Timur Tabi @ 2011-08-31 22:15 UTC (permalink / raw) To: u-boot Most 85xx boards can be built as a 32-bit or a 36-bit. Current code sometimes displays which of these is actually built, but it's inconsistent. This is especially problematic since the "default" build for a given 85xx board can be either one, so if you don't see a message, you can't always know which size is being used. Not only that, but each board includes code that displays the message, so there is duplication. So if a given SOC can support 36-bit addresses, display one of these two messages during boot: ADDR: 32-bit address map or ADDR: 36-bit address map Also delete the board-specific code that does this. Signed-off-by: Timur Tabi <timur@freescale.com> --- arch/powerpc/cpu/mpc85xx/cpu_init.c | 12 ++++++++++++ board/freescale/corenet_ds/corenet_ds.c | 4 ---- board/freescale/mpc8536ds/mpc8536ds.c | 7 +------ board/freescale/mpc8572ds/mpc8572ds.c | 6 +----- board/freescale/p1010rdb/p1010rdb.c | 6 +----- board/freescale/p1022ds/p1022ds.c | 8 ++------ board/freescale/p1_p2_rdb/p1_p2_rdb.c | 4 +--- board/freescale/p1_p2_rdb_pc/p1_p2_rdb_pc.c | 8 +------- board/freescale/p2020ds/p2020ds.c | 8 ++------ board/freescale/p2041rdb/p2041rdb.c | 4 ---- 10 files changed, 21 insertions(+), 46 deletions(-) diff --git a/arch/powerpc/cpu/mpc85xx/cpu_init.c b/arch/powerpc/cpu/mpc85xx/cpu_init.c index 27f836c..d691726 100644 --- a/arch/powerpc/cpu/mpc85xx/cpu_init.c +++ b/arch/powerpc/cpu/mpc85xx/cpu_init.c @@ -302,6 +302,18 @@ int cpu_init_r(void) sync(); #endif + /* + * If we support 36-bit addressing, then display whether this is a + * 32-bit build or a 36-bit build. + */ +#ifdef CONFIG_ENABLE_36BIT_PHYS +#ifdef CONFIG_PHYS_64BIT + puts("ADDR: 36-bit address map\n"); +#else + puts("ADDR: 32-bit address map\n"); +#endif +#endif + puts ("L2: "); #if defined(CONFIG_L2_CACHE) diff --git a/board/freescale/corenet_ds/corenet_ds.c b/board/freescale/corenet_ds/corenet_ds.c index b1eecc4..a33c936 100644 --- a/board/freescale/corenet_ds/corenet_ds.c +++ b/board/freescale/corenet_ds/corenet_ds.c @@ -62,10 +62,6 @@ int checkboard (void) else printf("invalid setting of SW%u\n", PIXIS_LBMAP_SWITCH); -#ifdef CONFIG_PHYS_64BIT - puts("36-bit Addressing\n"); -#endif - /* Display the RCW, so that no one gets confused as to what RCW * we're actually using for this boot. */ diff --git a/board/freescale/mpc8536ds/mpc8536ds.c b/board/freescale/mpc8536ds/mpc8536ds.c index b292e13..b407f1d 100644 --- a/board/freescale/mpc8536ds/mpc8536ds.c +++ b/board/freescale/mpc8536ds/mpc8536ds.c @@ -62,12 +62,7 @@ int checkboard (void) u8 vboot; u8 *pixis_base = (u8 *)PIXIS_BASE; - puts("Board: MPC8536DS "); -#ifdef CONFIG_PHYS_64BIT - puts("(36-bit addrmap) "); -#endif - - printf ("Sys ID: 0x%02x, " + printf ("Board: MPC8536DS Sys ID: 0x%02x, " "Sys Ver: 0x%02x, FPGA Ver: 0x%02x, ", in_8(pixis_base + PIXIS_ID), in_8(pixis_base + PIXIS_VER), in_8(pixis_base + PIXIS_PVER)); diff --git a/board/freescale/mpc8572ds/mpc8572ds.c b/board/freescale/mpc8572ds/mpc8572ds.c index b20299e..38eafe0 100644 --- a/board/freescale/mpc8572ds/mpc8572ds.c +++ b/board/freescale/mpc8572ds/mpc8572ds.c @@ -45,11 +45,7 @@ int checkboard (void) u8 vboot; u8 *pixis_base = (u8 *)PIXIS_BASE; - puts ("Board: MPC8572DS "); -#ifdef CONFIG_PHYS_64BIT - puts ("(36-bit addrmap) "); -#endif - printf ("Sys ID: 0x%02x, " + printf ("Board: MPC8572DS Sys ID: 0x%02x, " "Sys Ver: 0x%02x, FPGA Ver: 0x%02x, ", in_8(pixis_base + PIXIS_ID), in_8(pixis_base + PIXIS_VER), in_8(pixis_base + PIXIS_PVER)); diff --git a/board/freescale/p1010rdb/p1010rdb.c b/board/freescale/p1010rdb/p1010rdb.c index 03e9da1..7aa2117 100644 --- a/board/freescale/p1010rdb/p1010rdb.c +++ b/board/freescale/p1010rdb/p1010rdb.c @@ -165,11 +165,7 @@ int checkboard(void) struct cpu_type *cpu; cpu = gd->cpu; - printf("Board: %sRDB ", cpu->name); -#ifdef CONFIG_PHYS_64BIT - puts("(36-bit addrmap)"); -#endif - puts("\n"); + printf("Board: %sRDB\n", cpu->name); return 0; } diff --git a/board/freescale/p1022ds/p1022ds.c b/board/freescale/p1022ds/p1022ds.c index 456d9b0..aca30f3 100644 --- a/board/freescale/p1022ds/p1022ds.c +++ b/board/freescale/p1022ds/p1022ds.c @@ -56,12 +56,8 @@ int checkboard(void) { u8 sw; - puts("Board: P1022DS "); -#ifdef CONFIG_PHYS_64BIT - puts("(36-bit addrmap) "); -#endif - - printf("Sys ID: 0x%02x, Sys Ver: 0x%02x, FPGA Ver: 0x%02x, ", + printf("Board: P1022DS Sys ID: 0x%02x, " + "Sys Ver: 0x%02x, FPGA Ver: 0x%02x, ", in_8(&pixis->id), in_8(&pixis->arch), in_8(&pixis->scver)); sw = in_8(&PIXIS_SW(PIXIS_LBMAP_SWITCH)); diff --git a/board/freescale/p1_p2_rdb/p1_p2_rdb.c b/board/freescale/p1_p2_rdb/p1_p2_rdb.c index 864b3ce..6418710 100644 --- a/board/freescale/p1_p2_rdb/p1_p2_rdb.c +++ b/board/freescale/p1_p2_rdb/p1_p2_rdb.c @@ -110,9 +110,7 @@ int checkboard (void) cpu = gd->cpu; printf ("Board: %sRDB Rev%c\n", cpu->name, board_rev); -#ifdef CONFIG_PHYS_64BIT - puts ("(36-bit addrmap) \n"); -#endif + setbits_be32(&pgpio->gpdir, GPIO_DIR); /* diff --git a/board/freescale/p1_p2_rdb_pc/p1_p2_rdb_pc.c b/board/freescale/p1_p2_rdb_pc/p1_p2_rdb_pc.c index 4671128..abe087b 100644 --- a/board/freescale/p1_p2_rdb_pc/p1_p2_rdb_pc.c +++ b/board/freescale/p1_p2_rdb_pc/p1_p2_rdb_pc.c @@ -225,13 +225,7 @@ int checkboard(void) ccsr_gur_t *gur = (void *)(CONFIG_SYS_MPC85xx_GUTS_ADDR); u8 in, out, io_config, val; - printf("Board: %s ", CONFIG_BOARDNAME); - -#ifdef CONFIG_PHYS_64BIT - puts("(36-bit addrmap) "); -#endif - - printf("CPLD: V%d.%d PCBA: V%d.0\n", + printf("Board: %s CPLD: V%d.%d PCBA: V%d.0\n", CONFIG_BOARDNAME, in_8(&cpld_data->cpld_rev_major) & 0x0F, in_8(&cpld_data->cpld_rev_minor) & 0x0F, in_8(&cpld_data->pcba_rev) & 0x0F); diff --git a/board/freescale/p2020ds/p2020ds.c b/board/freescale/p2020ds/p2020ds.c index d3af6cf..e8d31a4 100644 --- a/board/freescale/p2020ds/p2020ds.c +++ b/board/freescale/p2020ds/p2020ds.c @@ -61,12 +61,8 @@ int checkboard(void) { u8 sw; - puts("Board: P2020DS "); -#ifdef CONFIG_PHYS_64BIT - puts("(36-bit addrmap) "); -#endif - - printf("Sys ID: 0x%02x, Sys Ver: 0x%02x, FPGA Ver: 0x%02x, ", + printf("Board: P2020DS Sys ID: 0x%02x, " + "Sys Ver: 0x%02x, FPGA Ver: 0x%02x, ", in_8(&pixis->id), in_8(&pixis->arch), in_8(&pixis->scver)); sw = in_8(&PIXIS_SW(PIXIS_LBMAP_SWITCH)); diff --git a/board/freescale/p2041rdb/p2041rdb.c b/board/freescale/p2041rdb/p2041rdb.c index 6ed404f..6e47204 100644 --- a/board/freescale/p2041rdb/p2041rdb.c +++ b/board/freescale/p2041rdb/p2041rdb.c @@ -54,10 +54,6 @@ int checkboard(void) sw = CPLD_READ(fbank_sel); printf("vBank: %d\n", sw & 0x1); -#ifdef CONFIG_PHYS_64BIT - puts("36-bit Addressing\n"); -#endif - /* * Display the RCW, so that no one gets confused as to what RCW * we're actually using for this boot. -- 1.7.3.4 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* [U-Boot] [PATCH 2/2] powerpc/85xx: standardize display of address map size (32-bit vs. 36-bit) 2011-08-31 22:15 ` [U-Boot] [PATCH 2/2] powerpc/85xx: standardize display of address map size (32-bit vs. 36-bit) Timur Tabi @ 2011-09-01 14:07 ` Kumar Gala 2011-09-01 14:13 ` Timur Tabi 2011-09-01 14:22 ` Wolfgang Denk 0 siblings, 2 replies; 18+ messages in thread From: Kumar Gala @ 2011-09-01 14:07 UTC (permalink / raw) To: u-boot On Aug 31, 2011, at 5:15 PM, Timur Tabi wrote: > Most 85xx boards can be built as a 32-bit or a 36-bit. Current code sometimes > displays which of these is actually built, but it's inconsistent. This is > especially problematic since the "default" build for a given 85xx board can > be either one, so if you don't see a message, you can't always know which > size is being used. Not only that, but each board includes code that displays > the message, so there is duplication. > > So if a given SOC can support 36-bit addresses, display one of these two > messages during boot: > > ADDR: 32-bit address map > > or > > ADDR: 36-bit address map > > Also delete the board-specific code that does this. > > Signed-off-by: Timur Tabi <timur@freescale.com> > --- > arch/powerpc/cpu/mpc85xx/cpu_init.c | 12 ++++++++++++ > board/freescale/corenet_ds/corenet_ds.c | 4 ---- > board/freescale/mpc8536ds/mpc8536ds.c | 7 +------ > board/freescale/mpc8572ds/mpc8572ds.c | 6 +----- > board/freescale/p1010rdb/p1010rdb.c | 6 +----- > board/freescale/p1022ds/p1022ds.c | 8 ++------ > board/freescale/p1_p2_rdb/p1_p2_rdb.c | 4 +--- > board/freescale/p1_p2_rdb_pc/p1_p2_rdb_pc.c | 8 +------- > board/freescale/p2020ds/p2020ds.c | 8 ++------ > board/freescale/p2041rdb/p2041rdb.c | 4 ---- > 10 files changed, 21 insertions(+), 46 deletions(-) > > diff --git a/arch/powerpc/cpu/mpc85xx/cpu_init.c b/arch/powerpc/cpu/mpc85xx/cpu_init.c > index 27f836c..d691726 100644 > --- a/arch/powerpc/cpu/mpc85xx/cpu_init.c > +++ b/arch/powerpc/cpu/mpc85xx/cpu_init.c > @@ -302,6 +302,18 @@ int cpu_init_r(void) > sync(); > #endif > > + /* > + * If we support 36-bit addressing, then display whether this is a > + * 32-bit build or a 36-bit build. > + */ > +#ifdef CONFIG_ENABLE_36BIT_PHYS > +#ifdef CONFIG_PHYS_64BIT #if defined(A) && defined(B) > + puts("ADDR: 36-bit address map\n"); > +#else > + puts("ADDR: 32-bit address map\n"); > +#endif > +#endif > + > puts ("L2: "); > > #if defined(CONFIG_L2_CACHE) Wolfgang's being making comments about reducing the boot info we've been outputting so would like his 2 cents on this. - k ^ permalink raw reply [flat|nested] 18+ messages in thread
* [U-Boot] [PATCH 2/2] powerpc/85xx: standardize display of address map size (32-bit vs. 36-bit) 2011-09-01 14:07 ` Kumar Gala @ 2011-09-01 14:13 ` Timur Tabi 2011-09-01 14:22 ` Wolfgang Denk 1 sibling, 0 replies; 18+ messages in thread From: Timur Tabi @ 2011-09-01 14:13 UTC (permalink / raw) To: u-boot On Sep 1, 2011, at 9:07 AM, Kumar Gala <kumar.gala@freescale.com> wrote: > > On Aug 31, 2011, at 5:15 PM, Timur Tabi wrote: > >> >> + /* >> + * If we support 36-bit addressing, then display whether this is a >> + * 32-bit build or a 36-bit build. >> + */ >> +#ifdef CONFIG_ENABLE_36BIT_PHYS >> +#ifdef CONFIG_PHYS_64BIT > > #if defined(A) && defined(B) That's not what I want to do. Look at the code more closely. > > Wolfgang's being making comments about reducing the boot info we've been outputting so would like his 2 cents on this. I understand that, but the patch description explains why this is important. > ^ permalink raw reply [flat|nested] 18+ messages in thread
* [U-Boot] [PATCH 2/2] powerpc/85xx: standardize display of address map size (32-bit vs. 36-bit) 2011-09-01 14:07 ` Kumar Gala 2011-09-01 14:13 ` Timur Tabi @ 2011-09-01 14:22 ` Wolfgang Denk 2011-09-01 14:54 ` Timur Tabi 1 sibling, 1 reply; 18+ messages in thread From: Wolfgang Denk @ 2011-09-01 14:22 UTC (permalink / raw) To: u-boot Dear Kumar Gala, In message <CA7ACAD5-C754-4F8A-BE2E-97E04DD005B2@freescale.com> you wrote: > > Wolfgang's being making comments about reducing the boot info we've been = > outputting so would like his 2 cents on this. I understand that this is just replacing the implementation, i. e. not adding new output. But you are right - I'd much rather see this printed for example as part of the "bdinfo" command than with the regular boot messages. 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 "I may be synthetic, but I'm not stupid" - the artificial person, from _Aliens_ ^ permalink raw reply [flat|nested] 18+ messages in thread
* [U-Boot] [PATCH 2/2] powerpc/85xx: standardize display of address map size (32-bit vs. 36-bit) 2011-09-01 14:22 ` Wolfgang Denk @ 2011-09-01 14:54 ` Timur Tabi 2011-09-01 21:59 ` Wolfgang Denk 0 siblings, 1 reply; 18+ messages in thread From: Timur Tabi @ 2011-09-01 14:54 UTC (permalink / raw) To: u-boot Wolfgang Denk wrote: > But you are right - I'd much rather see this printed for example as > part of the "bdinfo" command than with the regular boot messages. Recently, we've been adding boards that have CONFIG_PCI_SCAN_SHOW defined. Are you saying that we should not be doing that? That adds a lot more text than my patch does. Examples: http://patchwork.ozlabs.org/patch/108682/ http://patchwork.ozlabs.org/patch/108533/ Kumar, are you okay with not displaying the address map size at all during boot time? If we bury this information in the 'bdinfo' command, we're going to have even more confusion as to which U-Boot we're booting. -- Timur Tabi Linux kernel developer at Freescale ^ permalink raw reply [flat|nested] 18+ messages in thread
* [U-Boot] [PATCH 2/2] powerpc/85xx: standardize display of address map size (32-bit vs. 36-bit) 2011-09-01 14:54 ` Timur Tabi @ 2011-09-01 21:59 ` Wolfgang Denk 2011-09-01 22:09 ` Timur Tabi 2011-09-02 3:38 ` Kumar Gala 0 siblings, 2 replies; 18+ messages in thread From: Wolfgang Denk @ 2011-09-01 21:59 UTC (permalink / raw) To: u-boot Dear Timur Tabi, In message <4E5F9C8D.3080503@freescale.com> you wrote: > Wolfgang Denk wrote: > > But you are right - I'd much rather see this printed for example as > > part of the "bdinfo" command than with the regular boot messages. > > Recently, we've been adding boards that have CONFIG_PCI_SCAN_SHOW defined. Are > you saying that we should not be doing that? That adds a lot more text than my > patch does. Yes, that's what I'm sayin. All this "useful" information should be available easily to everybody who is interested in it - no boubt of that. But it shoudld NOT be printed on each and every boot. > Kumar, are you okay with not displaying the address map size at all during boot > time? If we bury this information in the 'bdinfo' command, we're going to have > even more confusion as to which U-Boot we're booting. What sort of "confusion" do you have? I see two situations: in 99.99% of all cases U-Boot is just a means to boot an OS, and nobody cares a bit about the actual U-Boot output, as long as the OS is runnign after a few seconds, and rather sooner than later. For a few use cases where developers are working on a board, they might wonder which state a board is in - then itis very useful to have a command to display this information any time they are interested in it - even without having to reboot the system. It is actually pretty silly to print certain interesting information only at boot time. 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 For every complex problem, there is a solution that is simple, neat, and wrong. - Mark Twain ^ permalink raw reply [flat|nested] 18+ messages in thread
* [U-Boot] [PATCH 2/2] powerpc/85xx: standardize display of address map size (32-bit vs. 36-bit) 2011-09-01 21:59 ` Wolfgang Denk @ 2011-09-01 22:09 ` Timur Tabi 2011-09-05 14:11 ` Wolfgang Denk 2011-09-02 3:38 ` Kumar Gala 1 sibling, 1 reply; 18+ messages in thread From: Timur Tabi @ 2011-09-01 22:09 UTC (permalink / raw) To: u-boot Wolfgang Denk wrote: > What sort of "confusion" do you have? I see two situations: in > 99.99% of all cases U-Boot is just a means to boot an OS, and nobody > cares a bit about the actual U-Boot output, as long as the OS is > runnign after a few seconds, and rather sooner than later. The confusion stems mostly from the fact that we've been displaying this information for years. If suddenly we stop displaying it, people are going to think that something is wrong. Also, you need to know which U-Boot you're running before you boot the kernel, because the device tree needs to match. No one is going to think to use the 'bdinfo' command to find it. Many board configurations come in 32-bit and 36-bit versions. E.g. make P1022DS_config vs. make P1022DS_36BIT_config The problem is that this isn't standard. "make P1022DS" builds a 32-bit U-Boot. But "make P4080DS" builds a 36-bit U-Boot. So we've been displaying the address map size at boot time in some situations, but not all. My patch makes this standard. > For a few > use cases where developers are working on a board, they might wonder > which state a board is in - then itis very useful to have a command to > display this information any time they are interested in it - even > without having to reboot the system. I agree with adding that information to the bdinfo command. I'm just worried about having it available only there. -- Timur Tabi Linux kernel developer at Freescale ^ permalink raw reply [flat|nested] 18+ messages in thread
* [U-Boot] [PATCH 2/2] powerpc/85xx: standardize display of address map size (32-bit vs. 36-bit) 2011-09-01 22:09 ` Timur Tabi @ 2011-09-05 14:11 ` Wolfgang Denk 0 siblings, 0 replies; 18+ messages in thread From: Wolfgang Denk @ 2011-09-05 14:11 UTC (permalink / raw) To: u-boot Dear Timur Tabi, In message <4E6002B4.90503@freescale.com> you wrote: > > > What sort of "confusion" do you have? I see two situations: in > > 99.99% of all cases U-Boot is just a means to boot an OS, and nobody > > cares a bit about the actual U-Boot output, as long as the OS is > > runnign after a few seconds, and rather sooner than later. > > The confusion stems mostly from the fact that we've been displaying this > information for years. If suddenly we stop displaying it, people are going to > think that something is wrong. Also, you need to know which U-Boot you're > running before you boot the kernel, because the device tree needs to match. No > one is going to think to use the 'bdinfo' command to find it. Software changes, output changes. Document your changes if you want to make sure your users happy. Feel free to send them detailled changed logs. > The problem is that this isn't standard. "make P1022DS" builds a 32-bit U-Boot. > But "make P4080DS" builds a 36-bit U-Boot. So we've been displaying the > address map size at boot time in some situations, but not all. My patch makes > this standard. But it's not you who defines what the standard looks like. > I agree with adding that information to the bdinfo command. I'm just worried > about having it available only there. You will get used to it, and so will your users. 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 Being schizophrenic is better than living alone. ^ permalink raw reply [flat|nested] 18+ messages in thread
* [U-Boot] [PATCH 2/2] powerpc/85xx: standardize display of address map size (32-bit vs. 36-bit) 2011-09-01 21:59 ` Wolfgang Denk 2011-09-01 22:09 ` Timur Tabi @ 2011-09-02 3:38 ` Kumar Gala 2011-09-02 11:36 ` Tabi Timur-B04825 2011-09-05 14:14 ` Wolfgang Denk 1 sibling, 2 replies; 18+ messages in thread From: Kumar Gala @ 2011-09-02 3:38 UTC (permalink / raw) To: u-boot On Sep 1, 2011, at 4:59 PM, Wolfgang Denk wrote: > Dear Timur Tabi, > > In message <4E5F9C8D.3080503@freescale.com> you wrote: >> Wolfgang Denk wrote: >>> But you are right - I'd much rather see this printed for example as >>> part of the "bdinfo" command than with the regular boot messages. >> >> Recently, we've been adding boards that have CONFIG_PCI_SCAN_SHOW defined. Are >> you saying that we should not be doing that? That adds a lot more text than my >> patch does. > > Yes, that's what I'm sayin. All this "useful" information should be > available easily to everybody who is interested in it - no boubt of > that. But it shoudld NOT be printed on each and every boot. > >> Kumar, are you okay with not displaying the address map size at all during boot >> time? If we bury this information in the 'bdinfo' command, we're going to have >> even more confusion as to which U-Boot we're booting. > > What sort of "confusion" do you have? I see two situations: in > 99.99% of all cases U-Boot is just a means to boot an OS, and nobody > cares a bit about the actual U-Boot output, as long as the OS is > runnign after a few seconds, and rather sooner than later. For a few > use cases where developers are working on a board, they might wonder > which state a board is in - then itis very useful to have a command to > display this information any time they are interested in it - even > without having to reboot the system. > > It is actually pretty silly to print certain interesting information > only at boot time. I agree with this, however I think it should be left to the board config/port to decide if it wants to print and enable this info on boot. The FSL PPC board ports are not the typical use case. I view our board ports having a few different purposes: * Examples for people building their own boards * Used for customer evaluation and support Having the additional information printed on boot is extremely useful to FSL for our board ports for supporting customers. So my take is the following: 1. We reduce the amount printed in the "default" case 2. First choice should always be to have a command the print status info 3. Allow a board port to makes its own decision if it wants to do something like enable CONFIG_PCI_SCAN_SHOW Any concerns w/that proposal? - k ^ permalink raw reply [flat|nested] 18+ messages in thread
* [U-Boot] [PATCH 2/2] powerpc/85xx: standardize display of address map size (32-bit vs. 36-bit) 2011-09-02 3:38 ` Kumar Gala @ 2011-09-02 11:36 ` Tabi Timur-B04825 2011-09-02 13:12 ` Kumar Gala 2011-09-05 14:14 ` Wolfgang Denk 1 sibling, 1 reply; 18+ messages in thread From: Tabi Timur-B04825 @ 2011-09-02 11:36 UTC (permalink / raw) To: u-boot Kumar Gala wrote: > 1. We reduce the amount printed in the "default" case > 2. First choice should always be to have a command the print status info > 3. Allow a board port to makes its own decision if it wants to do something like enable CONFIG_PCI_SCAN_SHOW > > Any concerns w/that proposal? Are you talking about my patch, or about CONFIG_PCI_SCAN_SHOW? The reason I like my patch as-is is because it completely eliminates the board from the decision. It's only one line, and it's CONSISTENT. That's the key part. We have not been consistent about this, and it seems silly for each board to implement the same feature. -- Timur Tabi Linux kernel developer at Freescale ^ permalink raw reply [flat|nested] 18+ messages in thread
* [U-Boot] [PATCH 2/2] powerpc/85xx: standardize display of address map size (32-bit vs. 36-bit) 2011-09-02 11:36 ` Tabi Timur-B04825 @ 2011-09-02 13:12 ` Kumar Gala 2011-09-02 13:41 ` Timur Tabi 0 siblings, 1 reply; 18+ messages in thread From: Kumar Gala @ 2011-09-02 13:12 UTC (permalink / raw) To: u-boot On Sep 2, 2011, at 6:36 AM, Tabi Timur-B04825 wrote: > Kumar Gala wrote: >> 1. We reduce the amount printed in the "default" case >> 2. First choice should always be to have a command the print status info >> 3. Allow a board port to makes its own decision if it wants to do something like enable CONFIG_PCI_SCAN_SHOW >> >> Any concerns w/that proposal? > > Are you talking about my patch, or about CONFIG_PCI_SCAN_SHOW? > > The reason I like my patch as-is is because it completely eliminates the > board from the decision. It's only one line, and it's CONSISTENT. That's > the key part. We have not been consistent about this, and it seems silly > for each board to implement the same feature. Both. I'm think for your patch we'd add some general config option for extra print info. - k ^ permalink raw reply [flat|nested] 18+ messages in thread
* [U-Boot] [PATCH 2/2] powerpc/85xx: standardize display of address map size (32-bit vs. 36-bit) 2011-09-02 13:12 ` Kumar Gala @ 2011-09-02 13:41 ` Timur Tabi 2011-09-02 18:24 ` Kumar Gala 2011-09-05 14:15 ` Wolfgang Denk 0 siblings, 2 replies; 18+ messages in thread From: Timur Tabi @ 2011-09-02 13:41 UTC (permalink / raw) To: u-boot Kumar Gala wrote: > Both. I'm think for your patch we'd add some general config option for extra print info. So you want to see this instead: /* * Display whether this is a 32-bit build or a 36-bit build. */ #ifdef CONFIG_DISPLAY_ADDR_SIZE #ifdef CONFIG_PHYS_64BIT puts("ADDR: 36-bit address map\n"); #else puts("ADDR: 32-bit address map\n"); #endif #endif I still like my way better. It eliminates the need to think about another CONFIG option. I think adding another CONFIG option is worse than adding another line of text. It think it's silly to complain about adding one line of text that is only displayed on e500 systems that support 36-bit addressing, especially since we display this information on most of our boards anyway. Surely we can find some other line of text that we can shorten or eliminate to make up for it. For instance, we can combine these two lines into one: CPU0: P1022E, Version: 1.0, (0x80ee0010) Core: E500, Version: 5.0, (0x80211050) or these two lines: L1: D-cache 32 kB enabled I-cache 32 kB enabled -- Timur Tabi Linux kernel developer at Freescale ^ permalink raw reply [flat|nested] 18+ messages in thread
* [U-Boot] [PATCH 2/2] powerpc/85xx: standardize display of address map size (32-bit vs. 36-bit) 2011-09-02 13:41 ` Timur Tabi @ 2011-09-02 18:24 ` Kumar Gala 2011-09-02 18:25 ` Timur Tabi 2011-09-02 22:10 ` Wolfgang Denk 2011-09-05 14:15 ` Wolfgang Denk 1 sibling, 2 replies; 18+ messages in thread From: Kumar Gala @ 2011-09-02 18:24 UTC (permalink / raw) To: u-boot On Sep 2, 2011, at 8:41 AM, Timur Tabi wrote: > Kumar Gala wrote: >> Both. I'm think for your patch we'd add some general config option for extra print info. > > So you want to see this instead: > > /* > * Display whether this is a 32-bit build or a 36-bit build. > */ > #ifdef CONFIG_DISPLAY_ADDR_SIZE > #ifdef CONFIG_PHYS_64BIT > puts("ADDR: 36-bit address map\n"); > #else > puts("ADDR: 32-bit address map\n"); > #endif > #endif > > I still like my way better. It eliminates the need to think about another > CONFIG option. I think adding another CONFIG option is worse than adding > another line of text. I think we could introduce kernel style "printk" levels that would allow more control over something like this. - k ^ permalink raw reply [flat|nested] 18+ messages in thread
* [U-Boot] [PATCH 2/2] powerpc/85xx: standardize display of address map size (32-bit vs. 36-bit) 2011-09-02 18:24 ` Kumar Gala @ 2011-09-02 18:25 ` Timur Tabi 2011-09-02 22:10 ` Wolfgang Denk 1 sibling, 0 replies; 18+ messages in thread From: Timur Tabi @ 2011-09-02 18:25 UTC (permalink / raw) To: u-boot Kumar Gala wrote: > I think we could introduce kernel style "printk" levels that would allow more control over something like this. > Or we could implement Kconfig and defconfigs. But neither of these options is going to help me now. -- Timur Tabi Linux kernel developer at Freescale ^ permalink raw reply [flat|nested] 18+ messages in thread
* [U-Boot] [PATCH 2/2] powerpc/85xx: standardize display of address map size (32-bit vs. 36-bit) 2011-09-02 18:24 ` Kumar Gala 2011-09-02 18:25 ` Timur Tabi @ 2011-09-02 22:10 ` Wolfgang Denk 1 sibling, 0 replies; 18+ messages in thread From: Wolfgang Denk @ 2011-09-02 22:10 UTC (permalink / raw) To: u-boot Dear Kumar Gala, In message <A324986C-C4BF-459A-83AC-2B753FED7A1F@kernel.crashing.org> you wrote: > > I think we could introduce kernel style "printk" levels that would allow = > more control over something like this. We can invent many things, or we can keep the code lean and simple. Let's just move this to where it belongs - to some command that prints that information upon explicit request of the user. "bdinfo" comes to mind here. Feel free to run such a command as "preboot" sequence then. 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 The inappropriate cannot be beautiful. - Frank Lloyd Wright _The Future of Architecture_ (1953) ^ permalink raw reply [flat|nested] 18+ messages in thread
* [U-Boot] [PATCH 2/2] powerpc/85xx: standardize display of address map size (32-bit vs. 36-bit) 2011-09-02 13:41 ` Timur Tabi 2011-09-02 18:24 ` Kumar Gala @ 2011-09-05 14:15 ` Wolfgang Denk 1 sibling, 0 replies; 18+ messages in thread From: Wolfgang Denk @ 2011-09-05 14:15 UTC (permalink / raw) To: u-boot Dear Timur Tabi, In message <4E60DCEC.9090205@freescale.com> you wrote: > Kumar Gala wrote: > > Both. I'm think for your patch we'd add some general config option for extra print info. > > So you want to see this instead: > > /* > * Display whether this is a 32-bit build or a 36-bit build. > */ > #ifdef CONFIG_DISPLAY_ADDR_SIZE > #ifdef CONFIG_PHYS_64BIT > puts("ADDR: 36-bit address map\n"); > #else > puts("ADDR: 32-bit address map\n"); > #endif > #endif Please dump this completely. 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 "A complex system that works is invariably found to have evolved from a simple system that worked." - John Gall, _Systemantics_ ^ permalink raw reply [flat|nested] 18+ messages in thread
* [U-Boot] [PATCH 2/2] powerpc/85xx: standardize display of address map size (32-bit vs. 36-bit) 2011-09-02 3:38 ` Kumar Gala 2011-09-02 11:36 ` Tabi Timur-B04825 @ 2011-09-05 14:14 ` Wolfgang Denk 1 sibling, 0 replies; 18+ messages in thread From: Wolfgang Denk @ 2011-09-05 14:14 UTC (permalink / raw) To: u-boot Dear Kumar Gala, In message <6A841E78-5D44-4190-9899-1976234EF955@freescale.com> you wrote: > > The FSL PPC board ports are not the typical use case. I view our board > ports having a few different purposes: > * Examples for people building their own boards Especially of this you have additional responsibility not to set bad examples. > Having the additional information printed on boot is extremely useful to > FSL for our board ports for supporting customers. So my take is the "extremely useful" is a really big statement. I don't buy it. > Any concerns w/that proposal? I want to clean this up, and I'm extremely unhappy if there are "examples for people building their own boards" that use excessively verbose boot messages. 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 In my experience the best way to get something done is to give it to someone who is busy. - Terry Pratchett, _Going_Postal_ ^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2011-09-05 14:15 UTC | newest] Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2011-08-31 22:15 [U-Boot] [PATCH 1/2] powerpc/85xx: CONFIG_ENABLE_36BIT_PHYS does not depend on CONFIG_PHYS_64BIT Timur Tabi 2011-08-31 22:15 ` [U-Boot] [PATCH 2/2] powerpc/85xx: standardize display of address map size (32-bit vs. 36-bit) Timur Tabi 2011-09-01 14:07 ` Kumar Gala 2011-09-01 14:13 ` Timur Tabi 2011-09-01 14:22 ` Wolfgang Denk 2011-09-01 14:54 ` Timur Tabi 2011-09-01 21:59 ` Wolfgang Denk 2011-09-01 22:09 ` Timur Tabi 2011-09-05 14:11 ` Wolfgang Denk 2011-09-02 3:38 ` Kumar Gala 2011-09-02 11:36 ` Tabi Timur-B04825 2011-09-02 13:12 ` Kumar Gala 2011-09-02 13:41 ` Timur Tabi 2011-09-02 18:24 ` Kumar Gala 2011-09-02 18:25 ` Timur Tabi 2011-09-02 22:10 ` Wolfgang Denk 2011-09-05 14:15 ` Wolfgang Denk 2011-09-05 14:14 ` Wolfgang Denk
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.