[PATCH 1/3] ezx: Add camera support for A780 and A910 EZX phones

[Guennadi Liakhovetski]

(added Robert to CC)

On Wed, 4 Nov 2009, Antonio Ospite wrote:

> On Wed, 4 Nov 2009 09:13:16 +0100 (CET)
> Guennadi Liakhovetski <g.liakhovetski at gmx.de> wrote:
> 
> > > > +/* camera */
> > > > +static int a780_pxacamera_init(struct device *dev)
> > > > +{
> > > > +       int err;
> > > > +
> > > > +       /*
> > > > +        * GPIO50_GPIO is CAM_EN: active low
> > > > +        * GPIO19_GPIO is CAM_RST: active high
> > > > +        */
> > > > +       err = gpio_request(MFP_PIN_GPIO50, "nCAM_EN");
> > > 
> > > Mmm... MFP != GPIO, so this probably should be written simply as:
> > > 
> > > #define GPIO_nCAM_EN	(50)
> > 
> > ...but without parenthesis, please:
> > 
> > #define GPIO_nCAM_EN	50
> > 
> > same everywhere below
> >
> 
> OK.
> 
> BTW, Guennadi, shouldn't the pxa_camera platform_data expose also an
> exit() method for symmetry with the init() one, where we can free the
> requested resources?

Good that you mentioned this. In fact, I think, that .init should go. So 
far it is used in pcm990-baseboard.c to initialise pins. You're doing 
essentially the same - requesting and configuring GPIOs. And it has been 
agreed, that there is so far no real case, where a static 
GPIO-configuration wouldn't work. So, I would suggest you remove .init, 
configure GPIOs statically. And then submit a patch to remove .init 
completely from struct pxacamera_platform_data. Robert, do you agree?

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