Re: [PATCH 1/5] kconfig: remove unneeded setsym label in conf_read_simple()

2018-12-07 Thread Masahiro Yamada
On Fri, Nov 30, 2018 at 6:17 PM Masahiro Yamada
 wrote:
>
> The two 'goto setsym' statements are reachable only when sym == NULL.
>
> The code below the 'setsym:' label does nothing when sym == NULL
> since there is just one if-block guarded by 'if (sym && ...)'.
>
> Hence, 'goto setsym' can be replaced with 'continue'.
>
> Signed-off-by: Masahiro Yamada 
> ---

Series, applied to linux-kbuild/kconfig.



>  scripts/kconfig/confdata.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/scripts/kconfig/confdata.c b/scripts/kconfig/confdata.c
> index 91d0a5c..1e35529 100644
> --- a/scripts/kconfig/confdata.c
> +++ b/scripts/kconfig/confdata.c
> @@ -363,7 +363,7 @@ int conf_read_simple(const char *name, int def)
> sym = sym_find(line + 2 + strlen(CONFIG_));
> if (!sym) {
> sym_add_change_count(1);
> -   goto setsym;
> +   continue;
> }
> } else {
> sym = sym_lookup(line + 2 + strlen(CONFIG_), 
> 0);
> @@ -397,7 +397,7 @@ int conf_read_simple(const char *name, int def)
> sym = sym_find(line + strlen(CONFIG_));
> if (!sym) {
> sym_add_change_count(1);
> -   goto setsym;
> +   continue;
> }
> } else {
> sym = sym_lookup(line + strlen(CONFIG_), 0);
> @@ -416,7 +416,7 @@ int conf_read_simple(const char *name, int def)
>
> continue;
> }
> -setsym:
> +
> if (sym && sym_is_choice_value(sym)) {
> struct symbol *cs = 
> prop_get_symbol(sym_get_choice_prop(sym));
> switch (sym->def[def].tri) {
> --
> 2.7.4
>


-- 
Best Regards
Masahiro Yamada


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

2018-12-07 Thread Masahiro Yamada
On Thu, Dec 6, 2018 at 1:32 AM Michal Simek  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 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 
> > ---
> >
> >  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. 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

2018-12-07 Thread Masahiro Yamada
On Sat, Dec 8, 2018 at 12:20 AM Michal Simek  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  wrote:
> >>>
> >>> On 03. 12. 18 8:50, Masahiro Yamada wrote:
> >>>> This patch set fixes various issues in microblaze Makefiles.
> >>>>
> >>>> BTW, "simpleImage." works like a phony target to generate the
> >>>> following four images, where the first three are just aliases.
> >>>>
> >>>>  - arch/microblaze/boot/simpleImage.:
> >>>>  identical to arch/microblaze/boot/linux.bin
> >>>>
> >>>>  - arch/microblaze/boot/simpleImage..unstrip:
> >>>>  identical to vmlinux
> >>>>
> >>>>  - arch/microblaze/boot/simpleImage..ub:
> >>>>  identical to arch/microblaze/boot/linux.bin.ub
> >>>>
> >>>>  - arch/microblaze/boot/simpleImage..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= 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 d

[PATCH 1/6] microblaze: adjust the help to the real behavior

2018-12-07 Thread Masahiro Yamada
"make ARCH=microblaze help" mentions simpleImage..unstrip,
but it is not a real Make target. It does not work 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

simpleImage. works like a phony target that generates multiple
images. Reflect the real behavior. I removed the DT directory path
information because it is already explained a few lines below.

While I am here, I deleted the redundant *_defconfig explanation.

The top-level Makefile caters to list available defconfig files:

  mmu_defconfig- Build for mmu
  nommu_defconfig  - Build for nommu

Signed-off-by: Masahiro Yamada 
---

Changes in v2:
  - Show all the four images in help
  - Delete redundant *_defconfig explanation

 arch/microblaze/Makefile | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/arch/microblaze/Makefile b/arch/microblaze/Makefile
index ddb251a..f6b7ea6 100644
--- a/arch/microblaze/Makefile
+++ b/arch/microblaze/Makefile
@@ -92,11 +92,11 @@ 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. - ELF image with $(arch)/boot/dts/.dts linked 
in'
-  echo '   - stripped elf with fdt blob'
-  echo '  simpleImage..unstrip - full ELF image with fdt blob'
-  echo '  *_defconfig  - Select default config from 
arch/microblaze/configs'
-  echo ''
+  echo '  simpleImage. - Create the following images with .dtb linked 
in'
+  echo 'simpleImage.: raw image'
+  echo 'simpleImage..ub : raw image with U-Boot 
header'
+  echo 'simpleImage..unstrip: ELF (identical to 
vmlinux)'
+  echo 'simpleImage..strip  : stripped ELF'
   echo '  Targets with  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  with the'
-- 
2.7.4



[PATCH 4/6] microblaze: add linux.bin* and simpleImage.* to PHONY

2018-12-07 Thread 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 replaced it with equivalent
simpleImage.$(DTB).

Signed-off-by: Masahiro Yamada 
---

Changes in v2: None

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

diff --git a/arch/microblaze/Makefile b/arch/microblaze/Makefile
index ff5abbd..180dffa 100644
--- a/arch/microblaze/Makefile
+++ b/arch/microblaze/Makefile
@@ -82,11 +82,13 @@ archclean:
 archheaders:
$(Q)$(MAKE) $(build)=arch/microblaze/kernel/syscalls all
 
+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) $(addprefix $(boot)/$@., ub unstrip strip)
@echo 'Kernel: $(boot)/$@ is ready' ' (#'`cat .version`')'
 
-- 
2.7.4



[PATCH 3/6] microblaze: fix multiple bugs in arch/microblaze/boot/Makefile

2018-12-07 Thread Masahiro Yamada
This commit fixes some build issues.

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} 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 confusing.

Let's split the build recipe for each image.

simpleImage.*.unstrip is just a copy of vmlinux.

simpleImage. and simpleImage..ub are created in the same way
as linux.bin and linux.bin.ub, respectively.

I kept simpleImage.* recipes independent of linux.bin.* ones to not
change the behavior.

Lastly, this commit fixes "make ARCH=microblaze clean". Previously,
it only cleaned up the unstrip image. Now, all the simpleImage files
are cleaned.

Signed-off-by: Masahiro Yamada 
---

Changes in v2:
  - Squash the first patch into this
  - Separate simpleImage recipes from linux.bin* recipes

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

diff --git a/arch/microblaze/Makefile b/arch/microblaze/Makefile
index 6cee1ca..ff5abbd 100644
--- a/arch/microblaze/Makefile
+++ b/arch/microblaze/Makefile
@@ -87,7 +87,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) $(addprefix $(boot)/$@., ub unstrip strip)
@echo 'Kernel: $(boot)/$@ is ready' ' (#'`cat .version`')'
 
 define archhelp
diff --git a/arch/microblaze/boot/Makefile b/arch/microblaze/boot/Makefile
index 96eefdc..cff570a 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.*
 
 OBJCOPYFLAGS := -R .note -R .comment -R .note.gnu.build-id -O binary
 
@@ -16,21 +16,20 @@ $(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)
+$(obj)/simpleImage.$(DTB): vmlinux FORCE
$(call if_changed,objcopy)
+
+$(obj)/simpleImage.$(DTB).ub: $(obj)/simpleImage.$(DTB) FORCE
$(call if_changed,uimage)
-   $(call if_changed,strip,.strip)
 
-clean-files += simpleImage.*.unstrip linux.bin.ub
+$(obj)/simpleImage.$(DTB).unstrip: vmlinux FORCE
+   $(call if_changed,shipped)
+
+$(obj)/simpleImage.$(DTB).strip: vmlinux FORCE
+   $(call if_changed,strip)
-- 
2.7.4



[PATCH 6/6] microblaze: remove the explicit removal of system.dtb

2018-12-07 Thread Masahiro Yamada
I guess

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

... should be

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

In fact, no Makefile needs to delete a target explicitly on error.

It is automatically done since commit 9c2af1c7377a ("kbuild: add
.DELETE_ON_ERROR special target").

I also reused equivalent cmd_shipped from scripts/Makefile.lib.

Signed-off-by: Masahiro Yamada 
---

Changes in v2: None

 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



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

2018-12-07 Thread 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 ARCH=microblaze all linux.bin.ub" results in a broken build
because two threads descend into arch/microblaze/boot simultaneously.

Add proper dependencies to avoid it.

Signed-off-by: Masahiro Yamada 
---

Changes in v2: None

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

diff --git a/arch/microblaze/Makefile b/arch/microblaze/Makefile
index 180dffa..7b340a3 100644
--- a/arch/microblaze/Makefile
+++ b/arch/microblaze/Makefile
@@ -83,7 +83,9 @@ archheaders:
$(Q)$(MAKE) $(build)=arch/microblaze/kernel/syscalls all
 
 PHONY += linux.bin linux.bin.gz linux.bin.ub
-linux.bin linux.bin.gz linux.bin.ub: vmlinux
+linux.bin.ub linux.bin.gz: linux.bin
+linux.bin: vmlinux
+linux.bin linux.bin.gz linux.bin.ub:
$(Q)$(MAKE) $(build)=$(boot) $(boot)/$@
@echo 'Kernel: $(boot)/$@ is ready' ' (#'`cat .version`')'
 
-- 
2.7.4



[PATCH 2/6] microblaze: move "... is ready" messages to arch/microblaze/Makefile

2018-12-07 Thread Masahiro Yamada
To prepare for more fixes, move this to arch/microblaze/Makefile.
Otherwise, the same "... is ready" would be printed multiple times.

Signed-off-by: Masahiro Yamada 
---

Changes in v2: None

 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 f6b7ea6..6cee1ca 100644
--- a/arch/microblaze/Makefile
+++ b/arch/microblaze/Makefile
@@ -84,9 +84,11 @@ archheaders:
 
 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 600e5a1..96eefdc 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.*.unstrip linux.bin.ub
-- 
2.7.4



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

2018-12-07 Thread Masahiro Yamada
This patch set fixes various issues in microblaze Makefiles.

V2 reflected Michal's comments, and cleaned up a little more.

I did not add Michals' Acked-by.
If this patch set goes to the MicroBlaze tree, he will add
Signed-off-by anyway.

This patch set is independent of Kbuild tree,
so it should apply to MicroBlaze tree.

Resolved the conflict with:

commit 1e17ab5320a654eaf1e4ce121c61e7aa9732805a
Author: Firoz Khan 
Date:   Tue Nov 13 11:34:34 2018 +0530

microblaze: generate uapi header and system call table files




Masahiro Yamada (6):
  microblaze: adjust the help to the real behavior
  microblaze: move "... is ready" messages 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 explicit removal of system.dtb

 arch/microblaze/Makefile  | 22 ++
 arch/microblaze/boot/Makefile | 23 +--
 arch/microblaze/boot/dts/Makefile |  5 +
 3 files changed, 24 insertions(+), 26 deletions(-)

-- 
2.7.4



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

2018-12-07 Thread Masahiro Yamada
On Thu, Dec 6, 2018 at 11:55 PM Michal Simek  wrote:
>
> On 03. 12. 18 8:50, Masahiro Yamada wrote:
> > This patch set fixes various issues in microblaze Makefiles.
> >
> > BTW, "simpleImage." works like a phony target to generate the
> > following four images, where the first three are just aliases.
> >
> >  - arch/microblaze/boot/simpleImage.:
> >  identical to arch/microblaze/boot/linux.bin
> >
> >  - arch/microblaze/boot/simpleImage..unstrip:
> >  identical to vmlinux
> >
> >  - arch/microblaze/boot/simpleImage..ub:
> >  identical to arch/microblaze/boot/linux.bin.ub
> >
> >  - arch/microblaze/boot/simpleImage..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= 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

2018-12-07 Thread Masahiro Yamada
On Thu, Dec 6, 2018 at 10:10 PM Michal Simek  wrote:
>
> Hi,
>
> On 06. 12. 18 6:08, Masahiro Yamada wrote:
> > Hi Michal,
> >
> > On Thu, Dec 6, 2018 at 1:41 AM Michal Simek  wrote:
> >>
> >> On 03. 12. 18 8:50, Masahiro Yamada wrote:
> >>> This patch set fixes various issues in microblaze Makefiles.
> >>>
> >>> BTW, "simpleImage." works like a phony target to generate the
> >>> following four images, where the first three are just aliases.
> >>>
> >>>  - arch/microblaze/boot/simpleImage.:
> >>>  identical to arch/microblaze/boot/linux.bin
> >>
> >> It is not - fdt section should be empty.
> >> simpleImage has this section filled.
> >>
> >>>
> >>>  - arch/microblaze/boot/simpleImage..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..ub:
> >>>  identical to arch/microblaze/boot/linux.bin.ub
> >>
> >> as above.
> >>
> >>>
> >>>  - arch/microblaze/boot/simpleImage..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= 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.
> > is objcopy'ed binary of vmlinux.
> >
> > arch/microblaze/boot/simpleImage..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." is just a shorthand of
> > "make DTB= 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..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." 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 2/7] microblaze: adjust the help to the real behavior

2018-12-07 Thread Masahiro Yamada
On Thu, Dec 6, 2018 at 9:54 PM Michal Simek  wrote:
>
> On 06. 12. 18 6:27, Masahiro Yamada wrote:
> > Hi Michal,
> >
> > On Thu, Dec 6, 2018 at 12:41 AM Michal Simek  wrote:
> >>
> >> On 03. 12. 18 8:50, Masahiro Yamada wrote:
> >>> "make ARCH=microblaze help" mentions simpleImage..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. works like a phony target that
> >>> generates multiple images. Reflect the behavior to the help message.
> >>>
> >>> Signed-off-by: Masahiro Yamada 
> >>> ---
> >>>
> >>>  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. - ELF image with $(arch)/boot/dts/.dts 
> >>> linked in'
> >>> -  echo '   - stripped elf with fdt blob'
> >>> -  echo '  simpleImage..unstrip - full ELF image with fdt blob'
> >>> +  echo '  simpleImage. - Create images with 
> >>> $(arch)/boot/dts/.dts linked in'
> >>>echo '  *_defconfig  - Select default config from 
> >>> arch/microblaze/configs'
> >>>echo ''
> >>>echo '  Targets with  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..strip and
> >> simpleImage..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. - Create the following images with
> > arch/macroblaze/boot/dts/.dts linked in
>
> type - microblaze
> maybe  here.


I will keep  as is.

I see more  references a few lines below:


  echo '  Targets with  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  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

2018-12-07 Thread Masahiro Yamada
On Thu, Dec 6, 2018 at 9:54 PM Michal Simek  wrote:
>
> On 06. 12. 18 6:27, Masahiro Yamada wrote:
> > Hi Michal,
> >
> > On Thu, Dec 6, 2018 at 12:41 AM Michal Simek  wrote:
> >>
> >> On 03. 12. 18 8:50, Masahiro Yamada wrote:
> >>> "make ARCH=microblaze help" mentions simpleImage..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. works like a phony target that
> >>> generates multiple images. Reflect the behavior to the help message.
> >>>
> >>> Signed-off-by: Masahiro Yamada 
> >>> ---
> >>>
> >>>  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. - ELF image with $(arch)/boot/dts/.dts 
> >>> linked in'
> >>> -  echo '   - stripped elf with fdt blob'
> >>> -  echo '  simpleImage..unstrip - full ELF image with fdt blob'
> >>> +  echo '  simpleImage. - Create images with 
> >>> $(arch)/boot/dts/.dts linked in'
> >>>echo '  *_defconfig  - Select default config from 
> >>> arch/microblaze/configs'
> >>>echo ''
> >>>echo '  Targets with  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..strip and
> >> simpleImage..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. - Create the following images with
> > arch/macroblaze/boot/dts/.dts linked in
>
> type - microblaze
> maybe  here.
>
> >  simpleImage.: raw image
> >  simpleImage..ub : raw image with U-Boot header
> >  simpleImage..unstrip: ELF (identical to vmlinux)
> >  simpleImage..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

2018-12-05 Thread Masahiro Yamada
Hi Michal,

On Thu, Dec 6, 2018 at 12:41 AM Michal Simek  wrote:
>
> On 03. 12. 18 8:50, Masahiro Yamada wrote:
> > "make ARCH=microblaze help" mentions simpleImage..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. works like a phony target that
> > generates multiple images. Reflect the behavior to the help message.
> >
> > Signed-off-by: Masahiro Yamada 
> > ---
> >
> >  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. - ELF image with $(arch)/boot/dts/.dts 
> > linked in'
> > -  echo '   - stripped elf with fdt blob'
> > -  echo '  simpleImage..unstrip - full ELF image with fdt blob'
> > +  echo '  simpleImage. - Create images with $(arch)/boot/dts/.dts 
> > linked in'
> >echo '  *_defconfig  - Select default config from 
> > arch/microblaze/configs'
> >echo ''
> >echo '  Targets with  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..strip and
> simpleImage..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. - Create the following images with
arch/macroblaze/boot/dts/.dts linked in
 simpleImage.: raw image
 simpleImage..ub : raw image with U-Boot header
 simpleImage..unstrip: ELF (identical to vmlinux)
 simpleImage..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. - ELF image with /boot/dts/.dts linked in
   - stripped elf with fdt blob
  simpleImage..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/.dts linked in" looks strange.



--
Best Regards
Masahiro Yamada


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

2018-12-05 Thread Masahiro Yamada
Hi Michal,

On Thu, Dec 6, 2018 at 1:41 AM Michal Simek  wrote:
>
> On 03. 12. 18 8:50, Masahiro Yamada wrote:
> > This patch set fixes various issues in microblaze Makefiles.
> >
> > BTW, "simpleImage." works like a phony target to generate the
> > following four images, where the first three are just aliases.
> >
> >  - arch/microblaze/boot/simpleImage.:
> >  identical to arch/microblaze/boot/linux.bin
>
> It is not - fdt section should be empty.
> simpleImage has this section filled.
>
> >
> >  - arch/microblaze/boot/simpleImage..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..ub:
> >  identical to arch/microblaze/boot/linux.bin.ub
>
> as above.
>
> >
> >  - arch/microblaze/boot/simpleImage..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= 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.
is objcopy'ed binary of vmlinux.

arch/microblaze/boot/simpleImage..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." is just a shorthand of
"make DTB= 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


[PATCH 0/4] UniPhier I2C fixes

2018-12-05 Thread Masahiro Yamada


Masahiro Yamada (4):
  i2c: uniphier-f: fix timeout error after reading 8 bytes
  i2c: uniphier-f: fill TX-FIFO only in IRQ handler for repeated START
  i2c: uniphier: fix violation of tLOW requirement for Fast-mode
  i2c: uniphier-f: fix violation of tLOW requirement for Fast-mode

 drivers/i2c/busses/i2c-uniphier-f.c | 49 +++--
 drivers/i2c/busses/i2c-uniphier.c   |  8 +-
 2 files changed, 48 insertions(+), 9 deletions(-)

-- 
2.7.4



[PATCH 1/4] i2c: uniphier-f: fix timeout error after reading 8 bytes

2018-12-05 Thread Masahiro Yamada
I was totally screwed up in commit eaba68785c2d ("i2c: uniphier-f:
fix race condition when IRQ is cleared"). Since that commit, if the
number of read bytes is multiple of the FIFO size (8, 16, 24... bytes),
the STOP condition could be issued twice, depending on the timing.
If this happens, the controller will go wrong, resulting in the timeout
error.

It was more than 3 years ago when I wrote this driver, so my memory
about this hardware was vague. Please let me correct the description
in the commit log of eaba68785c2d.

Clearing the IRQ status on exiting the IRQ handler is absolutely
fine. This controller makes a pause while any IRQ status is asserted.
If the IRQ status is cleared first, the hardware may start the next
transaction before the IRQ handler finishes what it supposed to do.

This partially reverts the bad commit with clear comments so that I
will never repeat this mistake.

I also investigated what is happening at the last moment of the read
mode. The UNIPHIER_FI2C_INT_RF interrupt is asserted a bit earlier
(by half a period of the clock cycle) than UNIPHIER_FI2C_INT_RB.

I consulted a hardware engineer, and I got the following information:

UNIPHIER_FI2C_INT_RF
asserted at the falling edge of SCL at the 8th bit.

UNIPHIER_FI2C_INT_RB
asserted at the rising edge of SCL at the 9th (ACK) bit.

In order to avoid calling uniphier_fi2c_stop() twice, check the latter
interrupt. I also commented this because it is obscure hardware internal.

Fixes: eaba68785c2d ("i2c: uniphier-f: fix race condition when IRQ is cleared")
Signed-off-by: Masahiro Yamada 
---

 drivers/i2c/busses/i2c-uniphier-f.c | 17 ++---
 1 file changed, 14 insertions(+), 3 deletions(-)

diff --git a/drivers/i2c/busses/i2c-uniphier-f.c 
b/drivers/i2c/busses/i2c-uniphier-f.c
index dd38474..fad2b00 100644
--- a/drivers/i2c/busses/i2c-uniphier-f.c
+++ b/drivers/i2c/busses/i2c-uniphier-f.c
@@ -173,8 +173,6 @@ static irqreturn_t uniphier_fi2c_interrupt(int irq, void 
*dev_id)
"interrupt: enabled_irqs=%04x, irq_status=%04x\n",
priv->enabled_irqs, irq_status);
 
-   uniphier_fi2c_clear_irqs(priv, irq_status);
-
if (irq_status & UNIPHIER_FI2C_INT_STOP)
goto complete;
 
@@ -214,7 +212,13 @@ static irqreturn_t uniphier_fi2c_interrupt(int irq, void 
*dev_id)
 
if (irq_status & (UNIPHIER_FI2C_INT_RF | UNIPHIER_FI2C_INT_RB)) {
uniphier_fi2c_drain_rxfifo(priv);
-   if (!priv->len)
+   /*
+* If the number of bytes to read is multiple of the FIFO size
+* (msg->len == 8, 16, 24, ...), the INT_RF bit is set a little
+* earlier than INT_RB. We wait for INT_RB to confirm the
+* completion of the current message.
+*/
+   if (!priv->len && (irq_status & UNIPHIER_FI2C_INT_RB))
goto data_done;
 
if (unlikely(priv->flags & UNIPHIER_FI2C_MANUAL_NACK)) {
@@ -253,6 +257,13 @@ static irqreturn_t uniphier_fi2c_interrupt(int irq, void 
*dev_id)
}
 
 handled:
+   /*
+* This controller makes a pause while any bit of the IRQ status is
+* asserted. Clear the asserted bit to kick the controller just before
+* exiting the handler.
+*/
+   uniphier_fi2c_clear_irqs(priv, irq_status);
+
spin_unlock(>lock);
 
return IRQ_HANDLED;
-- 
2.7.4



[PATCH 3/4] i2c: uniphier: fix violation of tLOW requirement for Fast-mode

2018-12-05 Thread Masahiro Yamada
Currently, the clock duty is set as tLOW/tHIGH = 1/1. For Fast-mode,
tLOW is set to 1.25 us while the I2C spec requires tLOW >= 1.3 us.

tLOW/tHIGH = 5/4 would meet both Standard-mode and Fast-mode:
  Standard-mode: tLOW = 5.56 us, tHIGH = 4.44 us
  Fast-mode: tLOW = 1.39 us, tHIGH = 1.11 us

Signed-off-by: Masahiro Yamada 
---

 drivers/i2c/busses/i2c-uniphier.c | 8 +++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/i2c/busses/i2c-uniphier.c 
b/drivers/i2c/busses/i2c-uniphier.c
index 454f914..c488e55 100644
--- a/drivers/i2c/busses/i2c-uniphier.c
+++ b/drivers/i2c/busses/i2c-uniphier.c
@@ -320,7 +320,13 @@ static void uniphier_i2c_hw_init(struct uniphier_i2c_priv 
*priv)
 
uniphier_i2c_reset(priv, true);
 
-   writel((cyc / 2 << 16) | cyc, priv->membase + UNIPHIER_I2C_CLK);
+   /*
+* Bit30-16: clock cycles of tLOW.
+*  Standard-mode: tLOW = 4.7 us, tHIGH = 4.0 us
+*  Fast-mode: tLOW = 1.3 us, tHIGH = 0.6 us
+* "tLow/tHIGH = 5/4" meets both.
+*/
+   writel((cyc * 5 / 9 << 16) | cyc, priv->membase + UNIPHIER_I2C_CLK);
 
uniphier_i2c_reset(priv, false);
 }
-- 
2.7.4



[PATCH 4/4] i2c: uniphier-f: fix violation of tLOW requirement for Fast-mode

2018-12-05 Thread Masahiro Yamada
Currently, the clock duty is set as tLOW/tHIGH = 1/1. For Fast-mode,
tLOW is set to 1.25 us while the I2C spec requires tLOW >= 1.3 us.

tLOW/tHIGH = 5/4 would meet both Standard-mode and Fast-mode:
  Standard-mode: tLOW = 5.56 us, tHIGH = 4.44 us
  Fast-mode: tLOW = 1.39 us, tHIGH = 1.11 us

Signed-off-by: Masahiro Yamada 
---

 drivers/i2c/busses/i2c-uniphier-f.c | 19 ++-
 1 file changed, 18 insertions(+), 1 deletion(-)

diff --git a/drivers/i2c/busses/i2c-uniphier-f.c 
b/drivers/i2c/busses/i2c-uniphier-f.c
index d8a5db14..03da4a5 100644
--- a/drivers/i2c/busses/i2c-uniphier-f.c
+++ b/drivers/i2c/busses/i2c-uniphier-f.c
@@ -518,9 +518,26 @@ static void uniphier_fi2c_hw_init(struct 
uniphier_fi2c_priv *priv)
 
uniphier_fi2c_reset(priv);
 
+   /*
+*  Standard-mode: tLOW + tHIGH = 10 us
+*  Fast-mode: tLOW + tHIGH = 2.5 us
+*/
writel(cyc, priv->membase + UNIPHIER_FI2C_CYC);
-   writel(cyc / 2, priv->membase + UNIPHIER_FI2C_LCTL);
+   /*
+*  Standard-mode: tLOW = 4.7 us, tHIGH = 4.0 us, tBUF = 4.7 us
+*  Fast-mode: tLOW = 1.3 us, tHIGH = 0.6 us, tBUF = 1.3 us
+* "tLow/tHIGH = 5/4" meets both.
+*/
+   writel(cyc * 5 / 9, priv->membase + UNIPHIER_FI2C_LCTL);
+   /*
+*  Standard-mode: tHD;STA = 4.0 us, tSU;STA = 4.7 us, tSU;STO = 4.0 us
+*  Fast-mode: tHD;STA = 0.6 us, tSU;STA = 0.6 us, tSU;STO = 0.6 us
+*/
writel(cyc / 2, priv->membase + UNIPHIER_FI2C_SSUT);
+   /*
+*  Standard-mode: tSU;DAT = 250 ns
+*  Fast-mode: tSU;DAT = 100 ns
+*/
writel(cyc / 16, priv->membase + UNIPHIER_FI2C_DSUT);
 
uniphier_fi2c_prepare_operation(priv);
-- 
2.7.4



[PATCH 2/4] i2c: uniphier-f: fill TX-FIFO only in IRQ handler for repeated START

2018-12-05 Thread Masahiro Yamada
 - For a repeated START condition, this controller starts data transfer
   immediately after the slave address is written to the TX-FIFO.

 - Once the TX-FIFO empty interrupt is asserted, the controller makes
   a pause even if additional data are written to the TX-FIFO.

Given those circumstances, the data after a repeated START may not be
transferred if the interrupt is asserted while the TX-FIFO is being
filled up. A more reliable way is to append TX data only in the
interrupt handler.

Signed-off-by: Masahiro Yamada 
---

 drivers/i2c/busses/i2c-uniphier-f.c | 13 +
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/drivers/i2c/busses/i2c-uniphier-f.c 
b/drivers/i2c/busses/i2c-uniphier-f.c
index fad2b00..d8a5db14 100644
--- a/drivers/i2c/busses/i2c-uniphier-f.c
+++ b/drivers/i2c/busses/i2c-uniphier-f.c
@@ -269,7 +269,8 @@ static irqreturn_t uniphier_fi2c_interrupt(int irq, void 
*dev_id)
return IRQ_HANDLED;
 }
 
-static void uniphier_fi2c_tx_init(struct uniphier_fi2c_priv *priv, u16 addr)
+static void uniphier_fi2c_tx_init(struct uniphier_fi2c_priv *priv, u16 addr,
+ bool repeat)
 {
priv->enabled_irqs |= UNIPHIER_FI2C_INT_TE;
uniphier_fi2c_set_irqs(priv);
@@ -279,8 +280,12 @@ static void uniphier_fi2c_tx_init(struct 
uniphier_fi2c_priv *priv, u16 addr)
/* set slave address */
writel(UNIPHIER_FI2C_DTTX_CMD | addr << 1,
   priv->membase + UNIPHIER_FI2C_DTTX);
-   /* first chunk of data */
-   uniphier_fi2c_fill_txfifo(priv, true);
+   /*
+* First chunk of data. For a repeated START condition, do not write
+* data to the TX fifo here to avoid the timing issue.
+*/
+   if (!repeat)
+   uniphier_fi2c_fill_txfifo(priv, true);
 }
 
 static void uniphier_fi2c_rx_init(struct uniphier_fi2c_priv *priv, u16 addr)
@@ -361,7 +366,7 @@ static int uniphier_fi2c_master_xfer_one(struct i2c_adapter 
*adap,
if (is_read)
uniphier_fi2c_rx_init(priv, msg->addr);
else
-   uniphier_fi2c_tx_init(priv, msg->addr);
+   uniphier_fi2c_tx_init(priv, msg->addr, repeat);
 
dev_dbg(>dev, "start condition\n");
/*
-- 
2.7.4



[PATCH] gpio: uniphier: convert to SPDX License Identifier

2018-12-05 Thread Masahiro Yamada
checkpatch.pl suggests to use SPDX license tag. I am happy to
follow it.

Signed-off-by: Masahiro Yamada 
---

 drivers/gpio/gpio-uniphier.c | 17 -
 1 file changed, 4 insertions(+), 13 deletions(-)

diff --git a/drivers/gpio/gpio-uniphier.c b/drivers/gpio/gpio-uniphier.c
index 74551cbd..0f662b2 100644
--- a/drivers/gpio/gpio-uniphier.c
+++ b/drivers/gpio/gpio-uniphier.c
@@ -1,16 +1,7 @@
-/*
- * Copyright (C) 2017 Socionext Inc.
- *   Author: Masahiro Yamada 
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License version 2 as
- * published by the Free Software Foundation.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU General Public License for more details.
- */
+// SPDX-License-Identifier: GPL-2.0
+//
+// Copyright (C) 2017 Socionext Inc.
+//   Author: Masahiro Yamada 
 
 #include 
 #include 
-- 
2.7.4



[PATCH] pinctrl: uniphier: convert to SPDX License Identifier

2018-12-05 Thread Masahiro Yamada
checkpatch.pl suggests to use SPDX license tag. I am happy to
follow it.

Signed-off-by: Masahiro Yamada 
---

 drivers/pinctrl/uniphier/pinctrl-uniphier-core.c | 18 --
 drivers/pinctrl/uniphier/pinctrl-uniphier-ld11.c | 18 --
 drivers/pinctrl/uniphier/pinctrl-uniphier-ld20.c | 18 --
 drivers/pinctrl/uniphier/pinctrl-uniphier-ld4.c  | 18 --
 drivers/pinctrl/uniphier/pinctrl-uniphier-ld6b.c | 18 --
 drivers/pinctrl/uniphier/pinctrl-uniphier-pro4.c | 18 --
 drivers/pinctrl/uniphier/pinctrl-uniphier-pro5.c | 18 --
 drivers/pinctrl/uniphier/pinctrl-uniphier-pxs2.c | 18 --
 drivers/pinctrl/uniphier/pinctrl-uniphier-pxs3.c | 18 --
 drivers/pinctrl/uniphier/pinctrl-uniphier-sld8.c | 18 --
 drivers/pinctrl/uniphier/pinctrl-uniphier.h  | 11 +--
 11 files changed, 41 insertions(+), 150 deletions(-)

diff --git a/drivers/pinctrl/uniphier/pinctrl-uniphier-core.c 
b/drivers/pinctrl/uniphier/pinctrl-uniphier-core.c
index add8e87..b2412f1 100644
--- a/drivers/pinctrl/uniphier/pinctrl-uniphier-core.c
+++ b/drivers/pinctrl/uniphier/pinctrl-uniphier-core.c
@@ -1,17 +1,7 @@
-/*
- * Copyright (C) 2015-2017 Socionext Inc.
- *   Author: Masahiro Yamada 
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation; either version 2 of the License, or
- * (at your option) any later version.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU General Public License for more details.
- */
+// SPDX-License-Identifier: GPL-2.0+
+//
+// Copyright (C) 2015-2017 Socionext Inc.
+//   Author: Masahiro Yamada 
 
 #include 
 #include 
diff --git a/drivers/pinctrl/uniphier/pinctrl-uniphier-ld11.c 
b/drivers/pinctrl/uniphier/pinctrl-uniphier-ld11.c
index 280dca7..ce7cde0 100644
--- a/drivers/pinctrl/uniphier/pinctrl-uniphier-ld11.c
+++ b/drivers/pinctrl/uniphier/pinctrl-uniphier-ld11.c
@@ -1,17 +1,7 @@
-/*
- * Copyright (C) 2016-2017 Socionext Inc.
- *   Author: Masahiro Yamada 
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation; either version 2 of the License, or
- * (at your option) any later version.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU General Public License for more details.
- */
+// SPDX-License-Identifier: GPL-2.0+
+//
+// Copyright (C) 2016-2017 Socionext Inc.
+//   Author: Masahiro Yamada 
 
 #include 
 #include 
diff --git a/drivers/pinctrl/uniphier/pinctrl-uniphier-ld20.c 
b/drivers/pinctrl/uniphier/pinctrl-uniphier-ld20.c
index d2d56c9..91670913 100644
--- a/drivers/pinctrl/uniphier/pinctrl-uniphier-ld20.c
+++ b/drivers/pinctrl/uniphier/pinctrl-uniphier-ld20.c
@@ -1,17 +1,7 @@
-/*
- * Copyright (C) 2016-2017 Socionext Inc.
- *   Author: Masahiro Yamada 
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation; either version 2 of the License, or
- * (at your option) any later version.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU General Public License for more details.
- */
+// SPDX-License-Identifier: GPL-2.0+
+//
+// Copyright (C) 2016-2017 Socionext Inc.
+//   Author: Masahiro Yamada 
 
 #include 
 #include 
diff --git a/drivers/pinctrl/uniphier/pinctrl-uniphier-ld4.c 
b/drivers/pinctrl/uniphier/pinctrl-uniphier-ld4.c
index 03d87ad..4e7a99f 100644
--- a/drivers/pinctrl/uniphier/pinctrl-uniphier-ld4.c
+++ b/drivers/pinctrl/uniphier/pinctrl-uniphier-ld4.c
@@ -1,17 +1,7 @@
-/*
- * Copyright (C) 2015-2017 Socionext Inc.
- *   Author: Masahiro Yamada 
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation; either version 2 of the License, or
- * (at your option) any later version.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU General Public License for more details.
- */
+// SPDX-License-Identifier: GPL-2.0+
+//
+// Copyright (C) 2015-2017 Socionext Inc.
+//   Author: Masahiro Yamada 
 
 #include

[PATCH 1/3] kbuild: refactor Makefile.asm-generic

2018-12-05 Thread Masahiro Yamada
 - Use conventional $(MAKE) $(asm-generic)= style
   for directory descending

 - Remove unneeded FORCE since "all" is a phony target

 - Remove unneeded "_dummy :=" assignment

 - Skip $(shell mkdir ...) when headers exist in the directory

 - Misc cleanups

Signed-off-by: Masahiro Yamada 
---

 Makefile |  8 
 scripts/Makefile.asm-generic | 37 +++--
 2 files changed, 23 insertions(+), 22 deletions(-)

diff --git a/Makefile b/Makefile
index ab38eb2..3af0379 100644
--- a/Makefile
+++ b/Makefile
@@ -1122,13 +1122,13 @@ prepare0: archprepare
 prepare: prepare0 prepare-objtool
 
 # Support for using generic headers in asm-generic
+asm-generic := -f $(srctree)/scripts/Makefile.asm-generic obj
+
 PHONY += asm-generic uapi-asm-generic
 asm-generic: uapi-asm-generic
-   $(Q)$(MAKE) -f $(srctree)/scripts/Makefile.asm-generic \
-   src=asm obj=arch/$(SRCARCH)/include/generated/asm
+   $(Q)$(MAKE) $(asm-generic)=arch/$(SRCARCH)/include/generated/asm
 uapi-asm-generic:
-   $(Q)$(MAKE) -f $(srctree)/scripts/Makefile.asm-generic \
-   src=uapi/asm obj=arch/$(SRCARCH)/include/generated/uapi/asm
+   $(Q)$(MAKE) $(asm-generic)=arch/$(SRCARCH)/include/generated/uapi/asm
 
 PHONY += prepare-objtool
 prepare-objtool: $(objtool_target)
diff --git a/scripts/Makefile.asm-generic b/scripts/Makefile.asm-generic
index 32ad8e9..760323e 100644
--- a/scripts/Makefile.asm-generic
+++ b/scripts/Makefile.asm-generic
@@ -2,41 +2,42 @@
 # include/asm-generic contains a lot of files that are used
 # verbatim by several architectures.
 #
-# This Makefile reads the file arch/$(SRCARCH)/include/$(src)/Kbuild
+# This Makefile reads the file arch/$(SRCARCH)/include/(uapi/)/asm/Kbuild
 # and for each file listed in this file with generic-y creates
-# a small wrapper file in $(obj) (arch/$(SRCARCH)/include/generated/$(src))
+# a small wrapper file in arch/$(SRCARCH)/include/generated/(uapi/)/asm.
 
 PHONY := all
 all:
 
-kbuild-file := $(srctree)/arch/$(SRCARCH)/include/$(src)/Kbuild
--include $(kbuild-file)
+src := $(subst /generated,,$(obj))
+-include $(src)/Kbuild
 
 include scripts/Kbuild.include
 
-# Create output directory if not already present
-_dummy := $(shell [ -d $(obj) ] || mkdir -p $(obj))
+generic-y   := $(addprefix $(obj)/, $(generic-y))
+generated-y := $(addprefix $(obj)/, $(generated-y))
 
-# Stale wrappers when the corresponding files are removed from generic-y
-# need removing.
-generated-y   := $(generic-y) $(generated-y)
-all-files := $(patsubst %, $(obj)/%, $(generated-y))
-old-headers   := $(wildcard $(obj)/*.h)
-unwanted  := $(filter-out $(all-files),$(old-headers))
+# Remove stale wrappers when the corresponding files are removed from generic-y
+old-headers := $(wildcard $(obj)/*.h)
+unwanted:= $(filter-out $(generic-y) $(generated-y),$(old-headers))
 
 quiet_cmd_wrap = WRAP$@
-cmd_wrap = echo "\#include " >$@
+  cmd_wrap = echo "\#include " > $@
 
 quiet_cmd_remove = REMOVE  $(unwanted)
-cmd_remove = rm -f $(unwanted)
+  cmd_remove = rm -f $(unwanted)
 
-all: $(patsubst %, $(obj)/%, $(generic-y)) FORCE
-   $(if $(unwanted),$(call cmd,remove),)
+all: $(generic-y)
+   $(if $(unwanted),$(call cmd,remove))
@:
 
 $(obj)/%.h:
$(call cmd,wrap)
 
-PHONY += FORCE
+# Create output directory. Skip it if at least one old header exists
+# since we know the output directory already exists.
+ifeq ($(old-headers),)
+$(shell mkdir -p $(obj))
+endif
+
 .PHONY: $(PHONY)
-FORCE: ;
-- 
2.7.4



[tip:x86/urgent] x86/build: Fix compiler support check for CONFIG_RETPOLINE

2018-12-04 Thread tip-bot for Masahiro Yamada
Commit-ID:  25896d073d8a0403b07e6dec56f58e6c33678207
Gitweb: https://git.kernel.org/tip/25896d073d8a0403b07e6dec56f58e6c33678207
Author: Masahiro Yamada 
AuthorDate: Wed, 5 Dec 2018 15:27:19 +0900
Committer:  Ingo Molnar 
CommitDate: Wed, 5 Dec 2018 08:44:02 +0100

x86/build: Fix compiler support check for CONFIG_RETPOLINE

It is troublesome to add a diagnostic like this to the Makefile
parse stage because the top-level Makefile could be parsed with
a stale include/config/auto.conf.

Once you are hit by the error about non-retpoline compiler, the
compilation still breaks even after disabling CONFIG_RETPOLINE.

The easiest fix is to move this check to the "archprepare" like
this commit did:

  829fe4aa9ac1 ("x86: Allow generating user-space headers without a compiler")

Reported-by: Meelis Roos 
Tested-by: Meelis Roos 
Signed-off-by: Masahiro Yamada 
Acked-by: Zhenzhong Duan 
Cc: Borislav Petkov 
Cc: Linus Torvalds 
Cc: Peter Zijlstra 
Cc: Thomas Gleixner 
Cc: Zhenzhong Duan 
Fixes: 4cd24de3a098 ("x86/retpoline: Make CONFIG_RETPOLINE depend on compiler 
support")
Link: 
http://lkml.kernel.org/r/1543991239-18476-1-git-send-email-yamada.masah...@socionext.com
Link: https://lkml.org/lkml/2018/12/4/206
Signed-off-by: Ingo Molnar 
---
 arch/x86/Makefile | 10 +++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/arch/x86/Makefile b/arch/x86/Makefile
index f5d7f4134524..75ef499a66e2 100644
--- a/arch/x86/Makefile
+++ b/arch/x86/Makefile
@@ -220,9 +220,6 @@ KBUILD_CFLAGS += -fno-asynchronous-unwind-tables
 
 # Avoid indirect branches in kernel to deal with Spectre
 ifdef CONFIG_RETPOLINE
-ifeq ($(RETPOLINE_CFLAGS),)
-  $(error You are building kernel with non-retpoline compiler, please update 
your compiler.)
-endif
   KBUILD_CFLAGS += $(RETPOLINE_CFLAGS)
 endif
 
@@ -307,6 +304,13 @@ ifndef CC_HAVE_ASM_GOTO
@echo Compiler lacks asm-goto support.
@exit 1
 endif
+ifdef CONFIG_RETPOLINE
+ifeq ($(RETPOLINE_CFLAGS),)
+   @echo "You are building kernel with non-retpoline compiler." >&2
+   @echo "Please update your compiler." >&2
+   @false
+endif
+endif
 
 archclean:
$(Q)rm -rf $(objtree)/arch/i386


[tip:x86/urgent] x86/build: Fix compiler support check for CONFIG_RETPOLINE

2018-12-04 Thread tip-bot for Masahiro Yamada
Commit-ID:  10b87bc9f630a38a7f0c1e7cf13ff23fea1692ec
Gitweb: https://git.kernel.org/tip/10b87bc9f630a38a7f0c1e7cf13ff23fea1692ec
Author: Masahiro Yamada 
AuthorDate: Wed, 5 Dec 2018 15:27:19 +0900
Committer:  Ingo Molnar 
CommitDate: Wed, 5 Dec 2018 08:42:09 +0100

x86/build: Fix compiler support check for CONFIG_RETPOLINE

It is troublesome to add a diagnostic like this to the Makefile
parse stage because the top-level Makefile could be parsed with
a stale include/config/auto.conf.

Once you are hit by the error about non-retpoline compiler, the
compilation still breaks even after disabling CONFIG_RETPOLINE.

The easiest fix is to move this check to the "archprepare" like
this commit did:

  829fe4aa9ac1 ("x86: Allow generating user-space headers without a compiler")

Reported-by: Meelis Roos 
Signed-off-by: Masahiro Yamada 
Acked-by: Zhenzhong Duan 
Cc: Borislav Petkov 
Cc: Linus Torvalds 
Cc: Peter Zijlstra 
Cc: Thomas Gleixner 
Cc: Zhenzhong Duan 
Fixes: 4cd24de3a098 ("x86/retpoline: Make CONFIG_RETPOLINE depend on compiler 
support")
Link: 
http://lkml.kernel.org/r/1543991239-18476-1-git-send-email-yamada.masah...@socionext.com
Link: https://lkml.org/lkml/2018/12/4/206
Signed-off-by: Ingo Molnar 
---
 arch/x86/Makefile | 10 +++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/arch/x86/Makefile b/arch/x86/Makefile
index f5d7f4134524..75ef499a66e2 100644
--- a/arch/x86/Makefile
+++ b/arch/x86/Makefile
@@ -220,9 +220,6 @@ KBUILD_CFLAGS += -fno-asynchronous-unwind-tables
 
 # Avoid indirect branches in kernel to deal with Spectre
 ifdef CONFIG_RETPOLINE
-ifeq ($(RETPOLINE_CFLAGS),)
-  $(error You are building kernel with non-retpoline compiler, please update 
your compiler.)
-endif
   KBUILD_CFLAGS += $(RETPOLINE_CFLAGS)
 endif
 
@@ -307,6 +304,13 @@ ifndef CC_HAVE_ASM_GOTO
@echo Compiler lacks asm-goto support.
@exit 1
 endif
+ifdef CONFIG_RETPOLINE
+ifeq ($(RETPOLINE_CFLAGS),)
+   @echo "You are building kernel with non-retpoline compiler." >&2
+   @echo "Please update your compiler." >&2
+   @false
+endif
+endif
 
 archclean:
$(Q)rm -rf $(objtree)/arch/i386


[PATCH] kbuild: exploit parallel building for CONFIG_HEADERS_CHECK

2018-12-04 Thread Masahiro Yamada
When CONFIG_HEADERS_CHECK is enabled, the headers_check is executed
as a serialized task in the vmlinux recipe.

Make it independent of vmlinux so that parallel building can process
the headers_check and other build targets simultaneously.

Signed-off-by: Masahiro Yamada 
---

 Makefile | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/Makefile b/Makefile
index 3af0379..2808a7d 100644
--- a/Makefile
+++ b/Makefile
@@ -1035,9 +1035,6 @@ cmd_link-vmlinux =
 \
$(if $(ARCH_POSTLINK), $(MAKE) -f $(ARCH_POSTLINK) $@, true)
 
 vmlinux: scripts/link-vmlinux.sh autoksyms_recursive $(vmlinux-deps) FORCE
-ifdef CONFIG_HEADERS_CHECK
-   $(Q)$(MAKE) -f $(srctree)/Makefile headers_check
-endif
 ifdef CONFIG_GDB_SCRIPTS
$(Q)ln -fsn $(abspath $(srctree)/scripts/gdb/vmlinux-gdb.py)
 endif
@@ -1208,6 +1205,10 @@ headers_check: headers_install
$(Q)$(MAKE) $(hdr-inst)=include/uapi dst=include HDRCHECK=1
$(Q)$(MAKE) $(hdr-inst)=arch/$(SRCARCH)/include/uapi $(hdr-dst) 
HDRCHECK=1
 
+ifdef CONFIG_HEADERS_CHECK
+all: headers_check
+endif
+
 # ---
 # Kernel selftest
 
-- 
2.7.4



[PATCH] kbuild: remove a special handling for *.agh in Makefile.headersinst

2018-12-04 Thread Masahiro Yamada
scripts/Makefile.headersinst takes care of *.agh just for

  arch/cris/include/uapi/arch-v10/arch/sv_addr.agh

because renaming exported headers is difficult (or impossible).

This code is no longer necessary thanks to commit c690eddc2f3b ("CRIS:
Drop support for the CRIS port").

Signed-off-by: Masahiro Yamada 
---

 scripts/Makefile.headersinst | 1 -
 1 file changed, 1 deletion(-)

diff --git a/scripts/Makefile.headersinst b/scripts/Makefile.headersinst
index 0d4a96d..3d1ebaa 100644
--- a/scripts/Makefile.headersinst
+++ b/scripts/Makefile.headersinst
@@ -44,7 +44,6 @@ kbuild-file := $(srctree)/$(obj)/Kbuild
 installdir:= $(INSTALL_HDR_PATH)/$(dst)
 gendir:= $(objtree)/$(subst include/,include/generated/,$(obj))
 header-files  := $(notdir $(wildcard $(srcdir)/*.h))
-header-files  += $(notdir $(wildcard $(srcdir)/*.agh))
 header-files  := $(filter-out $(no-export-headers), $(header-files))
 genhdr-files  := $(notdir $(wildcard $(gendir)/*.h))
 genhdr-files  := $(filter-out $(header-files), $(genhdr-files))
-- 
2.7.4



[PATCH v2] x86/build: fix compiler support check for CONFIG_RETPOLINE

2018-12-04 Thread Masahiro Yamada
It is troublesome to add a diagnostic like this to the Makefile
parse stage because the top-level Makefile could be parsed with
a stale include/config/auto.conf.

Once you are hit by the error about non-retpoline compiler, the
compilation still breaks even after disabling CONFIG_RETPOLINE.

The easiest fix is to move this check to the "archprepare" like commit
829fe4aa9ac1 ("x86: Allow generating user-space headers without a
compiler") did.

Link: https://lkml.org/lkml/2018/12/4/206
Fixes: 4cd24de3a098 ("x86/retpoline: Make CONFIG_RETPOLINE depend on compiler 
support")
Reported-by: Meelis Roos 
Signed-off-by: Masahiro Yamada 
---

Changes in v2:
  - Revive ifdef CONFIG_RETPOLINE surrounding the KBUILD_CFLAGS addition
  - Rephase the commit log a bit, hoping the cause of the issue will be clearer

 arch/x86/Makefile | 10 +++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/arch/x86/Makefile b/arch/x86/Makefile
index f5d7f41..75ef499 100644
--- a/arch/x86/Makefile
+++ b/arch/x86/Makefile
@@ -220,9 +220,6 @@ KBUILD_CFLAGS += -fno-asynchronous-unwind-tables
 
 # Avoid indirect branches in kernel to deal with Spectre
 ifdef CONFIG_RETPOLINE
-ifeq ($(RETPOLINE_CFLAGS),)
-  $(error You are building kernel with non-retpoline compiler, please update 
your compiler.)
-endif
   KBUILD_CFLAGS += $(RETPOLINE_CFLAGS)
 endif
 
@@ -307,6 +304,13 @@ ifndef CC_HAVE_ASM_GOTO
@echo Compiler lacks asm-goto support.
@exit 1
 endif
+ifdef CONFIG_RETPOLINE
+ifeq ($(RETPOLINE_CFLAGS),)
+   @echo "You are building kernel with non-retpoline compiler." >&2
+   @echo "Please update your compiler." >&2
+   @false
+endif
+endif
 
 archclean:
$(Q)rm -rf $(objtree)/arch/i386
-- 
2.7.4



Re: [PATCH] x86/build: fix compiler support check for CONFIG_RETPOLINE

2018-12-04 Thread Masahiro Yamada
On Wed, Dec 5, 2018 at 3:08 PM Zhenzhong Duan  wrote:
>
>
> On 2018/12/5 11:00, Masahiro Yamada wrote:
> > It is wrong to add CONFIG option diagnostic to the Makefile parse
> > stage.
> >
> > Once you are hit by the error about non-retpoline compiler, the
> > compilation still breaks even after disabling CONFIG_RETPOLINE.
> >
> > The easiest fix is to move this check to the "archprepare" like commit
> > 829fe4aa9ac1 ("x86: Allow generating user-space headers without a
> > compiler") did.
> >
> > Link: https://lkml.org/lkml/2018/12/4/206
> > Reported-by: Meelis Roos 
> > Fixes: 4cd24de3a098 ("x86/retpoline: Make CONFIG_RETPOLINE depend on 
> > compiler support")
> > Signed-off-by: Masahiro Yamada 
> > ---
> >
> >   arch/x86/Makefile | 14 --
> >   1 file changed, 8 insertions(+), 6 deletions(-)
> >
> > diff --git a/arch/x86/Makefile b/arch/x86/Makefile
> > index f5d7f41..ae0148b 100644
> > --- a/arch/x86/Makefile
> > +++ b/arch/x86/Makefile
> > @@ -219,12 +219,7 @@ KBUILD_CFLAGS += -Wno-sign-compare
> >   KBUILD_CFLAGS += -fno-asynchronous-unwind-tables
> >
> >   # Avoid indirect branches in kernel to deal with Spectre
> > -ifdef CONFIG_RETPOLINE
> > -ifeq ($(RETPOLINE_CFLAGS),)
> > -  $(error You are building kernel with non-retpoline compiler, please 
> > update your compiler.)
> > -endif
> > -  KBUILD_CFLAGS += $(RETPOLINE_CFLAGS)
> > -endif
> > +KBUILD_CFLAGS += $(RETPOLINE_CFLAGS)
>
> Is it better to also move "# Avoid indirect branches in kernel to deal
> with Spectre"
>
> and "KBUILD_CFLAGS += $(RETPOLINE_CFLAGS)"?
>
> Seems unconditionaly using "KBUILD_CFLAGS += $(RETPOLINE_CFLAGS)" will
> have compiler
>
> using "__x86_indirect_thunk_\reg" call even ifCONFIG_RETPOLINE is
> disabled. I guess
>
> link process will fail?


Good catch.

I will send v2.

Thanks.



> Thanks
>
> Zhenzhong
>
> >
> >   archscripts: scripts_basic
> >   $(Q)$(MAKE) $(build)=arch/x86/tools relocs
> > @@ -307,6 +302,13 @@ ifndef CC_HAVE_ASM_GOTO
> >   @echo Compiler lacks asm-goto support.
> >   @exit 1
> >   endif
> > +ifdef CONFIG_RETPOLINE
> > +ifeq ($(RETPOLINE_CFLAGS),)
> > + @echo "You are building kernel with non-retpoline compiler." >&2
> > + @echo "Please update your compiler." >&2
> > + @false
> > +endif
> > +endif
> >
> >   archclean:
> >   $(Q)rm -rf $(objtree)/arch/i386



-- 
Best Regards
Masahiro Yamada


Re: [PATCH] x86/build: fix compiler support check for CONFIG_RETPOLINE

2018-12-04 Thread Masahiro Yamada
On Wed, Dec 5, 2018 at 12:23 PM Zhenzhong Duan
 wrote:
>
> Hi Masahiro,
>
>
> Thanks for fixing.
> There is a question still confusing me. There are many CONFIG_* used in
> arch/x86/Makefile.
> Will they also be impacted or not?


The top level Makefile (and arch/*/Makefile) is getting huge.
Parsing the whole Makefile everytime means,
it computes variables that may or may not be used.

$(call cc-option,...) is a bit costly,
but other parts are unnoticeable
in terms of performance.


I am addressing the use of $(error ...) / $(warning ...)
in the non-recipe context since they could break the build
for false positive reasons.




> # grep CONFIG_ arch/x86/Makefile
> ifeq ($(CONFIG_X86_32),y)
>  cflags-$(CONFIG_MK8) += $(call cc-option,-march=k8)
>  cflags-$(CONFIG_MPSC) += $(call cc-option,-march=nocona)
>   ..
> drivers-$(CONFIG_MATH_EMULATION) += arch/x86/math-emu/
> drivers-$(CONFIG_PCI)  += arch/x86/pci/
> drivers-$(CONFIG_OPROFILE) += arch/x86/oprofile/
> drivers-$(CONFIG_PM) += arch/x86/power/
> drivers-$(CONFIG_FB) += arch/x86/video/
> ifeq ($(CONFIG_X86_DECODER_SELFTEST),y)
>
> Thanks
> Zhenzhong
> On 2018/12/5 11:00, Masahiro Yamada wrote:
> > It is wrong to add CONFIG option diagnostic to the Makefile parse
> > stage.
> >
> > Once you are hit by the error about non-retpoline compiler, the
> > compilation still breaks even after disabling CONFIG_RETPOLINE.
> >
> > The easiest fix is to move this check to the "archprepare" like commit
> > 829fe4aa9ac1 ("x86: Allow generating user-space headers without a
> > compiler") did.
> >
> > Link: https://lkml.org/lkml/2018/12/4/206
> > Reported-by: Meelis Roos 
> > Fixes: 4cd24de3a098 ("x86/retpoline: Make CONFIG_RETPOLINE depend on 
> > compiler support")
> > Signed-off-by: Masahiro Yamada 
> > ---
> >
> >   arch/x86/Makefile | 14 --
> >   1 file changed, 8 insertions(+), 6 deletions(-)
> >
> > diff --git a/arch/x86/Makefile b/arch/x86/Makefile
> > index f5d7f41..ae0148b 100644
> > --- a/arch/x86/Makefile
> > +++ b/arch/x86/Makefile
> > @@ -219,12 +219,7 @@ KBUILD_CFLAGS += -Wno-sign-compare
> >   KBUILD_CFLAGS += -fno-asynchronous-unwind-tables
> >
> >   # Avoid indirect branches in kernel to deal with Spectre
> > -ifdef CONFIG_RETPOLINE
> > -ifeq ($(RETPOLINE_CFLAGS),)
> > -  $(error You are building kernel with non-retpoline compiler, please 
> > update your compiler.)
> > -endif
> > -  KBUILD_CFLAGS += $(RETPOLINE_CFLAGS)
> > -endif
> > +KBUILD_CFLAGS += $(RETPOLINE_CFLAGS)
> >
> >   archscripts: scripts_basic
> >   $(Q)$(MAKE) $(build)=arch/x86/tools relocs
> > @@ -307,6 +302,13 @@ ifndef CC_HAVE_ASM_GOTO
> >   @echo Compiler lacks asm-goto support.
> >   @exit 1
> >   endif
> > +ifdef CONFIG_RETPOLINE
> > +ifeq ($(RETPOLINE_CFLAGS),)
> > + @echo "You are building kernel with non-retpoline compiler." >&2
> > + @echo "Please update your compiler." >&2
> > + @false
> > +endif
> > +endif
> >
> >   archclean:
> >   $(Q)rm -rf $(objtree)/arch/i386



--
Best Regards
Masahiro Yamada


Re: [PATCH 07/14] clock: milbeaut: Add Milbeaut M10V clock control

2018-12-04 Thread Masahiro Yamada
On Wed, Dec 5, 2018 at 3:14 AM Stephen Boyd  wrote:
>
> Quoting Masahiro Yamada (2018-12-04 03:03:53)
> > Hi Stephen,
> >
> >
> > On Fri, Nov 30, 2018 at 5:31 PM Stephen Boyd  wrote:
> > >
> > > Quoting Sugaya Taichi (2018-11-18 17:01:12)
> > > > Add Milbeaut M10V clock ( including PLL ) control.
> > >
> > > Please give some more details here.
> > >
> > > >
> > > > Signed-off-by: Sugaya Taichi 
> > > > ---
> > > >  drivers/clk/Makefile   |   1 +
> > > >  drivers/clk/clk-m10v.c | 671 
> > > > +
> > >
> > > And this is different from Uniphier? Maybe we need a socionext
> > > directory under drivers/clk/.
> >
> >
> >
> > This is always a difficult question,
> > and I do not have a strong opinion.
> >
> >
> > I am fine with moving the files to drivers/clk/socionext
> > although no file would be shared.
> >
> >
> > FYI
> >
> > UniPhier and Milbeaut are completely different platforms
> > developed/maintained by different teams.
> >
> > They happen to live in the same company now
> > just because Socionext merged the LSI business from Panasonic and Fujitsu.
> >
> > UniPhier originates in Panasonic, while Milbeaut in Fujitsu.
> >
>
> Thanks for the background info. I'd prefer to defer to however the dts
> files are getting split up into directories. If they're all put under
> arch/arm64/boot/dts/socionext/ then I would say combine the two clk
> drivers into a socionext directory. Otherwise, keep them split out.


If you want to align with the DT directory structure,
the answer is clear.


Milbeaut DT files will be put together with UniPhier ones
into socionext directory.


For arm64, DT directories are already sorted out by vendors.

Even 32-bit ARM is going to that way.

Rob Herring just posted a python script
to move all DT files in arch/arm/boot/dts/
into vendor subdirectories.


Please let me know if you want me to
move drivers/clk/uniphier/* to drivers/clk/socionext/*.



-- 
Best Regards
Masahiro Yamada


[PATCH] x86/build: fix compiler support check for CONFIG_RETPOLINE

2018-12-04 Thread Masahiro Yamada
It is wrong to add CONFIG option diagnostic to the Makefile parse
stage.

Once you are hit by the error about non-retpoline compiler, the
compilation still breaks even after disabling CONFIG_RETPOLINE.

The easiest fix is to move this check to the "archprepare" like commit
829fe4aa9ac1 ("x86: Allow generating user-space headers without a
compiler") did.

Link: https://lkml.org/lkml/2018/12/4/206
Reported-by: Meelis Roos 
Fixes: 4cd24de3a098 ("x86/retpoline: Make CONFIG_RETPOLINE depend on compiler 
support")
Signed-off-by: Masahiro Yamada 
---

 arch/x86/Makefile | 14 --
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/arch/x86/Makefile b/arch/x86/Makefile
index f5d7f41..ae0148b 100644
--- a/arch/x86/Makefile
+++ b/arch/x86/Makefile
@@ -219,12 +219,7 @@ KBUILD_CFLAGS += -Wno-sign-compare
 KBUILD_CFLAGS += -fno-asynchronous-unwind-tables
 
 # Avoid indirect branches in kernel to deal with Spectre
-ifdef CONFIG_RETPOLINE
-ifeq ($(RETPOLINE_CFLAGS),)
-  $(error You are building kernel with non-retpoline compiler, please update 
your compiler.)
-endif
-  KBUILD_CFLAGS += $(RETPOLINE_CFLAGS)
-endif
+KBUILD_CFLAGS += $(RETPOLINE_CFLAGS)
 
 archscripts: scripts_basic
$(Q)$(MAKE) $(build)=arch/x86/tools relocs
@@ -307,6 +302,13 @@ ifndef CC_HAVE_ASM_GOTO
@echo Compiler lacks asm-goto support.
@exit 1
 endif
+ifdef CONFIG_RETPOLINE
+ifeq ($(RETPOLINE_CFLAGS),)
+   @echo "You are building kernel with non-retpoline compiler." >&2
+   @echo "Please update your compiler." >&2
+   @false
+endif
+endif
 
 archclean:
$(Q)rm -rf $(objtree)/arch/i386
-- 
2.7.4



Re: Compiling with old gcc breaks when CONFIG_RETPOLINE is off

2018-12-04 Thread Masahiro Yamada
On Wed, Dec 5, 2018 at 10:31 AM Zhenzhong Duan
 wrote:
>
> Hi Meelis,
>
> Could you try below change? It force syncconfig for  any 'make *config"
>
> so that autoconf.h and auto.conf are always updated.


NACK.

syncconfig should happen only when you are about to build something.

This patch let "make *config" touch include/config/* around
unnecessarily.




> diff --git a/scripts/kconfig/conf.c b/scripts/kconfig/conf.c
> index 98e0c7a..802875b 100644
> --- a/scripts/kconfig/conf.c
> +++ b/scripts/kconfig/conf.c
> @@ -38,7 +38,7 @@ enum input_mode {
>
>   static int indent = 1;
>   static int tty_stdio;
> -static int sync_kconfig;
> +static int sync_kconfig = 1;
>   static int conf_cnt;
>   static char line[PATH_MAX];
>   static struct menu *rootEntry;
> @@ -506,11 +506,11 @@ int main(int ac, char **av)
>   * Suppress distracting "configuration written
> to ..."
>   */
>  conf_set_message_callback(NULL);
> -   sync_kconfig = 1;
>  break;
>  case defconfig:
>  case savedefconfig:
>  defconfig_file = optarg;
> +   sync_kconfig = 0;
>  break;
>  case randconfig:
>  {
> @@ -537,13 +537,15 @@ int main(int ac, char **av)
>  srand(seed);
>  break;
>  }
> +   case listnewconfig:
> +   sync_kconfig = 0;
> +   /* fall through */
>  case oldaskconfig:
>  case oldconfig:
>  case allnoconfig:
>  case allyesconfig:
>  case allmodconfig:
>  case alldefconfig:
> -   case listnewconfig:
>  case olddefconfig:
>  break;
>  case '?':
>
> Thanks
>
> Zhenzhong
>
> On 2018/12/4 17:58, Meelis Roos wrote:
> > Just tried 4.20-rc5 on an old K6-2 PC with gcc 5.3.1, got an error
> > about non-retpoline compiler,
> > turned CONFIG_RETPOLINE off and retried.
> >
> > To my surprise, compilation still breaks with
> > arch/x86/Makefile:224: *** You are building kernel with non-retpoline
> > compiler, please update your compiler..  Stop.
> >
> > As I read the Makefile, it should error only when CONFIG_RETPOLINE is
> > enabled, but it still breaks.
> >
> > $ grep -r CONFIG_RETPOLINE .config
> > # CONFIG_RETPOLINE is not set
> >
> > $ grep -r CONFIG_RETPOLINE include/
> > include/generated/autoconf.h:#define CONFIG_RETPOLINE 1
> > include/config/auto.conf:CONFIG_RETPOLINE=y
> >
> > So the headers have not been updated yet, maybe?
> >



-- 
Best Regards
Masahiro Yamada


Re: [PATCH 11/14] pinctrl: milbeaut: Add Milbeaut M10V pinctrl

2018-12-04 Thread Masahiro Yamada
Hi Sugaya-san

On Mon, Nov 19, 2018 at 10:01 AM Sugaya Taichi
 wrote:
>
> Add Milbeaut M10V pinctrl.
> The M10V has the pins that can be used GPIOs or take multiple other
> functions.
>
> Signed-off-by: Sugaya Taichi 


This patch was sent to:

linux-...@vger.kernel.org,
devicet...@vger.kernel.org,
linux-arm-ker...@lists.infradead.org,
linux-kernel@vger.kernel.org,
linux-ser...@vger.kernel.org




Unfortunately, the most important ML

  linux-g...@vger.kernel.org

was not addressed.


The pinctrl maintainer may not notice this patch.







> diff --git a/drivers/pinctrl/pinctrl-m10v.c b/drivers/pinctrl/pinctrl-m10v.c
> new file mode 100644
> index 000..d4ca713
> --- /dev/null
> +++ b/drivers/pinctrl/pinctrl-m10v.c
> @@ -0,0 +1,765 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2018 Socionext Ltd.
> + * Copyright (C) 2015 Linaro Ltd.
> + * Author: Jassi Brar 


My company name is "Socionext Inc." instead of "Socionext Ltd."







> +static struct platform_driver m10v_pinctrl_driver = {
> +   .probe  = m10v_pinctrl_probe,
> +   .driver = {
> +   .name   = "m10v-pinctrl",
> +   .of_match_table = m10v_pmatch,
> +   },
> +};
> +
> +static int __init m10v_pinctrl_init(void)
> +{
> +   return platform_driver_register(_pinctrl_driver);
> +}
> +arch_initcall(m10v_pinctrl_init);


Can't it be builtin_platform_driver()?

Which device requires this to be arch_initcall()?





--
Best Regards
Masahiro Yamada


Re: [PATCH 07/14] clock: milbeaut: Add Milbeaut M10V clock control

2018-12-04 Thread Masahiro Yamada
Hi Stephen,


On Fri, Nov 30, 2018 at 5:31 PM Stephen Boyd  wrote:
>
> Quoting Sugaya Taichi (2018-11-18 17:01:12)
> > Add Milbeaut M10V clock ( including PLL ) control.
>
> Please give some more details here.
>
> >
> > Signed-off-by: Sugaya Taichi 
> > ---
> >  drivers/clk/Makefile   |   1 +
> >  drivers/clk/clk-m10v.c | 671 
> > +
>
> And this is different from Uniphier? Maybe we need a socionext
> directory under drivers/clk/.



This is always a difficult question,
and I do not have a strong opinion.


I am fine with moving the files to drivers/clk/socionext
although no file would be shared.


FYI

UniPhier and Milbeaut are completely different platforms
developed/maintained by different teams.

They happen to live in the same company now
just because Socionext merged the LSI business from Panasonic and Fujitsu.

UniPhier originates in Panasonic, while Milbeaut in Fujitsu.


Thanks.




--
Best Regards
Masahiro Yamada


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

2018-12-02 Thread 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." works like a phony target
to generate the following four images.

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

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

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

 - arch/microblaze/boot/simpleImage..strip:
   stripped vmlinux

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

Signed-off-by: Masahiro Yamada 
---

 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 1/7] microblaze: fix cleaning of boot images

2018-12-02 Thread Masahiro Yamada
The simpleImage. target generates the following files:

  arch/microblaze/boot/simpleImage.
  arch/microblaze/boot/simpleImage..ub
  arch/microblaze/boot/simpleImage..strip
  arch/microblaze/boot/simpleImage..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 
---

 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 6/7] microblaze: fix race condition in building boot images

2018-12-02 Thread 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 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 
---

 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

2018-12-02 Thread 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 
---

 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



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

2018-12-02 Thread 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 
---

 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 5/7] microblaze: add linux.bin* and simpleImage.* to PHONY

