[PATCH 0/7] microblaze: fix various problems in building boot images

[PATCH 0/7] microblaze: fix various problems in building boot images

[Masahiro Yamada]

This patch set fixes various issues in microblaze Makefiles.

BTW, "simpleImage.<dt>" works like a phony target to generate the
following four images, where the first three are just aliases.

 - arch/microblaze/boot/simpleImage.<dt>:
         identical to arch/microblaze/boot/linux.bin

 - arch/microblaze/boot/simpleImage.<dt>.unstrip:
         identical to vmlinux

 - arch/microblaze/boot/simpleImage.<dt>.ub:
         identical to arch/microblaze/boot/linux.bin.ub

 - arch/microblaze/boot/simpleImage.<dt>.strip:
         stripped vmlinux

I am not sure how much useful those copies are,
but, I tried my best to keep the same behavior.

IMHO, I guess DTB=<dt> would be more sensible,
but it is up to Michal.



Masahiro Yamada (7):
  microblaze: fix cleaning of boot images
  microblaze: adjust the help to the real behavior
  microblaze: move "... is ready" message to arch/microblaze/Makefile
  microblaze: fix multiple bugs in arch/microblaze/boot/Makefile
  microblaze: add linux.bin* and simpleImage.* to PHONY
  microblaze: fix race condition in building boot images
  microblaze: remove the unneeded code just in case file copy fails

 arch/microblaze/Makefile          | 14 +++++++++-----
 arch/microblaze/boot/Makefile     | 33 +++++++++++++++++----------------
 arch/microblaze/boot/dts/Makefile |  5 +----
 3 files changed, 27 insertions(+), 25 deletions(-)

-- 
2.7.4

[PATCH 1/7] microblaze: fix cleaning of boot images

[Masahiro Yamada]

The simpleImage.<dt> target generates the following files:

  arch/microblaze/boot/simpleImage.<dt>
  arch/microblaze/boot/simpleImage.<dt>.ub
  arch/microblaze/boot/simpleImage.<dt>.strip
  arch/microblaze/boot/simpleImage.<dt>.unstrip

However, "make ARCH=microblaze clean" only cleans up the unstrip
image. Fix the clean-files to take care of all the four.

Adding linux.bin.ub to clean-files is redundant because it is
already added into "targets".

Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---

 arch/microblaze/boot/Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/microblaze/boot/Makefile b/arch/microblaze/boot/Makefile
index 600e5a1..e8684a2 100644
--- a/arch/microblaze/boot/Makefile
+++ b/arch/microblaze/boot/Makefile
@@ -37,4 +37,4 @@ $(obj)/simpleImage.%: vmlinux FORCE
 	$(call if_changed,strip,.strip)
 	@echo 'Kernel: $(UIMAGE_OUT) is ready' ' (#'`cat .version`')'
 
-clean-files += simpleImage.*.unstrip linux.bin.ub
+clean-files += simpleImage.*
-- 
2.7.4

[PATCH 2/7] microblaze: adjust the help to the real behavior

[Masahiro Yamada]

"make ARCH=microblaze help" mentions simpleImage.<dt>.unstrip,
but it never works because Makefile assumes "system.unstrip" is
the name of DT.

$ make ARCH=microblaze CROSS_COMPILE=microblaze-linux- simpleImage.system.unstrip
  [ snip ]
make[1]: *** No rule to make target 'arch/microblaze/boot/dts/system.unstrip.dtb', needed by 'arch/microblaze/boot/dts/system.dtb'.  Stop.
make: *** [Makefile;1060: arch/microblaze/boot/dts] Error 2
make: *** Waiting for unfinished jobs....

Rip off the never-working target.

In my understanding, simpleImage.<dt> works like a phony target that
generates multiple images. Reflect the behavior to the help message.

Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---

 arch/microblaze/Makefile | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/arch/microblaze/Makefile b/arch/microblaze/Makefile
index 0823d29..97e1384 100644
--- a/arch/microblaze/Makefile
+++ b/arch/microblaze/Makefile
@@ -89,9 +89,7 @@ define archhelp
   echo '* linux.bin    - Create raw binary'
   echo '  linux.bin.gz - Create compressed raw binary'
   echo '  linux.bin.ub - Create U-Boot wrapped raw binary'
-  echo '  simpleImage.<dt> - ELF image with $(arch)/boot/dts/<dt>.dts linked in'
-  echo '                   - stripped elf with fdt blob'
-  echo '  simpleImage.<dt>.unstrip - full ELF image with fdt blob'
+  echo '  simpleImage.<dt> - Create images with $(arch)/boot/dts/<dt>.dts linked in'
   echo '  *_defconfig      - Select default config from arch/microblaze/configs'
   echo ''
   echo '  Targets with <dt> embed a device tree blob inside the image'
-- 
2.7.4

[PATCH 3/7] microblaze: move "... is ready" message to arch/microblaze/Makefile

[Masahiro Yamada]

To prepare for more fixes, move this to arch/microblaze/Makefile.
Otherwise, the same "... is ready" would be printed multiple times.

(Another solution would be, to remove these messages entirely unless
people persist with them.)

Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---

 arch/microblaze/Makefile      | 2 ++
 arch/microblaze/boot/Makefile | 4 ----
 2 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/arch/microblaze/Makefile b/arch/microblaze/Makefile
index 97e1384..cfd7bfca 100644
--- a/arch/microblaze/Makefile
+++ b/arch/microblaze/Makefile
@@ -81,9 +81,11 @@ archclean:
 
 linux.bin linux.bin.gz linux.bin.ub: vmlinux
 	$(Q)$(MAKE) $(build)=$(boot) $(boot)/$@
+	@echo 'Kernel: $(boot)/$@ is ready' ' (#'`cat .version`')'
 
 simpleImage.%: vmlinux
 	$(Q)$(MAKE) $(build)=$(boot) $(boot)/$@
+	@echo 'Kernel: $(boot)/$@ is ready' ' (#'`cat .version`')'
 
 define archhelp
   echo '* linux.bin    - Create raw binary'
diff --git a/arch/microblaze/boot/Makefile b/arch/microblaze/boot/Makefile
index e8684a2..c2579a7 100644
--- a/arch/microblaze/boot/Makefile
+++ b/arch/microblaze/boot/Makefile
@@ -9,15 +9,12 @@ OBJCOPYFLAGS := -R .note -R .comment -R .note.gnu.build-id -O binary
 
 $(obj)/linux.bin: vmlinux FORCE
 	$(call if_changed,objcopy)
-	@echo 'Kernel: $@ is ready' ' (#'`cat .version`')'
 
 $(obj)/linux.bin.ub: $(obj)/linux.bin FORCE
 	$(call if_changed,uimage)
-	@echo 'Kernel: $@ is ready' ' (#'`cat .version`')'
 
 $(obj)/linux.bin.gz: $(obj)/linux.bin FORCE
 	$(call if_changed,gzip)
-	@echo 'Kernel: $@ is ready' ' (#'`cat .version`')'
 
 quiet_cmd_cp = CP      $< $@$2
 	cmd_cp = cat $< >$@$2 || (rm -f $@ && echo false)
@@ -35,6 +32,5 @@ $(obj)/simpleImage.%: vmlinux FORCE
 	$(call if_changed,objcopy)
 	$(call if_changed,uimage)
 	$(call if_changed,strip,.strip)
-	@echo 'Kernel: $(UIMAGE_OUT) is ready' ' (#'`cat .version`')'
 
 clean-files += simpleImage.*
-- 
2.7.4

[PATCH 4/7] microblaze: fix multiple bugs in arch/microblaze/boot/Makefile

[Masahiro Yamada]

This Makefile is wrong in multiple ways.

The first issue is the breakage of 'linux.bin.ub' target since commit
ece97f3a5fb5 ("microblaze: Fix simpleImage format generation")
because the addition of UIMAGE_{IN,OUT} obviously affected it.

make ARCH=microblaze CROSS_COMPILE=microblaze-linux- linux.bin.ub
  [ snip ]
  OBJCOPY arch/microblaze/boot/linux.bin
  UIMAGE  arch/microblaze/boot/linux.bin.ub.ub
/usr/bin/mkimage: Can't open arch/microblaze/boot/linux.bin.ub: No such file or directory
make[1]: *** [arch/microblaze/boot/Makefile;14: arch/microblaze/boot/linux.bin.ub] Error 1
make: *** [arch/microblaze/Makefile;83: linux.bin.ub] Error 2

The second issue is the use of the "if_changed" multiple times for
the same target.

As commit 92a4728608a8 ("x86/boot: Fix if_changed build flip/flop bug")
pointed out, this never works properly. Moreover, generating multiple
images as a side-effect is extremely confusing and wrong.

As far as I understood, "simpleImage.<dt>" works like a phony target
to generate the following four images.

 - arch/microblaze/boot/simpleImage.<dt>:
       identical to arch/microblaze/boot/linux.bin

 - arch/microblaze/boot/simpleImage.<dt>.unstrip:
       identical to vmlinux

 - arch/microblaze/boot/simpleImage.<dt>.ub:
       identical to arch/microblaze/boot/linux.bin.ub

 - arch/microblaze/boot/simpleImage.<dt>.strip:
       stripped vmlinux

The first three are just aliases of other images. Separate the recipe
for each image.

Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---

 arch/microblaze/Makefile      |  2 +-
 arch/microblaze/boot/Makefile | 27 ++++++++++++++++-----------
 2 files changed, 17 insertions(+), 12 deletions(-)

diff --git a/arch/microblaze/Makefile b/arch/microblaze/Makefile
index cfd7bfca..c5d5b0e 100644
--- a/arch/microblaze/Makefile
+++ b/arch/microblaze/Makefile
@@ -84,7 +84,7 @@ linux.bin linux.bin.gz linux.bin.ub: vmlinux
 	@echo 'Kernel: $(boot)/$@ is ready' ' (#'`cat .version`')'
 
 simpleImage.%: vmlinux
-	$(Q)$(MAKE) $(build)=$(boot) $(boot)/$@
+	$(Q)$(MAKE) $(build)=$(boot) simple_images
 	@echo 'Kernel: $(boot)/$@ is ready' ' (#'`cat .version`')'
 
 define archhelp
diff --git a/arch/microblaze/boot/Makefile b/arch/microblaze/boot/Makefile
index c2579a7..f12a9d7 100644
--- a/arch/microblaze/boot/Makefile
+++ b/arch/microblaze/boot/Makefile
@@ -3,7 +3,7 @@
 # arch/microblaze/boot/Makefile
 #
 
-targets := linux.bin linux.bin.gz linux.bin.ub simpleImage.%
+targets := linux.bin linux.bin.gz linux.bin.ub simpleImage.*.strip
 
 OBJCOPYFLAGS := -R .note -R .comment -R .note.gnu.build-id -O binary
 
@@ -16,21 +16,26 @@ $(obj)/linux.bin.ub: $(obj)/linux.bin FORCE
 $(obj)/linux.bin.gz: $(obj)/linux.bin FORCE
 	$(call if_changed,gzip)
 
-quiet_cmd_cp = CP      $< $@$2
-	cmd_cp = cat $< >$@$2 || (rm -f $@ && echo false)
-
 quiet_cmd_strip = STRIP   $< $@$2
 	cmd_strip = $(STRIP) -K microblaze_start -K _end -K __log_buf \
 				-K _fdt_start $< -o $@$2
 
 UIMAGE_LOADADDR = $(CONFIG_KERNEL_BASE_ADDR)
-UIMAGE_IN = $@
-UIMAGE_OUT = $@.ub
 
-$(obj)/simpleImage.%: vmlinux FORCE
-	$(call if_changed,cp,.unstrip)
-	$(call if_changed,objcopy)
-	$(call if_changed,uimage)
-	$(call if_changed,strip,.strip)
+PHONY += simple_images
+simple_images: $(addprefix $(obj)/simpleImage., $(DTB) $(DTB).ub $(DTB).unstrip $(DTB).strip)
+	@:
+
+$(obj)/simpleImage.$(DTB): $(obj)/linux.bin
+	$(call cmd,shipped)
+
+$(obj)/simpleImage.$(DTB).ub: $(obj)/linux.bin.ub
+	$(call cmd,shipped)
+
+$(obj)/simpleImage.$(DTB).unstrip: vmlinux
+	$(call cmd,shipped)
+
+$(obj)/simpleImage.$(DTB).strip: vmlinux FORCE
+	$(call if_changed,strip)
 
 clean-files += simpleImage.*
-- 
2.7.4

[PATCH 5/7] microblaze: add linux.bin* and simpleImage.* to PHONY

[Masahiro Yamada]

linux.bin, linux.bin.gz, and linux.bin.ub are phony targets to
generate a corresponding image under arch/microblaze/boot/.

simpleImage.% also works like a PHONY target, but a pattern that
contains '%' cannot be a PHONY target. I renamed it to equivalent
simpleImage.$(DTB).

Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---

 arch/microblaze/Makefile | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/arch/microblaze/Makefile b/arch/microblaze/Makefile
index c5d5b0e..7a5df02 100644
--- a/arch/microblaze/Makefile
+++ b/arch/microblaze/Makefile
@@ -79,11 +79,13 @@ all: linux.bin
 archclean:
 	$(Q)$(MAKE) $(clean)=$(boot)
 
+PHONY += linux.bin linux.bin.gz linux.bin.ub
 linux.bin linux.bin.gz linux.bin.ub: vmlinux
 	$(Q)$(MAKE) $(build)=$(boot) $(boot)/$@
 	@echo 'Kernel: $(boot)/$@ is ready' ' (#'`cat .version`')'
 
-simpleImage.%: vmlinux
+PHONY += simpleImage.$(DTB)
+simpleImage.$(DTB): vmlinux
 	$(Q)$(MAKE) $(build)=$(boot) simple_images
 	@echo 'Kernel: $(boot)/$@ is ready' ' (#'`cat .version`')'
 
-- 
2.7.4

[PATCH 6/7] microblaze: fix race condition in building boot images

[Masahiro Yamada]

