[3/3] pxa_camera: remove init() callback

Message ID 1258495463-26029-4-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() callback is sometimes abused to setup MFP for PXA CIF, or
even to request GPIOs to be used by the camera *sensor*. These initializations
can be performed statically in machine init functions.

The current semantics for this init() callback is ambiguous anyways, it is
invoked in pxa_camera_activate(), hence at device node open, but its users use
it like a generic initialization to be done at module init time (configure
MFP, request GPIOs for *sensor* control).

Signed-off-by: Antonio Ospite <ospite@studenti.unina.it>
---
 arch/arm/mach-pxa/include/mach/camera.h |    2 --
 drivers/media/video/pxa_camera.c        |   10 ----------
 2 files changed, 0 insertions(+), 12 deletions(-)
  

Comments

Guennadi Liakhovetski Nov. 27, 2009, 2:06 p.m. UTC | #1
On Tue, 17 Nov 2009, Antonio Ospite wrote:

> pxa_camera init() callback is sometimes abused to setup MFP for PXA CIF, or
> even to request GPIOs to be used by the camera *sensor*. These initializations
> can be performed statically in machine init functions.
> 
> The current semantics for this init() callback is ambiguous anyways, it is
> invoked in pxa_camera_activate(), hence at device node open, but its users use
> it like a generic initialization to be done at module init time (configure
> MFP, request GPIOs for *sensor* control).
> 
> Signed-off-by: Antonio Ospite <ospite@studenti.unina.it>

Antonio, to make the merging easier and avoid imposing extra dependencies, 
I would postpone this to 2.6.34, and just remove uses of .init() by 
pxa-camera users as per your other two patches. Would this be ok with you?

Thanks
Guennadi

