[U-Boot] [PATCH v3] arm: dra7xx: Update the EXTRA_ENV_SETTINGS

[U-Boot] [PATCH v3] arm: dra7xx: Update the EXTRA_ENV_SETTINGS

[Dan Murphy]

Update the EXTRA_ENV_SETTING for the dra7xx.
The console needs to be set to ttyO0 and the
findfdt needs to be updated to load the
dra7xx-evm.dtb file.

Signed-off-by: Dan Murphy <dmurphy@ti.com>
---
v3 - Updated based on comments - http://patchwork.ozlabs.org/patch/248687/
v2 - Updated with side bar maintainer comments.
 include/configs/dra7xx_evm.h   |    2 ++
 include/configs/omap5_common.h |    8 ++++++--
 include/configs/omap5_uevm.h   |    1 +
 3 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/include/configs/dra7xx_evm.h b/include/configs/dra7xx_evm.h
index 28a306b..2db0fbd 100644
--- a/include/configs/dra7xx_evm.h
+++ b/include/configs/dra7xx_evm.h
@@ -35,4 +35,6 @@
 #define CONFIG_DRA7XX		/* in a TI DRA7XX core */
 #define CONFIG_SYS_PROMPT		"DRA752 EVM # "
 
+#define CONSOLEDEV		"ttyO0"
+
 #endif /* __CONFIG_DRA7XX_EVM_H */
diff --git a/include/configs/omap5_common.h b/include/configs/omap5_common.h
index deb5e9f..b261176 100644
--- a/include/configs/omap5_common.h
+++ b/include/configs/omap5_common.h
@@ -144,7 +144,7 @@
 
 #define CONFIG_EXTRA_ENV_SETTINGS \
 	"loadaddr=0x82000000\0" \
-	"console=ttyO2,115200n8\0" \
+	"console=" CONSOLEDEV ",115200n8\0" \
 	"fdt_high=0xffffffff\0" \
 	"fdtaddr=0x80f80000\0" \
 	"bootpart=0:2\0" \
@@ -174,7 +174,11 @@
 		"bootz ${loadaddr} - ${fdtaddr}\0" \
 	"findfdt="\
 		"if test $board_name = omap5_uevm; then " \
-			"setenv fdtfile omap5-uevm.dtb; fi;\0 " \
+			"setenv fdtfile omap5-uevm.dtb; fi; " \
+		"if test $board_name = dra7xx; then " \
+			"setenv fdtfile dra7-evm.dtb; fi; " \
+		"if test -z ${fdtfile}; then " \
+			"echo WARNING: Could not determine device tree to use; fi; \0" \
 	"loadfdt=load mmc ${bootpart} ${fdtaddr} ${bootdir}/${fdtfile};\0" \
 
 #define CONFIG_BOOTCOMMAND \
diff --git a/include/configs/omap5_uevm.h b/include/configs/omap5_uevm.h
index 9e0339b..f2cbb02 100644
--- a/include/configs/omap5_uevm.h
+++ b/include/configs/omap5_uevm.h
@@ -55,6 +55,7 @@
 #define CONFIG_CMD_PART
 
 #define CONFIG_SYS_PROMPT		"OMAP5430 EVM # "
+#define CONSOLEDEV		"ttyO2"
 
 #define CONFIG_OMAP_PLATFORM_RESET_TIME_MAX_USEC	16296
 #endif /* __CONFIG_OMAP5_EVM_H */
-- 
1.7.5.4

[U-Boot] [PATCH v3] arm: dra7xx: Update the EXTRA_ENV_SETTINGS

[Tom Rini]

On Wed, Jun 05, 2013 at 09:39:47AM -0500, Dan Murphy wrote:
> Update the EXTRA_ENV_SETTING for the dra7xx.
> The console needs to be set to ttyO0 and the
> findfdt needs to be updated to load the
> dra7xx-evm.dtb file.
> 
> Signed-off-by: Dan Murphy <dmurphy@ti.com>