I fixed a race condition in the parallel building of ARM in commit
3939f3345050 ("ARM: 8418/1: add boot image dependencies to not
generate invalid images").

I see the same problem for MicroBlaze too.

"make -j<N> ARCH=microblaze all linux.bin.ub" results in a broken build
since two threads descend into arch/microblaze/boot simultaneously.

Add proper dependencies to avoid it.

Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---

 arch/microblaze/Makefile | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/arch/microblaze/Makefile b/arch/microblaze/Makefile
index 7a5df02..544796d 100644
--- a/arch/microblaze/Makefile
+++ b/arch/microblaze/Makefile
@@ -79,13 +79,15 @@ all: linux.bin
 archclean:
 	$(Q)$(MAKE) $(clean)=$(boot)
 
+linux.bin.ub linux.bin.gz: linux.bin
+
 PHONY += linux.bin linux.bin.gz linux.bin.ub
 linux.bin linux.bin.gz linux.bin.ub: vmlinux
 	$(Q)$(MAKE) $(build)=$(boot) $(boot)/$@
 	@echo 'Kernel: $(boot)/$@ is ready' ' (#'`cat .version`')'
 
 PHONY += simpleImage.$(DTB)
-simpleImage.$(DTB): vmlinux
+simpleImage.$(DTB): linux.bin.ub
 	$(Q)$(MAKE) $(build)=$(boot) simple_images
 	@echo 'Kernel: $(boot)/$@ is ready' ' (#'`cat .version`')'
 
-- 
2.7.4

[PATCH 7/7] microblaze: remove the unneeded code just in case file copy fails

[Masahiro Yamada]

I guess

    || (rm -f $@ && echo false)

... should be

    || (rm -f $@ && false)

since printing the string "false" on the console has no point.

Moreover, no Makefile needs to delete a target on error explicitly
since commit 9c2af1c7377a ("kbuild: add .DELETE_ON_ERROR special
target").

Reuse equivalent cmd_shipped from scripts/Makefile.lib.

Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---

 arch/microblaze/boot/dts/Makefile | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/arch/microblaze/boot/dts/Makefile b/arch/microblaze/boot/dts/Makefile
index c7324e7..ef00dd3 100644
--- a/arch/microblaze/boot/dts/Makefile
+++ b/arch/microblaze/boot/dts/Makefile
@@ -12,12 +12,9 @@ $(obj)/linked_dtb.o: $(obj)/system.dtb
 # Generate system.dtb from $(DTB).dtb
 ifneq ($(DTB),system)
 $(obj)/system.dtb: $(obj)/$(DTB).dtb
-	$(call if_changed,cp)
+	$(call if_changed,shipped)
 endif
 endif
 
-quiet_cmd_cp = CP      $< $@$2
-	cmd_cp = cat $< >$@$2 || (rm -f $@ && echo false)
-
 # Rule to build device tree blobs
 DTC_FLAGS := -p 1024
-- 
2.7.4

Re: [PATCH 2/7] microblaze: adjust the help to the real behavior

[Michal Simek]

[-- Attachment #1.1: Type: text/plain, Size: 2494 bytes --]

On 03. 12. 18 8:50, Masahiro Yamada wrote:
> "make ARCH=microblaze help" mentions simpleImage.<dt>.unstrip,
> but it never works because Makefile assumes "system.unstrip" is
> the name of DT.
> 
> $ make ARCH=microblaze CROSS_COMPILE=microblaze-linux- simpleImage.system.unstrip
>   [ snip ]
> make[1]: *** No rule to make target 'arch/microblaze/boot/dts/system.unstrip.dtb', needed by 'arch/microblaze/boot/dts/system.dtb'.  Stop.
> make: *** [Makefile;1060: arch/microblaze/boot/dts] Error 2
> make: *** Waiting for unfinished jobs....
> 
> Rip off the never-working target.
> 
> In my understanding, simpleImage.<dt> works like a phony target that
> generates multiple images. Reflect the behavior to the help message.
> 
> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> ---
> 
>  arch/microblaze/Makefile | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/arch/microblaze/Makefile b/arch/microblaze/Makefile
> index 0823d29..97e1384 100644
> --- a/arch/microblaze/Makefile
> +++ b/arch/microblaze/Makefile
> @@ -89,9 +89,7 @@ define archhelp
>    echo '* linux.bin    - Create raw binary'
>    echo '  linux.bin.gz - Create compressed raw binary'
>    echo '  linux.bin.ub - Create U-Boot wrapped raw binary'
> -  echo '  simpleImage.<dt> - ELF image with $(arch)/boot/dts/<dt>.dts linked in'
> -  echo '                   - stripped elf with fdt blob'
> -  echo '  simpleImage.<dt>.unstrip - full ELF image with fdt blob'
> +  echo '  simpleImage.<dt> - Create images with $(arch)/boot/dts/<dt>.dts linked in'
>    echo '  *_defconfig      - Select default config from arch/microblaze/configs'
>    echo ''
>    echo '  Targets with <dt> embed a device tree blob inside the image'
> 

I understand what you are trying to say but I would still like to keep
information about unstrip file.
It is correct that it is not build target. It is just generated file and
message above was used for description what it is.
Definitely agree that this should be the part of targets but it should
be in description.
The same is for missing description for simpleImage.<dt>.strip and
simpleImage.<dt>.ub files.

Thanks,
Michal

-- 
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Xilinx Microblaze
Maintainer of Linux kernel - Xilinx Zynq ARM and ZynqMP ARM64 SoCs
U-Boot custodian - Xilinx Microblaze/Zynq/ZynqMP/Versal SoCs



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCH 1/7] microblaze: fix cleaning of boot images

[Michal Simek]

[-- Attachment #1.1: Type: text/plain, Size: 1511 bytes --]

On 03. 12. 18 8:50, Masahiro Yamada wrote:
> The simpleImage.<dt> target generates the following files:
> 
>   arch/microblaze/boot/simpleImage.<dt>
>   arch/microblaze/boot/simpleImage.<dt>.ub
>   arch/microblaze/boot/simpleImage.<dt>.strip
>   arch/microblaze/boot/simpleImage.<dt>.unstrip
> 
> However, "make ARCH=microblaze clean" only cleans up the unstrip
> image. Fix the clean-files to take care of all the four.
> 
> Adding linux.bin.ub to clean-files is redundant because it is
> already added into "targets".
> 
> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> ---
> 
>  arch/microblaze/boot/Makefile | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/microblaze/boot/Makefile b/arch/microblaze/boot/Makefile
> index 600e5a1..e8684a2 100644
> --- a/arch/microblaze/boot/Makefile
> +++ b/arch/microblaze/boot/Makefile
> @@ -37,4 +37,4 @@ $(obj)/simpleImage.%: vmlinux FORCE
>  	$(call if_changed,strip,.strip)
>  	@echo 'Kernel: $(UIMAGE_OUT) is ready' ' (#'`cat .version`')'
>  
> -clean-files += simpleImage.*.unstrip linux.bin.ub
> +clean-files += simpleImage.*
> 

Reviewed-by: Michal Simek <michal.simek@xilinx.com>

Thanks,
Michal

-- 
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Xilinx Microblaze
Maintainer of Linux kernel - Xilinx Zynq ARM and ZynqMP ARM64 SoCs
U-Boot custodian - Xilinx Microblaze/Zynq/ZynqMP/Versal SoCs



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCH 3/7] microblaze: move "... is ready" message to arch/microblaze/Makefile

[Michal Simek]

[-- Attachment #1.1: Type: text/plain, Size: 2541 bytes --]

On 03. 12. 18 8:50, Masahiro Yamada wrote:
> To prepare for more fixes, move this to arch/microblaze/Makefile.
> Otherwise, the same "... is ready" would be printed multiple times.
> 
> (Another solution would be, to remove these messages entirely unless
> people persist with them.)
> 
> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> ---
> 
>  arch/microblaze/Makefile      | 2 ++
>  arch/microblaze/boot/Makefile | 4 ----
>  2 files changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/microblaze/Makefile b/arch/microblaze/Makefile
> index 97e1384..cfd7bfca 100644
> --- a/arch/microblaze/Makefile
> +++ b/arch/microblaze/Makefile
> @@ -81,9 +81,11 @@ archclean:
>  
>  linux.bin linux.bin.gz linux.bin.ub: vmlinux
>  	$(Q)$(MAKE) $(build)=$(boot) $(boot)/$@
> +	@echo 'Kernel: $(boot)/$@ is ready' ' (#'`cat .version`')'
>  
>  simpleImage.%: vmlinux
>  	$(Q)$(MAKE) $(build)=$(boot) $(boot)/$@
> +	@echo 'Kernel: $(boot)/$@ is ready' ' (#'`cat .version`')'
>  
>  define archhelp
>    echo '* linux.bin    - Create raw binary'
> diff --git a/arch/microblaze/boot/Makefile b/arch/microblaze/boot/Makefile
> index e8684a2..c2579a7 100644
> --- a/arch/microblaze/boot/Makefile
> +++ b/arch/microblaze/boot/Makefile
> @@ -9,15 +9,12 @@ OBJCOPYFLAGS := -R .note -R .comment -R .note.gnu.build-id -O binary
>  
>  $(obj)/linux.bin: vmlinux FORCE
>  	$(call if_changed,objcopy)
> -	@echo 'Kernel: $@ is ready' ' (#'`cat .version`')'
>  
>  $(obj)/linux.bin.ub: $(obj)/linux.bin FORCE
>  	$(call if_changed,uimage)
> -	@echo 'Kernel: $@ is ready' ' (#'`cat .version`')'
>  
>  $(obj)/linux.bin.gz: $(obj)/linux.bin FORCE
>  	$(call if_changed,gzip)
> -	@echo 'Kernel: $@ is ready' ' (#'`cat .version`')'
>  
>  quiet_cmd_cp = CP      $< $@$2
>  	cmd_cp = cat $< >$@$2 || (rm -f $@ && echo false)
> @@ -35,6 +32,5 @@ $(obj)/simpleImage.%: vmlinux FORCE
>  	$(call if_changed,objcopy)
>  	$(call if_changed,uimage)
>  	$(call if_changed,strip,.strip)
> -	@echo 'Kernel: $(UIMAGE_OUT) is ready' ' (#'`cat .version`')'
>  
>  clean-files += simpleImage.*
> 

TBH I can't see this problematic but if this is aligned with others I
have no problem with it.

Thanks,
Michal


-- 
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Xilinx Microblaze
Maintainer of Linux kernel - Xilinx Zynq ARM and ZynqMP ARM64 SoCs
U-Boot custodian - Xilinx Microblaze/Zynq/ZynqMP/Versal SoCs



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCH 4/7] microblaze: fix multiple bugs in arch/microblaze/boot/Makefile

[Michal Simek]

[-- Attachment #1.1: Type: text/plain, Size: 2212 bytes --]

On 03. 12. 18 8:50, Masahiro Yamada wrote:
> This Makefile is wrong in multiple ways.
> 
> The first issue is the breakage of 'linux.bin.ub' target since commit
> ece97f3a5fb5 ("microblaze: Fix simpleImage format generation")
> because the addition of UIMAGE_{IN,OUT} obviously affected it.
> 
> make ARCH=microblaze CROSS_COMPILE=microblaze-linux- linux.bin.ub
>   [ snip ]
>   OBJCOPY arch/microblaze/boot/linux.bin
>   UIMAGE  arch/microblaze/boot/linux.bin.ub.ub
> /usr/bin/mkimage: Can't open arch/microblaze/boot/linux.bin.ub: No such file or directory
> make[1]: *** [arch/microblaze/boot/Makefile;14: arch/microblaze/boot/linux.bin.ub] Error 1
> make: *** [arch/microblaze/Makefile;83: linux.bin.ub] Error 2
> 
> The second issue is the use of the "if_changed" multiple times for
> the same target.
> 
> As commit 92a4728608a8 ("x86/boot: Fix if_changed build flip/flop bug")
> pointed out, this never works properly. Moreover, generating multiple
> images as a side-effect is extremely confusing and wrong.
> 
> As far as I understood, "simpleImage.<dt>" works like a phony target
> to generate the following four images.
> 
>  - arch/microblaze/boot/simpleImage.<dt>:
>        identical to arch/microblaze/boot/linux.bin
> 
>  - arch/microblaze/boot/simpleImage.<dt>.unstrip:
>        identical to vmlinux
> 
>  - arch/microblaze/boot/simpleImage.<dt>.ub:
>        identical to arch/microblaze/boot/linux.bin.ub
> 
>  - arch/microblaze/boot/simpleImage.<dt>.strip:
>        stripped vmlinux

Your understanding is correct. It always worked like that
simpleImage.<dt> was generating 4 files.
I have wired that long time ago because I wanted to boot image in U-Boot
that's why .ub was also generated.
Definitely simpleImage.<dt>.ub can be another target and not
automatically generated by simpleImage.<dt>

Acked-by: Michal Simek <michal.simek@xilinx.com>

Thanks,
Michal


-- 
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Xilinx Microblaze
Maintainer of Linux kernel - Xilinx Zynq ARM and ZynqMP ARM64 SoCs
U-Boot custodian - Xilinx Microblaze/Zynq/ZynqMP/Versal SoCs



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCH 5/7] microblaze: add linux.bin* and simpleImage.* to PHONY

[Michal Simek]

[-- Attachment #1.1: Type: text/plain, Size: 1551 bytes --]

On 03. 12. 18 8:50, Masahiro Yamada wrote:
> linux.bin, linux.bin.gz, and linux.bin.ub are phony targets to
> generate a corresponding image under arch/microblaze/boot/.
> 
> simpleImage.% also works like a PHONY target, but a pattern that
> contains '%' cannot be a PHONY target. I renamed it to equivalent
> simpleImage.$(DTB).
> 
> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> ---
> 
>  arch/microblaze/Makefile | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/microblaze/Makefile b/arch/microblaze/Makefile
> index c5d5b0e..7a5df02 100644
> --- a/arch/microblaze/Makefile
> +++ b/arch/microblaze/Makefile
> @@ -79,11 +79,13 @@ all: linux.bin
>  archclean:
>  	$(Q)$(MAKE) $(clean)=$(boot)
>  
> +PHONY += linux.bin linux.bin.gz linux.bin.ub
>  linux.bin linux.bin.gz linux.bin.ub: vmlinux
>  	$(Q)$(MAKE) $(build)=$(boot) $(boot)/$@
>  	@echo 'Kernel: $(boot)/$@ is ready' ' (#'`cat .version`')'
>  
> -simpleImage.%: vmlinux
> +PHONY += simpleImage.$(DTB)
> +simpleImage.$(DTB): vmlinux
>  	$(Q)$(MAKE) $(build)=$(boot) simple_images
>  	@echo 'Kernel: $(boot)/$@ is ready' ' (#'`cat .version`')'
>  
> 

Acked-by: Michal Simek <michal.simek@xilinx.com>

Thanks,
Michal


-- 
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Xilinx Microblaze
Maintainer of Linux kernel - Xilinx Zynq ARM and ZynqMP ARM64 SoCs
U-Boot custodian - Xilinx Microblaze/Zynq/ZynqMP/Versal SoCs



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCH 6/7] microblaze: fix race condition in building boot images

[Michal Simek]

[-- Attachment #1.1: Type: text/plain, Size: 1890 bytes --]

On 03. 12. 18 8:50, Masahiro Yamada wrote:
> I fixed a race condition in the parallel building of ARM in commit
> 3939f3345050 ("ARM: 8418/1: add boot image dependencies to not
> generate invalid images").
> 
> I see the same problem for MicroBlaze too.
> 
> "make -j<N> ARCH=microblaze all linux.bin.ub" results in a broken build
> since two threads descend into arch/microblaze/boot simultaneously.

I see also different problem that when I run that make above
linux.bin.ub is not generated at all.


> 
> Add proper dependencies to avoid it.
> 
> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> ---
> 
>  arch/microblaze/Makefile | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/microblaze/Makefile b/arch/microblaze/Makefile
> index 7a5df02..544796d 100644
> --- a/arch/microblaze/Makefile
> +++ b/arch/microblaze/Makefile
> @@ -79,13 +79,15 @@ all: linux.bin
>  archclean:
>  	$(Q)$(MAKE) $(clean)=$(boot)
>  
> +linux.bin.ub linux.bin.gz: linux.bin
> +
>  PHONY += linux.bin linux.bin.gz linux.bin.ub
>  linux.bin linux.bin.gz linux.bin.ub: vmlinux
>  	$(Q)$(MAKE) $(build)=$(boot) $(boot)/$@
>  	@echo 'Kernel: $(boot)/$@ is ready' ' (#'`cat .version`')'
>  
>  PHONY += simpleImage.$(DTB)
> -simpleImage.$(DTB): vmlinux
> +simpleImage.$(DTB): linux.bin.ub

It looks weird that simpleImage requires linux.bin.ub.
Is it really linux.bin.ub here or just linux.bin?

>  	$(Q)$(MAKE) $(build)=$(boot) simple_images
>  	@echo 'Kernel: $(boot)/$@ is ready' ' (#'`cat .version`')'
>  
> 


Thanks,
Michal

-- 
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Xilinx Microblaze
Maintainer of Linux kernel - Xilinx Zynq ARM and ZynqMP ARM64 SoCs
U-Boot custodian - Xilinx Microblaze/Zynq/ZynqMP/Versal SoCs



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCH 7/7] microblaze: remove the unneeded code just in case file copy fails

[Michal Simek]

[-- Attachment #1.1: Type: text/plain, Size: 1597 bytes --]

On 03. 12. 18 8:50, Masahiro Yamada wrote:
> I guess
> 
>     || (rm -f $@ && echo false)
> 
> ... should be
> 
>     || (rm -f $@ && false)
> 
> since printing the string "false" on the console has no point.
> 
> Moreover, no Makefile needs to delete a target on error explicitly
> since commit 9c2af1c7377a ("kbuild: add .DELETE_ON_ERROR special
> target").
> 
> Reuse equivalent cmd_shipped from scripts/Makefile.lib.
> 
> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> ---
> 
>  arch/microblaze/boot/dts/Makefile | 5 +----
>  1 file changed, 1 insertion(+), 4 deletions(-)
> 
> diff --git a/arch/microblaze/boot/dts/Makefile b/arch/microblaze/boot/dts/Makefile
> index c7324e7..ef00dd3 100644
> --- a/arch/microblaze/boot/dts/Makefile
> +++ b/arch/microblaze/boot/dts/Makefile
> @@ -12,12 +12,9 @@ $(obj)/linked_dtb.o: $(obj)/system.dtb
>  # Generate system.dtb from $(DTB).dtb
>  ifneq ($(DTB),system)
>  $(obj)/system.dtb: $(obj)/$(DTB).dtb
> -	$(call if_changed,cp)
> +	$(call if_changed,shipped)
>  endif
>  endif
>  
> -quiet_cmd_cp = CP      $< $@$2
> -	cmd_cp = cat $< >$@$2 || (rm -f $@ && echo false)
> -
>  # Rule to build device tree blobs
>  DTC_FLAGS := -p 1024
> 

Acked-by: Michal Simek <michal.simek@xilinx.com>

Thanks,
Michal


-- 
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Xilinx Microblaze
Maintainer of Linux kernel - Xilinx Zynq ARM and ZynqMP ARM64 SoCs
U-Boot custodian - Xilinx Microblaze/Zynq/ZynqMP/Versal SoCs



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCH 0/7] microblaze: fix various problems in building boot images

[Michal Simek]

[-- Attachment #1.1: Type: text/plain, Size: 1592 bytes --]

On 03. 12. 18 8:50, Masahiro Yamada wrote:
> This patch set fixes various issues in microblaze Makefiles.
> 
> BTW, "simpleImage.<dt>" works like a phony target to generate the
> following four images, where the first three are just aliases.
> 
>  - arch/microblaze/boot/simpleImage.<dt>:
>          identical to arch/microblaze/boot/linux.bin

It is not - fdt section should be empty.
simpleImage has this section filled.

> 
>  - arch/microblaze/boot/simpleImage.<dt>.unstrip:
>          identical to vmlinux

The same here with filled section.

> 
>  - arch/microblaze/boot/simpleImage.<dt>.ub:
>          identical to arch/microblaze/boot/linux.bin.ub

as above.

> 
>  - arch/microblaze/boot/simpleImage.<dt>.strip:
>          stripped vmlinux

And this is there because unstrip version is quite huge and for early
issues you need to know only some symbols that's why debugger is not
overflow with stuff which none needs.
Maybe this is not an issue now but that strip version is used a lot.


> 
> I am not sure how much useful those copies are,
> but, I tried my best to keep the same behavior.
> 
> IMHO, I guess DTB=<dt> would be more sensible,
> but it is up to Michal.

What do you mean by this exactly?

Definitely thanks for looking at this.
Michal



-- 
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Xilinx Microblaze
Maintainer of Linux kernel - Xilinx Zynq ARM and ZynqMP ARM64 SoCs
U-Boot custodian - Xilinx Microblaze/Zynq/ZynqMP/Versal SoCs



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCH 0/7] microblaze: fix various problems in building boot images

[Masahiro Yamada]

Hi Michal,

On Thu, Dec 6, 2018 at 1:41 AM Michal Simek <monstr@monstr.eu> wrote:
>
> On 03. 12. 18 8:50, Masahiro Yamada wrote:
> > This patch set fixes various issues in microblaze Makefiles.
> >
> > BTW, "simpleImage.<dt>" works like a phony target to generate the
> > following four images, where the first three are just aliases.
> >
> >  - arch/microblaze/boot/simpleImage.<dt>:
> >          identical to arch/microblaze/boot/linux.bin
>
> It is not - fdt section should be empty.
> simpleImage has this section filled.
>
> >
> >  - arch/microblaze/boot/simpleImage.<dt>.unstrip:
> >          identical to vmlinux
>
> The same here with filled section.


vmlinux is built anyway
for whatever target you are building.

What is the point of making a copy of vmlinux?
They are the same.
You can confirm it by 'diff'

$ export CROSS_COMPILE=microblaze-linux-
$ make ARCH=microblaze  defconfig
$ make -j8 ARCH=microblaze   simpleImage.system
$ diff arch/microblaze/boot/simpleImage.system.unstrip  vmlinux






> >
> >  - arch/microblaze/boot/simpleImage.<dt>.ub:
> >          identical to arch/microblaze/boot/linux.bin.ub
>
> as above.
>
> >
> >  - arch/microblaze/boot/simpleImage.<dt>.strip:
> >          stripped vmlinux
>
> And this is there because unstrip version is quite huge and for early
> issues you need to know only some symbols that's why debugger is not
> overflow with stuff which none needs.
> Maybe this is not an issue now but that strip version is used a lot.
>
>
> >
> > I am not sure how much useful those copies are,
> > but, I tried my best to keep the same behavior.
> >
> > IMHO, I guess DTB=<dt> would be more sensible,
> > but it is up to Michal.
>
> What do you mean by this exactly?


As I showed above,
arch/microblaze/boot/simpleImage.system.unstrip
is exactly the same as vmlinux.



arch/microblaze/boot/simpleImage.<dt>
is objcopy'ed binary of vmlinux.

arch/microblaze/boot/simpleImage.<dt>.ub
is objcopy'ed binary of vmlinux, with u-boot header prepended.

You have already build-rules for them.



It is true that the stripped image only exist in simpleImage,
but I think "arch/microblaze/boot/vmlinux.strip"
is a more sensible name.



What I want to point out is:
    "Which file should be compiled in",
    is a part of the configuration.
    We generally do not change the final
    target name just for the difference of
    configuration.
    For example, ARM just uses "vmlinux", "Image", "zImage", etc.
    Never duplicate target-specific copies depending on configuration.


Why does microblaze create copies for each DT
instead of using generic image like linux.bin, linux.bin.ub, etc. ?

If using generic image names is acceptable,
"make simpleImage.<dt>" is just a shorthand of
"make DTB=<dt> vmlinux linux.bin linux.bin.ub vmlinux.strip"


Only the benefit of this approach is,
you can keep all images for multiple boards at the same time.

$ make ARCH=microblaze simpleImage.board1
$ make ARCH=microblaze simpleImage.board2
$ make ARCH=microblaze simpleImage.board3




Since I do not know how many users rely on this usage,
I said "it is up to you".









> Definitely thanks for looking at this.
> Michal
>
>
>
> --
> Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
> w: www.monstr.eu p: +42-0-721842854
> Maintainer of Linux kernel - Xilinx Microblaze
> Maintainer of Linux kernel - Xilinx Zynq ARM and ZynqMP ARM64 SoCs
> U-Boot custodian - Xilinx Microblaze/Zynq/ZynqMP/Versal SoCs
>
>


-- 
Best Regards
Masahiro Yamada

Re: [PATCH 2/7] microblaze: adjust the help to the real behavior

[Masahiro Yamada]

Hi Michal,

On Thu, Dec 6, 2018 at 12:41 AM Michal Simek <monstr@monstr.eu> wrote:
>
> On 03. 12. 18 8:50, Masahiro Yamada wrote:
> > "make ARCH=microblaze help" mentions simpleImage.<dt>.unstrip,
> > but it never works because Makefile assumes "system.unstrip" is
> > the name of DT.
> >
> > $ make ARCH=microblaze CROSS_COMPILE=microblaze-linux- simpleImage.system.unstrip
> >   [ snip ]
> > make[1]: *** No rule to make target 'arch/microblaze/boot/dts/system.unstrip.dtb', needed by 'arch/microblaze/boot/dts/system.dtb'.  Stop.
> > make: *** [Makefile;1060: arch/microblaze/boot/dts] Error 2
> > make: *** Waiting for unfinished jobs....
> >
> > Rip off the never-working target.
> >
> > In my understanding, simpleImage.<dt> works like a phony target that
> > generates multiple images. Reflect the behavior to the help message.
> >
> > Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> > ---
> >
> >  arch/microblaze/Makefile | 4 +---
> >  1 file changed, 1 insertion(+), 3 deletions(-)
> >
> > diff --git a/arch/microblaze/Makefile b/arch/microblaze/Makefile
> > index 0823d29..97e1384 100644
> > --- a/arch/microblaze/Makefile
> > +++ b/arch/microblaze/Makefile
> > @@ -89,9 +89,7 @@ define archhelp
> >    echo '* linux.bin    - Create raw binary'
> >    echo '  linux.bin.gz - Create compressed raw binary'
> >    echo '  linux.bin.ub - Create U-Boot wrapped raw binary'
> > -  echo '  simpleImage.<dt> - ELF image with $(arch)/boot/dts/<dt>.dts linked in'
> > -  echo '                   - stripped elf with fdt blob'
> > -  echo '  simpleImage.<dt>.unstrip - full ELF image with fdt blob'
> > +  echo '  simpleImage.<dt> - Create images with $(arch)/boot/dts/<dt>.dts linked in'
> >    echo '  *_defconfig      - Select default config from arch/microblaze/configs'
> >    echo ''
> >    echo '  Targets with <dt> embed a device tree blob inside the image'
> >
>
> I understand what you are trying to say but I would still like to keep
> information about unstrip file.
> It is correct that it is not build target. It is just generated file and
> message above was used for description what it is.
> Definitely agree that this should be the part of targets but it should
> be in description.
> The same is for missing description for simpleImage.<dt>.strip and
> simpleImage.<dt>.ub files.
>
> Thanks,
> Michal
>

If we want to be precise to the current behavior,
we could describe more.
(Is it too much?)


Architecture specific targets (microblaze):
* linux.bin    - Create raw binary
  linux.bin.gz - Create compressed raw binary
  linux.bin.ub - Create U-Boot wrapped raw binary
  simpleImage.<dt> - Create the following images with
arch/macroblaze/boot/dts/<dt>.dts linked in
                     simpleImage.<dt>        : raw image
                     simpleImage.<dt>.ub     : raw image with U-Boot header
                     simpleImage.<dt>.unstrip: ELF (identical to vmlinux)
                     simpleImage.<dt>.strip  : stripped ELF
  *_defconfig      - Select default config from arch/microblaze/configs




If you want to modify as you like,
I will not touch it though.





BTW, "make ARCH=microblaze help" looks like follows:

* linux.bin    - Create raw binary
  linux.bin.gz - Create compressed raw binary
  linux.bin.ub - Create U-Boot wrapped raw binary
  simpleImage.<dt> - ELF image with /boot/dts/<dt>.dts linked in
                   - stripped elf with fdt blob
  simpleImage.<dt>.unstrip - full ELF image with fdt blob
  *_defconfig      - Select default config from arch/microblaze/configs




Since "arch" is not set anywhere, $(arch) is empty,
"ELF image with /boot/dts/<dt>.dts linked in" looks strange.



--
Best Regards
Masahiro Yamada

Re: [PATCH 2/7] microblaze: adjust the help to the real behavior

[Michal Simek]

[-- Attachment #1.1: Type: text/plain, Size: 4350 bytes --]

On 06. 12. 18 6:27, Masahiro Yamada wrote:
> Hi Michal,
> 
> On Thu, Dec 6, 2018 at 12:41 AM Michal Simek <monstr@monstr.eu> wrote:
>>
>> On 03. 12. 18 8:50, Masahiro Yamada wrote:
>>> "make ARCH=microblaze help" mentions simpleImage.<dt>.unstrip,
>>> but it never works because Makefile assumes "system.unstrip" is
>>> the name of DT.
>>>
>>> $ make ARCH=microblaze CROSS_COMPILE=microblaze-linux- simpleImage.system.unstrip
>>>   [ snip ]
>>> make[1]: *** No rule to make target 'arch/microblaze/boot/dts/system.unstrip.dtb', needed by 'arch/microblaze/boot/dts/system.dtb'.  Stop.
>>> make: *** [Makefile;1060: arch/microblaze/boot/dts] Error 2
>>> make: *** Waiting for unfinished jobs....
>>>
>>> Rip off the never-working target.
>>>
>>> In my understanding, simpleImage.<dt> works like a phony target that
>>> generates multiple images. Reflect the behavior to the help message.
>>>
>>> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
>>> ---
>>>
>>>  arch/microblaze/Makefile | 4 +---
>>>  1 file changed, 1 insertion(+), 3 deletions(-)
>>>
>>> diff --git a/arch/microblaze/Makefile b/arch/microblaze/Makefile
>>> index 0823d29..97e1384 100644
>>> --- a/arch/microblaze/Makefile
>>> +++ b/arch/microblaze/Makefile
>>> @@ -89,9 +89,7 @@ define archhelp
>>>    echo '* linux.bin    - Create raw binary'
>>>    echo '  linux.bin.gz - Create compressed raw binary'
>>>    echo '  linux.bin.ub - Create U-Boot wrapped raw binary'
>>> -  echo '  simpleImage.<dt> - ELF image with $(arch)/boot/dts/<dt>.dts linked in'
>>> -  echo '                   - stripped elf with fdt blob'
>>> -  echo '  simpleImage.<dt>.unstrip - full ELF image with fdt blob'
>>> +  echo '  simpleImage.<dt> - Create images with $(arch)/boot/dts/<dt>.dts linked in'
>>>    echo '  *_defconfig      - Select default config from arch/microblaze/configs'
>>>    echo ''
>>>    echo '  Targets with <dt> embed a device tree blob inside the image'
>>>
>>
>> I understand what you are trying to say but I would still like to keep
>> information about unstrip file.
>> It is correct that it is not build target. It is just generated file and
>> message above was used for description what it is.
>> Definitely agree that this should be the part of targets but it should
>> be in description.
>> The same is for missing description for simpleImage.<dt>.strip and
>> simpleImage.<dt>.ub files.
>>
>> Thanks,
>> Michal
>>
> 
> If we want to be precise to the current behavior,
> we could describe more.
> (Is it too much?)
> 
> 
> Architecture specific targets (microblaze):
> * linux.bin    - Create raw binary
>   linux.bin.gz - Create compressed raw binary
>   linux.bin.ub - Create U-Boot wrapped raw binary
>   simpleImage.<dt> - Create the following images with
> arch/macroblaze/boot/dts/<dt>.dts linked in

type - microblaze
maybe <dtb> here.

>                      simpleImage.<dt>        : raw image
>                      simpleImage.<dt>.ub     : raw image with U-Boot header
>                      simpleImage.<dt>.unstrip: ELF (identical to vmlinux)
>                      simpleImage.<dt>.strip  : stripped ELF
>   *_defconfig      - Select default config from arch/microblaze/configs
> 
> 
> 
> 
> If you want to modify as you like,
> I will not touch it though.

what you have above is fine for me.

> 
> 
> 
> 
> 
> BTW, "make ARCH=microblaze help" looks like follows:
> 
> * linux.bin    - Create raw binary
>   linux.bin.gz - Create compressed raw binary
>   linux.bin.ub - Create U-Boot wrapped raw binary
>   simpleImage.<dt> - ELF image with /boot/dts/<dt>.dts linked in
>                    - stripped elf with fdt blob
>   simpleImage.<dt>.unstrip - full ELF image with fdt blob
>   *_defconfig      - Select default config from arch/microblaze/configs
> 
> 
> 
> 
> Since "arch" is not set anywhere, $(arch) is empty,
> "ELF image with /boot/dts/<dt>.dts linked in" looks strange.

As I said good to fix it in a way you have above.

Thanks,
Michal

-- 
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Xilinx Microblaze
Maintainer of Linux kernel - Xilinx Zynq ARM and ZynqMP ARM64 SoCs
U-Boot custodian - Xilinx Microblaze/Zynq/ZynqMP/Versal SoCs



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCH 0/7] microblaze: fix various problems in building boot images

[Michal Simek]

[-- Attachment #1.1: Type: text/plain, Size: 4634 bytes --]

Hi,

On 06. 12. 18 6:08, Masahiro Yamada wrote:
> Hi Michal,
> 
> On Thu, Dec 6, 2018 at 1:41 AM Michal Simek <monstr@monstr.eu> wrote:
>>
>> On 03. 12. 18 8:50, Masahiro Yamada wrote:
>>> This patch set fixes various issues in microblaze Makefiles.
>>>
>>> BTW, "simpleImage.<dt>" works like a phony target to generate the
>>> following four images, where the first three are just aliases.
>>>
>>>  - arch/microblaze/boot/simpleImage.<dt>:
>>>          identical to arch/microblaze/boot/linux.bin
>>
>> It is not - fdt section should be empty.
>> simpleImage has this section filled.
>>
>>>
>>>  - arch/microblaze/boot/simpleImage.<dt>.unstrip:
>>>          identical to vmlinux
>>
>> The same here with filled section.
> 
> 
> vmlinux is built anyway
> for whatever target you are building.
> 
> What is the point of making a copy of vmlinux?
> They are the same.
> You can confirm it by 'diff'
> 
> $ export CROSS_COMPILE=microblaze-linux-
> $ make ARCH=microblaze  defconfig
> $ make -j8 ARCH=microblaze   simpleImage.system
> $ diff arch/microblaze/boot/simpleImage.system.unstrip  vmlinux

I can't remember the reason for this. Maybe it was just a copy from
PowerPC which started to use this simpleImage format in past and MB just
copied it.

>>>
>>>  - arch/microblaze/boot/simpleImage.<dt>.ub:
>>>          identical to arch/microblaze/boot/linux.bin.ub
>>
>> as above.
>>
>>>
>>>  - arch/microblaze/boot/simpleImage.<dt>.strip:
>>>          stripped vmlinux
>>
>> And this is there because unstrip version is quite huge and for early
>> issues you need to know only some symbols that's why debugger is not
>> overflow with stuff which none needs.
>> Maybe this is not an issue now but that strip version is used a lot.
>>
>>
>>>
>>> I am not sure how much useful those copies are,
>>> but, I tried my best to keep the same behavior.
>>>
>>> IMHO, I guess DTB=<dt> would be more sensible,
>>> but it is up to Michal.
>>
>> What do you mean by this exactly?
> 
> 
> As I showed above,
> arch/microblaze/boot/simpleImage.system.unstrip
> is exactly the same as vmlinux.
> 
> arch/microblaze/boot/simpleImage.<dt>
> is objcopy'ed binary of vmlinux.
> 
> arch/microblaze/boot/simpleImage.<dt>.ub
> is objcopy'ed binary of vmlinux, with u-boot header prepended.
> 
> You have already build-rules for them.
> 
> 
> 
> It is true that the stripped image only exist in simpleImage,
> but I think "arch/microblaze/boot/vmlinux.strip"
> is a more sensible name.
> 
> 
> 
> What I want to point out is:
>     "Which file should be compiled in",
>     is a part of the configuration.
>     We generally do not change the final
>     target name just for the difference of
>     configuration.
>     For example, ARM just uses "vmlinux", "Image", "zImage", etc.
>     Never duplicate target-specific copies depending on configuration.
> 
> 
> Why does microblaze create copies for each DT
> instead of using generic image like linux.bin, linux.bin.ub, etc. ?
> 
> If using generic image names is acceptable,
> "make simpleImage.<dt>" is just a shorthand of
> "make DTB=<dt> vmlinux linux.bin linux.bin.ub vmlinux.strip"
> 
> 
> Only the benefit of this approach is,
> you can keep all images for multiple boards at the same time.
> 
> $ make ARCH=microblaze simpleImage.board1
> $ make ARCH=microblaze simpleImage.board2
> $ make ARCH=microblaze simpleImage.board3

yes that's one thing which came to my mind too.

> 
> 
> 
> 
> Since I do not know how many users rely on this usage,
> I said "it is up to you".

One thing is what it is sensible and the second thing is historical
connection to that names. Because Xilinx was having ppc405 and ppc440
and microblaze as big endian architecture at that time was taking a lot
of stuff from powerpc that's why we took also that targets just to be
the same in distributions.
PPC was using simpleImage format in the same way that's why we have
adopted that too.

Personally for me it is not a problem to remove that simpleImage format
but I have no idea how many people rely on that.

I can't see any problem not to generate/copy simpleImage.<dt>.unstrip
version but I would keep the rest same as before just to make sure that
we are not breaking anybody.

Thanks,
Michal


-- 
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Xilinx Microblaze
Maintainer of Linux kernel - Xilinx Zynq ARM and ZynqMP ARM64 SoCs
U-Boot custodian - Xilinx Microblaze/Zynq/ZynqMP/Versal SoCs



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCH 0/7] microblaze: fix various problems in building boot images

[Michal Simek]

[-- Attachment #1.1: Type: text/plain, Size: 2460 bytes --]

On 03. 12. 18 8:50, Masahiro Yamada wrote:
> This patch set fixes various issues in microblaze Makefiles.
> 
> BTW, "simpleImage.<dt>" works like a phony target to generate the
> following four images, where the first three are just aliases.
> 
>  - arch/microblaze/boot/simpleImage.<dt>:
>          identical to arch/microblaze/boot/linux.bin
> 
>  - arch/microblaze/boot/simpleImage.<dt>.unstrip:
>          identical to vmlinux
> 
>  - arch/microblaze/boot/simpleImage.<dt>.ub:
>          identical to arch/microblaze/boot/linux.bin.ub
> 
>  - arch/microblaze/boot/simpleImage.<dt>.strip:
>          stripped vmlinux
> 
> I am not sure how much useful those copies are,
> but, I tried my best to keep the same behavior.
> 
> IMHO, I guess DTB=<dt> would be more sensible,
> but it is up to Michal.
> 
> 
> 
> Masahiro Yamada (7):
>   microblaze: fix cleaning of boot images
>   microblaze: adjust the help to the real behavior
>   microblaze: move "... is ready" message to arch/microblaze/Makefile
>   microblaze: fix multiple bugs in arch/microblaze/boot/Makefile
>   microblaze: add linux.bin* and simpleImage.* to PHONY
>   microblaze: fix race condition in building boot images
>   microblaze: remove the unneeded code just in case file copy fails
> 
>  arch/microblaze/Makefile          | 14 +++++++++-----
>  arch/microblaze/boot/Makefile     | 33 +++++++++++++++++----------------
>  arch/microblaze/boot/dts/Makefile |  5 +----
>  3 files changed, 27 insertions(+), 25 deletions(-)
> 

One more thing I have in my mind for a while is that will be good to
configure kernel build flags from DT and completely get rid of these
symbols.

XILINX_MICROBLAZE0_USE_MSR_INSTR
XILINX_MICROBLAZE0_USE_PCMP_INSTR
XILINX_MICROBLAZE0_USE_BARREL
XILINX_MICROBLAZE0_USE_DIV
XILINX_MICROBLAZE0_USE_HW_MUL
XILINX_MICROBLAZE0_USE_FPU

It means setup CPUFLAGS based on extracting that values from DT that it
all the time match the hardware.
It will also simplify all the CPUFLAGS logic which is in
arch/microblaze/Makefile.

Do you have any idea how this can be done?

(I have the same issue in U-Boot too)

Thanks,
Michal

-- 
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Xilinx Microblaze
Maintainer of Linux kernel - Xilinx Zynq ARM and ZynqMP ARM64 SoCs
U-Boot custodian - Xilinx Microblaze/Zynq/ZynqMP/Versal SoCs



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCH 2/7] microblaze: adjust the help to the real behavior

[Masahiro Yamada]

On Thu, Dec 6, 2018 at 9:54 PM Michal Simek <monstr@monstr.eu> wrote:
>
> On 06. 12. 18 6:27, Masahiro Yamada wrote:
> > Hi Michal,
> >
> > On Thu, Dec 6, 2018 at 12:41 AM Michal Simek <monstr@monstr.eu> wrote:
> >>
> >> On 03. 12. 18 8:50, Masahiro Yamada wrote:
> >>> "make ARCH=microblaze help" mentions simpleImage.<dt>.unstrip,
> >>> but it never works because Makefile assumes "system.unstrip" is
> >>> the name of DT.
> >>>
> >>> $ make ARCH=microblaze CROSS_COMPILE=microblaze-linux- simpleImage.system.unstrip
> >>>   [ snip ]
> >>> make[1]: *** No rule to make target 'arch/microblaze/boot/dts/system.unstrip.dtb', needed by 'arch/microblaze/boot/dts/system.dtb'.  Stop.
> >>> make: *** [Makefile;1060: arch/microblaze/boot/dts] Error 2
> >>> make: *** Waiting for unfinished jobs....
> >>>
> >>> Rip off the never-working target.
> >>>
> >>> In my understanding, simpleImage.<dt> works like a phony target that
> >>> generates multiple images. Reflect the behavior to the help message.
> >>>
> >>> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> >>> ---
> >>>
> >>>  arch/microblaze/Makefile | 4 +---
> >>>  1 file changed, 1 insertion(+), 3 deletions(-)
> >>>
> >>> diff --git a/arch/microblaze/Makefile b/arch/microblaze/Makefile
> >>> index 0823d29..97e1384 100644
> >>> --- a/arch/microblaze/Makefile
> >>> +++ b/arch/microblaze/Makefile
> >>> @@ -89,9 +89,7 @@ define archhelp
> >>>    echo '* linux.bin    - Create raw binary'
> >>>    echo '  linux.bin.gz - Create compressed raw binary'
> >>>    echo '  linux.bin.ub - Create U-Boot wrapped raw binary'
> >>> -  echo '  simpleImage.<dt> - ELF image with $(arch)/boot/dts/<dt>.dts linked in'
> >>> -  echo '                   - stripped elf with fdt blob'
> >>> -  echo '  simpleImage.<dt>.unstrip - full ELF image with fdt blob'
> >>> +  echo '  simpleImage.<dt> - Create images with $(arch)/boot/dts/<dt>.dts linked in'
> >>>    echo '  *_defconfig      - Select default config from arch/microblaze/configs'
> >>>    echo ''
> >>>    echo '  Targets with <dt> embed a device tree blob inside the image'
> >>>
> >>
> >> I understand what you are trying to say but I would still like to keep
> >> information about unstrip file.
> >> It is correct that it is not build target. It is just generated file and
> >> message above was used for description what it is.
> >> Definitely agree that this should be the part of targets but it should
> >> be in description.
> >> The same is for missing description for simpleImage.<dt>.strip and
> >> simpleImage.<dt>.ub files.
> >>
> >> Thanks,
> >> Michal
> >>
> >
> > If we want to be precise to the current behavior,
> > we could describe more.
> > (Is it too much?)
> >
> >
> > Architecture specific targets (microblaze):
> > * linux.bin    - Create raw binary
> >   linux.bin.gz - Create compressed raw binary
> >   linux.bin.ub - Create U-Boot wrapped raw binary
> >   simpleImage.<dt> - Create the following images with
> > arch/macroblaze/boot/dts/<dt>.dts linked in
>
> type - microblaze
> maybe <dtb> here.
>
> >                      simpleImage.<dt>        : raw image
> >                      simpleImage.<dt>.ub     : raw image with U-Boot header
> >                      simpleImage.<dt>.unstrip: ELF (identical to vmlinux)
> >                      simpleImage.<dt>.strip  : stripped ELF
> >   *_defconfig      - Select default config from arch/microblaze/configs
> >
> >
> >
> >
> > If you want to modify as you like,
> > I will not touch it though.
>
> what you have above is fine for me.


OK, I will send v2
with the typo fixed.

Thanks.




-- 
Best Regards
Masahiro Yamada

Re: [PATCH 2/7] microblaze: adjust the help to the real behavior

[Masahiro Yamada]

On Thu, Dec 6, 2018 at 9:54 PM Michal Simek <monstr@monstr.eu> wrote:
>
> On 06. 12. 18 6:27, Masahiro Yamada wrote:
> > Hi Michal,
> >
> > On Thu, Dec 6, 2018 at 12:41 AM Michal Simek <monstr@monstr.eu> wrote:
> >>
> >> On 03. 12. 18 8:50, Masahiro Yamada wrote:
> >>> "make ARCH=microblaze help" mentions simpleImage.<dt>.unstrip,
> >>> but it never works because Makefile assumes "system.unstrip" is
> >>> the name of DT.
> >>>
> >>> $ make ARCH=microblaze CROSS_COMPILE=microblaze-linux- simpleImage.system.unstrip
> >>>   [ snip ]
> >>> make[1]: *** No rule to make target 'arch/microblaze/boot/dts/system.unstrip.dtb', needed by 'arch/microblaze/boot/dts/system.dtb'.  Stop.
> >>> make: *** [Makefile;1060: arch/microblaze/boot/dts] Error 2
> >>> make: *** Waiting for unfinished jobs....
> >>>
> >>> Rip off the never-working target.
> >>>
> >>> In my understanding, simpleImage.<dt> works like a phony target that
> >>> generates multiple images. Reflect the behavior to the help message.
> >>>
> >>> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> >>> ---
> >>>
> >>>  arch/microblaze/Makefile | 4 +---
> >>>  1 file changed, 1 insertion(+), 3 deletions(-)
> >>>
> >>> diff --git a/arch/microblaze/Makefile b/arch/microblaze/Makefile
> >>> index 0823d29..97e1384 100644
> >>> --- a/arch/microblaze/Makefile
> >>> +++ b/arch/microblaze/Makefile
> >>> @@ -89,9 +89,7 @@ define archhelp
> >>>    echo '* linux.bin    - Create raw binary'
> >>>    echo '  linux.bin.gz - Create compressed raw binary'
> >>>    echo '  linux.bin.ub - Create U-Boot wrapped raw binary'
> >>> -  echo '  simpleImage.<dt> - ELF image with $(arch)/boot/dts/<dt>.dts linked in'
> >>> -  echo '                   - stripped elf with fdt blob'
> >>> -  echo '  simpleImage.<dt>.unstrip - full ELF image with fdt blob'
> >>> +  echo '  simpleImage.<dt> - Create images with $(arch)/boot/dts/<dt>.dts linked in'
> >>>    echo '  *_defconfig      - Select default config from arch/microblaze/configs'
> >>>    echo ''
> >>>    echo '  Targets with <dt> embed a device tree blob inside the image'
> >>>
> >>
> >> I understand what you are trying to say but I would still like to keep
> >> information about unstrip file.
> >> It is correct that it is not build target. It is just generated file and
> >> message above was used for description what it is.
> >> Definitely agree that this should be the part of targets but it should
> >> be in description.
> >> The same is for missing description for simpleImage.<dt>.strip and
> >> simpleImage.<dt>.ub files.
> >>
> >> Thanks,
> >> Michal
> >>
> >
> > If we want to be precise to the current behavior,
> > we could describe more.
> > (Is it too much?)
> >
> >
> > Architecture specific targets (microblaze):
> > * linux.bin    - Create raw binary
> >   linux.bin.gz - Create compressed raw binary
> >   linux.bin.ub - Create U-Boot wrapped raw binary
> >   simpleImage.<dt> - Create the following images with
> > arch/macroblaze/boot/dts/<dt>.dts linked in
>
> type - microblaze
> maybe <dtb> here.


I will keep <dt> as is.

I see more <dt> references a few lines below:


  echo '  Targets with <dt> embed a device tree blob inside the image'
  echo '  These targets support board with firmware that does not'
  echo '  support passing a device tree directly. Replace <dt> with the'
  echo '  name of a dts file from the arch/microblaze/boot/dts/ directory'
  echo '  (minus the .dts extension).'






-- 
Best Regards
Masahiro Yamada

Re: [PATCH 2/7] microblaze: adjust the help to the real behavior

[Michal Simek]

On 07. 12. 18 11:21, Masahiro Yamada wrote:
> On Thu, Dec 6, 2018 at 9:54 PM Michal Simek <monstr@monstr.eu> wrote:
>>
>> On 06. 12. 18 6:27, Masahiro Yamada wrote:
>>> Hi Michal,
>>>
>>> On Thu, Dec 6, 2018 at 12:41 AM Michal Simek <monstr@monstr.eu> wrote:
>>>>
>>>> On 03. 12. 18 8:50, Masahiro Yamada wrote:
>>>>> "make ARCH=microblaze help" mentions simpleImage.<dt>.unstrip,
>>>>> but it never works because Makefile assumes "system.unstrip" is
>>>>> the name of DT.
>>>>>
>>>>> $ make ARCH=microblaze CROSS_COMPILE=microblaze-linux- simpleImage.system.unstrip
>>>>>   [ snip ]
>>>>> make[1]: *** No rule to make target 'arch/microblaze/boot/dts/system.unstrip.dtb', needed by 'arch/microblaze/boot/dts/system.dtb'.  Stop.
>>>>> make: *** [Makefile;1060: arch/microblaze/boot/dts] Error 2
>>>>> make: *** Waiting for unfinished jobs....
>>>>>
>>>>> Rip off the never-working target.
>>>>>
>>>>> In my understanding, simpleImage.<dt> works like a phony target that
>>>>> generates multiple images. Reflect the behavior to the help message.
>>>>>
>>>>> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
>>>>> ---
>>>>>
>>>>>  arch/microblaze/Makefile | 4 +---
>>>>>  1 file changed, 1 insertion(+), 3 deletions(-)
>>>>>
>>>>> diff --git a/arch/microblaze/Makefile b/arch/microblaze/Makefile
>>>>> index 0823d29..97e1384 100644
>>>>> --- a/arch/microblaze/Makefile
>>>>> +++ b/arch/microblaze/Makefile
>>>>> @@ -89,9 +89,7 @@ define archhelp
>>>>>    echo '* linux.bin    - Create raw binary'
>>>>>    echo '  linux.bin.gz - Create compressed raw binary'
>>>>>    echo '  linux.bin.ub - Create U-Boot wrapped raw binary'
>>>>> -  echo '  simpleImage.<dt> - ELF image with $(arch)/boot/dts/<dt>.dts linked in'
>>>>> -  echo '                   - stripped elf with fdt blob'
>>>>> -  echo '  simpleImage.<dt>.unstrip - full ELF image with fdt blob'
>>>>> +  echo '  simpleImage.<dt> - Create images with $(arch)/boot/dts/<dt>.dts linked in'
>>>>>    echo '  *_defconfig      - Select default config from arch/microblaze/configs'
>>>>>    echo ''
>>>>>    echo '  Targets with <dt> embed a device tree blob inside the image'
>>>>>
>>>>
>>>> I understand what you are trying to say but I would still like to keep
>>>> information about unstrip file.
>>>> It is correct that it is not build target. It is just generated file and
>>>> message above was used for description what it is.
>>>> Definitely agree that this should be the part of targets but it should
>>>> be in description.
>>>> The same is for missing description for simpleImage.<dt>.strip and
>>>> simpleImage.<dt>.ub files.
>>>>
>>>> Thanks,
>>>> Michal
>>>>
>>>
>>> If we want to be precise to the current behavior,
>>> we could describe more.
>>> (Is it too much?)
>>>
>>>
>>> Architecture specific targets (microblaze):
>>> * linux.bin    - Create raw binary
>>>   linux.bin.gz - Create compressed raw binary
>>>   linux.bin.ub - Create U-Boot wrapped raw binary
>>>   simpleImage.<dt> - Create the following images with
>>> arch/macroblaze/boot/dts/<dt>.dts linked in
>>
>> type - microblaze
>> maybe <dtb> here.
> 
> 
> I will keep <dt> as is.
> 
> I see more <dt> references a few lines below:
> 
> 
>   echo '  Targets with <dt> embed a device tree blob inside the image'
>   echo '  These targets support board with firmware that does not'
>   echo '  support passing a device tree directly. Replace <dt> with the'
>   echo '  name of a dts file from the arch/microblaze/boot/dts/ directory'
>   echo '  (minus the .dts extension).'
> 

ok. Not a problem.
M

Re: [PATCH 0/7] microblaze: fix various problems in building boot images

[Masahiro Yamada]

On Thu, Dec 6, 2018 at 10:10 PM Michal Simek <monstr@monstr.eu> wrote:
>
> Hi,
>
> On 06. 12. 18 6:08, Masahiro Yamada wrote:
> > Hi Michal,
> >
> > On Thu, Dec 6, 2018 at 1:41 AM Michal Simek <monstr@monstr.eu> wrote:
> >>
> >> On 03. 12. 18 8:50, Masahiro Yamada wrote:
> >>> This patch set fixes various issues in microblaze Makefiles.
> >>>
> >>> BTW, "simpleImage.<dt>" works like a phony target to generate the
> >>> following four images, where the first three are just aliases.
> >>>
> >>>  - arch/microblaze/boot/simpleImage.<dt>:
> >>>          identical to arch/microblaze/boot/linux.bin
> >>
> >> It is not - fdt section should be empty.
> >> simpleImage has this section filled.
> >>
> >>>
> >>>  - arch/microblaze/boot/simpleImage.<dt>.unstrip:
> >>>          identical to vmlinux
> >>
> >> The same here with filled section.
> >
> >
> > vmlinux is built anyway
> > for whatever target you are building.
> >
> > What is the point of making a copy of vmlinux?
> > They are the same.
> > You can confirm it by 'diff'
> >
> > $ export CROSS_COMPILE=microblaze-linux-
> > $ make ARCH=microblaze  defconfig
> > $ make -j8 ARCH=microblaze   simpleImage.system
> > $ diff arch/microblaze/boot/simpleImage.system.unstrip  vmlinux
>
> I can't remember the reason for this. Maybe it was just a copy from
> PowerPC which started to use this simpleImage format in past and MB just
> copied it.
>
> >>>
> >>>  - arch/microblaze/boot/simpleImage.<dt>.ub:
> >>>          identical to arch/microblaze/boot/linux.bin.ub
> >>
> >> as above.
> >>
> >>>
> >>>  - arch/microblaze/boot/simpleImage.<dt>.strip:
> >>>          stripped vmlinux
> >>
> >> And this is there because unstrip version is quite huge and for early
> >> issues you need to know only some symbols that's why debugger is not
> >> overflow with stuff which none needs.
> >> Maybe this is not an issue now but that strip version is used a lot.
> >>
> >>
> >>>
> >>> I am not sure how much useful those copies are,
> >>> but, I tried my best to keep the same behavior.
> >>>
> >>> IMHO, I guess DTB=<dt> would be more sensible,
> >>> but it is up to Michal.
> >>
> >> What do you mean by this exactly?
> >
> >
> > As I showed above,
> > arch/microblaze/boot/simpleImage.system.unstrip
> > is exactly the same as vmlinux.
> >
> > arch/microblaze/boot/simpleImage.<dt>
> > is objcopy'ed binary of vmlinux.
> >
> > arch/microblaze/boot/simpleImage.<dt>.ub
> > is objcopy'ed binary of vmlinux, with u-boot header prepended.
> >
> > You have already build-rules for them.
> >
> >
> >
> > It is true that the stripped image only exist in simpleImage,
> > but I think "arch/microblaze/boot/vmlinux.strip"
> > is a more sensible name.
> >
> >
> >
> > What I want to point out is:
> >     "Which file should be compiled in",
> >     is a part of the configuration.
> >     We generally do not change the final
> >     target name just for the difference of
> >     configuration.
> >     For example, ARM just uses "vmlinux", "Image", "zImage", etc.
> >     Never duplicate target-specific copies depending on configuration.
> >
> >
> > Why does microblaze create copies for each DT
> > instead of using generic image like linux.bin, linux.bin.ub, etc. ?
> >
> > If using generic image names is acceptable,
> > "make simpleImage.<dt>" is just a shorthand of
> > "make DTB=<dt> vmlinux linux.bin linux.bin.ub vmlinux.strip"
> >
> >
> > Only the benefit of this approach is,
> > you can keep all images for multiple boards at the same time.
> >
> > $ make ARCH=microblaze simpleImage.board1
> > $ make ARCH=microblaze simpleImage.board2
> > $ make ARCH=microblaze simpleImage.board3
>
> yes that's one thing which came to my mind too.
>
> >
> >
> >
> >
> > Since I do not know how many users rely on this usage,
> > I said "it is up to you".
>
> One thing is what it is sensible and the second thing is historical
> connection to that names. Because Xilinx was having ppc405 and ppc440
> and microblaze as big endian architecture at that time was taking a lot
> of stuff from powerpc that's why we took also that targets just to be
> the same in distributions.
> PPC was using simpleImage format in the same way that's why we have
> adopted that too.
>
> Personally for me it is not a problem to remove that simpleImage format
> but I have no idea how many people rely on that.
>
> I can't see any problem not to generate/copy simpleImage.<dt>.unstrip
> version but I would keep the rest same as before just to make sure that
> we are not breaking anybody.


OK, let's keep all simpleImage images.


BTW, I noticed this series changed the behavior a bit.
"make simpleImage.<dt>" will overwrite linux.bin.ub
where linux.bin.ub should not contain built-in DT.

I will fix it just in case.


--
Best Regards
Masahiro Yamada

Re: [PATCH 0/7] microblaze: fix various problems in building boot images

[Masahiro Yamada]

On Thu, Dec 6, 2018 at 11:55 PM Michal Simek <monstr@monstr.eu> wrote:
>
> On 03. 12. 18 8:50, Masahiro Yamada wrote:
> > This patch set fixes various issues in microblaze Makefiles.
> >
> > BTW, "simpleImage.<dt>" works like a phony target to generate the
> > following four images, where the first three are just aliases.
> >
> >  - arch/microblaze/boot/simpleImage.<dt>:
> >          identical to arch/microblaze/boot/linux.bin
> >
> >  - arch/microblaze/boot/simpleImage.<dt>.unstrip:
> >          identical to vmlinux
> >
> >  - arch/microblaze/boot/simpleImage.<dt>.ub:
> >          identical to arch/microblaze/boot/linux.bin.ub
> >
> >  - arch/microblaze/boot/simpleImage.<dt>.strip:
> >          stripped vmlinux
> >
> > I am not sure how much useful those copies are,
> > but, I tried my best to keep the same behavior.
> >
> > IMHO, I guess DTB=<dt> would be more sensible,
> > but it is up to Michal.
> >
> >
> >
> > Masahiro Yamada (7):
> >   microblaze: fix cleaning of boot images
> >   microblaze: adjust the help to the real behavior
> >   microblaze: move "... is ready" message to arch/microblaze/Makefile
> >   microblaze: fix multiple bugs in arch/microblaze/boot/Makefile
> >   microblaze: add linux.bin* and simpleImage.* to PHONY
> >   microblaze: fix race condition in building boot images
> >   microblaze: remove the unneeded code just in case file copy fails
> >
> >  arch/microblaze/Makefile          | 14 +++++++++-----
> >  arch/microblaze/boot/Makefile     | 33 +++++++++++++++++----------------
> >  arch/microblaze/boot/dts/Makefile |  5 +----
> >  3 files changed, 27 insertions(+), 25 deletions(-)
> >
>
> One more thing I have in my mind for a while is that will be good to
> configure kernel build flags from DT and completely get rid of these
> symbols.
>
> XILINX_MICROBLAZE0_USE_MSR_INSTR
> XILINX_MICROBLAZE0_USE_PCMP_INSTR
> XILINX_MICROBLAZE0_USE_BARREL
> XILINX_MICROBLAZE0_USE_DIV
> XILINX_MICROBLAZE0_USE_HW_MUL
> XILINX_MICROBLAZE0_USE_FPU
>
> It means setup CPUFLAGS based on extracting that values from DT that it
> all the time match the hardware.
> It will also simplify all the CPUFLAGS logic which is in
> arch/microblaze/Makefile.
>
> Do you have any idea how this can be done?


I have no idea.

Parsing DTS with sed or something would be possible, but ugly.

In my opinion, the kernel should be multi platform image,
in other words, it is the least common denominator
of supported platforms.

So, the kernel should enable all features that may or may not be used
depending on platform.

I do not know if this works for MB.


-- 
Best Regards
Masahiro Yamada

Re: [PATCH 0/7] microblaze: fix various problems in building boot images

[Michal Simek]

On 07. 12. 18 12:29, Masahiro Yamada wrote:
> On Thu, Dec 6, 2018 at 11:55 PM Michal Simek <monstr@monstr.eu> wrote:
>>
>> On 03. 12. 18 8:50, Masahiro Yamada wrote:
>>> This patch set fixes various issues in microblaze Makefiles.
>>>
>>> BTW, "simpleImage.<dt>" works like a phony target to generate the
>>> following four images, where the first three are just aliases.
>>>
>>>  - arch/microblaze/boot/simpleImage.<dt>:
>>>          identical to arch/microblaze/boot/linux.bin
>>>
>>>  - arch/microblaze/boot/simpleImage.<dt>.unstrip:
>>>          identical to vmlinux
>>>
>>>  - arch/microblaze/boot/simpleImage.<dt>.ub:
>>>          identical to arch/microblaze/boot/linux.bin.ub
>>>
>>>  - arch/microblaze/boot/simpleImage.<dt>.strip:
>>>          stripped vmlinux
>>>
>>> I am not sure how much useful those copies are,
>>> but, I tried my best to keep the same behavior.
>>>
>>> IMHO, I guess DTB=<dt> would be more sensible,
>>> but it is up to Michal.
>>>
>>>
>>>
>>> Masahiro Yamada (7):
>>>   microblaze: fix cleaning of boot images
>>>   microblaze: adjust the help to the real behavior
>>>   microblaze: move "... is ready" message to arch/microblaze/Makefile
>>>   microblaze: fix multiple bugs in arch/microblaze/boot/Makefile
>>>   microblaze: add linux.bin* and simpleImage.* to PHONY
>>>   microblaze: fix race condition in building boot images
>>>   microblaze: remove the unneeded code just in case file copy fails
>>>
>>>  arch/microblaze/Makefile          | 14 +++++++++-----
>>>  arch/microblaze/boot/Makefile     | 33 +++++++++++++++++----------------
>>>  arch/microblaze/boot/dts/Makefile |  5 +----
>>>  3 files changed, 27 insertions(+), 25 deletions(-)
>>>
>>
>> One more thing I have in my mind for a while is that will be good to
>> configure kernel build flags from DT and completely get rid of these
>> symbols.
>>
>> XILINX_MICROBLAZE0_USE_MSR_INSTR
>> XILINX_MICROBLAZE0_USE_PCMP_INSTR
>> XILINX_MICROBLAZE0_USE_BARREL
>> XILINX_MICROBLAZE0_USE_DIV
>> XILINX_MICROBLAZE0_USE_HW_MUL
>> XILINX_MICROBLAZE0_USE_FPU
>>
>> It means setup CPUFLAGS based on extracting that values from DT that it
>> all the time match the hardware.
>> It will also simplify all the CPUFLAGS logic which is in
>> arch/microblaze/Makefile.
>>
>> Do you have any idea how this can be done?
> 
> 
> I have no idea.
> 
> Parsing DTS with sed or something would be possible, but ugly.
> 
> In my opinion, the kernel should be multi platform image,
> in other words, it is the least common denominator
> of supported platforms.
> 
> So, the kernel should enable all features that may or may not be used
> depending on platform.
> 
> I do not know if this works for MB.

Microblaze is soft core CPU where you can select if you want to have it
with multiplier, divider, barrel shifter, etc.
You can of course say that you don't have them and you have "universal"
kernel but very slow.
That's why user has to setup these configs which are converted to cflags
to say GCC what can be used.
And these configs can be simply parsed from dt.

For example like this based on dtb (quick hack)

for i in `echo use-msr-instr use-pcmp-instr use-barrel use-div
use-hw-mul use-fpu`; do
	UPPER=`echo $i | tr '-' '_' | tr '[a-z]' '[A-Z}'`
	echo $i $UPPER;

	VAR=`fdtget -t i $FILE/arch/microblaze/boot/dts/system.dtb /cpus/cpu@0/
xlnx,$i`
	FLAGS+="CONFIG_XILINX_MICROBLAZE0_${UPPER}=${VAR} "
done

make $FLAGS


When simpleImage is requested dt could be parsed to setup proper build
flags.

Thanks,
Michal

Re: [PATCH 0/7] microblaze: fix various problems in building boot images

[Michal Simek]

On 07. 12. 18 14:29, Michal Simek wrote:
> On 07. 12. 18 12:29, Masahiro Yamada wrote:
>> On Thu, Dec 6, 2018 at 11:55 PM Michal Simek <monstr@monstr.eu> wrote:
>>>
>>> On 03. 12. 18 8:50, Masahiro Yamada wrote:
>>>> This patch set fixes various issues in microblaze Makefiles.
>>>>
>>>> BTW, "simpleImage.<dt>" works like a phony target to generate the
>>>> following four images, where the first three are just aliases.
>>>>
>>>>  - arch/microblaze/boot/simpleImage.<dt>:
>>>>          identical to arch/microblaze/boot/linux.bin
>>>>
>>>>  - arch/microblaze/boot/simpleImage.<dt>.unstrip:
>>>>          identical to vmlinux
>>>>
>>>>  - arch/microblaze/boot/simpleImage.<dt>.ub:
>>>>          identical to arch/microblaze/boot/linux.bin.ub
>>>>
>>>>  - arch/microblaze/boot/simpleImage.<dt>.strip:
>>>>          stripped vmlinux
>>>>
>>>> I am not sure how much useful those copies are,
>>>> but, I tried my best to keep the same behavior.
>>>>
>>>> IMHO, I guess DTB=<dt> would be more sensible,
>>>> but it is up to Michal.
>>>>
>>>>
>>>>
>>>> Masahiro Yamada (7):
>>>>   microblaze: fix cleaning of boot images
>>>>   microblaze: adjust the help to the real behavior
>>>>   microblaze: move "... is ready" message to arch/microblaze/Makefile
>>>>   microblaze: fix multiple bugs in arch/microblaze/boot/Makefile
>>>>   microblaze: add linux.bin* and simpleImage.* to PHONY
>>>>   microblaze: fix race condition in building boot images
>>>>   microblaze: remove the unneeded code just in case file copy fails
>>>>
>>>>  arch/microblaze/Makefile          | 14 +++++++++-----
>>>>  arch/microblaze/boot/Makefile     | 33 +++++++++++++++++----------------
>>>>  arch/microblaze/boot/dts/Makefile |  5 +----
>>>>  3 files changed, 27 insertions(+), 25 deletions(-)
>>>>
>>>
>>> One more thing I have in my mind for a while is that will be good to
>>> configure kernel build flags from DT and completely get rid of these
>>> symbols.
>>>
>>> XILINX_MICROBLAZE0_USE_MSR_INSTR
>>> XILINX_MICROBLAZE0_USE_PCMP_INSTR
>>> XILINX_MICROBLAZE0_USE_BARREL
>>> XILINX_MICROBLAZE0_USE_DIV
>>> XILINX_MICROBLAZE0_USE_HW_MUL
>>> XILINX_MICROBLAZE0_USE_FPU
>>>
>>> It means setup CPUFLAGS based on extracting that values from DT that it
>>> all the time match the hardware.
>>> It will also simplify all the CPUFLAGS logic which is in
>>> arch/microblaze/Makefile.
>>>
>>> Do you have any idea how this can be done?
>>
>>
>> I have no idea.
>>
>> Parsing DTS with sed or something would be possible, but ugly.
>>
>> In my opinion, the kernel should be multi platform image,
>> in other words, it is the least common denominator
>> of supported platforms.
>>
>> So, the kernel should enable all features that may or may not be used
>> depending on platform.
>>
>> I do not know if this works for MB.
> 
> Microblaze is soft core CPU where you can select if you want to have it
> with multiplier, divider, barrel shifter, etc.
> You can of course say that you don't have them and you have "universal"
> kernel but very slow.
> That's why user has to setup these configs which are converted to cflags
> to say GCC what can be used.
> And these configs can be simply parsed from dt.
> 
> For example like this based on dtb (quick hack)
> 
> for i in `echo use-msr-instr use-pcmp-instr use-barrel use-div
> use-hw-mul use-fpu`; do
> 	UPPER=`echo $i | tr '-' '_' | tr '[a-z]' '[A-Z}'`
> 	echo $i $UPPER;
> 
> 	VAR=`fdtget -t i $FILE/arch/microblaze/boot/dts/system.dtb /cpus/cpu@0/
> xlnx,$i`
> 	FLAGS+="CONFIG_XILINX_MICROBLAZE0_${UPPER}=${VAR} "
> done
> 
> make $FLAGS
> 
> 
> When simpleImage is requested dt could be parsed to setup proper build
> flags.

One more thing I found is that I have done these changes

diff --git a/arch/microblaze/kernel/setup.c b/arch/microblaze/kernel/setup.c
index bbd6968ce55b..dc6a6fee3ae2 100644
--- a/arch/microblaze/kernel/setup.c
+++ b/arch/microblaze/kernel/setup.c
@@ -153,11 +153,13 @@ void __init machine_early_init(const char
*cmdline, unsigned int ram,
 #endif

 #if CONFIG_XILINX_MICROBLAZE0_USE_MSR_INSTR
+#error MSR is enabled
        if (msr) {
                pr_info("!!!Your kernel has setup MSR instruction but ");
                pr_cont("CPU don't have it %x\n", msr);
        }
 #else
+#error MSR is not enabled
        if (!msr) {
                pr_info("!!!Your kernel not setup MSR instruction but ");
                pr_cont("CPU have it %x\n", msr);

and run MSR with 1
make defconfig && make CONFIG_XILINX_MICROBLAZE0_USE_MSR_INSTR=1
arch/microblaze/kernel/setup.o
it errors #error MSR is enabled
and all is good.

And when I run MSR with 0
make defconfig && make CONFIG_XILINX_MICROBLAZE0_USE_MSR_INSTR=0
arch/microblaze/kernel/setup.o
also getting #error MSR is enabled
which is wrong.

Is there any rule what can be setup at compilation time?

Thanks,
Michal

Re: [PATCH 0/7] microblaze: fix various problems in building boot images

[Masahiro Yamada]

On Sat, Dec 8, 2018 at 12:20 AM Michal Simek <michal.simek@xilinx.com> wrote:
>
> On 07. 12. 18 14:29, Michal Simek wrote:
> > On 07. 12. 18 12:29, Masahiro Yamada wrote:
> >> On Thu, Dec 6, 2018 at 11:55 PM Michal Simek <monstr@monstr.eu> wrote:
> >>>
> >>> On 03. 12. 18 8:50, Masahiro Yamada wrote:
> >>>> This patch set fixes various issues in microblaze Makefiles.
> >>>>
> >>>> BTW, "simpleImage.<dt>" works like a phony target to generate the
> >>>> following four images, where the first three are just aliases.
> >>>>
> >>>>  - arch/microblaze/boot/simpleImage.<dt>:
> >>>>          identical to arch/microblaze/boot/linux.bin
> >>>>
> >>>>  - arch/microblaze/boot/simpleImage.<dt>.unstrip:
> >>>>          identical to vmlinux
> >>>>
> >>>>  - arch/microblaze/boot/simpleImage.<dt>.ub:
> >>>>          identical to arch/microblaze/boot/linux.bin.ub
> >>>>
> >>>>  - arch/microblaze/boot/simpleImage.<dt>.strip:
> >>>>          stripped vmlinux
> >>>>
> >>>> I am not sure how much useful those copies are,
> >>>> but, I tried my best to keep the same behavior.
> >>>>
> >>>> IMHO, I guess DTB=<dt> would be more sensible,
> >>>> but it is up to Michal.
> >>>>
> >>>>
> >>>>
> >>>> Masahiro Yamada (7):
> >>>>   microblaze: fix cleaning of boot images
> >>>>   microblaze: adjust the help to the real behavior
> >>>>   microblaze: move "... is ready" message to arch/microblaze/Makefile
> >>>>   microblaze: fix multiple bugs in arch/microblaze/boot/Makefile
> >>>>   microblaze: add linux.bin* and simpleImage.* to PHONY
> >>>>   microblaze: fix race condition in building boot images
> >>>>   microblaze: remove the unneeded code just in case file copy fails
> >>>>
> >>>>  arch/microblaze/Makefile          | 14 +++++++++-----
> >>>>  arch/microblaze/boot/Makefile     | 33 +++++++++++++++++----------------
> >>>>  arch/microblaze/boot/dts/Makefile |  5 +----
> >>>>  3 files changed, 27 insertions(+), 25 deletions(-)
> >>>>
> >>>
> >>> One more thing I have in my mind for a while is that will be good to
> >>> configure kernel build flags from DT and completely get rid of these
> >>> symbols.
> >>>
> >>> XILINX_MICROBLAZE0_USE_MSR_INSTR
> >>> XILINX_MICROBLAZE0_USE_PCMP_INSTR
> >>> XILINX_MICROBLAZE0_USE_BARREL
> >>> XILINX_MICROBLAZE0_USE_DIV
> >>> XILINX_MICROBLAZE0_USE_HW_MUL
> >>> XILINX_MICROBLAZE0_USE_FPU
> >>>
> >>> It means setup CPUFLAGS based on extracting that values from DT that it
> >>> all the time match the hardware.
> >>> It will also simplify all the CPUFLAGS logic which is in
> >>> arch/microblaze/Makefile.
> >>>
> >>> Do you have any idea how this can be done?
> >>
> >>
> >> I have no idea.
> >>
> >> Parsing DTS with sed or something would be possible, but ugly.
> >>
> >> In my opinion, the kernel should be multi platform image,
> >> in other words, it is the least common denominator
> >> of supported platforms.
> >>
> >> So, the kernel should enable all features that may or may not be used
> >> depending on platform.
> >>
> >> I do not know if this works for MB.
> >
> > Microblaze is soft core CPU where you can select if you want to have it
> > with multiplier, divider, barrel shifter, etc.
> > You can of course say that you don't have them and you have "universal"
> > kernel but very slow.
> > That's why user has to setup these configs which are converted to cflags
> > to say GCC what can be used.
> > And these configs can be simply parsed from dt.
> >
> > For example like this based on dtb (quick hack)
> >
> > for i in `echo use-msr-instr use-pcmp-instr use-barrel use-div
> > use-hw-mul use-fpu`; do
> >       UPPER=`echo $i | tr '-' '_' | tr '[a-z]' '[A-Z}'`
> >       echo $i $UPPER;
> >
> >       VAR=`fdtget -t i $FILE/arch/microblaze/boot/dts/system.dtb /cpus/cpu@0/
> > xlnx,$i`
> >       FLAGS+="CONFIG_XILINX_MICROBLAZE0_${UPPER}=${VAR} "
> > done
> >
> > make $FLAGS
> >
> >
> > When simpleImage is requested dt could be parsed to setup proper build
> > flags.
>
> One more thing I found is that I have done these changes
>
> diff --git a/arch/microblaze/kernel/setup.c b/arch/microblaze/kernel/setup.c
> index bbd6968ce55b..dc6a6fee3ae2 100644
> --- a/arch/microblaze/kernel/setup.c
> +++ b/arch/microblaze/kernel/setup.c
> @@ -153,11 +153,13 @@ void __init machine_early_init(const char
> *cmdline, unsigned int ram,
>  #endif
>
>  #if CONFIG_XILINX_MICROBLAZE0_USE_MSR_INSTR
> +#error MSR is enabled
>         if (msr) {
>                 pr_info("!!!Your kernel has setup MSR instruction but ");
>                 pr_cont("CPU don't have it %x\n", msr);
>         }
>  #else
> +#error MSR is not enabled
>         if (!msr) {
>                 pr_info("!!!Your kernel not setup MSR instruction but ");
>                 pr_cont("CPU have it %x\n", msr);
>
> and run MSR with 1
> make defconfig && make CONFIG_XILINX_MICROBLAZE0_USE_MSR_INSTR=1
> arch/microblaze/kernel/setup.o
> it errors #error MSR is enabled
> and all is good.
>
> And when I run MSR with 0
> make defconfig && make CONFIG_XILINX_MICROBLAZE0_USE_MSR_INSTR=0
> arch/microblaze/kernel/setup.o
> also getting #error MSR is enabled
> which is wrong.
>
> Is there any rule what can be setup at compilation time?

You are trying to do very specific things,
I do not have a clean solution.

I do not mind whatever you do in arch-Makefile.

If you look at stack_protector_prepare in arch/powerpc/Makefile,
it is parsing a file with awk to setup compiler flags.


-- 
Best Regards
Masahiro Yamada

Re: [PATCH 6/7] microblaze: fix race condition in building boot images

[Masahiro Yamada]

On Thu, Dec 6, 2018 at 1:32 AM Michal Simek <monstr@monstr.eu> wrote:
>
> On 03. 12. 18 8:50, Masahiro Yamada wrote:
> > I fixed a race condition in the parallel building of ARM in commit
> > 3939f3345050 ("ARM: 8418/1: add boot image dependencies to not
> > generate invalid images").
> >
> > I see the same problem for MicroBlaze too.
> >
> > "make -j<N> ARCH=microblaze all linux.bin.ub" results in a broken build
> > since two threads descend into arch/microblaze/boot simultaneously.
>
> I see also different problem that when I run that make above
> linux.bin.ub is not generated at all.


That's why I am fixing the problem.

Before the fix, the parallel build fails.


/usr/bin/mkimage: Can't read arch/microblaze/boot/linux.bin: Invalid argument
arch/microblaze/boot/Makefile:14: recipe for target
'arch/microblaze/boot/linux.bin.ub' failed
make[1]: *** [arch/microblaze/boot/linux.bin.ub] Error 1
make[1]: *** Deleting file 'arch/microblaze/boot/linux.bin.ub'
arch/microblaze/Makefile:87: recipe for target 'linux.bin.ub' failed
make: *** [linux.bin.ub] Error 2
make: *** Waiting for unfinished jobs....
  MODPOST 6 modules
Kernel: arch/microblaze/boot/linux.bin is ready  (#2)





>
> >
> > Add proper dependencies to avoid it.
> >
> > Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> > ---
> >
> >  arch/microblaze/Makefile | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/arch/microblaze/Makefile b/arch/microblaze/Makefile
> > index 7a5df02..544796d 100644
> > --- a/arch/microblaze/Makefile
> > +++ b/arch/microblaze/Makefile
> > @@ -79,13 +79,15 @@ all: linux.bin
> >  archclean:
> >       $(Q)$(MAKE) $(clean)=$(boot)
> >
> > +linux.bin.ub linux.bin.gz: linux.bin
> > +
> >  PHONY += linux.bin linux.bin.gz linux.bin.ub
> >  linux.bin linux.bin.gz linux.bin.ub: vmlinux
> >       $(Q)$(MAKE) $(build)=$(boot) $(boot)/$@
> >       @echo 'Kernel: $(boot)/$@ is ready' ' (#'`cat .version`')'
> >
> >  PHONY += simpleImage.$(DTB)
> > -simpleImage.$(DTB): vmlinux
> > +simpleImage.$(DTB): linux.bin.ub
>
> It looks weird that simpleImage requires linux.bin.ub.
> Is it really linux.bin.ub here or just linux.bin?


This is intentional to avoid a race in
"make simpleImage.<dt> linux.bin.ub"


But, I chose to make simpleImage* independent of linux.bin* in v2.

I hope you will like it.



> >       $(Q)$(MAKE) $(build)=$(boot) simple_images
> >       @echo 'Kernel: $(boot)/$@ is ready' ' (#'`cat .version`')'
> >
> >
>
>
> Thanks,
> Michal
>
> --
> Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
> w: www.monstr.eu p: +42-0-721842854
> Maintainer of Linux kernel - Xilinx Microblaze
> Maintainer of Linux kernel - Xilinx Zynq ARM and ZynqMP ARM64 SoCs
> U-Boot custodian - Xilinx Microblaze/Zynq/ZynqMP/Versal SoCs
>
>


-- 
Best Regards
Masahiro Yamada

Re: [PATCH 0/7] microblaze: fix various problems in building boot images

[Michal Simek]

On 08. 12. 18 7:14, Masahiro Yamada wrote:
> On Sat, Dec 8, 2018 at 12:20 AM Michal Simek <michal.simek@xilinx.com> wrote:
>>
>> On 07. 12. 18 14:29, Michal Simek wrote:
>>> On 07. 12. 18 12:29, Masahiro Yamada wrote:
>>>> On Thu, Dec 6, 2018 at 11:55 PM Michal Simek <monstr@monstr.eu> wrote:
>>>>>
>>>>> On 03. 12. 18 8:50, Masahiro Yamada wrote:
>>>>>> This patch set fixes various issues in microblaze Makefiles.
>>>>>>
>>>>>> BTW, "simpleImage.<dt>" works like a phony target to generate the
>>>>>> following four images, where the first three are just aliases.
>>>>>>
>>>>>>  - arch/microblaze/boot/simpleImage.<dt>:
>>>>>>          identical to arch/microblaze/boot/linux.bin
>>>>>>
>>>>>>  - arch/microblaze/boot/simpleImage.<dt>.unstrip:
>>>>>>          identical to vmlinux
>>>>>>
>>>>>>  - arch/microblaze/boot/simpleImage.<dt>.ub:
>>>>>>          identical to arch/microblaze/boot/linux.bin.ub
>>>>>>
>>>>>>  - arch/microblaze/boot/simpleImage.<dt>.strip:
>>>>>>          stripped vmlinux
>>>>>>
>>>>>> I am not sure how much useful those copies are,
>>>>>> but, I tried my best to keep the same behavior.
>>>>>>
>>>>>> IMHO, I guess DTB=<dt> would be more sensible,
>>>>>> but it is up to Michal.
>>>>>>
>>>>>>
>>>>>>
>>>>>> Masahiro Yamada (7):
>>>>>>   microblaze: fix cleaning of boot images
>>>>>>   microblaze: adjust the help to the real behavior
>>>>>>   microblaze: move "... is ready" message to arch/microblaze/Makefile
>>>>>>   microblaze: fix multiple bugs in arch/microblaze/boot/Makefile
>>>>>>   microblaze: add linux.bin* and simpleImage.* to PHONY
>>>>>>   microblaze: fix race condition in building boot images
>>>>>>   microblaze: remove the unneeded code just in case file copy fails
>>>>>>
>>>>>>  arch/microblaze/Makefile          | 14 +++++++++-----
>>>>>>  arch/microblaze/boot/Makefile     | 33 +++++++++++++++++----------------
>>>>>>  arch/microblaze/boot/dts/Makefile |  5 +----
>>>>>>  3 files changed, 27 insertions(+), 25 deletions(-)
>>>>>>
>>>>>
>>>>> One more thing I have in my mind for a while is that will be good to
>>>>> configure kernel build flags from DT and completely get rid of these
>>>>> symbols.
>>>>>
>>>>> XILINX_MICROBLAZE0_USE_MSR_INSTR
>>>>> XILINX_MICROBLAZE0_USE_PCMP_INSTR
>>>>> XILINX_MICROBLAZE0_USE_BARREL
>>>>> XILINX_MICROBLAZE0_USE_DIV
>>>>> XILINX_MICROBLAZE0_USE_HW_MUL
>>>>> XILINX_MICROBLAZE0_USE_FPU
>>>>>
>>>>> It means setup CPUFLAGS based on extracting that values from DT that it
>>>>> all the time match the hardware.
>>>>> It will also simplify all the CPUFLAGS logic which is in
>>>>> arch/microblaze/Makefile.
>>>>>
>>>>> Do you have any idea how this can be done?
>>>>
>>>>
>>>> I have no idea.
>>>>
>>>> Parsing DTS with sed or something would be possible, but ugly.
>>>>
>>>> In my opinion, the kernel should be multi platform image,
>>>> in other words, it is the least common denominator
>>>> of supported platforms.
>>>>
>>>> So, the kernel should enable all features that may or may not be used
>>>> depending on platform.
>>>>
>>>> I do not know if this works for MB.
>>>
>>> Microblaze is soft core CPU where you can select if you want to have it
>>> with multiplier, divider, barrel shifter, etc.
>>> You can of course say that you don't have them and you have "universal"
>>> kernel but very slow.
>>> That's why user has to setup these configs which are converted to cflags
>>> to say GCC what can be used.
>>> And these configs can be simply parsed from dt.
>>>
>>> For example like this based on dtb (quick hack)
>>>
>>> for i in `echo use-msr-instr use-pcmp-instr use-barrel use-div
>>> use-hw-mul use-fpu`; do
>>>       UPPER=`echo $i | tr '-' '_' | tr '[a-z]' '[A-Z}'`
>>>       echo $i $UPPER;
>>>
>>>       VAR=`fdtget -t i $FILE/arch/microblaze/boot/dts/system.dtb /cpus/cpu@0/
>>> xlnx,$i`
>>>       FLAGS+="CONFIG_XILINX_MICROBLAZE0_${UPPER}=${VAR} "
>>> done
>>>
>>> make $FLAGS
>>>
>>>
>>> When simpleImage is requested dt could be parsed to setup proper build
>>> flags.
>>
>> One more thing I found is that I have done these changes
>>
>> diff --git a/arch/microblaze/kernel/setup.c b/arch/microblaze/kernel/setup.c
>> index bbd6968ce55b..dc6a6fee3ae2 100644
>> --- a/arch/microblaze/kernel/setup.c
>> +++ b/arch/microblaze/kernel/setup.c
>> @@ -153,11 +153,13 @@ void __init machine_early_init(const char
>> *cmdline, unsigned int ram,
>>  #endif
>>
>>  #if CONFIG_XILINX_MICROBLAZE0_USE_MSR_INSTR
>> +#error MSR is enabled
>>         if (msr) {
>>                 pr_info("!!!Your kernel has setup MSR instruction but ");
>>                 pr_cont("CPU don't have it %x\n", msr);
>>         }
>>  #else
>> +#error MSR is not enabled
>>         if (!msr) {
>>                 pr_info("!!!Your kernel not setup MSR instruction but ");
>>                 pr_cont("CPU have it %x\n", msr);
>>
>> and run MSR with 1
>> make defconfig && make CONFIG_XILINX_MICROBLAZE0_USE_MSR_INSTR=1
>> arch/microblaze/kernel/setup.o
>> it errors #error MSR is enabled
>> and all is good.
>>
>> And when I run MSR with 0
>> make defconfig && make CONFIG_XILINX_MICROBLAZE0_USE_MSR_INSTR=0
>> arch/microblaze/kernel/setup.o
>> also getting #error MSR is enabled
>> which is wrong.
>>
>> Is there any rule what can be setup at compilation time?
> 
> You are trying to do very specific things,
> I do not have a clean solution.
> 
> I do not mind whatever you do in arch-Makefile.

ok.

> 
> If you look at stack_protector_prepare in arch/powerpc/Makefile,
> it is parsing a file with awk to setup compiler flags.

Ok. Will do more experiments with it.

Can you please at least confirm me that config options passed via
command line as above should be used instead of that one in .config?
(Just want to understand why USE_MSR is so special that it is not
working properly).

Thanks,
Michal

Re: [PATCH 0/7] microblaze: fix various problems in building boot images

[Masahiro Yamada]

On Wed, Dec 12, 2018 at 10:50 PM Michal Simek <michal.simek@xilinx.com> wrote:
>
> On 08. 12. 18 7:14, Masahiro Yamada wrote:
> > On Sat, Dec 8, 2018 at 12:20 AM Michal Simek <michal.simek@xilinx.com> wrote:
> >>
> >> On 07. 12. 18 14:29, Michal Simek wrote:
> >>> On 07. 12. 18 12:29, Masahiro Yamada wrote:
> >>>> On Thu, Dec 6, 2018 at 11:55 PM Michal Simek <monstr@monstr.eu> wrote:
> >>>>>
> >>>>> On 03. 12. 18 8:50, Masahiro Yamada wrote:
> >>>>>> This patch set fixes various issues in microblaze Makefiles.
> >>>>>>
> >>>>>> BTW, "simpleImage.<dt>" works like a phony target to generate the
> >>>>>> following four images, where the first three are just aliases.
> >>>>>>
> >>>>>>  - arch/microblaze/boot/simpleImage.<dt>:
> >>>>>>          identical to arch/microblaze/boot/linux.bin
> >>>>>>
> >>>>>>  - arch/microblaze/boot/simpleImage.<dt>.unstrip:
> >>>>>>          identical to vmlinux
> >>>>>>
> >>>>>>  - arch/microblaze/boot/simpleImage.<dt>.ub:
> >>>>>>          identical to arch/microblaze/boot/linux.bin.ub
> >>>>>>
> >>>>>>  - arch/microblaze/boot/simpleImage.<dt>.strip:
> >>>>>>          stripped vmlinux
> >>>>>>
> >>>>>> I am not sure how much useful those copies are,
> >>>>>> but, I tried my best to keep the same behavior.
> >>>>>>
> >>>>>> IMHO, I guess DTB=<dt> would be more sensible,
> >>>>>> but it is up to Michal.
> >>>>>>
> >>>>>>
> >>>>>>
> >>>>>> Masahiro Yamada (7):
> >>>>>>   microblaze: fix cleaning of boot images
> >>>>>>   microblaze: adjust the help to the real behavior
> >>>>>>   microblaze: move "... is ready" message to arch/microblaze/Makefile
> >>>>>>   microblaze: fix multiple bugs in arch/microblaze/boot/Makefile
> >>>>>>   microblaze: add linux.bin* and simpleImage.* to PHONY
> >>>>>>   microblaze: fix race condition in building boot images
> >>>>>>   microblaze: remove the unneeded code just in case file copy fails
> >>>>>>
> >>>>>>  arch/microblaze/Makefile          | 14 +++++++++-----
> >>>>>>  arch/microblaze/boot/Makefile     | 33 +++++++++++++++++----------------
> >>>>>>  arch/microblaze/boot/dts/Makefile |  5 +----
> >>>>>>  3 files changed, 27 insertions(+), 25 deletions(-)
> >>>>>>
> >>>>>
> >>>>> One more thing I have in my mind for a while is that will be good to
> >>>>> configure kernel build flags from DT and completely get rid of these
> >>>>> symbols.
> >>>>>
> >>>>> XILINX_MICROBLAZE0_USE_MSR_INSTR
> >>>>> XILINX_MICROBLAZE0_USE_PCMP_INSTR
> >>>>> XILINX_MICROBLAZE0_USE_BARREL
> >>>>> XILINX_MICROBLAZE0_USE_DIV
> >>>>> XILINX_MICROBLAZE0_USE_HW_MUL
> >>>>> XILINX_MICROBLAZE0_USE_FPU
> >>>>>
> >>>>> It means setup CPUFLAGS based on extracting that values from DT that it
> >>>>> all the time match the hardware.
> >>>>> It will also simplify all the CPUFLAGS logic which is in
> >>>>> arch/microblaze/Makefile.
> >>>>>
> >>>>> Do you have any idea how this can be done?
> >>>>
> >>>>
> >>>> I have no idea.
> >>>>
> >>>> Parsing DTS with sed or something would be possible, but ugly.
> >>>>
> >>>> In my opinion, the kernel should be multi platform image,
> >>>> in other words, it is the least common denominator
> >>>> of supported platforms.
> >>>>
> >>>> So, the kernel should enable all features that may or may not be used
> >>>> depending on platform.
> >>>>
> >>>> I do not know if this works for MB.
> >>>
> >>> Microblaze is soft core CPU where you can select if you want to have it
> >>> with multiplier, divider, barrel shifter, etc.
> >>> You can of course say that you don't have them and you have "universal"
> >>> kernel but very slow.
> >>> That's why user has to setup these configs which are converted to cflags
> >>> to say GCC what can be used.
> >>> And these configs can be simply parsed from dt.
> >>>
> >>> For example like this based on dtb (quick hack)
> >>>
> >>> for i in `echo use-msr-instr use-pcmp-instr use-barrel use-div
> >>> use-hw-mul use-fpu`; do
> >>>       UPPER=`echo $i | tr '-' '_' | tr '[a-z]' '[A-Z}'`
> >>>       echo $i $UPPER;
> >>>
> >>>       VAR=`fdtget -t i $FILE/arch/microblaze/boot/dts/system.dtb /cpus/cpu@0/
> >>> xlnx,$i`
> >>>       FLAGS+="CONFIG_XILINX_MICROBLAZE0_${UPPER}=${VAR} "
> >>> done
> >>>
> >>> make $FLAGS
> >>>
> >>>
> >>> When simpleImage is requested dt could be parsed to setup proper build
> >>> flags.
> >>
> >> One more thing I found is that I have done these changes
> >>
> >> diff --git a/arch/microblaze/kernel/setup.c b/arch/microblaze/kernel/setup.c
> >> index bbd6968ce55b..dc6a6fee3ae2 100644
> >> --- a/arch/microblaze/kernel/setup.c
> >> +++ b/arch/microblaze/kernel/setup.c
> >> @@ -153,11 +153,13 @@ void __init machine_early_init(const char
> >> *cmdline, unsigned int ram,
> >>  #endif
> >>
> >>  #if CONFIG_XILINX_MICROBLAZE0_USE_MSR_INSTR
> >> +#error MSR is enabled
> >>         if (msr) {
> >>                 pr_info("!!!Your kernel has setup MSR instruction but ");
> >>                 pr_cont("CPU don't have it %x\n", msr);
> >>         }
> >>  #else
> >> +#error MSR is not enabled
> >>         if (!msr) {
> >>                 pr_info("!!!Your kernel not setup MSR instruction but ");
> >>                 pr_cont("CPU have it %x\n", msr);
> >>
> >> and run MSR with 1
> >> make defconfig && make CONFIG_XILINX_MICROBLAZE0_USE_MSR_INSTR=1
> >> arch/microblaze/kernel/setup.o
> >> it errors #error MSR is enabled
> >> and all is good.
> >>
> >> And when I run MSR with 0
> >> make defconfig && make CONFIG_XILINX_MICROBLAZE0_USE_MSR_INSTR=0
> >> arch/microblaze/kernel/setup.o
> >> also getting #error MSR is enabled
> >> which is wrong.
> >>
> >> Is there any rule what can be setup at compilation time?
> >
> > You are trying to do very specific things,
> > I do not have a clean solution.
> >
> > I do not mind whatever you do in arch-Makefile.
>
> ok.
>
> >
> > If you look at stack_protector_prepare in arch/powerpc/Makefile,
> > it is parsing a file with awk to setup compiler flags.
>
> Ok. Will do more experiments with it.
>
> Can you please at least confirm me that config options passed via
> command line as above should be used instead of that one in .config?
> (Just want to understand why USE_MSR is so special that it is not
> working properly).




make defconfig && make CONFIG_XILINX_MICROBLAZE0_USE_MSR_INSTR=1

does not work.

It overrides only references in Makefiles.



C files still refer to include/generated/autoconf.h
which is created based on Kconfig.


KBUILD_CPPFLAGS +=
-DCONFIG_XILINX_MICROBLAZE0_USE_MSR_INSTR=$(CONFIG_XILINX_MICROBLAZE0_USE_MSR_INSTR)

might work although it is ugly.

(At least, CONFIG_ prefix should be ripped off)



-- 
Best Regards
Masahiro Yamada

Re: [PATCH 0/7] microblaze: fix various problems in building boot images

[Masahiro Yamada]

On Wed, Dec 12, 2018 at 11:11 PM Masahiro Yamada
<yamada.masahiro@socionext.com> wrote:
>
> On Wed, Dec 12, 2018 at 10:50 PM Michal Simek <michal.simek@xilinx.com> wrote:
> >
> > On 08. 12. 18 7:14, Masahiro Yamada wrote:
> > > On Sat, Dec 8, 2018 at 12:20 AM Michal Simek <michal.simek@xilinx.com> wrote:
> > >>
> > >> On 07. 12. 18 14:29, Michal Simek wrote:
> > >>> On 07. 12. 18 12:29, Masahiro Yamada wrote:
> > >>>> On Thu, Dec 6, 2018 at 11:55 PM Michal Simek <monstr@monstr.eu> wrote:
> > >>>>>
> > >>>>> On 03. 12. 18 8:50, Masahiro Yamada wrote:
> > >>>>>> This patch set fixes various issues in microblaze Makefiles.
> > >>>>>>
> > >>>>>> BTW, "simpleImage.<dt>" works like a phony target to generate the
> > >>>>>> following four images, where the first three are just aliases.
> > >>>>>>
> > >>>>>>  - arch/microblaze/boot/simpleImage.<dt>:
> > >>>>>>          identical to arch/microblaze/boot/linux.bin
> > >>>>>>
> > >>>>>>  - arch/microblaze/boot/simpleImage.<dt>.unstrip:
> > >>>>>>          identical to vmlinux
> > >>>>>>
> > >>>>>>  - arch/microblaze/boot/simpleImage.<dt>.ub:
> > >>>>>>          identical to arch/microblaze/boot/linux.bin.ub
> > >>>>>>
> > >>>>>>  - arch/microblaze/boot/simpleImage.<dt>.strip:
> > >>>>>>          stripped vmlinux
> > >>>>>>
> > >>>>>> I am not sure how much useful those copies are,
> > >>>>>> but, I tried my best to keep the same behavior.
> > >>>>>>
> > >>>>>> IMHO, I guess DTB=<dt> would be more sensible,
> > >>>>>> but it is up to Michal.
> > >>>>>>
> > >>>>>>
> > >>>>>>
> > >>>>>> Masahiro Yamada (7):
> > >>>>>>   microblaze: fix cleaning of boot images
> > >>>>>>   microblaze: adjust the help to the real behavior
> > >>>>>>   microblaze: move "... is ready" message to arch/microblaze/Makefile
> > >>>>>>   microblaze: fix multiple bugs in arch/microblaze/boot/Makefile
> > >>>>>>   microblaze: add linux.bin* and simpleImage.* to PHONY
> > >>>>>>   microblaze: fix race condition in building boot images
> > >>>>>>   microblaze: remove the unneeded code just in case file copy fails
> > >>>>>>
> > >>>>>>  arch/microblaze/Makefile          | 14 +++++++++-----
> > >>>>>>  arch/microblaze/boot/Makefile     | 33 +++++++++++++++++----------------
> > >>>>>>  arch/microblaze/boot/dts/Makefile |  5 +----
> > >>>>>>  3 files changed, 27 insertions(+), 25 deletions(-)
> > >>>>>>
> > >>>>>
> > >>>>> One more thing I have in my mind for a while is that will be good to
> > >>>>> configure kernel build flags from DT and completely get rid of these
> > >>>>> symbols.
> > >>>>>
> > >>>>> XILINX_MICROBLAZE0_USE_MSR_INSTR
> > >>>>> XILINX_MICROBLAZE0_USE_PCMP_INSTR
> > >>>>> XILINX_MICROBLAZE0_USE_BARREL
> > >>>>> XILINX_MICROBLAZE0_USE_DIV
> > >>>>> XILINX_MICROBLAZE0_USE_HW_MUL
> > >>>>> XILINX_MICROBLAZE0_USE_FPU
> > >>>>>
> > >>>>> It means setup CPUFLAGS based on extracting that values from DT that it
> > >>>>> all the time match the hardware.
> > >>>>> It will also simplify all the CPUFLAGS logic which is in
> > >>>>> arch/microblaze/Makefile.
> > >>>>>
> > >>>>> Do you have any idea how this can be done?
> > >>>>
> > >>>>
> > >>>> I have no idea.
> > >>>>
> > >>>> Parsing DTS with sed or something would be possible, but ugly.
> > >>>>
> > >>>> In my opinion, the kernel should be multi platform image,
> > >>>> in other words, it is the least common denominator
> > >>>> of supported platforms.
> > >>>>
> > >>>> So, the kernel should enable all features that may or may not be used
> > >>>> depending on platform.
> > >>>>
> > >>>> I do not know if this works for MB.
> > >>>
> > >>> Microblaze is soft core CPU where you can select if you want to have it
> > >>> with multiplier, divider, barrel shifter, etc.
> > >>> You can of course say that you don't have them and you have "universal"
> > >>> kernel but very slow.
> > >>> That's why user has to setup these configs which are converted to cflags
> > >>> to say GCC what can be used.
> > >>> And these configs can be simply parsed from dt.
> > >>>
> > >>> For example like this based on dtb (quick hack)
> > >>>
> > >>> for i in `echo use-msr-instr use-pcmp-instr use-barrel use-div
> > >>> use-hw-mul use-fpu`; do
> > >>>       UPPER=`echo $i | tr '-' '_' | tr '[a-z]' '[A-Z}'`
> > >>>       echo $i $UPPER;
> > >>>
> > >>>       VAR=`fdtget -t i $FILE/arch/microblaze/boot/dts/system.dtb /cpus/cpu@0/
> > >>> xlnx,$i`
> > >>>       FLAGS+="CONFIG_XILINX_MICROBLAZE0_${UPPER}=${VAR} "
> > >>> done
> > >>>
> > >>> make $FLAGS
> > >>>
> > >>>
> > >>> When simpleImage is requested dt could be parsed to setup proper build
> > >>> flags.
> > >>
> > >> One more thing I found is that I have done these changes
> > >>
> > >> diff --git a/arch/microblaze/kernel/setup.c b/arch/microblaze/kernel/setup.c
> > >> index bbd6968ce55b..dc6a6fee3ae2 100644
> > >> --- a/arch/microblaze/kernel/setup.c
> > >> +++ b/arch/microblaze/kernel/setup.c
> > >> @@ -153,11 +153,13 @@ void __init machine_early_init(const char
> > >> *cmdline, unsigned int ram,
> > >>  #endif
> > >>
> > >>  #if CONFIG_XILINX_MICROBLAZE0_USE_MSR_INSTR
> > >> +#error MSR is enabled
> > >>         if (msr) {
> > >>                 pr_info("!!!Your kernel has setup MSR instruction but ");
> > >>                 pr_cont("CPU don't have it %x\n", msr);
> > >>         }
> > >>  #else
> > >> +#error MSR is not enabled
> > >>         if (!msr) {
> > >>                 pr_info("!!!Your kernel not setup MSR instruction but ");
> > >>                 pr_cont("CPU have it %x\n", msr);
> > >>
> > >> and run MSR with 1
> > >> make defconfig && make CONFIG_XILINX_MICROBLAZE0_USE_MSR_INSTR=1
> > >> arch/microblaze/kernel/setup.o
> > >> it errors #error MSR is enabled
> > >> and all is good.
> > >>
> > >> And when I run MSR with 0
> > >> make defconfig && make CONFIG_XILINX_MICROBLAZE0_USE_MSR_INSTR=0
> > >> arch/microblaze/kernel/setup.o
> > >> also getting #error MSR is enabled
> > >> which is wrong.
> > >>
> > >> Is there any rule what can be setup at compilation time?
> > >
> > > You are trying to do very specific things,
> > > I do not have a clean solution.
> > >
> > > I do not mind whatever you do in arch-Makefile.
> >
> > ok.
> >
> > >
> > > If you look at stack_protector_prepare in arch/powerpc/Makefile,
> > > it is parsing a file with awk to setup compiler flags.
> >
> > Ok. Will do more experiments with it.
> >
> > Can you please at least confirm me that config options passed via
> > command line as above should be used instead of that one in .config?
> > (Just want to understand why USE_MSR is so special that it is not
> > working properly).
>
>
>
>
> make defconfig && make CONFIG_XILINX_MICROBLAZE0_USE_MSR_INSTR=1
>
> does not work.
>
> It overrides only references in Makefiles.
>
>
>
> C files still refer to include/generated/autoconf.h
> which is created based on Kconfig.
>
>
> KBUILD_CPPFLAGS +=
> -DCONFIG_XILINX_MICROBLAZE0_USE_MSR_INSTR=$(CONFIG_XILINX_MICROBLAZE0_USE_MSR_INSTR)
>
> might work although it is ugly.
>
> (At least, CONFIG_ prefix should be ripped off)



Perhaps, another idea might be merge_config.


arch/mips/Makefile manipulates .config
by using scripts/kconfig/merge_config.sh


You can enable/disable CONFIG_XILINX_MICROBLAZE0_USE*
based on the information extracted from DT.




-- 
Best Regards
Masahiro Yamada