* [PATCH] i2c: i801: mark PM functions as __maybe_unused
@ 2018-05-08 7:38 Anders Roxell
2018-05-09 17:00 ` Jean Delvare
0 siblings, 1 reply; 8+ messages in thread
From: Anders Roxell @ 2018-05-08 7:38 UTC (permalink / raw)
To: jdelvare; +Cc: linux-i2c, linux-kernel, Anders Roxell
With CONFIG_PM, we get a harmless build warning:
drivers/i2c/busses/i2c-i801.c:1723:12: warning: ‘i801_resume’ defined but not used [-Wunused-function]
static int i801_resume(struct device *dev)
^~~~~~~~~~~
drivers/i2c/busses/i2c-i801.c:1714:12: warning: ‘i801_suspend’ defined but not used [-Wunused-function]
static int i801_suspend(struct device *dev)
^~~~~~~~~~~~
This marks the affected functions as __maybe_unused.
Fixes: a9c8088c7988 ("i2c: i801: Don't restore config registers on runtime PM")
Signed-off-by: Anders Roxell <anders.roxell@linaro.org>
---
drivers/i2c/busses/i2c-i801.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/i2c/busses/i2c-i801.c b/drivers/i2c/busses/i2c-i801.c
index ed07f9002710..ff18c6ed2bec 100644
--- a/drivers/i2c/busses/i2c-i801.c
+++ b/drivers/i2c/busses/i2c-i801.c
@@ -1711,7 +1711,7 @@ static void i801_shutdown(struct pci_dev *dev)
}
#ifdef CONFIG_PM
-static int i801_suspend(struct device *dev)
+static int __maybe_unused i801_suspend(struct device *dev)
{
struct pci_dev *pci_dev = to_pci_dev(dev);
struct i801_priv *priv = pci_get_drvdata(pci_dev);
@@ -1720,7 +1720,7 @@ static int i801_suspend(struct device *dev)
return 0;
}
-static int i801_resume(struct device *dev)
+static int __maybe_unused i801_resume(struct device *dev)
{
struct pci_dev *pci_dev = to_pci_dev(dev);
struct i801_priv *priv = pci_get_drvdata(pci_dev);
--
2.17.0
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH] i2c: i801: mark PM functions as __maybe_unused
2018-05-08 7:38 [PATCH] i2c: i801: mark PM functions as __maybe_unused Anders Roxell
@ 2018-05-09 17:00 ` Jean Delvare
2018-05-10 13:27 ` Jean Delvare
0 siblings, 1 reply; 8+ messages in thread
From: Jean Delvare @ 2018-05-09 17:00 UTC (permalink / raw)
To: Anders Roxell; +Cc: linux-i2c, linux-kernel
Hi Anders,
On Tue, 8 May 2018 09:38:52 +0200, Anders Roxell wrote:
> With CONFIG_PM, we get a harmless build warning:
> drivers/i2c/busses/i2c-i801.c:1723:12: warning: ‘i801_resume’ defined but not used [-Wunused-function]
> static int i801_resume(struct device *dev)
> ^~~~~~~~~~~
> drivers/i2c/busses/i2c-i801.c:1714:12: warning: ‘i801_suspend’ defined but not used [-Wunused-function]
> static int i801_suspend(struct device *dev)
> ^~~~~~~~~~~~
I have CONFIG_PM=y and I don't get this warning, even with W=1. Which
gcc version are you using, which exact kernel version are you building,
and what is the value of all the CONFIG_PM_* options?
> This marks the affected functions as __maybe_unused.
I'm not a big fan of __maybe_unused, at least not in this specific
situation. We should be able to know exactly when these functions are
needed, and only include them when this is the case. Building unused
code just to discard it later (hopefully?) is a waste of CPU time.
>
> Fixes: a9c8088c7988 ("i2c: i801: Don't restore config registers on runtime PM")
If SIMPLE_DEV_PM_OPS causes it but UNIVERSAL_DEV_PM_OPS did not, I
suppose that what matters is CONFIG_PM_SLEEP.
So maybe we can just replace "#ifdef CONFIG_PM" with "ifdef
CONFIG_PM_SLEEP" in the code below?
> Signed-off-by: Anders Roxell <anders.roxell@linaro.org>
> ---
> drivers/i2c/busses/i2c-i801.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-i801.c b/drivers/i2c/busses/i2c-i801.c
> index ed07f9002710..ff18c6ed2bec 100644
> --- a/drivers/i2c/busses/i2c-i801.c
> +++ b/drivers/i2c/busses/i2c-i801.c
> @@ -1711,7 +1711,7 @@ static void i801_shutdown(struct pci_dev *dev)
> }
>
> #ifdef CONFIG_PM
> -static int i801_suspend(struct device *dev)
> +static int __maybe_unused i801_suspend(struct device *dev)
> {
> struct pci_dev *pci_dev = to_pci_dev(dev);
> struct i801_priv *priv = pci_get_drvdata(pci_dev);
> @@ -1720,7 +1720,7 @@ static int i801_suspend(struct device *dev)
> return 0;
> }
>
> -static int i801_resume(struct device *dev)
> +static int __maybe_unused i801_resume(struct device *dev)
> {
> struct pci_dev *pci_dev = to_pci_dev(dev);
> struct i801_priv *priv = pci_get_drvdata(pci_dev);
--
Jean Delvare
SUSE L3 Support
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH] i2c: i801: mark PM functions as __maybe_unused
2018-05-09 17:00 ` Jean Delvare
@ 2018-05-10 13:27 ` Jean Delvare
2018-05-14 9:33 ` [PATCH v2] i2c: i801: fix unused-function warning Anders Roxell
0 siblings, 1 reply; 8+ messages in thread
From: Jean Delvare @ 2018-05-10 13:27 UTC (permalink / raw)
To: Anders Roxell; +Cc: linux-i2c, linux-kernel
On Wed, 9 May 2018 19:00:55 +0200, Jean Delvare wrote:
> If SIMPLE_DEV_PM_OPS causes it but UNIVERSAL_DEV_PM_OPS did not, I
> suppose that what matters is CONFIG_PM_SLEEP.
>
> So maybe we can just replace "#ifdef CONFIG_PM" with "ifdef
> CONFIG_PM_SLEEP" in the code below?
It seems that drivers i2c-brcmstb, i2c-mpc, i2c-ocores, i2c-pnx,
i2c-puv3, i2c-st, i2c-stu300 and i2c-mux-pca954x are doing exactly that
already, so that must be a valid way to solve the problem.
Will you send a new patch?
Thanks,
--
Jean Delvare
SUSE L3 Support
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v2] i2c: i801: fix unused-function warning
2018-05-10 13:27 ` Jean Delvare
@ 2018-05-14 9:33 ` Anders Roxell
2018-05-14 15:02 ` Jean Delvare
` (2 more replies)
0 siblings, 3 replies; 8+ messages in thread
From: Anders Roxell @ 2018-05-14 9:33 UTC (permalink / raw)
To: jdelvare; +Cc: linux-i2c, linux-kernel, Anders Roxell
With CONFIG_PM, we get a harmless build warning:
drivers/i2c/busses/i2c-i801.c:1723:12: warning: ‘i801_resume’ defined but not used [-Wunused-function]
static int i801_resume(struct device *dev)
^~~~~~~~~~~
drivers/i2c/busses/i2c-i801.c:1714:12: warning: ‘i801_suspend’ defined but not used [-Wunused-function]
static int i801_suspend(struct device *dev)
^~~~~~~~~~~~
Follow design pattern from other drivers like i2c-brcmstb, i2c-mpc,
i2c-ocores, i2c-pnx, i2c-puv3, i2c-st, i2c-stu300 and i2c-mux-pca954x
and changing the ifdef CONFIG_PM to CONFIG_PM_SLEEP.
Fixes: a9c8088c7988 ("i2c: i801: Don't restore config registers on runtime PM")
Signed-off-by: Anders Roxell <anders.roxell@linaro.org>
---
drivers/i2c/busses/i2c-i801.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/i2c/busses/i2c-i801.c b/drivers/i2c/busses/i2c-i801.c
index ed07f9002710..954fb3f3b7fc 100644
--- a/drivers/i2c/busses/i2c-i801.c
+++ b/drivers/i2c/busses/i2c-i801.c
@@ -1710,7 +1710,7 @@ static void i801_shutdown(struct pci_dev *dev)
pci_write_config_byte(dev, SMBHSTCFG, priv->original_hstcfg);
}
-#ifdef CONFIG_PM
+#ifdef CONFIG_PM_SLEEP
static int i801_suspend(struct device *dev)
{
struct pci_dev *pci_dev = to_pci_dev(dev);
--
2.17.0
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH v2] i2c: i801: fix unused-function warning
2018-05-14 9:33 ` [PATCH v2] i2c: i801: fix unused-function warning Anders Roxell
@ 2018-05-14 15:02 ` Jean Delvare
2018-05-14 17:18 ` Andy Shevchenko
2018-05-17 13:54 ` Wolfram Sang
2 siblings, 0 replies; 8+ messages in thread
From: Jean Delvare @ 2018-05-14 15:02 UTC (permalink / raw)
To: Anders Roxell; +Cc: linux-i2c, linux-kernel
On Mon, 14 May 2018 11:33:26 +0200, Anders Roxell wrote:
> With CONFIG_PM, we get a harmless build warning:
> drivers/i2c/busses/i2c-i801.c:1723:12: warning: ‘i801_resume’ defined but not used [-Wunused-function]
> static int i801_resume(struct device *dev)
> ^~~~~~~~~~~
> drivers/i2c/busses/i2c-i801.c:1714:12: warning: ‘i801_suspend’ defined but not used [-Wunused-function]
> static int i801_suspend(struct device *dev)
> ^~~~~~~~~~~~
>
> Follow design pattern from other drivers like i2c-brcmstb, i2c-mpc,
> i2c-ocores, i2c-pnx, i2c-puv3, i2c-st, i2c-stu300 and i2c-mux-pca954x
> and changing the ifdef CONFIG_PM to CONFIG_PM_SLEEP.
>
> Fixes: a9c8088c7988 ("i2c: i801: Don't restore config registers on runtime PM")
> Signed-off-by: Anders Roxell <anders.roxell@linaro.org>
> ---
> drivers/i2c/busses/i2c-i801.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/i2c/busses/i2c-i801.c b/drivers/i2c/busses/i2c-i801.c
> index ed07f9002710..954fb3f3b7fc 100644
> --- a/drivers/i2c/busses/i2c-i801.c
> +++ b/drivers/i2c/busses/i2c-i801.c
> @@ -1710,7 +1710,7 @@ static void i801_shutdown(struct pci_dev *dev)
> pci_write_config_byte(dev, SMBHSTCFG, priv->original_hstcfg);
> }
>
> -#ifdef CONFIG_PM
> +#ifdef CONFIG_PM_SLEEP
> static int i801_suspend(struct device *dev)
> {
> struct pci_dev *pci_dev = to_pci_dev(dev);
Reviewed-by: Jean Delvare <jdelvare@suse.de>
Thanks,
--
Jean Delvare
SUSE L3 Support
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH v2] i2c: i801: fix unused-function warning
2018-05-14 9:33 ` [PATCH v2] i2c: i801: fix unused-function warning Anders Roxell
2018-05-14 15:02 ` Jean Delvare
@ 2018-05-14 17:18 ` Andy Shevchenko
2018-05-15 8:16 ` Jean Delvare
2018-05-17 13:54 ` Wolfram Sang
2 siblings, 1 reply; 8+ messages in thread
From: Andy Shevchenko @ 2018-05-14 17:18 UTC (permalink / raw)
To: Anders Roxell; +Cc: Jean Delvare, linux-i2c, Linux Kernel Mailing List
On Mon, May 14, 2018 at 12:33 PM, Anders Roxell
<anders.roxell@linaro.org> wrote:
> With CONFIG_PM, we get a harmless build warning:
> drivers/i2c/busses/i2c-i801.c:1723:12: warning: ‘i801_resume’ defined but not used [-Wunused-function]
> static int i801_resume(struct device *dev)
> ^~~~~~~~~~~
> drivers/i2c/busses/i2c-i801.c:1714:12: warning: ‘i801_suspend’ defined but not used [-Wunused-function]
> static int i801_suspend(struct device *dev)
> ^~~~~~~~~~~~
> -#ifdef CONFIG_PM
> +#ifdef CONFIG_PM_SLEEP
> static int i801_suspend(struct device *dev)
The better pattern is to get rid of ugly ifdef and supply
__maybe_unused annotation to each function in question.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] i2c: i801: fix unused-function warning
2018-05-14 17:18 ` Andy Shevchenko
@ 2018-05-15 8:16 ` Jean Delvare
0 siblings, 0 replies; 8+ messages in thread
From: Jean Delvare @ 2018-05-15 8:16 UTC (permalink / raw)
To: Andy Shevchenko; +Cc: Anders Roxell, linux-i2c, Linux Kernel Mailing List
Hi Andy,
On Mon, 14 May 2018 20:18:37 +0300, Andy Shevchenko wrote:
> On Mon, May 14, 2018 at 12:33 PM, Anders Roxell
> <anders.roxell@linaro.org> wrote:
> > With CONFIG_PM, we get a harmless build warning:
> > drivers/i2c/busses/i2c-i801.c:1723:12: warning: ‘i801_resume’ defined but not used [-Wunused-function]
> > static int i801_resume(struct device *dev)
> > ^~~~~~~~~~~
> > drivers/i2c/busses/i2c-i801.c:1714:12: warning: ‘i801_suspend’ defined but not used [-Wunused-function]
> > static int i801_suspend(struct device *dev)
> > ^~~~~~~~~~~~
>
> > -#ifdef CONFIG_PM
> > +#ifdef CONFIG_PM_SLEEP
> > static int i801_suspend(struct device *dev)
>
> The better pattern is to get rid of ugly ifdef and supply
> __maybe_unused annotation to each function in question.
That was Anders' first proposal, but it was declined by the driver
maintainer (me.) See:
https://marc.info/?l=linux-kernel&m=152588526520326&w=2
__maybe_unused is just a way to prevent the compiler from doing its
job. If it's really what you want, you might as well build with
-Wno-unused, instead of crippling the code with yet another annotation.
I can't see how building unused code only to discard it later can be
better than a proper #ifdef which will only build the code when we
actually need it.
Maybe there are cases where __maybe_unused is actually needed, but in
my opinion that should be the last resort option. That's not the case
here.
--
Jean Delvare
SUSE L3 Support
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] i2c: i801: fix unused-function warning
2018-05-14 9:33 ` [PATCH v2] i2c: i801: fix unused-function warning Anders Roxell
2018-05-14 15:02 ` Jean Delvare
2018-05-14 17:18 ` Andy Shevchenko
@ 2018-05-17 13:54 ` Wolfram Sang
2 siblings, 0 replies; 8+ messages in thread
From: Wolfram Sang @ 2018-05-17 13:54 UTC (permalink / raw)
To: Anders Roxell; +Cc: jdelvare, linux-i2c, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 871 bytes --]
On Mon, May 14, 2018 at 11:33:26AM +0200, Anders Roxell wrote:
> With CONFIG_PM, we get a harmless build warning:
> drivers/i2c/busses/i2c-i801.c:1723:12: warning: ‘i801_resume’ defined but not used [-Wunused-function]
> static int i801_resume(struct device *dev)
> ^~~~~~~~~~~
> drivers/i2c/busses/i2c-i801.c:1714:12: warning: ‘i801_suspend’ defined but not used [-Wunused-function]
> static int i801_suspend(struct device *dev)
> ^~~~~~~~~~~~
>
> Follow design pattern from other drivers like i2c-brcmstb, i2c-mpc,
> i2c-ocores, i2c-pnx, i2c-puv3, i2c-st, i2c-stu300 and i2c-mux-pca954x
> and changing the ifdef CONFIG_PM to CONFIG_PM_SLEEP.
>
> Fixes: a9c8088c7988 ("i2c: i801: Don't restore config registers on runtime PM")
> Signed-off-by: Anders Roxell <anders.roxell@linaro.org>
Applied to for-next, thanks!
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2018-05-17 13:54 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-05-08 7:38 [PATCH] i2c: i801: mark PM functions as __maybe_unused Anders Roxell
2018-05-09 17:00 ` Jean Delvare
2018-05-10 13:27 ` Jean Delvare
2018-05-14 9:33 ` [PATCH v2] i2c: i801: fix unused-function warning Anders Roxell
2018-05-14 15:02 ` Jean Delvare
2018-05-14 17:18 ` Andy Shevchenko
2018-05-15 8:16 ` Jean Delvare
2018-05-17 13:54 ` Wolfram Sang
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).