On Wed, 11 Nov 2009 19:02:11 +0100 (CET)
Guennadi Liakhovetski <g.liakhovetski@xxxxxx> wrote:
> Hi Antonio
>
> Still one more nitpick:
>
Comments below.
> On Wed, 11 Nov 2009, Antonio Ospite wrote:
>
[...]
> >
> > +/* camera */
> > +static int a780_camera_init(void)
>
> This function returns an error but...
>
> > +{
> > + int err;
> > +
> > + /*
> > + * GPIO50_nCAM_EN is active low
> > + * GPIO19_GEN1_CAM_RST is active on rising edge
> > + */
> > + err = gpio_request(GPIO50_nCAM_EN, "nCAM_EN");
> > + if (err) {
> > + pr_err("%s: Failed to request nCAM_EN\n", __func__);
> > + goto fail;
> > + }
[...]
> > + return err;
> > +}
> > +
[...]
> > static void __init a780_init(void)
> > @@ -699,6 +782,9 @@ static void __init a780_init(void)
> >
> > pxa_set_keypad_info(&a780_keypad_platform_data);
> >
> > + a780_camera_init();
>
> ...it is not used. So, I would either make it void, or check the return
> code, and maybe even not register the camera since it's going to be
> useless anyway? Same for a910.
>
Actually I was keeping returning an error just in case there would be a
soc-camera .init() someday. But it's indeed a good idea to check the
return value once that we have it.
I am inlining here only the incremental change for a faster review,
I am going to send v4 of the patch separately for you ACK, hopefully :).
Note, now the return value of platform_device_register is ignored but
this is not grave, I guess.
Thanks for your time,
Antonio
diff --git a/arch/arm/mach-pxa/ezx.c b/arch/arm/mach-pxa/ezx.c
index 74423a6..1a73b7b 100644
--- a/arch/arm/mach-pxa/ezx.c
+++ b/arch/arm/mach-pxa/ezx.c
@@ -767,7 +767,6 @@ static struct platform_device a780_camera = {
static struct platform_device *a780_devices[] __initdata = {
&a780_gpio_keys,
- &a780_camera,
};
static void __init a780_init(void)
@@ -782,8 +781,10 @@ static void __init a780_init(void)
pxa_set_keypad_info(&a780_keypad_platform_data);
- a780_camera_init();
- pxa_set_camera_info(&a780_pxacamera_platform_data);
+ if (a780_camera_init() == 0) {
+ pxa_set_camera_info(&a780_pxacamera_platform_data);
+ platform_device_register(&a780_camera);
+ }
platform_add_devices(ARRAY_AND_SIZE(ezx_devices));
platform_add_devices(ARRAY_AND_SIZE(a780_devices));
@@ -1026,7 +1027,6 @@ static struct platform_device a910_camera = {
static struct platform_device *a910_devices[] __initdata = {
&a910_gpio_keys,
- &a910_camera,
};
static void __init a910_init(void)
@@ -1041,8 +1041,10 @@ static void __init a910_init(void)
pxa_set_keypad_info(&a910_keypad_platform_data);
- a910_camera_init();
- pxa_set_camera_info(&a910_pxacamera_platform_data);
+ if (a910_camera_init() == 0) {
+ pxa_set_camera_info(&a910_pxacamera_platform_data);
+ platform_device_register(&a910_camera);
+ }
platform_add_devices(ARRAY_AND_SIZE(ezx_devices));
platform_add_devices(ARRAY_AND_SIZE(a910_devices));
--
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:
pgpntvdHQf29U.pgp
Description: PGP signature
