Re: [U-Boot] [PATCH v2 2/6] apalis/colibri_t20/t30: deactivate displaying board info
On 10/05/2016 10:12 AM, Stefan Agner wrote: On 2016-10-05 08:53, Stephen Warren wrote: On 10/03/2016 02:27 PM, Stefan Agner wrote: On 03.10.2016 10:28, Stephen Warren wrote: On 09/30/2016 04:00 AM, Marcel Ziswiler wrote: On Wed, 2016-09-28 at 12:00 -0600, Stephen Warren wrote: On 09/28/2016 03:35 AM, Marcel Ziswiler wrote: Avoid a checkboard() name clash with our upcoming custom implementation thereof. If you want to avoid naming conflicts, please simply name your new function something that doesn't conflict. That way it will avoid confusion is someone actually wants to enable the CONFIG_DISPLAY_BOARDINFO option themselves, plus it avoids taking the current feature set away. No, it is not just any function. We do want our custom checkboard() to be called upon boot and not the Tegra generic one just printing a hard coded string. I guess alternatively we could gate the checkboard() call in arch/arm/mach-tegra/board2.c with a #if !defined(CONFIG_CUSTOM_BOARDINFO) just as introduced a while ago in common/board_info.c http://git.denx.de/?p=u-boot.git;a=blob;f=common/board_info.c;h=bd5dcfa 066358c2cc44ce5d19fcc3e77d555cd09;hb=HEAD#l20 in order to not print the hard coded name from the device tree. I'd prefer to keep the behaviour standard across all Tegra boards. If you want to do additional actions in the checkboard() function, I suggest making it call an optional additional function: __weak int tegra_checkboard(void) { return 0; } int checkboard(void) { ... return tegra_checkboard(); } Well that would print a message "Board: " ... twice, which is rather strange. Surely you simply make tegra_checkboard() not contain duplicate code? What do you think of my idea? I'd rather not introduce any more ifdefs, but instead have a single path through the code-base. Sorry, I was a bit unclear, with my other idea I meant the answer I sent to patch 3/6 of this patchset: http://lists.denx.de/pipermail/u-boot/2016-October/268669.html It does remove a ifdef... That's probably better than adding more ifdefs. It does have the disadvantage of not using the common show_board_info() or Tegra-wide checkboard() implementations though, which means that anything added there won't execute on all Tegras, which could potentially be confusing. ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v2 2/6] apalis/colibri_t20/t30: deactivate displaying board info
On 2016-10-05 08:53, Stephen Warren wrote: > On 10/03/2016 02:27 PM, Stefan Agner wrote: >> On 03.10.2016 10:28, Stephen Warren wrote: >>> On 09/30/2016 04:00 AM, Marcel Ziswiler wrote: On Wed, 2016-09-28 at 12:00 -0600, Stephen Warren wrote: > On 09/28/2016 03:35 AM, Marcel Ziswiler wrote: >> >> Avoid a checkboard() name clash with our upcoming custom >> implementation >> thereof. > If you want to avoid naming conflicts, please simply name your new > function something that doesn't conflict. That way it will avoid > confusion is someone actually wants to enable the > CONFIG_DISPLAY_BOARDINFO option themselves, plus it avoids taking > the > current feature set away. No, it is not just any function. We do want our custom checkboard() to be called upon boot and not the Tegra generic one just printing a hard coded string. I guess alternatively we could gate the checkboard() call in arch/arm/mach-tegra/board2.c with a #if !defined(CONFIG_CUSTOM_BOARDINFO) just as introduced a while ago in common/board_info.c http://git.denx.de/?p=u-boot.git;a=blob;f=common/board_info.c;h=bd5dcfa 066358c2cc44ce5d19fcc3e77d555cd09;hb=HEAD#l20 in order to not print the hard coded name from the device tree. >>> >>> I'd prefer to keep the behaviour standard across all Tegra boards. If >>> you want to do additional actions in the checkboard() function, I >>> suggest making it call an optional additional function: >>> >>> __weak int tegra_checkboard(void) >>> { >>> return 0; >>> } >>> >>> int checkboard(void) >>> { >>> ... >>> return tegra_checkboard(); >>> } >> >> Well that would print a message "Board: " ... twice, which is rather >> strange. > > Surely you simply make tegra_checkboard() not contain duplicate code? > >> What do you think of my idea? > > I'd rather not introduce any more ifdefs, but instead have a single > path through the code-base. Sorry, I was a bit unclear, with my other idea I meant the answer I sent to patch 3/6 of this patchset: http://lists.denx.de/pipermail/u-boot/2016-October/268669.html It does remove a ifdef... -- Stefan ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v2 2/6] apalis/colibri_t20/t30: deactivate displaying board info
On 10/03/2016 02:27 PM, Stefan Agner wrote: On 03.10.2016 10:28, Stephen Warren wrote: On 09/30/2016 04:00 AM, Marcel Ziswiler wrote: On Wed, 2016-09-28 at 12:00 -0600, Stephen Warren wrote: On 09/28/2016 03:35 AM, Marcel Ziswiler wrote: Avoid a checkboard() name clash with our upcoming custom implementation thereof. If you want to avoid naming conflicts, please simply name your new function something that doesn't conflict. That way it will avoid confusion is someone actually wants to enable the CONFIG_DISPLAY_BOARDINFO option themselves, plus it avoids taking the current feature set away. No, it is not just any function. We do want our custom checkboard() to be called upon boot and not the Tegra generic one just printing a hard coded string. I guess alternatively we could gate the checkboard() call in arch/arm/mach-tegra/board2.c with a #if !defined(CONFIG_CUSTOM_BOARDINFO) just as introduced a while ago in common/board_info.c http://git.denx.de/?p=u-boot.git;a=blob;f=common/board_info.c;h=bd5dcfa 066358c2cc44ce5d19fcc3e77d555cd09;hb=HEAD#l20 in order to not print the hard coded name from the device tree. I'd prefer to keep the behaviour standard across all Tegra boards. If you want to do additional actions in the checkboard() function, I suggest making it call an optional additional function: __weak int tegra_checkboard(void) { return 0; } int checkboard(void) { ... return tegra_checkboard(); } > Well that would print a message "Board: " ... twice, which is rather strange. Surely you simply make tegra_checkboard() not contain duplicate code? What do you think of my idea? I'd rather not introduce any more ifdefs, but instead have a single path through the code-base. ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v2 2/6] apalis/colibri_t20/t30: deactivate displaying board info
On 03.10.2016 10:28, Stephen Warren wrote: > On 09/30/2016 04:00 AM, Marcel Ziswiler wrote: >> On Wed, 2016-09-28 at 12:00 -0600, Stephen Warren wrote: >>> On 09/28/2016 03:35 AM, Marcel Ziswiler wrote: Avoid a checkboard() name clash with our upcoming custom implementation thereof. >>> If you want to avoid naming conflicts, please simply name your new >>> function something that doesn't conflict. That way it will avoid >>> confusion is someone actually wants to enable the >>> CONFIG_DISPLAY_BOARDINFO option themselves, plus it avoids taking >>> the >>> current feature set away. >> >> No, it is not just any function. We do want our custom checkboard() to >> be called upon boot and not the Tegra generic one just printing a hard >> coded string. >> >> I guess alternatively we could gate the checkboard() call >> in arch/arm/mach-tegra/board2.c with a >> >> #if !defined(CONFIG_CUSTOM_BOARDINFO) >> >> just as introduced a while ago in common/board_info.c >> >> http://git.denx.de/?p=u-boot.git;a=blob;f=common/board_info.c;h=bd5dcfa >> 066358c2cc44ce5d19fcc3e77d555cd09;hb=HEAD#l20 >> >> in order to not print the hard coded name from the device tree. > > I'd prefer to keep the behaviour standard across all Tegra boards. If > you want to do additional actions in the checkboard() function, I > suggest making it call an optional additional function: > > __weak int tegra_checkboard(void) > { > return 0; > } > > int checkboard(void) > { > ... > return tegra_checkboard(); > } Well that would print a message "Board: " ... twice, which is rather strange. What do you think of my idea? -- Stefan ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v2 2/6] apalis/colibri_t20/t30: deactivate displaying board info
On 09/30/2016 04:00 AM, Marcel Ziswiler wrote: On Wed, 2016-09-28 at 12:00 -0600, Stephen Warren wrote: On 09/28/2016 03:35 AM, Marcel Ziswiler wrote: Avoid a checkboard() name clash with our upcoming custom implementation thereof. If you want to avoid naming conflicts, please simply name your new function something that doesn't conflict. That way it will avoid confusion is someone actually wants to enable the CONFIG_DISPLAY_BOARDINFO option themselves, plus it avoids taking the current feature set away. No, it is not just any function. We do want our custom checkboard() to be called upon boot and not the Tegra generic one just printing a hard coded string. I guess alternatively we could gate the checkboard() call in arch/arm/mach-tegra/board2.c with a #if !defined(CONFIG_CUSTOM_BOARDINFO) just as introduced a while ago in common/board_info.c http://git.denx.de/?p=u-boot.git;a=blob;f=common/board_info.c;h=bd5dcfa 066358c2cc44ce5d19fcc3e77d555cd09;hb=HEAD#l20 in order to not print the hard coded name from the device tree. I'd prefer to keep the behaviour standard across all Tegra boards. If you want to do additional actions in the checkboard() function, I suggest making it call an optional additional function: __weak int tegra_checkboard(void) { return 0; } int checkboard(void) { ... return tegra_checkboard(); } ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v2 2/6] apalis/colibri_t20/t30: deactivate displaying board info
On Wed, 2016-09-28 at 12:00 -0600, Stephen Warren wrote: > On 09/28/2016 03:35 AM, Marcel Ziswiler wrote: > > > > Avoid a checkboard() name clash with our upcoming custom > > implementation > > thereof. > If you want to avoid naming conflicts, please simply name your new > function something that doesn't conflict. That way it will avoid > confusion is someone actually wants to enable the > CONFIG_DISPLAY_BOARDINFO option themselves, plus it avoids taking > the > current feature set away. No, it is not just any function. We do want our custom checkboard() to be called upon boot and not the Tegra generic one just printing a hard coded string. I guess alternatively we could gate the checkboard() call in arch/arm/mach-tegra/board2.c with a #if !defined(CONFIG_CUSTOM_BOARDINFO) just as introduced a while ago in common/board_info.c http://git.denx.de/?p=u-boot.git;a=blob;f=common/board_info.c;h=bd5dcfa 066358c2cc44ce5d19fcc3e77d555cd09;hb=HEAD#l20 in order to not print the hard coded name from the device tree. ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v2 2/6] apalis/colibri_t20/t30: deactivate displaying board info
On 09/28/2016 03:35 AM, Marcel Ziswiler wrote: Avoid a checkboard() name clash with our upcoming custom implementation thereof. If you want to avoid naming conflicts, please simply name your new function something that doesn't conflict. That way it will avoid confusion is someone actually wants to enable the CONFIG_DISPLAY_BOARDINFO option themselves, plus it avoids taking the current feature set away. ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [PATCH v2 2/6] apalis/colibri_t20/t30: deactivate displaying board info
Avoid a checkboard() name clash with our upcoming custom implementation thereof. Signed-off-by: Marcel Ziswiler --- Changes in v2: None include/configs/apalis_t30.h | 1 + include/configs/colibri_t20.h | 1 + include/configs/colibri_t30.h | 1 + 3 files changed, 3 insertions(+) diff --git a/include/configs/apalis_t30.h b/include/configs/apalis_t30.h index 3fc1779..dc17733 100644 --- a/include/configs/apalis_t30.h +++ b/include/configs/apalis_t30.h @@ -17,6 +17,7 @@ /* High-level configuration options */ #define CONFIG_TEGRA_BOARD_STRING "Toradex Apalis T30" +#undef CONFIG_DISPLAY_BOARDINFO/* As defined in tegra-common */ /* Board-specific serial config */ #define CONFIG_TEGRA_ENABLE_UARTA diff --git a/include/configs/colibri_t20.h b/include/configs/colibri_t20.h index b299e15..431fec0 100644 --- a/include/configs/colibri_t20.h +++ b/include/configs/colibri_t20.h @@ -15,6 +15,7 @@ /* High-level configuration options */ #define CONFIG_TEGRA_BOARD_STRING "Toradex Colibri T20" +#undef CONFIG_DISPLAY_BOARDINFO/* As defined in tegra-common */ /* Board-specific serial config */ #define CONFIG_TEGRA_ENABLE_UARTA diff --git a/include/configs/colibri_t30.h b/include/configs/colibri_t30.h index e2a2549..d83c5a0 100644 --- a/include/configs/colibri_t30.h +++ b/include/configs/colibri_t30.h @@ -17,6 +17,7 @@ /* High-level configuration options */ #define CONFIG_TEGRA_BOARD_STRING "Toradex Colibri T30" +#undef CONFIG_DISPLAY_BOARDINFO/* As defined in tegra-common */ /* Board-specific serial config */ #define CONFIG_TEGRA_ENABLE_UARTA -- 2.5.5 ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot