Re: [PATCH] mmc: sdhci-esdhc-imx: avoid unused function warnings

[Arnd Bergmann]

On Wednesday, July 27, 2016 12:48:23 AM CEST Ulf Hansson wrote:
> On 26 July 2016 at 22:56, Arnd Bergmann  wrote:
> > On Tuesday, July 26, 2016 10:18:53 PM CEST Ulf Hansson wrote:
> >> > diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c 
> >> > b/drivers/mmc/host/sdhci-esdhc-imx.c
> >> > index 2bb326bbc34a..593e34053c4b 100644
> >> > --- a/drivers/mmc/host/sdhci-esdhc-imx.c
> >> > +++ b/drivers/mmc/host/sdhci-esdhc-imx.c
> >> > @@ -1293,13 +1293,12 @@ static int sdhci_esdhc_imx_remove(struct 
> >> > platform_device *pdev)
> >> > return 0;
> >> >  }
> >> >
> >> > -#ifdef CONFIG_PM
> >> > -static int sdhci_esdhc_suspend(struct device *dev)
> >> > +static int __maybe_unused sdhci_esdhc_suspend(struct device *dev)
> >>
> >> Instead of using __maybe_unused, I prefer to change above "#ifdef
> >> CONFIG_PMf" to "#ifdef CONFIG_PM_SLEEP".
> >> I do realize that the runtime PM callbacks still requires #ifdef
> >> CONFIG_PM, so yes that's requires an extra "ifdef".
> >>
> >> Sure, it's more a matter of taste (and micro optimizations).
> >
> > I was hoping that we could eventually do a mass-conversion to
> > __maybe_unused, as everybody seems to get the #ifdef wrong.
> >
> > Any specific reason for your preference?
> 
> Only that this is how I get used to do it - and that it becomes a bit
> more clear what is needed to support the various PM configurations.
> 
> If you still insist on the "maybe_unused" option, that's okay as well.

I only have a mild preference for __maybe_unused because it's harder
to get wrong. I have sent dozens of bugfixes for drivers that have
incorrect #ifdef, and I'd prefer not having to send that many in the
future, so I try to convert them to __maybe_unused whenever a problem
shows up to slowly change the coding style in the kernel.

Build time coverage is another point here, but that is also what caused
problems in my patch since the declarations were hidden (I assume not
intentionally).

Arnd

Re: [PATCH] mmc: sdhci-esdhc-imx: avoid unused function warnings

[Arnd Bergmann]

On Wednesday, July 27, 2016 12:48:23 AM CEST Ulf Hansson wrote:
> On 26 July 2016 at 22:56, Arnd Bergmann  wrote:
> > On Tuesday, July 26, 2016 10:18:53 PM CEST Ulf Hansson wrote:
> >> > diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c 
> >> > b/drivers/mmc/host/sdhci-esdhc-imx.c
> >> > index 2bb326bbc34a..593e34053c4b 100644
> >> > --- a/drivers/mmc/host/sdhci-esdhc-imx.c
> >> > +++ b/drivers/mmc/host/sdhci-esdhc-imx.c
> >> > @@ -1293,13 +1293,12 @@ static int sdhci_esdhc_imx_remove(struct 
> >> > platform_device *pdev)
> >> > return 0;
> >> >  }
> >> >
> >> > -#ifdef CONFIG_PM
> >> > -static int sdhci_esdhc_suspend(struct device *dev)
> >> > +static int __maybe_unused sdhci_esdhc_suspend(struct device *dev)
> >>
> >> Instead of using __maybe_unused, I prefer to change above "#ifdef
> >> CONFIG_PMf" to "#ifdef CONFIG_PM_SLEEP".
> >> I do realize that the runtime PM callbacks still requires #ifdef
> >> CONFIG_PM, so yes that's requires an extra "ifdef".
> >>
> >> Sure, it's more a matter of taste (and micro optimizations).
> >
> > I was hoping that we could eventually do a mass-conversion to
> > __maybe_unused, as everybody seems to get the #ifdef wrong.
> >
> > Any specific reason for your preference?
> 
> Only that this is how I get used to do it - and that it becomes a bit
> more clear what is needed to support the various PM configurations.
> 
> If you still insist on the "maybe_unused" option, that's okay as well.

I only have a mild preference for __maybe_unused because it's harder
to get wrong. I have sent dozens of bugfixes for drivers that have
incorrect #ifdef, and I'd prefer not having to send that many in the
future, so I try to convert them to __maybe_unused whenever a problem
shows up to slowly change the coding style in the kernel.

Build time coverage is another point here, but that is also what caused
problems in my patch since the declarations were hidden (I assume not
intentionally).

Arnd

Re: [PATCH] mmc: sdhci-esdhc-imx: avoid unused function warnings

[Arnd Bergmann]

