| Message ID | 1543823457-32478-1-git-send-email-yamada.masahiro@socionext.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | microblaze: fix various problems in building boot images | expand |
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
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 > >
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
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
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
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.
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
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
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.
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
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)
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.
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(-)