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

On Wed, 4 Nov 2009 14:38:40 +0800
Eric Miao [off-list ref] wrote:

Hi Antonio,

Patch looks generally OK except for the MFP/GPIO usage, check my
comments below, thanks.

Ok, will resend ASAP. Some questions inlined below after your comments.

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)

If the use of parentheses here is recommended, should I send another
patch to add them to current defines for GPIOs in ezx.c, for style
consistency?

or (which tends to be more accurate but not necessary)

#define GPIO_nCAM_EN	mfp_to_gpio(MFP_PIN_GPIO50)

For me it is the same, just tell me if you really prefer this one.

quoted

+
+static int a780_pxacamera_power(struct device *dev, int on)
+{
+ ? ? ? gpio_set_value(MFP_PIN_GPIO50, on ? 0 : 1);

	gpio_set_value(GPIO_nCAM_EN, on ? 0 : 1);

quoted

+
+#if 0
+ ? ? ? /*
+ ? ? ? ?* This is reported to resolve the "vertical line in view finder"
+ ? ? ? ?* issue (LIBff11930), in the original source code released by
+ ? ? ? ?* Motorola, but we never experienced the problem, so we don't use
+ ? ? ? ?* this for now.
+ ? ? ? ?*
+ ? ? ? ?* AP Kernel camera driver: set TC_MM_EN to low when camera is running
+ ? ? ? ?* and TC_MM_EN to high when camera stops.
+ ? ? ? ?*
+ ? ? ? ?* BP Software: if TC_MM_EN is low, BP do not shut off 26M clock, but
+ ? ? ? ?* BP can sleep itself.
+ ? ? ? ?*/
+ ? ? ? gpio_set_value(MFP_PIN_GPIO99, on ? 0 : 1);
+#endif

This is a little bit confusing - can we remove this for this stage?

Ok, I am removing it for now. I might put this note in again in
future, hopefully with a better description.

[...]

Thanks,
   Antonio

-- 
Antonio Ospite
http://ao2.it

PGP public key ID: 0x4553B001

A: Because it messes up the order in which people normally read text.
   See http://en.wikipedia.org/wiki/Posting_style
Q: Why is top-posting such a bad thing?
A: Top-posting.
Q: What is the most annoying thing in e-mail?
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20091104/34863856/attachment-0001.sig>