On Tuesday, July 26, 2016 2:32:34 PM CEST Arnd Bergmann wrote:
> The driver has just gained a slightly incorrect pm-sleep implementation that 
> causes
> warnings when CONFIG_PM is set but CONFIG_PM_SLEEP is not:
> 
> drivers/mmc/host/sdhci-esdhc-imx.c:1302:12: error: 'sdhci_esdhc_resume' 
> defined but not used [-Werror=unused-function]
>  static int sdhci_esdhc_resume(struct device *dev)
> ^~
> drivers/mmc/host/sdhci-esdhc-imx.c:1297:12: error: 'sdhci_esdhc_suspend' 
> defined but not used [-Werror=unused-function]
>  static int sdhci_esdhc_suspend(struct device *dev)
> 
> This replaces the incorrect #ifdef with a __maybe_unused annotation that does
> the right thing in all configurations and is more readable.
> 
> Signed-off-by: Arnd Bergmann 
> Fixes: b70d0b3b5b29 ("mmc: sdhci-esdhc-imx: add esdhc specific suspend resume 
> callback")


It turns out that the version of the fix above requires another change
below. Fixing up the #ifdef as Ulf suggested would not require that,
but on the other hand, we generally don't hide declarations in header
files unless there is an alternative in the #else path, so we
probably want to do that anyway.

We could either fold this patch into the one I sent already, or
have this one applied separately and use the version that Ulf
preferred for the warning fix. I can send whichever we decide to
do with a proper changelog once we have consensus.

Signed-off-by: Arnd Bergmann 

diff --git a/drivers/mmc/host/sdhci-pltfm.h b/drivers/mmc/host/sdhci-pltfm.h
index d38053bf9e4d..173ef914281d 100644
--- a/drivers/mmc/host/sdhci-pltfm.h
+++ b/drivers/mmc/host/sdhci-pltfm.h
@@ -109,9 +109,9 @@ static inline void *sdhci_pltfm_priv(struct 
sdhci_pltfm_host *host)
return (void *)host->private;
 }
 
-#ifdef CONFIG_PM
 extern int sdhci_pltfm_suspend(struct device *dev);
 extern int sdhci_pltfm_resume(struct device *dev);
+#ifdef CONFIG_PM
 extern const struct dev_pm_ops sdhci_pltfm_pmops;
 #define SDHCI_PLTFM_PMOPS (_pltfm_pmops)
 #else
diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
index 0411c9f36461..9a77cb009fc8 100644
--- a/drivers/mmc/host/sdhci.h
+++ b/drivers/mmc/host/sdhci.h
@@ -686,12 +686,10 @@ void sdhci_set_bus_width(struct sdhci_host *host, int 
width);
 void sdhci_reset(struct sdhci_host *host, u8 mask);
 void sdhci_set_uhs_signaling(struct sdhci_host *host, unsigned timing);
 
-#ifdef CONFIG_PM
 extern int sdhci_suspend_host(struct sdhci_host *host);
 extern int sdhci_resume_host(struct sdhci_host *host);
 extern void sdhci_enable_irq_wakeups(struct sdhci_host *host);
 extern int sdhci_runtime_suspend_host(struct sdhci_host *host);
 extern int sdhci_runtime_resume_host(struct sdhci_host *host);
-#endif
 
 #endif /* __SDHCI_HW_H */

Re: [PATCH] mmc: sdhci-esdhc-imx: avoid unused function warnings

[Arnd Bergmann]

On Tuesday, July 26, 2016 2:32:34 PM CEST Arnd Bergmann wrote:
> The driver has just gained a slightly incorrect pm-sleep implementation that 
> causes
> warnings when CONFIG_PM is set but CONFIG_PM_SLEEP is not:
> 
> drivers/mmc/host/sdhci-esdhc-imx.c:1302:12: error: 'sdhci_esdhc_resume' 
> defined but not used [-Werror=unused-function]
>  static int sdhci_esdhc_resume(struct device *dev)
> ^~
> drivers/mmc/host/sdhci-esdhc-imx.c:1297:12: error: 'sdhci_esdhc_suspend' 
> defined but not used [-Werror=unused-function]
>  static int sdhci_esdhc_suspend(struct device *dev)
> 
> This replaces the incorrect #ifdef with a __maybe_unused annotation that does
> the right thing in all configurations and is more readable.
> 
> Signed-off-by: Arnd Bergmann 
> Fixes: b70d0b3b5b29 ("mmc: sdhci-esdhc-imx: add esdhc specific suspend resume 
> callback")


It turns out that the version of the fix above requires another change
below. Fixing up the #ifdef as Ulf suggested would not require that,
but on the other hand, we generally don't hide declarations in header
files unless there is an alternative in the #else path, so we
probably want to do that anyway.

We could either fold this patch into the one I sent already, or
have this one applied separately and use the version that Ulf
preferred for the warning fix. I can send whichever we decide to
do with a proper changelog once we have consensus.

Signed-off-by: Arnd Bergmann 

diff --git a/drivers/mmc/host/sdhci-pltfm.h b/drivers/mmc/host/sdhci-pltfm.h
index d38053bf9e4d..173ef914281d 100644
--- a/drivers/mmc/host/sdhci-pltfm.h
+++ b/drivers/mmc/host/sdhci-pltfm.h
@@ -109,9 +109,9 @@ static inline void *sdhci_pltfm_priv(struct 
sdhci_pltfm_host *host)
return (void *)host->private;
 }
 
-#ifdef CONFIG_PM
 extern int sdhci_pltfm_suspend(struct device *dev);
 extern int sdhci_pltfm_resume(struct device *dev);
+#ifdef CONFIG_PM
 extern const struct dev_pm_ops sdhci_pltfm_pmops;
 #define SDHCI_PLTFM_PMOPS (_pltfm_pmops)
 #else
diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
index 0411c9f36461..9a77cb009fc8 100644
--- a/drivers/mmc/host/sdhci.h
+++ b/drivers/mmc/host/sdhci.h
@@ -686,12 +686,10 @@ void sdhci_set_bus_width(struct sdhci_host *host, int 
width);
 void sdhci_reset(struct sdhci_host *host, u8 mask);
 void sdhci_set_uhs_signaling(struct sdhci_host *host, unsigned timing);
 
-#ifdef CONFIG_PM
 extern int sdhci_suspend_host(struct sdhci_host *host);
 extern int sdhci_resume_host(struct sdhci_host *host);
 extern void sdhci_enable_irq_wakeups(struct sdhci_host *host);
 extern int sdhci_runtime_suspend_host(struct sdhci_host *host);
 extern int sdhci_runtime_resume_host(struct sdhci_host *host);
-#endif
 
 #endif /* __SDHCI_HW_H */

Re: [PATCH] mmc: sdhci-esdhc-imx: avoid unused function warnings

[Ulf Hansson]

On 26 July 2016 at 22:56, Arnd Bergmann  wrote:
> On Tuesday, July 26, 2016 10:18:53 PM CEST Ulf Hansson wrote:
>> > diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c 
>> > b/drivers/mmc/host/sdhci-esdhc-imx.c
>> > index 2bb326bbc34a..593e34053c4b 100644
>> > --- a/drivers/mmc/host/sdhci-esdhc-imx.c
>> > +++ b/drivers/mmc/host/sdhci-esdhc-imx.c
>> > @@ -1293,13 +1293,12 @@ static int sdhci_esdhc_imx_remove(struct 
>> > platform_device *pdev)
>> > return 0;
>> >  }
>> >
>> > -#ifdef CONFIG_PM
>> > -static int sdhci_esdhc_suspend(struct device *dev)
>> > +static int __maybe_unused sdhci_esdhc_suspend(struct device *dev)
>>
>> Instead of using __maybe_unused, I prefer to change above "#ifdef
>> CONFIG_PMf" to "#ifdef CONFIG_PM_SLEEP".
>> I do realize that the runtime PM callbacks still requires #ifdef
>> CONFIG_PM, so yes that's requires an extra "ifdef".
>>
>> Sure, it's more a matter of taste (and micro optimizations).
>
> I was hoping that we could eventually do a mass-conversion to
> __maybe_unused, as everybody seems to get the #ifdef wrong.
>
> Any specific reason for your preference?

Only that this is how I get used to do it - and that it becomes a bit
more clear what is needed to support the various PM configurations.

If you still insist on the "maybe_unused" option, that's okay as well.

Kind regards
Uffe

Re: [PATCH] mmc: sdhci-esdhc-imx: avoid unused function warnings

[Ulf Hansson]

On 26 July 2016 at 22:56, Arnd Bergmann  wrote:
> On Tuesday, July 26, 2016 10:18:53 PM CEST Ulf Hansson wrote:
>> > diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c 
>> > b/drivers/mmc/host/sdhci-esdhc-imx.c
>> > index 2bb326bbc34a..593e34053c4b 100644
>> > --- a/drivers/mmc/host/sdhci-esdhc-imx.c
>> > +++ b/drivers/mmc/host/sdhci-esdhc-imx.c
>> > @@ -1293,13 +1293,12 @@ static int sdhci_esdhc_imx_remove(struct 
>> > platform_device *pdev)
>> > return 0;
>> >  }
>> >
>> > -#ifdef CONFIG_PM
>> > -static int sdhci_esdhc_suspend(struct device *dev)
>> > +static int __maybe_unused sdhci_esdhc_suspend(struct device *dev)
>>
>> Instead of using __maybe_unused, I prefer to change above "#ifdef
>> CONFIG_PMf" to "#ifdef CONFIG_PM_SLEEP".
>> I do realize that the runtime PM callbacks still requires #ifdef
>> CONFIG_PM, so yes that's requires an extra "ifdef".
>>
>> Sure, it's more a matter of taste (and micro optimizations).
>
> I was hoping that we could eventually do a mass-conversion to
> __maybe_unused, as everybody seems to get the #ifdef wrong.
>
> Any specific reason for your preference?

Only that this is how I get used to do it - and that it becomes a bit
more clear what is needed to support the various PM configurations.

If you still insist on the "maybe_unused" option, that's okay as well.

Kind regards
Uffe

Re: [PATCH] mmc: sdhci-esdhc-imx: avoid unused function warnings

[Arnd Bergmann]

On Tuesday, July 26, 2016 10:18:53 PM CEST Ulf Hansson wrote:
> > diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c 
> > b/drivers/mmc/host/sdhci-esdhc-imx.c
> > index 2bb326bbc34a..593e34053c4b 100644
> > --- a/drivers/mmc/host/sdhci-esdhc-imx.c
> > +++ b/drivers/mmc/host/sdhci-esdhc-imx.c
> > @@ -1293,13 +1293,12 @@ static int sdhci_esdhc_imx_remove(struct 
> > platform_device *pdev)
> > return 0;
> >  }
> >
> > -#ifdef CONFIG_PM
> > -static int sdhci_esdhc_suspend(struct device *dev)
> > +static int __maybe_unused sdhci_esdhc_suspend(struct device *dev)
> 
> Instead of using __maybe_unused, I prefer to change above "#ifdef
> CONFIG_PMf" to "#ifdef CONFIG_PM_SLEEP".
> I do realize that the runtime PM callbacks still requires #ifdef
> CONFIG_PM, so yes that's requires an extra "ifdef".
> 
> Sure, it's more a matter of taste (and micro optimizations).

I was hoping that we could eventually do a mass-conversion to
__maybe_unused, as everybody seems to get the #ifdef wrong.

Any specific reason for your preference?

Arnd

Re: [PATCH] mmc: sdhci-esdhc-imx: avoid unused function warnings

[Arnd Bergmann]

On Tuesday, July 26, 2016 10:18:53 PM CEST Ulf Hansson wrote:
> > diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c 
> > b/drivers/mmc/host/sdhci-esdhc-imx.c
> > index 2bb326bbc34a..593e34053c4b 100644
> > --- a/drivers/mmc/host/sdhci-esdhc-imx.c
> > +++ b/drivers/mmc/host/sdhci-esdhc-imx.c
> > @@ -1293,13 +1293,12 @@ static int sdhci_esdhc_imx_remove(struct 
> > platform_device *pdev)
> > return 0;
> >  }
> >
> > -#ifdef CONFIG_PM
> > -static int sdhci_esdhc_suspend(struct device *dev)
> > +static int __maybe_unused sdhci_esdhc_suspend(struct device *dev)
> 
> Instead of using __maybe_unused, I prefer to change above "#ifdef
> CONFIG_PMf" to "#ifdef CONFIG_PM_SLEEP".
> I do realize that the runtime PM callbacks still requires #ifdef
> CONFIG_PM, so yes that's requires an extra "ifdef".
> 
> Sure, it's more a matter of taste (and micro optimizations).

I was hoping that we could eventually do a mass-conversion to
__maybe_unused, as everybody seems to get the #ifdef wrong.

Any specific reason for your preference?

Arnd

Re: [PATCH] mmc: sdhci-esdhc-imx: avoid unused function warnings

[Ulf Hansson]

On 26 July 2016 at 14:32, Arnd Bergmann  wrote:
> The driver has just gained a slightly incorrect pm-sleep implementation that 
> causes
> warnings when CONFIG_PM is set but CONFIG_PM_SLEEP is not:
>
> drivers/mmc/host/sdhci-esdhc-imx.c:1302:12: error: 'sdhci_esdhc_resume' 
> defined but not used [-Werror=unused-function]
>  static int sdhci_esdhc_resume(struct device *dev)
> ^~
> drivers/mmc/host/sdhci-esdhc-imx.c:1297:12: error: 'sdhci_esdhc_suspend' 
> defined but not used [-Werror=unused-function]
>  static int sdhci_esdhc_suspend(struct device *dev)
>
> This replaces the incorrect #ifdef with a __maybe_unused annotation that does
> the right thing in all configurations and is more readable.
>
> Signed-off-by: Arnd Bergmann 
> Fixes: b70d0b3b5b29 ("mmc: sdhci-esdhc-imx: add esdhc specific suspend resume 
> callback")
> ---
>  drivers/mmc/host/sdhci-esdhc-imx.c | 10 --
>  1 file changed, 4 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c 
> b/drivers/mmc/host/sdhci-esdhc-imx.c
> index 2bb326bbc34a..593e34053c4b 100644
> --- a/drivers/mmc/host/sdhci-esdhc-imx.c
> +++ b/drivers/mmc/host/sdhci-esdhc-imx.c
> @@ -1293,13 +1293,12 @@ static int sdhci_esdhc_imx_remove(struct 
> platform_device *pdev)
> return 0;
>  }
>
> -#ifdef CONFIG_PM
> -static int sdhci_esdhc_suspend(struct device *dev)
> +static int __maybe_unused sdhci_esdhc_suspend(struct device *dev)

Instead of using __maybe_unused, I prefer to change above "#ifdef
CONFIG_PMf" to "#ifdef CONFIG_PM_SLEEP".
I do realize that the runtime PM callbacks still requires #ifdef
CONFIG_PM, so yes that's requires an extra "ifdef".

Sure, it's more a matter of taste (and micro optimizations).

