diff mbox

[v2,19/19] fbdev: sh-mobile-lcdcfb: Enable the driver on all ARM platforms

Message ID alpine.DEB.2.02.1310312039330.13521@ayla.of.borg
State New
Headers show

Commit Message

Geert Uytterhoeven Oct. 31, 2013, 7:42 p.m. UTC 
Hi Laurent,

On Tue, 29 Oct 2013, Laurent Pinchart wrote:
> Renesas ARM platforms are transitioning from single-platform to
> multi-platform kernels using the new ARCH_SHMOBILE_MULTI. Make the
> driver available on all ARM platforms to enable it on both ARCH_SHMOBILE
> and ARCH_SHMOBILE_MULTI, and increase build testing coverage with
> COMPILE_TEST.

> diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
> index 84b685f..32b5c86 100644
> --- a/drivers/video/Kconfig
> +++ b/drivers/video/Kconfig
> @@ -2482,7 +2482,7 @@ endif
>  
>  config FB_SH_MOBILE_MERAM
>  	tristate "SuperH Mobile MERAM read ahead support"
> -	depends on (SUPERH || ARCH_SHMOBILE)
> +	depends on (SUPERH || ARM || COMPILE_TEST)
>  	select GENERIC_ALLOCATOR
>  	---help---
>  	  Enable MERAM support for the SuperH controller.

While the below compiler warnings have been seen with sh-randconfig
before, they're more likely to happen with COMPILE_TEST=y.
I now see them with e.g. m68k-allmodconfig, so I created a patch.

From 15eb69172457c675cde177a6f742b6f1dabdeb18 Mon Sep 17 00:00:00 2001
From: Geert Uytterhoeven <geert@linux-m68k.org>
Date: Thu, 31 Oct 2013 20:35:14 +0100
Subject: [PATCH] fbdev: sh_mobile_meram: Fix defined but not used compiler
 warnings
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

If CONFIG_PM_SLEEP resp. CONFIG_PM_RUNTIME are not set:

drivers/video/sh_mobile_meram.c:573: warning: ‘sh_mobile_meram_suspend’ defined but not used
drivers/video/sh_mobile_meram.c:597: warning: ‘sh_mobile_meram_resume’ defined but not used

Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
---
 drivers/video/sh_mobile_meram.c |    4 ++++
 1 file changed, 4 insertions(+)

Comments

Laurent Pinchart Nov. 6, 2013, 12:27 a.m. UTC | #1 
Hi Geert,

Thank you for the patch.