> ---
>  arch/arm/mach-pxa/include/mach/camera.h |    2 --
>  drivers/media/video/pxa_camera.c        |   10 ----------
>  2 files changed, 0 insertions(+), 12 deletions(-)
> 
> diff --git a/arch/arm/mach-pxa/include/mach/camera.h b/arch/arm/mach-pxa/include/mach/camera.h
> index 31abe6d..6709b1c 100644
> --- a/arch/arm/mach-pxa/include/mach/camera.h
> +++ b/arch/arm/mach-pxa/include/mach/camera.h
> @@ -35,8 +35,6 @@
>  #define PXA_CAMERA_VSP		0x400
>  
>  struct pxacamera_platform_data {
> -	int (*init)(struct device *);
> -
>  	unsigned long flags;
>  	unsigned long mclk_10khz;
>  };
> diff --git a/drivers/media/video/pxa_camera.c b/drivers/media/video/pxa_camera.c
> index 51b683c..49f2bf9 100644
> --- a/drivers/media/video/pxa_camera.c
> +++ b/drivers/media/video/pxa_camera.c
> @@ -882,18 +882,8 @@ static void recalculate_fifo_timeout(struct pxa_camera_dev *pcdev,
>  
>  static void pxa_camera_activate(struct pxa_camera_dev *pcdev)
>  {
> -	struct pxacamera_platform_data *pdata = pcdev->pdata;
> -	struct device *dev = pcdev->soc_host.v4l2_dev.dev;
>  	u32 cicr4 = 0;
>  
> -	dev_dbg(dev, "Registered platform device at %p data %p\n",
> -		pcdev, pdata);
> -
> -	if (pdata && pdata->init) {
> -		dev_dbg(dev, "%s: Init gpios\n", __func__);
> -		pdata->init(dev);
> -	}
> -
>  	/* disable all interrupts */
>  	__raw_writel(0x3ff, pcdev->base + CICR0);
>  
> -- 
> 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. 27, 2009, 2:32 p.m. UTC | #2
On Fri, 27 Nov 2009 15:06:53 +0100 (CET)
Guennadi Liakhovetski <g.liakhovetski@gmx.de> wrote:

> On Tue, 17 Nov 2009, Antonio Ospite wrote:
> 
> > pxa_camera init() callback is sometimes abused to setup MFP for PXA CIF, or
> > even to request GPIOs to be used by the camera *sensor*. These initializations
> > can be performed statically in machine init functions.
> > 
> > The current semantics for this init() callback is ambiguous anyways, it is
> > invoked in pxa_camera_activate(), hence at device node open, but its users use
> > it like a generic initialization to be done at module init time (configure
> > MFP, request GPIOs for *sensor* control).
> > 
> > Signed-off-by: Antonio Ospite <ospite@studenti.unina.it>
> 
> Antonio, to make the merging easier and avoid imposing extra dependencies, 
> I would postpone this to 2.6.34, and just remove uses of .init() by 
> pxa-camera users as per your other two patches. Would this be ok with you?
> 
> Thanks
> Guennadi
>

Perfectly fine with me.

Feel also free to anticipate me and edit the commit messages to
whatever you want in the first two patches. Now that we aren't removing
init() immediately after these it makes even more sense to change the
phrasing from a future referencing
	"init() is going to be removed"
to a more present focused
	"better not to use init() at all"
form.

Thanks,
   Antonio
  
Guennadi Liakhovetski Nov. 27, 2009, 2:37 p.m. UTC | #3
On Fri, 27 Nov 2009, Antonio Ospite wrote:

> On Fri, 27 Nov 2009 15:06:53 +0100 (CET)
> Guennadi Liakhovetski <g.liakhovetski@gmx.de> wrote:
> 
> > On Tue, 17 Nov 2009, Antonio Ospite wrote:
> > 
> > > pxa_camera init() callback is sometimes abused to setup MFP for PXA CIF, or
> > > even to request GPIOs to be used by the camera *sensor*. These initializations
> > > can be performed statically in machine init functions.
> > > 
> > > The current semantics for this init() callback is ambiguous anyways, it is
> > > invoked in pxa_camera_activate(), hence at device node open, but its users use
> > > it like a generic initialization to be done at module init time (configure
> > > MFP, request GPIOs for *sensor* control).
> > > 
> > > Signed-off-by: Antonio Ospite <ospite@studenti.unina.it>
> > 
> > Antonio, to make the merging easier and avoid imposing extra dependencies, 
> > I would postpone this to 2.6.34, and just remove uses of .init() by 
> > pxa-camera users as per your other two patches. Would this be ok with you?
> > 
> > Thanks
> > Guennadi
> >
> 
> Perfectly fine with me.
> 
> Feel also free to anticipate me and edit the commit messages to
> whatever you want in the first two patches. Now that we aren't removing
> init() immediately after these it makes even more sense to change the
> phrasing from a future referencing
> 	"init() is going to be removed"
> to a more present focused
> 	"better not to use init() at all"
> form.

I cannot edit those subject lines, because I will not be handling those 
patches, they will go via the PXA tree, that's why it is easier to wait 
with the pxa patch.

Thanks
Guennadi
---
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. 27, 2009, 2:47 p.m. UTC | #4
On Fri, 27 Nov 2009 15:37:19 +0100 (CET)
Guennadi Liakhovetski <g.liakhovetski@gmx.de> wrote:

> On Fri, 27 Nov 2009, Antonio Ospite wrote:
> 
> > On Fri, 27 Nov 2009 15:06:53 +0100 (CET)
> > Guennadi Liakhovetski <g.liakhovetski@gmx.de> wrote:
> > 
> > > On Tue, 17 Nov 2009, Antonio Ospite wrote:
> > > 
> > > > pxa_camera init() callback is sometimes abused to setup MFP for PXA CIF, or
> > > > even to request GPIOs to be used by the camera *sensor*. These initializations
> > > > can be performed statically in machine init functions.
> > > > 
> > > > The current semantics for this init() callback is ambiguous anyways, it is
> > > > invoked in pxa_camera_activate(), hence at device node open, but its users use
> > > > it like a generic initialization to be done at module init time (configure
> > > > MFP, request GPIOs for *sensor* control).
> > > > 
> > > > Signed-off-by: Antonio Ospite <ospite@studenti.unina.it>
> > > 
> > > Antonio, to make the merging easier and avoid imposing extra dependencies, 
> > > I would postpone this to 2.6.34, and just remove uses of .init() by 
> > > pxa-camera users as per your other two patches. Would this be ok with you?
> > > 
> > > Thanks
> > > Guennadi
> > >
> > 
> > Perfectly fine with me.
> > 
> > Feel also free to anticipate me and edit the commit messages to
> > whatever you want in the first two patches. Now that we aren't removing
> > init() immediately after these it makes even more sense to change the
> > phrasing from a future referencing
> > 	"init() is going to be removed"
> > to a more present focused
> > 	"better not to use init() at all"
> > form.
> 
> I cannot edit those subject lines, because I will not be handling those 
> patches, they will go via the PXA tree, that's why it is easier to wait 
> with the pxa patch.
>

I see, I am sending a v2 for the first two patches with changed commit
messages in some hours then. Sorry for the delay.

> Thanks
> Guennadi
> ---
> Guennadi Liakhovetski, Ph.D.
> Freelance Open-Source Software Developer
> http://www.open-technology.de/

Regards,
   Antonio
  

Patch

diff --git a/arch/arm/mach-pxa/include/mach/camera.h b/arch/arm/mach-pxa/include/mach/camera.h
index 31abe6d..6709b1c 100644
--- a/arch/arm/mach-pxa/include/mach/camera.h
+++ b/arch/arm/mach-pxa/include/mach/camera.h
@@ -35,8 +35,6 @@ 
 #define PXA_CAMERA_VSP		0x400
 
 struct pxacamera_platform_data {
-	int (*init)(struct device *);
-
 	unsigned long flags;
 	unsigned long mclk_10khz;
 };
diff --git a/drivers/media/video/pxa_camera.c b/drivers/media/video/pxa_camera.c
index 51b683c..49f2bf9 100644
--- a/drivers/media/video/pxa_camera.c
+++ b/drivers/media/video/pxa_camera.c
@@ -882,18 +882,8 @@  static void recalculate_fifo_timeout(struct pxa_camera_dev *pcdev,
 
 static void pxa_camera_activate(struct pxa_camera_dev *pcdev)
 {
-	struct pxacamera_platform_data *pdata = pcdev->pdata;
-	struct device *dev = pcdev->soc_host.v4l2_dev.dev;
 	u32 cicr4 = 0;
 
-	dev_dbg(dev, "Registered platform device at %p data %p\n",
-		pcdev, pdata);
-
-	if (pdata && pdata->init) {
-		dev_dbg(dev, "%s: Init gpios\n", __func__);
-		pdata->init(dev);
-	}
-
 	/* disable all interrupts */
 	__raw_writel(0x3ff, pcdev->base + CICR0);