>  {
> return sdhci_pltfm_suspend(dev);
>  }
>
> -static int sdhci_esdhc_resume(struct device *dev)
> +static int __maybe_unused sdhci_esdhc_resume(struct device *dev)
>  {
> struct sdhci_host *host = dev_get_drvdata(dev);
>
> @@ -1309,7 +1308,7 @@ static int sdhci_esdhc_resume(struct device *dev)
> return sdhci_pltfm_resume(dev);
>  }
>
> -static int sdhci_esdhc_runtime_suspend(struct device *dev)
> +static int __maybe_unused sdhci_esdhc_runtime_suspend(struct device *dev)
>  {
> struct sdhci_host *host = dev_get_drvdata(dev);
> struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> @@ -1327,7 +1326,7 @@ static int sdhci_esdhc_runtime_suspend(struct device 
> *dev)
> return ret;
>  }
>
> -static int sdhci_esdhc_runtime_resume(struct device *dev)
> +static int __maybe_unused sdhci_esdhc_runtime_resume(struct device *dev)
>  {
> struct sdhci_host *host = dev_get_drvdata(dev);
> struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> @@ -1341,7 +1340,6 @@ static int sdhci_esdhc_runtime_resume(struct device 
> *dev)
>
> return sdhci_runtime_resume_host(host);
>  }
> -#endif
>
>  static const struct dev_pm_ops sdhci_esdhc_pmops = {
> SET_SYSTEM_SLEEP_PM_OPS(sdhci_esdhc_suspend, sdhci_esdhc_resume)
> --
> 2.9.0
>

Kind regards
Uffe

Re: [PATCH] mmc: sdhci-esdhc-imx: avoid unused function warnings

[Ulf Hansson]

On 26 July 2016 at 14:32, Arnd Bergmann  wrote:
> The driver has just gained a slightly incorrect pm-sleep implementation that 
> causes
> warnings when CONFIG_PM is set but CONFIG_PM_SLEEP is not:
>
> drivers/mmc/host/sdhci-esdhc-imx.c:1302:12: error: 'sdhci_esdhc_resume' 
> defined but not used [-Werror=unused-function]
>  static int sdhci_esdhc_resume(struct device *dev)
> ^~
> drivers/mmc/host/sdhci-esdhc-imx.c:1297:12: error: 'sdhci_esdhc_suspend' 
> defined but not used [-Werror=unused-function]
>  static int sdhci_esdhc_suspend(struct device *dev)
>
> This replaces the incorrect #ifdef with a __maybe_unused annotation that does
> the right thing in all configurations and is more readable.
>
> Signed-off-by: Arnd Bergmann 
> Fixes: b70d0b3b5b29 ("mmc: sdhci-esdhc-imx: add esdhc specific suspend resume 
> callback")
> ---
>  drivers/mmc/host/sdhci-esdhc-imx.c | 10 --
>  1 file changed, 4 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c 
> b/drivers/mmc/host/sdhci-esdhc-imx.c
> index 2bb326bbc34a..593e34053c4b 100644
> --- a/drivers/mmc/host/sdhci-esdhc-imx.c
> +++ b/drivers/mmc/host/sdhci-esdhc-imx.c
> @@ -1293,13 +1293,12 @@ static int sdhci_esdhc_imx_remove(struct 
> platform_device *pdev)
> return 0;
>  }
>
> -#ifdef CONFIG_PM
> -static int sdhci_esdhc_suspend(struct device *dev)
> +static int __maybe_unused sdhci_esdhc_suspend(struct device *dev)

Instead of using __maybe_unused, I prefer to change above "#ifdef
CONFIG_PMf" to "#ifdef CONFIG_PM_SLEEP".
I do realize that the runtime PM callbacks still requires #ifdef
CONFIG_PM, so yes that's requires an extra "ifdef".

Sure, it's more a matter of taste (and micro optimizations).

>  {
> return sdhci_pltfm_suspend(dev);
>  }
>
> -static int sdhci_esdhc_resume(struct device *dev)
> +static int __maybe_unused sdhci_esdhc_resume(struct device *dev)
>  {
> struct sdhci_host *host = dev_get_drvdata(dev);
>
> @@ -1309,7 +1308,7 @@ static int sdhci_esdhc_resume(struct device *dev)
> return sdhci_pltfm_resume(dev);
>  }
>
> -static int sdhci_esdhc_runtime_suspend(struct device *dev)
> +static int __maybe_unused sdhci_esdhc_runtime_suspend(struct device *dev)
>  {
> struct sdhci_host *host = dev_get_drvdata(dev);
> struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> @@ -1327,7 +1326,7 @@ static int sdhci_esdhc_runtime_suspend(struct device 
> *dev)
> return ret;
>  }
>
> -static int sdhci_esdhc_runtime_resume(struct device *dev)
> +static int __maybe_unused sdhci_esdhc_runtime_resume(struct device *dev)
>  {
> struct sdhci_host *host = dev_get_drvdata(dev);
> struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> @@ -1341,7 +1340,6 @@ static int sdhci_esdhc_runtime_resume(struct device 
> *dev)
>
> return sdhci_runtime_resume_host(host);
>  }
> -#endif
>
>  static const struct dev_pm_ops sdhci_esdhc_pmops = {
> SET_SYSTEM_SLEEP_PM_OPS(sdhci_esdhc_suspend, sdhci_esdhc_resume)
> --
> 2.9.0
>

Kind regards
Uffe

Re: [PATCH] mmc: sdhci-esdhc-imx: avoid unused function warnings

[Arnd Bergmann]

On Tuesday, July 26, 2016 9:41:50 PM CEST Dong Aisheng wrote:
> 
> On Tue, Jul 26, 2016 at 8:32 PM, Arnd Bergmann  wrote:
> > The driver has just gained a slightly incorrect pm-sleep implementation 
> > that causes
> > warnings when CONFIG_PM is set but CONFIG_PM_SLEEP is not:
> >
> > drivers/mmc/host/sdhci-esdhc-imx.c:1302:12: error: 'sdhci_esdhc_resume' 
> > defined but not used [-Werror=unused-function]
> >  static int sdhci_esdhc_resume(struct device *dev)
> > ^~
> > drivers/mmc/host/sdhci-esdhc-imx.c:1297:12: error: 'sdhci_esdhc_suspend' 
> > defined but not used [-Werror=unused-function]
> >  static int sdhci_esdhc_suspend(struct device *dev)
> >
> > This replaces the incorrect #ifdef with a __maybe_unused annotation that 
> > does
> > the right thing in all configurations and is more readable.
> >
> > Signed-off-by: Arnd Bergmann 
> > Fixes: b70d0b3b5b29 ("mmc: sdhci-esdhc-imx: add esdhc specific suspend 
> > resume callback")
> 
> Thanks for the fix.
> Looks like the default kernel Makefile does not enable
> -Werror=unused-function wanrings,
> so did not expose it before.

They are enabled by default as warnings, not errors. My test build setup
uses -Werror to turn all warnings into errors so I can catch them
more easily as failed builds.

> Acked-by: Dong Aisheng 
> 

Thanks,

Arnd

Re: [PATCH] mmc: sdhci-esdhc-imx: avoid unused function warnings

[Arnd Bergmann]

On Tuesday, July 26, 2016 9:41:50 PM CEST Dong Aisheng wrote:
> 
> On Tue, Jul 26, 2016 at 8:32 PM, Arnd Bergmann  wrote:
> > The driver has just gained a slightly incorrect pm-sleep implementation 
> > that causes
> > warnings when CONFIG_PM is set but CONFIG_PM_SLEEP is not:
> >
> > drivers/mmc/host/sdhci-esdhc-imx.c:1302:12: error: 'sdhci_esdhc_resume' 
> > defined but not used [-Werror=unused-function]
> >  static int sdhci_esdhc_resume(struct device *dev)
> > ^~
> > drivers/mmc/host/sdhci-esdhc-imx.c:1297:12: error: 'sdhci_esdhc_suspend' 
> > defined but not used [-Werror=unused-function]
> >  static int sdhci_esdhc_suspend(struct device *dev)
> >
> > This replaces the incorrect #ifdef with a __maybe_unused annotation that 
> > does
> > the right thing in all configurations and is more readable.
> >
> > Signed-off-by: Arnd Bergmann 
> > Fixes: b70d0b3b5b29 ("mmc: sdhci-esdhc-imx: add esdhc specific suspend 
> > resume callback")
> 
> Thanks for the fix.
> Looks like the default kernel Makefile does not enable
> -Werror=unused-function wanrings,
> so did not expose it before.

They are enabled by default as warnings, not errors. My test build setup
uses -Werror to turn all warnings into errors so I can catch them
more easily as failed builds.

> Acked-by: Dong Aisheng 
> 

Thanks,

Arnd

Re: [PATCH] mmc: sdhci-esdhc-imx: avoid unused function warnings

[Dong Aisheng]

Hi Arnd,

On Tue, Jul 26, 2016 at 8:32 PM, Arnd Bergmann  wrote:
> The driver has just gained a slightly incorrect pm-sleep implementation that 
> causes
> warnings when CONFIG_PM is set but CONFIG_PM_SLEEP is not:
>
> drivers/mmc/host/sdhci-esdhc-imx.c:1302:12: error: 'sdhci_esdhc_resume' 
> defined but not used [-Werror=unused-function]
>  static int sdhci_esdhc_resume(struct device *dev)
> ^~
> drivers/mmc/host/sdhci-esdhc-imx.c:1297:12: error: 'sdhci_esdhc_suspend' 
> defined but not used [-Werror=unused-function]
>  static int sdhci_esdhc_suspend(struct device *dev)
>
> This replaces the incorrect #ifdef with a __maybe_unused annotation that does
> the right thing in all configurations and is more readable.
>
> Signed-off-by: Arnd Bergmann 
> Fixes: b70d0b3b5b29 ("mmc: sdhci-esdhc-imx: add esdhc specific suspend resume 
> callback")

Thanks for the fix.
Looks like the default kernel Makefile does not enable
-Werror=unused-function wanrings,
so did not expose it before.

Acked-by: Dong Aisheng 

Regards
Dong Aisheng

> ---
>  drivers/mmc/host/sdhci-esdhc-imx.c | 10 --
>  1 file changed, 4 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c 
> b/drivers/mmc/host/sdhci-esdhc-imx.c
> index 2bb326bbc34a..593e34053c4b 100644
> --- a/drivers/mmc/host/sdhci-esdhc-imx.c
> +++ b/drivers/mmc/host/sdhci-esdhc-imx.c
> @@ -1293,13 +1293,12 @@ static int sdhci_esdhc_imx_remove(struct 
> platform_device *pdev)
> return 0;
>  }
>
> -#ifdef CONFIG_PM
> -static int sdhci_esdhc_suspend(struct device *dev)
> +static int __maybe_unused sdhci_esdhc_suspend(struct device *dev)
>  {
> return sdhci_pltfm_suspend(dev);
>  }
>
> -static int sdhci_esdhc_resume(struct device *dev)
> +static int __maybe_unused sdhci_esdhc_resume(struct device *dev)
>  {
> struct sdhci_host *host = dev_get_drvdata(dev);
>
> @@ -1309,7 +1308,7 @@ static int sdhci_esdhc_resume(struct device *dev)
> return sdhci_pltfm_resume(dev);
>  }
>
> -static int sdhci_esdhc_runtime_suspend(struct device *dev)
> +static int __maybe_unused sdhci_esdhc_runtime_suspend(struct device *dev)
>  {
> struct sdhci_host *host = dev_get_drvdata(dev);
> struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> @@ -1327,7 +1326,7 @@ static int sdhci_esdhc_runtime_suspend(struct device 
> *dev)
> return ret;
>  }
>
> -static int sdhci_esdhc_runtime_resume(struct device *dev)
> +static int __maybe_unused sdhci_esdhc_runtime_resume(struct device *dev)
>  {
> struct sdhci_host *host = dev_get_drvdata(dev);
> struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> @@ -1341,7 +1340,6 @@ static int sdhci_esdhc_runtime_resume(struct device 
> *dev)
>
> return sdhci_runtime_resume_host(host);
>  }
> -#endif
>
>  static const struct dev_pm_ops sdhci_esdhc_pmops = {
> SET_SYSTEM_SLEEP_PM_OPS(sdhci_esdhc_suspend, sdhci_esdhc_resume)
> --
> 2.9.0
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] mmc: sdhci-esdhc-imx: avoid unused function warnings

[Dong Aisheng]

Hi Arnd,

On Tue, Jul 26, 2016 at 8:32 PM, Arnd Bergmann  wrote:
> The driver has just gained a slightly incorrect pm-sleep implementation that 
> causes
> warnings when CONFIG_PM is set but CONFIG_PM_SLEEP is not:
>
> drivers/mmc/host/sdhci-esdhc-imx.c:1302:12: error: 'sdhci_esdhc_resume' 
> defined but not used [-Werror=unused-function]
>  static int sdhci_esdhc_resume(struct device *dev)
> ^~
> drivers/mmc/host/sdhci-esdhc-imx.c:1297:12: error: 'sdhci_esdhc_suspend' 
> defined but not used [-Werror=unused-function]
>  static int sdhci_esdhc_suspend(struct device *dev)
>
> This replaces the incorrect #ifdef with a __maybe_unused annotation that does
> the right thing in all configurations and is more readable.
>
> Signed-off-by: Arnd Bergmann 
> Fixes: b70d0b3b5b29 ("mmc: sdhci-esdhc-imx: add esdhc specific suspend resume 
> callback")

Thanks for the fix.
Looks like the default kernel Makefile does not enable
-Werror=unused-function wanrings,
so did not expose it before.

Acked-by: Dong Aisheng 

Regards
Dong Aisheng

> ---
>  drivers/mmc/host/sdhci-esdhc-imx.c | 10 --
>  1 file changed, 4 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c 
> b/drivers/mmc/host/sdhci-esdhc-imx.c
> index 2bb326bbc34a..593e34053c4b 100644
> --- a/drivers/mmc/host/sdhci-esdhc-imx.c
> +++ b/drivers/mmc/host/sdhci-esdhc-imx.c
> @@ -1293,13 +1293,12 @@ static int sdhci_esdhc_imx_remove(struct 
> platform_device *pdev)
> return 0;
>  }
>
> -#ifdef CONFIG_PM
> -static int sdhci_esdhc_suspend(struct device *dev)
> +static int __maybe_unused sdhci_esdhc_suspend(struct device *dev)
>  {
> return sdhci_pltfm_suspend(dev);
>  }
>
> -static int sdhci_esdhc_resume(struct device *dev)
> +static int __maybe_unused sdhci_esdhc_resume(struct device *dev)
>  {
> struct sdhci_host *host = dev_get_drvdata(dev);
>
> @@ -1309,7 +1308,7 @@ static int sdhci_esdhc_resume(struct device *dev)
> return sdhci_pltfm_resume(dev);
>  }
>
> -static int sdhci_esdhc_runtime_suspend(struct device *dev)
> +static int __maybe_unused sdhci_esdhc_runtime_suspend(struct device *dev)
>  {
> struct sdhci_host *host = dev_get_drvdata(dev);
> struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> @@ -1327,7 +1326,7 @@ static int sdhci_esdhc_runtime_suspend(struct device 
> *dev)
> return ret;
>  }
>
> -static int sdhci_esdhc_runtime_resume(struct device *dev)
> +static int __maybe_unused sdhci_esdhc_runtime_resume(struct device *dev)
>  {
> struct sdhci_host *host = dev_get_drvdata(dev);
> struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> @@ -1341,7 +1340,6 @@ static int sdhci_esdhc_runtime_resume(struct device 
> *dev)
>
> return sdhci_runtime_resume_host(host);
>  }
> -#endif
>
>  static const struct dev_pm_ops sdhci_esdhc_pmops = {
> SET_SYSTEM_SLEEP_PM_OPS(sdhci_esdhc_suspend, sdhci_esdhc_resume)
> --
> 2.9.0
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] mmc: sdhci-esdhc-imx: avoid unused function warnings

[Arnd Bergmann]

The driver has just gained a slightly incorrect pm-sleep implementation that 
causes
warnings when CONFIG_PM is set but CONFIG_PM_SLEEP is not:

drivers/mmc/host/sdhci-esdhc-imx.c:1302:12: error: 'sdhci_esdhc_resume' defined 
but not used [-Werror=unused-function]
 static int sdhci_esdhc_resume(struct device *dev)
^~
drivers/mmc/host/sdhci-esdhc-imx.c:1297:12: error: 'sdhci_esdhc_suspend' 
defined but not used [-Werror=unused-function]
 static int sdhci_esdhc_suspend(struct device *dev)

This replaces the incorrect #ifdef with a __maybe_unused annotation that does
the right thing in all configurations and is more readable.

Signed-off-by: Arnd Bergmann 
Fixes: b70d0b3b5b29 ("mmc: sdhci-esdhc-imx: add esdhc specific suspend resume 
callback")
---
 drivers/mmc/host/sdhci-esdhc-imx.c | 10 --
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c 
b/drivers/mmc/host/sdhci-esdhc-imx.c
index 2bb326bbc34a..593e34053c4b 100644
--- a/drivers/mmc/host/sdhci-esdhc-imx.c
+++ b/drivers/mmc/host/sdhci-esdhc-imx.c
@@ -1293,13 +1293,12 @@ static int sdhci_esdhc_imx_remove(struct 
platform_device *pdev)
return 0;
 }
 
-#ifdef CONFIG_PM
-static int sdhci_esdhc_suspend(struct device *dev)
+static int __maybe_unused sdhci_esdhc_suspend(struct device *dev)
 {
return sdhci_pltfm_suspend(dev);
 }
 
-static int sdhci_esdhc_resume(struct device *dev)
+static int __maybe_unused sdhci_esdhc_resume(struct device *dev)
 {
struct sdhci_host *host = dev_get_drvdata(dev);
 
@@ -1309,7 +1308,7 @@ static int sdhci_esdhc_resume(struct device *dev)
return sdhci_pltfm_resume(dev);
 }
 
-static int sdhci_esdhc_runtime_suspend(struct device *dev)
+static int __maybe_unused sdhci_esdhc_runtime_suspend(struct device *dev)
 {
struct sdhci_host *host = dev_get_drvdata(dev);
struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
@@ -1327,7 +1326,7 @@ static int sdhci_esdhc_runtime_suspend(struct device *dev)
return ret;
 }
 
-static int sdhci_esdhc_runtime_resume(struct device *dev)
+static int __maybe_unused sdhci_esdhc_runtime_resume(struct device *dev)
 {
struct sdhci_host *host = dev_get_drvdata(dev);
struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
@@ -1341,7 +1340,6 @@ static int sdhci_esdhc_runtime_resume(struct device *dev)
 
return sdhci_runtime_resume_host(host);
 }
-#endif
 
 static const struct dev_pm_ops sdhci_esdhc_pmops = {
SET_SYSTEM_SLEEP_PM_OPS(sdhci_esdhc_suspend, sdhci_esdhc_resume)
-- 
2.9.0

[PATCH] mmc: sdhci-esdhc-imx: avoid unused function warnings

[Arnd Bergmann]

The driver has just gained a slightly incorrect pm-sleep implementation that 
causes
warnings when CONFIG_PM is set but CONFIG_PM_SLEEP is not:

drivers/mmc/host/sdhci-esdhc-imx.c:1302:12: error: 'sdhci_esdhc_resume' defined 
but not used [-Werror=unused-function]
 static int sdhci_esdhc_resume(struct device *dev)
^~
drivers/mmc/host/sdhci-esdhc-imx.c:1297:12: error: 'sdhci_esdhc_suspend' 
defined but not used [-Werror=unused-function]
 static int sdhci_esdhc_suspend(struct device *dev)

This replaces the incorrect #ifdef with a __maybe_unused annotation that does
the right thing in all configurations and is more readable.

Signed-off-by: Arnd Bergmann 
Fixes: b70d0b3b5b29 ("mmc: sdhci-esdhc-imx: add esdhc specific suspend resume 
callback")
---
 drivers/mmc/host/sdhci-esdhc-imx.c | 10 --
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c 
b/drivers/mmc/host/sdhci-esdhc-imx.c
index 2bb326bbc34a..593e34053c4b 100644
--- a/drivers/mmc/host/sdhci-esdhc-imx.c
+++ b/drivers/mmc/host/sdhci-esdhc-imx.c
@@ -1293,13 +1293,12 @@ static int sdhci_esdhc_imx_remove(struct 
platform_device *pdev)
return 0;
 }
 
-#ifdef CONFIG_PM
-static int sdhci_esdhc_suspend(struct device *dev)
+static int __maybe_unused sdhci_esdhc_suspend(struct device *dev)
 {
return sdhci_pltfm_suspend(dev);
 }
 
-static int sdhci_esdhc_resume(struct device *dev)
+static int __maybe_unused sdhci_esdhc_resume(struct device *dev)
 {
struct sdhci_host *host = dev_get_drvdata(dev);
 
@@ -1309,7 +1308,7 @@ static int sdhci_esdhc_resume(struct device *dev)
return sdhci_pltfm_resume(dev);
 }
 
-static int sdhci_esdhc_runtime_suspend(struct device *dev)
+static int __maybe_unused sdhci_esdhc_runtime_suspend(struct device *dev)
 {
struct sdhci_host *host = dev_get_drvdata(dev);
struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
@@ -1327,7 +1326,7 @@ static int sdhci_esdhc_runtime_suspend(struct device *dev)
return ret;
 }
 
-static int sdhci_esdhc_runtime_resume(struct device *dev)
+static int __maybe_unused sdhci_esdhc_runtime_resume(struct device *dev)
 {
struct sdhci_host *host = dev_get_drvdata(dev);
struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
@@ -1341,7 +1340,6 @@ static int sdhci_esdhc_runtime_resume(struct device *dev)
 
return sdhci_runtime_resume_host(host);
 }
-#endif
 
 static const struct dev_pm_ops sdhci_esdhc_pmops = {
SET_SYSTEM_SLEEP_PM_OPS(sdhci_esdhc_suspend, sdhci_esdhc_resume)
-- 
2.9.0