From patchwork Thu Nov 12 14:41:06 2009 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Antonio Ospite X-Patchwork-Id: 2060 Return-path: Envelope-to: mchehab@infradead.org Delivery-date: Thu, 12 Nov 2009 14:41:31 +0000 Received: from bombadil.infradead.org [18.85.46.34] by pedra.chehab.org with IMAP (fetchmail-6.3.6) for (single-drop); Thu, 12 Nov 2009 12:42:12 -0200 (BRST) Received: from vger.kernel.org ([209.132.176.167]) by bombadil.infradead.org with esmtp (Exim 4.69 #1 (Red Hat Linux)) id 1N8ara-0005i7-Pk; Thu, 12 Nov 2009 14:41:31 +0000 Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752216AbZKLOlR (ORCPT + 1 other); Thu, 12 Nov 2009 09:41:17 -0500 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1752648AbZKLOlR (ORCPT ); Thu, 12 Nov 2009 09:41:17 -0500 Received: from smtp-out113.alice.it ([85.37.17.113]:3562 "EHLO smtp-out113.alice.it" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752199AbZKLOlP (ORCPT ); Thu, 12 Nov 2009 09:41:15 -0500 Received: from FBCMMO03.fbc.local ([192.168.68.197]) by smtp-out113.alice.it with Microsoft SMTPSVC(6.0.3790.3959); Thu, 12 Nov 2009 15:41:19 +0100 Received: from FBCMCL01B03.fbc.local ([192.168.69.84]) by FBCMMO03.fbc.local with Microsoft SMTPSVC(6.0.3790.3959); Thu, 12 Nov 2009 15:41:18 +0100 Received: from badebec ([82.61.46.40]) by FBCMCL01B03.fbc.local with Microsoft SMTPSVC(6.0.3790.3959); Thu, 12 Nov 2009 15:41:17 +0100 Date: Thu, 12 Nov 2009 15:41:06 +0100 From: Antonio Ospite To: Guennadi Liakhovetski Cc: linux-arm-kernel@lists.infradead.org, Eric Miao , openezx-devel@lists.openezx.org, Bart Visscher , Linux Media Mailing List Subject: Re: [PATCH 1/3 v3] Add camera support for A780 and A910 EZX phones Message-Id: <20091112154106.a00bbb95.ospite@studenti.unina.it> In-Reply-To: References: <1257937317-16655-1-git-send-email-ospite@studenti.unina.it> X-Mailer: Sylpheed 2.7.1 (GTK+ 2.18.3; i686-pc-linux-gnu) X-Face: z*RaLf`X<@C75u6Ig9}{oW$H; 1_\2t5)({*|jhM/Vb; ]yA5\I~93>J<_`<4)A{':UrE Mime-Version: 1.0 X-OriginalArrivalTime: 12 Nov 2009 14:41:18.0006 (UTC) FILETIME=[3185E560:01CA63A6] Sender: linux-media-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-media@vger.kernel.org On Wed, 11 Nov 2009 19:02:11 +0100 (CET) Guennadi Liakhovetski 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));