On Wed, 4 Nov 2009 09:13:16 +0100 (CET) Guennadi Liakhovetski <g.liakhovetski@xxxxxx> wrote: > On Wed, 4 Nov 2009, Eric Miao wrote: > > > Hi Antonio, > > > > Patch looks generally OK except for the MFP/GPIO usage, check my > > comments below, thanks. > > > > > +/* 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? If you want I think I can add it. [...] > > > + > > > +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); > > IMHO better yet > > gpio_set_value(GPIO_nCAM_EN, !on); > > Also throughout the patch below. > > I'm still to look at it miself and maybe provide a couple more comments, > if any. > Ok, 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?
Attachment:
pgpMrNQEJvbxt.pgp
Description: PGP signature