Did you run-time test this?  I'm a little concerned about the include
order, but that might turn out alright.  But I think:

> +		"if test -z ${fdtfile}; then " \

Really needs to be:
if test -z "${fdtfile}"; then...

So that it doesn't become:
if test -z; then
when fdtfile isn't set and we get a parse error there.

Thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20130606/500d2738/attachment.pgp>

[U-Boot] [PATCH v3] arm: dra7xx: Update the EXTRA_ENV_SETTINGS

[Dan Murphy]

On 06/06/2013 10:31 AM, Tom Rini wrote:
> On Wed, Jun 05, 2013 at 09:39:47AM -0500, Dan Murphy wrote:
>> Update the EXTRA_ENV_SETTING for the dra7xx.
>> The console needs to be set to ttyO0 and the
>> findfdt needs to be updated to load the
>> dra7xx-evm.dtb file.
>>
>> Signed-off-by: Dan Murphy <dmurphy@ti.com>
> Did you run-time test this?  I'm a little concerned about the include
> order, but that might turn out alright.  But I think:
I compile and boot tested this on OMAP5.
>
>> +		"if test -z ${fdtfile}; then " \
> Really needs to be:
> if test -z "${fdtfile}"; then...
>
> So that it doesn't become:
> if test -z; then
> when fdtfile isn't set and we get a parse error there.
>
> Thanks!
>
OK I will update again once I get other comments.  Although I am not sure why we really need to protect someone putting the wrong uBoot on their device unless
we have a single uboot for all devices.  I think we are being over protective here.

-- 
------------------
Dan Murphy

[U-Boot] [PATCH v3] arm: dra7xx: Update the EXTRA_ENV_SETTINGS

[Tom Rini]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 06/06/2013 12:18 PM, Dan Murphy wrote:
> On 06/06/2013 10:31 AM, Tom Rini wrote:
>> On Wed, Jun 05, 2013 at 09:39:47AM -0500, Dan Murphy wrote:
>>> Update the EXTRA_ENV_SETTING for the dra7xx. The console needs 
>>> to be set to ttyO0 and the findfdt needs to be updated to load 
>>> the dra7xx-evm.dtb file.
>>> 
>>> Signed-off-by: Dan Murphy <dmurphy@ti.com>
>> Did you run-time test this?  I'm a little concerned about the 
>> include order, but that might turn out alright.  But I think:
> I compile and boot tested this on OMAP5.

OK, thanks.

>>> +		"if test -z ${fdtfile}; then " \
>> Really needs to be: if test -z "${fdtfile}"; then...
>> 
>> So that it doesn't become: if test -z; then when fdtfile isn't 
>> set and we get a parse error there.
>> 
>> Thanks!
>> 
> OK I will update again once I get other comments.  Although I am 
> not sure why we really need to protect someone putting the wrong 
> uBoot on their device unless we have a single uboot for all 
> devices.  I think we are being over protective here.

This is omap5_common.h tho.  You ran into the problem of findfdt ran,
didn't set an fdtfile, then we tried to bootz ${loadaddr} - ${fdtaddr}
which complained about not having a valid device tree.  This should
make it clearer on the next omap5_common.h using board where to fix
things so that their fdt is set right, automatically.

- -- 
Tom
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iQIcBAEBAgAGBQJRsLbgAAoJENk4IS6UOR1Wmj0P/jt1CPAtkqTImYnL1077CyC0
yR4sUcSodiSwR5TlXiEiOJveOYvYjVpcLvH4byH/0Yv9yVnVdoG4+j82QUV7I8zU
YjkuusbfGP5rQNlUcE0bWdUGalTUMrlTbnxMJ13q2dDIr4naqMPGLetO3wdpWYKJ
fdHBRz0phr1k2+OvbAoWEtsjxCj8XBv32yqRj+zZX3ErNcZWsBslRz7QUyDLQU1E
FGBRUAzb7tw0N0VbR+mwg9nuqu2/cBM5hprpsiZ6NfrtuLMP89ucIh61xsXf97K5
T4VFWKZhBHU2Fa2udLBHizJOLlW6NLwNpGD8ac7j9aVC2TKxdgHMtFeho6br/8sA
YkWvL5h/1E4i/qzO3MxuuAakTjhf0P4KU2U7UolH2A707sIKeII2L8ppSiT4cF6w
zB+uN9WgFDTZYYmTxXneD4fy/YWWooZtmCUvDS/7fOCvxGhJKehnwrO18LoCSrYZ
iIMpzB59dfWlYBg64zw6dbmQxgsYUUW7s7xpLi4m+8JnVdJCeArrRAq/9x69Dp18
OywMxiZuPNnDGC9XB5wAimyW3ygvXr4HG4WD4Uv/sw7Uhp6cxz04mzyAVlWeKeGO
0aoUra34tutMQjyqtomqpA7EegwXG0UvAM2XEZi5o8DzeMVwnkZXMxwe/lPE4g8u
hJklyBClfW1nvEIsYgAV
=HAAX
-----END PGP SIGNATURE-----

[U-Boot] [PATCH v3] arm: dra7xx: Update the EXTRA_ENV_SETTINGS

[Dan Murphy]

On 06/06/2013 11:20 AM, Tom Rini wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> On 06/06/2013 12:18 PM, Dan Murphy wrote:
>> On 06/06/2013 10:31 AM, Tom Rini wrote:
>>> On Wed, Jun 05, 2013 at 09:39:47AM -0500, Dan Murphy wrote:
>>>> Update the EXTRA_ENV_SETTING for the dra7xx. The console needs 
>>>> to be set to ttyO0 and the findfdt needs to be updated to load 
>>>> the dra7xx-evm.dtb file.
>>>>
>>>> Signed-off-by: Dan Murphy <dmurphy@ti.com>
>>> Did you run-time test this?  I'm a little concerned about the 
>>> include order, but that might turn out alright.  But I think:
>> I compile and boot tested this on OMAP5.
> OK, thanks.
>
>>>> +		"if test -z ${fdtfile}; then " \
>>> Really needs to be: if test -z "${fdtfile}"; then...
>>>
>>> So that it doesn't become: if test -z; then when fdtfile isn't 
>>> set and we get a parse error there.
>>>
>>> Thanks!
>>>
>> OK I will update again once I get other comments.  Although I am 
>> not sure why we really need to protect someone putting the wrong 
>> uBoot on their device unless we have a single uboot for all 
>> devices.  I think we are being over protective here.
> This is omap5_common.h tho.  You ran into the problem of findfdt ran,
> didn't set an fdtfile, then we tried to bootz ${loadaddr} - ${fdtaddr}
> which complained about not having a valid device tree.  This should
> make it clearer on the next omap5_common.h using board where to fix
> things so that their fdt is set right, automatically.
>
> - -- 
> Tom
OK sounds sane to me.
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.11 (GNU/Linux)
> Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/
>
> iQIcBAEBAgAGBQJRsLbgAAoJENk4IS6UOR1Wmj0P/jt1CPAtkqTImYnL1077CyC0
> yR4sUcSodiSwR5TlXiEiOJveOYvYjVpcLvH4byH/0Yv9yVnVdoG4+j82QUV7I8zU
> YjkuusbfGP5rQNlUcE0bWdUGalTUMrlTbnxMJ13q2dDIr4naqMPGLetO3wdpWYKJ
> fdHBRz0phr1k2+OvbAoWEtsjxCj8XBv32yqRj+zZX3ErNcZWsBslRz7QUyDLQU1E
> FGBRUAzb7tw0N0VbR+mwg9nuqu2/cBM5hprpsiZ6NfrtuLMP89ucIh61xsXf97K5
> T4VFWKZhBHU2Fa2udLBHizJOLlW6NLwNpGD8ac7j9aVC2TKxdgHMtFeho6br/8sA
> YkWvL5h/1E4i/qzO3MxuuAakTjhf0P4KU2U7UolH2A707sIKeII2L8ppSiT4cF6w
> zB+uN9WgFDTZYYmTxXneD4fy/YWWooZtmCUvDS/7fOCvxGhJKehnwrO18LoCSrYZ
> iIMpzB59dfWlYBg64zw6dbmQxgsYUUW7s7xpLi4m+8JnVdJCeArrRAq/9x69Dp18
> OywMxiZuPNnDGC9XB5wAimyW3ygvXr4HG4WD4Uv/sw7Uhp6cxz04mzyAVlWeKeGO
> 0aoUra34tutMQjyqtomqpA7EegwXG0UvAM2XEZi5o8DzeMVwnkZXMxwe/lPE4g8u
> hJklyBClfW1nvEIsYgAV
> =HAAX
> -----END PGP SIGNATURE-----


