(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@xxxxxx> 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/
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
