2019-05-17 07:56:13

by Masahiro Yamada

Subject: [PATCH 1/4] s390: do not pass $(LINUXINCLUDE) to gen_opcode_table.c

I guess HOSTCFLAGS_gen_opcode_table.o was blindly copied from
HOSTCFLAGS_gen_facilities.o

The reason of adding $(LINUXINCLUDE) to HOSTCFLAGS_gen_facilities.o
is because gen_facilities.c references some CONFIG options. (Kbuild
does not cater to this for host tools automatically.)

On the other hand, gen_opcode_table.c does not reference CONFIG
options at all. So, there is no good reason to pass $(LINUXINCLUDE).

Signed-off-by: Masahiro Yamada <[email protected]>
---

arch/s390/tools/Makefile | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/s390/tools/Makefile b/arch/s390/tools/Makefile
index 2342b84b3386..4ff6a2124522 100644
--- a/arch/s390/tools/Makefile
+++ b/arch/s390/tools/Makefile
@@ -15,7 +15,7 @@ hostprogs-y += gen_facilities
hostprogs-y += gen_opcode_table

HOSTCFLAGS_gen_facilities.o += -Wall $(LINUXINCLUDE)
-HOSTCFLAGS_gen_opcode_table.o += -Wall $(LINUXINCLUDE)
+HOSTCFLAGS_gen_opcode_table.o += -Wall

# Ensure output directory exists
_dummy := $(shell [ -d '$(kapi)' ] || mkdir -p '$(kapi)')
--
2.17.1

2019-05-17 07:57:15

by Masahiro Yamada

Subject: [PATCH 3/4] s390: drop redundant directory creation from tools Makefile

As you can see in scripts/Kbuild.include, the filechk creates the
parent directory of the target as needed.

This Makefile does not need to explicitly create the directory.

Signed-off-by: Masahiro Yamada <[email protected]>
---

arch/s390/tools/Makefile | 3 ---
1 file changed, 3 deletions(-)

diff --git a/arch/s390/tools/Makefile b/arch/s390/tools/Makefile
index 8fb66c99840a..4864ea5e6ceb 100644
--- a/arch/s390/tools/Makefile
+++ b/arch/s390/tools/Makefile
@@ -16,9 +16,6 @@ hostprogs-y += gen_opcode_table

HOSTCFLAGS_gen_facilities.o += $(LINUXINCLUDE)

-# Ensure output directory exists
-_dummy := $(shell [ -d '$(kapi)' ] || mkdir -p '$(kapi)')
-
filechk_facility-defs.h = $(obj)/gen_facilities

filechk_dis-defs.h = \
--
2.17.1

2019-05-17 09:08:20

by Masahiro Yamada

Subject: [PATCH 2/4] s390: drop unneeded -Wall addition from tools Makefile

The top level Makefile adds -Wall globally for all host tools:

KBUILD_HOSTCFLAGS := -Wall -Wmissing-prototypes -Wstrict-prototypes -O2 \

I see two "-Wall" added for compiling these tools.

Of course, it is allowed to pass the same option multiple times, but
we do not need to do so.

Signed-off-by: Masahiro Yamada <[email protected]>
---

arch/s390/tools/Makefile | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/arch/s390/tools/Makefile b/arch/s390/tools/Makefile
index 4ff6a2124522..8fb66c99840a 100644
--- a/arch/s390/tools/Makefile
+++ b/arch/s390/tools/Makefile
@@ -14,8 +14,7 @@ kapi: $(kapi-hdrs-y)
hostprogs-y += gen_facilities
hostprogs-y += gen_opcode_table

-HOSTCFLAGS_gen_facilities.o += -Wall $(LINUXINCLUDE)
-HOSTCFLAGS_gen_opcode_table.o += -Wall
+HOSTCFLAGS_gen_facilities.o += $(LINUXINCLUDE)

# Ensure output directory exists
_dummy := $(shell [ -d '$(kapi)' ] || mkdir -p '$(kapi)')
--
2.17.1

2019-05-17 09:26:18

by Masahiro Yamada

Subject: [PATCH 4/4] s390: drop meaningless 'targets' from tools Makefile

'targets' should be specified to include .*.cmd files to evaluate
if_changed or friends.

Here, facility-defs.h and dis-defs.h are generated by filechk.

Because filechk does not generate .*.cmd file, the 'targets' addition
is meaningless. The filechk correctly updates the target when its
content is changed.

Signed-off-by: Masahiro Yamada <[email protected]>
---

arch/s390/tools/Makefile | 1 -
1 file changed, 1 deletion(-)

diff --git a/arch/s390/tools/Makefile b/arch/s390/tools/Makefile
index 4864ea5e6ceb..b5e35e8f999a 100644
--- a/arch/s390/tools/Makefile
+++ b/arch/s390/tools/Makefile
@@ -6,7 +6,6 @@
kapi := arch/$(ARCH)/include/generated/asm
kapi-hdrs-y := $(kapi)/facility-defs.h $(kapi)/dis-defs.h

-targets += $(addprefix ../../../,$(kapi-hdrs-y))
PHONY += kapi

kapi: $(kapi-hdrs-y)
--
2.17.1

2019-06-01 10:23:35

by Heiko Carstens

Subject: Re: [PATCH 1/4] s390: do not pass $(LINUXINCLUDE) to gen_opcode_table.c