-- 
------------------
Dan Murphy

[U-Boot] [PATCH v3] arm: dra7xx: Update the EXTRA_ENV_SETTINGS

[Dan Murphy]

Tom
On 06/06/2013 11:24 AM, Dan Murphy wrote:
> On 06/06/2013 11:20 AM, Tom Rini wrote:
>> -----BEGIN PGP SIGNED MESSAGE-----
>> Hash: SHA1
>>
>> On 06/06/2013 12:18 PM, Dan Murphy wrote:
>>> On 06/06/2013 10:31 AM, Tom Rini wrote:
>>>> On Wed, Jun 05, 2013 at 09:39:47AM -0500, Dan Murphy wrote:
>>>>> Update the EXTRA_ENV_SETTING for the dra7xx. The console needs 
>>>>> to be set to ttyO0 and the findfdt needs to be updated to load 
>>>>> the dra7xx-evm.dtb file.
>>>>>
>>>>> Signed-off-by: Dan Murphy <dmurphy@ti.com>
>>>> Did you run-time test this?  I'm a little concerned about the 
>>>> include order, but that might turn out alright.  But I think:
>>> I compile and boot tested this on OMAP5.
>> OK, thanks.
>>
>>>>> +		"if test -z ${fdtfile}; then " \
>>>> Really needs to be: if test -z "${fdtfile}"; then...
>>>>
>>>> So that it doesn't become: if test -z; then when fdtfile isn't 
>>>> set and we get a parse error there.
>>>>
>>>> Thanks!
>>>>
>>> OK I will update again once I get other comments.  Although I am 
>>> not sure why we really need to protect someone putting the wrong 
>>> uBoot on their device unless we have a single uboot for all 
>>> devices.  I think we are being over protective here.
>> This is omap5_common.h tho.  You ran into the problem of findfdt ran,
>> didn't set an fdtfile, then we tried to bootz ${loadaddr} - ${fdtaddr}
>> which complained about not having a valid device tree.  This should
>> make it clearer on the next omap5_common.h using board where to fix
>> things so that their fdt is set right, automatically.
>>
>> - -- 
>> Tom
> OK sounds sane to me.

So this check does not work at all.  I could never get the -z to show that the arg was not defined.
Even if I defined fdtfile=\0 the variable indicates that is is still undefined.

And I cannot add in the "" because the compiler bonks thinking the macro line is completed.

So what I am going to do is pull this out of this patch completely because this has no impact on the intent of the patch.
I will then go ahead and fix the omap4_common, omap5_common and the am335 common in a separate patch.

Thoughts?

Dan

>> -----BEGIN PGP SIGNATURE-----
>> Version: GnuPG v1.4.11 (GNU/Linux)
>> Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/
>>
>> iQIcBAEBAgAGBQJRsLbgAAoJENk4IS6UOR1Wmj0P/jt1CPAtkqTImYnL1077CyC0
>> yR4sUcSodiSwR5TlXiEiOJveOYvYjVpcLvH4byH/0Yv9yVnVdoG4+j82QUV7I8zU
>> YjkuusbfGP5rQNlUcE0bWdUGalTUMrlTbnxMJ13q2dDIr4naqMPGLetO3wdpWYKJ
>> fdHBRz0phr1k2+OvbAoWEtsjxCj8XBv32yqRj+zZX3ErNcZWsBslRz7QUyDLQU1E
>> FGBRUAzb7tw0N0VbR+mwg9nuqu2/cBM5hprpsiZ6NfrtuLMP89ucIh61xsXf97K5
>> T4VFWKZhBHU2Fa2udLBHizJOLlW6NLwNpGD8ac7j9aVC2TKxdgHMtFeho6br/8sA
>> YkWvL5h/1E4i/qzO3MxuuAakTjhf0P4KU2U7UolH2A707sIKeII2L8ppSiT4cF6w
>> zB+uN9WgFDTZYYmTxXneD4fy/YWWooZtmCUvDS/7fOCvxGhJKehnwrO18LoCSrYZ
>> iIMpzB59dfWlYBg64zw6dbmQxgsYUUW7s7xpLi4m+8JnVdJCeArrRAq/9x69Dp18
>> OywMxiZuPNnDGC9XB5wAimyW3ygvXr4HG4WD4Uv/sw7Uhp6cxz04mzyAVlWeKeGO
>> 0aoUra34tutMQjyqtomqpA7EegwXG0UvAM2XEZi5o8DzeMVwnkZXMxwe/lPE4g8u
>> hJklyBClfW1nvEIsYgAV
>> =HAAX
>> -----END PGP SIGNATURE-----
>


-- 
------------------
Dan Murphy

[U-Boot] [PATCH v3] arm: dra7xx: Update the EXTRA_ENV_SETTINGS

[Tom Rini]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 06/06/2013 02:02 PM, Dan Murphy wrote:
> So what I am going to do is pull this out of this patch completely
> because this has no impact on the intent of the patch. I will then
> go ahead and fix the omap4_common, omap5_common and the am335
> common in a separate patch.


Alright, lets go that way, thanks.

- -- 
Tom
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iQIcBAEBAgAGBQJRsNRZAAoJENk4IS6UOR1Wx2IP/ihq7GITzEtcEORldDBfenlI
NQAwQUilibSyxW465OShdUXLhQBMhA96G85MhozDBOL6aoQXO0Zir7B7DiImOkYA
bcZMgdett+tQTwnAtfSnUYeIjfREG8G6A40C4YBk7Yw0vc6VT8xuaBjHE7Entewn
UAjZrTQ2/ltzTHrHkJxKZq7QlH0VEj+hL/b66xngZNJ1BuAHgIsqisGxG+YBfwp3
yrH1rVYCmDI5adx9cpVjHFHvDmEezIeyG28miYJGbvTki5D0Xcf35v0BFU7MLLNh
46oeF65PFc46UctggxwSzTBuE57quGKMCdwg9W6LZsOxncEfnUW0eW5O3syvfJ/f
qaY2Kk0iJD4h6TnV1Pn4bPwrDQ46JWw4wR98SYoHkAPhxD6qdllgxQ43JNEFV4bC
GzvrGw2ytAOsNRUSauE0HP4oqETXnGvpgzWt8HyLNbjBOKILoSTQt2zS4BioIGij
9AjGSeXXrAGPiyNLnxarkNBGbaoCeMrQUfCVo9jLSiCAJ/PPAYLRKnPeo2nGXC6p
ntf8wBoIqZ0AnqbwzPL5kafqIUd8krzKjtwNxdNIh27g/DG345+PiL+DCLJVs4Y/
KQsYLSqzV9G7TY8iCTtsZoB3yABKnyQcf0vRDeavNo/vqAub2UxhH+IG1xQ2+t7e
1BiR4TOqDr7QTfseKckR
=S/7s
-----END PGP SIGNATURE-----