Message ID | 1303296425-14806-1-git-send-email-jason.hui@linaro.org |
---|---|
State | Changes Requested |
Headers | show |
On 04/20/2011 12:47 PM, Jason Liu wrote: > The boot cause code has been factor out to soc common > code,we need drop the part from the board support code > > Signed-off-by: Jason Liu <jason.hui@linaro.org> Hi Jason, > --- > board/efikamx/efikamx.c | 30 ++++++------------------------ > board/freescale/mx51evk/mx51evk.c | 26 ++++++-------------------- > board/freescale/mx53evk/mx53evk.c | 21 +-------------------- > board/ttcontrol/vision2/vision2.c | 28 ++++++---------------------- > 4 files changed, 19 insertions(+), 86 deletions(-) > > diff --git a/board/efikamx/efikamx.c b/board/efikamx/efikamx.c > index f735260..0aef654 100644 > --- a/board/efikamx/efikamx.c > +++ b/board/efikamx/efikamx.c > @@ -644,46 +644,28 @@ int board_late_init(void) > int checkboard(void) > { > u32 system_rev = get_cpu_rev(); > - u32 cause; > - struct src *src_regs = (struct src *)SRC_BASE_ADDR; This seems to me not the best solution. If we have now factored out code to print the reset cause and the silicon version (inside print_cpuinfo), why do we need to repeat this code for each board ? Calling get_cpu_rev seems to me redundant (then each board should only set CONFIG_DISPLAY_CPUINFO). And then the CPU revision is printed again, and this is redundant. > > puts("Board: Efika MX "); > > switch (system_rev & 0xff) { As I see in code, in system_rev & 0xff we can find the cpu revision, and the output is already part of print_cpuinfo. I think we need more clean up, removing all part related to CPU revision and leaving (if any) only the output related to the board revision. Best regards, Stefano Babic
Hi, Stefano, On Fri, Apr 22, 2011 at 1:18 AM, Stefano Babic <sbabic@denx.de> wrote: > On 04/20/2011 12:47 PM, Jason Liu wrote: >> The boot cause code has been factor out to soc common >> code,we need drop the part from the board support code >> >> Signed-off-by: Jason Liu <jason.hui@linaro.org> > > Hi Jason, > >> --- >> board/efikamx/efikamx.c | 30 ++++++------------------------ >> board/freescale/mx51evk/mx51evk.c | 26 ++++++-------------------- >> board/freescale/mx53evk/mx53evk.c | 21 +-------------------- >> board/ttcontrol/vision2/vision2.c | 28 ++++++---------------------- >> 4 files changed, 19 insertions(+), 86 deletions(-) >> >> diff --git a/board/efikamx/efikamx.c b/board/efikamx/efikamx.c >> index f735260..0aef654 100644 >> --- a/board/efikamx/efikamx.c >> +++ b/board/efikamx/efikamx.c >> @@ -644,46 +644,28 @@ int board_late_init(void) >> int checkboard(void) >> { >> u32 system_rev = get_cpu_rev(); >> - u32 cause; >> - struct src *src_regs = (struct src *)SRC_BASE_ADDR; > > This seems to me not the best solution. If we have now factored out code > to print the reset cause and the silicon version (inside print_cpuinfo), > why do we need to repeat this code for each board ? Calling get_cpu_rev > seems to me redundant (then each board should only set > CONFIG_DISPLAY_CPUINFO). And then the CPU revision is printed again, and > this is redundant. The purpose for this patch is to remove the boot cause code and and don't change any cpu rev code. The cpu rev part of code is as it is as before. > >> >> puts("Board: Efika MX "); >> >> switch (system_rev & 0xff) { > > As I see in code, in system_rev & 0xff we can find the cpu revision, and > the output is already part of print_cpuinfo. Ditto, as I only remove the boot cause part of code as the patch tile said. > > I think we need more clean up, removing all part related to CPU revision > and leaving (if any) only the output related to the board revision. If that, I need change the patch tile, and include more clean up in the patch and send again. Jason > > Best regards, > Stefano Babic > > -- > ===================================================================== > DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel > HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany > Phone: +49-8142-66989-0 Fax: +49-8142-66989-80 Email: office@denx.de > ===================================================================== >
On 04/22/2011 07:45 AM, Jason Hui wrote: > Hi, Stefano, Hi Jason, >>> int checkboard(void) >>> { >>> u32 system_rev = get_cpu_rev(); >>> - u32 cause; >>> - struct src *src_regs = (struct src *)SRC_BASE_ADDR; >> >> This seems to me not the best solution. If we have now factored out code >> to print the reset cause and the silicon version (inside print_cpuinfo), >> why do we need to repeat this code for each board ? Calling get_cpu_rev >> seems to me redundant (then each board should only set >> CONFIG_DISPLAY_CPUINFO). And then the CPU revision is printed again, and >> this is redundant. > > The purpose for this patch is to remove the boot cause code and and don't change > any cpu rev code. The cpu rev part of code is as it is as before. However, it seems to me a half-way clean up. As we have already factorize function for printing the cpu revision and the reset cause, I will see in the checkboard only board related information. If there is no revision number for board, printing only the board name as you make for the LOCO is correct. Taking as example the efika board (all boards make the same): > >> >>> >>> puts("Board: Efika MX "); >>> >>> switch (system_rev & 0xff) { The only new information is the board name. If I am not wrong, system_rev & 0xff contains only the cpu revision, and the switch prints out the silicon version. Everything already done in print_cpuinfo as well. > > Ditto, as I only remove the boot cause part of code as the patch tile said. Yes, but this it is redundant with the print_cpuinfo(). As you plan to clean up the code, this part should be cleaned up as well. Or do you think there is something not covered by the common code ? >> I think we need more clean up, removing all part related to CPU revision >> and leaving (if any) only the output related to the board revision. > > If that, I need change the patch tile, and include more clean up in the patch > and send again. Agree. Best regards, Stefano Babic
Hi, Stefano, 2011/4/22 Stefano Babic <sbabic@denx.de>: > On 04/22/2011 07:45 AM, Jason Hui wrote: >> Hi, Stefano, > > Hi Jason, > >>>> int checkboard(void) >>>> { >>>> u32 system_rev = get_cpu_rev(); >>>> - u32 cause; >>>> - struct src *src_regs = (struct src *)SRC_BASE_ADDR; >>> >>> This seems to me not the best solution. If we have now factored out code >>> to print the reset cause and the silicon version (inside print_cpuinfo), >>> why do we need to repeat this code for each board ? Calling get_cpu_rev >>> seems to me redundant (then each board should only set >>> CONFIG_DISPLAY_CPUINFO). And then the CPU revision is printed again, and >>> this is redundant. >> >> The purpose for this patch is to remove the boot cause code and and don't change >> any cpu rev code. The cpu rev part of code is as it is as before. > > However, it seems to me a half-way clean up. As we have already > factorize function for printing the cpu revision and the reset cause, I > will see in the checkboard only board related information. If there is > no revision number for board, printing only the board name as you make > for the LOCO is correct. > > Taking as example the efika board (all boards make the same): > >> >>> >>>> >>>> puts("Board: Efika MX "); >>>> >>>> switch (system_rev & 0xff) { > > The only new information is the board name. If I am not wrong, > system_rev & 0xff contains only the cpu revision, and the switch prints > out the silicon version. Everything already done in print_cpuinfo as well. > >> >> Ditto, as I only remove the boot cause part of code as the patch tile said. > > Yes, but this it is redundant with the print_cpuinfo(). As you plan to > clean up the code, this part should be cleaned up as well. Or do you > think there is something not covered by the common code ? > >>> I think we need more clean up, removing all part related to CPU revision >>> and leaving (if any) only the output related to the board revision. >> >> If that, I need change the patch tile, and include more clean up in the patch >> and send again. > > Agree. OK, I will re-send the patch with more clean-up. Jason > > Best regards, > Stefano Babic > > -- > ===================================================================== > DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel > HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany > Phone: +49-8142-66989-0 Fax: +49-8142-66989-80 Email: office@denx.de > ===================================================================== > _______________________________________________ > U-Boot mailing list > U-Boot@lists.denx.de > http://lists.denx.de/mailman/listinfo/u-boot >
diff --git a/board/efikamx/efikamx.c b/board/efikamx/efikamx.c index f735260..0aef654 100644 --- a/board/efikamx/efikamx.c +++ b/board/efikamx/efikamx.c @@ -644,46 +644,28 @@ int board_late_init(void) int checkboard(void) { u32 system_rev = get_cpu_rev(); - u32 cause; - struct src *src_regs = (struct src *)SRC_BASE_ADDR; puts("Board: Efika MX "); switch (system_rev & 0xff) { case CHIP_REV_3_0: - puts("3.0 ["); + puts("3.0"); break; case CHIP_REV_2_5: - puts("2.5 ["); + puts("2.5"); break; case CHIP_REV_2_0: - puts("2.0 ["); + puts("2.0"); break; case CHIP_REV_1_1: - puts("1.1 ["); + puts("1.1"); break; case CHIP_REV_1_0: default: - puts("1.0 ["); + puts("1.0"); break; } - cause = src_regs->srsr; - switch (cause) { - case 0x0001: - puts("POR"); - break; - case 0x0009: - puts("RST"); - break; - case 0x0010: - case 0x0011: - puts("WDOG"); - break; - default: - printf("unknown 0x%x", cause); - } - puts("]\n"); - + puts("\n"); return 0; } diff --git a/board/freescale/mx51evk/mx51evk.c b/board/freescale/mx51evk/mx51evk.c index 02a765d..cff2ff1 100644 --- a/board/freescale/mx51evk/mx51evk.c +++ b/board/freescale/mx51evk/mx51evk.c @@ -435,37 +435,23 @@ int checkboard(void) switch (system_rev & 0xff) { case CHIP_REV_3_0: - puts("3.0 ["); + puts("3.0"); break; case CHIP_REV_2_5: - puts("2.5 ["); + puts("2.5"); break; case CHIP_REV_2_0: - puts("2.0 ["); + puts("2.0"); break; case CHIP_REV_1_1: - puts("1.1 ["); + puts("1.1"); break; case CHIP_REV_1_0: default: - puts("1.0 ["); + puts("1.0"); break; } - switch (__raw_readl(SRC_BASE_ADDR + 0x8)) { - case 0x0001: - puts("POR"); - break; - case 0x0009: - puts("RST"); - break; - case 0x0010: - case 0x0011: - puts("WDOG"); - break; - default: - puts("unknown"); - } - puts("]\n"); + puts("\n"); return 0; } diff --git a/board/freescale/mx53evk/mx53evk.c b/board/freescale/mx53evk/mx53evk.c index e71701b..a89aa25 100644 --- a/board/freescale/mx53evk/mx53evk.c +++ b/board/freescale/mx53evk/mx53evk.c @@ -372,26 +372,7 @@ int board_late_init(void) int checkboard(void) { - u32 cause; - struct src *src_regs = (struct src *)SRC_BASE_ADDR; + puts("Board: MX53EVK\n"); - puts("Board: MX53EVK ["); - - cause = src_regs->srsr; - switch (cause) { - case 0x0001: - printf("POR"); - break; - case 0x0009: - printf("RST"); - break; - case 0x0010: - case 0x0011: - printf("WDOG"); - break; - default: - printf("unknown"); - } - printf("]\n"); return 0; } diff --git a/board/ttcontrol/vision2/vision2.c b/board/ttcontrol/vision2/vision2.c index f8ef4fc..8423110 100644 --- a/board/ttcontrol/vision2/vision2.c +++ b/board/ttcontrol/vision2/vision2.c @@ -708,40 +708,24 @@ int checkboard(void) switch (system_rev & 0xff) { case CHIP_REV_3_0: - puts("3.0 ["); + puts("3.0"); break; case CHIP_REV_2_5: - puts("2.5 ["); + puts("2.5"); break; case CHIP_REV_2_0: - puts("2.0 ["); + puts("2.0"); break; case CHIP_REV_1_1: - puts("1.1 ["); + puts("1.1"); break; case CHIP_REV_1_0: default: - puts("1.0 ["); + puts("1.0"); break; } - cause = src_regs->srsr; - switch (cause) { - case 0x0001: - puts("POR"); - break; - case 0x0009: - puts("RST"); - break; - case 0x0010: - case 0x0011: - puts("WDOG"); - break; - default: - printf("unknown 0x%x", cause); - } - puts("]\n"); - + puts("\n"); return 0; }
The boot cause code has been factor out to soc common code,we need drop the part from the board support code Signed-off-by: Jason Liu <jason.hui@linaro.org> --- board/efikamx/efikamx.c | 30 ++++++------------------------ board/freescale/mx51evk/mx51evk.c | 26 ++++++-------------------- board/freescale/mx53evk/mx53evk.c | 21 +-------------------- board/ttcontrol/vision2/vision2.c | 28 ++++++---------------------- 4 files changed, 19 insertions(+), 86 deletions(-)