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

(added Robert to CC)

On Wed, 4 Nov 2009, Antonio Ospite wrote:

On Wed, 4 Nov 2009 09:13:16 +0100 (CET)
Guennadi Liakhovetski [off-list ref] wrote:

quoted

quoted

quoted

+/* 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/