Re: [PATCH v2] ADP1653 board code for Nokia RX-51

From: Pali RohÃr
Date: Fri Sep 06 2013 - 16:34:17 EST


On Wednesday 06 March 2013 21:12:06 Pali RohÃr wrote:
> On Sunday 17 February 2013 20:03:03 Aaro Koskinen wrote:
> > Hi,
> >
> > On Sun, Feb 17, 2013 at 04:16:49PM +0100, Pali RohÃr wrote:
> > > I'm sending ADP1653 flash torch board code for Nokia
> > > RX-51. Kernel driver ADP1653 is already in upstream
> > > kernel. Board code was extracted from this big camera
> > > meego patch:
> > >
> > > https://api.pub.meego.com/public/source/CE:Adaptation:N900
> > > /k
> > > ernel-adaptation-n900/linux-2.6-Camera-for-Meego-N900-Ada
> > > pta tion-kernel-2.6.37-patch.patch
> >
> > You need to sign-off the patch.
>
> Signed-off-by: Pali RohÃr <pali.rohar@xxxxxxxxx>
>
> > > --- /dev/null
> > > +++ b/arch/arm/mach-omap2/board-rx51-camera.c
> >
> > I'm not sure if adding a new file is sensible. There are
> > already 3 board files for RX-51, which I think is overkill.
>
> You can see that camera board code is big, so code was moved
> to separate file. Because not all camera drivers are
> upstreamed yet there is no camera support in RX-51 board
> code. Current peripheral file for RX-51 is big too and split
> camera code into separate file can be usefull...
>
> > > @@ -0,0 +1,177 @@
> > > +/*
> > > + * arch/arm/mach-omap2/board-rx51-camera.c
> > > + *
> > > + * Copyright (C) 2008 Nokia Corporation
> > > + *
> > > + * Contact: Sakari Ailus <sakari.ailus@xxxxxxxxx>
> > > + * Tuukka Toivonen <tuukka.o.toivonen@xxxxxxxxx>
> >
> > You should put these people to CC... Just to see if the
> > addresses are still valid (which I doubt).
>
> Ok, trying :-)
>
> > > +static int __init rx51_adp1653_init(void)
> > > +{
> > > + int err;
> > > +
> > > + err = gpio_request(ADP1653_GPIO_ENABLE, "adp1653
> > > enable"); + if (err) {
> > > + printk(KERN_ERR ADP1653_NAME
> > > + " Failed to request EN gpio\n");
> > > + err = -ENODEV;
> > > + goto err_omap_request_gpio;
> > > + }
> > > +
> > > + err = gpio_request(ADP1653_GPIO_INT, "adp1653
> > > interrupt"); + if (err) {
> > > + printk(KERN_ERR ADP1653_NAME " Failed to request IRQ
> > > gpio\n"); + err = -ENODEV;
> > > + goto err_omap_request_gpio_2;
> > > + }
> > > +
> > > + err = gpio_request(ADP1653_GPIO_STROBE, "adp1653
> > > strobe"); + if (err) {
> > > + printk(KERN_ERR ADP1653_NAME
> > > + " Failed to request STROBE gpio\n");
> > > + err = -ENODEV;
> > > + goto err_omap_request_gpio_3;
> > > + }
> > > +
> > > + gpio_direction_output(ADP1653_GPIO_ENABLE, 0);
> > > + gpio_direction_input(ADP1653_GPIO_INT);
> > > + gpio_direction_output(ADP1653_GPIO_STROBE, 0);
> >
> > gpio_request_array() should be used.
>
> Ok, fixing this.
>
> > > +void __init rx51_camera_init(void)
> > > +{
> > > + if (rx51_camera_hw_init()) {
> > > + printk(KERN_WARNING "%s: Unable to initialize
> > > camera\n", + __func__);
> > > + return;
> > > + }
> > > +
> > > + if (omap3_init_camera(&rx51_isp_platform_data) < 0)
> > > + printk(KERN_WARNING "%s: Unable to register camera
> > > platform " + "device\n", __func__);
> >
> > pr_warn() should be used.
> >
> > A.
>
> Fixed too.
>
> Here is new version v2 of this patch:
>
> diff --git a/arch/arm/mach-omap2/Kconfig
> b/arch/arm/mach-omap2/Kconfig index 41b581f..268fa57 100644
> --- a/arch/arm/mach-omap2/Kconfig
> +++ b/arch/arm/mach-omap2/Kconfig
> @@ -287,6 +287,7 @@ config MACH_NOKIA_RX51
> depends on ARCH_OMAP3
> default y
> select OMAP_PACKAGE_CBB
> + select VIDEO_ADP1653 if VIDEO_OMAP3 &&
> VIDEO_HELPER_CHIPS_AUTO
>
> config MACH_OMAP_ZOOM2
> bool "OMAP3 Zoom2 board"
> diff --git a/arch/arm/mach-omap2/Makefile
> b/arch/arm/mach-omap2/Makefile index 947cafe..f20f693 100644
> --- a/arch/arm/mach-omap2/Makefile
> +++ b/arch/arm/mach-omap2/Makefile
> @@ -236,6 +236,7 @@ obj-$(CONFIG_MACH_NOKIA_RM680) +=
> board-rm680.o sdram-nokia.o obj-$(CONFIG_MACH_NOKIA_RX51) +=
> board-rx51.o sdram-nokia.o obj-$(CONFIG_MACH_NOKIA_RX51)
+=
> board-rx51-peripherals.o obj-$(CONFIG_MACH_NOKIA_RX51) +=
> board-rx51-video.o +obj-$(CONFIG_MACH_NOKIA_RX51) +=
> board-rx51-camera.o obj-$(CONFIG_MACH_OMAP_ZOOM2) +=
> board-zoom.o board-zoom-peripherals.o
> obj-$(CONFIG_MACH_OMAP_ZOOM2) += board-zoom-display.o
> obj-$(CONFIG_MACH_OMAP_ZOOM2) += board-zoom-debugboard.o
> diff --git a/arch/arm/mach-omap2/board-rx51-camera.c
> b/arch/arm/mach-omap2/board-rx51-camera.c new file mode
> 100644
> index 0000000..80057ab
> --- /dev/null
> +++ b/arch/arm/mach-omap2/board-rx51-camera.c
> @@ -0,0 +1,152 @@
> +/*
> + * arch/arm/mach-omap2/board-rx51-camera.c
> + *
> + * Copyright (C) 2008 Nokia Corporation
> + *
> + * Contact: Sakari Ailus <sakari.ailus@xxxxxxxxx>
> + * Tuukka Toivonen <tuukka.o.toivonen@xxxxxxxxx>
> + *
> + * This program is free software; you can redistribute it
> and/or + * modify it under the terms of the GNU General
> Public License + * version 2 as published by the Free
> Software Foundation. + *
> + * This program is distributed in the hope that it will be
> useful, but + * WITHOUT ANY WARRANTY; without even the
> implied warranty of + * MERCHANTABILITY or FITNESS FOR A
> PARTICULAR PURPOSE. See the GNU + * General Public License
> for more details.
> + *
> + * You should have received a copy of the GNU General Public
> License + * along with this program; if not, write to the
> Free Software + * Foundation, Inc., 51 Franklin St, Fifth
> Floor, Boston, MA + * 02110-1301 USA
> + *
> + */
> +
> +#include <linux/i2c.h>
> +#include <linux/delay.h>
> +#include <linux/gpio.h>
> +
> +#include "../../../drivers/media/platform/omap3isp/isp.h"
> +
> +#include <media/adp1653.h>
> +
> +#include "devices.h"
> +
> +#define ADP1653_GPIO_ENABLE 88 /* Used for resetting ADP1653
> */ +#define ADP1653_GPIO_INT 167 /* Fault interrupt */
> +#define ADP1653_GPIO_STROBE 126 /* Pin used in cam_strobe
> mode -> + * control using ISP drivers */
> +
> +static struct gpio rx51_adp1653_gpios[] __initdata = {
> + { ADP1653_GPIO_ENABLE, GPIOF_OUT_INIT_LOW, "adp1653
enable"
> }, + { ADP1653_GPIO_INT, GPIOF_IN, "adp1653 interrupt" },
> + { ADP1653_GPIO_STROBE, GPIOF_OUT_INIT_LOW, "adp1653
strobe"
> }, +};
> +
> +static int __init rx51_adp1653_init(void)
> +{
> + int ret;
> +
> + ret = gpio_request_array(rx51_adp1653_gpios,
> + ARRAY_SIZE(rx51_adp1653_gpios));
> + if (ret < 0) {
> + pr_err(ADP1653_NAME ": Failed to request gpios\n");
> + return -ENODEV;
> + }
> +
> + return 0;
> +}
> +
> +static int __init rx51_camera_hw_init(void)
> +{
> + int rval;
> +
> + rval = rx51_adp1653_init();
> + if (rval)
> + return rval;
> +
> + return 0;
> +}
> +
> +/*
> + *
> + * ADP1653
> + *
> + */
> +
> +static int rx51_adp1653_power(struct v4l2_subdev *subdev, int
> on) +{
> + gpio_set_value(ADP1653_GPIO_ENABLE, on);
> + if (on) {
> + /* Some delay is apparently required. */
> + udelay(20);
> + }
> +
> + return 0;
> +}
> +
> +static struct adp1653_platform_data
> rx51_adp1653_platform_data = { + .power =
> rx51_adp1653_power,
> + /* Must be limited to 500 ms in RX-51 */
> + .max_flash_timeout = 500000, /* us */
> + /* Must be limited to 320 mA in RX-51 B3 and newer hardware
> */ + .max_flash_intensity =
> ADP1653_FLASH_INTENSITY_REG_TO_mA(19), + /* Must be limited
> to 50 mA in RX-51 */
> + .max_torch_intensity =
> ADP1653_FLASH_INTENSITY_REG_TO_mA(1),
> + .max_indicator_intensity =
> ADP1653_INDICATOR_INTENSITY_REG_TO_uA(
> + ADP1653_REG_OUT_SEL_ILED_MAX),
> +};
> +
> +/*
> + *
> + * Init it all
> + *
> + */
> +
> +#define ADP1653_I2C_BUS_NUM 2
> +
> +static struct i2c_board_info rx51_camera_i2c_devices[] = {
> + {
> + I2C_BOARD_INFO(ADP1653_NAME, ADP1653_I2C_ADDR),
> + .platform_data = &rx51_adp1653_platform_data,
> + },
> +};
> +
> +static struct isp_subdev_i2c_board_info
> rx51_camera_primary_subdevs[] = { + {
> + .board_info = &rx51_camera_i2c_devices[0],
> + .i2c_adapter_id = ADP1653_I2C_BUS_NUM,
> + },
> + { NULL, 0, },
> +};
> +
> +static struct isp_v4l2_subdevs_group rx51_camera_subdevs[] =
> { + {
> + .subdevs = rx51_camera_primary_subdevs,
> + .interface = ISP_INTERFACE_CCP2B_PHY1,
> + .bus = { .ccp2 = {
> + .strobe_clk_pol = 0,
> + .crc = 1,
> + .ccp2_mode = 1,
> + .phy_layer = 1,
> + .vpclk_div = 1,
> + } },
> + },
> + { NULL, 0, },
> +};
> +
> +static struct isp_platform_data rx51_isp_platform_data = {
> + .subdevs = rx51_camera_subdevs,
> +};
> +
> +void __init rx51_camera_init(void)
> +{
> + if (rx51_camera_hw_init()) {
> + pr_warn("%s: Unable to initialize camera\n", __func__);
> + return;
> + }
> +
> + if (omap3_init_camera(&rx51_isp_platform_data) < 0)
> + pr_warn("%s: Unable to register camera platform "
> + "device\n", __func__);
> +}
> diff --git a/arch/arm/mach-omap2/board-rx51.c
> b/arch/arm/mach-omap2/board-rx51.c index d0374ea..92117a13
> 100644
> --- a/arch/arm/mach-omap2/board-rx51.c
> +++ b/arch/arm/mach-omap2/board-rx51.c
> @@ -75,6 +75,7 @@ static struct platform_device leds_gpio = {
> */
>
> extern void __init rx51_peripherals_init(void);
> +extern void __init rx51_camera_init(void);
>
> #ifdef CONFIG_OMAP_MUX
> static struct omap_board_mux board_mux[] __initdata = {
> @@ -100,6 +101,7 @@ static void __init rx51_init(void)
>
> usb_musb_init(&musb_board_data);
> rx51_peripherals_init();
> + rx51_camera_init();
>
> /* Ensure SDRC pins are mux'd for self-refresh */
> omap_mux_init_signal("sdrc_cke0", OMAP_PIN_OUTPUT);

Ping, can you review this patch v2?

--
Pali RohÃr
pali.rohar@xxxxxxxxx

Attachment: signature.asc
Description: This is a digitally signed message part.