On Fri, May 17, 2019 at 04:54:24PM +0900, Masahiro Yamada wrote:
> I guess HOSTCFLAGS_gen_opcode_table.o was blindly copied from
> HOSTCFLAGS_gen_facilities.o
>
> The reason of adding $(LINUXINCLUDE) to HOSTCFLAGS_gen_facilities.o
> is because gen_facilities.c references some CONFIG options. (Kbuild
> does not cater to this for host tools automatically.)
>
> On the other hand, gen_opcode_table.c does not reference CONFIG
> options at all. So, there is no good reason to pass $(LINUXINCLUDE).
>
> Signed-off-by: Masahiro Yamada <[email protected]>
> ---
>
> arch/s390/tools/Makefile | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/s390/tools/Makefile b/arch/s390/tools/Makefile
> index 2342b84b3386..4ff6a2124522 100644
> --- a/arch/s390/tools/Makefile
> +++ b/arch/s390/tools/Makefile
> @@ -15,7 +15,7 @@ hostprogs-y += gen_facilities
> hostprogs-y += gen_opcode_table
>
> HOSTCFLAGS_gen_facilities.o += -Wall $(LINUXINCLUDE)
> -HOSTCFLAGS_gen_opcode_table.o += -Wall $(LINUXINCLUDE)
> +HOSTCFLAGS_gen_opcode_table.o += -Wall
>
> # Ensure output directory exists
> _dummy := $(shell [ -d '$(kapi)' ] || mkdir -p '$(kapi)')

Applied, thanks.

2019-06-01 10:23:35

by Heiko Carstens

Subject: Re: [PATCH 2/4] s390: drop unneeded -Wall addition from tools Makefile

On Fri, May 17, 2019 at 04:54:25PM +0900, Masahiro Yamada wrote:
> The top level Makefile adds -Wall globally for all host tools:
>
> KBUILD_HOSTCFLAGS := -Wall -Wmissing-prototypes -Wstrict-prototypes -O2 \
>
> I see two "-Wall" added for compiling these tools.
>
> Of course, it is allowed to pass the same option multiple times, but
> we do not need to do so.
>
> Signed-off-by: Masahiro Yamada <[email protected]>
> ---
>
> arch/s390/tools/Makefile | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/arch/s390/tools/Makefile b/arch/s390/tools/Makefile
> index 4ff6a2124522..8fb66c99840a 100644
> --- a/arch/s390/tools/Makefile
> +++ b/arch/s390/tools/Makefile
> @@ -14,8 +14,7 @@ kapi: $(kapi-hdrs-y)
> hostprogs-y += gen_facilities
> hostprogs-y += gen_opcode_table
>
> -HOSTCFLAGS_gen_facilities.o += -Wall $(LINUXINCLUDE)
> -HOSTCFLAGS_gen_opcode_table.o += -Wall
> +HOSTCFLAGS_gen_facilities.o += $(LINUXINCLUDE)

Applied, thanks.

2019-06-01 10:24:15

by Heiko Carstens

Subject: Re: [PATCH 3/4] s390: drop redundant directory creation from tools Makefile

On Fri, May 17, 2019 at 04:54:26PM +0900, Masahiro Yamada wrote:
> As you can see in scripts/Kbuild.include, the filechk creates the
> parent directory of the target as needed.
>
> This Makefile does not need to explicitly create the directory.
>
> Signed-off-by: Masahiro Yamada <[email protected]>
> ---
>
> arch/s390/tools/Makefile | 3 ---
> 1 file changed, 3 deletions(-)
>
> diff --git a/arch/s390/tools/Makefile b/arch/s390/tools/Makefile
> index 8fb66c99840a..4864ea5e6ceb 100644
> --- a/arch/s390/tools/Makefile
> +++ b/arch/s390/tools/Makefile
> @@ -16,9 +16,6 @@ hostprogs-y += gen_opcode_table
>
> HOSTCFLAGS_gen_facilities.o += $(LINUXINCLUDE)
>
> -# Ensure output directory exists
> -_dummy := $(shell [ -d '$(kapi)' ] || mkdir -p '$(kapi)')
> -

Applied, thanks.

2019-06-01 10:25:19

by Heiko Carstens

Subject: Re: [PATCH 4/4] s390: drop meaningless 'targets' from tools Makefile

On Fri, May 17, 2019 at 04:54:27PM +0900, Masahiro Yamada wrote:
> 'targets' should be specified to include .*.cmd files to evaluate
> if_changed or friends.
>
> Here, facility-defs.h and dis-defs.h are generated by filechk.
>
> Because filechk does not generate .*.cmd file, the 'targets' addition
> is meaningless. The filechk correctly updates the target when its
> content is changed.
>
> Signed-off-by: Masahiro Yamada <[email protected]>
> ---
>
> arch/s390/tools/Makefile | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/arch/s390/tools/Makefile b/arch/s390/tools/Makefile
> index 4864ea5e6ceb..b5e35e8f999a 100644
> --- a/arch/s390/tools/Makefile
> +++ b/arch/s390/tools/Makefile
> @@ -6,7 +6,6 @@
> kapi := arch/$(ARCH)/include/generated/asm
> kapi-hdrs-y := $(kapi)/facility-defs.h $(kapi)/dis-defs.h
>
> -targets += $(addprefix ../../../,$(kapi-hdrs-y))

Applied, thanks.