diff mbox series

[U-Boot,2/2] imx8mq_evk/README: remove ARCH environment variable

Message ID 3d575c3968ea5266b3d642d083cc32d1770885f5.1546412309.git.baruch@tkos.co.il
State Accepted
Commit 430630cb032c14880e69ae9d45403d78c318628a
Delegated to: Stefano Babic
Headers show
Series [U-Boot,1/2] tools/imx8m_image.sh: remove bashism | expand

Commit Message

Baruch Siach Jan. 2, 2019, 6:58 a.m. UTC
There is no need to set the ARCH variable when building U-Boot. In fact,
the ARCH name in U-Boot is 'arm'.

Signed-off-by: Baruch Siach <baruch@tkos.co.il>
---
 board/freescale/imx8mq_evk/README | 1 -
 1 file changed, 1 deletion(-)

Comments

Baruch Siach Jan. 30, 2019, 9:48 a.m. UTC | #1
Hi Stefano,

On Wed, Jan 30, 2019 at 10:40:19AM +0200, sbabic@denx.de wrote:
> > There is no need to set the ARCH variable when building U-Boot. In fact,
> > the ARCH name in U-Boot is 'arm'.
> > Signed-off-by: Baruch Siach <baruch@tkos.co.il>
> 
> Applied to u-boot-imx, master, thanks !

Thanks.

What about patch #1 in this series? It fixes build failure when /bin/sh is not 
bash.

baruch
Stefano Babic Jan. 30, 2019, 2:33 p.m. UTC | #2
Hi Baruch,

On 30/01/19 10:48, Baruch Siach wrote:
> Hi Stefano,
> 
> On Wed, Jan 30, 2019 at 10:40:19AM +0200, sbabic@denx.de wrote:
>>> There is no need to set the ARCH variable when building U-Boot. In fact,
>>> the ARCH name in U-Boot is 'arm'.
>>> Signed-off-by: Baruch Siach <baruch@tkos.co.il>
>>
>> Applied to u-boot-imx, master, thanks !
> 
> Thanks.
> 
> What about patch #1 in this series? It fixes build failure when /bin/sh is not 
> bash.
> 

I suffer from the prejudice that a single "=" is always bad in a
comparison.....I am surely influenced from bugs in C/C++ due to a
missing "=" in my experience as developer.

I searched for such as comparison in other parts of build to justify
this patch : I have just found it in "test/fs" (ok, this is not really
part of build) and in ./lib/lzma/import_lzmasdk.sh. I fully agree that
this could be just a problem of mine, and I confess I was already merged
your patch and then removed again..

Apart of this,  it looks like that a single "=" was already avoided in
U-Boot scripts like the pest. Apart of the single entry I found above,
it was avoided..

What about to substitute "=" with an explicit "-eq" ? Like :

	[ $f -eq "spl/u-boot-spl-ddr.bin" ]

Best regards,
Stefano
Baruch Siach Jan. 30, 2019, 3:13 p.m. UTC | #3
Hi Stefano,

On Wed, Jan 30, 2019 at 03:33:21PM +0100, Stefano Babic wrote:
> On 30/01/19 10:48, Baruch Siach wrote:
> > On Wed, Jan 30, 2019 at 10:40:19AM +0200, sbabic@denx.de wrote:
> >>> There is no need to set the ARCH variable when building U-Boot. In fact,
> >>> the ARCH name in U-Boot is 'arm'.
> >>> Signed-off-by: Baruch Siach <baruch@tkos.co.il>
> >>
> >> Applied to u-boot-imx, master, thanks !
> > 
> > Thanks.
> > 
> > What about patch #1 in this series? It fixes build failure when /bin/sh is not 
> > bash.
> 
> I suffer from the prejudice that a single "=" is always bad in a
> comparison.....I am surely influenced from bugs in C/C++ due to a
> missing "=" in my experience as developer.
> 
> I searched for such as comparison in other parts of build to justify
> this patch : I have just found it in "test/fs" (ok, this is not really
> part of build) and in ./lib/lzma/import_lzmasdk.sh. I fully agree that
> this could be just a problem of mine, and I confess I was already merged
> your patch and then removed again..
> 
> Apart of this,  it looks like that a single "=" was already avoided in
> U-Boot scripts like the pest. Apart of the single entry I found above,
> it was avoided..
> 
> What about to substitute "=" with an explicit "-eq" ? Like :
> 
> 	[ $f -eq "spl/u-boot-spl-ddr.bin" ]

The '-eq' operator only accept integers:

$ [ f -eq "spl/u-boot-spl-ddr.bin" ]
bash: test: f: integer expression expected

The alternative is to set the script shebang to /bin/bash. I don't like this 
solution. What do you think?

baruch
Stefano Babic Jan. 30, 2019, 3:30 p.m. UTC | #4
On 30/01/19 16:13, Baruch Siach wrote:
> Hi Stefano,
> 
> On Wed, Jan 30, 2019 at 03:33:21PM +0100, Stefano Babic wrote:
>> On 30/01/19 10:48, Baruch Siach wrote:
>>> On Wed, Jan 30, 2019 at 10:40:19AM +0200, sbabic@denx.de wrote:
>>>>> There is no need to set the ARCH variable when building U-Boot. In fact,
>>>>> the ARCH name in U-Boot is 'arm'.
>>>>> Signed-off-by: Baruch Siach <baruch@tkos.co.il>
>>>>
>>>> Applied to u-boot-imx, master, thanks !
>>>
>>> Thanks.
>>>
>>> What about patch #1 in this series? It fixes build failure when /bin/sh is not 
>>> bash.
>>
>> I suffer from the prejudice that a single "=" is always bad in a
>> comparison.....I am surely influenced from bugs in C/C++ due to a
>> missing "=" in my experience as developer.
>>
>> I searched for such as comparison in other parts of build to justify
>> this patch : I have just found it in "test/fs" (ok, this is not really
>> part of build) and in ./lib/lzma/import_lzmasdk.sh. I fully agree that
>> this could be just a problem of mine, and I confess I was already merged
>> your patch and then removed again..
>>
>> Apart of this,  it looks like that a single "=" was already avoided in
>> U-Boot scripts like the pest. Apart of the single entry I found above,
>> it was avoided..
>>
>> What about to substitute "=" with an explicit "-eq" ? Like :
>>
>> 	[ $f -eq "spl/u-boot-spl-ddr.bin" ]
> 
> The '-eq' operator only accept integers:
> 
> $ [ f -eq "spl/u-boot-spl-ddr.bin" ]
> bash: test: f: integer expression expected
> 
> The alternative is to set the script shebang to /bin/bash. I don't like this 
> solution. What do you think?

Is not possible to change the behavior of the script filtering the
$blobs variable before evaluating ? The check is just for skipping these
two files.

Something like :

 blobs=`awk '/^SIGNED_HDMI/ {print $2} /^LOADER/ {print $2}
/^SECOND_LOADER/ {print $2} /^DDR_FW/ {print $2}' $file` | grep -v
""spl/u-boot-spl-ddr.bin" | ......

It look to me better to put in $blobs just the values that must be
evaluated instead of checking them later. $blobs is not evaluated anymore.

Best regards,
Stefano
diff mbox series

Patch

diff --git a/board/freescale/imx8mq_evk/README b/board/freescale/imx8mq_evk/README
index 07dbfb01fed8..1da6eaf24277 100644
--- a/board/freescale/imx8mq_evk/README
+++ b/board/freescale/imx8mq_evk/README
@@ -23,7 +23,6 @@  $ cp firmware-imx-7.9/firmware-imx-7.9/firmware/ddr/synopsys/lpddr4*.bin $(srcte
 
 Build U-Boot
 ====================
-$ export ARCH=arm64
 $ export CROSS_COMPILE=aarch64-poky-linux-
 $ make imx8mq_evk_defconfig
 $ make flash.bin