Skip to contentSkip to conversation

SFI: fix compilation warnings

PATCH In review Andy Shevchenko 2 participants 5 messages Dec 9, 2013 – Dec 11, 2013 ACPI

lore
Share & tools
lore

Andy Shevchenko

author
#0 lore raw

PATCH v2 1/1 SFI: fix compilation warnings

When build kernel with make W=1 we get the following compiler error.

In file included from drivers/sfi/sfi_acpi.c:66:0:
include/linux/sfi_acpi.h: In function ‘acpi_sfi_table_parse’:
include/linux/sfi_acpi.h:72:2: error: implicit declaration of function ‘acpi_table_parse’ [-Werror=implicit-function-declaration]
if (!acpi_table_parse(signature, handler))
^
cc1: some warnings being treated as errors

The patch add linux/acpi.h to the top of file.

Signed-off-by Andy Shevchenko <[email protected]>

---
Changes to v2:
- fix for build when ACPI=n, SFI=y

 drivers/sfi/sfi_acpi.c   | 2 +-
 include/linux/sfi_acpi.h | 4 +++-
 2 files changed, 4 insertions(+), 2 deletions(-)
drivers/sfi/sfi_acpi.c +1 -1
index 5e753d7..5589ec5 100644
@@ -60,7 +60,7 @@
 #define pr_fmt(fmt) KMSG_COMPONENT ": " fmt
 
 #include <linux/kernel.h>
-#include <acpi/acpi.h>	/* FIXME: inclusion should be removed */
+#include <linux/acpi.h>
 
 #include <linux/sfi.h>
 #include "sfi_core.h"
include/linux/sfi_acpi.h +3 -1
index 2cfcb79..ecc2344 100644
@@ -60,7 +60,9 @@
 #define _LINUX_SFI_ACPI_H
 
 #ifdef CONFIG_SFI