2018-12-02 Thread 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 
---

 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 0/7] microblaze: fix various problems in building boot images

2018-12-02 Thread Masahiro Yamada
This patch set fixes various issues in microblaze Makefiles.

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

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

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

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

 - arch/microblaze/boot/simpleImage..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= 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 2/7] microblaze: adjust the help to the real behavior

2018-12-02 Thread Masahiro Yamada
"make ARCH=microblaze help" mentions simpleImage..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. works like a phony target that
generates multiple images. Reflect the behavior to the help message.

Signed-off-by: Masahiro Yamada 
---

 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. - ELF image with $(arch)/boot/dts/.dts linked 
in'
-  echo '   - stripped elf with fdt blob'
-  echo '  simpleImage..unstrip - full ELF image with fdt blob'
+  echo '  simpleImage. - Create images with $(arch)/boot/dts/.dts 
linked in'
   echo '  *_defconfig  - Select default config from 
arch/microblaze/configs'
   echo ''
   echo '  Targets with  embed a device tree blob inside the image'
-- 
2.7.4



Re: [PATCH v2 1/2] kbuild: make 'archprepare' depend on 'scripts'

2018-12-01 Thread Masahiro Yamada
On Thu, Nov 29, 2018 at 12:57 PM Masahiro Yamada
 wrote:
>
> Before start descending, Kbuild needs to run 'prepare' and 'scripts',
> which has been orthogonal to each other.
>
> Going forward, let's consider 'scripts' is a part of the preparation.
> This will allow more cleanups.
>
> Move 'scripts' to the prerequisite of 'archprepare', where UML starts
> compiling target *.c files.
>
> Signed-off-by: Masahiro Yamada 
> ---
>
> Changes in v2:
>   - It turned out UML + CONFIG_GCC_PLUGINS is broken for a long time.
> https://patchwork.kernel.org/patch/10703853/
> Rebased on the top of the fix-up

Applied to linux-kbuild.


>  Makefile | 32 +++-
>  1 file changed, 15 insertions(+), 17 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index f7cc3ee..9eb3f4f 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1062,7 +1062,7 @@ $(sort $(vmlinux-deps)): $(vmlinux-dirs) ;
>  # Error messages still appears in the original language
>
>  PHONY += $(vmlinux-dirs)
> -$(vmlinux-dirs): prepare scripts
> +$(vmlinux-dirs): prepare
> $(Q)$(MAKE) $(build)=$@ need-builtin=1
>
>  define filechk_kernel.release
> @@ -1112,7 +1112,7 @@ prepare1: prepare2 $(version_h) $(autoksyms_h) 
> include/generated/utsrelease.h
>
>  macroprepare: prepare1 archmacros
>
> -archprepare: archheaders archscripts macroprepare scripts_basic gcc-plugins
> +archprepare: archheaders archscripts macroprepare scripts gcc-plugins
>
>  prepare0: archprepare
> $(Q)$(MAKE) $(build)=scripts/mod
> @@ -1286,7 +1286,7 @@ modules.builtin: $(vmlinux-dirs:%=%/modules.builtin)
>
>  # Target to prepare building external modules
>  PHONY += modules_prepare
> -modules_prepare: prepare scripts
> +modules_prepare: prepare
>
>  # Target to install modules
>  PHONY += modules_install
> @@ -1604,11 +1604,9 @@ help:
> @echo  '  clean   - remove generated files in module 
> directory only'
> @echo  ''
>
> -# Dummies...
> -PHONY += prepare scripts
> +PHONY += prepare
>  prepare:
> $(cmd_crmodverdir)
> -scripts: ;
>  endif # KBUILD_EXTMOD
>
>  clean: $(clean-dirs)
> @@ -1712,33 +1710,33 @@ else
>  target-dir = $(if $(KBUILD_EXTMOD),$(dir $<),$(dir $@))
>  endif
>
> -%.s: %.c prepare scripts FORCE
> +%.s: %.c prepare FORCE
> $(Q)$(MAKE) $(build)=$(build-dir) $(target-dir)$(notdir $@)
> -%.i: %.c prepare scripts FORCE
> +%.i: %.c prepare FORCE
> $(Q)$(MAKE) $(build)=$(build-dir) $(target-dir)$(notdir $@)
> -%.o: %.c prepare scripts FORCE
> +%.o: %.c prepare FORCE
> $(Q)$(MAKE) $(build)=$(build-dir) $(target-dir)$(notdir $@)
> -%.lst: %.c prepare scripts FORCE
> +%.lst: %.c prepare FORCE
> $(Q)$(MAKE) $(build)=$(build-dir) $(target-dir)$(notdir $@)
> -%.s: %.S prepare scripts FORCE
> +%.s: %.S prepare FORCE
> $(Q)$(MAKE) $(build)=$(build-dir) $(target-dir)$(notdir $@)
> -%.o: %.S prepare scripts FORCE
> +%.o: %.S prepare FORCE
> $(Q)$(MAKE) $(build)=$(build-dir) $(target-dir)$(notdir $@)
> -%.symtypes: %.c prepare scripts FORCE
> +%.symtypes: %.c prepare FORCE
> $(Q)$(MAKE) $(build)=$(build-dir) $(target-dir)$(notdir $@)
> -%.ll: %.c prepare scripts FORCE
> +%.ll: %.c prepare FORCE
> $(Q)$(MAKE) $(build)=$(build-dir) $(target-dir)$(notdir $@)
>
>  # Modules
> -/: prepare scripts FORCE
> +/: prepare FORCE
> $(Q)$(MAKE) KBUILD_MODULES=$(if $(CONFIG_MODULES),1) \
> $(build)=$(build-dir)
>  # Make sure the latest headers are built for Documentation
>  Documentation/ samples/: headers_install
> -%/: prepare scripts FORCE
> +%/: prepare FORCE
> $(Q)$(MAKE) KBUILD_MODULES=$(if $(CONFIG_MODULES),1) \
> $(build)=$(build-dir)
> -%.ko: prepare scripts FORCE
> +%.ko: prepare FORCE
> $(Q)$(MAKE) KBUILD_MODULES=$(if $(CONFIG_MODULES),1)   \
> $(build)=$(build-dir) $(@:.ko=.o)
> $(Q)$(MAKE) -f $(srctree)/scripts/Makefile.modpost
> --
> 2.7.4
>


-- 
Best Regards
Masahiro Yamada


Re: [PATCH v2 2/2] kbuild: descend into scripts/gcc-plugins/ via scripts/Makefile

2018-12-01 Thread Masahiro Yamada
On Thu, Nov 29, 2018 at 12:57 PM Masahiro Yamada
 wrote:
>
> Now that 'archprepare' depends on 'scripts', Kbuild can descend into
> scripts/gcc-plugins in a more standard way.
>
> Signed-off-by: Masahiro Yamada 
> Reviewed-by: Kees Cook 
> ---

Applied to linux-kbuild.


> Changes in v2:
>   - It turned out UML + CONFIG_GCC_PLUGINS is broken for a long time.
> https://patchwork.kernel.org/patch/10703853/
> Rebased on the top of the fix-up
>
>  Makefile | 2 +-
>  scripts/Makefile | 3 ++-
>  scripts/Makefile.gcc-plugins | 8 
>  3 files changed, 3 insertions(+), 10 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index 9eb3f4f..c1f87cf 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1112,7 +1112,7 @@ prepare1: prepare2 $(version_h) $(autoksyms_h) 
> include/generated/utsrelease.h
>
>  macroprepare: prepare1 archmacros
>
> -archprepare: archheaders archscripts macroprepare scripts gcc-plugins
> +archprepare: archheaders archscripts macroprepare scripts
>
>  prepare0: archprepare
> $(Q)$(MAKE) $(build)=scripts/mod
> diff --git a/scripts/Makefile b/scripts/Makefile
> index b48259d..feb1f71 100644
> --- a/scripts/Makefile
> +++ b/scripts/Makefile
> @@ -36,9 +36,10 @@ PHONY += build_unifdef
>  build_unifdef: $(obj)/unifdef
> @:
>
> +subdir-$(CONFIG_GCC_PLUGINS) += gcc-plugins
>  subdir-$(CONFIG_MODVERSIONS) += genksyms
>  subdir-$(CONFIG_SECURITY_SELINUX) += selinux
>  subdir-$(CONFIG_GDB_SCRIPTS) += gdb
>
>  # Let clean descend into subdirs
> -subdir-+= basic dtc kconfig mod package gcc-plugins
> +subdir-+= basic dtc kconfig mod package
> diff --git a/scripts/Makefile.gcc-plugins b/scripts/Makefile.gcc-plugins
> index 46c5c68..c36f199 100644
> --- a/scripts/Makefile.gcc-plugins
> +++ b/scripts/Makefile.gcc-plugins
> @@ -49,11 +49,3 @@ KBUILD_CFLAGS += $(GCC_PLUGINS_CFLAGS)
>  # All enabled GCC plugins are collected here for building below.
>  GCC_PLUGIN := $(gcc-plugin-y)
>  export GCC_PLUGIN
> -
> -# Actually do the build, if requested.
> -PHONY += gcc-plugins
> -gcc-plugins: scripts_basic
> -ifdef CONFIG_GCC_PLUGINS
> -   $(Q)$(MAKE) $(build)=scripts/gcc-plugins
> -endif
> -   @:
> --
> 2.7.4
>


-- 
Best Regards
Masahiro Yamada


Re: [PATCH] pcmcia: remove per-arch PCMCIA config entry

2018-12-01 Thread Masahiro Yamada
On Mon, Nov 26, 2018 at 6:24 PM Dominik Brodowski
 wrote:
>
> On Mon, Nov 26, 2018 at 05:15:41PM +0900, Masahiro Yamada wrote:
> > Now that all architectures include drivers/pcmcia/Kconfig where
> > the PCMCIA config is defined, the PCMCIA config entries in per-arch
> > Kconfig files are redundant.
> >
> > Signed-off-by: Masahiro Yamada 
> > ---
> >
> > I will queue this up to my kbuild tree
> > along with Christoph's clean-up patch set.
>
> Acked-by: Dominik Brodowski 
>
> Thanks,
>     Dominik

Applied to linux-kbuild.


-- 
Best Regards
Masahiro Yamada


Re: [PATCH 1/3] modpost: refactor seen flag clearing in add_depends()

2018-12-01 Thread Masahiro Yamada
On Sat, Nov 24, 2018 at 5:08 PM Masahiro Yamada
 wrote:
>
> You do not need to iterate over all modules for resetting ->seen flag
> because add_depends() is only interested in modules that export symbols
> referenced from the given 'mod'.
>
> This also avoids shadowing the 'modules' parameter of add_depends().
>
> Signed-off-by: Masahiro Yamada 
> ---


Applied to linux-kbuild.



