[1/3] em-x270: don't use pxa_camera init() callback

Message ID 1258495463-26029-2-git-send-email-ospite@studenti.unina.it (mailing list archive)
State Superseded, archived
Headers

Commit Message

Antonio Ospite Nov. 17, 2009, 10:04 p.m. UTC
  pxa_camera init() is going to be removed.

Signed-off-by: Antonio Ospite <ospite@studenti.unina.it>
---
 arch/arm/mach-pxa/em-x270.c |    9 +++++----
 1 files changed, 5 insertions(+), 4 deletions(-)
  

Comments

Mike Rapoport Nov. 18, 2009, 6:34 a.m. UTC | #1
Antonio Ospite wrote:
> pxa_camera init() is going to be removed.
> 
> Signed-off-by: Antonio Ospite <ospite@studenti.unina.it>
> ---
>  arch/arm/mach-pxa/em-x270.c |    9 +++++----
>  1 files changed, 5 insertions(+), 4 deletions(-)

Acked-by: Mike Rapoport <mike@compulab.co.il>

> diff --git a/arch/arm/mach-pxa/em-x270.c b/arch/arm/mach-pxa/em-x270.c
> index aec7f42..f71f34c 100644
> --- a/arch/arm/mach-pxa/em-x270.c
> +++ b/arch/arm/mach-pxa/em-x270.c
> @@ -967,7 +967,7 @@ static inline void em_x270_init_gpio_keys(void) {}
>  #if defined(CONFIG_VIDEO_PXA27x) || defined(CONFIG_VIDEO_PXA27x_MODULE)
>  static struct regulator *em_x270_camera_ldo;
>  
> -static int em_x270_sensor_init(struct device *dev)
> +static int em_x270_sensor_init(void)
>  {
>  	int ret;
>  
> @@ -996,7 +996,6 @@ static int em_x270_sensor_init(struct device *dev)
>  }
>  
>  struct pxacamera_platform_data em_x270_camera_platform_data = {
> -	.init	= em_x270_sensor_init,
>  	.flags  = PXA_CAMERA_MASTER | PXA_CAMERA_DATAWIDTH_8 |
>  		PXA_CAMERA_PCLK_EN | PXA_CAMERA_MCLK_EN,
>  	.mclk_10khz = 2600,
> @@ -1049,8 +1048,10 @@ static struct platform_device em_x270_camera = {
>  
>  static void  __init em_x270_init_camera(void)
>  {
> -	pxa_set_camera_info(&em_x270_camera_platform_data);
> -	platform_device_register(&em_x270_camera);
> +	if (em_x270_sensor_init() == 0) {
> +		pxa_set_camera_info(&em_x270_camera_platform_data);
> +		platform_device_register(&em_x270_camera);
> +	}
>  }
>  #else
>  static inline void em_x270_init_camera(void) {}
  
Guennadi Liakhovetski Nov. 18, 2009, 10:10 a.m. UTC | #2
On Tue, 17 Nov 2009, Antonio Ospite wrote:

> pxa_camera init() is going to be removed.

My nitpick here would be - I would put it the other way round. We do not 
remove .init() in platforms, because it is going to be removed, but rather 
we perform initialisation statically, because we think this is better so, 
and then .init becomes useless and gets removed.

> 
> Signed-off-by: Antonio Ospite <ospite@studenti.unina.it>

Thanks
Guennadi

> ---
>  arch/arm/mach-pxa/em-x270.c |    9 +++++----
>  1 files changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/arm/mach-pxa/em-x270.c b/arch/arm/mach-pxa/em-x270.c
> index aec7f42..f71f34c 100644
> --- a/arch/arm/mach-pxa/em-x270.c
> +++ b/arch/arm/mach-pxa/em-x270.c
> @@ -967,7 +967,7 @@ static inline void em_x270_init_gpio_keys(void) {}
>  #if defined(CONFIG_VIDEO_PXA27x) || defined(CONFIG_VIDEO_PXA27x_MODULE)
>  static struct regulator *em_x270_camera_ldo;
>  
> -static int em_x270_sensor_init(struct device *dev)
> +static int em_x270_sensor_init(void)
>  {
>  	int ret;
>  
> @@ -996,7 +996,6 @@ static int em_x270_sensor_init(struct device *dev)
>  }
>  
>  struct pxacamera_platform_data em_x270_camera_platform_data = {
> -	.init	= em_x270_sensor_init,
>  	.flags  = PXA_CAMERA_MASTER | PXA_CAMERA_DATAWIDTH_8 |
>  		PXA_CAMERA_PCLK_EN | PXA_CAMERA_MCLK_EN,
>  	.mclk_10khz = 2600,
> @@ -1049,8 +1048,10 @@ static struct platform_device em_x270_camera = {
>  
>  static void  __init em_x270_init_camera(void)
>  {
> -	pxa_set_camera_info(&em_x270_camera_platform_data);
> -	platform_device_register(&em_x270_camera);
> +	if (em_x270_sensor_init() == 0) {
> +		pxa_set_camera_info(&em_x270_camera_platform_data);
> +		platform_device_register(&em_x270_camera);
> +	}
>  }
>  #else
>  static inline void em_x270_init_camera(void) {}
> -- 
> 1.6.5.2
> 

---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
  
Antonio Ospite Nov. 18, 2009, 5:02 p.m. UTC | #3
On Wed, 18 Nov 2009 11:10:06 +0100 (CET)
Guennadi Liakhovetski <g.liakhovetski@gmx.de> wrote:

> On Tue, 17 Nov 2009, Antonio Ospite wrote:
> 
> > pxa_camera init() is going to be removed.
> 
> My nitpick here would be - I would put it the other way round. We do not 
> remove .init() in platforms, because it is going to be removed, but rather 
> we perform initialisation statically, because we think this is better so, 
> and then .init becomes useless and gets removed.
> 

TBH, I am persuaded that the current use of init() is ambiguous /per se/
and so we'd just better not use it at all. If static initialization for
sensor GPIOs is better, well I just trust you on that.
However, the point here is not about static/dynamic initialization, it
is more about pxa_camera init() used one time to configure MFP pins, and
another time to request resources for the *sensor*, and in both cases
(mis)used as it was going to be called at _module_init_ time only, which
it wasn't.

So, can you see why I consider these changes (patches 1 and 2) as
merely functional to the removal of init() from pxa_camera?

Anyhow, if you don't like references to a future change without an
explanation I can arrange something in commit messages for the first
two patches :)

Regards,
   Antonio
  

Patch

diff --git a/arch/arm/mach-pxa/em-x270.c b/arch/arm/mach-pxa/em-x270.c
index aec7f42..f71f34c 100644
--- a/arch/arm/mach-pxa/em-x270.c
+++ b/arch/arm/mach-pxa/em-x270.c
@@ -967,7 +967,7 @@  static inline void em_x270_init_gpio_keys(void) {}
 #if defined(CONFIG_VIDEO_PXA27x) || defined(CONFIG_VIDEO_PXA27x_MODULE)
 static struct regulator *em_x270_camera_ldo;
 
-static int em_x270_sensor_init(struct device *dev)
+static int em_x270_sensor_init(void)
 {
 	int ret;
 
@@ -996,7 +996,6 @@  static int em_x270_sensor_init(struct device *dev)
 }
 
 struct pxacamera_platform_data em_x270_camera_platform_data = {
-	.init	= em_x270_sensor_init,
 	.flags  = PXA_CAMERA_MASTER | PXA_CAMERA_DATAWIDTH_8 |
 		PXA_CAMERA_PCLK_EN | PXA_CAMERA_MCLK_EN,
 	.mclk_10khz = 2600,
@@ -1049,8 +1048,10 @@  static struct platform_device em_x270_camera = {
 
 static void  __init em_x270_init_camera(void)
 {
-	pxa_set_camera_info(&em_x270_camera_platform_data);
-	platform_device_register(&em_x270_camera);
+	if (em_x270_sensor_init() == 0) {
+		pxa_set_camera_info(&em_x270_camera_platform_data);
+		platform_device_register(&em_x270_camera);
+	}
 }
 #else
 static inline void em_x270_init_camera(void) {}