-#include <acpi/acpi.h>	/* FIXME: inclusion should be removed */
+
+#include <acpi/acpi.h>
+#include <linux/acpi.h>
 
 extern int sfi_acpi_table_parse(char *signature, char *oem_id,
 				char *oem_table_id,
Signature · 1 line
1.8.4.4
#1 lore raw

Hi, Andy

IMO:
1. sfi_acpi.h should include both <linux/acpi.h> and <linux/sfi.h> and it should include <acpi/acpi.h> for !CONFIG_ACPI builds.
2. then we should include sfi_acpi.h instead of <linux/acpi.h>, <linux/sfi.h> and <acpi/acpi.h> for all of its users.

I think the patch in the linux-pm/bleeding-edge queue can fix the issue you've reported:
https://git.kernel.org/cgit/linux/kernel/git/rafael/linux-pm.git/commit/?h=bleeding-edge&id=c099eacbcaec4475936fbf73e499507728ce47e1

I made it a part of my header cleanup series. The story can be found in this mailing list thread:
http://www.spinics.net/lists/linux-acpi/msg47510.html

Thanks and best regards
-Lv

Quoted 56 lines

-----Original Message-----
From: Andy Shevchenko [mailto:[email protected]]
Sent: Monday, December 09, 2013 5:09 PM
To: Wysocki, Rafael J; linux-acpi @ vger . kernel . org; LKML; Zheng, Lv
Cc: Andy Shevchenko
Subject: [PATCH v2] SFI: fix compilation warnings

When build kernel with make W=1 we get the following compiler error.

In file included from drivers/sfi/sfi_acpi.c:66:0:
include/linux/sfi_acpi.h: In function ‘acpi_sfi_table_parse’:
include/linux/sfi_acpi.h:72:2: error: implicit declaration of function ‘acpi_table_parse’ [-Werror=implicit-function-declaration]
if (!acpi_table_parse(signature, handler))
^
cc1: some warnings being treated as errors

The patch add linux/acpi.h to the top of file.

Signed-off-by Andy Shevchenko <[email protected]>

---
Changes to v2:
- fix for build when ACPI=n, SFI=y

 drivers/sfi/sfi_acpi.c   | 2 +-
 include/linux/sfi_acpi.h | 4 +++-
 2 files changed, 4 insertions(+), 2 deletions(-)
drivers/sfi/sfi_acpi.c +1 -1
index 5e753d7..5589ec5 100644
@@ -60,7 +60,7 @@
 #define pr_fmt(fmt) KMSG_COMPONENT ": " fmt
 
 #include <linux/kernel.h>
-#include <acpi/acpi.h>	/* FIXME: inclusion should be removed */
+#include <linux/acpi.h>
 
 #include <linux/sfi.h>
 #include "sfi_core.h"
include/linux/sfi_acpi.h +3 -2
index 2cfcb79..ecc2344 100644
@@ -60,7 +60,9 @@
 #define _LINUX_SFI_ACPI_H
 
 #ifdef CONFIG_SFI
-#include <acpi/acpi.h>	/* FIXME: inclusion should be removed */
+
+#include <acpi/acpi.h>
+#include <linux/acpi.h>
 
 extern int sfi_acpi_table_parse(char *signature, char *oem_id,
 				char *oem_table_id,
--

1.8.4.4

author
#2 lore raw
Quoted 8 lines · Lv

Hi, Andy

IMO:
1. sfi_acpi.h should include both <linux/acpi.h> and <linux/sfi.h> and it should include <acpi/acpi.h> for !CONFIG_ACPI builds.
2. then we should include sfi_acpi.h instead of <linux/acpi.h>, <linux/sfi.h> and <acpi/acpi.h> for all of its users.

I think the patch in the linux-pm/bleeding-edge queue can fix the issue you've reported:
https://git.kernel.org/cgit/linux/kernel/git/rafael/linux-pm.git/commit/?h=bleeding-edge&id=c099eacbcaec4475936fbf73e499507728ce47e1

I think so, I'll test this later.

Only one thing I don't like is the FIXME stuff in the sfi_acpi.h. Could it be fixed as well?

Quoted 66 lines

I made it a part of my header cleanup series. The story can be found in this mailing list thread:
http://www.spinics.net/lists/linux-acpi/msg47510.html

Thanks and best regards
-Lv

Quoted 56 lines

-----Original Message-----
From: Andy Shevchenko [mailto:[email protected]]
Sent: Monday, December 09, 2013 5:09 PM
To: Wysocki, Rafael J; linux-acpi @ vger . kernel . org; LKML; Zheng, Lv
Cc: Andy Shevchenko
Subject: [PATCH v2] SFI: fix compilation warnings

When build kernel with make W=1 we get the following compiler error.

In file included from drivers/sfi/sfi_acpi.c:66:0:
include/linux/sfi_acpi.h: In function ‘acpi_sfi_table_parse’:
include/linux/sfi_acpi.h:72:2: error: implicit declaration of function ‘acpi_table_parse’ [-Werror=implicit-function-declaration]
if (!acpi_table_parse(signature, handler))
^
cc1: some warnings being treated as errors

The patch add linux/acpi.h to the top of file.

Signed-off-by Andy Shevchenko <[email protected]>

---
Changes to v2:
- fix for build when ACPI=n, SFI=y

 drivers/sfi/sfi_acpi.c   | 2 +-
 include/linux/sfi_acpi.h | 4 +++-
 2 files changed, 4 insertions(+), 2 deletions(-)
drivers/sfi/sfi_acpi.c +1 -1
index 5e753d7..5589ec5 100644
@@ -60,7 +60,7 @@
 #define pr_fmt(fmt) KMSG_COMPONENT ": " fmt
 
 #include <linux/kernel.h>
-#include <acpi/acpi.h>	/* FIXME: inclusion should be removed */
+#include <linux/acpi.h>
 
 #include <linux/sfi.h>
 #include "sfi_core.h"
include/linux/sfi_acpi.h +3 -2
index 2cfcb79..ecc2344 100644
@@ -60,7 +60,9 @@
 #define _LINUX_SFI_ACPI_H
 
 #ifdef CONFIG_SFI
-#include <acpi/acpi.h>	/* FIXME: inclusion should be removed */
+
+#include <acpi/acpi.h>
+#include <linux/acpi.h>
 
 extern int sfi_acpi_table_parse(char *signature, char *oem_id,
 				char *oem_table_id,
--

1.8.4.4

Signature · 7 lines
Andy Shevchenko <[email protected]>
Intel Finland Oy

--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
#3 lore raw

Hi,

Quoted 10 lines

From: [email protected] [mailto:[email protected]] On Behalf Of Andy Shevchenko
Sent: Tuesday, December 10, 2013 8:08 PM

Quoted 6 lines · Lv

Hi, Andy

IMO:
1. sfi_acpi.h should include both <linux/acpi.h> and <linux/sfi.h> and it should include <acpi/acpi.h> for !CONFIG_ACPI builds.
2. then we should include sfi_acpi.h instead of <linux/acpi.h>, <linux/sfi.h> and <acpi/acpi.h> for all of its users.

Quoted 8 lines
Quoted 2 lines

I think the patch in the linux-pm/bleeding-edge queue can fix the issue you've reported:
https://git.kernel.org/cgit/linux/kernel/git/rafael/linux-pm.git/commit/?h=bleeding-

edge&id=c099eacbcaec4475936fbf73e499507728ce47e1

I think so, I'll test this later.

Only one thing I don't like is the FIXME stuff in the sfi_acpi.h. Could it be fixed as well?

It is just a reminder to tell people <acpi/acpi.h> inclusion here cannot be a reference for those who want to include it directly.

I have offered 2 solutions to remove this FIXME:
1. Implementing stubs for <acpi/acpi.h>, then we can move <acpi/acpi.h> inclusion out of the "#ifdef CONFIG_ACPI" block. To achieve this, I drafted more than 10 commits. Someone might think it was too complicated to address such an issue, but we might utilize this to kill more "#ifdef CONFIG_ACPI" blocks in the entire kernel source tree.
2. Re-defining necessary table structures for !CONFIG_ACPI builds. 1 commit was ready for this. But having things defined twice looks ugly.
They are still under discussion.

If the decision was determined, you might see further patches sent from me to the above mentioned thread or during the ACPICA release process to remove it.

Thanks and best regards
-Lv

Quoted 77 lines
Quoted 63 lines

I made it a part of my header cleanup series. The story can be found in this mailing list thread:
http://www.spinics.net/lists/linux-acpi/msg47510.html

Thanks and best regards
-Lv

Quoted 52 lines

-----Original Message-----
From: Andy Shevchenko [mailto:[email protected]]
Sent: Monday, December 09, 2013 5:09 PM
To: Wysocki, Rafael J; linux-acpi @ vger . kernel . org; LKML; Zheng, Lv
Cc: Andy Shevchenko
Subject: [PATCH v2] SFI: fix compilation warnings

When build kernel with make W=1 we get the following compiler error.

In file included from drivers/sfi/sfi_acpi.c:66:0:
include/linux/sfi_acpi.h: In function ‘acpi_sfi_table_parse’:
include/linux/sfi_acpi.h:72:2: error: implicit declaration of function ‘acpi_table_parse’ [-Werror=implicit-function-declaration]
if (!acpi_table_parse(signature, handler))
^
cc1: some warnings being treated as errors

The patch add linux/acpi.h to the top of file.

Signed-off-by Andy Shevchenko <[email protected]>

---
Changes to v2:
- fix for build when ACPI=n, SFI=y

 drivers/sfi/sfi_acpi.c   | 2 +-
 include/linux/sfi_acpi.h | 4 +++-
 2 files changed, 4 insertions(+), 2 deletions(-)
drivers/sfi/sfi_acpi.c +1 -1
index 5e753d7..5589ec5 100644
@@ -60,7 +60,7 @@
 #define pr_fmt(fmt) KMSG_COMPONENT ": " fmt
 
 #include <linux/kernel.h>
-#include <acpi/acpi.h>	/* FIXME: inclusion should be removed */
+#include <linux/acpi.h>
 
 #include <linux/sfi.h>
 #include "sfi_core.h"
include/linux/sfi_acpi.h +3 -1
index 2cfcb79..ecc2344 100644
@@ -60,7 +60,9 @@
 #define _LINUX_SFI_ACPI_H
 
 #ifdef CONFIG_SFI
-#include <acpi/acpi.h>	/* FIXME: inclusion should be removed */
+
+#include <acpi/acpi.h>
+#include <linux/acpi.h>
Quoted 5 lines
Quoted 4 lines
include/linux/sfi_acpi.h +0 -1
 extern int sfi_acpi_table_parse(char *signature, char *oem_id,
 				char *oem_table_id,
--

1.8.4.4

--
Andy Shevchenko <[email protected]> Intel Finland Oy

--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

author Tested-by
#4 lore raw
In reply to Zheng, Lv
Quoted 8 lines · Lv

Hi, Andy

IMO:
1. sfi_acpi.h should include both <linux/acpi.h> and <linux/sfi.h> and it should include <acpi/acpi.h> for !CONFIG_ACPI builds.
2. then we should include sfi_acpi.h instead of <linux/acpi.h>, <linux/sfi.h> and <acpi/acpi.h> for all of its users.

I think the patch in the linux-pm/bleeding-edge queue can fix the issue you've reported:
https://git.kernel.org/cgit/linux/kernel/git/rafael/linux-pm.git/commit/?h=bleeding-edge&id=c099eacbcaec4475936fbf73e499507728ce47e1

Seems okay for me:

Tested-by Andy Shevchenko <[email protected]>
Quoted 66 lines

I made it a part of my header cleanup series. The story can be found in this mailing list thread:
http://www.spinics.net/lists/linux-acpi/msg47510.html

Thanks and best regards
-Lv

Quoted 56 lines

-----Original Message-----
From: Andy Shevchenko [mailto:[email protected]]
Sent: Monday, December 09, 2013 5:09 PM
To: Wysocki, Rafael J; linux-acpi @ vger . kernel . org; LKML; Zheng, Lv
Cc: Andy Shevchenko
Subject: [PATCH v2] SFI: fix compilation warnings

When build kernel with make W=1 we get the following compiler error.

In file included from drivers/sfi/sfi_acpi.c:66:0:
include/linux/sfi_acpi.h: In function ‘acpi_sfi_table_parse’:
include/linux/sfi_acpi.h:72:2: error: implicit declaration of function ‘acpi_table_parse’ [-Werror=implicit-function-declaration]
if (!acpi_table_parse(signature, handler))
^
cc1: some warnings being treated as errors

The patch add linux/acpi.h to the top of file.

Signed-off-by Andy Shevchenko <[email protected]>

---
Changes to v2:
- fix for build when ACPI=n, SFI=y

 drivers/sfi/sfi_acpi.c   | 2 +-
 include/linux/sfi_acpi.h | 4 +++-
 2 files changed, 4 insertions(+), 2 deletions(-)
drivers/sfi/sfi_acpi.c +1 -1
index 5e753d7..5589ec5 100644
@@ -60,7 +60,7 @@
 #define pr_fmt(fmt) KMSG_COMPONENT ": " fmt
 
 #include <linux/kernel.h>
-#include <acpi/acpi.h>	/* FIXME: inclusion should be removed */
+#include <linux/acpi.h>
 
 #include <linux/sfi.h>
 #include "sfi_core.h"
include/linux/sfi_acpi.h +3 -2
index 2cfcb79..ecc2344 100644
@@ -60,7 +60,9 @@
 #define _LINUX_SFI_ACPI_H
 
 #ifdef CONFIG_SFI
-#include <acpi/acpi.h>	/* FIXME: inclusion should be removed */
+
+#include <acpi/acpi.h>
+#include <linux/acpi.h>
 
 extern int sfi_acpi_table_parse(char *signature, char *oem_id,
 				char *oem_table_id,
--

1.8.4.4

Signature · 7 lines
Andy Shevchenko <[email protected]>
Intel Finland Oy

--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html