Re: [U-Boot] [PATCH v2 2/6] apalis/colibri_t20/t30: deactivate displaying board info

2016-10-10 Thread Stephen Warren

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

2016-10-05 Thread Stefan Agner
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

2016-10-05 Thread Stephen Warren

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

2016-10-03 Thread Stefan Agner
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

2016-10-03 Thread Stephen Warren

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

2016-09-30 Thread Marcel Ziswiler
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

2016-09-28 Thread Stephen Warren

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

2016-09-28 Thread Marcel Ziswiler
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