On Thursday 31 October 2013 20:42:52 Geert Uytterhoeven wrote:
> On Tue, 29 Oct 2013, Laurent Pinchart wrote:
> > Renesas ARM platforms are transitioning from single-platform to
> > multi-platform kernels using the new ARCH_SHMOBILE_MULTI. Make the
> > driver available on all ARM platforms to enable it on both ARCH_SHMOBILE
> > and ARCH_SHMOBILE_MULTI, and increase build testing coverage with
> > COMPILE_TEST.
> > 
> > diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
> > index 84b685f..32b5c86 100644
> > --- a/drivers/video/Kconfig
> > +++ b/drivers/video/Kconfig
> > @@ -2482,7 +2482,7 @@ endif
> > 
> >  config FB_SH_MOBILE_MERAM
> >  
> >  	tristate "SuperH Mobile MERAM read ahead support"
> > 
> > -	depends on (SUPERH || ARCH_SHMOBILE)
> > +	depends on (SUPERH || ARM || COMPILE_TEST)
> > 
> >  	select GENERIC_ALLOCATOR
> >  	---help---
> >  	
> >  	  Enable MERAM support for the SuperH controller.
> 
> While the below compiler warnings have been seen with sh-randconfig
> before, they're more likely to happen with COMPILE_TEST=y.
> I now see them with e.g. m68k-allmodconfig, so I created a patch.
> 
> From 15eb69172457c675cde177a6f742b6f1dabdeb18 Mon Sep 17 00:00:00 2001
> From: Geert Uytterhoeven <geert@linux-m68k.org>
> Date: Thu, 31 Oct 2013 20:35:14 +0100
> Subject: [PATCH] fbdev: sh_mobile_meram: Fix defined but not used compiler
>  warnings
> MIME-Version: 1.0
> Content-Type: text/plain; charset=UTF-8
> Content-Transfer-Encoding: 8bit
> 
> If CONFIG_PM_SLEEP resp. CONFIG_PM_RUNTIME are not set:
> 
> drivers/video/sh_mobile_meram.c:573: warning: ‘sh_mobile_meram_suspend’
> defined but not used drivers/video/sh_mobile_meram.c:597: warning:
> ‘sh_mobile_meram_resume’ defined but not used
> 
> Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
> ---
>  drivers/video/sh_mobile_meram.c |    4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/video/sh_mobile_meram.c
> b/drivers/video/sh_mobile_meram.c index e0f098562a74..56a8e47ffceb 100644
> --- a/drivers/video/sh_mobile_meram.c
> +++ b/drivers/video/sh_mobile_meram.c
> @@ -569,6 +569,7 @@ EXPORT_SYMBOL_GPL(sh_mobile_meram_cache_update);
>   * Power management
>   */
> 
> +#ifdef CONFIG_PM_SLEEP
>  static int sh_mobile_meram_suspend(struct device *dev)
>  {
>  	struct platform_device *pdev = to_platform_device(dev);
> @@ -592,7 +593,9 @@ static int sh_mobile_meram_suspend(struct device *dev)
>  	}
>  	return 0;
>  }
> +#endif
> 
> +#ifdef CONFIG_PM_RUNTIME
>  static int sh_mobile_meram_resume(struct device *dev)
>  {
>  	struct platform_device *pdev = to_platform_device(dev);
> @@ -611,6 +614,7 @@ static int sh_mobile_meram_resume(struct device *dev)
>  		meram_write_reg(priv->base, common_regs[i], priv->regs[i]);
>  	return 0;
>  }
> +#endif
> 
>  static UNIVERSAL_DEV_PM_OPS(sh_mobile_meram_dev_pm_ops,
>  			    sh_mobile_meram_suspend,

I'm a bit surprised, looking at the definition of UNIVERSAL_DEV_PM_OPS, I 
would have thought that both functions would be used when either 
CONFIG_PM_SLEEP or CONFIG_PM_RUNTIME is defined. I would thus have guarded 
both functions with #if defined(CONFIG_PM_SLEEP) || 
defined(CONFIG_PM_RUNTIME).
Geert Uytterhoeven Nov. 6, 2013, 8:46 a.m. UTC | #2 
On Wed, Nov 6, 2013 at 1:27 AM, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
>> +#ifdef CONFIG_PM_SLEEP
>>  static int sh_mobile_meram_suspend(struct device *dev)
>>  {
>>       struct platform_device *pdev = to_platform_device(dev);
>> @@ -592,7 +593,9 @@ static int sh_mobile_meram_suspend(struct device *dev)
>>       }
>>       return 0;
>>  }
>> +#endif
>>
>> +#ifdef CONFIG_PM_RUNTIME
>>  static int sh_mobile_meram_resume(struct device *dev)
>>  {
>>       struct platform_device *pdev = to_platform_device(dev);
>> @@ -611,6 +614,7 @@ static int sh_mobile_meram_resume(struct device *dev)
>>               meram_write_reg(priv->base, common_regs[i], priv->regs[i]);
>>       return 0;
>>  }
>> +#endif
>>
>>  static UNIVERSAL_DEV_PM_OPS(sh_mobile_meram_dev_pm_ops,
>>                           sh_mobile_meram_suspend,
>
> I'm a bit surprised, looking at the definition of UNIVERSAL_DEV_PM_OPS, I
> would have thought that both functions would be used when either
> CONFIG_PM_SLEEP or CONFIG_PM_RUNTIME is defined. I would thus have guarded
> both functions with #if defined(CONFIG_PM_SLEEP) ||
> defined(CONFIG_PM_RUNTIME).

You're right. I missed that both function pointers are passed to both macros.

Will send v2.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
diff mbox

Patch

diff --git a/drivers/video/sh_mobile_meram.c b/drivers/video/sh_mobile_meram.c
index e0f098562a74..56a8e47ffceb 100644
--- a/drivers/video/sh_mobile_meram.c
+++ b/drivers/video/sh_mobile_meram.c
@@ -569,6 +569,7 @@  EXPORT_SYMBOL_GPL(sh_mobile_meram_cache_update);
  * Power management
  */
 
+#ifdef CONFIG_PM_SLEEP
 static int sh_mobile_meram_suspend(struct device *dev)
 {
 	struct platform_device *pdev = to_platform_device(dev);
@@ -592,7 +593,9 @@  static int sh_mobile_meram_suspend(struct device *dev)
 	}
 	return 0;
 }
+#endif
 
+#ifdef CONFIG_PM_RUNTIME
 static int sh_mobile_meram_resume(struct device *dev)
 {
 	struct platform_device *pdev = to_platform_device(dev);
@@ -611,6 +614,7 @@  static int sh_mobile_meram_resume(struct device *dev)
 		meram_write_reg(priv->base, common_regs[i], priv->regs[i]);
 	return 0;
 }
+#endif
 
 static UNIVERSAL_DEV_PM_OPS(sh_mobile_meram_dev_pm_ops,
 			    sh_mobile_meram_suspend,