[PATCH 1/2] s390: delete unneeded #include <linux/kconfig.h> from facilities_src.h

[PATCH 1/2] s390: delete unneeded #include <linux/kconfig.h> from facilities_src.h

[Masahiro Yamada]

The header facilities_src.h is only included from gen_facilities.c
and the tool is compiled with the following extra options:

    HOSTCFLAGS_gen_facilities.o += -Wall $(LINUXINCLUDE)

Please note $(LINUXINCLUDE) is expanded into build options including:

    -include $(srctree)/include/linux/kconfig.h

So, the Makefile always forces the tool to include kconfig.h, i.e.,
the #include <linux/kconfig.h> directive in the header is redundant.

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

 arch/s390/include/asm/facilities_src.h | 2 --
 1 file changed, 2 deletions(-)

diff --git a/arch/s390/include/asm/facilities_src.h b/arch/s390/include/asm/facilities_src.h
index 3b758f6..8b1a692 100644
--- a/arch/s390/include/asm/facilities_src.h
+++ b/arch/s390/include/asm/facilities_src.h
@@ -6,8 +6,6 @@
 #error "This file can only be included by gen_facilities.c"
 #endif
 
-#include <linux/kconfig.h>
-
 struct facility_def {
 	char *name;
 	int *bits;
-- 
1.9.1

[PATCH 2/2] s390: squash facilities_src.h into gen_facilities.c

[Masahiro Yamada]

We generally expect headers in arch/$(ARCH)/include/asm directory
are included from kernel sources, but facilities_src.h is not;
it is included from the arch/s390/tools/gen_facilities.c tool.

There is no reason to expose this header to the public include path.
Furthermore, facilities_src.h makes sure to be included only from
gen_facilities.c by the following:

  #ifndef S390_GEN_FACILITIES_C
  #error "This file can only be included by gen_facilities.c"
  #endif

This check can be removed by merging the two files.

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

 arch/s390/include/asm/facilities_src.h | 80 ----------------------------------
 arch/s390/tools/gen_facilities.c       | 76 ++++++++++++++++++++++++++++++--
 2 files changed, 73 insertions(+), 83 deletions(-)
 delete mode 100644 arch/s390/include/asm/facilities_src.h

diff --git a/arch/s390/include/asm/facilities_src.h b/arch/s390/include/asm/facilities_src.h
deleted file mode 100644
index 8b1a692..0000000
--- a/arch/s390/include/asm/facilities_src.h
+++ /dev/null
@@ -1,80 +0,0 @@
-/*
- *    Copyright IBM Corp. 2015
- */
-
-#ifndef S390_GEN_FACILITIES_C
-#error "This file can only be included by gen_facilities.c"
-#endif
-
-struct facility_def {
-	char *name;
-	int *bits;
-};
-
-static struct facility_def facility_defs[] = {
-	{
-		/*
-		 * FACILITIES_ALS contains the list of facilities that are
-		 * required to run a kernel that is compiled e.g. with
-		 * -march=<machine>.
-		 */
-		.name = "FACILITIES_ALS",
-		.bits = (int[]){
-#ifdef CONFIG_HAVE_MARCH_Z900_FEATURES
-			0,  /* N3 instructions */
-			1,  /* z/Arch mode installed */
-#endif
-#ifdef CONFIG_HAVE_MARCH_Z990_FEATURES
-			18, /* long displacement facility */
-#endif
-#ifdef CONFIG_HAVE_MARCH_Z9_109_FEATURES
-			7,  /* stfle */
-			17, /* message security assist */
-			21, /* extended-immediate facility */
-			25, /* store clock fast */
-#endif
-#ifdef CONFIG_HAVE_MARCH_Z10_FEATURES
-			27, /* mvcos */
-			32, /* compare and swap and store */
-			33, /* compare and swap and store 2 */
-			34, /* general extension facility */
-			35, /* execute extensions */
-#endif
-#ifdef CONFIG_HAVE_MARCH_Z196_FEATURES
-			45, /* fast-BCR, etc. */
-#endif
-#ifdef CONFIG_HAVE_MARCH_ZEC12_FEATURES
-			49, /* misc-instruction-extensions */
-			52, /* interlocked facility 2 */
-#endif
-#ifdef CONFIG_HAVE_MARCH_Z13_FEATURES
-			53, /* load-and-zero-rightmost-byte, etc. */
-#endif
-			-1 /* END */
-		}
-	},
-	{
-		.name = "FACILITIES_KVM",
-		.bits = (int[]){
-			0,  /* N3 instructions */
-			1,  /* z/Arch mode installed */
-			2,  /* z/Arch mode active */
-			3,  /* DAT-enhancement */
-			4,  /* idte segment table */
-			5,  /* idte region table */
-			6,  /* ASN-and-LX reuse */
-			7,  /* stfle */
-			8,  /* enhanced-DAT 1 */
-			9,  /* sense-running-status */
-			10, /* conditional sske */
-			13, /* ipte-range */
-			14, /* nonquiescing key-setting */
-			73, /* transactional execution */
-			75, /* access-exception-fetch/store indication */
-			76, /* msa extension 3 */
-			77, /* msa extension 4 */
-			78, /* enhanced-DAT 2 */
-			-1  /* END */
-		}
-	},
-};
diff --git a/arch/s390/tools/gen_facilities.c b/arch/s390/tools/gen_facilities.c
index fe4e6c9..8cc53b1 100644
--- a/arch/s390/tools/gen_facilities.c
+++ b/arch/s390/tools/gen_facilities.c
@@ -7,13 +7,83 @@
  *
  */
 
-#define S390_GEN_FACILITIES_C
-
 #include <strings.h>
 #include <string.h>
 #include <stdlib.h>
 #include <stdio.h>
-#include <asm/facilities_src.h>
+
+struct facility_def {
+	char *name;
+	int *bits;
+};
+
+static struct facility_def facility_defs[] = {
+	{
+		/*
+		 * FACILITIES_ALS contains the list of facilities that are
+		 * required to run a kernel that is compiled e.g. with
+		 * -march=<machine>.
+		 */
+		.name = "FACILITIES_ALS",
+		.bits = (int[]){
+#ifdef CONFIG_HAVE_MARCH_Z900_FEATURES
+			0,  /* N3 instructions */
+			1,  /* z/Arch mode installed */
+#endif
+#ifdef CONFIG_HAVE_MARCH_Z990_FEATURES
+			18, /* long displacement facility */
+#endif
+#ifdef CONFIG_HAVE_MARCH_Z9_109_FEATURES
+			7,  /* stfle */
+			17, /* message security assist */
+			21, /* extended-immediate facility */
+			25, /* store clock fast */
+#endif
+#ifdef CONFIG_HAVE_MARCH_Z10_FEATURES
+			27, /* mvcos */
+			32, /* compare and swap and store */
+			33, /* compare and swap and store 2 */
+			34, /* general extension facility */
+			35, /* execute extensions */
+#endif
+#ifdef CONFIG_HAVE_MARCH_Z196_FEATURES
+			45, /* fast-BCR, etc. */
+#endif
+#ifdef CONFIG_HAVE_MARCH_ZEC12_FEATURES
+			49, /* misc-instruction-extensions */
+			52, /* interlocked facility 2 */
+#endif
+#ifdef CONFIG_HAVE_MARCH_Z13_FEATURES
+			53, /* load-and-zero-rightmost-byte, etc. */
+#endif
+			-1 /* END */
+		}
+	},
+	{
+		.name = "FACILITIES_KVM",
+		.bits = (int[]){
+			0,  /* N3 instructions */
+			1,  /* z/Arch mode installed */
+			2,  /* z/Arch mode active */
+			3,  /* DAT-enhancement */
+			4,  /* idte segment table */
+			5,  /* idte region table */
+			6,  /* ASN-and-LX reuse */
+			7,  /* stfle */
+			8,  /* enhanced-DAT 1 */
+			9,  /* sense-running-status */
+			10, /* conditional sske */
+			13, /* ipte-range */
+			14, /* nonquiescing key-setting */
+			73, /* transactional execution */
+			75, /* access-exception-fetch/store indication */
+			76, /* msa extension 3 */
+			77, /* msa extension 4 */
+			78, /* enhanced-DAT 2 */
+			-1  /* END */
+		}
+	},
+};
 
 static void print_facility_list(struct facility_def *def)
 {
-- 
1.9.1

Re: [PATCH 2/2] s390: squash facilities_src.h into gen_facilities.c

[Heiko Carstens]

On Sun, Nov 06, 2016 at 12:45:28PM +0900, Masahiro Yamada wrote:
> We generally expect headers in arch/$(ARCH)/include/asm directory
> are included from kernel sources, but facilities_src.h is not;
> it is included from the arch/s390/tools/gen_facilities.c tool.
> 
> There is no reason to expose this header to the public include path.
> Furthermore, facilities_src.h makes sure to be included only from
> gen_facilities.c by the following:
> 
>   #ifndef S390_GEN_FACILITIES_C
>   #error "This file can only be included by gen_facilities.c"
>   #endif
> 
> This check can be removed by merging the two files.
> 
> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> ---

Both patches:
Acked-by: Heiko Carstens <heiko.carstens@de.ibm.com>

Martin, can you please pick them up?

Re: [PATCH 2/2] s390: squash facilities_src.h into gen_facilities.c

[Martin Schwidefsky]

On Mon, 7 Nov 2016 08:03:22 +0100
Heiko Carstens <heiko.carstens@de.ibm.com> wrote:

> On Sun, Nov 06, 2016 at 12:45:28PM +0900, Masahiro Yamada wrote:
> > We generally expect headers in arch/$(ARCH)/include/asm directory
> > are included from kernel sources, but facilities_src.h is not;
> > it is included from the arch/s390/tools/gen_facilities.c tool.
> > 
> > There is no reason to expose this header to the public include path.
> > Furthermore, facilities_src.h makes sure to be included only from
> > gen_facilities.c by the following:
> > 
> >   #ifndef S390_GEN_FACILITIES_C
> >   #error "This file can only be included by gen_facilities.c"
> >   #endif
> > 
> > This check can be removed by merging the two files.
> > 
> > Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> > ---
> 
> Both patches:
> Acked-by: Heiko Carstens <heiko.carstens@de.ibm.com>
> 
> Martin, can you please pick them up?

I will add these two patches to the merge branch for 4.10.

-- 
blue skies,
   Martin.

"Reality continues to ruin my life." - Calvin.

Re: [PATCH 2/2] s390: squash facilities_src.h into gen_facilities.c

[Paul Bolle]

On Mon, 2016-11-07 at 10:50 +0100, Martin Schwidefsky wrote:
> Heiko Carstens <heiko.carstens@de.ibm.com> wrote:
> > On Sun, Nov 06, 2016 at 12:45:28PM +0900, Masahiro Yamada wrote:
> > > 
> > > We generally expect headers in arch/$(ARCH)/include/asm directory
> > > are included from kernel sources,  but facilities_src.h is not;
> > > it is included from the arch/s390/tools/gen_facilities.c tool.
> > > 
> > > There is no reason to expose this header to the public include path.
> > > Furthermore, facilities_src.h makes sure to be included only from
> > > gen_facilities.c by the following:
> > > 
> > >   #ifndef S390_GEN_FACILITIES_C
> > >   #error "This file can only be included by gen_facilities.c"
> > >   #endif
> > > 
> > > This check can be removed by merging the two files.
> > > 
> > > Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>

It took me some time to figure out that gen_facilities is only used to
generate a small header file (generated/facilities.h). And that header's only
goal is to define FACILITIES_ALS and FACILITIES_KVM.

Pasted below is an attempt to use asm/facilities.h instead of
generated/facilities.h. That allows to drop arch/s390/tools/ entirely. I don't
actually have an s390 machine at hand to test this, but this does build and
the preprocessed code this generates looks sane.

(Yes, asm/facilities.h might need another level of preprocessor defines to
become actually readable.)

Thanks,


Paul Bolle

diff --git a/arch/s390/Makefile b/arch/s390/Makefile
index 54e00526b8df..a0ee0a1ee677 100644
--- a/arch/s390/Makefile
+++ b/arch/s390/Makefile
@@ -108,7 +108,6 @@ drivers-y	+= drivers/s390/
 drivers-$(CONFIG_OPROFILE)	+= arch/s390/oprofile/
 
 boot		:= arch/s390/boot
-tools		:= arch/s390/tools
 
 all: image bzImage
 
@@ -127,10 +126,6 @@ vdso_install:
 
 archclean:
 	$(Q)$(MAKE) $(clean)=$(boot)
-	$(Q)$(MAKE) $(clean)=$(tools)
-
-archprepare:
-	$(Q)$(MAKE) $(build)=$(tools) include/generated/facilities.h
 
 # Don't use tabs in echo arguments
 define archhelp
diff --git a/arch/s390/include/asm/facilities.h b/arch/s390/include/asm/facilities.h
new file mode 100644
index 000000000000..c87f18d29217
--- /dev/null
+++ b/arch/s390/include/asm/facilities.h
@@ -0,0 +1,43 @@
+#ifndef __ASM_FACILITIES_H
+#define __ASM_FACILITIES_H
+
+#define FACILITIES_ALS \
+	_AC(IS_BUILTIN(CONFIG_HAVE_MARCH_Z900_FEATURES), UL) << 0 |	/* N3 instructions */ \
+	_AC(IS_BUILTIN(CONFIG_HAVE_MARCH_Z900_FEATURES), UL) << 1 |	/* z/Arch mode installed */ \
+	_AC(IS_BUILTIN(CONFIG_HAVE_MARCH_Z9_109_FEATURES), UL) << 7 |	/* stfle */ \
+	_AC(IS_BUILTIN(CONFIG_HAVE_MARCH_Z9_109_FEATURES), UL) << 17 |	/* message security assist */ \
+	_AC(IS_BUILTIN(CONFIG_HAVE_MARCH_Z990_FEATURES), UL) << 18 |	/* long displacement facility */ \
+	_AC(IS_BUILTIN(CONFIG_HAVE_MARCH_Z9_109_FEATURES), UL) << 21 |	/* extended-immediate facility */ \
+	_AC(IS_BUILTIN(CONFIG_HAVE_MARCH_Z9_109_FEATURES), UL) << 25 |	/* store clock fast */ \
+	_AC(IS_BUILTIN(CONFIG_HAVE_MARCH_Z10_FEATURES), UL) << 27 |	/* mvcos */ \
+	_AC(IS_BUILTIN(CONFIG_HAVE_MARCH_Z10_FEATURES), UL) << 32 |	/* compare and swap and store */ \
+	_AC(IS_BUILTIN(CONFIG_HAVE_MARCH_Z10_FEATURES), UL) << 33 |	/* compare and swap and store 2 */ \
+	_AC(IS_BUILTIN(CONFIG_HAVE_MARCH_Z10_FEATURES), UL) << 34 |	/* general extension facility */ \
+	_AC(IS_BUILTIN(CONFIG_HAVE_MARCH_Z10_FEATURES), UL) << 35 |	/* execute extensions */ \
+	_AC(IS_BUILTIN(CONFIG_HAVE_MARCH_Z196_FEATURES), UL) << 45 |	/* fast-BCR, etc. */ \
+	_AC(IS_BUILTIN(CONFIG_HAVE_MARCH_ZEC12_FEATURES), UL) << 49 |	/* misc-instruction-extensions */ \
+	_AC(IS_BUILTIN(CONFIG_HAVE_MARCH_ZEC12_FEATURES), UL) << 52 |	/* interlocked facility 2 */ \
+	_AC(IS_BUILTIN(CONFIG_HAVE_MARCH_Z13_FEATURES), UL) << 53	/* load-and-zero-rightmost-byte, etc. */
+
+#define FACILITIES_KVM \
+	_BITUL(0)  |	/* N3 instructions */ \
+	_BITUL(1)  |	/* z/Arch mode installed */ \
+	_BITUL(2)  |	/* z/Arch mode active */ \
+	_BITUL(3)  |	/* DAT-enhancement */ \
+	_BITUL(4)  |	/* idte segment table */ \
+	_BITUL(5)  |	/* idte region table */ \
+	_BITUL(6)  |	/* ASN-and-LX reuse */ \
+	_BITUL(7)  |	/* stfle */ \
+	_BITUL(8)  |	/* enhanced-DAT 1 */ \
+	_BITUL(9)  |	/* sense-running-status */ \
+	_BITUL(10) |	/* conditional sske */ \
+	_BITUL(13) |	/* ipte-range */ \
+	_BITUL(14)  	/* nonquiescing key-setting */ \
+	, \
+	_BITUL(9)  |	/* transactional execution */ \
+	_BITUL(11) |	/* access-exception-fetch/store indication */ \
+	_BITUL(12) |	/* msa extension 3 */ \
+	_BITUL(13) |	/* msa extension 4 */ \
+	_BITUL(14)	/* enhanced-DAT 2 */
+
+#endif
diff --git a/arch/s390/include/asm/facility.h b/arch/s390/include/asm/facility.h
index 09b406db7529..aed6b5454662 100644
--- a/arch/s390/include/asm/facility.h
+++ b/arch/s390/include/asm/facility.h
@@ -7,7 +7,7 @@
 #ifndef __ASM_FACILITY_H
 #define __ASM_FACILITY_H
 
-#include <generated/facilities.h>
+#include <asm/facilities.h>
 
 #ifndef __ASSEMBLY__
 
diff --git a/arch/s390/tools/.gitignore b/arch/s390/tools/.gitignore
deleted file mode 100644
index 72a4b2cf1365..000000000000
diff --git a/arch/s390/tools/Makefile b/arch/s390/tools/Makefile
deleted file mode 100644
index 6d9814c9df2b..000000000000
diff --git a/arch/s390/tools/gen_facilities.c b/arch/s390/tools/gen_facilities.c
deleted file mode 100644
index fe4e6c910dd7..000000000000

Re: [PATCH 2/2] s390: squash facilities_src.h into gen_facilities.c

[Heiko Carstens]

On Mon, Nov 07, 2016 at 02:13:06PM +0100, Paul Bolle wrote:
> It took me some time to figure out that gen_facilities is only used to
> generate a small header file (generated/facilities.h). And that header's only
> goal is to define FACILITIES_ALS and FACILITIES_KVM.
> 
> Pasted below is an attempt to use asm/facilities.h instead of
> generated/facilities.h. That allows to drop arch/s390/tools/ entirely. I don't
> actually have an s390 machine at hand to test this, but this does build and
> the preprocessed code this generates looks sane.
> 
> (Yes, asm/facilities.h might need another level of preprocessor defines to
> become actually readable.)

The whole point of the tool is that we can use the bit numbers that are
specified within the architecture document, with the additional odd IBM bit
numbering scheme. Where the most significant bit has bit number 0(!).

> diff --git a/arch/s390/include/asm/facilities.h b/arch/s390/include/asm/facilities.h
> new file mode 100644
> index 000000000000..c87f18d29217
> --- /dev/null
> +++ b/arch/s390/include/asm/facilities.h
> @@ -0,0 +1,43 @@
> +#ifndef __ASM_FACILITIES_H
> +#define __ASM_FACILITIES_H
> +
> +#define FACILITIES_ALS \
> +	_AC(IS_BUILTIN(CONFIG_HAVE_MARCH_Z900_FEATURES), UL) << 0 |	/* N3 instructions */ \

So, this is wrong. It should be " << 63" to match the existing code.

> +#define FACILITIES_KVM \
> +	_BITUL(0)  |	/* N3 instructions */ \
> +	_BITUL(1)  |	/* z/Arch mode installed */ \
> +	_BITUL(2)  |	/* z/Arch mode active */ \
> +	_BITUL(3)  |	/* DAT-enhancement */ \
> +	_BITUL(4)  |	/* idte segment table */ \
> +	_BITUL(5)  |	/* idte region table */ \
> +	_BITUL(6)  |	/* ASN-and-LX reuse */ \
> +	_BITUL(7)  |	/* stfle */ \
> +	_BITUL(8)  |	/* enhanced-DAT 1 */ \
> +	_BITUL(9)  |	/* sense-running-status */ \
> +	_BITUL(10) |	/* conditional sske */ \
> +	_BITUL(13) |	/* ipte-range */ \
> +	_BITUL(14)  	/* nonquiescing key-setting */ \
> +	, \
> +	_BITUL(9)  |	/* transactional execution */ \
> +	_BITUL(11) |	/* access-exception-fetch/store indication */ \

And this is exactly what I want to avoid: start counting from zero again if
we cross a double word. It _must_ read 73, 75, otherwise this becomes the
unmaintainable and error prone mess we had before.

I just want a list with bit numbers and the rest must be created
automatically.

Re: [PATCH 2/2] s390: squash facilities_src.h into gen_facilities.c

[Paul Bolle]

On Mon, 2016-11-07 at 14:38 +0100, Heiko Carstens wrote:
> On Mon, Nov 07, 2016 at 02:13:06PM +0100, Paul Bolle wrote:
> > --- /dev/null
> > +++ b/arch/s390/include/asm/facilities.h
> > @@ -0,0 +1,43 @@
> > +#ifndef __ASM_FACILITIES_H
> > +#define __ASM_FACILITIES_H
> > +
> > +#define FACILITIES_ALS \
> > +	_AC(IS_BUILTIN(CONFIG_HAVE_MARCH_Z900_FEATURES), UL) << 0 |	/* N3 instructions */ \
> 
> So, this is wrong. It should be " << 63" to match the existing code.

Bother.

> > +#define FACILITIES_KVM \
> > +	_BITUL(0)  |	/* N3 instructions */ \
> > +	_BITUL(1)  |	/* z/Arch mode installed */ \
> > +	_BITUL(2)  |	/* z/Arch mode active */ \
> > +	_BITUL(3)  |	/* DAT-enhancement */ \
> > +	_BITUL(4)  |	/* idte segment table */ \
> > +	_BITUL(5)  |	/* idte region table */ \
> > +	_BITUL(6)  |	/* ASN-and-LX reuse */ \
> > +	_BITUL(7)  |	/* stfle */ \
> > +	_BITUL(8)  |	/* enhanced-DAT 1 */ \
> > +	_BITUL(9)  |	/* sense-running-status */ \
> > +	_BITUL(10) |	/* conditional sske */ \
> > +	_BITUL(13) |	/* ipte-range */ \
> > +	_BITUL(14)  	/* nonquiescing key-setting */ \
> > +	, \
> > +	_BITUL(9)  |	/* transactional execution */ \
> > +	_BITUL(11) |	/* access-exception-fetch/store indication */ \
> 
> And this is exactly what I want to avoid: start counting from zero again if
> we cross a double word. It _must_ read 73, 75, otherwise this becomes the
> unmaintainable and error prone mess we had before.
> 
> I just want a list with bit numbers and the rest must be created
> automatically.

This sounds like a can of worms. Better keep it closed.

Thanks,


Paul Bolle

Re: [PATCH 1/2] s390: delete unneeded #include <linux/kconfig.h> from facilities_src.h

[Paul Bolle]

On Sun, 2016-11-06 at 12:45 +0900, Masahiro Yamada wrote:
> The header facilities_src.h is only included from gen_facilities.c
> and the tool is compiled with the following extra options:
> 
>     HOSTCFLAGS_gen_facilities.o += -Wall $(LINUXINCLUDE)
> 
> Please note $(LINUXINCLUDE) is expanded into build options including:
> 
>     -include $(srctree)/include/linux/kconfig.h
> 
> So, the Makefile always forces the tool to include kconfig.h, i.e.,
> the #include <linux/kconfig.h> directive in the header is redundant.

As far as I can see the only kernel header that gen_facilities.c is actually
interested in is autoconf.h. (autoconf.h will be included via in kconfig.h.)
So it seems the odd $(LINUXINCLUDE) variable in that Makefile could be
replaced with something like:
    -include $(srctree)/include/generated/autoconf.h


Paul Bolle

Re: [PATCH 1/2] s390: delete unneeded #include <linux/kconfig.h> from facilities_src.h

[Masahiro Yamada]

Hi Paul,


2016-11-07 21:52 GMT+09:00 Paul Bolle <pebolle@tiscali.nl>:
> On Sun, 2016-11-06 at 12:45 +0900, Masahiro Yamada wrote:
>> The header facilities_src.h is only included from gen_facilities.c
>> and the tool is compiled with the following extra options:
>>
>>     HOSTCFLAGS_gen_facilities.o += -Wall $(LINUXINCLUDE)
>>
>> Please note $(LINUXINCLUDE) is expanded into build options including:
>>
>>     -include $(srctree)/include/linux/kconfig.h
>>
>> So, the Makefile always forces the tool to include kconfig.h, i.e.,
>> the #include <linux/kconfig.h> directive in the header is redundant.
>
> As far as I can see the only kernel header that gen_facilities.c is actually
> interested in is autoconf.h. (autoconf.h will be included via in kconfig.h.)
> So it seems the odd $(LINUXINCLUDE) variable in that Makefile could be
> replaced with something like:
>     -include $(srctree)/include/generated/autoconf.h


This would break O= build because autoconf.h is a generated file.

Rather, it should be
      -include $(objtree)/include/generated/autoconf.h



I thought of this at first, but I was not quite sure
if the file path include/generated/autoconf.h is a guaranteed interface.


Basically, now we are supposed to include autoconf.h via kconfig.h.

So, I thought $(LINUXINCLUDE) is a more stable interface
than specifying the exact path to autoconf.h

I doubt that nobody would try to change it, but it is just two my cents.

Anyway, arch/x86/boot/Makefile already
referenced the path to autoconf.h


So, if you want to change it, I will not oppose to it.



-- 
Best Regards
Masahiro Yamada

Re: [PATCH 1/2] s390: delete unneeded #include <linux/kconfig.h> from facilities_src.h

[Paul Bolle]

Hi Mashiro,

On Tue, 2016-11-08 at 10:50 +0900, Masahiro Yamada wrote:
> 2016-11-07 21:52 GMT+09:00 Paul Bolle <pebolle@tiscali.nl>:
> > So it seems the odd $(LINUXINCLUDE) variable in that Makefile could be
> > replaced with something like:
> >     -include $(srctree)/include/generated/autoconf.h
> 
> This would break O= build because autoconf.h is a generated file.
> 
> Rather, it should be
>       -include $(objtree)/include/generated/autoconf.h

Three cheers for weasel words like "something like"!

> I thought of this at first, but I was not quite sure
> if the file path include/generated/autoconf.h is a guaranteed interface.
> 
> Basically, now we are supposed to include autoconf.h via kconfig.h.

Yes, that seems to go back to commit 2a11c8ea20bf ("kconfig: Introduce
IS_ENABLED(), IS_BUILTIN() and IS_MODULE()"). And when the current approach to
the IS_*() macros was introduced - with that breathtaking hack that introduced
__is_defined() - this was no longer needed but was not changed again.

> So, I thought $(LINUXINCLUDE) is a more stable interface
> than specifying the exact path to autoconf.h
> 
> I doubt that nobody would try to change it, but it is just two my cents.

A bit of cruft accumulated around LINUXINCLUDE: a few dubious uses of it (and
I think this is one of those); typos (ie, LINUX_INCLUDE); the pointless
USERINCLUDE; things like that. It would be nice to remove that cruft. But it
needs to be done carefully.

> Anyway, arch/x86/boot/Makefile already
> referenced the path to autoconf.h
> 
> So, if you want to change it, I will not oppose to it.


Paul Bolle