>  scripts/mod/modpost.c | 12 ++--
>  1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
> index c64066d..39432c2 100644
> --- a/scripts/mod/modpost.c
> +++ b/scripts/mod/modpost.c
> @@ -2229,15 +2229,15 @@ static int add_versions(struct buffer *b, struct 
> module *mod)
> return err;
>  }
>
> -static void add_depends(struct buffer *b, struct module *mod,
> -   struct module *modules)
> +static void add_depends(struct buffer *b, struct module *mod)
>  {
> struct symbol *s;
> -   struct module *m;
> int first = 1;
>
> -   for (m = modules; m; m = m->next)
> -   m->seen = is_vmlinux(m->name);
> +   /* Clear ->seen flag of modules that own symbols needed by this. */
> +   for (s = mod->unres; s; s = s->next)
> +   if (s->module)
> +   s->module->seen = is_vmlinux(s->module->name);
>
> buf_printf(b, "\n");
> buf_printf(b, "static const char __module_depends[]\n");
> @@ -2506,7 +2506,7 @@ int main(int argc, char **argv)
> add_retpoline();
> add_staging_flag(, mod->name);
> err |= add_versions(, mod);
> -   add_depends(, mod, modules);
> +   add_depends(, mod);
> add_moddevtable(, mod);
> add_srcversion(, mod);
>
> --
> 2.7.4
>


-- 
Best Regards
Masahiro Yamada


Re: [PATCH] kbuild: fix UML build error with CONFIG_GCC_PLUGINS

2018-12-01 Thread Masahiro Yamada
On Thu, Nov 29, 2018 at 12:01 PM Masahiro Yamada
 wrote:
>
> UML fails to build with CONFIG_GCC_PLUGINS=y.
>
> $ make -s ARCH=um mrproper
> $ make -s ARCH=um allmodconfig
> $ make ARCH=um
>   UPD include/generated/uapi/linux/version.h
>   WRAParch/x86/include/generated/uapi/asm/bpf_perf_event.h
>   WRAParch/x86/include/generated/uapi/asm/poll.h
>   WRAParch/x86/include/generated/asm/dma-contiguous.h
>   WRAParch/x86/include/generated/asm/early_ioremap.h
>   WRAParch/x86/include/generated/asm/export.h
>   WRAParch/x86/include/generated/asm/mcs_spinlock.h
>   WRAParch/x86/include/generated/asm/mm-arch-hooks.h
>   SYSTBL  arch/x86/include/generated/asm/syscalls_32.h
>   SYSHDR  arch/x86/include/generated/asm/unistd_32_ia32.h
>   SYSHDR  arch/x86/include/generated/asm/unistd_64_x32.h
>   SYSTBL  arch/x86/include/generated/asm/syscalls_64.h
>   SYSHDR  arch/x86/include/generated/uapi/asm/unistd_32.h
>   SYSHDR  arch/x86/include/generated/uapi/asm/unistd_64.h
>   SYSHDR  arch/x86/include/generated/uapi/asm/unistd_x32.h
>   HOSTCC  scripts/unifdef
>   CC  arch/x86/um/user-offsets.s
> cc1: error: cannot load plugin ./scripts/gcc-plugins/cyc_complexity_plugin.so
> ./scripts/gcc-plugins/cyc_complexity_plugin.so: cannot open shared object 
> file: No such file or directory
> cc1: error: cannot load plugin ./scripts/gcc-plugins/structleak_plugin.so
> ./scripts/gcc-plugins/structleak_plugin.so: cannot open shared object file: 
> No such file or directory
> cc1: error: cannot load plugin ./scripts/gcc-plugins/latent_entropy_plugin.so
> ./scripts/gcc-plugins/latent_entropy_plugin.so: cannot open shared object 
> file: No such file or directory
> cc1: error: cannot load plugin 
> ./scripts/gcc-plugins/randomize_layout_plugin.so
> ./scripts/gcc-plugins/randomize_layout_plugin.so: cannot open shared object 
> file: No such file or directory
> make[1]: *** [scripts/Makefile.build;119: arch/x86/um/user-offsets.s] Error 1
> make: *** [arch/um/Makefile;152: arch/x86/um/user-offsets.s] Error 2
>
> Change the order of the preparation stage (with cleanups) to make sure
> gcc-plugins is built before descending to arch/x86/um/.
>
> Reported-by: kbuild test robot 
> Fixes: 6b90bd4ba40b ("GCC plugin infrastructure")
> Signed-off-by: Masahiro Yamada 
> ---


Applied to linux-kbuild.


>
> I will pick up this to kbuild tree to avoid conflicts.
> Ack from UML people is appreciated, though.
>
>
-- 
Best Regards
Masahiro Yamada


Re: [PATCH v4 1/3] kernel.h: disable type-checks in container_of() for Sparse

2018-11-30 Thread Masahiro Yamada
Hi Andrew,


On Sat, Nov 24, 2018 at 6:06 PM Miguel Ojeda
 wrote:
>
> On Fri, Nov 23, 2018 at 10:14 PM Luc Van Oostenryck
>  wrote:
> >
> > On Thu, Nov 22, 2018 at 12:14:20PM +0900, Masahiro Yamada wrote:
> > > When I tried to enable BUILD_BUG_ON for Sparse, the kbuild test robot
> > > reported lots of "unknown expression" warnings from container_of(),
> > > which seemed false positive.
> > >
> > > I addressed this in [1], but fixing Sparse is the right thing to do.
> > >
> > > The issue was fixed by Sparse commit 0eb8175d3e9c ("fix expansion of
> > > function designator"), but it will take time until the fixed version
> > > of Sparse is widely available.
> > >
> > > Disable the container_of() type checks for Sparse for now.
> >
> > I would prefer that developers upgrade their version of sparse but ...
> >
> > Reviewed-by: Luc Van Oostenryck 
>
> Indeed. If someone is writing code for the latest kernels, I think it
> is reasonable to assume they are able to use the latest sparse too,
> since it is not required for compilation anyway.



I was reconsidering about this.

I saw other Sparse warnings anyway
unless I use the state-of-the-art version of Sparse.


So, now I think Luc and Miguel were right.


Requiring the latest Sparse is the right solution.
We do not need to take care of old Sparse.


Andrew,
Could you drop this patch please?


-- 
Best Regards
Masahiro Yamada


[PATCH 2/5] kconfig: rename conf_split_config() to conf_touch_deps()

2018-11-30 Thread Masahiro Yamada
According to commit 2e3646e51b2d ("kconfig: integrate split config
into silentoldconfig"), this function was named after split-include
tool, which used to exist in old versions of Linux.

Setting aside the historical reason, rename it into a more intuitive
name. This function touches timestamp files under include/config/
in order to interact with the fixdep tool.

Signed-off-by: Masahiro Yamada 
---

 scripts/kconfig/confdata.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/scripts/kconfig/confdata.c b/scripts/kconfig/confdata.c
index 1e35529..4c76d56 100644
--- a/scripts/kconfig/confdata.c
+++ b/scripts/kconfig/confdata.c
@@ -906,7 +906,7 @@ static int conf_write_dep(const char *name)
return 0;
 }
 
-static int conf_split_config(void)
+static int conf_touch_deps(void)
 {
const char *name;
char path[PATH_MAX+1];
@@ -1028,7 +1028,7 @@ int conf_write_autoconf(int overwrite)
 
conf_write_dep("include/config/auto.conf.cmd");
 
-   if (conf_split_config())
+   if (conf_touch_deps())
return 1;
 
out = fopen(".tmpconfig", "w");
-- 
2.7.4



[PATCH 4/5] kconfig: remove S_OTHER symbol type and correct dependency tracking

2018-11-30 Thread Masahiro Yamada
The S_OTHER type could be set only when conf_read_simple() is reading
include/config/auto.conf file.

For example, CONFIG_FOO=y exists in include/config/auto.conf but it is
missing from the currently parsed Kconfig files, sym_lookup() allocates
a new symbol, and sets its type to S_OTHER.

Strangely, it will be set to S_STRING by conf_set_sym_val() a few lines
below while it is obviously bool or tristate type. On the other hand,
when CONFIG_BAR="bar" is being dropped from include/config/auto.conf,
its type remains S_OTHER. Because for_all_symbols() omits S_OTHER
symbols, conf_touch_deps() misses to touch include/config/bar.h

This behavior has been a pretty mystery for me, and digging the git
histroy did not help. At least, touching depfiles is broken for string
type symbols.

I removed S_OTHER entirely, and reimplemented it more simply.

If CONFIG_FOO was visible in the previous syncconfig, but is missing
now, what we want to do is quite simple; just call conf_touch_dep()
to touch include/config/foo.h instead of allocating a new symbol data.

Signed-off-by: Masahiro Yamada 
---

 scripts/kconfig/confdata.c | 33 ++---
 scripts/kconfig/expr.h |  4 ++--
 scripts/kconfig/symbol.c   |  3 ---
 3 files changed, 16 insertions(+), 24 deletions(-)

diff --git a/scripts/kconfig/confdata.c b/scripts/kconfig/confdata.c
index 7263b83..800fb16 100644
--- a/scripts/kconfig/confdata.c
+++ b/scripts/kconfig/confdata.c
@@ -227,14 +227,6 @@ static int conf_set_sym_val(struct symbol *sym, int def, 
int def_flags, char *p)
conf_warning("symbol value '%s' invalid for %s",
 p, sym->name);
return 1;
-   case S_OTHER:
-   if (*p != '"') {
-   for (p2 = p; *p2 && !isspace(*p2); p2++)
-   ;
-   sym->type = S_STRING;
-   goto done;
-   }
-   /* fall through */
case S_STRING:
if (*p++ != '"')
break;
@@ -253,7 +245,6 @@ static int conf_set_sym_val(struct symbol *sym, int def, 
int def_flags, char *p)
/* fall through */
case S_INT:
case S_HEX:
-   done:
if (sym_string_valid(sym, p)) {
sym->def[def].val = xstrdup(p);
sym->flags |= def_flags;
@@ -434,17 +425,22 @@ int conf_read_simple(const char *name, int def)
if (*p2 == '\r')
*p2 = 0;
}
-   if (def == S_DEF_USER) {
-   sym = sym_find(line + strlen(CONFIG_));
-   if (!sym) {
+
+   sym = sym_find(line + strlen(CONFIG_));
+   if (!sym) {
+   if (def == S_DEF_AUTO)
+   /*
+* Reading from include/config/auto.conf
+* If CONFIG_FOO previously existed in
+* auto.conf but it is missing now,
+* include/config/foo.h must be touched.
+*/
+   conf_touch_dep(line + strlen(CONFIG_));
+   else
sym_add_change_count(1);
-   continue;
-   }
-   } else {
-   sym = sym_lookup(line + strlen(CONFIG_), 0);
-   if (sym->type == S_UNKNOWN)
-   sym->type = S_OTHER;
+   continue;
}
+
if (sym->flags & def_flags) {
conf_warning("override: reassigning to symbol 
%s", sym->name);
}
@@ -710,7 +706,6 @@ static void conf_write_symbol(FILE *fp, struct symbol *sym,
const char *str;
 
switch (sym->type) {
-   case S_OTHER:
case S_UNKNOWN:
break;
case S_STRING:
diff --git a/scripts/kconfig/expr.h b/scripts/kconfig/expr.h
index 7c329e1..2b7e222 100644
--- a/scripts/kconfig/expr.h
+++ b/scripts/kconfig/expr.h
@@ -62,7 +62,7 @@ struct symbol_value {
 };
 
 enum symbol_type {
-   S_UNKNOWN, S_BOOLEAN, S_TRISTATE, S_INT, S_HEX, S_STRING, S_OTHER
+   S_UNKNOWN, S_BOOLEAN, S_TRISTATE, S_INT, S_HEX, S_STRING
 };
 
 /* enum values are used as index to symbol.def[] */
@@ -131,7 +131,7 @@ struct symbol {
struct expr_value implied;
 };
 
-#define for_all_symbols(i, sym) for (i = 0; i < SYMBOL_HASHSIZE; i++) for (sym 
= symbol_hash[i]; sym; 

[PATCH 3/5] kconfig: split out code touching a file to conf_touch_dep()

2018-11-30 Thread Masahiro Yamada
conf_touch_deps() iterates over symbols, touching corresponding
include/config/*.h files as needed.

Split the part that touches a single file into a new helper so it can
be reused.

The new helper, conf_touch_dep(), takes a symbol name as a parameter,
and touches the corresponding include/config/*.h file.

Signed-off-by: Masahiro Yamada 
---

 scripts/kconfig/confdata.c | 92 --
 1 file changed, 49 insertions(+), 43 deletions(-)

diff --git a/scripts/kconfig/confdata.c b/scripts/kconfig/confdata.c
index 4c76d56..7263b83 100644
--- a/scripts/kconfig/confdata.c
+++ b/scripts/kconfig/confdata.c
@@ -74,6 +74,47 @@ static int make_parent_dir(const char *path)
return 0;
 }
 
+static char depfile_path[PATH_MAX];
+static size_t depfile_prefix_len;
+
+/* touch depfile for symbol 'name' */
+static int conf_touch_dep(const char *name)
+{
+   int fd, ret;
+   const char *s;
+   char *d, c;
+
+   /* check overflow: prefix + name + ".h" + '\0' must fix in buffer. */
+   if (depfile_prefix_len + strlen(name) + 3 > sizeof(depfile_path))
+   return -1;
+
+   d = depfile_path + depfile_prefix_len;
+   s = name;
+
+   while ((c = *s++))
+   *d++ = (c == '_') ? '/' : tolower(c);
+   strcpy(d, ".h");
+
+   /* Assume directory path already exists. */
+   fd = open(depfile_path, O_WRONLY | O_CREAT | O_TRUNC, 0644);
+   if (fd == -1) {
+   if (errno != ENOENT)
+   return -1;
+
+   ret = make_parent_dir(depfile_path);
+   if (ret)
+   return ret;
+
+   /* Try it again. */
+   fd = open(depfile_path, O_WRONLY | O_CREAT | O_TRUNC, 0644);
+   if (fd == -1)
+   return -1;
+   }
+   close(fd);
+
+   return 0;
+}
+
 struct conf_printer {
void (*print_symbol)(FILE *, struct symbol *, const char *, void *);
void (*print_comment)(FILE *, const char *, void *);
@@ -909,21 +950,16 @@ static int conf_write_dep(const char *name)
 static int conf_touch_deps(void)
 {
const char *name;
-   char path[PATH_MAX+1];
-   char *s, *d, c;
struct symbol *sym;
-   int res, i, fd;
+   int res, i;
+
+   strcpy(depfile_path, "include/config/");
+   depfile_prefix_len = strlen(depfile_path);
 
name = conf_get_autoconfig_name();
conf_read_simple(name, S_DEF_AUTO);
sym_calc_value(modules_sym);
 
-   if (make_parent_dir("include/config/foo.h"))
-   return 1;
-   if (chdir("include/config"))
-   return 1;
-
-   res = 0;
for_all_symbols(i, sym) {
sym_calc_value(sym);
if ((sym->flags & SYMBOL_NO_WRITE) || !sym->name)
@@ -975,42 +1011,12 @@ static int conf_touch_deps(void)
 *  different from 'no').
 */
 
-   /* Replace all '_' and append ".h" */
-   s = sym->name;
-   d = path;
-   while ((c = *s++)) {
-   c = tolower(c);
-   *d++ = (c == '_') ? '/' : c;
-   }
-   strcpy(d, ".h");
-
-   /* Assume directory path already exists. */
-   fd = open(path, O_WRONLY | O_CREAT | O_TRUNC, 0644);
-   if (fd == -1) {
-   if (errno != ENOENT) {
-   res = 1;
-   break;
-   }
-
-   if (make_parent_dir(path)) {
-   res = 1;
-   goto out;
-   }
-
-   /* Try it again. */
-   fd = open(path, O_WRONLY | O_CREAT | O_TRUNC, 0644);
-   if (fd == -1) {
-   res = 1;
-   break;
-   }
-   }
-   close(fd);
+   res = conf_touch_dep(sym->name);
+   if (res)
+   return res;
}
-out:
-   if (chdir("../.."))
-   return 1;
 
-   return res;
+   return 0;
 }
 
 int conf_write_autoconf(int overwrite)
-- 
2.7.4



[PATCH 5/5] kconfig: remove k_invalid from expr_parse_string() return type

2018-11-30 Thread Masahiro Yamada
The only possibility of k_invalid being returned was when
expr_parse_sting() parsed S_OTHER type symbol. This actually never
happened, and this is even clearer since S_OTHER has gone.

Clean up unreachable code.

Signed-off-by: Masahiro Yamada 
---

 scripts/kconfig/expr.c | 14 ++
 1 file changed, 2 insertions(+), 12 deletions(-)

diff --git a/scripts/kconfig/expr.c b/scripts/kconfig/expr.c
index e1a39e9..57ebf71 100644
--- a/scripts/kconfig/expr.c
+++ b/scripts/kconfig/expr.c
@@ -980,7 +980,6 @@ enum string_value_kind {
k_string,
k_signed,
k_unsigned,
-   k_invalid
 };
 
 union string_value {
@@ -1011,13 +1010,10 @@ static enum string_value_kind expr_parse_string(const 
char *str,
val->u = strtoull(str, , 16);
kind = k_unsigned;
break;
-   case S_STRING:
-   case S_UNKNOWN:
+   default:
val->s = strtoll(str, , 0);
kind = k_signed;
break;
-   default:
-   return k_invalid;
}
return !errno && !*tail && tail > str && isxdigit(tail[-1])
   ? kind : k_string;
@@ -1073,13 +1069,7 @@ tristate expr_calc_value(struct expr *e)
 
if (k1 == k_string || k2 == k_string)
res = strcmp(str1, str2);
-   else if (k1 == k_invalid || k2 == k_invalid) {
-   if (e->type != E_EQUAL && e->type != E_UNEQUAL) {
-   printf("Cannot compare \"%s\" and \"%s\"\n", str1, 
str2);
-   return no;
-   }
-   res = strcmp(str1, str2);
-   } else if (k1 == k_unsigned || k2 == k_unsigned)
+   else if (k1 == k_unsigned || k2 == k_unsigned)
res = (lval.u > rval.u) - (lval.u < rval.u);
else /* if (k1 == k_signed && k2 == k_signed) */
res = (lval.s > rval.s) - (lval.s < rval.s);
-- 
2.7.4



[PATCH 1/5] kconfig: remove unneeded setsym label in conf_read_simple()

2018-11-30 Thread Masahiro Yamada
The two 'goto setsym' statements are reachable only when sym == NULL.

The code below the 'setsym:' label does nothing when sym == NULL
since there is just one if-block guarded by 'if (sym && ...)'.

Hence, 'goto setsym' can be replaced with 'continue'.

Signed-off-by: Masahiro Yamada 
---

 scripts/kconfig/confdata.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/scripts/kconfig/confdata.c b/scripts/kconfig/confdata.c
index 91d0a5c..1e35529 100644
--- a/scripts/kconfig/confdata.c
+++ b/scripts/kconfig/confdata.c
@@ -363,7 +363,7 @@ int conf_read_simple(const char *name, int def)
sym = sym_find(line + 2 + strlen(CONFIG_));
if (!sym) {
sym_add_change_count(1);
-   goto setsym;
+   continue;
}
} else {
sym = sym_lookup(line + 2 + strlen(CONFIG_), 0);
@@ -397,7 +397,7 @@ int conf_read_simple(const char *name, int def)
sym = sym_find(line + strlen(CONFIG_));
if (!sym) {
sym_add_change_count(1);
-   goto setsym;
+   continue;
}
} else {
sym = sym_lookup(line + strlen(CONFIG_), 0);
@@ -416,7 +416,7 @@ int conf_read_simple(const char *name, int def)
 
continue;
}
-setsym:
+
if (sym && sym_is_choice_value(sym)) {
struct symbol *cs = 
prop_get_symbol(sym_get_choice_prop(sym));
switch (sym->def[def].tri) {
-- 
2.7.4



[PATCH v2 1/2] kbuild: make 'archprepare' depend on 'scripts'

2018-11-28 Thread Masahiro Yamada
Before start descending, Kbuild needs to run 'prepare' and 'scripts',
which has been orthogonal to each other.

Going forward, let's consider 'scripts' is a part of the preparation.
This will allow more cleanups.

Move 'scripts' to the prerequisite of 'archprepare', where UML starts
compiling target *.c files.

Signed-off-by: Masahiro Yamada 
---

Changes in v2:
  - It turned out UML + CONFIG_GCC_PLUGINS is broken for a long time.
https://patchwork.kernel.org/patch/10703853/
Rebased on the top of the fix-up

 Makefile | 32 +++-
 1 file changed, 15 insertions(+), 17 deletions(-)

diff --git a/Makefile b/Makefile
index f7cc3ee..9eb3f4f 100644
--- a/Makefile
+++ b/Makefile
@@ -1062,7 +1062,7 @@ $(sort $(vmlinux-deps)): $(vmlinux-dirs) ;
 # Error messages still appears in the original language
 
 PHONY += $(vmlinux-dirs)
-$(vmlinux-dirs): prepare scripts
+$(vmlinux-dirs): prepare
$(Q)$(MAKE) $(build)=$@ need-builtin=1
 
 define filechk_kernel.release
@@ -1112,7 +1112,7 @@ prepare1: prepare2 $(version_h) $(autoksyms_h) 
include/generated/utsrelease.h
 
 macroprepare: prepare1 archmacros
 
-archprepare: archheaders archscripts macroprepare scripts_basic gcc-plugins
+archprepare: archheaders archscripts macroprepare scripts gcc-plugins
 
 prepare0: archprepare
$(Q)$(MAKE) $(build)=scripts/mod
@@ -1286,7 +1286,7 @@ modules.builtin: $(vmlinux-dirs:%=%/modules.builtin)
 
 # Target to prepare building external modules
 PHONY += modules_prepare
-modules_prepare: prepare scripts
+modules_prepare: prepare
 
 # Target to install modules
 PHONY += modules_install
@@ -1604,11 +1604,9 @@ help:
@echo  '  clean   - remove generated files in module directory 
only'
@echo  ''
 
-# Dummies...
-PHONY += prepare scripts
+PHONY += prepare
 prepare:
$(cmd_crmodverdir)
-scripts: ;
 endif # KBUILD_EXTMOD
 
 clean: $(clean-dirs)
@@ -1712,33 +1710,33 @@ else
 target-dir = $(if $(KBUILD_EXTMOD),$(dir $<),$(dir $@))
 endif
 
-%.s: %.c prepare scripts FORCE
+%.s: %.c prepare FORCE
$(Q)$(MAKE) $(build)=$(build-dir) $(target-dir)$(notdir $@)
-%.i: %.c prepare scripts FORCE
+%.i: %.c prepare FORCE
$(Q)$(MAKE) $(build)=$(build-dir) $(target-dir)$(notdir $@)
-%.o: %.c prepare scripts FORCE
+%.o: %.c prepare FORCE
$(Q)$(MAKE) $(build)=$(build-dir) $(target-dir)$(notdir $@)
-%.lst: %.c prepare scripts FORCE
+%.lst: %.c prepare FORCE
$(Q)$(MAKE) $(build)=$(build-dir) $(target-dir)$(notdir $@)
-%.s: %.S prepare scripts FORCE
+%.s: %.S prepare FORCE
$(Q)$(MAKE) $(build)=$(build-dir) $(target-dir)$(notdir $@)
-%.o: %.S prepare scripts FORCE
+%.o: %.S prepare FORCE
$(Q)$(MAKE) $(build)=$(build-dir) $(target-dir)$(notdir $@)
-%.symtypes: %.c prepare scripts FORCE
+%.symtypes: %.c prepare FORCE
$(Q)$(MAKE) $(build)=$(build-dir) $(target-dir)$(notdir $@)
-%.ll: %.c prepare scripts FORCE
+%.ll: %.c prepare FORCE
$(Q)$(MAKE) $(build)=$(build-dir) $(target-dir)$(notdir $@)
 
 # Modules
-/: prepare scripts FORCE
+/: prepare FORCE
$(Q)$(MAKE) KBUILD_MODULES=$(if $(CONFIG_MODULES),1) \
$(build)=$(build-dir)
 # Make sure the latest headers are built for Documentation
 Documentation/ samples/: headers_install
-%/: prepare scripts FORCE
+%/: prepare FORCE
$(Q)$(MAKE) KBUILD_MODULES=$(if $(CONFIG_MODULES),1) \
$(build)=$(build-dir)
-%.ko: prepare scripts FORCE
+%.ko: prepare FORCE
$(Q)$(MAKE) KBUILD_MODULES=$(if $(CONFIG_MODULES),1)   \
$(build)=$(build-dir) $(@:.ko=.o)
$(Q)$(MAKE) -f $(srctree)/scripts/Makefile.modpost
-- 
2.7.4



[PATCH v2 2/2] kbuild: descend into scripts/gcc-plugins/ via scripts/Makefile

2018-11-28 Thread Masahiro Yamada
Now that 'archprepare' depends on 'scripts', Kbuild can descend into
scripts/gcc-plugins in a more standard way.

Signed-off-by: Masahiro Yamada 
Reviewed-by: Kees Cook 
---

Changes in v2:
  - It turned out UML + CONFIG_GCC_PLUGINS is broken for a long time.
https://patchwork.kernel.org/patch/10703853/
Rebased on the top of the fix-up

 Makefile | 2 +-
 scripts/Makefile | 3 ++-
 scripts/Makefile.gcc-plugins | 8 
 3 files changed, 3 insertions(+), 10 deletions(-)

diff --git a/Makefile b/Makefile
index 9eb3f4f..c1f87cf 100644
--- a/Makefile
+++ b/Makefile
@@ -1112,7 +1112,7 @@ prepare1: prepare2 $(version_h) $(autoksyms_h) 
include/generated/utsrelease.h
 
 macroprepare: prepare1 archmacros
 
-archprepare: archheaders archscripts macroprepare scripts gcc-plugins
+archprepare: archheaders archscripts macroprepare scripts
 
 prepare0: archprepare
$(Q)$(MAKE) $(build)=scripts/mod
diff --git a/scripts/Makefile b/scripts/Makefile
index b48259d..feb1f71 100644
--- a/scripts/Makefile
+++ b/scripts/Makefile
@@ -36,9 +36,10 @@ PHONY += build_unifdef
 build_unifdef: $(obj)/unifdef
@:
 
+subdir-$(CONFIG_GCC_PLUGINS) += gcc-plugins
 subdir-$(CONFIG_MODVERSIONS) += genksyms
 subdir-$(CONFIG_SECURITY_SELINUX) += selinux
 subdir-$(CONFIG_GDB_SCRIPTS) += gdb
 
 # Let clean descend into subdirs
-subdir-+= basic dtc kconfig mod package gcc-plugins
+subdir-+= basic dtc kconfig mod package
diff --git a/scripts/Makefile.gcc-plugins b/scripts/Makefile.gcc-plugins
index 46c5c68..c36f199 100644
--- a/scripts/Makefile.gcc-plugins
+++ b/scripts/Makefile.gcc-plugins
@@ -49,11 +49,3 @@ KBUILD_CFLAGS += $(GCC_PLUGINS_CFLAGS)
 # All enabled GCC plugins are collected here for building below.
 GCC_PLUGIN := $(gcc-plugin-y)
 export GCC_PLUGIN
-
-# Actually do the build, if requested.
-PHONY += gcc-plugins
-gcc-plugins: scripts_basic
-ifdef CONFIG_GCC_PLUGINS
-   $(Q)$(MAKE) $(build)=scripts/gcc-plugins
-endif
-   @:
-- 
2.7.4



[PATCH] kbuild: fix UML build error with CONFIG_GCC_PLUGINS

2018-11-28 Thread Masahiro Yamada
UML fails to build with CONFIG_GCC_PLUGINS=y.

$ make -s ARCH=um mrproper
$ make -s ARCH=um allmodconfig
$ make ARCH=um
  UPD include/generated/uapi/linux/version.h
  WRAParch/x86/include/generated/uapi/asm/bpf_perf_event.h
  WRAParch/x86/include/generated/uapi/asm/poll.h
  WRAParch/x86/include/generated/asm/dma-contiguous.h
  WRAParch/x86/include/generated/asm/early_ioremap.h
  WRAParch/x86/include/generated/asm/export.h
  WRAParch/x86/include/generated/asm/mcs_spinlock.h
  WRAParch/x86/include/generated/asm/mm-arch-hooks.h
  SYSTBL  arch/x86/include/generated/asm/syscalls_32.h
  SYSHDR  arch/x86/include/generated/asm/unistd_32_ia32.h
  SYSHDR  arch/x86/include/generated/asm/unistd_64_x32.h
  SYSTBL  arch/x86/include/generated/asm/syscalls_64.h
  SYSHDR  arch/x86/include/generated/uapi/asm/unistd_32.h
  SYSHDR  arch/x86/include/generated/uapi/asm/unistd_64.h
  SYSHDR  arch/x86/include/generated/uapi/asm/unistd_x32.h
  HOSTCC  scripts/unifdef
  CC  arch/x86/um/user-offsets.s
cc1: error: cannot load plugin ./scripts/gcc-plugins/cyc_complexity_plugin.so
./scripts/gcc-plugins/cyc_complexity_plugin.so: cannot open shared object file: 
No such file or directory
cc1: error: cannot load plugin ./scripts/gcc-plugins/structleak_plugin.so
./scripts/gcc-plugins/structleak_plugin.so: cannot open shared object file: No 
such file or directory
cc1: error: cannot load plugin ./scripts/gcc-plugins/latent_entropy_plugin.so
./scripts/gcc-plugins/latent_entropy_plugin.so: cannot open shared object file: 
No such file or directory
cc1: error: cannot load plugin ./scripts/gcc-plugins/randomize_layout_plugin.so
./scripts/gcc-plugins/randomize_layout_plugin.so: cannot open shared object 
file: No such file or directory
make[1]: *** [scripts/Makefile.build;119: arch/x86/um/user-offsets.s] Error 1
make: *** [arch/um/Makefile;152: arch/x86/um/user-offsets.s] Error 2

Change the order of the preparation stage (with cleanups) to make sure
gcc-plugins is built before descending to arch/x86/um/.

Reported-by: kbuild test robot 
Fixes: 6b90bd4ba40b ("GCC plugin infrastructure")
Signed-off-by: Masahiro Yamada 
---

I will pick up this to kbuild tree to avoid conflicts.
Ack from UML people is appreciated, though.


 Makefile |  4 ++--
 arch/um/Makefile | 24 ++--
 arch/x86/um/Makefile |  4 +++-
 3 files changed, 7 insertions(+), 25 deletions(-)

diff --git a/Makefile b/Makefile
index 0ce4e29..a066dab 100644
--- a/Makefile
+++ b/Makefile
@@ -1106,9 +1106,9 @@ prepare1: prepare2 $(version_h) $(autoksyms_h) 
include/generated/utsrelease.h
 
 macroprepare: prepare1 archmacros
 
-archprepare: archheaders archscripts macroprepare scripts_basic
+archprepare: archheaders archscripts macroprepare scripts_basic gcc-plugins
 
-prepare0: archprepare gcc-plugins
+prepare0: archprepare
$(Q)$(MAKE) $(build)=.
 
 # All the preparing..
diff --git a/arch/um/Makefile b/arch/um/Makefile
index ab1066c..c080359 100644
--- a/arch/um/Makefile
+++ b/arch/um/Makefile
@@ -116,7 +116,8 @@ endef
 archheaders:
$(Q)$(MAKE) -f $(srctree)/Makefile ARCH=$(HEADER_ARCH) asm-generic 
archheaders
 
-archprepare: include/generated/user_constants.h
+archprepare:
+   $(Q)$(MAKE) $(build)=$(HOST_DIR)/um include/generated/user_constants.h
 
 LINK-$(CONFIG_LD_SCRIPT_STATIC) += -static
 LINK-$(CONFIG_LD_SCRIPT_DYN) += -Wl,-rpath,/lib $(call cc-option, -no-pie)
@@ -146,25 +147,4 @@ archclean:
@find . \( -name '*.bb' -o -name '*.bbg' -o -name '*.da' \
-o -name '*.gcov' \) -type f -print | xargs rm -f
 
-# Generated files
-
-$(HOST_DIR)/um/user-offsets.s: __headers FORCE
-   $(Q)$(MAKE) $(build)=$(HOST_DIR)/um $@
-
-define filechk_gen-asm-offsets
-(set -e; \
- echo "/*"; \
- echo " * DO NOT MODIFY."; \
- echo " *"; \
- echo " * This file was generated by arch/$(ARCH)/Makefile"; \
- echo " *"; \
- echo " */"; \
- echo ""; \
- sed -ne "/^->/{s:^->\([^ ]*\) [\$$#]*\([^ ]*\) \(.*\):#define \1 \2 
/* \3 */:; s:->::; p;}" < $<; \
- echo ""; )
-endef
-
-include/generated/user_constants.h: $(HOST_DIR)/um/user-offsets.s
-   $(call filechk,gen-asm-offsets)
-
 export HEADER_ARCH SUBARCH USER_CFLAGS CFLAGS_NO_HARDENING OS DEV_NULL_PATH
diff --git a/arch/x86/um/Makefile b/arch/x86/um/Makefile
index c2d3d7c..1792464 100644
--- a/arch/x86/um/Makefile
+++ b/arch/x86/um/Makefile
@@ -36,10 +36,12 @@ subarch-$(CONFIG_MODULES) += ../kernel/module.o
 
 USER_OBJS := bugs_$(BITS).o ptrace_user.o fault.o
 
-extra-y += user-offsets.s
 $(obj)/user-offsets.s: c_flags = -Wp,-MD,$(depfile) $(USER_CFLAGS) \
-Iarch/x86/include/generated
 
+include/generated/user_constants.h: $(obj)/user-offsets.s
+   $(call filechk,offsets,__USER_CONSTANT_H__)
+
 UNPROFILE_OBJS := stub_segv.o
 CFLAGS_stub_segv.o := $(CFLAGS_NO_HARDENING)
 
-- 
2.7.4



[PATCH v2 1/2] mtd: rawnand: denali: remove ->dev_ready() hook

2018-11-27 Thread Masahiro Yamada
The Denali NAND IP has no way to read out the current signal level
of the R/B# pin. Instead, denali_dev_ready() checks if the R/B#
transition has already happened. (The INTR__INT_ACT interrupt is
asserted at the rising edge of the R/B# pin.) It is not a correct
way to implement the ->dev_ready() hook.

In fact, it has a drawback; in the nand_scan_ident phase, the chip
detection iterates over maxchips until it fails to find a homogeneous
chip. For the last loop, nand_reset() fails if no chip is there.

If ->dev_ready hook exists, nand_command(_lp) calls nand_wait_ready()
after NAND_CMD_RESET. However, we know denali_dev_ready() never
returns 1 unless there exists a chip that toggles R/B# in that chip
select. Then, nand_wait_ready() just ends up with wasting 400 msec,
in the end, shows the "timeout while waiting for chip to become ready"
warning.

Let's remove the mis-implemented dev_ready hook, and fallback to
sending the NAND_CMD_STATUS and nand_wait_status_ready(), which
bails out more quickly.

Signed-off-by: Masahiro Yamada 
---

Changes in v2:
  - Rebase

 drivers/mtd/nand/raw/denali.c | 23 +--
 1 file changed, 1 insertion(+), 22 deletions(-)

diff --git a/drivers/mtd/nand/raw/denali.c b/drivers/mtd/nand/raw/denali.c
index 830ea24..ead6e60 100644
--- a/drivers/mtd/nand/raw/denali.c
+++ b/drivers/mtd/nand/raw/denali.c
@@ -204,18 +204,6 @@ static uint32_t denali_wait_for_irq(struct 
denali_nand_info *denali,
return denali->irq_status;
 }
 
-static uint32_t denali_check_irq(struct denali_nand_info *denali)
-{
-   unsigned long flags;
-   uint32_t irq_status;
-
-   spin_lock_irqsave(>irq_lock, flags);
-   irq_status = denali->irq_status;
-   spin_unlock_irqrestore(>irq_lock, flags);
-
-   return irq_status;
-}
-
 static void denali_read_buf(struct nand_chip *chip, uint8_t *buf, int len)
 {
struct mtd_info *mtd = nand_to_mtd(chip);
@@ -288,8 +276,7 @@ static void denali_cmd_ctrl(struct nand_chip *chip, int 
dat, unsigned int ctrl)
return;
 
/*
-* Some commands are followed by chip->legacy.dev_ready or
-* chip->legacy.waitfunc.
+* Some commands are followed by chip->legacy.waitfunc.
 * irq_status must be cleared here to catch the R/B# interrupt later.
 */
if (ctrl & NAND_CTRL_CHANGE)
@@ -298,13 +285,6 @@ static void denali_cmd_ctrl(struct nand_chip *chip, int 
dat, unsigned int ctrl)
denali->host_write(denali, DENALI_BANK(denali) | type, dat);
 }
 
-static int denali_dev_ready(struct nand_chip *chip)
-{
-   struct denali_nand_info *denali = mtd_to_denali(nand_to_mtd(chip));
-
-   return !!(denali_check_irq(denali) & INTR__INT_ACT);
-}
-
 static int denali_check_erased_page(struct mtd_info *mtd,
struct nand_chip *chip, uint8_t *buf,
unsigned long uncor_ecc_flags,
@@ -1359,7 +1339,6 @@ int denali_init(struct denali_nand_info *denali)
chip->legacy.read_byte = denali_read_byte;
chip->legacy.write_byte = denali_write_byte;
chip->legacy.cmd_ctrl = denali_cmd_ctrl;
-   chip->legacy.dev_ready = denali_dev_ready;
chip->legacy.waitfunc = denali_waitfunc;
 
if (features & FEATURES__INDEX_ADDR) {
-- 
2.7.4



[PATCH v2 0/2] mtd: rawnand: denali: clean-up unnecessary hook and device reset

2018-11-27 Thread Masahiro Yamada
I sent this series on September,
and Miquel replied this series was applied:
http://patchwork.ozlabs.org/patch/967242/

But, It turned out not applied.

I rebased it and resending now.


Masahiro Yamada (2):
  mtd: rawnand: denali: remove ->dev_ready() hook
  mtd: rawnand: denali: remove denali_reset_banks()

 drivers/mtd/nand/raw/denali.c | 52 +--
 1 file changed, 1 insertion(+), 51 deletions(-)

-- 
2.7.4



[PATCH v2 2/2] mtd: rawnand: denali: remove denali_reset_banks()

2018-11-27 Thread Masahiro Yamada
In nand_scan_ident(), the controller driver resets every NAND chip.
This is done by sending NAND_CMD_RESET. The Denali IP provides
another way to do the equivalent thing; if a bit is set in the
DEVICE_RESET register, the controller sends the RESET command to
the corresponding device. denali_reset_banks() uses it to reset
all devices beforehand.

This redundant reset sequence was needed to know the actual number
of chips before calling nand_scan_ident(); if DEVICE_RESET fails,
there is no chip in that chip select. Then, denali_reset_banks()
sets denali->max_banks to the number of detected chips.

As commit f486287d2372 ("mtd: nand: denali: fix bank reset function
to detect the number of chips") explained, nand_scan_ident() issued
Set Features (0xEF) command to all CS lines, some of which may not be
connected with a chip. Then, the driver would wait for R/B# response,
which never happens.

This problem was solved by commit 107b7d6a7ad4 ("mtd: rawnand: avoid
setting again the timings to mode 0 after a reset"). In the current
code, nand_setup_data_interface() is called from nand_scan_tail(),
which is invoked after the chip detection.

Now, we can really remove the redundant denali_nand_banks() by simply
passing the maximum number of chip selects supported by this IP
(typically 4 or 8) to nand_scan(). Let's leave all the chip detection
process to nand_scan_ident().

Signed-off-by: Masahiro Yamada 
---

Changes in v2: None

 drivers/mtd/nand/raw/denali.c | 29 -
 1 file changed, 29 deletions(-)

diff --git a/drivers/mtd/nand/raw/denali.c b/drivers/mtd/nand/raw/denali.c
index ead6e60..2302010 100644
--- a/drivers/mtd/nand/raw/denali.c
+++ b/drivers/mtd/nand/raw/denali.c
@@ -1045,29 +1045,6 @@ static int denali_setup_data_interface(struct nand_chip 
*chip, int chipnr,
return 0;
 }
 
-static void denali_reset_banks(struct denali_nand_info *denali)
-{
-   u32 irq_status;
-   int i;
-
-   for (i = 0; i < denali->max_banks; i++) {
-   denali->active_bank = i;
-
-   denali_reset_irq(denali);
-
-   iowrite32(DEVICE_RESET__BANK(i),
- denali->reg + DEVICE_RESET);
-
-   irq_status = denali_wait_for_irq(denali,
-   INTR__RST_COMP | INTR__INT_ACT | INTR__TIME_OUT);
-   if (!(irq_status & INTR__INT_ACT))
-   break;
-   }
-
-   dev_dbg(denali->dev, "%d chips connected\n", i);
-   denali->max_banks = i;
-}
-
 static void denali_hw_init(struct denali_nand_info *denali)
 {
/*
@@ -1321,12 +1298,6 @@ int denali_init(struct denali_nand_info *denali)
}
 
denali_enable_irq(denali);
-   denali_reset_banks(denali);
-   if (!denali->max_banks) {
-   /* Error out earlier if no chip is found for some reasons. */
-   ret = -ENODEV;
-   goto disable_irq;
-   }
 
denali->active_bank = DENALI_INVALID_BANK;
 
-- 
2.7.4



[PATCH] ARM: multi_v7_defconfig: enable CONFIG_UNIPHIER_MDMAC

2018-11-27 Thread Masahiro Yamada
Enable the UniPhier MIO DMAC driver. This is used as the DMA engine
for accelerating the SD/eMMC controller drivers.

Signed-off-by: Masahiro Yamada 
---

The insertion context was decided by savedefconfig on next-20181127.


 arch/arm/configs/multi_v7_defconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/configs/multi_v7_defconfig 
b/arch/arm/configs/multi_v7_defconfig
index 63af623..715da88 100644
--- a/arch/arm/configs/multi_v7_defconfig
+++ b/arch/arm/configs/multi_v7_defconfig
@@ -862,6 +862,7 @@ CONFIG_STM32_DMA=y
 CONFIG_STM32_DMAMUX=y
 CONFIG_STM32_MDMA=y
 CONFIG_TEGRA20_APB_DMA=y
+CONFIG_UNIPHIER_MDMAC=y
 CONFIG_XILINX_DMA=y
 CONFIG_QCOM_BAM_DMA=y
 CONFIG_DW_DMAC=y
-- 
2.7.4



[PATCH] ARM: dts: uniphier: add MIO DMAC nodes

2018-11-27 Thread Masahiro Yamada
Add MIO-DMAC (Media IO DMA Controller) nodes, and use them as
the DMA engine of SD/eMMC controllers.

Signed-off-by: Masahiro Yamada 
---

 arch/arm/boot/dts/uniphier-ld4.dtsi  | 14 ++
 arch/arm/boot/dts/uniphier-pro4.dtsi | 16 
 arch/arm/boot/dts/uniphier-sld8.dtsi | 14 ++
 3 files changed, 44 insertions(+)

diff --git a/arch/arm/boot/dts/uniphier-ld4.dtsi 
b/arch/arm/boot/dts/uniphier-ld4.dtsi
index b73d594..c2706ce 100644
--- a/arch/arm/boot/dts/uniphier-ld4.dtsi
+++ b/arch/arm/boot/dts/uniphier-ld4.dtsi
@@ -235,6 +235,16 @@
};
};
 
+   dmac: dma-controller@5a00 {
+   compatible = "socionext,uniphier-mio-dmac";
+   reg = <0x5a00 0x1000>;
+   interrupts = <0 68 4>, <0 68 4>, <0 69 4>, <0 70 4>,
+<0 71 4>, <0 72 4>, <0 73 4>;
+   clocks = <_clk 7>;
+   resets = <_rst 7>;
+   #dma-cells = <1>;
+   };
+
sd: sdhc@5a40 {
compatible = "socionext,uniphier-sd-v2.91";
status = "disabled";
@@ -246,6 +256,8 @@
clocks = <_clk 0>;
reset-names = "host", "bridge";
resets = <_rst 0>, <_rst 3>;
+   dma-names = "rx-tx";
+   dmas = < 4>;
bus-width = <4>;
cap-sd-highspeed;
sd-uhs-sdr12;
@@ -263,6 +275,8 @@
clocks = <_clk 1>;
reset-names = "host", "bridge", "hw";
resets = <_rst 1>, <_rst 4>, <_rst 6>;
+   dma-names = "rx-tx";
+   dmas = < 6>;
bus-width = <8>;
cap-mmc-highspeed;
cap-mmc-hw-reset;
diff --git a/arch/arm/boot/dts/uniphier-pro4.dtsi 
b/arch/arm/boot/dts/uniphier-pro4.dtsi
index 0beb606..97d051e 100644
--- a/arch/arm/boot/dts/uniphier-pro4.dtsi
+++ b/arch/arm/boot/dts/uniphier-pro4.dtsi
@@ -269,6 +269,16 @@
};
};
 
+   dmac: dma-controller@5a00 {
+   compatible = "socionext,uniphier-mio-dmac";
+   reg = <0x5a00 0x1000>;
+   interrupts = <0 68 4>, <0 68 4>, <0 69 4>, <0 70 4>,
+<0 71 4>, <0 72 4>, <0 73 4>, <0 74 4>;
+   clocks = <_clk 7>;
+   resets = <_rst 7>;
+   #dma-cells = <1>;
+   };
+
sd: sdhc@5a40 {
compatible = "socionext,uniphier-sd-v2.91";
status = "disabled";
@@ -280,6 +290,8 @@
clocks = <_clk 0>;
reset-names = "host", "bridge";
resets = <_rst 0>, <_rst 3>;
+   dma-names = "rx-tx";
+   dmas = < 4>;
bus-width = <4>;
cap-sd-highspeed;
sd-uhs-sdr12;
@@ -297,6 +309,8 @@
clocks = <_clk 1>;
reset-names = "host", "bridge", "hw";
resets = <_rst 1>, <_rst 4>, <_rst 6>;
+   dma-names = "rx-tx";
+   dmas = < 5>;
bus-width = <8>;
cap-mmc-highspeed;
cap-mmc-hw-reset;
@@ -313,6 +327,8 @@
clocks = <_clk 2>;
reset-names = "host", "bridge";
resets = <_rst 2>, <_rst 5>;
+   dma-names = "rx-tx";
+   dmas = < 6>;
bus-width = <4>;
cap-sd-highspeed;
};
diff --git a/arch/arm/boot/dts/uniphier-sld8.dtsi 
b/arch/arm/boot/dts/uniphier-sld8.dtsi
index f7fcf6b..efce027 100644
--- a/arch/arm/boot/dts/uniphier-sld8.dtsi
+++ b/arch/arm/boot/dts/uniphier-sld8.dtsi
@@ -239,6 +239,16 @@
};
};
 
+   dmac: dma-controller@5a00 {
+   compatible = "socionext,uniphier-mio-dmac";
+

Re: [PATCH v4 0/2] dmaengine: add UniPhier MIO DMAC driver

2018-11-27 Thread Masahiro Yamada
On Wed, Nov 28, 2018 at 12:01 AM Vinod Koul  wrote:
>
> On 25-11-18, 22:53, Masahiro Yamada wrote:
> > On Sat, Nov 24, 2018 at 11:16 PM Vinod Koul  wrote:
> > >
> > > On 12-10-18, 01:41, Masahiro Yamada wrote:
> > > > 1/2: DT-binding
> > > > 2/2: driver
> > >
> > > Applied this series, thanks
> >
> >
> > Thanks.
> >
> >
> > > While building I noticed that we get few warns when compiling with
> > > C=1, I would prefer you fix them. Please send fixes on top of the
> > > applied patches.
> >
> > My code is fine.
>
> Lets see..
>
> > If you are talking about the following, your sparse is too old.
> > Upgrade your sparse.
>
> Not really!!
>
> $ sparse --version
> 0.5.2
>
> .. is the last released version!
>
> Do you use from git?


Yes.

My version is v0.5.2-853-g06a4545


I only see './include/linux/slab.h:332:43: warning: dubious: x & !y'
in linux-next.


The warning in slab.h is already on the way in mm subsystem.



masahiro@pug:~/ref/linux-next$ git log --oneline -1
9d52999 Add linux-next specific files for 20181127
masahiro@pug:~/ref/linux-next$ make ARCH=arm64
CROSS_COMPILE=aarch64-linux-gnu- C=2 defconfig
drivers/dma/uniphier-mdmac.o
*** Default configuration is based on 'defconfig'
#
# configuration written to .config
#
scripts/kconfig/conf  --syncconfig Kconfig
  CHECK   scripts/mod/empty.c
  CC  scripts/mod/empty.o
  MKELF   scripts/mod/elfconfig.h
  HOSTCC  scripts/mod/modpost.o
  CC  scripts/mod/devicetable-offsets.s
  HOSTCC  scripts/mod/file2alias.o
  HOSTCC  scripts/mod/sumversion.o
  HOSTLD  scripts/mod/modpost
  CC  kernel/bounds.s
  UPD include/generated/bounds.h
  UPD include/generated/timeconst.h
  CC  arch/arm64/kernel/asm-offsets.s
  UPD include/generated/asm-offsets.h
  CALLscripts/checksyscalls.sh
:1338:2: warning: #warning syscall open_tree not implemented [-Wcpp]
:1341:2: warning: #warning syscall move_mount not implemented [-Wcpp]
:1344:2: warning: #warning syscall fsopen not implemented [-Wcpp]
:1347:2: warning: #warning syscall fsconfig not implemented [-Wcpp]
:1350:2: warning: #warning syscall fsmount not implemented [-Wcpp]
:1353:2: warning: #warning syscall fspick not implemented [-Wcpp]
:1356:2: warning: #warning syscall fsinfo not implemented [-Wcpp]
  CALLscripts/atomic/check-atomics.sh
  LDS arch/arm64/kernel/vdso/vdso.lds
  VDSOA   arch/arm64/kernel/vdso/gettimeofday.o
  VDSOA   arch/arm64/kernel/vdso/sigreturn.o
  VDSOL   arch/arm64/kernel/vdso/vdso.so.dbg
  VDSOSYM include/generated/vdso-offsets.h
  CHECK   drivers/dma/uniphier-mdmac.c
./include/linux/slab.h:332:43: warning: dubious: x & !y
  CC  drivers/dma/uniphier-mdmac.o




> > ./include/linux/overflow.h:285:13: error: undefined identifier
> > '__builtin_mul_overflow'
> > ./include/linux/overflow.h:285:13: error: incorrect type in conditional
> > ./include/linux/overflow.h:285:13:got void
> > ./include/linux/overflow.h:287:13: error: undefined identifier
> > '__builtin_add_overflow'
> > ./include/linux/overflow.h:287:13: error: incorrect type in conditional
> > ./include/linux/overflow.h:287:13:got void
> > ./include/linux/overflow.h:285:13: warning: call with no type!
> > ./include/linux/overflow.h:287:13: warning: call with no type!
>
> and I was also talking about:
>
> arch/arm64/include/asm/sysreg.h:471:42: warning: constant 0x 
> is so big it is unsigned long
> arch/arm64/include/asm/sysreg.h:512:42: warning: constant 0x 
> is so big it is unsigned long
> include/linux/slab.h:332:43: warning: dubious: x & !y
>
> --
> ~Vinod



-- 
Best Regards
Masahiro Yamada


Re: [PATCH v3 1/2] dt-bindings: uniphier: add bindings for UniPhier SoC family

2018-11-26 Thread Masahiro Yamada
On Fri, Oct 19, 2018 at 7:32 PM Masahiro Yamada
 wrote:
>
> Document the list of SoCs and boards of UniPhier platform.
>
> Signed-off-by: Masahiro Yamada 
> Reviewed-by: Rob Herring 
> ---

Applied to linux-uniphier.


> Changes in v3:
>   - Capitalize 'Board' consistently
>   - Add Rob's Reviewed-by
>
> Changes in v2:
>   Changes suggested by Rob.
>   - Move the file to
> Documentation/devicetree/bindings/arm/socionext/uniphier.txt
>   - Group boards by each SoC
>   Previous post:
>   v1: https://patchwork.ozlabs.org/patch/842010/
>
>  .../devicetree/bindings/arm/socionext/uniphier.txt | 47 
> ++
>  MAINTAINERS|  1 +
>  2 files changed, 48 insertions(+)
>  create mode 100644 
> Documentation/devicetree/bindings/arm/socionext/uniphier.txt
>
> diff --git a/Documentation/devicetree/bindings/arm/socionext/uniphier.txt 
> b/Documentation/devicetree/bindings/arm/socionext/uniphier.txt
> new file mode 100644
> index 000..b3ed103
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/arm/socionext/uniphier.txt
> @@ -0,0 +1,47 @@
> +Socionext UniPhier SoC family
> +-
> +
> +Required properties in the root node:
> +  - compatible: should contain board and SoC compatible strings
> +
> +SoC and board compatible strings:
> +  (sorted chronologically)
> +
> +  - LD4 SoC:  "socionext,uniphier-ld4"
> +  - Reference Board: "socionext,uniphier-ld4-ref"
> +
> +  - Pro4 SoC: "socionext,uniphier-pro4"
> +  - Reference Board: "socionext,uniphier-pro4-ref"
> +  - Ace Board:   "socionext,uniphier-pro4-ace"
> +  - Sanji Board: "socionext,uniphier-pro4-sanji"
> +
> +  - sLD8 SoC: "socionext,uniphier-sld8"
> +  - Reference Board: "socionext,uniphier-sld8-ref"
> +
> +  - PXs2 SoC: "socionext,uniphier-pxs2"
> +  - Gentil Board:"socionext,uniphier-pxs2-gentil"
> +  - Vodka Board: "socionext,uniphier-pxs2-vodka"
> +
> +  - LD6b SoC: "socionext,uniphier-ld6b"
> +  - Reference Board: "socionext,uniphier-ld6b-ref"
> +
> +  - LD11 SoC: "socionext,uniphier-ld11"
> +  - Reference Board: "socionext,uniphier-ld11-ref"
> +  - Global Board:"socionext,uniphier-ld11-global"
> +
> +  - LD20 SoC: "socionext,uniphier-ld20"
> +  - Reference Board: "socionext,uniphier-ld20-ref"
> +  - Global Board:"socionext,uniphier-ld20-global"
> +
> +  - PXs3 SoC: "socionext,uniphier-pxs3"
> +  - Reference Board: "socionext,uniphier-pxs3-ref"
> +
> +Example:
> +
> +/dts-v1/;
> +
> +/ {
> +   compatible = "socionext,uniphier-ld20-ref", "socionext,uniphier-ld20";
> +
> +   ...
> +};
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 5e6a1da..87b2c2c 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -2183,6 +2183,7 @@ M:Masahiro Yamada 
> 
>  L:     linux-arm-ker...@lists.infradead.org (moderated for non-subscribers)
>  T: git 
> git://git.kernel.org/pub/scm/linux/kernel/git/masahiroy/linux-uniphier.git
>  S: Maintained
> +F: Documentation/devicetree/bindings/arm/socionext/uniphier.txt
>  F: Documentation/devicetree/bindings/gpio/gpio-uniphier.txt
>  F: 
> Documentation/devicetree/bindings/pinctrl/socionext,uniphier-pinctrl.txt
>  F: arch/arm/boot/dts/uniphier*
> --
> 2.7.4
>


-- 
Best Regards
Masahiro Yamada


Re: [PATCH v3 2/2] dt-bindings: uniphier: move cache-uniphier.txt to vendor directory

2018-11-26 Thread Masahiro Yamada
On Fri, Oct 19, 2018 at 7:33 PM Masahiro Yamada
 wrote:
>
> Now, the Socionext vendor directory is available at
> Documentation/devicetree/bindings/arm/socionext/
>
> Move cache-uniphier.txt over to it.
>
> Signed-off-by: Masahiro Yamada 
> Reviewed-by: Rob Herring 
> ---

Applied to linux-uniphier.



> Changes in v3: None
> Changes in v2:
>   - New patch
>
>  .../devicetree/bindings/arm/{uniphier => socionext}/cache-uniphier.txt| 0
>  1 file changed, 0 insertions(+), 0 deletions(-)
>  rename Documentation/devicetree/bindings/arm/{uniphier => 
> socionext}/cache-uniphier.txt (100%)
>
> diff --git 
> a/Documentation/devicetree/bindings/arm/uniphier/cache-uniphier.txt 
> b/Documentation/devicetree/bindings/arm/socionext/cache-uniphier.txt
> similarity index 100%
> rename from Documentation/devicetree/bindings/arm/uniphier/cache-uniphier.txt
> rename to Documentation/devicetree/bindings/arm/socionext/cache-uniphier.txt
> --
> 2.7.4
>


-- 
Best Regards
Masahiro Yamada


Re: [PATCH 6/6] ARM: dts: uniphier: Add all CPUs in cooling maps

2018-11-26 Thread Masahiro Yamada
On Fri, Nov 16, 2018 at 7:02 PM Viresh Kumar  wrote:
>
> Each CPU can (and does) participate in cooling down the system but the
> DT only captures a handful of them, normally CPU0, in the cooling maps.
> Things work by chance currently as under normal circumstances its the
> first CPU of each cluster which is used by the operating systems to
> probe the cooling devices. But as soon as this CPU ordering changes and
> any other CPU is used to bring up the cooling device, we will start
> seeing failures.
>
> Also the DT is rather incomplete when we list only one CPU in the
> cooling maps, as the hardware doesn't have any such limitations.
>
> Update cooling maps to include all devices affected by individual trip
> points.
>
> Signed-off-by: Viresh Kumar 


Applied to linux-uniphier.
Thanks.

> ---
>  arch/arm/boot/dts/uniphier-pxs2.dtsi | 6 --
>  1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm/boot/dts/uniphier-pxs2.dtsi 
> b/arch/arm/boot/dts/uniphier-pxs2.dtsi
> index 8d20e9548e39..06a049f6edf8 100644
> --- a/arch/arm/boot/dts/uniphier-pxs2.dtsi
> +++ b/arch/arm/boot/dts/uniphier-pxs2.dtsi
> @@ -141,8 +141,10 @@
> cooling-maps {
> map {
> trip = <_alert>;
> -   cooling-device = <
> -   THERMAL_NO_LIMIT 
> THERMAL_NO_LIMIT>;
> +   cooling-device = < 
> THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
> +< 
> THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
> +< 
> THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
> +< 
> THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;
> };
> };
> };
> --
> 2.19.1.568.g152ad8e3369a
>


-- 
Best Regards
Masahiro Yamada


Re: [PATCH 10/10] ARM64: dts: uniphier: Add all CPUs in cooling maps

2018-11-26 Thread Masahiro Yamada
On Fri, Nov 16, 2018 at 7:06 PM Viresh Kumar  wrote:
>
> Each CPU can (and does) participate in cooling down the system but the
> DT only captures a handful of them, normally CPU0, in the cooling maps.
> Things work by chance currently as under normal circumstances its the
> first CPU of each cluster which is used by the operating systems to
> probe the cooling devices. But as soon as this CPU ordering changes and
> any other CPU is used to bring up the cooling device, we will start
> seeing failures.
>
> Also the DT is rather incomplete when we list only one CPU in the
> cooling maps, as the hardware doesn't have any such limitations.
>
> Update cooling maps to include all devices affected by individual trip
> points.
>
> Signed-off-by: Viresh Kumar 


Applied to linux-uniphier.
Thanks.




> ---
>  arch/arm64/boot/dts/socionext/uniphier-ld20.dtsi | 11 ---
>  1 file changed, 4 insertions(+), 7 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/socionext/uniphier-ld20.dtsi 
> b/arch/arm64/boot/dts/socionext/uniphier-ld20.dtsi
> index d7e2d8969601..4a0c46cb11cd 100644
> --- a/arch/arm64/boot/dts/socionext/uniphier-ld20.dtsi
> +++ b/arch/arm64/boot/dts/socionext/uniphier-ld20.dtsi
> @@ -206,13 +206,10 @@
> cooling-maps {
> map0 {
> trip = <_alert>;
> -   cooling-device = <
> -   THERMAL_NO_LIMIT 
> THERMAL_NO_LIMIT>;
> -   };
> -   map1 {
> -   trip = <_alert>;
> -   cooling-device = <
> -   THERMAL_NO_LIMIT 
> THERMAL_NO_LIMIT>;
> +   cooling-device = < 
> THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
> +< 
> THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
> +< 
> THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
> +< 
> THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;
> };
> };
> };
> --
> 2.19.1.568.g152ad8e3369a
>


-- 
Best Regards
Masahiro Yamada


Re: [PATCH 2/2] kbuild: descend into scripts/gcc-plugins/ via scripts/Makefile

2018-11-26 Thread Masahiro Yamada
On Sat, Nov 24, 2018 at 4:57 PM Masahiro Yamada
 wrote:
>
> Now that 'prepare0' depends on 'scripts', building GCC plugins can
> go into scripts/Makefile, which is a more standard way.
>
> Signed-off-by: Masahiro Yamada 
> ---


Applied to linux-kbuild.


>  Makefile | 2 +-
>  scripts/Makefile | 3 ++-
>  scripts/Makefile.gcc-plugins | 8 
>  3 files changed, 3 insertions(+), 10 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index cee4cec..a8bbe68 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1114,7 +1114,7 @@ macroprepare: prepare1 archmacros
>
>  archprepare: archheaders archscripts macroprepare scripts_basic
>
> -prepare0: scripts archprepare gcc-plugins
> +prepare0: scripts archprepare
> $(Q)$(MAKE) $(build)=scripts/mod
> $(Q)$(MAKE) $(build)=.
>
> diff --git a/scripts/Makefile b/scripts/Makefile
> index b48259d..feb1f71 100644
> --- a/scripts/Makefile
> +++ b/scripts/Makefile
> @@ -36,9 +36,10 @@ PHONY += build_unifdef
>  build_unifdef: $(obj)/unifdef
> @:
>
> +subdir-$(CONFIG_GCC_PLUGINS) += gcc-plugins
>  subdir-$(CONFIG_MODVERSIONS) += genksyms
>  subdir-$(CONFIG_SECURITY_SELINUX) += selinux
>  subdir-$(CONFIG_GDB_SCRIPTS) += gdb
>
>  # Let clean descend into subdirs
> -subdir-+= basic dtc kconfig mod package gcc-plugins
> +subdir-+= basic dtc kconfig mod package
> diff --git a/scripts/Makefile.gcc-plugins b/scripts/Makefile.gcc-plugins
> index 46c5c68..c36f199 100644
> --- a/scripts/Makefile.gcc-plugins
> +++ b/scripts/Makefile.gcc-plugins
> @@ -49,11 +49,3 @@ KBUILD_CFLAGS += $(GCC_PLUGINS_CFLAGS)
>  # All enabled GCC plugins are collected here for building below.
>  GCC_PLUGIN := $(gcc-plugin-y)
>  export GCC_PLUGIN
> -
> -# Actually do the build, if requested.
> -PHONY += gcc-plugins
> -gcc-plugins: scripts_basic
> -ifdef CONFIG_GCC_PLUGINS
> -   $(Q)$(MAKE) $(build)=scripts/gcc-plugins
> -endif
> -   @:
> --
> 2.7.4
>


-- 
Best Regards
Masahiro Yamada


Re: [PATCH 1/2] kbuild: make 'scripts' depend on 'prepare0'

2018-11-26 Thread Masahiro Yamada
On Sat, Nov 24, 2018 at 4:57 PM Masahiro Yamada
 wrote:
>
> Before start descending, Kbuild needs to run 'prepare' and 'scripts',
> which were orthogonal to each other prior to this commit.
>
> Let's consider 'scripts' is a part of the preparation. This will allow
> more cleanups.
>
> Move 'scripts' to the prerequisite of 'prepare0', which starts compiling
> target *.c files.
>
> Signed-off-by: Masahiro Yamada 
> ---

Applied to linux-kbuild.



>
>  Makefile | 32 +++-
>  1 file changed, 15 insertions(+), 17 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index 36f3f0e..cee4cec 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1062,7 +1062,7 @@ $(sort $(vmlinux-deps)): $(vmlinux-dirs) ;
>  # Error messages still appears in the original language
>
>  PHONY += $(vmlinux-dirs)
> -$(vmlinux-dirs): prepare scripts
> +$(vmlinux-dirs): prepare
> $(Q)$(MAKE) $(build)=$@ need-builtin=1
>
>  define filechk_kernel.release
> @@ -1114,7 +1114,7 @@ macroprepare: prepare1 archmacros
>
>  archprepare: archheaders archscripts macroprepare scripts_basic
>
> -prepare0: archprepare gcc-plugins
> +prepare0: scripts archprepare gcc-plugins
> $(Q)$(MAKE) $(build)=scripts/mod
> $(Q)$(MAKE) $(build)=.
>
> @@ -1286,7 +1286,7 @@ modules.builtin: $(vmlinux-dirs:%=%/modules.builtin)
>
>  # Target to prepare building external modules
>  PHONY += modules_prepare
> -modules_prepare: prepare scripts
> +modules_prepare: prepare
>
>  # Target to install modules
>  PHONY += modules_install
> @@ -1604,11 +1604,9 @@ help:
> @echo  '  clean   - remove generated files in module 
> directory only'
> @echo  ''
>
> -# Dummies...
> -PHONY += prepare scripts
> +PHONY += prepare
>  prepare:
> $(cmd_crmodverdir)
> -scripts: ;
>  endif # KBUILD_EXTMOD
>
>  clean: $(clean-dirs)
> @@ -1712,33 +1710,33 @@ else
>  target-dir = $(if $(KBUILD_EXTMOD),$(dir $<),$(dir $@))
>  endif
>
> -%.s: %.c prepare scripts FORCE
> +%.s: %.c prepare FORCE
> $(Q)$(MAKE) $(build)=$(build-dir) $(target-dir)$(notdir $@)
> -%.i: %.c prepare scripts FORCE
> +%.i: %.c prepare FORCE
> $(Q)$(MAKE) $(build)=$(build-dir) $(target-dir)$(notdir $@)
> -%.o: %.c prepare scripts FORCE
> +%.o: %.c prepare FORCE
> $(Q)$(MAKE) $(build)=$(build-dir) $(target-dir)$(notdir $@)
> -%.lst: %.c prepare scripts FORCE
> +%.lst: %.c prepare FORCE
> $(Q)$(MAKE) $(build)=$(build-dir) $(target-dir)$(notdir $@)
> -%.s: %.S prepare scripts FORCE
> +%.s: %.S prepare FORCE
> $(Q)$(MAKE) $(build)=$(build-dir) $(target-dir)$(notdir $@)
> -%.o: %.S prepare scripts FORCE
> +%.o: %.S prepare FORCE
> $(Q)$(MAKE) $(build)=$(build-dir) $(target-dir)$(notdir $@)
> -%.symtypes: %.c prepare scripts FORCE
> +%.symtypes: %.c prepare FORCE
> $(Q)$(MAKE) $(build)=$(build-dir) $(target-dir)$(notdir $@)
> -%.ll: %.c prepare scripts FORCE
> +%.ll: %.c prepare FORCE
> $(Q)$(MAKE) $(build)=$(build-dir) $(target-dir)$(notdir $@)
>
>  # Modules
> -/: prepare scripts FORCE
> +/: prepare FORCE
> $(Q)$(MAKE) KBUILD_MODULES=$(if $(CONFIG_MODULES),1) \
> $(build)=$(build-dir)
>  # Make sure the latest headers are built for Documentation
>  Documentation/ samples/: headers_install
> -%/: prepare scripts FORCE
> +%/: prepare FORCE
> $(Q)$(MAKE) KBUILD_MODULES=$(if $(CONFIG_MODULES),1) \
> $(build)=$(build-dir)
> -%.ko: prepare scripts FORCE
> +%.ko: prepare FORCE
> $(Q)$(MAKE) KBUILD_MODULES=$(if $(CONFIG_MODULES),1)   \
> $(build)=$(build-dir) $(@:.ko=.o)
> $(Q)$(MAKE) -f $(srctree)/scripts/Makefile.modpost
> --
> 2.7.4
>


-- 
Best Regards
Masahiro Yamada


Re: [PATCH 2/2] modpost: file2alias: check prototype of handler

2018-11-26 Thread Masahiro Yamada
On Fri, Nov 23, 2018 at 1:51 AM Masahiro Yamada
 wrote:
>
> Use specific prototype instead of an opaque pointer so that the
> compiler can catch incompatible pointer type.
>
> Signed-off-by: Masahiro Yamada 
> ---

Applied to linux-kbuild.



>  scripts/mod/file2alias.c | 7 +++
>  1 file changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/scripts/mod/file2alias.c b/scripts/mod/file2alias.c
> index 7e4aede..a37af7d 100644
> --- a/scripts/mod/file2alias.c
> +++ b/scripts/mod/file2alias.c
> @@ -47,7 +47,7 @@ typedef struct {
>  struct devtable {
> const char *device_id; /* name of table, 
> __mod___*_device_table. */
> unsigned long id_size;
> -   void *function;
> +   int (*do_entry)(const char *filename, void *symval, char *alias);
>  };
>
>  /* Define a variable f that holds the value of field f of struct devid
> @@ -1299,12 +1299,11 @@ static bool sym_is(const char *name, unsigned 
> namelen, const char *symbol)
>  static void do_table(void *symval, unsigned long size,
>  unsigned long id_size,
>  const char *device_id,
> -void *function,
> +int (*do_entry)(const char *filename, void *symval, char 
> *alias),
>  struct module *mod)
>  {
> unsigned int i;
> char alias[500];
> -   int (*do_entry)(const char *, void *entry, char *alias) = function;
>
> device_id_check(mod->name, device_id, size, id_size, symval);
> /* Leave last one: it's the terminator. */
> @@ -1420,7 +1419,7 @@ void handle_moddevtable(struct module *mod, struct 
> elf_info *info,
>
> if (sym_is(name, namelen, p->device_id)) {
> do_table(symval, sym->st_size, p->id_size,
> -p->device_id, p->function, mod);
> +p->device_id, p->do_entry, mod);
> break;
> }
> }
> --
> 2.7.4
>


-- 
Best Regards
Masahiro Yamada


Re: [PATCH 1/2] modpost: file2alias: go back to simple devtable lookup

2018-11-26 Thread Masahiro Yamada
On Fri, Nov 23, 2018 at 1:52 AM Masahiro Yamada
 wrote:
>
> Commit e49ce14150c6 ("modpost: use linker section to generate table.")
> was not so cool as we had expected because it ended up with ugly section
> hacks when commit dd2a3acaecd7 ("mod/file2alias: make modpost compile on
> darwin again") came in.
>
> Given a certain degree of unknowledge about the link stage of host
> programs, I really want to see simple, stupid table lookup so that
> this works in the same way regardless of the underlying executable
> binary format.
>
> Signed-off-by: Masahiro Yamada 
> ---


Applied to linux-kbuild.



>  scripts/mod/file2alias.c | 144 
> ---
>  1 file changed, 49 insertions(+), 95 deletions(-)
>
> diff --git a/scripts/mod/file2alias.c b/scripts/mod/file2alias.c
> index 28a6166..7e4aede 100644
> --- a/scripts/mod/file2alias.c
> +++ b/scripts/mod/file2alias.c
> @@ -50,46 +50,6 @@ struct devtable {
> void *function;
>  };
>
> -#define ___cat(a,b) a ## b
> -#define __cat(a,b) ___cat(a,b)
> -
> -/* we need some special handling for this host tool running eventually on
> - * Darwin. The Mach-O section handling is a bit different than ELF section
> - * handling. The differnces in detail are:
> - *  a) we have segments which have sections
> - *  b) we need a API call to get the respective section symbols */
> -#if defined(__MACH__)
> -#include 
> -
> -#define INIT_SECTION(name)  do {   \
> -   unsigned long name ## _len; \
> -   char *__cat(pstart_,name) = getsectdata("__TEXT",   \
> -   #name, &__cat(name,_len));  \
> -   char *__cat(pstop_,name) = __cat(pstart_,name) +\
> -   __cat(name, _len);  \
> -   __cat(__start_,name) = (void *)__cat(pstart_,name); \
> -   __cat(__stop_,name) = (void *)__cat(pstop_,name);   \
> -   } while (0)
> -#define SECTION(name)   __attribute__((section("__TEXT, " #name)))
> -
> -struct devtable **__start___devtable, **__stop___devtable;
> -#else
> -#define INIT_SECTION(name) /* no-op for ELF */
> -#define SECTION(name)   __attribute__((section(#name)))
> -
> -/* We construct a table of pointers in an ELF section (pointers generally
> - * go unpadded by gcc).  ld creates boundary syms for us. */
> -extern struct devtable *__start___devtable[], *__stop___devtable[];
> -#endif /* __MACH__ */
> -
> -#if !defined(__used)
> -# if __GNUC__ == 3 && __GNUC_MINOR__ < 3
> -#  define __used   __attribute__((__unused__))
> -# else
> -#  define __used   __attribute__((__used__))
> -# endif
> -#endif
> -
>  /* Define a variable f that holds the value of field f of struct devid
>   * based at address m.
>   */
> @@ -110,16 +70,6 @@ extern struct devtable *__start___devtable[], 
> *__stop___devtable[];
>  #define DEF_FIELD_ADDR(m, devid, f) \
> DEF_FIELD_ADDR_VAR(m, devid, f, f)
>
> -/* Add a table entry.  We test function type matches while we're here. */
> -#define ADD_TO_DEVTABLE(device_id, type, function) \
> -   static struct devtable __cat(devtable,__LINE__) = { \
> -   device_id + 0*sizeof((function)((const char *)NULL, \
> -   (void *)NULL,   \
> -   (char *)NULL)), \
> -   SIZE_##type, (function) };  \
> -   static struct devtable *SECTION(__devtable) __used \
> -   __cat(devtable_ptr,__LINE__) = &__cat(devtable,__LINE__)
> -
>  #define ADD(str, sep, cond, field)  \
>  do {\
>  strcat(str, sep);   \
> @@ -439,7 +389,6 @@ static int do_hid_entry(const char *filename,
>
> return 1;
>  }
> -ADD_TO_DEVTABLE("hid", hid_device_id, do_hid_entry);
>
>  /* Looks like: ieee1394:venNmoNspNverN */
>  static int do_ieee1394_entry(const char *filename,
> @@ -464,7 +413,6 @@ static int do_ieee1394_entry(const char *filename,
> add_wildcard(alias);
> return 1;
>  }
> -ADD_TO_DEVTABLE("ieee1394", ieee1394_device_id, do_ieee1394_entry);
>
>  /* Looks like: pci:vNdNsvNsdNbcNscNiN. */
>  static int do_pci_entry(const char *filename,
> @@ -508,7 +456,6 @@ static int do_pci_entry(const char *filename,
> add_wildcard(alias);
> return 1;
>  }
> 

Re: [PATCH 1/2] kbuild: fix single target build for external module

2018-11-26 Thread Masahiro Yamada
On Thu, Nov 22, 2018 at 6:33 PM Masahiro Yamada
 wrote:
>
> Building a single target in an external module fails due to missing
> .tmp_versions directory.
>
> For example,
>
>   $ make -C /lib/modules/$(uname -r)/build M=$PWD foo.o
>
> will fail in the following way:
>
>   CC [M]  /home/masahiro/foo/foo.o
> /bin/sh: 1: cannot create /home/masahiro/foo/.tmp_versions/foo.mod: Directory 
> nonexistent
>
> This is because $(cmd_crmodverdir) is executed only for /, %/, %.ko
> single targets for external modules. Create .tmp_versions in the
> 'prepare' target.
>
> Signed-off-by: Masahiro Yamada 
> ---


Applied to linux-kbuild.



>  Makefile | 11 +++
>  1 file changed, 3 insertions(+), 8 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index d5da1df..36f3f0e 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1554,9 +1554,6 @@ else # KBUILD_EXTMOD
>
>  # We are always building modules
>  KBUILD_MODULES := 1
> -PHONY += crmodverdir
> -crmodverdir:
> -   $(cmd_crmodverdir)
>
>  PHONY += $(objtree)/Module.symvers
>  $(objtree)/Module.symvers:
> @@ -1568,7 +1565,7 @@ $(objtree)/Module.symvers:
>
>  module-dirs := $(addprefix _module_,$(KBUILD_EXTMOD))
>  PHONY += $(module-dirs) modules
> -$(module-dirs): crmodverdir $(objtree)/Module.symvers
> +$(module-dirs): prepare $(objtree)/Module.symvers
> $(Q)$(MAKE) $(build)=$(patsubst _module_%,%,$@)
>
>  modules: $(module-dirs)
> @@ -1609,7 +1606,8 @@ help:
>
>  # Dummies...
>  PHONY += prepare scripts
> -prepare: ;
> +prepare:
> +   $(cmd_crmodverdir)
>  scripts: ;
>  endif # KBUILD_EXTMOD
>
> @@ -1733,17 +1731,14 @@ endif
>
>  # Modules
>  /: prepare scripts FORCE
> -   $(cmd_crmodverdir)
> $(Q)$(MAKE) KBUILD_MODULES=$(if $(CONFIG_MODULES),1) \
> $(build)=$(build-dir)
>  # Make sure the latest headers are built for Documentation
>  Documentation/ samples/: headers_install
>  %/: prepare scripts FORCE
> -   $(cmd_crmodverdir)
> $(Q)$(MAKE) KBUILD_MODULES=$(if $(CONFIG_MODULES),1) \
> $(build)=$(build-dir)
>  %.ko: prepare scripts FORCE
> -   $(cmd_crmodverdir)
> $(Q)$(MAKE) KBUILD_MODULES=$(if $(CONFIG_MODULES),1)   \
> $(build)=$(build-dir) $(@:.ko=.o)
> $(Q)$(MAKE) -f $(srctree)/scripts/Makefile.modpost
> --
> 2.7.4
>


-- 
Best Regards
Masahiro Yamada


Re: [PATCH 1/2] kconfig: fix reverse dependency with tristate if-conditional

2018-11-26 Thread Masahiro Yamada
On Mon, Nov 26, 2018 at 5:17 PM kbuild test robot  wrote:
>
> Hi Masahiro,
>
> I love your patch! Yet something to improve:
>
> [auto build test ERROR on masahiroy/kconfig]
> [also build test ERROR on v4.20-rc4]
> [if your patch is applied to the wrong git tree, please drop us a note to 
> help improve the system]
>
> url:
> https://github.com/0day-ci/linux/commits/Masahiro-Yamada/kconfig-fix-reverse-dependency-with-tristate-if-conditional/20181126-152716
> base:   
> https://git.kernel.org/pub/scm/linux/kernel/git/masahiroy/linux-kbuild.git 
> kconfig
> config: i386-randconfig-s1-201847 (attached as .config)
> compiler: gcc-6 (Debian 6.4.0-9) 6.4.0 20171026
> reproduce:
> # save the attached .config to linux build tree
> make ARCH=i386
>
> All errors (new ones prefixed by >>):


OK, this is not so easy to fix as I expected.


UML is not defined at all for i386,

'UML != n' is evaluated to 'y'.


I need to think about how to fix this.








>drivers/input//mouse/synaptics.c: In function 
> 'synaptics_create_intertouch':
> >> drivers/input//mouse/synaptics.c:1754:9: error: implicit declaration of 
> >> function 'psmouse_smbus_init' [-Werror=implicit-function-declaration]
>  return psmouse_smbus_init(psmouse, _board,
> ^~
>cc1: some warnings being treated as errors
>
> vim +/psmouse_smbus_init +1754 drivers/input//mouse/synaptics.c
>
> e839ffab Benjamin Tissoires 2017-03-02  1728
> e839ffab Benjamin Tissoires 2017-03-02  1729  static int 
> synaptics_create_intertouch(struct psmouse *psmouse,
> e839ffab Benjamin Tissoires 2017-03-02  1730  
>  struct synaptics_device_info *info,
> e839ffab Benjamin Tissoires 2017-03-02  1731  
>  bool leave_breadcrumbs)
> e839ffab Benjamin Tissoires 2017-03-02  1732  {
> e839ffab Benjamin Tissoires 2017-03-02  1733bool topbuttonpad =
> e839ffab Benjamin Tissoires 2017-03-02  1734
> psmouse_matches_pnp_id(psmouse, topbuttonpad_pnp_ids) &&
> e839ffab Benjamin Tissoires 2017-03-02  1735
> !SYN_CAP_EXT_BUTTONS_STICK(info->ext_cap_10);
> e839ffab Benjamin Tissoires 2017-03-02  1736const struct 
> rmi_device_platform_data pdata = {
> e839ffab Benjamin Tissoires 2017-03-02  1737.sensor_pdata = {
> e839ffab Benjamin Tissoires 2017-03-02  1738.sensor_type 
> = rmi_sensor_touchpad,
> e839ffab Benjamin Tissoires 2017-03-02  1739
> .axis_align.flip_y = true,
> 2b30297d Andrew Duggan  2017-10-09  1740
> .kernel_tracking = false,
> e839ffab Benjamin Tissoires 2017-03-02  1741.topbuttonpad 
> = topbuttonpad,
> e839ffab Benjamin Tissoires 2017-03-02  1742},
> e839ffab Benjamin Tissoires 2017-03-02  1743.f30_data = {
> e839ffab Benjamin Tissoires 2017-03-02  1744.buttonpad = 
> SYN_CAP_CLICKPAD(info->ext_cap_0c),
> e839ffab Benjamin Tissoires 2017-03-02  1745
> .trackstick_buttons =
> e839ffab Benjamin Tissoires 2017-03-02  1746
> !!SYN_CAP_EXT_BUTTONS_STICK(info->ext_cap_10),
> e839ffab Benjamin Tissoires 2017-03-02  1747},
> e839ffab Benjamin Tissoires 2017-03-02  1748};
> e839ffab Benjamin Tissoires 2017-03-02  1749const struct i2c_board_info 
> intertouch_board = {
> e839ffab Benjamin Tissoires 2017-03-02  1750
> I2C_BOARD_INFO("rmi4_smbus", 0x2c),
> e839ffab Benjamin Tissoires 2017-03-02  1751.flags = 
> I2C_CLIENT_HOST_NOTIFY,
> e839ffab Benjamin Tissoires 2017-03-02  1752};
> e839ffab Benjamin Tissoires 2017-03-02  1753
> e839ffab Benjamin Tissoires 2017-03-02 @1754return 
> psmouse_smbus_init(psmouse, _board,
> bf232e46 Benjamin Tissoires 2018-05-22  1755  
> , sizeof(pdata), true,
> e839ffab Benjamin Tissoires 2017-03-02  1756  
> leave_breadcrumbs);
> e839ffab Benjamin Tissoires 2017-03-02  1757  }
> e839ffab Benjamin Tissoires 2017-03-02  1758
>
> :: The code at line 1754 was first introduced by commit
> :: e839ffab028981ac77f650faf8c84f16e1719738 Input: synaptics - add 
> support for Intertouch devices
>
> :: TO: Benjamin Tissoires 
> :: CC: Dmitry Torokhov 
>
> ---
> 0-DAY kernel test infrastructureOpen Source Technology Center
> https://lists.01.org/pipermail/kbuild-all   Intel Corporation



-- 
Best Regards
Masahiro Yamada


[PATCH] pcmcia: remove per-arch PCMCIA config entry

2018-11-26 Thread Masahiro Yamada
Now that all architectures include drivers/pcmcia/Kconfig where
the PCMCIA config is defined, the PCMCIA config entries in per-arch
Kconfig files are redundant.

Signed-off-by: Masahiro Yamada 
---

I will queue this up to my kbuild tree
along with Christoph's clean-up patch set.


 arch/s390/Kconfig | 3 ---
 arch/um/Kconfig   | 3 ---
 2 files changed, 6 deletions(-)

diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig
index 5173366..b212c59 100644
--- a/arch/s390/Kconfig
+++ b/arch/s390/Kconfig
@@ -837,9 +837,6 @@ source "kernel/power/Kconfig"
 
 endmenu
 
-config PCMCIA
-   def_bool n
-
 config CCW
def_bool y
 
diff --git a/arch/um/Kconfig b/arch/um/Kconfig
index 6b99389..a7d09a7 100644
--- a/arch/um/Kconfig
+++ b/arch/um/Kconfig
@@ -34,9 +34,6 @@ config SBUS
 config PCI
bool
 
-config PCMCIA
-   bool
-
 config TRACE_IRQFLAGS_SUPPORT
bool
default y
-- 
2.7.4



[PATCH] nios2: remove unneeded HAS_DMA define

2018-11-25 Thread Masahiro Yamada
kernel/dma/Kconfig globally defines HAS_DMA as follows:

  config HAS_DMA
  bool
  depends on !NO_DMA
  default y

Signed-off-by: Masahiro Yamada 
---

 arch/nios2/Kconfig | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/arch/nios2/Kconfig b/arch/nios2/Kconfig
index 7e95506..69c4af1 100644
--- a/arch/nios2/Kconfig
+++ b/arch/nios2/Kconfig
@@ -37,9 +37,6 @@ config GENERIC_CALIBRATE_DELAY
 config NO_IOPORT_MAP
def_bool y
 
-config HAS_DMA
-   def_bool y
-
 config FPU
def_bool n
 
-- 
2.7.4



[PATCH 1/2] kconfig: fix reverse dependency with tristate if-conditional

2018-11-25 Thread Masahiro Yamada
A Kconfig property can have an optional if-expression, which describes
its visibility. The property is visible when the if-expression part is
evaluated to 'y' or 'm'.

The 'select' and 'imply' properties are internally converted to reverse
dependencies, but they are wrongly converted if they have a tristate
if-expression.

Example:

  config A
  tristate "a"

  config B
  tristate "b"
  select A if C

  config C
  tristate "c"

Currently, the reverse dependency of 'A' results in 'B && C'.
It is incorrect because the combination of B=y and C=m allows
'A' to become 'm', while its lower limit must be 'y'.

The reverse dependency should be 'B && C != n'.

Randy Dunlap reported that people are trying to fix an individual
Kconfig file [1], and I also found another example in the past,
commit 9d9c98e89ee2 ("pcmcia: fix yenta dependency on PCCARD_NONSTATIC")
but I suspect this is a bug of Kconfig itself.

[1] https://www.spinics.net/lists/netfilter-devel/msg56985.html

Reported-by: Taehee Yoo 
Reported-by: Randy Dunlap 
Signed-off-by: Masahiro Yamada 
---

 scripts/kconfig/menu.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/scripts/kconfig/menu.c b/scripts/kconfig/menu.c
index 4cf15d4..2b18833 100644
--- a/scripts/kconfig/menu.c
+++ b/scripts/kconfig/menu.c
@@ -401,11 +401,13 @@ void menu_finalize(struct menu *parent)
if (prop->type == P_SELECT) {
struct symbol *es = 
prop_get_symbol(prop);
es->rev_dep.expr = 
expr_alloc_or(es->rev_dep.expr,
-   
expr_alloc_and(expr_alloc_symbol(menu->sym), expr_copy(dep)));
+   
expr_alloc_and(expr_alloc_symbol(menu->sym),
+   
expr_trans_compare(dep, E_UNEQUAL, _no)));
} else if (prop->type == P_IMPLY) {
struct symbol *es = 
prop_get_symbol(prop);
es->implied.expr = 
expr_alloc_or(es->implied.expr,
-   
expr_alloc_and(expr_alloc_symbol(menu->sym), expr_copy(dep)));
+   
expr_alloc_and(expr_alloc_symbol(menu->sym),
+   
expr_trans_compare(dep, E_UNEQUAL, _no)));
}
}
}
-- 
2.7.4



[PATCH 2/2] kconfig: tests: test reverse dependency with tristate if-conditional

2018-11-25 Thread Masahiro Yamada
Add a test-case for the fixed reverse dependency handling.

Signed-off-by: Masahiro Yamada 
---

 .../kconfig/tests/revdep_with_tristate_if/Kconfig   | 21 +
 .../tests/revdep_with_tristate_if/__init__.py   | 14 ++
 .../tests/revdep_with_tristate_if/expected_config   |  9 +
 3 files changed, 44 insertions(+)
 create mode 100644 scripts/kconfig/tests/revdep_with_tristate_if/Kconfig
 create mode 100644 scripts/kconfig/tests/revdep_with_tristate_if/__init__.py
 create mode 100644 
scripts/kconfig/tests/revdep_with_tristate_if/expected_config

diff --git a/scripts/kconfig/tests/revdep_with_tristate_if/Kconfig 
b/scripts/kconfig/tests/revdep_with_tristate_if/Kconfig
new file mode 100644
index 000..2bd1141
--- /dev/null
+++ b/scripts/kconfig/tests/revdep_with_tristate_if/Kconfig
@@ -0,0 +1,21 @@
+# SPDX-License-Identifier: GPL-2.0
+
+config MODULES
+   def_bool y
+   option modules
+
+config A
+   tristate
+
+config B
+   tristate "b"
+   default y
+   select A if C
+   imply D if C
+
+config C
+   tristate "c"
+   default m
+
+config D
+   tristate "d"
diff --git a/scripts/kconfig/tests/revdep_with_tristate_if/__init__.py 
b/scripts/kconfig/tests/revdep_with_tristate_if/__init__.py
new file mode 100644
index 000..ad95cec
--- /dev/null
+++ b/scripts/kconfig/tests/revdep_with_tristate_if/__init__.py
@@ -0,0 +1,14 @@
+# SPDX-License-Identifier: GPL-2.0
+"""
+select/imply property with tristate if-conditional
+
+The reverse dependencies (select/imply) are used to define a lower limit
+of another symbol. The current value of the selector is set to the lower
+limit of the selectee. This did not handled correctly in the past when the
+property has a tristate if-conditional.
+"""
+
+
+def test(conf):
+assert conf.alldefconfig() == 0
+assert conf.config_matches('expected_config')
diff --git a/scripts/kconfig/tests/revdep_with_tristate_if/expected_config 
b/scripts/kconfig/tests/revdep_with_tristate_if/expected_config
new file mode 100644
index 000..9826223
--- /dev/null
+++ b/scripts/kconfig/tests/revdep_with_tristate_if/expected_config
@@ -0,0 +1,9 @@
+#
+# Automatically generated file; DO NOT EDIT.
+# Main menu
+#
+CONFIG_MODULES=y
+CONFIG_A=y
+CONFIG_B=y
+CONFIG_C=m
+CONFIG_D=y
-- 
2.7.4



Re: [PATCH v4 1/3] kernel.h: disable type-checks in container_of() for Sparse

2018-11-25 Thread Masahiro Yamada
On Sat, Nov 24, 2018 at 6:06 PM Miguel Ojeda
 wrote:
>
> On Fri, Nov 23, 2018 at 10:14 PM Luc Van Oostenryck
>  wrote:
> >
> > On Thu, Nov 22, 2018 at 12:14:20PM +0900, Masahiro Yamada wrote:
> > > When I tried to enable BUILD_BUG_ON for Sparse, the kbuild test robot
> > > reported lots of "unknown expression" warnings from container_of(),
> > > which seemed false positive.
> > >
> > > I addressed this in [1], but fixing Sparse is the right thing to do.
> > >
> > > The issue was fixed by Sparse commit 0eb8175d3e9c ("fix expansion of
> > > function designator"), but it will take time until the fixed version
> > > of Sparse is widely available.
> > >
> > > Disable the container_of() type checks for Sparse for now.
> >
> > I would prefer that developers upgrade their version of sparse but ...
> >
> > Reviewed-by: Luc Van Oostenryck 
>
> Indeed. If someone is writing code for the latest kernels, I think it
> is reasonable to assume they are able to use the latest sparse too,
> since it is not required for compilation anyway.


I am OK with either way.

I leave this to Andrew.



-- 
Best Regards
Masahiro Yamada


Re: [PATCH v4 0/2] dmaengine: add UniPhier MIO DMAC driver

2018-11-25 Thread Masahiro Yamada
On Sat, Nov 24, 2018 at 11:16 PM Vinod Koul  wrote:
>
> On 12-10-18, 01:41, Masahiro Yamada wrote:
> > 1/2: DT-binding
> > 2/2: driver
>
> Applied this series, thanks


Thanks.


> While building I noticed that we get few warns when compiling with
> C=1, I would prefer you fix them. Please send fixes on top of the
> applied patches.

My code is fine.

If you are talking about the following, your sparse is too old.
Upgrade your sparse.

./include/linux/overflow.h:285:13: error: undefined identifier
'__builtin_mul_overflow'
./include/linux/overflow.h:285:13: error: incorrect type in conditional
./include/linux/overflow.h:285:13:got void
./include/linux/overflow.h:287:13: error: undefined identifier
'__builtin_add_overflow'
./include/linux/overflow.h:287:13: error: incorrect type in conditional
./include/linux/overflow.h:287:13:got void
./include/linux/overflow.h:285:13: warning: call with no type!
./include/linux/overflow.h:287:13: warning: call with no type!





> Thanks
> --
> ~Vinod



-- 
Best Regards
Masahiro Yamada


[PATCH 2/3] modpost: merge module iterations

2018-11-22 Thread Masahiro Yamada
Probably, this is just a matter of the order of error/warning
messages. Merge the two for-loops.

Signed-off-by: Masahiro Yamada 
---

 scripts/mod/modpost.c | 7 +--
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index 39432c2..05e41eb 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -2484,12 +2484,6 @@ int main(int argc, char **argv)
if (files_source)
read_symbols_from_files(files_source);
 
-   for (mod = modules; mod; mod = mod->next) {
-   if (mod->skip)
-   continue;
-   check_exports(mod);
-   }
-
err = 0;
 
for (mod = modules; mod; mod = mod->next) {
@@ -2501,6 +2495,7 @@ int main(int argc, char **argv)
buf.pos = 0;
 
err |= check_modname_len(mod);
+   check_exports(mod);
add_header(, mod);
add_intree_flag(, !external_module);
add_retpoline();
-- 
2.7.4



[PATCH 1/3] modpost: refactor seen flag clearing in add_depends()

2018-11-22 Thread Masahiro Yamada
You do not need to iterate over all modules for resetting ->seen flag
because add_depends() is only interested in modules that export symbols
referenced from the given 'mod'.

This also avoids shadowing the 'modules' parameter of add_depends().

Signed-off-by: Masahiro Yamada 
---

 scripts/mod/modpost.c | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index c64066d..39432c2 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -2229,15 +2229,15 @@ static int add_versions(struct buffer *b, struct module 
*mod)
return err;
 }
 
-static void add_depends(struct buffer *b, struct module *mod,
-   struct module *modules)
+static void add_depends(struct buffer *b, struct module *mod)
 {
struct symbol *s;
-   struct module *m;
int first = 1;
 
-   for (m = modules; m; m = m->next)
-   m->seen = is_vmlinux(m->name);
+   /* Clear ->seen flag of modules that own symbols needed by this. */
+   for (s = mod->unres; s; s = s->next)
+   if (s->module)
+   s->module->seen = is_vmlinux(s->module->name);
 
buf_printf(b, "\n");
buf_printf(b, "static const char __module_depends[]\n");
@@ -2506,7 +2506,7 @@ int main(int argc, char **argv)
add_retpoline();
add_staging_flag(, mod->name);
err |= add_versions(, mod);
-   add_depends(, mod, modules);
+   add_depends(, mod);
add_moddevtable(, mod);
add_srcversion(, mod);
 
-- 
2.7.4



[PATCH 3/3] modpost: move unresolved symbol checks to check_exports()

2018-11-22 Thread Masahiro Yamada
This will fit better in check_exports() than add_versions().

Signed-off-by: Masahiro Yamada 
---

 scripts/mod/modpost.c | 33 ++---
 1 file changed, 18 insertions(+), 15 deletions(-)

diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index 05e41eb..32e5026 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -2085,15 +2085,27 @@ static void check_for_unused(enum export exp, const 
char *m, const char *s)
}
 }
 
-static void check_exports(struct module *mod)
+static int check_exports(struct module *mod)
 {
struct symbol *s, *exp;
+   int err = 0;
 
for (s = mod->unres; s; s = s->next) {
const char *basename;
exp = find_symbol(s->name);
-   if (!exp || exp->module == mod)
+   if (!exp || exp->module == mod) {
+   if (have_vmlinux && !s->weak) {
+   if (warn_unresolved) {
+   warn("\"%s\" [%s.ko] undefined!\n",
+s->name, mod->name);
+   } else {
+   merror("\"%s\" [%s.ko] undefined!\n",
+  s->name, mod->name);
+   err = 1;
+   }
+   }
continue;
+   }
basename = strrchr(mod->name, '/');
if (basename)
basename++;
@@ -2103,6 +2115,8 @@ static void check_exports(struct module *mod)
check_for_gpl_usage(exp->export, basename, exp->name);
check_for_unused(exp->export, basename, exp->name);
}
+
+   return err;
 }
 
 static int check_modname_len(struct module *mod)
@@ -2180,19 +2194,8 @@ static int add_versions(struct buffer *b, struct module 
*mod)
 
for (s = mod->unres; s; s = s->next) {
exp = find_symbol(s->name);
-   if (!exp || exp->module == mod) {
-   if (have_vmlinux && !s->weak) {
-   if (warn_unresolved) {
-   warn("\"%s\" [%s.ko] undefined!\n",
-s->name, mod->name);
-   } else {
-   merror("\"%s\" [%s.ko] undefined!\n",
-  s->name, mod->name);
-   err = 1;
-   }
-   }
+   if (!exp || exp->module == mod)
continue;
-   }
s->module = exp->module;
s->crc_valid = exp->crc_valid;
s->crc = exp->crc;
@@ -2495,7 +2498,7 @@ int main(int argc, char **argv)
buf.pos = 0;
 
err |= check_modname_len(mod);
-   check_exports(mod);
+   err |= check_exports(mod);
add_header(, mod);
add_intree_flag(, !external_module);
add_retpoline();
-- 
2.7.4



Re: [PATCH] kbuild: move modpost out of 'scripts' target

2018-11-22 Thread Masahiro Yamada
On Wed, Nov 21, 2018 at 2:16 AM Masahiro Yamada
 wrote:
>
> I am eagar to build under the scripts/ directory only with $(HOSTCC),
> but scripts/mod/ highly depends on the $(CC) and target arch headers.
> That it why the 'scripts' target must depend on 'asm-generic',
> 'gcc-plugins', and $(autoksyms_h).
>
> Move it to the 'prepare0' stage. I know this is a cheesy workaround,
> but better than the current situation.
>
> Signed-off-by: Masahiro Yamada 


Applied to linux-kbuild.



> ---
>
>  Makefile | 3 ++-
>  scripts/Makefile | 3 +--
>  scripts/mod/Makefile | 2 --
>  3 files changed, 3 insertions(+), 5 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index b78cc97..21a7729 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1073,7 +1073,7 @@ include/config/kernel.release: $(srctree)/Makefile FORCE
>  # Carefully list dependencies so we do not try to build scripts twice
>  # in parallel
>  PHONY += scripts
> -scripts: scripts_basic scripts_dtc asm-generic gcc-plugins $(autoksyms_h)
> +scripts: scripts_basic scripts_dtc
> $(Q)$(MAKE) $(build)=$(@)
>
>  # Things we need to do before we recursively start building the kernel
> @@ -,6 +,7 @@ macroprepare: prepare1 archmacros
>  archprepare: archheaders archscripts macroprepare scripts_basic
>
>  prepare0: archprepare gcc-plugins
> +   $(Q)$(MAKE) $(build)=scripts/mod
> $(Q)$(MAKE) $(build)=.
>
>  # All the preparing..
> diff --git a/scripts/Makefile b/scripts/Makefile
> index ece52ff..b48259d 100644
> --- a/scripts/Makefile
> +++ b/scripts/Makefile
> @@ -37,9 +37,8 @@ build_unifdef: $(obj)/unifdef
> @:
>
>  subdir-$(CONFIG_MODVERSIONS) += genksyms
> -subdir-y += mod
>  subdir-$(CONFIG_SECURITY_SELINUX) += selinux
>  subdir-$(CONFIG_GDB_SCRIPTS) += gdb
>
>  # Let clean descend into subdirs
> -subdir-+= basic dtc kconfig package gcc-plugins
> +subdir-+= basic dtc kconfig mod package gcc-plugins
> diff --git a/scripts/mod/Makefile b/scripts/mod/Makefile
> index a5b4af4..42c5d50 100644
> --- a/scripts/mod/Makefile
> +++ b/scripts/mod/Makefile
> @@ -4,8 +4,6 @@ OBJECT_FILES_NON_STANDARD := y
>  hostprogs-y:= modpost mk_elfconfig
>  always := $(hostprogs-y) empty.o
>
> -CFLAGS_REMOVE_empty.o := $(ASM_MACRO_FLAGS)
> -
>  modpost-objs   := modpost.o file2alias.o sumversion.o
>
>  devicetable-offsets-file := devicetable-offsets.h
> --
> 2.7.4
>


-- 
Best Regards
Masahiro Yamada


[PATCH 2/2] kbuild: descend into scripts/gcc-plugins/ via scripts/Makefile

2018-11-22 Thread Masahiro Yamada
Now that 'prepare0' depends on 'scripts', building GCC plugins can
go into scripts/Makefile, which is a more standard way.

Signed-off-by: Masahiro Yamada 
---

 Makefile | 2 +-
 scripts/Makefile | 3 ++-
 scripts/Makefile.gcc-plugins | 8 
 3 files changed, 3 insertions(+), 10 deletions(-)

diff --git a/Makefile b/Makefile
index cee4cec..a8bbe68 100644
--- a/Makefile
+++ b/Makefile
@@ -1114,7 +1114,7 @@ macroprepare: prepare1 archmacros
 
 archprepare: archheaders archscripts macroprepare scripts_basic
 
-prepare0: scripts archprepare gcc-plugins
+prepare0: scripts archprepare
$(Q)$(MAKE) $(build)=scripts/mod
$(Q)$(MAKE) $(build)=.
 
diff --git a/scripts/Makefile b/scripts/Makefile
index b48259d..feb1f71 100644
--- a/scripts/Makefile
+++ b/scripts/Makefile
@@ -36,9 +36,10 @@ PHONY += build_unifdef
 build_unifdef: $(obj)/unifdef
@:
 
+subdir-$(CONFIG_GCC_PLUGINS) += gcc-plugins
 subdir-$(CONFIG_MODVERSIONS) += genksyms
 subdir-$(CONFIG_SECURITY_SELINUX) += selinux
 subdir-$(CONFIG_GDB_SCRIPTS) += gdb
 
 # Let clean descend into subdirs
-subdir-+= basic dtc kconfig mod package gcc-plugins
+subdir-+= basic dtc kconfig mod package
diff --git a/scripts/Makefile.gcc-plugins b/scripts/Makefile.gcc-plugins
index 46c5c68..c36f199 100644
--- a/scripts/Makefile.gcc-plugins
+++ b/scripts/Makefile.gcc-plugins
@@ -49,11 +49,3 @@ KBUILD_CFLAGS += $(GCC_PLUGINS_CFLAGS)
 # All enabled GCC plugins are collected here for building below.
 GCC_PLUGIN := $(gcc-plugin-y)
 export GCC_PLUGIN
-
-# Actually do the build, if requested.
-PHONY += gcc-plugins
-gcc-plugins: scripts_basic
-ifdef CONFIG_GCC_PLUGINS
-   $(Q)$(MAKE) $(build)=scripts/gcc-plugins
-endif
-   @:
-- 
2.7.4



[PATCH 1/2] kbuild: make 'scripts' depend on 'prepare0'

2018-11-22 Thread Masahiro Yamada
Before start descending, Kbuild needs to run 'prepare' and 'scripts',
which were orthogonal to each other prior to this commit.

Let's consider 'scripts' is a part of the preparation. This will allow
more cleanups.

Move 'scripts' to the prerequisite of 'prepare0', which starts compiling
target *.c files.

Signed-off-by: Masahiro Yamada 
---

 Makefile | 32 +++-
 1 file changed, 15 insertions(+), 17 deletions(-)

diff --git a/Makefile b/Makefile
index 36f3f0e..cee4cec 100644
--- a/Makefile
+++ b/Makefile
@@ -1062,7 +1062,7 @@ $(sort $(vmlinux-deps)): $(vmlinux-dirs) ;
 # Error messages still appears in the original language
 
 PHONY += $(vmlinux-dirs)
-$(vmlinux-dirs): prepare scripts
+$(vmlinux-dirs): prepare
$(Q)$(MAKE) $(build)=$@ need-builtin=1
 
 define filechk_kernel.release
@@ -1114,7 +1114,7 @@ macroprepare: prepare1 archmacros
 
 archprepare: archheaders archscripts macroprepare scripts_basic
 
-prepare0: archprepare gcc-plugins
+prepare0: scripts archprepare gcc-plugins
$(Q)$(MAKE) $(build)=scripts/mod
$(Q)$(MAKE) $(build)=.
 
@@ -1286,7 +1286,7 @@ modules.builtin: $(vmlinux-dirs:%=%/modules.builtin)
 
 # Target to prepare building external modules
 PHONY += modules_prepare
-modules_prepare: prepare scripts
+modules_prepare: prepare
 
 # Target to install modules
 PHONY += modules_install
@@ -1604,11 +1604,9 @@ help:
@echo  '  clean   - remove generated files in module directory 
only'
@echo  ''
 
-# Dummies...
-PHONY += prepare scripts
+PHONY += prepare
 prepare:
$(cmd_crmodverdir)
-scripts: ;
 endif # KBUILD_EXTMOD
 
 clean: $(clean-dirs)
@@ -1712,33 +1710,33 @@ else
 target-dir = $(if $(KBUILD_EXTMOD),$(dir $<),$(dir $@))
 endif
 
-%.s: %.c prepare scripts FORCE
+%.s: %.c prepare FORCE
$(Q)$(MAKE) $(build)=$(build-dir) $(target-dir)$(notdir $@)
-%.i: %.c prepare scripts FORCE
+%.i: %.c prepare FORCE
$(Q)$(MAKE) $(build)=$(build-dir) $(target-dir)$(notdir $@)
-%.o: %.c prepare scripts FORCE
+%.o: %.c prepare FORCE
$(Q)$(MAKE) $(build)=$(build-dir) $(target-dir)$(notdir $@)
-%.lst: %.c prepare scripts FORCE
+%.lst: %.c prepare FORCE
$(Q)$(MAKE) $(build)=$(build-dir) $(target-dir)$(notdir $@)
-%.s: %.S prepare scripts FORCE
+%.s: %.S prepare FORCE
$(Q)$(MAKE) $(build)=$(build-dir) $(target-dir)$(notdir $@)
-%.o: %.S prepare scripts FORCE
+%.o: %.S prepare FORCE
$(Q)$(MAKE) $(build)=$(build-dir) $(target-dir)$(notdir $@)
-%.symtypes: %.c prepare scripts FORCE
+%.symtypes: %.c prepare FORCE
$(Q)$(MAKE) $(build)=$(build-dir) $(target-dir)$(notdir $@)
-%.ll: %.c prepare scripts FORCE
+%.ll: %.c prepare FORCE
$(Q)$(MAKE) $(build)=$(build-dir) $(target-dir)$(notdir $@)
 
 # Modules
-/: prepare scripts FORCE
+/: prepare FORCE
$(Q)$(MAKE) KBUILD_MODULES=$(if $(CONFIG_MODULES),1) \
$(build)=$(build-dir)
 # Make sure the latest headers are built for Documentation
 Documentation/ samples/: headers_install
-%/: prepare scripts FORCE
+%/: prepare FORCE
$(Q)$(MAKE) KBUILD_MODULES=$(if $(CONFIG_MODULES),1) \
$(build)=$(build-dir)
-%.ko: prepare scripts FORCE
+%.ko: prepare FORCE
$(Q)$(MAKE) KBUILD_MODULES=$(if $(CONFIG_MODULES),1)   \
$(build)=$(build-dir) $(@:.ko=.o)
$(Q)$(MAKE) -f $(srctree)/scripts/Makefile.modpost
-- 
2.7.4



Re: [PATCH] modpost: skip ELF local symbols during section mismatch check

2018-11-22 Thread Masahiro Yamada
On Thu, Nov 22, 2018 at 4:11 PM Paul Walmsley  wrote:
>
>
> During development of a serial console driver with a gcc 8.2.0
> toolchain for RISC-V, the following modpost warning appeared:
>
> 
> WARNING: vmlinux.o(.data+0x19b10): Section mismatch in reference from the 
> variable .LANCHOR1 to the function .init.text:sifive_serial_console_setup()
> The variable .LANCHOR1 references
> the function __init sifive_serial_console_setup()
> If the reference is valid then annotate the
> variable with __init* or __refdata (see linux/init.h) or name the variable:
> *_template, *_timer, *_sht, *_ops, *_probe, *_probe_one, *_console
> 
>
> ".LANCHOR1" is an ELF local symbol, automatically created by gcc's section
> anchor generation code:
>
> https://gcc.gnu.org/onlinedocs/gccint/Anchored-Addresses.html
>
> https://gcc.gnu.org/git/?p=gcc.git;a=blob;f=gcc/varasm.c;h=cd9591a45617464946dcf9a126dde277d9de9804;hb=9fb89fa845c1b2e0a18d85ada0b077c84508ab78#l7473
>
> This was verified by compiling the kernel with -fno-section-anchors
> and observing that the ".LANCHOR1" ELF local symbol disappeared, and
> modpost no longer warned about the section mismatch.  The serial
> driver code idiom triggering the warning is standard Linux serial
> driver practice that has a specific whitelist inclusion in modpost.c.
>
> I'm neither a modpost nor an ELF expert, but naively, it doesn't seem
> useful for modpost to report section mismatch warnings caused by ELF
> local symbols by default.  Local symbols have compiler-generated
> names, and thus bypass modpost's whitelisting algorithm, which relies
> on the presence of a non-autogenerated symbol name.  This increases
> the likelihood that false positive warnings will be generated (as in
> the above case).
>
> Thus, disable section mismatch reporting on ELF local symbols.  The
> rationale here is similar to that of commit 2e3a10a1551d ("ARM: avoid
> ARM binutils leaking ELF local symbols") and of similar code already
> present in modpost.c:
>
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/scripts/mod/modpost.c?h=v4.19-rc4=7876320f88802b22d4e2daf7eb027dd14175a0f8#n1256
>
> This third version of the patch implements a suggestion from Masahiro
> Yamada  to restructure the code as an
> additional pattern matching step inside secref_whitelist(), and
> further improves the patch description.
>
> Cc: Russell King 
> Cc: Jim Wilson 
> Cc: Masahiro Yamada 
> Cc: Michal Marek 
> Cc: Sam Ravnborg 
> Cc: linux-kbu...@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> Signed-off-by: Paul Walmsley 
> Signed-off-by: Paul Walmsley 
> ---

Applied with Sam's Ack.

Thanks!


>   scripts/mod/modpost.c | 12 
>   1 file changed, 12 insertions(+)
>
> diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
> index 0d998c54564d..90bb04b4e166 100644
> --- a/scripts/mod/modpost.c
> +++ b/scripts/mod/modpost.c
> @@ -1163,6 +1163,14 @@ static const struct sectioncheck *section_mismatch(
>*   fromsec = text section
>*   refsymname = *.constprop.*
>*
> + * Pattern 6:
> + *   Hide section mismatch warnings for ELF local symbols.  The goal
> + *   is to eliminate false positive modpost warnings caused by
> + *   compiler-generated ELF local symbol names such as ".LANCHOR1".
> + *   Autogenerated symbol names bypass modpost's "Pattern 2"
> + *   whitelisting, which relies on pattern-matching against symbol
> + *   names to work.  (One situation where gcc can autogenerate ELF
> + *   local symbols is when "-fsection-anchors" is used.)
>**/
>   static int secref_whitelist(const struct sectioncheck *mismatch,
> const char *fromsec, const char *fromsym,
> @@ -1201,6 +1209,10 @@ static int secref_whitelist(const struct sectioncheck 
> *mismatch,
> match(fromsym, optim_symbols))
> return 0;
>
> +   /* Check for pattern 6 */
> +   if (strstarts(fromsym, ".L"))
> +   return 0;
> +
> return 1;
>   }
>
> --
> 2.19.1
>
>


-- 
Best Regards
Masahiro Yamada


[PATCH 1/2] modpost: file2alias: go back to simple devtable lookup

2018-11-21 Thread Masahiro Yamada
Commit e49ce14150c6 ("modpost: use linker section to generate table.")
was not so cool as we had expected because it ended up with ugly section
hacks when commit dd2a3acaecd7 ("mod/file2alias: make modpost compile on
darwin again") came in.

Given a certain degree of unknowledge about the link stage of host
programs, I really want to see simple, stupid table lookup so that
this works in the same way regardless of the underlying executable
binary format.

Signed-off-by: Masahiro Yamada 
---

 scripts/mod/file2alias.c | 144 ---
 1 file changed, 49 insertions(+), 95 deletions(-)

diff --git a/scripts/mod/file2alias.c b/scripts/mod/file2alias.c
index 28a6166..7e4aede 100644
--- a/scripts/mod/file2alias.c
+++ b/scripts/mod/file2alias.c
@@ -50,46 +50,6 @@ struct devtable {
void *function;
 };
 
-#define ___cat(a,b) a ## b
-#define __cat(a,b) ___cat(a,b)
-
-/* we need some special handling for this host tool running eventually on
- * Darwin. The Mach-O section handling is a bit different than ELF section
- * handling. The differnces in detail are:
- *  a) we have segments which have sections
- *  b) we need a API call to get the respective section symbols */
-#if defined(__MACH__)
-#include 
-
-#define INIT_SECTION(name)  do {   \
-   unsigned long name ## _len; \
-   char *__cat(pstart_,name) = getsectdata("__TEXT",   \
-   #name, &__cat(name,_len));  \
-   char *__cat(pstop_,name) = __cat(pstart_,name) +\
-   __cat(name, _len);  \
-   __cat(__start_,name) = (void *)__cat(pstart_,name); \
-   __cat(__stop_,name) = (void *)__cat(pstop_,name);   \
-   } while (0)
-#define SECTION(name)   __attribute__((section("__TEXT, " #name)))
-
-struct devtable **__start___devtable, **__stop___devtable;
-#else
-#define INIT_SECTION(name) /* no-op for ELF */
-#define SECTION(name)   __attribute__((section(#name)))
-
-/* We construct a table of pointers in an ELF section (pointers generally
- * go unpadded by gcc).  ld creates boundary syms for us. */
-extern struct devtable *__start___devtable[], *__stop___devtable[];
-#endif /* __MACH__ */
-
-#if !defined(__used)
-# if __GNUC__ == 3 && __GNUC_MINOR__ < 3
-#  define __used   __attribute__((__unused__))
-# else
-#  define __used   __attribute__((__used__))
-# endif
-#endif
-
 /* Define a variable f that holds the value of field f of struct devid
  * based at address m.
  */
@@ -110,16 +70,6 @@ extern struct devtable *__start___devtable[], 
*__stop___devtable[];
 #define DEF_FIELD_ADDR(m, devid, f) \
DEF_FIELD_ADDR_VAR(m, devid, f, f)
 
-/* Add a table entry.  We test function type matches while we're here. */
-#define ADD_TO_DEVTABLE(device_id, type, function) \
-   static struct devtable __cat(devtable,__LINE__) = { \
-   device_id + 0*sizeof((function)((const char *)NULL, \
-   (void *)NULL,   \
-   (char *)NULL)), \
-   SIZE_##type, (function) };  \
-   static struct devtable *SECTION(__devtable) __used \
-   __cat(devtable_ptr,__LINE__) = &__cat(devtable,__LINE__)
-
 #define ADD(str, sep, cond, field)  \
 do {\
 strcat(str, sep);   \
@@ -439,7 +389,6 @@ static int do_hid_entry(const char *filename,
 
return 1;
 }
-ADD_TO_DEVTABLE("hid", hid_device_id, do_hid_entry);
 
 /* Looks like: ieee1394:venNmoNspNverN */
 static int do_ieee1394_entry(const char *filename,
@@ -464,7 +413,6 @@ static int do_ieee1394_entry(const char *filename,
add_wildcard(alias);
return 1;
 }
-ADD_TO_DEVTABLE("ieee1394", ieee1394_device_id, do_ieee1394_entry);
 
 /* Looks like: pci:vNdNsvNsdNbcNscNiN. */
 static int do_pci_entry(const char *filename,
@@ -508,7 +456,6 @@ static int do_pci_entry(const char *filename,
add_wildcard(alias);
return 1;
 }
-ADD_TO_DEVTABLE("pci", pci_device_id, do_pci_entry);
 
 /* looks like: "ccw:tNmNdtNdmN" */
 static int do_ccw_entry(const char *filename,
@@ -532,7 +479,6 @@ static int do_ccw_entry(const char *filename,
add_wildcard(alias);
return 1;
 }
-ADD_TO_DEVTABLE("ccw", ccw_device_id, do_ccw_entry);
 
 /* looks like: "ap:tN" */
 static int do_ap_entry(const char *filename,
@@ -543,7 +489,6 @@ static int do_ap_entry(const char *filename,
sprintf(alias, "ap:t%02X*", dev_type);
return 1;
 }
-ADD_TO_DEVTABLE("ap", ap_device

[PATCH 2/2] modpost: file2alias: check prototype of handler

2018-11-21 Thread Masahiro Yamada
Use specific prototype instead of an opaque pointer so that the
compiler can catch incompatible pointer type.

Signed-off-by: Masahiro Yamada 
---

 scripts/mod/file2alias.c | 7 +++
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/scripts/mod/file2alias.c b/scripts/mod/file2alias.c
index 7e4aede..a37af7d 100644
--- a/scripts/mod/file2alias.c
+++ b/scripts/mod/file2alias.c
@@ -47,7 +47,7 @@ typedef struct {
 struct devtable {
const char *device_id; /* name of table, __mod___*_device_table. 
*/
unsigned long id_size;
-   void *function;
+   int (*do_entry)(const char *filename, void *symval, char *alias);
 };
 
 /* Define a variable f that holds the value of field f of struct devid
@@ -1299,12 +1299,11 @@ static bool sym_is(const char *name, unsigned namelen, 
const char *symbol)
 static void do_table(void *symval, unsigned long size,
 unsigned long id_size,
 const char *device_id,
-void *function,
+int (*do_entry)(const char *filename, void *symval, char 
*alias),
 struct module *mod)
 {
unsigned int i;
char alias[500];
-   int (*do_entry)(const char *, void *entry, char *alias) = function;
 
device_id_check(mod->name, device_id, size, id_size, symval);
/* Leave last one: it's the terminator. */
@@ -1420,7 +1419,7 @@ void handle_moddevtable(struct module *mod, struct 
elf_info *info,
 
if (sym_is(name, namelen, p->device_id)) {
do_table(symval, sym->st_size, p->id_size,
-p->device_id, p->function, mod);
+p->device_id, p->do_entry, mod);
break;
}
}
-- 
2.7.4



[PATCH v4 3/3] build_bug.h: remove most of dummy BUILD_BUG_ON stubs for Sparse

2018-11-21 Thread Masahiro Yamada
The introduction of these dummy BUILD_BUG_ON stubs dates back to
commit 903c0c7cdc21 ("sparse: define dummy BUILD_BUG_ON definition
for sparse").

At that time, BUILD_BUG_ON() was implemented with the negative array
trick *and* the link-time trick, like this:

  extern int __build_bug_on_failed;
  #define BUILD_BUG_ON(condition)\
  do {   \
  ((void)sizeof(char[1 - 2*!!(condition)])); \
  if (condition) __build_bug_on_failed = 1;  \
  } while(0)

Sparse is more strict about the negative array trick than GCC because
Sparse requires the array length to be really constant.

Here is the simple test code for the macro above:

  static const int x = 0;
  BUILD_BUG_ON(x);

GCC is absolutely fine with it (-Wvla was enabled only very recently),
but Sparse warns like this:

  error: bad constant expression
  error: cannot size expression

(If you are using a newer version of Sparse, you will see a different
warning message, "warning: Variable length array is used".)

Anyway, Sparse was producing many false positives, and noisier than
it should be at that time.

With the previous commit, the leftover negative array trick is gone.
Sparse is fine with the current BUILD_BUG_ON(), which is implemented
by using the 'error' attribute.

I am keeping the stub for BUILD_BUG_ON_ZERO(). Otherwise, Sparse
would complain about the following code, which GCC is fine with:

  static const int x = 0;
  int y = BUILD_BUG_ON_ZERO(x);

Signed-off-by: Masahiro Yamada 
Acked-by: Kees Cook 
Reviewed-by: Luc Van Oostenryck 
Reviewed-by: Nick Desaulniers 
Tested-by: Nick Desaulniers 
---

Changes in v4: None
Changes in v3:
 - Add Kees' Acked-by
 - Clarify log that BUILD_BUG_ON() *was* producing false positives
 - Keep the stub for BUILD_BUG_ON_ZERO()

Changes in v2:
 - Fix a coding style error (two consecutive blank lines)

 include/linux/build_bug.h | 22 +++---
 1 file changed, 7 insertions(+), 15 deletions(-)

diff --git a/include/linux/build_bug.h b/include/linux/build_bug.h
index d415c64..faeec74 100644
--- a/include/linux/build_bug.h
+++ b/include/linux/build_bug.h
@@ -5,21 +5,8 @@
 #include 
 
 #ifdef __CHECKER__
-#define __BUILD_BUG_ON_NOT_POWER_OF_2(n) (0)
-#define BUILD_BUG_ON_NOT_POWER_OF_2(n) (0)
 #define BUILD_BUG_ON_ZERO(e) (0)
-#define BUILD_BUG_ON_INVALID(e) (0)
-#define BUILD_BUG_ON_MSG(cond, msg) (0)
-#define BUILD_BUG_ON(condition) (0)
-#define BUILD_BUG() (0)
 #else /* __CHECKER__ */
-
-/* Force a compilation error if a constant expression is not a power of 2 */
-#define __BUILD_BUG_ON_NOT_POWER_OF_2(n)   \
-   BUILD_BUG_ON(((n) & ((n) - 1)) != 0)
-#define BUILD_BUG_ON_NOT_POWER_OF_2(n) \
-   BUILD_BUG_ON((n) == 0 || (((n) & ((n) - 1)) != 0))
-
 /*
  * Force a compilation error if condition is true, but also produce a
  * result (of value 0 and type size_t), so the expression can be used
@@ -27,6 +14,13 @@
  * aren't permitted).
  */
 #define BUILD_BUG_ON_ZERO(e) (sizeof(struct { int:(-!!(e)); }))
+#endif /* __CHECKER__ */
+
+/* Force a compilation error if a constant expression is not a power of 2 */
+#define __BUILD_BUG_ON_NOT_POWER_OF_2(n)   \
+   BUILD_BUG_ON(((n) & ((n) - 1)) != 0)
+#define BUILD_BUG_ON_NOT_POWER_OF_2(n) \
+   BUILD_BUG_ON((n) == 0 || (((n) & ((n) - 1)) != 0))
 
 /*
  * BUILD_BUG_ON_INVALID() permits the compiler to check the validity of the
@@ -64,6 +58,4 @@
  */
 #define BUILD_BUG() BUILD_BUG_ON_MSG(1, "BUILD_BUG failed")
 
-#endif /* __CHECKER__ */
-
 #endif /* _LINUX_BUILD_BUG_H */
-- 
2.7.4



[PATCH v4 2/3] build_bug.h: remove negative-array fallback for BUILD_BUG_ON()

2018-11-21 Thread Masahiro Yamada
The kernel can only be compiled with an optimization option (-O2, -Os,
or the currently proposed -Og). Hence, __OPTIMIZE__ is always defined
in the kernel source.

The fallback for the -O0 case is just hypothetical and pointless.
Moreover, commit 0bb95f80a38f ("Makefile: Globally enable VLA warning")
enabled -Wvla warning. The use of variable length arrays is banned.

Signed-off-by: Masahiro Yamada 
Acked-by: Kees Cook 
Reviewed-by: Nick Desaulniers 
Tested-by: Nick Desaulniers 
---

Changes in v4: None
Changes in v3: None
Changes in v2: None

 include/linux/build_bug.h | 14 --
 1 file changed, 14 deletions(-)

diff --git a/include/linux/build_bug.h b/include/linux/build_bug.h
index 43d1fd5..d415c64 100644
--- a/include/linux/build_bug.h
+++ b/include/linux/build_bug.h
@@ -51,23 +51,9 @@
  * If you have some code which relies on certain constants being equal, or
  * some other compile-time-evaluated condition, you should use BUILD_BUG_ON to
  * detect if someone changes it.
- *
- * The implementation uses gcc's reluctance to create a negative array, but gcc
- * (as of 4.4) only emits that error for obvious cases (e.g. not arguments to
- * inline functions).  Luckily, in 4.3 they added the "error" function
- * attribute just for this type of case.  Thus, we use a negative sized array
- * (should always create an error on gcc versions older than 4.4) and then call
- * an undefined function with the error attribute (should always create an
- * error on gcc 4.3 and later).  If for some reason, neither creates a
- * compile-time error, we'll still have a link-time error, which is harder to
- * track down.
  */
-#ifndef __OPTIMIZE__
-#define BUILD_BUG_ON(condition) ((void)sizeof(char[1 - 2*!!(condition)]))
-#else
 #define BUILD_BUG_ON(condition) \
BUILD_BUG_ON_MSG(condition, "BUILD_BUG_ON failed: " #condition)
-#endif
 
 /**
  * BUILD_BUG - break compile if used.
-- 
2.7.4



[PATCH v4 1/3] kernel.h: disable type-checks in container_of() for Sparse

2018-11-21 Thread Masahiro Yamada
When I tried to enable BUILD_BUG_ON for Sparse, the kbuild test robot
reported lots of "unknown expression" warnings from container_of(),
which seemed false positive.

I addressed this in [1], but fixing Sparse is the right thing to do.

The issue was fixed by Sparse commit 0eb8175d3e9c ("fix expansion of
function designator"), but it will take time until the fixed version
of Sparse is widely available.

Disable the container_of() type checks for Sparse for now.

[1] 
https://lore.kernel.org/lkml/1542623503-3755-1-git-send-email-yamada.masah...@socionext.com/

Signed-off-by: Masahiro Yamada 
---

Changes in v4:
  - New patch

Changes in v3: None
Changes in v2: None

 include/linux/kernel.h | 23 +--
 1 file changed, 17 insertions(+), 6 deletions(-)

diff --git a/include/linux/kernel.h b/include/linux/kernel.h
index d6aac75..d8c4adb 100644
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
@@ -985,6 +985,21 @@ static inline void ftrace_dump(enum ftrace_dump_mode 
oops_dump_mode) { }
 #define __CONCAT(a, b) a ## b
 #define CONCATENATE(a, b) __CONCAT(a, b)
 
+/*
+ * TODO:
+ * Sparse emits "unknown expression" warnings.
+ * It was fixed by commit 0eb8175d3e9c0d20354763d07ce3d4c0e543d988 in Sparse.
+ * Remove the following workaround when the fixed Sparse is widely available.
+ */
+#ifdef __CHECKER__
+#define TYPE_CHECK_CONTAINER_OF(ptr, type, member) do {} while (0)
+#else
+#define TYPE_CHECK_CONTAINER_OF(ptr, type, member) \
+   BUILD_BUG_ON_MSG(!__same_type(*(ptr), ((type *)0)->member) &&   \
+!__same_type(*(ptr), void),\
+"pointer type mismatch in container_of()")
+#endif
+
 /**
  * container_of - cast a member of a structure out to the containing structure
  * @ptr:   the pointer to the member.
@@ -994,9 +1009,7 @@ static inline void ftrace_dump(enum ftrace_dump_mode 
oops_dump_mode) { }
  */
 #define container_of(ptr, type, member) ({ \
void *__mptr = (void *)(ptr);   \
-   BUILD_BUG_ON_MSG(!__same_type(*(ptr), ((type *)0)->member) &&   \
-!__same_type(*(ptr), void),\
-"pointer type mismatch in container_of()");\
+   TYPE_CHECK_CONTAINER_OF(ptr, type, member); \
((type *)(__mptr - offsetof(type, member))); })
 
 /**
@@ -1009,9 +1022,7 @@ static inline void ftrace_dump(enum ftrace_dump_mode 
oops_dump_mode) { }
  */
 #define container_of_safe(ptr, type, member) ({
\
void *__mptr = (void *)(ptr);   \
-   BUILD_BUG_ON_MSG(!__same_type(*(ptr), ((type *)0)->member) &&   \
-!__same_type(*(ptr), void),\
-"pointer type mismatch in container_of()");\
+   TYPE_CHECK_CONTAINER_OF(ptr, type, member); \
IS_ERR_OR_NULL(__mptr) ? ERR_CAST(__mptr) : \
((type *)(__mptr - offsetof(type, member))); })
 
-- 
2.7.4



[PATCH 1/2] kbuild: fix single target build for external module

2018-11-21 Thread Masahiro Yamada
Building a single target in an external module fails due to missing
.tmp_versions directory.

For example,

  $ make -C /lib/modules/$(uname -r)/build M=$PWD foo.o

will fail in the following way:

  CC [M]  /home/masahiro/foo/foo.o
/bin/sh: 1: cannot create /home/masahiro/foo/.tmp_versions/foo.mod: Directory 
nonexistent

This is because $(cmd_crmodverdir) is executed only for /, %/, %.ko
single targets for external modules. Create .tmp_versions in the
'prepare' target.

Signed-off-by: Masahiro Yamada 
---

 Makefile | 11 +++
 1 file changed, 3 insertions(+), 8 deletions(-)

diff --git a/Makefile b/Makefile
index d5da1df..36f3f0e 100644
--- a/Makefile
+++ b/Makefile
@@ -1554,9 +1554,6 @@ else # KBUILD_EXTMOD
 
 # We are always building modules
 KBUILD_MODULES := 1
-PHONY += crmodverdir
-crmodverdir:
-   $(cmd_crmodverdir)
 
 PHONY += $(objtree)/Module.symvers
 $(objtree)/Module.symvers:
@@ -1568,7 +1565,7 @@ $(objtree)/Module.symvers:
 
 module-dirs := $(addprefix _module_,$(KBUILD_EXTMOD))
 PHONY += $(module-dirs) modules
-$(module-dirs): crmodverdir $(objtree)/Module.symvers
+$(module-dirs): prepare $(objtree)/Module.symvers
$(Q)$(MAKE) $(build)=$(patsubst _module_%,%,$@)
 
 modules: $(module-dirs)
@@ -1609,7 +1606,8 @@ help:
 
 # Dummies...
 PHONY += prepare scripts
-prepare: ;
+prepare:
+   $(cmd_crmodverdir)
 scripts: ;
 endif # KBUILD_EXTMOD
 
@@ -1733,17 +1731,14 @@ endif
 
 # Modules
 /: prepare scripts FORCE
-   $(cmd_crmodverdir)
$(Q)$(MAKE) KBUILD_MODULES=$(if $(CONFIG_MODULES),1) \
$(build)=$(build-dir)
 # Make sure the latest headers are built for Documentation
 Documentation/ samples/: headers_install
 %/: prepare scripts FORCE
-   $(cmd_crmodverdir)
$(Q)$(MAKE) KBUILD_MODULES=$(if $(CONFIG_MODULES),1) \
$(build)=$(build-dir)
 %.ko: prepare scripts FORCE
-   $(cmd_crmodverdir)
$(Q)$(MAKE) KBUILD_MODULES=$(if $(CONFIG_MODULES),1)   \
$(build)=$(build-dir) $(@:.ko=.o)
$(Q)$(MAKE) -f $(srctree)/scripts/Makefile.modpost
-- 
2.7.4



[PATCH 2/2] kbuild: remove 'scripts' dummy target for external module build

2018-11-21 Thread Masahiro Yamada
Make simply skips a missing rule as long as it is marked as PHONY.
Remove the dummy target.

Signed-off-by: Masahiro Yamada 
---

 Makefile | 2 --
 1 file changed, 2 deletions(-)

diff --git a/Makefile b/Makefile
index 36f3f0e..269a9bf 100644
--- a/Makefile
+++ b/Makefile
@@ -1604,11 +1604,9 @@ help:
@echo  '  clean   - remove generated files in module directory 
only'
@echo  ''
 
-# Dummies...
 PHONY += prepare scripts
 prepare:
$(cmd_crmodverdir)
-scripts: ;
 endif # KBUILD_EXTMOD
 
 clean: $(clean-dirs)
-- 
2.7.4



Re: [PATCH v2 0/9] kbuild: clean-up modversion, TRIM_UNUSED_KSYMS, if_changed_rule, etc.

2018-11-21 Thread Masahiro Yamada
On Tue, Nov 20, 2018 at 10:11 AM Masahiro Yamada
 wrote:
>
> As a Kbuild maintainer, I always struggle to keep the core makefiles
> clean because people tend to squeeze more and more clutter code into
> the kbuild core in order to do what they want to do.
>
> The biggest step forward in this series is to re-implement
> the build trick of CONFIG_TRIM_UNUSED_KSYMS in a cleaner way.
> scripts/Kbuild.include now looks nice again.
> Also, in my rough estimation, building with CONFIG_TRIM_UNUSED_KSYMS
> became 40-50 % faster.
>
> Besides those, nice cleanups are here and there.
>
> Masahiro Yamada (9):
>   kbuild: let fixdep directly write to .*.cmd files
>   kbuild: remove redundant 'set -e' from filechk_* defines
>   kbuild: remove redundant 'set -e' from sub_cmd_record_mcount
>   kbuild: refactor modversions build rules
>   kbuild: simplify dependency generation for CONFIG_TRIM_UNUSED_KSYMS
>   kbuild: change if_changed_rule for multi-line recipe
>   kbuild: remove trailing semicolon from cmd_* passed to if_changed_rule
>   kbuild: refactor if_changed and if_changed_dep
>   kbuild: remove redundant 'set -e' from cmd_* defines

Series, applied to linux-kbuild.





>  Makefile |  13 +++---
>  arch/um/Makefile |   2 +-
>  include/asm-generic/export.h |  13 +++---
>  include/linux/export.h   |  18 
>  scripts/Kbuild.include   |  49 +++-
>  scripts/Makefile.build   | 105 
> ++-
>  scripts/Makefile.lib |   2 +-
>  scripts/basic/fixdep.c   |  31 ++---
>  scripts/gen_ksymdeps.sh  |  25 +++
>  scripts/package/Makefile |   1 -
>  10 files changed, 106 insertions(+), 153 deletions(-)
>  create mode 100755 scripts/gen_ksymdeps.sh
>
> --
> 2.7.4
>


-- 
Best Regards
Masahiro Yamada


Re: Backed up kernels

2018-11-20 Thread Masahiro Yamada
Hi Jean,


On Tue, Nov 20, 2018 at 10:40 PM Jean Delvare  wrote:
>
> Hi Masahiro, Michal,
>
> When I run "make install", if a kernel by the same version number +
> flavor string already exists, a backup is created with ".old" appended.
> Over time, this adds many entries to my boot menu, makes some package
> updates take much longer (e.g. when all initrds must be regenerated),
> and ultimately confuses grub2, which fails to find the matching modules
> directory under /lib/modules.
>
> You could argue that grub2 could be fixed to find the right modules
> directory, but in fact there is no guarantee that the modules built for
> the new kernel are fully compatible with the old kernel. Keeping a
> backup copy of the old modules is also not possible, because both
> kernels have the same $(uname -r) and therefore the modules of both
> kernels must live under the same /lib/modules/$(uname -r), which
> collides.
>
> Given that, is there really any practical value in saving a backup of
> old kernels? I'm doing kernel development for 15 years and I can't
> remember ever booting one of these ".old" kernels. If my latest
> development kernel doesn't work for any reason, I will just boot back
> to the distribution kernel.
>
> Therefore I am asking, can we change "make install" so that it does NOT
> create a backup copy of an existing kernel?


I think your suggestion makes sense,
but "make install" is basically implemented
by arch-specific shell script.
(For example, arch/x86/boot/install.sh)

Will you talk to the maintainers
of architecture you are interested in?

(or send it to linux-a...@vger.kernel.org)


> Thanks,
> --
> Jean Delvare
> SUSE L3 Support



-- 
Best Regards
Masahiro Yamada


Re: [PATCH] modpost: validate symbol names also in find_elf_symbol

2018-11-20 Thread Masahiro Yamada
On Wed, Oct 24, 2018 at 7:15 AM Sami Tolvanen  wrote:
>
> If an ARM mapping symbol shares an address with a valid symbol,
> find_elf_symbol can currently return the mapping symbol instead, as the
> symbol is not validated. This can result in confusing warnings:
>
>   WARNING: vmlinux.o(.text+0x18f4028): Section mismatch in reference
>   from the function set_reset_devices() to the variable .init.text:$x.0
>
> This change adds a call to is_valid_name to find_elf_symbol, similarly
> to how it's already used in find_elf_symbol2.
>
> Signed-off-by: Sami Tolvanen 
> ---


Applied to linux-kbuild.

Thanks!



>  scripts/mod/modpost.c | 50 ++-
>  1 file changed, 26 insertions(+), 24 deletions(-)
>
> diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
> index 0d998c54564d..b709b2e623d6 100644
> --- a/scripts/mod/modpost.c
> +++ b/scripts/mod/modpost.c
> @@ -1204,6 +1204,30 @@ static int secref_whitelist(const struct sectioncheck 
> *mismatch,
> return 1;
>  }
>
> +static inline int is_arm_mapping_symbol(const char *str)
> +{
> +   return str[0] == '$' && strchr("axtd", str[1])
> +  && (str[2] == '\0' || str[2] == '.');
> +}
> +
> +/*
> + * If there's no name there, ignore it; likewise, ignore it if it's
> + * one of the magic symbols emitted used by current ARM tools.
> + *
> + * Otherwise if find_symbols_between() returns those symbols, they'll
> + * fail the whitelist tests and cause lots of false alarms ... fixable
> + * only by merging __exit and __init sections into __text, bloating
> + * the kernel (which is especially evil on embedded platforms).
> + */
> +static inline int is_valid_name(struct elf_info *elf, Elf_Sym *sym)
> +{
> +   const char *name = elf->strtab + sym->st_name;
> +
> +   if (!name || !strlen(name))
> +   return 0;
> +   return !is_arm_mapping_symbol(name);
> +}
> +
>  /**
>   * Find symbol based on relocation record info.
>   * In some cases the symbol supplied is a valid symbol so
> @@ -1229,6 +1253,8 @@ static Elf_Sym *find_elf_symbol(struct elf_info *elf, 
> Elf64_Sword addr,
> continue;
> if (ELF_ST_TYPE(sym->st_info) == STT_SECTION)
> continue;
> +   if (!is_valid_name(elf, sym))
> +   continue;
> if (sym->st_value == addr)
> return sym;
> /* Find a symbol nearby - addr are maybe negative */
> @@ -1247,30 +1273,6 @@ static Elf_Sym *find_elf_symbol(struct elf_info *elf, 
> Elf64_Sword addr,
> return NULL;
>  }
>
> -static inline int is_arm_mapping_symbol(const char *str)
> -{
> -   return str[0] == '$' && strchr("axtd", str[1])
> -  && (str[2] == '\0' || str[2] == '.');
> -}
> -
> -/*
> - * If there's no name there, ignore it; likewise, ignore it if it's
> - * one of the magic symbols emitted used by current ARM tools.
> - *
> - * Otherwise if find_symbols_between() returns those symbols, they'll
> - * fail the whitelist tests and cause lots of false alarms ... fixable
> - * only by merging __exit and __init sections into __text, bloating
> - * the kernel (which is especially evil on embedded platforms).
> - */
> -static inline int is_valid_name(struct elf_info *elf, Elf_Sym *sym)
> -{
> -   const char *name = elf->strtab + sym->st_name;
> -
> -   if (!name || !strlen(name))
> -   return 0;
> -   return !is_arm_mapping_symbol(name);
> -}
> -
>  /*
>   * Find symbols before or equal addr and after addr - in the section sec.
>   * If we find two symbols with equal offset prefer one with a valid name.
> --
> 2.19.1.568.g152ad8e336-goog
>


-- 
Best Regards
Masahiro Yamada


Re: [PATCH v2 1/2] modpost: drop unused command line switches

2018-11-20 Thread Masahiro Yamada
On Thu, Nov 15, 2018 at 9:56 AM Paul Walmsley  wrote:
>
> Drop modpost command line switches that are no longer used by
> makefile.modpost, upon request from Sam Ravnborg ,
> who wrote:
>
> modpost is not supposed to be used outside the kernel build. [...]
> I checked if there were any options supported by modpost that
> was not configurable in makefile.modpost.
> And I could see that the -M and -K options in getopt() was leftovers.
> The code that used these option was was dropped in:
> commit a8773769d1a1 ("Kbuild: clear marker out of modpost")
>
> Could you add a patch that delete these on top of what you already have.
>
> https://lore.kernel.org/lkml/20181020140835.ga3...@ravnborg.org/
>
> Cc: Sam Ravnborg 
> Cc: Masahiro Yamada 
> Cc: Michal Marek 
> Cc: linux-kbu...@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> Signed-off-by: Paul Walmsley 
> Signed-off-by: Paul Walmsley 
> ---

Applied to linux-kbuild.
Thanks!



>  scripts/mod/modpost.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
> index 0d998c54564d..85bd93c63180 100644
> --- a/scripts/mod/modpost.c
> +++ b/scripts/mod/modpost.c
> @@ -2416,7 +2416,7 @@ int main(int argc, char **argv)
> struct ext_sym_list *extsym_iter;
> struct ext_sym_list *extsym_start = NULL;
>
> -   while ((opt = getopt(argc, argv, "i:I:e:mnsST:o:awM:K:E")) != -1) {
> +   while ((opt = getopt(argc, argv, "i:I:e:mnsST:o:awE")) != -1) {
>         switch (opt) {
> case 'i':
> kernel_read = optarg;
> --
> 2.19.1
>


-- 
Best Regards
Masahiro Yamada


Re: [PATCH v2 2/2] modpost: skip ELF local symbols by default during section mismatch check

2018-11-20 Thread Masahiro Yamada
Hi Paul,


On Thu, Nov 15, 2018 at 9:57 AM Paul Walmsley  wrote:
>
> During development of a serial console driver with a gcc 8.2.0
> toolchain for RISC-V, the following modpost warning appeared:
>
> 
> WARNING: vmlinux.o(.data+0x19b10): Section mismatch in reference from the 
> variable .LANCHOR1 to the function .init.text:sifive_serial_console_setup()
> The variable .LANCHOR1 references
> the function __init sifive_serial_console_setup()
> If the reference is valid then annotate the
> variable with __init* or __refdata (see linux/init.h) or name the variable:
> *_template, *_timer, *_sht, *_ops, *_probe, *_probe_one, *_console
> 
>
> ".LANCHOR1" is an ELF local symbol, automatically created by gcc's section
> anchor generation code:
>
> https://gcc.gnu.org/onlinedocs/gccint/Anchored-Addresses.html
>
> https://gcc.gnu.org/git/?p=gcc.git;a=blob;f=gcc/varasm.c;h=cd9591a45617464946dcf9a126dde277d9de9804;hb=9fb89fa845c1b2e0a18d85ada0b077c84508ab78#l7473
>
> This was verified by compiling the kernel with -fno-section-anchors.
> The serial driver code idiom triggering the warning is standard serial
> driver practice, and one that has a specific whitelist inclusion in
> modpost.c.
>
> I'm neither a modpost nor an ELF expert, but naively, it doesn't seem
> useful for modpost to report section mismatch warnings caused by ELF
> local symbols by default.  Local symbols have compiler-generated
> names, and thus bypass modpost's whitelisting algorithm, which relies
> on the presence of a non-autogenerated symbol name.  This increases
> the likelihood that false positive warnings will be generated (as in
> the above case).
>
> Thus, disable section mismatch reporting on ELF local symbols.  The
> rationale here is similar to that of commit 2e3a10a1551d ("ARM: avoid
> ARM binutils leaking ELF local symbols") and of similar code already
> present in modpost.c:
>
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/scripts/mod/modpost.c?h=v4.19-rc4=7876320f88802b22d4e2daf7eb027dd14175a0f8#n1256
>
> This second version of the patch drops the option to keep section
> mismatch warnings for local sections, based on feedback from Sam
> Ravnborg ; and clarifies that these warnings
> appear with gcc 8.2.0.
>
> Cc: Russell King 
> Cc: Jim Wilson 
> Cc: Masahiro Yamada 
> Cc: Michal Marek 
> Cc: Sam Ravnborg 
> Cc: linux-kbu...@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> Signed-off-by: Paul Walmsley 
> Signed-off-by: Paul Walmsley 
> ---
>  scripts/mod/modpost.c | 14 ++
>  1 file changed, 14 insertions(+)
>
> diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
> index 85bd93c63180..0fb148171b78 100644
> --- a/scripts/mod/modpost.c
> +++ b/scripts/mod/modpost.c
> @@ -1253,6 +1253,17 @@ static inline int is_arm_mapping_symbol(const char 
> *str)
>&& (str[2] == '\0' || str[2] == '.');
>  }
>
> +/*
> + * If a symbol name follows the convention for ELF-local symbols (i.e., the
> + * name begins with a ".L"), return true; otherwise false.  This is used to
> + * skip section mismatch reporting on ELF-local symbols, due to the risk of
> + * false positives.
> + */
> +static inline int is_local_symbol(const char *str)
> +{
> +   return str[0] == '.' && str[1] == 'L';
> +}
> +
>  /*
>   * If there's no name there, ignore it; likewise, ignore it if it's
>   * one of the magic symbols emitted used by current ARM tools.
> @@ -1535,6 +1546,9 @@ static void default_mismatch_handler(const char 
> *modname, struct elf_info *elf,
> if (strstarts(fromsym, "reference___initcall"))
> return;
>
> +   if (is_local_symbol(fromsym))
> +   return;
> +
> tosec = sec_name(elf, get_secindex(elf, sym));
> to = find_elf_symbol(elf, r->r_addend, sym);
> tosym = sym_name(elf, to);
> --
> 2.19.1
>


I think this is almost good.
Just a nit.

Maybe, putting this check in secref_whitelist()
(with a comment "Pattern 6:")
could look more consistency.


Also, if you use strstart() helper,
you can remove is_local_symbol() function.


/* Check for pattern 6 */
if (strstarts(fromsym, ".L"))
return 0;




-- 
Best Regards
Masahiro Yamada


Re: [PATCH v2 1/2] Makefile: Fix distcc compilation with x86 macros

2018-11-20 Thread Masahiro Yamada
On Tue, Nov 20, 2018 at 2:21 AM Nadav Amit  wrote:
>
> at 8:20 PM, Masahiro Yamada  wrote:
>
> > On Sat, Nov 17, 2018 at 6:02 AM Nadav Amit  wrote:
> >> From: Masahiro Yamada
> >> Sent: November 16, 2018 at 7:45:45 AM GMT
> >>> To: Nadav Amit 
> >>> Cc: Ingo Molnar , Michal Marek 
> >>> , Thomas Gleixner , Borislav 
> >>> Petkov , H. Peter Anvin , X86 ML 
> >>> , Linux Kbuild mailing list 
> >>> , Linux Kernel Mailing List 
> >>> 
> >>> Subject: Re: [PATCH v2 1/2] Makefile: Fix distcc compilation with x86 
> >>> macros
> >>>
> >>>
> >>> On Thu, Nov 15, 2018 at 1:01 PM Nadav Amit  wrote:
> >>>> Introducing the use of asm macros in c-code broke distcc, since it only
> >>>> sends the preprocessed source file. The solution is to break the
> >>>> compilation into two separate phases of compilation and assembly, and
> >>>> between the two concatenate the assembly macros and the compiled (yet
> >>>> not assembled) source file. Since this is less efficient, this
> >>>> compilation mode is only used when distcc or icecc are used.
> >>>>
> >>>> Note that the assembly stage should also be distributed, if distcc is
> >>>> configured using "CFLAGS=-DENABLE_REMOTE_ASSEMBLE".
> >>>>
> >>>> Reported-by: Logan Gunthorpe 
> >>>> Signed-off-by: Nadav Amit 
> >>>
> >>>
> >>> Wow, this is so ugly.
> >>
> >> Indeed, it is not pretty from the Makefile system point of view.
> >>
> >> But it does have merits when it comes to the actual use (by developers). If
> >> you look on x86, there are a lot of duplicated implementation for C and Asm
> >> and crazy C macros, for example for PV function call or the alternative
> >> mechanism. The code is also less readable in its current form.
> >>
> >>> I realized how much I hated this by now.
> >>>
> >>> My question is, how long do we need to carry this?
> >>
> >> If it is purely about performance, I am not sure it would make sense to
> >> put it in, as it will indeed be (eventually) solved by the compiler, and
> >> the penalty is not too great.
> >
> >
> > It is unfortunate to mess up the code
> > by having two different ways to achieve the same goal.
>
> Indeed, but I fail to see how this statement fits with the next sentence.

I am not tracking the progress on GCC side.
I just did not sure if the 'asm inline' work was on the right track.


> > When GCC with asm inline support is shipped,
> > would you revert 77b0bf55 ?
>
> This patch and the following ones were not written to be reverted, i.e.,
> reverting them later may not be easy since they remove redundant C inline
> assembly chunks.
>
> Since gcc will solve the inline assembly issues, the value of these patches
> is in unifying the assembly that is used in .c and .S files. Due to the lack
> of m4-like preprocessor in the Linux make-system, the solution is a bit
> “hacky” (in other words, I don’t see a reasonable alternative solution).
>
> So there is a downside in the form of performance degradation of distcc, and
> hacky (not too hacky?) Makefile changes. On the upside, except addressing
> the gcc statement cost computation (inline) issue, the patch removes
> redundant code, and improves assembly code readability.

It is true that the assembly part itself is readable
but the readability as a whole is worse IMHO.

Each macro implementation was split into "asm volatile" (in C) +
".macro" (in ASM) parts.


> In addition, it
> provides the option to make assembly manipulations after compilation, which
> HPA and others (me) look into.
>
>  Anyhow, if after all, the downside is considered greater than the upside,
> it is best to remove the patches sooner than later, as a revert later will
> be more painful.

Then, I'd be happier with the revert now.



--
Best Regards
Masahiro Yamada


  1   2   3   4   5   6   7   8   9   10   >