* Re: [RFC] Increase Minimal GNU Make version for Linux Kernel from 3.80 to 3.81
From: Michal Marek @ 2017-05-03 13:29 UTC (permalink / raw)
To: Masahiro Yamada
Cc: Linus Torvalds, Sam Ravnborg, Linux Kbuild mailing list,
Jonathan Corbet, Linux Kernel Mailing List, Greg Kroah-Hartman,
Andrew Morton, Thierry Reding, Ingo Molnar, Jan Beulich
In-Reply-To: <CAK7LNAT=jbA1GCp1ztoyjF+CxQHGGgOXEVCzr7+AXKaKQX1deg@mail.gmail.com>
On 2017-05-03 08:46, Masahiro Yamada wrote:
> Hello Linus and Kbuild developers.
>
>
> Documentation/process/changes.rst says the minimal version
> of GNU Make is 3.80, but actually building the kernel
> with this version has been broken for a long time.
>
> Specifically, it got broken by commit c8589d1e9e01 (i.e. Linux 3.18).
> Sorry, it's me who broke it.
>
> Here is my excuse:
> - It is almost 3 years since then, but nobody complained about it.
> - GNU Make 3.80 is almost 15 years old.
> (Even GNU Make 3.81 was released in 2006.)
> - People seldom test their makefiles on such old GNU Make version,
> so they often use some features that are not supported by version 3.80.
Agreed. It's not just the kbuild change that broke it, but as you say,
new make features tend creep into random Makefiles. So I'm fine
adjusting the documentation to match reality.
Adding Jan Beulich to Cc, who to fix these cases in the past.
Michal
^ permalink raw reply
* Re: [RFC] Increase Minimal GNU Make version for Linux Kernel from 3.80 to 3.81
From: Greg Kroah-Hartman @ 2017-05-03 12:05 UTC (permalink / raw)
To: Masahiro Yamada
Cc: Linus Torvalds, Sam Ravnborg, Michal Marek,
Linux Kbuild mailing list, Jonathan Corbet,
Linux Kernel Mailing List, Andrew Morton, Thierry Reding,
Ingo Molnar
In-Reply-To: <CAK7LNAT=jbA1GCp1ztoyjF+CxQHGGgOXEVCzr7+AXKaKQX1deg@mail.gmail.com>
On Wed, May 03, 2017 at 03:46:11PM +0900, Masahiro Yamada wrote:
> Hello Linus and Kbuild developers.
>
>
> Documentation/process/changes.rst says the minimal version
> of GNU Make is 3.80, but actually building the kernel
> with this version has been broken for a long time.
>
> Specifically, it got broken by commit c8589d1e9e01 (i.e. Linux 3.18).
> Sorry, it's me who broke it.
Given that no one has noticed since December of 2014, I think we are
safe to bump up the version required for make, as really you did it
already :)
So, no objection from me.
thanks,
greg k-h
^ permalink raw reply
* [RFC] Increase Minimal GNU Make version for Linux Kernel from 3.80 to 3.81
From: Masahiro Yamada @ 2017-05-03 6:46 UTC (permalink / raw)
To: Linus Torvalds, Sam Ravnborg, Michal Marek,
Linux Kbuild mailing list
Cc: Jonathan Corbet, Linux Kernel Mailing List, Greg Kroah-Hartman,
Andrew Morton, Thierry Reding, Ingo Molnar
Hello Linus and Kbuild developers.
Documentation/process/changes.rst says the minimal version
of GNU Make is 3.80, but actually building the kernel
with this version has been broken for a long time.
Specifically, it got broken by commit c8589d1e9e01 (i.e. Linux 3.18).
Sorry, it's me who broke it.
Here is my excuse:
- It is almost 3 years since then, but nobody complained about it.
- GNU Make 3.80 is almost 15 years old.
(Even GNU Make 3.81 was released in 2006.)
- People seldom test their makefiles on such old GNU Make version,
so they often use some features that are not supported by version 3.80.
We would have to make efforts if we wanted to get back
availability of GNU Make 3.80.
[1] multi_depend in scripts/Makefile.lib does not work on GNU Make 3.80
I fiddled with it for a while, but I could not find a workaround,
except reverting the following 4 commits:
221ecca6cafe
022af62d0190
97e3226e6e98
c8589d1e9e01
I do not want to revert them because we would lose many cleanups.
[2] "else ifeq" is not supported by GNU Make 3.80
'make help' on GNU Make 3.80 reports error:
./Documentation/Makefile.sphinx:25: Extraneous text after `else' directive
./Documentation/Makefile.sphinx:31: *** only one `else' per conditional. Stop.
make: *** [help] Error 2
We could rewrite the makefile, but nested if directives
would make the code unreadable.
[3] $(realpath ...) and $(abspath ...) are not supported by GNU Make 3.80
These two functions are only supported by 3.81 or later,
but they are already used here and there.
[4] semi-colon (;) is treated differently in $(warning ...) by GNU Make 3.80
'make ARCH=arm64 defconfig' does not work on GNU Make 3.80
$ make ARCH=arm64 defconfig
arch/arm64/Makefile:43: *** unterminated call to function `warning':
missing `)'. Stop.
I could not find a solution to make it work for both 3.80 and 3.81 (or later).
We would not be able to use a semi-colon in $(warning ...).
From the above, I'd like to propose to increase the minimal version of
GNU Make to 3.81.
Comments are appreciated.
--
Best Regards
Masahiro Yamada
^ permalink raw reply
* Re: [PATCH] Makefile: evaluate LDFLAGS_BUILD_ID only once
From: Masahiro Yamada @ 2017-05-03 4:49 UTC (permalink / raw)
To: Rabin Vincent
Cc: Michal Marek, Linux Kbuild mailing list,
Linux Kernel Mailing List
In-Reply-To: <20170430161600.15845-1-rabin@rab.in>
2017-05-01 1:16 GMT+09:00 Rabin Vincent <rabin@rab.in>:
> Evaluate LDFLAGS_BUILD_ID (which involves invoking the compiler) only
> once instead of over and over.
>
> This provides a ~20% reduction in null build time with x86 allnoconfig:
>
> $ make allnoconfig && make -j8
> $ perf stat -r5 -e sched:sched_process_exec make -j8
> - 2 119 sched:sched_process_exec
> + 1 878 sched:sched_process_exec
>
> - 1,238817018 seconds time elapsed
> + 0,971020553 seconds time elapsed
>
> Signed-off-by: Rabin Vincent <rabin@rab.in>
Applied to linux-kbuild/kbuild. Thanks!
--
Best Regards
Masahiro Yamada
^ permalink raw reply
* Re: [PATCH] kbuild: Do not use hyphen in exported variable name
From: Masahiro Yamada @ 2017-05-03 4:47 UTC (permalink / raw)
To: Ben Hutchings; +Cc: Linux Kbuild mailing list, debian-kernel
In-Reply-To: <1493563778.2564.11.camel@decadent.org.uk>
Hi Ben,
2017-04-30 23:49 GMT+09:00 Ben Hutchings <ben@decadent.org.uk>:
> On Sun, 2017-04-30 at 23:14 +0900, Masahiro Yamada wrote:
>> Hi Ben,
>>
>>
>> > 2017-04-23 16:23 GMT+09:00 Ben Hutchings <ben@decadent.org.uk>:
>> > On Sun, 2017-04-23 at 15:47 +0900, Masahiro Yamada wrote:
>> > [...]
>> > > I tested dtbs_install once again by myself, but
>> > > dtbinst-root is exported to the sub make
>> > > and the vendor directories are created correctly.
>> > >
>> > >
>> > > I checked the debian's forum you gave
>> > > > References: https://bugs.debian.org/833561
>> > >
>> > > In there, you mentioned:
>> > > "This looks like a bug in make, but we can at least work around it by
>> > > using a non-hyphenated variable name."
>> > >
>> > >
>> > > Does this issue happen on a specific Make version?
>> > >
>> > > I tested GNU make 3.81, 3.82, 4.0, 4.1, 4.2,
>> > > but I was not hit by the problem.
>> >
>> > I don't think this is make version dependent. I can't reproduce the
>> > issue today with make 4.1. But I would have been using the same
>> > version in August when I wrote that.
>> >
>> > What more can I say? Clearly the hyphenated variable gets passed to
>> > the sub-make in most cases. But it's not totally reliable because last
>> > year it wasn't working for us.
>> >
>> > > In the last post in the thread, you concluded:
>> > > "We believe that the bug you reported is fixed in the latest version of
>> > > linux, which is due to be installed in the Debian FTP archive."
>> >
>> > I didn't write that, it's a standard message generated for bugs marked
>> > as closed in a package changelog. :-)
>> >
>> > > If so, why is this patch here?
>> > > How is the dtbs_install procedure different in the Debian package?
>> >
>> > This is the patch I applied to the package.
>> >
>>
>> Do you still need this patch for Debian?
> [...]
>
> I don't think so. I just don't know for sure.
>
> Ben.
I postpone this patch for now.
Please re-post if this patch turns out to be really needed.
--
Best Regards
Masahiro Yamada
^ permalink raw reply
* Re: [PATCH 1/1] objtool: make it visible in make V=1 output
From: Masahiro Yamada @ 2017-05-03 4:42 UTC (permalink / raw)
To: Jiri Slaby
Cc: Linux Kernel Mailing List, Michal Marek,
Linux Kbuild mailing list, Josh Poimboeuf
In-Reply-To: <20170421151620.14297-1-jslaby@suse.cz>
2017-04-22 0:16 GMT+09:00 Jiri Slaby <jslaby@suse.cz>:
> It is currently impossible to see what is going on with objtool when
> building, so call echo-cmd to see the actions:
> gcc -Wp,-MD,arch/x86/entry/.entry_64.o.d -nostdinc -isystem ...
> ./tools/objtool/objtool check "arch/x86/entry/entry_64.o";
>
> Signed-off-by: Jiri Slaby <jslaby@suse.cz>
> Cc: Masahiro Yamada <yamada.masahiro@socionext.com>
> Cc: Michal Marek <mmarek@suse.com>
> Cc: linux-kbuild@vger.kernel.org
> Cc: Josh Poimboeuf <jpoimboe@redhat.com>
> ---
Applied to linux-kbuild/kbuild. Thanks!
--
Best Regards
Masahiro Yamada
^ permalink raw reply
* Re: [PATCH 1/2] kbuild: cleanup signing keys with mrproper
From: Masahiro Yamada @ 2017-05-03 4:37 UTC (permalink / raw)
To: Stephen Hemminger
Cc: Michal Marek, Linux Kbuild mailing list,
Linux Kernel Mailing List, Stephen Hemminger, David Woodhouse,
David Howells
In-Reply-To: <20170414215450.18307-2-sthemmin@microsoft.com>
+CC David Woodhouse
+CC David Howells
2017-04-15 6:54 GMT+09:00 Stephen Hemminger <stephen@networkplumber.org>:
> When 'make mrproper' is run it was supposed to remove the signing
> keys in the certs directory, but only the filename is given
> rather than the pathanme which is necessary to cause cleanup.
>
> Signed-off-by: Stephen Hemminger <sthemmin@microsoft.com>
> ---
> Makefile | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index efa267a92ba6..04ca211552f7 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1274,9 +1274,9 @@ MRPROPER_DIRS += include/config usr/include include/generated \
> arch/*/include/generated .tmp_objdiff
> MRPROPER_FILES += .config .config.old .version .old_version \
> Module.symvers tags TAGS cscope* GPATH GTAGS GRTAGS GSYMS \
> - signing_key.pem signing_key.priv signing_key.x509 \
> - x509.genkey extra_certificates signing_key.x509.keyid \
> - signing_key.x509.signer vmlinux-gdb.py
> + certs/signing_key.pem certs/signing_key.priv certs/signing_key.x509 \
> + certs/x509.genkey certs/extra_certificates certs/signing_key.x509.keyid \
> + certs/signing_key.x509.signer vmlinux-gdb.py
>
The logic seems quite simple,
but I am not quite sure which file is still valid?
[1] signing_key.pem - OK, this should be certs/signing_key.pem
and removed by 'make mrproper'
[2] signing_key.priv - deprecated by commit fb1179499134 ?
[3] signing_key.x509 - OK, this should be certs/signing_key.x509
and removed by 'make mrproper'
[4] x509.genkey - this is an intermediate file for generating signing_key.pem,
but unneeded for installing external modules.
Does it make more sense to delete this by 'make clean'?
[5] extra_certificates - I am not sure where this is generated, and used
[6] siging_key.x509.keyid - same as [5]
[7] signing_key.x509.signer - same as [5]
--
Best Regards
Masahiro Yamada
^ permalink raw reply
* Re: [PATCH 2/2] kbuild: remove initramfs cpio with mrproper
From: Masahiro Yamada @ 2017-05-03 4:10 UTC (permalink / raw)
To: Stephen Hemminger
Cc: Michal Marek, Linux Kbuild mailing list,
Linux Kernel Mailing List, Stephen Hemminger
In-Reply-To: <20170414215450.18307-3-sthemmin@microsoft.com>
Hi Stephen,
2017-04-15 6:54 GMT+09:00 Stephen Hemminger <stephen@networkplumber.org>:
> When 'make mrproper' is done, it should also remove the initramfs cpio
> file. I ran into this while doing test build on one machine, followed
> by make mrproper and rsync to a target machine. The build on the target
> machine would succeed but be unbootable because of the bad initramfs.
I think initramfs_data.cpio.* is unneeded for external modules.
So, shouldn't it removed by 'make clean', instead of 'make mrproper'?
> Signed-off-by: Stephen Hemminger <sthemmin@microsoft.com>
> ---
> Makefile | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/Makefile b/Makefile
> index 04ca211552f7..954292695bf6 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1276,7 +1276,8 @@ MRPROPER_FILES += .config .config.old .version .old_version \
> Module.symvers tags TAGS cscope* GPATH GTAGS GRTAGS GSYMS \
> certs/signing_key.pem certs/signing_key.priv certs/signing_key.x509 \
> certs/x509.genkey certs/extra_certificates certs/signing_key.x509.keyid \
> - certs/signing_key.x509.signer vmlinux-gdb.py
> + certs/signing_key.x509.signer vmlinux-gdb.py \
> + usr/initramfs_data.cpio.gz
>
As you see usr/Makefile,
datafile_y = initramfs_data.cpio$(suffix_y)
The suffix could be .gz, .bz2, .xz, etc.
Why only initramfs_data.cpio.gz?
--
Best Regards
Masahiro Yamada
^ permalink raw reply
* Re: [PATCH 1/2] kbuild: clang: Disable 'address-of-packed-member' warning
From: Matthias Kaehlcke @ 2017-05-02 1:23 UTC (permalink / raw)
To: Masahiro Yamada
Cc: Michal Marek, Linux Kbuild mailing list,
Linux Kernel Mailing List, Grant Grundler, Greg Hackmann,
Michael Davidson
In-Reply-To: <CAK7LNASEnzDPQs2AY115sMx-i2NTUu9h9LHv5eN1ygvW81vtFw@mail.gmail.com>
Hi Masahiro,
El Sun, Apr 30, 2017 at 10:59:52PM +0900 Masahiro Yamada ha dit:
> 2017-04-22 6:39 GMT+09:00 Matthias Kaehlcke <mka@chromium.org>:
> > clang generates plenty of these warnings in different parts of the code,
> > to an extent that the warnings are little more than noise. Disable the
> > 'address-of-packed-member' warning.
> >
> > Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
>
>
> As far as I compiled arch/x86/configs/x86_64_defconfig,
> all address-of-packed-member warnings came from the single point:
>
> ./arch/x86/include/asm/processor.h:534:30: warning: taking address of
> packed member 'sp0' of class or structure 'x86_hw_tss' may result in
> an unaligned pointer value [-Waddress-of-packed-member]
> return this_cpu_read_stable(cpu_tss.x86_tss.sp0);
> ^~~~~~~~~~~~~~~~~~~
> ./arch/x86/include/asm/percpu.h:391:59: note: expanded from macro
> 'this_cpu_read_stable'
> #define this_cpu_read_stable(var) percpu_stable_op("mov", var)
> ^~~
> ./arch/x86/include/asm/percpu.h:228:16: note: expanded from macro
> 'percpu_stable_op'
> : "p" (&(var))); \
> ^~~
>
>
>
> For this case, I was able to fix it with the following patch:
>
>
> diff --git a/arch/x86/include/asm/percpu.h b/arch/x86/include/asm/percpu.h
> index 9fa0360..de25d1c 100644
> --- a/arch/x86/include/asm/percpu.h
> +++ b/arch/x86/include/asm/percpu.h
> @@ -211,26 +211,27 @@ do {
> \
> #define percpu_stable_op(op, var) \
> ({ \
> typeof(var) pfo_ret__; \
> + void *__p = &(var); \
> switch (sizeof(var)) { \
> case 1: \
> asm(op "b "__percpu_arg(P1)",%0" \
> : "=q" (pfo_ret__) \
> - : "p" (&(var))); \
> + : "p" (__p)); \
> break; \
> case 2: \
> asm(op "w "__percpu_arg(P1)",%0" \
> : "=r" (pfo_ret__) \
> - : "p" (&(var))); \
> + : "p" (__p)); \
> break; \
> case 4: \
> asm(op "l "__percpu_arg(P1)",%0" \
> : "=r" (pfo_ret__) \
> - : "p" (&(var))); \
> + : "p" (__p)); \
> break; \
> case 8: \
> asm(op "q "__percpu_arg(P1)",%0" \
> : "=r" (pfo_ret__) \
> - : "p" (&(var))); \
> + : "p" (__p)); \
> break; \
> default: __bad_percpu_size(); \
> } \
Thanks for having a look!
It is odd though that you only see warnings from that origin, I
encounter plenty of others with x86_64_defconfig, mostly stemming
from uaccess macros:
kernel/power/user.c:439:35: warning: taking address of packed member
'dev' of class or structure 'compat_resume_swap_area' may result in an
unaligned pointer value [-Waddress-of-packed-member]
err |= get_user(swap_area.dev, &u_swap_area->dev);
^~~~~~~~~~~~~~~~
./arch/x86/include/asm/uaccess.h:168:23: note: expanded from macro 'get_user'
register __inttype(*(ptr)) __val_gu asm("%"_ASM_DX); \
^~~
./arch/x86/include/asm/uaccess.h:132:41: note: expanded from macro '__inttype'
__typeof__(__builtin_choose_expr(sizeof(x) > sizeof(0UL), 0ULL, 0UL))
^
I looked into fixing different cases, but didn't see a clear path
forward since we can't just cast the type away as in your patch above.
> I'd like to see as much effort as possible
> before we decide to hide this kind of warning.
>
> Is it OK with you ?
Sounds good, though I will probably leave this one for someone else :)
Cheers
Matthias
^ permalink raw reply
* [PATCH] Makefile: evaluate LDFLAGS_BUILD_ID only once
From: Rabin Vincent @ 2017-04-30 16:16 UTC (permalink / raw)
To: yamada.masahiro, mmarek; +Cc: linux-kbuild, linux-kernel, Rabin Vincent
Evaluate LDFLAGS_BUILD_ID (which involves invoking the compiler) only
once instead of over and over.
This provides a ~20% reduction in null build time with x86 allnoconfig:
$ make allnoconfig && make -j8
$ perf stat -r5 -e sched:sched_process_exec make -j8
- 2 119 sched:sched_process_exec
+ 1 878 sched:sched_process_exec
- 1,238817018 seconds time elapsed
+ 0,971020553 seconds time elapsed
Signed-off-by: Rabin Vincent <rabin@rab.in>
---
Makefile | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/Makefile b/Makefile
index 779302695453..c78a266594c1 100644
--- a/Makefile
+++ b/Makefile
@@ -815,7 +815,7 @@ KBUILD_AFLAGS += $(ARCH_AFLAGS) $(KAFLAGS)
KBUILD_CFLAGS += $(ARCH_CFLAGS) $(KCFLAGS)
# Use --build-id when available.
-LDFLAGS_BUILD_ID = $(patsubst -Wl$(comma)%,%,\
+LDFLAGS_BUILD_ID := $(patsubst -Wl$(comma)%,%,\
$(call cc-ldoption, -Wl$(comma)--build-id,))
KBUILD_LDFLAGS_MODULE += $(LDFLAGS_BUILD_ID)
LDFLAGS_vmlinux += $(LDFLAGS_BUILD_ID)
--
2.11.0
^ permalink raw reply related
* Re: [PATCH v2] tags: honor COMPILED_SOURCE with apart output directory
From: Masahiro Yamada @ 2017-04-30 14:53 UTC (permalink / raw)
To: Robert Jarzmik
Cc: Michal Marek, Linux Kernel Mailing List,
Linux Kbuild mailing list
In-Reply-To: <87shkwi73g.fsf@belgarion.home>
Hi Robert,
2017-04-26 5:07 GMT+09:00 Robert Jarzmik <robert.jarzmik@free.fr>:
> Masahiro Yamada <yamada.masahiro@socionext.com> writes:
>
>> Hi Robert,
>>> diff --git a/scripts/tags.sh b/scripts/tags.sh
>>> index a2ff3388e5ea..35cb64d5211c 100755
>>> --- a/scripts/tags.sh
>>> +++ b/scripts/tags.sh
>>> @@ -106,7 +106,8 @@ all_compiled_sources()
>>> case "$i" in
>>> *.[cS])
>>> j=${i/\.[cS]/\.o}
>>> - if [ -e $j ]; then
>>> + k="${j#$tree}"
>>> + if [ -e $j -o -e "$k" ]; then
>>
>>
>> Do we need to check both srctree and objtree?
>> I think checking objtree (after $tree is ripped off) is enough.
>
> If I remember correctly, as this goes back a couple of monthes when I made the
> tests of this patch, the srctree is checked for the case when the kernel is
> compiled without O=, and objtree for the case with O=.
I thought of this too, but if O= is given, objects in srctree
should not be checked.
For example, the kernel may be compiled for ARCH=arm with O= first,
then for ARCH=x86 without O= second.
If we include objects from both trees, the generated tag file
will be a mixture of arm and x86.
--
Best Regards
Masahiro Yamada
^ permalink raw reply
* Re: [PATCH] kbuild: Do not use hyphen in exported variable name
From: Ben Hutchings @ 2017-04-30 14:49 UTC (permalink / raw)
To: Masahiro Yamada; +Cc: Linux Kbuild mailing list, debian-kernel
In-Reply-To: <CAK7LNATV4vsczwkntLJPhvPew4NEL1JeckaBxeFk-6Q-Y_VfoQ@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 1936 bytes --]
On Sun, 2017-04-30 at 23:14 +0900, Masahiro Yamada wrote:
> Hi Ben,
>
>
> > 2017-04-23 16:23 GMT+09:00 Ben Hutchings <ben@decadent.org.uk>:
> > On Sun, 2017-04-23 at 15:47 +0900, Masahiro Yamada wrote:
> > [...]
> > > I tested dtbs_install once again by myself, but
> > > dtbinst-root is exported to the sub make
> > > and the vendor directories are created correctly.
> > >
> > >
> > > I checked the debian's forum you gave
> > > > References: https://bugs.debian.org/833561
> > >
> > > In there, you mentioned:
> > > "This looks like a bug in make, but we can at least work around it by
> > > using a non-hyphenated variable name."
> > >
> > >
> > > Does this issue happen on a specific Make version?
> > >
> > > I tested GNU make 3.81, 3.82, 4.0, 4.1, 4.2,
> > > but I was not hit by the problem.
> >
> > I don't think this is make version dependent. I can't reproduce the
> > issue today with make 4.1. But I would have been using the same
> > version in August when I wrote that.
> >
> > What more can I say? Clearly the hyphenated variable gets passed to
> > the sub-make in most cases. But it's not totally reliable because last
> > year it wasn't working for us.
> >
> > > In the last post in the thread, you concluded:
> > > "We believe that the bug you reported is fixed in the latest version of
> > > linux, which is due to be installed in the Debian FTP archive."
> >
> > I didn't write that, it's a standard message generated for bugs marked
> > as closed in a package changelog. :-)
> >
> > > If so, why is this patch here?
> > > How is the dtbs_install procedure different in the Debian package?
> >
> > This is the patch I applied to the package.
> >
>
> Do you still need this patch for Debian?
[...]
I don't think so. I just don't know for sure.
Ben.
--
Ben Hutchings
This sentence contradicts itself - no actually it doesn't.
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH] kbuild: Do not use hyphen in exported variable name
From: Masahiro Yamada @ 2017-04-30 14:14 UTC (permalink / raw)
To: Ben Hutchings; +Cc: Linux Kbuild mailing list, debian-kernel
In-Reply-To: <1492932183.31767.64.camel@decadent.org.uk>
Hi Ben,
2017-04-23 16:23 GMT+09:00 Ben Hutchings <ben@decadent.org.uk>:
> On Sun, 2017-04-23 at 15:47 +0900, Masahiro Yamada wrote:
> [...]
>> I tested dtbs_install once again by myself, but
>> dtbinst-root is exported to the sub make
>> and the vendor directories are created correctly.
>>
>>
>> I checked the debian's forum you gave
>> > References: https://bugs.debian.org/833561
>>
>> In there, you mentioned:
>> "This looks like a bug in make, but we can at least work around it by
>> using a non-hyphenated variable name."
>>
>>
>> Does this issue happen on a specific Make version?
>>
>> I tested GNU make 3.81, 3.82, 4.0, 4.1, 4.2,
>> but I was not hit by the problem.
>
> I don't think this is make version dependent. I can't reproduce the
> issue today with make 4.1. But I would have been using the same
> version in August when I wrote that.
>
> What more can I say? Clearly the hyphenated variable gets passed to
> the sub-make in most cases. But it's not totally reliable because last
> year it wasn't working for us.
>
>> In the last post in the thread, you concluded:
>> "We believe that the bug you reported is fixed in the latest version of
>> linux, which is due to be installed in the Debian FTP archive."
>
> I didn't write that, it's a standard message generated for bugs marked
> as closed in a package changelog. :-)
>
>> If so, why is this patch here?
>> How is the dtbs_install procedure different in the Debian package?
>
> This is the patch I applied to the package.
>
Do you still need this patch for Debian?
If so, I can pick it up because it is apparently harmless
(maybe safer as you said).
However, I may add the following excuse in the git-log:
[masahiro
I cound not reproduce the issue, but at least this patch is harmless.]
--
Best Regards
Masahiro Yamada
^ permalink raw reply
* Re: [PATCH 2/2] kbuild: clang: Disable the 'duplicate-decl-specifier' warning
From: Masahiro Yamada @ 2017-04-30 14:01 UTC (permalink / raw)
To: Matthias Kaehlcke
Cc: Michal Marek, Linux Kbuild mailing list,
Linux Kernel Mailing List, Grant Grundler, Greg Hackmann,
Michael Davidson, masahiroy
In-Reply-To: <20170421213931.155210-3-mka@chromium.org>
Hi Matthias,
2017-04-22 6:39 GMT+09:00 Matthias Kaehlcke <mka@chromium.org>:
> clang generates plenty of these warnings in different parts of the code.
> They are mostly caused by container_of() and other macros which declare
> a "const <type> *" variable for their internal use which triggers a
> "duplicate 'const' specifier" warning if the <type> is already const
> qualified.
>
> Wording-mostly-from: Michael Davidson <md@google.com>
> Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
I think container_of() can be more simple,
dropping the 'const'.
The following patch worked for me.
diff --git a/include/linux/kernel.h b/include/linux/kernel.h
index 4c26dc3..d53672b 100644
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
@@ -846,11 +846,9 @@ static inline void ftrace_dump(enum
ftrace_dump_mode oops_dump_mode) { }
* @ptr: the pointer to the member.
* @type: the type of the container struct this is embedded in.
* @member: the name of the member within the struct.
- *
*/
#define container_of(ptr, type, member) ({ \
- const typeof( ((type *)0)->member ) *__mptr = (ptr); \
- (type *)( (char *)__mptr - offsetof(type,member) );})
+ (type *)((void *)(ptr) - offsetof(type, member));})
/* Rebuild everything on CONFIG_FTRACE_MCOUNT_RECORD */
#ifdef CONFIG_FTRACE_MCOUNT_RECORD
For also this one,
I'd like to try to fix the code rather than hiding warnings.
(We may end up with applying your patch after all,
but I'd like to think about it for now.)
--
Best Regards
Masahiro Yamada
^ permalink raw reply related
* Re: [PATCH 1/2] kbuild: clang: Disable 'address-of-packed-member' warning
From: Masahiro Yamada @ 2017-04-30 13:59 UTC (permalink / raw)
To: Matthias Kaehlcke
Cc: Michal Marek, Linux Kbuild mailing list,
Linux Kernel Mailing List, Grant Grundler, Greg Hackmann,
Michael Davidson
In-Reply-To: <20170421213931.155210-2-mka@chromium.org>
Hi Matthias,
2017-04-22 6:39 GMT+09:00 Matthias Kaehlcke <mka@chromium.org>:
> clang generates plenty of these warnings in different parts of the code,
> to an extent that the warnings are little more than noise. Disable the
> 'address-of-packed-member' warning.
>
> Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
As far as I compiled arch/x86/configs/x86_64_defconfig,
all address-of-packed-member warnings came from the single point:
./arch/x86/include/asm/processor.h:534:30: warning: taking address of
packed member 'sp0' of class or structure 'x86_hw_tss' may result in
an unaligned pointer value [-Waddress-of-packed-member]
return this_cpu_read_stable(cpu_tss.x86_tss.sp0);
^~~~~~~~~~~~~~~~~~~
./arch/x86/include/asm/percpu.h:391:59: note: expanded from macro
'this_cpu_read_stable'
#define this_cpu_read_stable(var) percpu_stable_op("mov", var)
^~~
./arch/x86/include/asm/percpu.h:228:16: note: expanded from macro
'percpu_stable_op'
: "p" (&(var))); \
^~~
For this case, I was able to fix it with the following patch:
diff --git a/arch/x86/include/asm/percpu.h b/arch/x86/include/asm/percpu.h
index 9fa0360..de25d1c 100644
--- a/arch/x86/include/asm/percpu.h
+++ b/arch/x86/include/asm/percpu.h
@@ -211,26 +211,27 @@ do {
\
#define percpu_stable_op(op, var) \
({ \
typeof(var) pfo_ret__; \
+ void *__p = &(var); \
switch (sizeof(var)) { \
case 1: \
asm(op "b "__percpu_arg(P1)",%0" \
: "=q" (pfo_ret__) \
- : "p" (&(var))); \
+ : "p" (__p)); \
break; \
case 2: \
asm(op "w "__percpu_arg(P1)",%0" \
: "=r" (pfo_ret__) \
- : "p" (&(var))); \
+ : "p" (__p)); \
break; \
case 4: \
asm(op "l "__percpu_arg(P1)",%0" \
: "=r" (pfo_ret__) \
- : "p" (&(var))); \
+ : "p" (__p)); \
break; \
case 8: \
asm(op "q "__percpu_arg(P1)",%0" \
: "=r" (pfo_ret__) \
- : "p" (&(var))); \
+ : "p" (__p)); \
break; \
default: __bad_percpu_size(); \
} \
I'd like to see as much effort as possible
before we decide to hide this kind of warning.
Is it OK with you ?
--
Best Regards
Masahiro Yamada
^ permalink raw reply related
* Re: [PATCH v4 2/2] of: Add unit tests for applying overlays
From: Geert Uytterhoeven @ 2017-04-28 16:13 UTC (permalink / raw)
To: Frank Rowand
Cc: Rob Herring, stephen.boyd, Michal Marek,
devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-kbuild
In-Reply-To: <59035E93.2060106@gmail.com>
Hi Frank,
On Fri, Apr 28, 2017 at 5:24 PM, Frank Rowand <frowand.list@gmail.com> wrote:
> On 04/28/17 04:25, Geert Uytterhoeven wrote:
>> On Wed, Apr 26, 2017 at 2:09 AM, <frowand.list@gmail.com> wrote:
>>> create mode 100644 drivers/of/unittest-data/overlay.dts
>>> create mode 100644 drivers/of/unittest-data/overlay_bad_phandle.dts
>>> create mode 100644 drivers/of/unittest-data/overlay_base.dts
>>
>> Shouldn't these be called .dtso instead of .dts?
>
> That is a good question. I'm not worried about solving it this week for
> this patch, because this could turn into a bikeshed and I can always
> fix it with a patch if we decide to change. But if we do want to change
> the naming, I would like to make the decision in the next couple of
> months. I would like to see more progress on overlays in general
> this summer, and plan to be working on them myself.
>
> I _think_ there has been some discussion about source file naming on the
> devicetree-compiler or devicetree list in the far distant past. Or I
> may just be mis-remembering.
>
> As far as I know, the current dtc does not know any suffixes other than
> .dts for source files. Not that the compiler has to know, we can always
> specify '-I dts'.
That's not an obstacle, though. I've been compiling .dtso files into .dtbo
files for several years, cfr.
https://git.kernel.org/pub/scm/linux/kernel/git/geert/renesas-drivers.git/log/?h=topic/renesas-overlays
According to "make V=1", the kernel build system basically uses
./scripts/dtc/dtc -O dtb -o <file>.dtbo <file>.dtbo.dts.tmp
with <file>.dtbo.dts.tmp the preprocessed .dtso files.
And dtc copes fine with that.
>>> --- a/drivers/of/unittest-data/Makefile
>>> +++ b/drivers/of/unittest-data/Makefile
>>
>>> +# enable creation of __symbols__ node
>>> +DTC_FLAGS_overlay := -@
>>> +DTC_FLAGS_overlay_bad_phandle := -@
>>> +DTC_FLAGS_overlay_base := -@
>>
>> This flag is needed for all DTs that will be involved with overlays.
>>
>> Hence what about enabling this globally instead, cfr. "Enable DT symbols when"
>> CONFIG_OF_OVERLAY is used
>> ("http://www.spinics.net/lists/devicetree/msg103363.html")?
>
> And another really good question.
>
> There are some issues. I have thought about it enough to know there are issues,
> but do not have a solution and do not think I know all the issues. Some
> possible issues (or perceived issues) are:
>
> - The size of __symbols__ in an FDT (akd compile .dtb image) in either a kernel
> image or a bootloader if overlays are not actually needed on a given system
> (even if the system is physically capable of using overlays).
What do you mean with "physically capable of using overlays"?
The presence of expansion connectors, like Raspberry Pi and BeagleBone Black?
Even lacking such connectors, an overlay could add hardware description that
was just missing (forgotten, or lack of DT bindings) when the main DTB was
built.
I agree about the size, but you never know in advance if an overlay will
be used or needed later.
> - The size of __symbols__ in kernel memory if overlays are not actually needed
> on a given system (even if the system is physically capable of using overlays.)
> This could be possibly be enabled/disabled by a boot command, even if
> __symbols__ is in the FDT.
Agreed. Not allowing overlays is akin to not allowing to load (more) kernel
modules, and may increase security.
> - A base FDT might want to have __symbols__ included with the expectation that
> overlays will be used in the future. (The FDT might be built for the boot
> loader, then be stable for many kernel releases.)
>
> - Should the creation of __symbols__ be a global switch, or should it be
> controlled on a per dtb basis? Or a combination of both?
So you want to decouple OF overlay support in the Linux kernel from
__symbols__ present in the DTBs built?
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* Re: [PATCH v4 2/2] of: Add unit tests for applying overlays
From: Frank Rowand @ 2017-04-28 15:24 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Rob Herring, stephen.boyd, Michal Marek,
devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-kbuild
In-Reply-To: <CAMuHMdWLhLqjNgz6iUOY8LY1Jjn3-_iV2ncbOxYLCLZbf=Rz+w@mail.gmail.com>
On 04/28/17 04:25, Geert Uytterhoeven wrote:
> Hi Frank,
>
> On Wed, Apr 26, 2017 at 2:09 AM, <frowand.list@gmail.com> wrote:
>> From: Frank Rowand <frank.rowand@sony.com>
>>
>> Existing overlay unit tests examine individual pieces of the overlay
>> code. The new tests target the entire process of applying an overlay.
>>
>> Signed-off-by: Frank Rowand <frank.rowand@sony.com>
>> ---
>>
>> There are checkpatch warnings. I have reviewed them and feel they
>> can be ignored.
>>
>> drivers/of/fdt.c | 14 +-
>> drivers/of/of_private.h | 12 +
>> drivers/of/unittest-data/Makefile | 17 +-
>> drivers/of/unittest-data/overlay.dts | 53 ++++
>> drivers/of/unittest-data/overlay_bad_phandle.dts | 20 ++
>> drivers/of/unittest-data/overlay_base.dts | 80 ++++++
>> drivers/of/unittest.c | 317 +++++++++++++++++++++++
>> 7 files changed, 505 insertions(+), 8 deletions(-)
>>
>> create mode 100644 drivers/of/unittest-data/overlay.dts
>> create mode 100644 drivers/of/unittest-data/overlay_bad_phandle.dts
>> create mode 100644 drivers/of/unittest-data/overlay_base.dts
>
> Shouldn't these be called .dtso instead of .dts?
That is a good question. I'm not worried about solving it this week for
this patch, because this could turn into a bikeshed and I can always
fix it with a patch if we decide to change. But if we do want to change
the naming, I would like to make the decision in the next couple of
months. I would like to see more progress on overlays in general
this summer, and plan to be working on them myself.
I _think_ there has been some discussion about source file naming on the
devicetree-compiler or devicetree list in the far distant past. Or I
may just be mis-remembering.
As far as I know, the current dtc does not know any suffixes other than
.dts for source files. Not that the compiler has to know, we can always
specify '-I dts'.
>
>> --- a/drivers/of/unittest-data/Makefile
>> +++ b/drivers/of/unittest-data/Makefile
>
>> +# enable creation of __symbols__ node
>> +DTC_FLAGS_overlay := -@
>> +DTC_FLAGS_overlay_bad_phandle := -@
>> +DTC_FLAGS_overlay_base := -@
>
> This flag is needed for all DTs that will be involved with overlays.
>
> Hence what about enabling this globally instead, cfr. "Enable DT symbols when"
> CONFIG_OF_OVERLAY is used
> ("http://www.spinics.net/lists/devicetree/msg103363.html")?
And another really good question.
There are some issues. I have thought about it enough to know there are issues,
but do not have a solution and do not think I know all the issues. Some
possible issues (or perceived issues) are:
- The size of __symbols__ in an FDT (akd compile .dtb image) in either a kernel
image or a bootloader if overlays are not actually needed on a given system
(even if the system is physically capable of using overlays).
- The size of __symbols__ in kernel memory if overlays are not actually needed
on a given system (even if the system is physically capable of using overlays.)
This could be possibly be enabled/disabled by a boot command, even if
__symbols__ is in the FDT.
- A base FDT might want to have __symbols__ included with the expectation that
overlays will be used in the future. (The FDT might be built for the boot
loader, then be stable for many kernel releases.)
- Should the creation of __symbols__ be a global switch, or should it be
controlled on a per dtb basis? Or a combination of both?
Again, I'm not worried about an immediate, this week solution, but I would
like to make good progress on this in the next couple of months.
-Frank
>
> Gr{oetje,eeting}s,
>
> Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
>
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
> -- Linus Torvalds
>
^ permalink raw reply
* Re: [PATCH v4 2/2] of: Add unit tests for applying overlays
From: Geert Uytterhoeven @ 2017-04-28 11:25 UTC (permalink / raw)
To: Frank Rowand
Cc: Rob Herring, stephen.boyd, Michal Marek,
devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-kbuild
In-Reply-To: <1493165394-29367-3-git-send-email-frowand.list@gmail.com>
Hi Frank,
On Wed, Apr 26, 2017 at 2:09 AM, <frowand.list@gmail.com> wrote:
> From: Frank Rowand <frank.rowand@sony.com>
>
> Existing overlay unit tests examine individual pieces of the overlay
> code. The new tests target the entire process of applying an overlay.
>
> Signed-off-by: Frank Rowand <frank.rowand@sony.com>
> ---
>
> There are checkpatch warnings. I have reviewed them and feel they
> can be ignored.
>
> drivers/of/fdt.c | 14 +-
> drivers/of/of_private.h | 12 +
> drivers/of/unittest-data/Makefile | 17 +-
> drivers/of/unittest-data/overlay.dts | 53 ++++
> drivers/of/unittest-data/overlay_bad_phandle.dts | 20 ++
> drivers/of/unittest-data/overlay_base.dts | 80 ++++++
> drivers/of/unittest.c | 317 +++++++++++++++++++++++
> 7 files changed, 505 insertions(+), 8 deletions(-)
>
> create mode 100644 drivers/of/unittest-data/overlay.dts
> create mode 100644 drivers/of/unittest-data/overlay_bad_phandle.dts
> create mode 100644 drivers/of/unittest-data/overlay_base.dts
Shouldn't these be called .dtso instead of .dts?
> --- a/drivers/of/unittest-data/Makefile
> +++ b/drivers/of/unittest-data/Makefile
> +# enable creation of __symbols__ node
> +DTC_FLAGS_overlay := -@
> +DTC_FLAGS_overlay_bad_phandle := -@
> +DTC_FLAGS_overlay_base := -@
This flag is needed for all DTs that will be involved with overlays.
Hence what about enabling this globally instead, cfr. "Enable DT symbols when"
CONFIG_OF_OVERLAY is used
("http://www.spinics.net/lists/devicetree/msg103363.html")?
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* Re: [PATCH v2] kbuild: clang: add -no-integrated-as to KBUILD_[AC]FLAGS
From: Masahiro Yamada @ 2017-04-28 0:36 UTC (permalink / raw)
To: Matthias Kaehlcke
Cc: Michal Marek, Michael Davidson, Linux Kbuild mailing list,
Linux Kernel Mailing List, Grant Grundler, Greg Hackmann,
Saleem Abdulrasool
In-Reply-To: <20170425224735.131835-1-mka@chromium.org>
2017-04-26 7:47 GMT+09:00 Matthias Kaehlcke <mka@chromium.org>:
> From: Michael Davidson <md@google.com>
>
> The Linux Kernel relies on GCC's acceptance of inline assembly as an
> opaque object which will not have any validation performed on the content.
> The current behaviour in LLVM is to perform validation of the contents by
> means of parsing the input if the MC layer can handle it.
>
> Disable clangs integrated assembler and use the GNU assembler instead.
>
> Wording-mostly-from: Saleem Abdulrasool <compnerd@compnerd.org>
> Signed-off-by: Michael Davidson <md@google.com>
> Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
> ---
Applied to linux-kbuild/kbuild. Thanks!
--
Best Regards
Masahiro Yamada
^ permalink raw reply
* Re: [PATCH v4 2/2] of: Add unit tests for applying overlays
From: Rob Herring @ 2017-04-27 22:26 UTC (permalink / raw)
To: frowand.list; +Cc: stephen.boyd, mmarek, devicetree, linux-kernel, linux-kbuild
In-Reply-To: <1493165394-29367-3-git-send-email-frowand.list@gmail.com>
On Tue, Apr 25, 2017 at 05:09:54PM -0700, frowand.list@gmail.com wrote:
> From: Frank Rowand <frank.rowand@sony.com>
>
> Existing overlay unit tests examine individual pieces of the overlay
> code. The new tests target the entire process of applying an overlay.
>
> Signed-off-by: Frank Rowand <frank.rowand@sony.com>
> ---
>
> There are checkpatch warnings. I have reviewed them and feel they
> can be ignored.
>
> drivers/of/fdt.c | 14 +-
> drivers/of/of_private.h | 12 +
> drivers/of/unittest-data/Makefile | 17 +-
> drivers/of/unittest-data/overlay.dts | 53 ++++
> drivers/of/unittest-data/overlay_bad_phandle.dts | 20 ++
> drivers/of/unittest-data/overlay_base.dts | 80 ++++++
> drivers/of/unittest.c | 317 +++++++++++++++++++++++
> 7 files changed, 505 insertions(+), 8 deletions(-)
> create mode 100644 drivers/of/unittest-data/overlay.dts
> create mode 100644 drivers/of/unittest-data/overlay_bad_phandle.dts
> create mode 100644 drivers/of/unittest-data/overlay_base.dts
Applied.
Rob
^ permalink raw reply
* Re: [PATCH v4 1/2] of: per-file dtc compiler flags
From: Rob Herring @ 2017-04-27 22:25 UTC (permalink / raw)
To: frowand.list; +Cc: stephen.boyd, mmarek, devicetree, linux-kernel, linux-kbuild
In-Reply-To: <1493165394-29367-2-git-send-email-frowand.list@gmail.com>
On Tue, Apr 25, 2017 at 05:09:53PM -0700, frowand.list@gmail.com wrote:
> From: Frank Rowand <frank.rowand@sony.com>
>
> The dtc compiler version that adds initial support was available
> in 4.11-rc1. Add the ability to set an additional dtc compiler
> flag is needed by overlays.
>
> Signed-off-by: Frank Rowand <frank.rowand@sony.com>
> ---
> scripts/Makefile.lib | 2 ++
> 1 file changed, 2 insertions(+)
Applied.
^ permalink raw reply
* Re: [PATCH v4 1/2] of: per-file dtc compiler flags
From: Masahiro Yamada @ 2017-04-27 15:09 UTC (permalink / raw)
To: Frank Rowand
Cc: Rob Herring, Stephen Boyd, Michal Marek, devicetree,
Linux Kernel Mailing List, Linux Kbuild mailing list
In-Reply-To: <1493165394-29367-2-git-send-email-frowand.list@gmail.com>
2017-04-26 9:09 GMT+09:00 <frowand.list@gmail.com>:
> From: Frank Rowand <frank.rowand@sony.com>
>
> The dtc compiler version that adds initial support was available
> in 4.11-rc1. Add the ability to set an additional dtc compiler
> flag is needed by overlays.
>
> Signed-off-by: Frank Rowand <frank.rowand@sony.com>
> ---
Acked-by: Masahiro Yamada <yamada.masahiro@socionext.com>
--
Best Regards
Masahiro Yamada
^ permalink raw reply
* Re: [PATCH v2 1/2] of: support dtc compiler flags for overlays
From: Frank Rowand @ 2017-04-26 0:10 UTC (permalink / raw)
To: Masahiro Yamada
Cc: Rob Herring, Stephen Boyd, Michal Marek, devicetree,
Linux Kernel Mailing List, Linux Kbuild mailing list
In-Reply-To: <CAK7LNARd-XsCfSJhDj3JisH=u4XZm8DaPEa5Pg2ciWrq8Ueing@mail.gmail.com>
On 04/25/17 15:04, Masahiro Yamada wrote:
> 2017-04-25 8:05 GMT+09:00 <frowand.list@gmail.com>:
>> From: Frank Rowand <frank.rowand@sony.com>
>>
>> The dtc compiler version that adds initial support was available
>> in 4.11-rc1. Add the ability to set the dtc compiler flags needed
>> by overlays.
>>
>> Signed-off-by: Frank Rowand <frank.rowand@sony.com>
>> ---
>
>
> I know your motivation for 1/2 is overlay,
> but this patch itself is more generic.
> (support for per-file dtc compiler flag)
>
> Could you reword the commit subject a little bit?
>
> Otherwise,
>
> Acked-by: Masahiro Yamada <yamada.masahiro@socionext.com>
Fixed in version 4.
-Frank
>
>
>
>> scripts/Makefile.lib | 2 ++
>> 1 file changed, 2 insertions(+)
>>
>> diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
>> index 0a07f9014944..0bbec480d323 100644
>> --- a/scripts/Makefile.lib
>> +++ b/scripts/Makefile.lib
>> @@ -283,6 +283,8 @@ ifeq ($(KBUILD_ENABLE_EXTRA_GCC_CHECKS),)
>> DTC_FLAGS += -Wno-unit_address_vs_reg
>> endif
>>
>> +DTC_FLAGS += $(DTC_FLAGS_$(basetarget))
>> +
>> # Generate an assembly file to wrap the output of the device tree compiler
>> quiet_cmd_dt_S_dtb= DTB $@
>> cmd_dt_S_dtb= \
>> --
>
>
^ permalink raw reply
* [PATCH v4 2/2] of: Add unit tests for applying overlays
From: frowand.list @ 2017-04-26 0:09 UTC (permalink / raw)
To: Rob Herring, stephen.boyd, mmarek; +Cc: devicetree, linux-kernel, linux-kbuild
In-Reply-To: <1493165394-29367-1-git-send-email-frowand.list@gmail.com>
From: Frank Rowand <frank.rowand@sony.com>
Existing overlay unit tests examine individual pieces of the overlay
code. The new tests target the entire process of applying an overlay.
Signed-off-by: Frank Rowand <frank.rowand@sony.com>
---
There are checkpatch warnings. I have reviewed them and feel they
can be ignored.
drivers/of/fdt.c | 14 +-
drivers/of/of_private.h | 12 +
drivers/of/unittest-data/Makefile | 17 +-
drivers/of/unittest-data/overlay.dts | 53 ++++
drivers/of/unittest-data/overlay_bad_phandle.dts | 20 ++
drivers/of/unittest-data/overlay_base.dts | 80 ++++++
drivers/of/unittest.c | 317 +++++++++++++++++++++++
7 files changed, 505 insertions(+), 8 deletions(-)
create mode 100644 drivers/of/unittest-data/overlay.dts
create mode 100644 drivers/of/unittest-data/overlay_bad_phandle.dts
create mode 100644 drivers/of/unittest-data/overlay_base.dts
diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
index e5ce4b59e162..e33f7818bc6c 100644
--- a/drivers/of/fdt.c
+++ b/drivers/of/fdt.c
@@ -31,6 +31,8 @@
#include <asm/setup.h> /* for COMMAND_LINE_SIZE */
#include <asm/page.h>
+#include "of_private.h"
+
/*
* of_fdt_limit_memory - limit the number of regions in the /memory node
* @limit: maximum entries
@@ -469,11 +471,11 @@ static int unflatten_dt_nodes(const void *blob,
* Returns NULL on failure or the memory chunk containing the unflattened
* device tree on success.
*/
-static void *__unflatten_device_tree(const void *blob,
- struct device_node *dad,
- struct device_node **mynodes,
- void *(*dt_alloc)(u64 size, u64 align),
- bool detached)
+void *__unflatten_device_tree(const void *blob,
+ struct device_node *dad,
+ struct device_node **mynodes,
+ void *(*dt_alloc)(u64 size, u64 align),
+ bool detached)
{
int size;
void *mem;
@@ -1261,6 +1263,8 @@ void __init unflatten_device_tree(void)
/* Get pointer to "/chosen" and "/aliases" nodes for use everywhere */
of_alias_scan(early_init_dt_alloc_memory_arch);
+
+ unittest_unflatten_overlay_base();
}
/**
diff --git a/drivers/of/of_private.h b/drivers/of/of_private.h
index 18bbb4517e25..de5c604f5cc4 100644
--- a/drivers/of/of_private.h
+++ b/drivers/of/of_private.h
@@ -55,6 +55,18 @@ static inline int of_property_notify(int action, struct device_node *np,
}
#endif /* CONFIG_OF_DYNAMIC */
+#ifdef CONFIG_OF_UNITTEST
+extern void __init unittest_unflatten_overlay_base(void);
+#else
+static inline void unittest_unflatten_overlay_base(void) {};
+#endif
+
+extern void *__unflatten_device_tree(const void *blob,
+ struct device_node *dad,
+ struct device_node **mynodes,
+ void *(*dt_alloc)(u64 size, u64 align),
+ bool detached);
+
/**
* General utilities for working with live trees.
*
diff --git a/drivers/of/unittest-data/Makefile b/drivers/of/unittest-data/Makefile
index 1ac5cc01d627..6e00a9c69e58 100644
--- a/drivers/of/unittest-data/Makefile
+++ b/drivers/of/unittest-data/Makefile
@@ -1,7 +1,18 @@
obj-y += testcases.dtb.o
+obj-y += overlay.dtb.o
+obj-y += overlay_bad_phandle.dtb.o
+obj-y += overlay_base.dtb.o
targets += testcases.dtb testcases.dtb.S
+targets += overlay.dtb overlay.dtb.S
+targets += overlay_bad_phandle.dtb overlay_bad_phandle.dtb.S
+targets += overlay_base.dtb overlay_base.dtb.S
-.SECONDARY: \
- $(obj)/testcases.dtb.S \
- $(obj)/testcases.dtb
+.PRECIOUS: \
+ $(obj)/%.dtb.S \
+ $(obj)/%.dtb
+
+# enable creation of __symbols__ node
+DTC_FLAGS_overlay := -@
+DTC_FLAGS_overlay_bad_phandle := -@
+DTC_FLAGS_overlay_base := -@
diff --git a/drivers/of/unittest-data/overlay.dts b/drivers/of/unittest-data/overlay.dts
new file mode 100644
index 000000000000..6cd7e6a0c13e
--- /dev/null
+++ b/drivers/of/unittest-data/overlay.dts
@@ -0,0 +1,53 @@
+/dts-v1/;
+/plugin/;
+
+/ {
+
+ fragment@0 {
+ target = <&electric_1>;
+
+ __overlay__ {
+ status = "ok";
+
+ hvac_2: hvac-large-1 {
+ compatible = "ot,hvac-large";
+ heat-range = < 40 75 >;
+ cool-range = < 65 80 >;
+ };
+ };
+ };
+
+ fragment@1 {
+ target = <&rides_1>;
+
+ __overlay__ {
+ #address-cells = <1>;
+ #size-cells = <1>;
+ status = "ok";
+
+ ride@200 {
+ compatible = "ot,ferris-wheel";
+ reg = < 0x00000200 0x100 >;
+ hvac-provider = < &hvac_2 >;
+ hvac-thermostat = < 27 32 > ;
+ hvac-zones = < 12 5 >;
+ hvac-zone-names = "operator", "snack-bar";
+ spin-controller = < &spin_ctrl_1 3 >;
+ spin-rph = < 30 >;
+ gondolas = < 16 >;
+ gondola-capacity = < 6 >;
+ };
+ };
+ };
+
+ fragment@2 {
+ target = <&lights_2>;
+
+ __overlay__ {
+ status = "ok";
+ color = "purple", "white", "red", "green";
+ rate = < 3 256 >;
+ };
+ };
+
+};
diff --git a/drivers/of/unittest-data/overlay_bad_phandle.dts b/drivers/of/unittest-data/overlay_bad_phandle.dts
new file mode 100644
index 000000000000..270ee885a623
--- /dev/null
+++ b/drivers/of/unittest-data/overlay_bad_phandle.dts
@@ -0,0 +1,20 @@
+/dts-v1/;
+/plugin/;
+
+/ {
+
+ fragment@0 {
+ target = <&electric_1>;
+
+ __overlay__ {
+
+ // This label should cause an error when the overlay
+ // is applied. There is already a phandle value
+ // in the base tree for motor-1.
+ spin_ctrl_1_conflict: motor-1 {
+ accelerate = < 3 >;
+ decelerate = < 5 >;
+ };
+ };
+ };
+};
diff --git a/drivers/of/unittest-data/overlay_base.dts b/drivers/of/unittest-data/overlay_base.dts
new file mode 100644
index 000000000000..5566b27fb61a
--- /dev/null
+++ b/drivers/of/unittest-data/overlay_base.dts
@@ -0,0 +1,80 @@
+/dts-v1/;
+/plugin/;
+
+/*
+ * Base device tree that overlays will be applied against.
+ *
+ * Do not add any properties in node "/".
+ * Do not add any nodes other than "/testcase-data-2" in node "/".
+ * Do not add anything that would result in dtc creating node "/__fixups__".
+ * dtc will create nodes "/__symbols__" and "/__local_fixups__".
+ */
+
+/ {
+ testcase-data-2 {
+ #address-cells = <1>;
+ #size-cells = <1>;
+
+ electric_1: substation@100 {
+ compatible = "ot,big-volts-control";
+ reg = < 0x00000100 0x100 >;
+ status = "disabled";
+
+ hvac_1: hvac-medium-1 {
+ compatible = "ot,hvac-medium";
+ heat-range = < 50 75 >;
+ cool-range = < 60 80 >;
+ };
+
+ spin_ctrl_1: motor-1 {
+ compatible = "ot,ferris-wheel-motor";
+ spin = "clockwise";
+ };
+
+ spin_ctrl_2: motor-8 {
+ compatible = "ot,roller-coaster-motor";
+ };
+ };
+
+ rides_1: fairway-1 {
+ #address-cells = <1>;
+ #size-cells = <1>;
+ compatible = "ot,rides";
+ status = "disabled";
+ orientation = < 127 >;
+
+ ride@100 {
+ compatible = "ot,roller-coaster";
+ reg = < 0x00000100 0x100 >;
+ hvac-provider = < &hvac_1 >;
+ hvac-thermostat = < 29 > ;
+ hvac-zones = < 14 >;
+ hvac-zone-names = "operator";
+ spin-controller = < &spin_ctrl_2 5 &spin_ctrl_2 7 >;
+ spin-controller-names = "track_1", "track_2";
+ queues = < 2 >;
+ };
+ };
+
+ lights_1: lights@30000 {
+ compatible = "ot,work-lights";
+ reg = < 0x00030000 0x1000 >;
+ status = "disabled";
+ };
+
+ lights_2: lights@40000 {
+ compatible = "ot,show-lights";
+ reg = < 0x00040000 0x1000 >;
+ status = "disabled";
+ rate = < 13 138 >;
+ };
+
+ retail_1: vending@50000 {
+ reg = < 0x00050000 0x1000 >;
+ compatible = "ot,tickets";
+ status = "disabled";
+ };
+
+ };
+};
+
diff --git a/drivers/of/unittest.c b/drivers/of/unittest.c
index 62db55b97c10..12597ff8cfb0 100644
--- a/drivers/of/unittest.c
+++ b/drivers/of/unittest.c
@@ -8,6 +8,7 @@
#include <linux/err.h>
#include <linux/errno.h>
#include <linux/hashtable.h>
+#include <linux/libfdt.h>
#include <linux/of.h>
#include <linux/of_fdt.h>
#include <linux/of_irq.h>
@@ -1925,6 +1926,320 @@ static void __init of_unittest_overlay(void)
static inline void __init of_unittest_overlay(void) { }
#endif
+/*
+ * __dtb_ot_begin[] and __dtb_ot_end[] are created by cmd_dt_S_dtb
+ * in scripts/Makefile.lib
+ */
+
+#define OVERLAY_INFO_EXTERN(name) \
+ extern uint8_t __dtb_##name##_begin[]; \
+ extern uint8_t __dtb_##name##_end[]
+
+#define OVERLAY_INFO(name, expected) \
+{ .dtb_begin = __dtb_##name##_begin, \
+ .dtb_end = __dtb_##name##_end, \
+ .expected_result = expected, \
+}
+
+struct overlay_info {
+ uint8_t *dtb_begin;
+ uint8_t *dtb_end;
+ void *data;
+ struct device_node *np_overlay;
+ int expected_result;
+ int overlay_id;
+};
+
+OVERLAY_INFO_EXTERN(overlay_base);
+OVERLAY_INFO_EXTERN(overlay);
+OVERLAY_INFO_EXTERN(overlay_bad_phandle);
+
+#ifdef CONFIG_OF_OVERLAY
+
+/* order of entries is hard-coded into users of overlays[] */
+static struct overlay_info overlays[] = {
+ OVERLAY_INFO(overlay_base, -9999),
+ OVERLAY_INFO(overlay, 0),
+ OVERLAY_INFO(overlay_bad_phandle, -EINVAL),
+ {}
+};
+
+static struct device_node *overlay_base_root;
+
+/*
+ * Create base device tree for the overlay unittest.
+ *
+ * This is called from very early boot code.
+ *
+ * Do as much as possible the same way as done in __unflatten_device_tree
+ * and other early boot steps for the normal FDT so that the overlay base
+ * unflattened tree will have the same characteristics as the real tree
+ * (such as having memory allocated by the early allocator). The goal
+ * is to test "the real thing" as much as possible, and test "test setup
+ * code" as little as possible.
+ *
+ * Have to stop before resolving phandles, because that uses kmalloc.
+ */
+void __init unittest_unflatten_overlay_base(void)
+{
+ struct overlay_info *info;
+ u32 data_size;
+ u32 size;
+
+ info = &overlays[0];
+
+ if (info->expected_result != -9999) {
+ pr_err("No dtb 'overlay_base' to attach\n");
+ return;
+ }
+
+ data_size = info->dtb_end - info->dtb_begin;
+ if (!data_size) {
+ pr_err("No dtb 'overlay_base' to attach\n");
+ return;
+ }
+
+ size = fdt_totalsize(info->dtb_begin);
+ if (size != data_size) {
+ pr_err("dtb 'overlay_base' header totalsize != actual size");
+ return;
+ }
+
+ info->data = early_init_dt_alloc_memory_arch(size,
+ roundup_pow_of_two(FDT_V17_SIZE));
+ if (!info->data) {
+ pr_err("alloc for dtb 'overlay_base' failed");
+ return;
+ }
+
+ memcpy(info->data, info->dtb_begin, size);
+
+ __unflatten_device_tree(info->data, NULL, &info->np_overlay,
+ early_init_dt_alloc_memory_arch, true);
+ overlay_base_root = info->np_overlay;
+}
+
+/*
+ * The purpose of of_unittest_overlay_data_add is to add an
+ * overlay in the normal fashion. This is a test of the whole
+ * picture, instead of testing individual elements.
+ *
+ * A secondary purpose is to be able to verify that the contents of
+ * /proc/device-tree/ contains the updated structure and values from
+ * the overlay. That must be verified separately in user space.
+ *
+ * Return 0 on unexpected error.
+ */
+static int __init overlay_data_add(int onum)
+{
+ struct overlay_info *info;
+ int k;
+ int ret;
+ u32 size;
+ u32 size_from_header;
+
+ for (k = 0, info = overlays; info; info++, k++) {
+ if (k == onum)
+ break;
+ }
+ if (onum > k)
+ return 0;
+
+ size = info->dtb_end - info->dtb_begin;
+ if (!size) {
+ pr_err("no overlay to attach, %d\n", onum);
+ ret = 0;
+ }
+
+ size_from_header = fdt_totalsize(info->dtb_begin);
+ if (size_from_header != size) {
+ pr_err("overlay header totalsize != actual size, %d", onum);
+ return 0;
+ }
+
+ /*
+ * Must create permanent copy of FDT because of_fdt_unflatten_tree()
+ * will create pointers to the passed in FDT in the EDT.
+ */
+ info->data = kmemdup(info->dtb_begin, size, GFP_KERNEL);
+ if (!info->data) {
+ pr_err("unable to allocate memory for data, %d\n", onum);
+ return 0;
+ }
+
+ of_fdt_unflatten_tree(info->data, NULL, &info->np_overlay);
+ if (!info->np_overlay) {
+ pr_err("unable to unflatten overlay, %d\n", onum);
+ ret = 0;
+ goto out_free_data;
+ }
+ of_node_set_flag(info->np_overlay, OF_DETACHED);
+
+ ret = of_resolve_phandles(info->np_overlay);
+ if (ret) {
+ pr_err("resolve ot phandles (ret=%d), %d\n", ret, onum);
+ goto out_free_np_overlay;
+ }
+
+ ret = of_overlay_create(info->np_overlay);
+ if (ret < 0) {
+ pr_err("of_overlay_create() (ret=%d), %d\n", ret, onum);
+ goto out_free_np_overlay;
+ } else {
+ info->overlay_id = ret;
+ ret = 0;
+ }
+
+ pr_debug("__dtb_overlay_begin applied, overlay id %d\n", ret);
+
+ goto out;
+
+out_free_np_overlay:
+ /*
+ * info->np_overlay is the unflattened device tree
+ * It has not been spliced into the live tree.
+ */
+
+ /* todo: function to free unflattened device tree */
+
+out_free_data:
+ kfree(info->data);
+
+out:
+ return (ret == info->expected_result);
+}
+
+/*
+ * The purpose of of_unittest_overlay_high_level is to add an overlay
+ * in the normal fashion. This is a test of the whole picture,
+ * instead of individual elements.
+ *
+ * The first part of the function is _not_ normal overlay usage; it is
+ * finishing splicing the base overlay device tree into the live tree.
+ */
+static __init void of_unittest_overlay_high_level(void)
+{
+ struct device_node *last_sibling;
+ struct device_node *np;
+ struct device_node *of_symbols;
+ struct device_node *overlay_base_symbols;
+ struct device_node **pprev;
+ struct property *prop;
+ int ret;
+
+ if (!overlay_base_root) {
+ unittest(0, "overlay_base_root not initialized\n");
+ return;
+ }
+
+ /*
+ * Could not fixup phandles in unittest_unflatten_overlay_base()
+ * because kmalloc() was not yet available.
+ */
+ of_resolve_phandles(overlay_base_root);
+
+ /*
+ * do not allow overlay_base to duplicate any node already in
+ * tree, this greatly simplifies the code
+ */
+
+ /*
+ * remove overlay_base_root node "__local_fixups", after
+ * being used by of_resolve_phandles()
+ */
+ pprev = &overlay_base_root->child;
+ for (np = overlay_base_root->child; np; np = np->sibling) {
+ if (!of_node_cmp(np->name, "__local_fixups__")) {
+ *pprev = np->sibling;
+ break;
+ }
+ pprev = &np->sibling;
+ }
+
+ /* remove overlay_base_root node "__symbols__" if in live tree */
+ of_symbols = of_get_child_by_name(of_root, "__symbols__");
+ if (of_symbols) {
+ /* will have to graft properties from node into live tree */
+ pprev = &overlay_base_root->child;
+ for (np = overlay_base_root->child; np; np = np->sibling) {
+ if (!of_node_cmp(np->name, "__symbols__")) {
+ overlay_base_symbols = np;
+ *pprev = np->sibling;
+ break;
+ }
+ pprev = &np->sibling;
+ }
+ }
+
+ for (np = overlay_base_root->child; np; np = np->sibling) {
+ if (of_get_child_by_name(of_root, np->name)) {
+ unittest(0, "illegal node name in overlay_base %s",
+ np->name);
+ return;
+ }
+ }
+
+ /*
+ * overlay 'overlay_base' is not allowed to have root
+ * properties, so only need to splice nodes into main device tree.
+ *
+ * root node of *overlay_base_root will not be freed, it is lost
+ * memory.
+ */
+
+ for (np = overlay_base_root->child; np; np = np->sibling)
+ np->parent = of_root;
+
+ mutex_lock(&of_mutex);
+
+ for (np = of_root->child; np; np = np->sibling)
+ last_sibling = np;
+
+ if (last_sibling)
+ last_sibling->sibling = overlay_base_root->child;
+ else
+ of_root->child = overlay_base_root->child;
+
+ for_each_of_allnodes_from(overlay_base_root, np)
+ __of_attach_node_sysfs(np);
+
+ if (of_symbols) {
+ for_each_property_of_node(overlay_base_symbols, prop) {
+ ret = __of_add_property(of_symbols, prop);
+ if (ret) {
+ unittest(0,
+ "duplicate property '%s' in overlay_base node __symbols__",
+ prop->name);
+ return;
+ }
+ ret = __of_add_property_sysfs(of_symbols, prop);
+ if (ret) {
+ unittest(0,
+ "unable to add property '%s' in overlay_base node __symbols__ to sysfs",
+ prop->name);
+ return;
+ }
+ }
+ }
+
+ mutex_unlock(&of_mutex);
+
+
+ /* now do the normal overlay usage test */
+
+ unittest(overlay_data_add(1),
+ "Adding overlay 'overlay' failed\n");
+
+ unittest(overlay_data_add(2),
+ "Adding overlay 'overlay_bad_phandle' failed\n");
+}
+
+#else
+
+static inline __init void of_unittest_overlay_high_level(void) {}
+
+#endif
+
static int __init of_unittest(void)
{
struct device_node *np;
@@ -1962,6 +2277,8 @@ static int __init of_unittest(void)
/* Double check linkage after removing testcase data */
of_unittest_check_tree_linkage();
+ of_unittest_overlay_high_level();
+
pr_info("end of unittest - %i passed, %i failed\n",
unittest_results.passed, unittest_results.failed);
--
Frank Rowand <frank.rowand@sony.com>
^ permalink raw reply related
* [PATCH v4 1/2] of: per-file dtc compiler flags
From: frowand.list @ 2017-04-26 0:09 UTC (permalink / raw)
To: Rob Herring, stephen.boyd, mmarek; +Cc: devicetree, linux-kernel, linux-kbuild
In-Reply-To: <1493165394-29367-1-git-send-email-frowand.list@gmail.com>
From: Frank Rowand <frank.rowand@sony.com>
The dtc compiler version that adds initial support was available
in 4.11-rc1. Add the ability to set an additional dtc compiler
flag is needed by overlays.
Signed-off-by: Frank Rowand <frank.rowand@sony.com>
---
scripts/Makefile.lib | 2 ++
1 file changed, 2 insertions(+)
diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
index 0a07f9014944..0bbec480d323 100644
--- a/scripts/Makefile.lib
+++ b/scripts/Makefile.lib
@@ -283,6 +283,8 @@ ifeq ($(KBUILD_ENABLE_EXTRA_GCC_CHECKS),)
DTC_FLAGS += -Wno-unit_address_vs_reg
endif
+DTC_FLAGS += $(DTC_FLAGS_$(basetarget))
+
# Generate an assembly file to wrap the output of the device tree compiler
quiet_cmd_dt_S_dtb= DTB $@
cmd_dt_S_dtb= \
--
Frank Rowand <frank.rowand@sony.com>
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox