diff mbox series

[v2] i2c: i801: fix unused-function warning

Message ID 20180514093326.30314-1-anders.roxell@linaro.org
State Accepted
Commit 4b2f9bd5e39fb47011074c9a26b64b616acc18f0
Headers show
Series [v2] i2c: i801: fix unused-function warning | expand

Commit Message

Anders Roxell May 14, 2018, 9:33 a.m. UTC
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(-)

-- 
2.17.0

Comments

Jean Delvare May 14, 2018, 3:02 p.m. UTC | #1
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
Jean Delvare May 15, 2018, 8:16 a.m. UTC | #2
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
Wolfram Sang May 17, 2018, 1:54 p.m. UTC | #3
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!
diff mbox series

Patch

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);