Re: [PATCH 3/3] V4L2 Driver for OMAP3/3 DSS.
Hello again,
my previous message due to bad connection was send with no end.
Sorry about that.
> + vout->vid = k;
> + vid_dev->vouts[k] = vout;
> + vout->vid_info.vid_dev = vid_dev;
> + vout->vid_info.overlays[0] = vid_dev->overlays[k + 1];
> + vout->vid_info.num_overlays = 1;
> + vout->vid_info.id = k + 1;
> + vid_dev->num_videos++;
> +
> + /* Setup the default configuration for the video devices
> + */
> + if (omap_vout_setup_video_data(vout) != 0) {
> + r = -ENOMEM;
> + goto error;
> + }
> +
> + /* Allocate default number of buffers for the video streaming
> + * and reserve the VRFB space for rotation
> + */
> + if (omap_vout_setup_video_bufs(pdev, k) != 0) {
> + r = -ENOMEM;
> + goto error1;
> + }
> +
> + /* Register the Video device with V4L2
> + */
> + vfd = vout->vfd;
> + if (video_register_device(vfd, VFL_TYPE_GRABBER, k + 1) < 0) {
> + printk(KERN_ERR VOUT_NAME ": could not register \
> + Video for Linux device\n");
> + vfd->minor = -1;
> + r = -ENODEV;
> + goto error2;
> + }
> +
> + if (k == 0)
> + saved_v1out = vout;
> + else
> + saved_v2out = vout;
> +
> + r = omapvid_apply_changes(vid_dev->vouts[k], 0, 1);
> +
> + if (r)
> + goto error2;
> + else
> + goto success;
> +error2:
> + omap_vout_release_vrfb(vout);
> + omap_vout_free_buffers(vout);
> +error1:
> + video_device_release(vfd);
> +error:
> + kfree(vout);
> + return r;
> +
> +success:
> + printk(KERN_INFO VOUT_NAME ": registered and initialized\
> + video device %d [v4l2]\n", vfd->minor);
> + if (k == (pdev->num_resources - 1))
> + return 0;
> + }
> + return -ENODEV;
> +
> +}
> +
> +int omapvid_apply_changes(struct omap_vout_device *vout, u32 addr, int init)
> +{
> + int r = 0;
> + struct omapvideo_info *ovid = &(vout->vid_info);
> + struct omap_overlay *ovl;
> + int posx, posy;
> + int outw, outh, temp, rotation;
> + int i;
> + struct v4l2_window *win;
> + struct omap_video_timings *timing;
> + struct omap_display *cur_display;
> +
> + win = &vout->win;
> + rotation = vout->rotation;
> + for (i = 0; i < ovid->num_overlays; i++) {
> + ovl = ovid->overlays[i];
> + if (!ovl->manager || !ovl->manager->display)
> + return -EINVAL;
> +
> + timing = &ovl->manager->display->panel->timings;
> + cur_display = ovl->manager->display;
> +
> + if (init || (ovl->caps & OMAP_DSS_OVL_CAP_SCALE) == 0) {
> + outw = win->w.width;
> + outh = win->w.height;
> +
> + } else {
> + outw = win->w.width;
> + outh = win->w.height;
> + }
> + if (init) {
> + posx = 0;
> + posy = 0;
> + } else {
> + switch (rotation) {
> +
> + case 1:
> + /* Invert the height and widht for 90
> + * and 270 degree rotation
> + */
> + temp = outw;
> + outw = outh;
> + outh = temp;
> + posy = (timing->y_res - win->w.width)-
> + win->w.left;
> + posx = win->w.top;
> + break;
> +
> + case 2:
> + posx = (timing->x_res - win->w.width) -
> + win->w.left;
> + posy = (timing->y_res - win->w.height) -
> + win->w.top;
> + break;
> +
> + case 3:
> + temp = outw;
> + outw = outh;
> + outh = temp;
> + posy = win->w.left;
> + posx = (timing->x_res - win->w.height)
> + - win->w.top;
> + break;
> +
> + default:
> +
Re: [PATCH 4/5] OMAP3430SDP: Add support for Camera Kit v3
Hello, all On Fri, 2009-03-06 at 10:54 +0900, DongSoo(Nathaniel) Kim wrote: > Hi Alexey, > > On Fri, Mar 6, 2009 at 7:05 AM, Alexey Klimov wrote: > > Hello, all > > > > On Thu, Mar 5, 2009 at 7:42 PM, Curran, Dominic wrote: > >> > >> Hi Kim > >> > >>> -Original Message- > >>> From: linux-omap-ow...@vger.kernel.org [mailto:linux-omap- > >>> ow...@vger.kernel.org] On Behalf Of DongSoo(Nathaniel) Kim > >>> Sent: Wednesday, March 04, 2009 8:58 PM > >>> To: Aguirre Rodriguez, Sergio Alberto > >>> Cc: linux-me...@vger.kernel.org; linux-omap@vger.kernel.org; Sakari Ailus; > >>> Tuukka.O Toivonen; Hiroshi DOYU; MiaoStanley; Nagalla, Hari; Hiremath, > >>> Vaibhav; Lakhani, Amish; Menon, Nishanth > >>> Subject: Re: [PATCH 4/5] OMAP3430SDP: Add support for Camera Kit v3 > >>> > >>> Hi Sergio, > >>> > >>> > >>> > >>> On Wed, Mar 4, 2009 at 5:44 AM, Aguirre Rodriguez, Sergio Alberto > >>> wrote: > >>> > + /* turn on analog power */ > >>> > + twl4030_i2c_write_u8(TWL4030_MODULE_PM_RECEIVER, > >>> > + VAUX_2_8_V, TWL4030_VAUX2_DEDICATED); > >>> > + twl4030_i2c_write_u8(TWL4030_MODULE_PM_RECEIVER, > >>> > + VAUX_DEV_GRP_P1, TWL4030_VAUX2_DEV_GRP); > >>> > + > >>> > + /* out of standby */ > >>> > + gpio_set_value(MT9P012_STANDBY_GPIO, 0); > >>> > + udelay(1000); > >>> > >>> It seems better using msleep rather than udelay for 1000us much. Just > >>> to be safe :) > >>> How about you? > >>> > >> > >> Why is msleep safer than udelay ? > > > > I have small guess that he is wondering why you are using big delays > > with help of udelay(). (It's may be obvious but as we know udelay uses > > cpu loops to make delay and msleep calls to scheduler) So, msleep is > > more flexible and "softer" but if you need precise time or you can't > > sleep in code you need udelay. Sometimes using udelay is reasonably > > required. > > I totally agree with you. > But besides the "udelay and mdelay accuracy" issue, Sergio's power up > timing for MT9P012 seems to delay too much. (not for lens > controller.) > I also have experience using MT9P012 sensor with other ISP, but in > case of mine it took 600 to 800 ms for whole power up sequence. > But if that delay depends on SDP board and Sergio had no options > without making delay for that much, then it explains everything. > So I'm saying if there was no other option than making long delay to > bring up MT9P012 sensor properly, if I were Sergio I should rather use > mdelay than udelay. I agree with you. mdelay is really safer that udelay. >From file include/linux/delay.h: * Using udelay() for intervals greater than a few milliseconds can * risk overflow for high loops_per_jiffy (high bogomips) machines. The * mdelay() provides a wrapper to prevent this. For delays greater * than MAX_UDELAY_MS milliseconds, the wrapper is used. Architecture * specific values can be defined in asm-???/delay.h as an override. So, let's Sergio check and decide what he needed! :) > Cheers, > > Nate -- Best regards, Klimov Alexey -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/9] omap3isp: Add ISP main driver and register definitions
On Thu, 2009-03-05 at 13:34 +0200, Sakari Ailus wrote:
> Thanks for the comments, Alexey!
>
> Alexey Klimov wrote:
> >> +static int isp_probe(struct platform_device *pdev)
> >> +{
> >> + struct isp_device *isp;
> >> + int ret_err = 0;
> >> + int i;
> >> +
> >> + isp = kzalloc(sizeof(*isp), GFP_KERNEL);
> >> + if (!isp) {
> >> + dev_err(&pdev->dev, "could not allocate memory\n");
> >> + return -ENODEV;
> >
> > return -ENOMEM; ?
>
> Done.
>
> >> + }
> >> +
> >> + platform_set_drvdata(pdev, isp);
> >> +
> >> + isp->dev = &pdev->dev;
> >> +
> >> + for (i = 0; i <= OMAP3_ISP_IOMEM_CSI2PHY; i++) {
> >> + struct resource *mem;
> >> + /* request the mem region for the camera registers */
> >> + mem = platform_get_resource(pdev, IORESOURCE_MEM, i);
> >> + if (!mem) {
> >> + dev_err(isp->dev, "no mem resource?\n");
> >> + return -ENODEV;
> >
> > Maybe ENODEV is not apropriate here too..
>
> What else it could be? ENOENT comes to mind but I'm not sure it's
> significantly better.
ENODEV is okay, sorry.
--
Best regards, Klimov Alexey
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 9/9] omap34xxcam: Add camera driver
Hello, Sakari Ailus
On Thu, 2009-03-05 at 16:09 +0200, Sakari Ailus wrote:
> Alexey Klimov wrote:
> >> +static int vidioc_g_fmt_vid_cap(struct file *file, void *fh,
> >> + struct v4l2_format *f)
> >> +{
> >> + struct omap34xxcam_fh *ofh = fh;
> >> + struct omap34xxcam_videodev *vdev = ofh->vdev;
> >> +
> >> + if (vdev->vdev_sensor == v4l2_int_device_dummy())
> >> + return -EINVAL;
> >> +
> >> + mutex_lock(&vdev->mutex);
> >> + f->fmt.pix = vdev->pix;
> >> + mutex_unlock(&vdev->mutex);
> >
> > H, you are using mutex_lock to lock reading from vdev structure..
> > Well, i don't if this is right approach. I am used to that mutex_lock is
> > used to prevent _changing_ of members in structure..
>
> The vdev->mutex is acquired since we want to prevent concurrent access
> to vdev->pix. Otherwise it might change while we are reading it, right?
I thought more about this and looks like that i was wrong. You are
right. You are reading structure, and i wasn't able to notice that first
time. Sorry for bothering about this.
> >> +static int omap34xxcam_device_register(struct v4l2_int_device *s)
> >> +{
> >> + struct omap34xxcam_videodev *vdev = s->u.slave->master->priv;
> >> + struct omap34xxcam_hw_config hwc;
> >> + int rval;
> >> +
> >> + /* We need to check rval just once. The place is here. */
> >
> > I didn't understand this comment. You doing nothin in next few lines
> > with int variable rval(which introduced in this function). Is comment
> > talking about struct v4l2_int_device *s ?
>
> Yes. If the g_priv() succeeds now it will succeed in future, too. This
> comes from the platform data through the slave device.
Well, okay. I mean that for me this comment looks ambiguous. Please, if
you don't mind it's better not to use word "rval" because it creates
confusion with int rval;.
--
Best regards, Klimov Alexey
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/5] OMAP3430SDP: Add support for Camera Kit v3
Hello, all On Thu, Mar 5, 2009 at 7:42 PM, Curran, Dominic wrote: > > Hi Kim > >> -Original Message- >> From: linux-omap-ow...@vger.kernel.org [mailto:linux-omap- >> ow...@vger.kernel.org] On Behalf Of DongSoo(Nathaniel) Kim >> Sent: Wednesday, March 04, 2009 8:58 PM >> To: Aguirre Rodriguez, Sergio Alberto >> Cc: linux-me...@vger.kernel.org; linux-omap@vger.kernel.org; Sakari Ailus; >> Tuukka.O Toivonen; Hiroshi DOYU; MiaoStanley; Nagalla, Hari; Hiremath, >> Vaibhav; Lakhani, Amish; Menon, Nishanth >> Subject: Re: [PATCH 4/5] OMAP3430SDP: Add support for Camera Kit v3 >> >> Hi Sergio, >> >> >> >> On Wed, Mar 4, 2009 at 5:44 AM, Aguirre Rodriguez, Sergio Alberto >> wrote: >> > + /* turn on analog power */ >> > + twl4030_i2c_write_u8(TWL4030_MODULE_PM_RECEIVER, >> > + VAUX_2_8_V, TWL4030_VAUX2_DEDICATED); >> > + twl4030_i2c_write_u8(TWL4030_MODULE_PM_RECEIVER, >> > + VAUX_DEV_GRP_P1, TWL4030_VAUX2_DEV_GRP); >> > + >> > + /* out of standby */ >> > + gpio_set_value(MT9P012_STANDBY_GPIO, 0); >> > + udelay(1000); >> >> It seems better using msleep rather than udelay for 1000us much. Just >> to be safe :) >> How about you? >> > > Why is msleep safer than udelay ? I have small guess that he is wondering why you are using big delays with help of udelay(). (It's may be obvious but as we know udelay uses cpu loops to make delay and msleep calls to scheduler) So, msleep is more flexible and "softer" but if you need precise time or you can't sleep in code you need udelay. Sometimes using udelay is reasonably required. -- Best regards, Klimov Alexey -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 5/9] omap3isp: Add ISP backend (PRV and RSZ)
On Tue, 2009-03-03 at 12:06 +0200, Sakari Ailus wrote:
> Signed-off-by: Sakari Ailus
> ---
> drivers/media/video/isp/isppreview.c | 1896
> ++
> drivers/media/video/isp/isppreview.h | 350 +++
> drivers/media/video/isp/ispresizer.c | 897
> drivers/media/video/isp/ispresizer.h | 154 +++
> 4 files changed, 3297 insertions(+), 0 deletions(-)
> create mode 100644 drivers/media/video/isp/isppreview.c
> create mode 100644 drivers/media/video/isp/isppreview.h
> create mode 100644 drivers/media/video/isp/ispresizer.c
> create mode 100644 drivers/media/video/isp/ispresizer.h
>
> diff --git a/drivers/media/video/isp/isppreview.c
> b/drivers/media/video/isp/isppreview.c
> new file mode 100644
> index 000..242b578
> --- /dev/null
> +++ b/drivers/media/video/isp/isppreview.c
> @@ -0,0 +1,1896 @@
> +/*
> + * isppreview.c
> + *
> + * Driver Library for Preview module in TI's OMAP3 Camera ISP
> + *
> + * Copyright (C) 2009 Texas Instruments, Inc.
> + *
> + * Contributors:
> + * Senthilvadivu Guruswamy
> + * Pallavi Kulkarni
> + * Sergio Aguirre
> + *
> + * This package 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 PACKAGE IS PROVIDED ``AS IS'' AND WITHOUT ANY EXPRESS OR
> + * IMPLIED WARRANTIES, INCLUDING, WITHOUT LIMITATION, THE IMPLIED
> + * WARRANTIES OF MERCHANTIBILITY AND FITNESS FOR A PARTICULAR PURPOSE.
> + */
> +
> +#include
> +#include
> +#include
> +
> +#include "isp.h"
> +#include "ispreg.h"
> +#include "isppreview.h"
> +
> +static struct ispprev_nf prev_nf_t;
> +static struct prev_params *params;
> +static int rg_update, gg_update, bg_update, nf_enable, nf_update;
> +
> +/* Structure for saving/restoring preview module registers */
> +static struct isp_reg ispprev_reg_list[] = {
> + {OMAP3_ISP_IOMEM_PREV, ISPPRV_HORZ_INFO, 0x},
> + {OMAP3_ISP_IOMEM_PREV, ISPPRV_VERT_INFO, 0x},
> + {OMAP3_ISP_IOMEM_PREV, ISPPRV_RSDR_ADDR, 0x},
> + {OMAP3_ISP_IOMEM_PREV, ISPPRV_RADR_OFFSET, 0x},
> + {OMAP3_ISP_IOMEM_PREV, ISPPRV_DSDR_ADDR, 0x},
> + {OMAP3_ISP_IOMEM_PREV, ISPPRV_DRKF_OFFSET, 0x},
> + {OMAP3_ISP_IOMEM_PREV, ISPPRV_WSDR_ADDR, 0x},
> + {OMAP3_ISP_IOMEM_PREV, ISPPRV_WADD_OFFSET, 0x},
> + {OMAP3_ISP_IOMEM_PREV, ISPPRV_AVE, 0x},
> + {OMAP3_ISP_IOMEM_PREV, ISPPRV_HMED, 0x},
> + {OMAP3_ISP_IOMEM_PREV, ISPPRV_NF, 0x},
> + {OMAP3_ISP_IOMEM_PREV, ISPPRV_WB_DGAIN, 0x},
> + {OMAP3_ISP_IOMEM_PREV, ISPPRV_WBGAIN, 0x},
> + {OMAP3_ISP_IOMEM_PREV, ISPPRV_WBSEL, 0x},
> + {OMAP3_ISP_IOMEM_PREV, ISPPRV_CFA, 0x},
> + {OMAP3_ISP_IOMEM_PREV, ISPPRV_BLKADJOFF, 0x},
> + {OMAP3_ISP_IOMEM_PREV, ISPPRV_RGB_MAT1, 0x},
> + {OMAP3_ISP_IOMEM_PREV, ISPPRV_RGB_MAT2, 0x},
> + {OMAP3_ISP_IOMEM_PREV, ISPPRV_RGB_MAT3, 0x},
> + {OMAP3_ISP_IOMEM_PREV, ISPPRV_RGB_MAT4, 0x},
> + {OMAP3_ISP_IOMEM_PREV, ISPPRV_RGB_MAT5, 0x},
> + {OMAP3_ISP_IOMEM_PREV, ISPPRV_RGB_OFF1, 0x},
> + {OMAP3_ISP_IOMEM_PREV, ISPPRV_RGB_OFF2, 0x},
> + {OMAP3_ISP_IOMEM_PREV, ISPPRV_CSC0, 0x},
> + {OMAP3_ISP_IOMEM_PREV, ISPPRV_CSC1, 0x},
> + {OMAP3_ISP_IOMEM_PREV, ISPPRV_CSC2, 0x},
> + {OMAP3_ISP_IOMEM_PREV, ISPPRV_CSC_OFFSET, 0x},
> + {OMAP3_ISP_IOMEM_PREV, ISPPRV_CNT_BRT, 0x},
> + {OMAP3_ISP_IOMEM_PREV, ISPPRV_CSUP, 0x},
> + {OMAP3_ISP_IOMEM_PREV, ISPPRV_SETUP_YC, 0x},
> + {OMAP3_ISP_IOMEM_PREV, ISPPRV_CDC_THR0, 0x},
> + {OMAP3_ISP_IOMEM_PREV, ISPPRV_CDC_THR1, 0x},
> + {OMAP3_ISP_IOMEM_PREV, ISPPRV_CDC_THR2, 0x},
> + {OMAP3_ISP_IOMEM_PREV, ISPPRV_CDC_THR3, 0x},
> + {OMAP3_ISP_IOMEM_PREV, ISPPRV_PCR, 0x},
> + {0, ISP_TOK_TERM, 0x}
> +};
> +
> +
> +/* Default values in Office Flourescent Light for RGBtoRGB Blending */
> +static struct ispprev_rgbtorgb flr_rgb2rgb = {
> + { /* RGB-RGB Matrix */
> + {0x01E2, 0x0F30, 0x0FEE},
> + {0x0F9B, 0x01AC, 0x0FB9},
> + {0x0FE0, 0x0EC0, 0x0260}
> + }, /* RGB Offset */
> + {0x, 0x, 0x}
> +};
> +
> +/* Default values in Office Flourescent Light for RGB to YUV Conversion*/
> +static struct ispprev_csc flr_prev_csc[] = {
> + {
> + { /* CSC Coef Matrix */
> + {66, 129, 25},
> + {-38, -75, 112},
> + {112, -94 , -18}
> + }, /* CSC Offset */
> + {0x0, 0x0, 0x0}
> + },
> + {
> + { /* CSC Coef Matrix BW */
> + {66, 129, 25},
> + {0, 0, 0},
> + {0, 0, 0}
> + }, /* CSC Offset */
> + {0x0, 0x0, 0x0}
> + },
> + {
> + { /* CSC Coef Matrix
Re: [PATCH 4/9] omap3isp: Add ISP frontend (CCDC)
On Tue, 2009-03-03 at 12:06 +0200, Sakari Ailus wrote:
> Signed-off-by: Sakari Ailus
> ---
> drivers/media/video/isp/ispccdc.c | 1568
> +
> drivers/media/video/isp/ispccdc.h | 203 +
> 2 files changed, 1771 insertions(+), 0 deletions(-)
> create mode 100644 drivers/media/video/isp/ispccdc.c
> create mode 100644 drivers/media/video/isp/ispccdc.h
>
> diff --git a/drivers/media/video/isp/ispccdc.c
> b/drivers/media/video/isp/ispccdc.c
> new file mode 100644
> index 000..80ab762
> --- /dev/null
> +++ b/drivers/media/video/isp/ispccdc.c
> @@ -0,0 +1,1568 @@
> +/*
> + * ispccdc.c
> + *
> + * Driver Library for CCDC module in TI's OMAP3 Camera ISP
> + *
> + * Copyright (C) 2009 Texas Instruments, Inc.
> + *
> + * Contributors:
> + * Senthilvadivu Guruswamy
> + * Pallavi Kulkarni
> + * Sergio Aguirre
> + *
> + * This package 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 PACKAGE IS PROVIDED ``AS IS'' AND WITHOUT ANY EXPRESS OR
> + * IMPLIED WARRANTIES, INCLUDING, WITHOUT LIMITATION, THE IMPLIED
> + * WARRANTIES OF MERCHANTIBILITY AND FITNESS FOR A PARTICULAR PURPOSE.
> + */
> +
> +#include
> +#include
> +#include
> +
> +#include "isp.h"
> +#include "ispreg.h"
> +#include "ispccdc.h"
> +#include "ispmmu.h"
> +
> +#define LSC_TABLE_INIT_SIZE 50052
> +
> +static u32 *fpc_table_add;
> +static unsigned long fpc_table_add_m;
> +
> +/**
> + * struct isp_ccdc - Structure for the CCDC module to store its own
> information
> + * @ccdc_inuse: Flag to determine if CCDC has been reserved or not (0 or 1).
> + * @ccdcout_w: CCDC output width.
> + * @ccdcout_h: CCDC output height.
> + * @ccdcin_w: CCDC input width.
> + * @ccdcin_h: CCDC input height.
> + * @ccdcin_woffset: CCDC input horizontal offset.
> + * @ccdcin_hoffset: CCDC input vertical offset.
> + * @crop_w: Crop width.
> + * @crop_h: Crop weight.
> + * @ccdc_inpfmt: CCDC input format.
> + * @ccdc_outfmt: CCDC output format.
> + * @vpout_en: Video port output enable.
> + * @wen: Data write enable.
> + * @exwen: External data write enable.
> + * @refmt_en: Reformatter enable.
> + * @ccdcslave: CCDC slave mode enable.
> + * @syncif_ipmod: Image
> + * @obclamp_en: Data input format.
> + * @mutexlock: Mutex used to get access to the CCDC.
> + */
> +static struct isp_ccdc {
> + u8 ccdc_inuse;
> + u32 ccdcout_w;
> + u32 ccdcout_h;
> + u32 ccdcin_w;
> + u32 ccdcin_h;
> + u32 ccdcin_woffset;
> + u32 ccdcin_hoffset;
> + u32 crop_w;
> + u32 crop_h;
> + u8 ccdc_inpfmt;
> + u8 ccdc_outfmt;
> + u8 vpout_en;
> + u8 wen;
> + u8 exwen;
> + u8 refmt_en;
> + u8 ccdcslave;
> + u8 syncif_ipmod;
> + u8 obclamp_en;
> + u8 lsc_en;
> + struct mutex mutexlock; /* For checking/modifying ccdc_inuse */
> + u32 wenlog;
> +} ispccdc_obj;
> +
> +static struct ispccdc_lsc_config lsc_config;
> +static u8 *lsc_gain_table;
> +static unsigned long lsc_ispmmu_addr;
> +static int lsc_initialized;
> +static u8 ccdc_use_lsc;
> +static u8 *lsc_gain_table_tmp;
> +
> +/* Structure for saving/restoring CCDC module registers*/
> +static struct isp_reg ispccdc_reg_list[] = {
> + {OMAP3_ISP_IOMEM_CCDC, ISPCCDC_SYN_MODE, 0},
> + {OMAP3_ISP_IOMEM_CCDC, ISPCCDC_HD_VD_WID, 0},
> + {OMAP3_ISP_IOMEM_CCDC, ISPCCDC_PIX_LINES, 0},
> + {OMAP3_ISP_IOMEM_CCDC, ISPCCDC_HORZ_INFO, 0},
> + {OMAP3_ISP_IOMEM_CCDC, ISPCCDC_VERT_START, 0},
> + {OMAP3_ISP_IOMEM_CCDC, ISPCCDC_VERT_LINES, 0},
> + {OMAP3_ISP_IOMEM_CCDC, ISPCCDC_CULLING, 0},
> + {OMAP3_ISP_IOMEM_CCDC, ISPCCDC_HSIZE_OFF, 0},
> + {OMAP3_ISP_IOMEM_CCDC, ISPCCDC_SDOFST, 0},
> + {OMAP3_ISP_IOMEM_CCDC, ISPCCDC_SDR_ADDR, 0},
> + {OMAP3_ISP_IOMEM_CCDC, ISPCCDC_CLAMP, 0},
> + {OMAP3_ISP_IOMEM_CCDC, ISPCCDC_DCSUB, 0},
> + {OMAP3_ISP_IOMEM_CCDC, ISPCCDC_COLPTN, 0},
> + {OMAP3_ISP_IOMEM_CCDC, ISPCCDC_BLKCMP, 0},
> + {OMAP3_ISP_IOMEM_CCDC, ISPCCDC_FPC, 0},
> + {OMAP3_ISP_IOMEM_CCDC, ISPCCDC_FPC_ADDR, 0},
> + {OMAP3_ISP_IOMEM_CCDC, ISPCCDC_VDINT, 0},
> + {OMAP3_ISP_IOMEM_CCDC, ISPCCDC_ALAW, 0},
> + {OMAP3_ISP_IOMEM_CCDC, ISPCCDC_REC656IF, 0},
> + {OMAP3_ISP_IOMEM_CCDC, ISPCCDC_CFG, 0},
> + {OMAP3_ISP_IOMEM_CCDC, ISPCCDC_FMTCFG, 0},
> + {OMAP3_ISP_IOMEM_CCDC, ISPCCDC_FMT_HORZ, 0},
> + {OMAP3_ISP_IOMEM_CCDC, ISPCCDC_FMT_VERT, 0},
> + {OMAP3_ISP_IOMEM_CCDC, ISPCCDC_FMT_ADDR0, 0},
> + {OMAP3_ISP_IOMEM_CCDC, ISPCCDC_FMT_ADDR1, 0},
> + {OMAP3_ISP_IOMEM_CCDC, ISPCCDC_FMT_ADDR2, 0},
> + {OMAP3_ISP_IOMEM_CCDC, ISPCCDC_FMT_ADDR3, 0},
> + {OMAP3_ISP_IOMEM_CCDC, ISPCCDC_FMT_ADDR4, 0},
> + {OMAP3_ISP_IOMEM_CCDC, ISPCCDC_FMT_ADDR5, 0},
> + {OMAP3_ISP_IOMEM_CCDC, ISPCCDC_FMT_ADDR6, 0},
> + {OMAP3_ISP_IOMEM_CCDC, ISPCCDC_FMT_ADDR7, 0},
> + {OMAP3_ISP_IOMEM_CCDC, ISPCCDC_PRGEVEN0, 0},
>
Re: [REVIEW PATCH 13/14] OMAP: CAM: Add DW9710 Lens Driver
Hello, guys
Sorry me, answering to old letter. May i suggest two small points
described below ?
On Mon, 2009-01-12 at 20:03 -0600, Aguirre Rodriguez, Sergio Alberto
wrote:
> This adds the DW9710 Lens driver
>
> Signed-off-by: Sergio Aguirre
> ---
> drivers/media/video/Kconfig |8 +
> drivers/media/video/Makefile |1 +
> drivers/media/video/dw9710.c | 548
> +
> drivers/media/video/dw9710_priv.h | 57
> include/media/dw9710.h| 35 +++
> 5 files changed, 649 insertions(+), 0 deletions(-)
> create mode 100644 drivers/media/video/dw9710.c
> create mode 100644 drivers/media/video/dw9710_priv.h
> create mode 100644 include/media/dw9710.h
>
> diff --git a/drivers/media/video/Kconfig b/drivers/media/video/Kconfig
> index 1616c32..10075c3 100644
> --- a/drivers/media/video/Kconfig
> +++ b/drivers/media/video/Kconfig
> @@ -313,6 +313,14 @@ config VIDEO_MT9P012
> MT9P012 camera. It is currently working with the TI OMAP3
> camera controller.
>
> +config VIDEO_DW9710
> + tristate "Lens driver for DW9710"
> + depends on I2C && VIDEO_V4L2
> + ---help---
> + This is a Video4Linux2 lens driver for the Dongwoon
> + DW9710 coil. It is currently working with the TI OMAP3
> + camera controller and micron MT9P012 sensor.
> +
> config VIDEO_SAA7110
> tristate "Philips SAA7110 video decoder"
> depends on VIDEO_V4L1 && I2C
> diff --git a/drivers/media/video/Makefile b/drivers/media/video/Makefile
> index f73b65c..41c71d5 100644
> --- a/drivers/media/video/Makefile
> +++ b/drivers/media/video/Makefile
> @@ -101,6 +101,7 @@ obj-$(CONFIG_VIDEO_OV7670) += ov7670.o
>
> obj-$(CONFIG_VIDEO_TCM825X) += tcm825x.o
> obj-$(CONFIG_VIDEO_MT9P012)+= mt9p012.o
> +obj-$(CONFIG_VIDEO_DW9710) += dw9710.o
>
> obj-$(CONFIG_VIDEO_OMAP3) += omap34xxcam.o isp/
>
> diff --git a/drivers/media/video/dw9710.c b/drivers/media/video/dw9710.c
> new file mode 100644
> index 000..362cb0d
> --- /dev/null
> +++ b/drivers/media/video/dw9710.c
> @@ -0,0 +1,548 @@
> +/*
> + * drivers/media/video/dw9710.c
> + *
> + * DW9710 Coil Motor (LENS) driver
> + *
> + * Copyright (C) 2008 Texas Instruments.
> + *
> + * Contributors:
> + * Troy Laramy
> + * Mohit Jalori
> + *
> + * This file is licensed under the terms of the GNU General Public License
> + * version 2. This program is licensed "as is" without any warranty of any
> + * kind, whether express or implied.
> + *
> + */
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +#include
> +
> +#include
> +#include
> +
> +#include "dw9710_priv.h"
> +
> +static struct dw9710_device dw9710 = {
> + .state = DW9710_LENS_NOT_DETECTED,
> + .current_lens_posn = DW9710_DEF_LENS_POSN,
> +};
> +
> +static struct vcontrol {
> + struct v4l2_queryctrl qc;
> + int current_value;
> +} video_control[] = {
> + {
> + {
> + .id = V4L2_CID_FOCUS_ABSOLUTE,
> + .type = V4L2_CTRL_TYPE_INTEGER,
> + .name = "Focus, Absolute",
> + .minimum = 0,
> + .maximum = DW9710_MAX_FOCUS_POS,
> + .step = DW9710_LENS_POSN_STEP,
> + .default_value = DW9710_DEF_LENS_POSN,
> + },
> + .current_value = DW9710_DEF_LENS_POSN,
> + }
> +};
> +
> +/**
> + * find_vctrl - Finds the requested ID in the video control structure array
> + * @id: ID of control to search the video control array for
> + *
> + * Returns the index of the requested ID from the control structure array
> + */
> +static int find_vctrl(int id)
> +{
> + int i;
> +
> + if (id < V4L2_CID_BASE)
> + return -EDOM;
> +
> + for (i = (ARRAY_SIZE(video_control) - 1); i >= 0; i--) {
> + if (video_control[i].qc.id == id)
> + return i;
> + }
> +
> + return -EINVAL;
> +}
> +
> +/**
> + * dw9710_reg_read - Reads a value from a register in DW9710 Coil driver
> device.
> + * @client: Pointer to structure of I2C client.
> + * @value: Pointer to u16 for returning value of register to read.
> + *
> + * Returns zero if successful, or non-zero otherwise.
> + **/
> +static int dw9710_reg_read(struct i2c_client *client, u16 *value)
> +{
> + int err;
> + struct i2c_msg msg[1];
> + unsigned char data[2];
> +
> + if (!client->adapter)
> + return -ENODEV;
> +
> + msg->addr = client->addr;
> + msg->flags = I2C_M_RD;
> + msg->len = 2;
> + msg->buf = data;
> +
> + data[0] = 0;
> + data[1] = 0;
> +
> + err = i2c_transfer(client->adapter, msg, 1);
> +
> + if (err >= 0) {
> + err = ((data[0] & 0xFF) << 8) | (data[1]);
> + *value = err;
> + return 0;
> + }
> + r
Re: [REVIEW PATCH 2/2] OMAP3EVM Multi-Media Daughter Card Support
Hello, Vaibhav May i tell few suggestions ? On Fri, 2009-01-30 at 00:52 +0530, hvaib...@ti.com wrote: > From: Vaibhav Hiremath > > This is second version of OMAP3EVM Mulit-Media/Mass Market > Daughter Card support. > > Fixes: > - Cleaned unused header files, struct formating, and unused > comments. > - Pad/mux configuration handled in mux.ch > - mux.ch related changes moved to seperate patch > - Renamed file board-omap3evm-dc.c to board-omap3evm-dc-v4l.c > to make more explicit. > - Added some more meaningful name for Kconfig option > > TODO: > - Camera sensor support (for future development). > - Driver header file inclusion (dependency on ISP-Camera patches) > I am working with Sergio to seperate/move header file to standard > location. > - Still need to fix naming convention for DC > > Tested: > - TVP5146 (BT656) decoder interface on top of > Sergio's ISP-Camera patches. > - Loopback application, capturing image through TVP5146 > and saving it to file per frame. > - Basic functionality of HSUSB Transceiver USB-83320 > > Signed-off-by: Brijesh Jadav > Signed-off-by: Hardik Shah > Signed-off-by: Vaibhav Hiremath > --- > arch/arm/mach-omap2/Kconfig |8 +- > arch/arm/mach-omap2/Makefile|1 + > arch/arm/mach-omap2/board-omap3evm-dc-v4l.c | 348 > +++ > arch/arm/mach-omap2/board-omap3evm-dc.h | 42 > 4 files changed, 398 insertions(+), 1 deletions(-) > create mode 100644 arch/arm/mach-omap2/board-omap3evm-dc-v4l.c > create mode 100644 arch/arm/mach-omap2/board-omap3evm-dc.h > > diff --git a/arch/arm/mach-omap2/Kconfig b/arch/arm/mach-omap2/Kconfig > index 8fa650d..c1cf770 100644 > --- a/arch/arm/mach-omap2/Kconfig > +++ b/arch/arm/mach-omap2/Kconfig > @@ -113,7 +113,7 @@ config MACH_OMAP_LDP > bool "OMAP3 LDP board" > depends on ARCH_OMAP3 && ARCH_OMAP34XX > > -config MACH_OMAP2EVM > +config MACH_OMAP2EVM > bool "OMAP 2530 EVM board" > depends on ARCH_OMAP2 && ARCH_OMAP24XX > > @@ -125,6 +125,12 @@ config MACH_OMAP3EVM > bool "OMAP 3530 EVM board" > depends on ARCH_OMAP3 && ARCH_OMAP34XX > > +config MACH_OMAP3EVM_MMDC > + bool "OMAP 3530 EVM Mass Market Daughter Card board" > + depends on MACH_OMAP3EVM > + help > + Set this if you've got a Mass Market Daughter Card board. > + > config MACH_OMAP3_BEAGLE > bool "OMAP3 BEAGLE board" > depends on ARCH_OMAP3 && ARCH_OMAP34XX > diff --git a/arch/arm/mach-omap2/Makefile b/arch/arm/mach-omap2/Makefile > index 631166d..45f52ca 100644 > --- a/arch/arm/mach-omap2/Makefile > +++ b/arch/arm/mach-omap2/Makefile > @@ -58,6 +58,7 @@ obj-$(CONFIG_MACH_OMAP3EVM) += board-omap3evm.o \ > usb-musb.o usb-ehci.o \ > board-omap3evm-flash.o \ > twl4030-generic-scripts.o > +obj-$(CONFIG_MACH_OMAP3EVM_MMDC) += board-omap3evm-dc-v4l.o > obj-$(CONFIG_MACH_OMAP3_BEAGLE) += board-omap3beagle.o \ > usb-musb.o usb-ehci.o \ > mmc-twl4030.o \ > diff --git a/arch/arm/mach-omap2/board-omap3evm-dc-v4l.c > b/arch/arm/mach-omap2/board-omap3evm-dc-v4l.c > new file mode 100644 > index 000..a7b785e > --- /dev/null > +++ b/arch/arm/mach-omap2/board-omap3evm-dc-v4l.c > @@ -0,0 +1,348 @@ > +/* > + * arch/arm/mach-omap2/board-omap3evm-dc-v4l.c > + * > + * Driver for OMAP3 EVM Mass Market Daughter Card > + * > + * Copyright (C) 2008 Texas Instruments Inc > + * Author: Vaibhav Hiremath > + * > + * Contributors: > + * Anuj Aggarwal > + * Sivaraj R > + * > + * This package 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., 675 Mass Ave, Cambridge, MA 02139, USA. > + * > + */ > + > +#include > +#include > +#include > +#include > + > +#include > + > +#include > +#include > + > +/* Include V4L2 ISP-Camera driver related header file */ > +#include <../drivers/media/video/omap34xxcam.h> > +#include <../drivers/media/video/isp/ispreg.h> > + > +#include "board-omap3evm-dc.h" > + > +#define MODULE_NAME "omap3evmdc" > + > +/* Macro Definitions */ > + > +/* GPIO pins */ > +#define GPIO134_SEL_TVP_Y(134) > +#define GPIO54_SEL_EXP_CAM (54) > +#define GPIO136_SEL_CAM
Re: [OMAPZOOM][PATCH v2 2/6] Increase isp workaround buffer size for 8MP sensor.
Hello, Dominic
May i ask few questions ?
Well, looks like it's unrelated to your patch. Lines that don't looks
okay to me appear in your patch.
On Fri, 2009-01-30 at 17:45 -0600, Dominic Curran wrote:
> From: Dominic Curran
> Subject: [OMAPZOOM][PATCH v2 2/6] Increase isp workaround buffer size for 8MP
> sensor.
>
> A temporary buffer is created to hold the image while it is written by
> Previewer module and then read by Resizer module. This is called LSC
> Workaround. To take into account the Sony IMX046 8MP sensor that buffer
> needs to be increased in size.
> Changed the #defines to be upper case.
> Patch also fixes the initialization of a couple of CCDC values.
>
> Signed-off-by: Dominic Curran
> ---
> drivers/media/video/isp/isp.c | 10 +-
> drivers/media/video/isp/isp.h |7 +--
> drivers/media/video/isp/ispccdc.c |2 ++
> drivers/media/video/isp/ispmmu.h |3 +++
> 4 files changed, 15 insertions(+), 7 deletions(-)
>
> Index: omapzoom04/drivers/media/video/isp/isp.c
> ===
> --- omapzoom04.orig/drivers/media/video/isp/isp.c
> +++ omapzoom04/drivers/media/video/isp/isp.c
> @@ -1172,20 +1172,20 @@ void omapisp_unset_callback()
> **/
> u32 isp_buf_allocation(void)
> {
> - buff_addr = (void *) vmalloc(buffer_size);
> + buff_addr = (void *) vmalloc(ISP_BUFFER_MAX_SIZE);
>
> if (!buff_addr) {
> printk(KERN_ERR "Cannot allocate memory ");
Will user understand what module (or system of kernel) provide this
printk message ? Should module name be here ?
> return -ENOMEM;
> }
>
> - sglist_alloc = videobuf_vmalloc_to_sg(buff_addr, no_of_pages);
> + sglist_alloc = videobuf_vmalloc_to_sg(buff_addr, ISP_BUFFER_MAX_PAGES);
> if (!sglist_alloc) {
> printk(KERN_ERR "videobuf_vmalloc_to_sg error");
Well, may be here too..
By the way, why there is no "\n" in the end of messages in this
function ?
> return -ENOMEM;
> }
> - num_sc = dma_map_sg(NULL, sglist_alloc, no_of_pages, 1);
> - buff_addr_mapped = ispmmu_map_sg(sglist_alloc, no_of_pages);
> + num_sc = dma_map_sg(NULL, sglist_alloc, ISP_BUFFER_MAX_PAGES, 1);
> + buff_addr_mapped = ispmmu_map_sg(sglist_alloc, ISP_BUFFER_MAX_PAGES);
> if (!buff_addr_mapped) {
> printk(KERN_ERR "ispmmu_map_sg mapping failed ");
Probably the same thing here.
May be someone can correct sitation if necessary..
> return -ENOMEM;
> @@ -1217,7 +1217,7 @@ void isp_buf_free(void)
> {
> if (alloc_done == 1) {
> ispmmu_unmap(buff_addr_mapped);
> - dma_unmap_sg(NULL, sglist_alloc, no_of_pages, 1);
> + dma_unmap_sg(NULL, sglist_alloc, ISP_BUFFER_MAX_PAGES, 1);
> kfree(sglist_alloc);
> vfree(buff_addr);
> alloc_done = 0;
> Index: omapzoom04/drivers/media/video/isp/isp.h
> ===
> --- omapzoom04.orig/drivers/media/video/isp/isp.h
> +++ omapzoom04/drivers/media/video/isp/isp.h
> @@ -26,6 +26,9 @@
> #define OMAP_ISP_TOP_H
> #include
> #include
> +
> +#include "ispmmu.h"
> +
> #define OMAP_ISP_CCDC(1 << 0)
> #define OMAP_ISP_PREVIEW (1 << 1)
> #define OMAP_ISP_RESIZER (1 << 2)
> @@ -69,8 +72,8 @@
> #define NUM_ISP_CAPTURE_FORMATS (sizeof(isp_formats) /\
> sizeof(isp_formats[0]))
> #define ISP_WORKAROUND 1
> -#define buffer_size (1024 * 1024 * 10)
> -#define no_of_pages (buffer_size / (4 * 1024))
> +#define ISP_BUFFER_MAX_SIZE (1024 * 1024 * 16)
> +#define ISP_BUFFER_MAX_PAGES (ISP_BUFFER_MAX_SIZE / ISPMMU_PAGE_SIZE)
>
> typedef int (*isp_vbq_callback_ptr) (struct videobuf_buffer *vb);
> typedef void (*isp_callback_t) (unsigned long status,
> Index: omapzoom04/drivers/media/video/isp/ispccdc.c
> ===
> --- omapzoom04.orig/drivers/media/video/isp/ispccdc.c
> +++ omapzoom04/drivers/media/video/isp/ispccdc.c
> @@ -1265,6 +1265,8 @@ int ispccdc_config_size(u32 input_w, u32
> }
>
> if (ispccdc_obj.ccdc_outfmt == CCDC_OTHERS_VP) {
> + ispccdc_obj.ccdcin_woffset = 0;
> + ispccdc_obj.ccdcin_hoffset = 0;
> omap_writel((ispccdc_obj.ccdcin_woffset <<
> ISPCCDC_FMT_HORZ_FMTSPH_SHIFT) |
> (ispccdc_obj.ccdcin_w <<
> Index: omapzoom04/drivers/media/video/isp/ispmmu.h
> ===
> --- omapzoom04.orig/drivers/media/video/isp/ispmmu.h
> +++ omapzoom04/drivers/media/video/isp/ispmmu.h
> @@ -59,6 +59,9 @@
> /* Number of entries per L2 Page table */
> #define ISPMMU_L2D_ENTRIES_NR256
>
> +/* Size of MMU page in bytes */
> +#define ISPMMU_PAGE_SIZE
Re: [REVIEW PATCH 2/2] Added OMAP3EVM Multi-Media Daughter Card Support
Hello, just two small comments if you don't mind. On Wed, 2009-01-07 at 11:37 +0530, hvaib...@ti.com wrote: > From: Vaibhav Hiremath > > This is first version of OMAP3EVM Mulit-Media Daughter > Card support. > > Tested: > - TVP5146 (BT656) decoder interface on top of > Sergio's ISP-Camera patches. > - Loopback application, capturing image through TVP5146 > and displaying it onto the TV/LCD on top of Hardik's > V4L2 driver. > - Basic functionality of HSUSB Transceiver USB-83320 > - > > TODO: > - Camera sensor support > - Driver header file inclusion (dependency on ISP-Camera patches) > - Some more clean-up may required. > > Signed-off-by: Brijesh Jadav > Signed-off-by: Hardik Shah > Signed-off-by: Manjunath Hadli > Signed-off-by: R Sivaraj > Signed-off-by: Vaibhav Hiremath > --- > arch/arm/mach-omap2/Kconfig |4 + > arch/arm/mach-omap2/Makefile|1 + > arch/arm/mach-omap2/board-omap3evm-dc.c | 417 > +++ > arch/arm/mach-omap2/board-omap3evm-dc.h | 43 > arch/arm/mach-omap2/mux.c |7 + > arch/arm/plat-omap/include/mach/mux.h |4 + > 6 files changed, 476 insertions(+), 0 deletions(-) > mode change 100644 => 100755 arch/arm/mach-omap2/Kconfig > mode change 100644 => 100755 arch/arm/mach-omap2/Makefile > create mode 100755 arch/arm/mach-omap2/board-omap3evm-dc.c > create mode 100755 arch/arm/mach-omap2/board-omap3evm-dc.h > mode change 100644 => 100755 arch/arm/mach-omap2/mux.c > mode change 100644 => 100755 arch/arm/plat-omap/include/mach/mux.h > > diff --git a/arch/arm/mach-omap2/Kconfig b/arch/arm/mach-omap2/Kconfig > old mode 100644 > new mode 100755 > index ca24a7a..094c97f > --- a/arch/arm/mach-omap2/Kconfig > +++ b/arch/arm/mach-omap2/Kconfig > @@ -121,6 +121,10 @@ config MACH_OMAP3EVM > bool "OMAP 3530 EVM board" > depends on ARCH_OMAP3 && ARCH_OMAP34XX > > +config MACH_OMAP3EVM_DC > + bool "OMAP 3530 EVM daughter card board" > + depends on ARCH_OMAP3 && ARCH_OMAP34XX && MACH_OMAP3EVM > + > config MACH_OMAP3_BEAGLE > bool "OMAP3 BEAGLE board" > depends on ARCH_OMAP3 && ARCH_OMAP34XX > diff --git a/arch/arm/mach-omap2/Makefile b/arch/arm/mach-omap2/Makefile > old mode 100644 > new mode 100755 > index 3897347..16fa35a > --- a/arch/arm/mach-omap2/Makefile > +++ b/arch/arm/mach-omap2/Makefile > @@ -60,6 +60,7 @@ obj-$(CONFIG_MACH_OMAP3EVM) += board-omap3evm.o \ > usb-musb.o usb-ehci.o \ > board-omap3evm-flash.o \ > twl4030-generic-scripts.o > +obj-$(CONFIG_MACH_OMAP3EVM_DC) += board-omap3evm-dc.o > obj-$(CONFIG_MACH_OMAP3_BEAGLE) += board-omap3beagle.o \ > usb-musb.o usb-ehci.o \ > mmc-twl4030.o \ > diff --git a/arch/arm/mach-omap2/board-omap3evm-dc.c > b/arch/arm/mach-omap2/board-omap3evm-dc.c > new file mode 100755 > index 000..233c219 > --- /dev/null > +++ b/arch/arm/mach-omap2/board-omap3evm-dc.c > @@ -0,0 +1,417 @@ > +/* > + * arch/arm/mach-omap2/board-omap3evm-dc.c > + * > + * Driver for OMAP3 EVM Daughter Card > + * > + * Copyright (C) 2008 Texas Instruments Inc > + * Author: Vaibhav Hiremath > + * > + * Contributors: > + * Anuj Aggarwal > + * Sivaraj R > + * > + * This package 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., 675 Mass Ave, Cambridge, MA 02139, USA. > + * > + */ > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +#include > +#include > + > +#if defined(CONFIG_VIDEO_TVP514X) || defined(CONFIG_VIDEO_TVP514X_MODULE) > +#include > +#include > +#include > +/* include V4L2 camera driver related header file */ > +#if defined(CONFIG_VIDEO_OMAP3) || defined(CONFIG_VIDEO_OMAP3_MODULE) > +#include <../drivers/media/video/omap34xxcam.h> > +#include <../drivers/media/video/isp/ispreg.h> > +#endif /* #ifdef CONFIG_VIDEO_OMAP3 */ > +#endif /* #ifdef CONFIG_VIDEO_TVP514X*/ > + > +#include "board-omap3evm-dc.h" > + > +#define MODULE_NAME "omap3evmdc" > + > +#ifdef DEBUG > +#define dprintk(fmt, args...) printk(KERN_ERR MODULE_NAME ": " fmt, ## ar
Re: [REVIEW PATCH 13/14] OMAP: CAM: Add Lens Driver
Hello, all
Looked through the patch and have small suggestions, it's about coding
style and releted.
On Thu, Dec 11, 2008 at 11:38 PM, Aguirre Rodriguez, Sergio Alberto
wrote:
> >From 1341b74f5a90e4aa079a4fcb4fe2127ff344cce7 Mon Sep 17 00:00:00 2001
> From: Sergio Aguirre
> Date: Thu, 11 Dec 2008 13:35:46 -0600
> Subject: [PATCH] OMAP: CAM: Add Lens Driver
>
> This adds the DW9710 Lens driver
>
> Signed-off-by: Sergio Aguirre
> ---
> drivers/media/video/Kconfig |8 +
> drivers/media/video/Makefile |1 +
> drivers/media/video/dw9710.c | 569
> ++
> drivers/media/video/dw9710.h | 59 +
> 4 files changed, 637 insertions(+), 0 deletions(-)
> create mode 100644 drivers/media/video/dw9710.c
> create mode 100644 drivers/media/video/dw9710.h
>
> diff --git a/drivers/media/video/Kconfig b/drivers/media/video/Kconfig
> index 24957bc..50651f2 100644
> --- a/drivers/media/video/Kconfig
> +++ b/drivers/media/video/Kconfig
> @@ -313,6 +313,14 @@ config VIDEO_MT9P012
> MT9P012 camera. It is currently working with the TI OMAP3
> camera controller.
>
> +config VIDEO_DW9710
> + tristate "Lens driver for DW9710"
> + depends on I2C && VIDEO_V4L2
> + ---help---
> + This is a Video4Linux2 lens driver for the Dongwoon
> + DW9710 coil. It is currently working with the TI OMAP3
> + camera controller and micron MT9P012 sensor.
> +
> config VIDEO_SAA7110
>tristate "Philips SAA7110 video decoder"
>depends on VIDEO_V4L1 && I2C
> diff --git a/drivers/media/video/Makefile b/drivers/media/video/Makefile
> index f73b65c..41c71d5 100644
> --- a/drivers/media/video/Makefile
> +++ b/drivers/media/video/Makefile
> @@ -101,6 +101,7 @@ obj-$(CONFIG_VIDEO_OV7670) += ov7670.o
>
> obj-$(CONFIG_VIDEO_TCM825X) += tcm825x.o
> obj-$(CONFIG_VIDEO_MT9P012)+= mt9p012.o
> +obj-$(CONFIG_VIDEO_DW9710) += dw9710.o
>
> obj-$(CONFIG_VIDEO_OMAP3) += omap34xxcam.o isp/
>
> diff --git a/drivers/media/video/dw9710.c b/drivers/media/video/dw9710.c
> new file mode 100644
> index 000..b5d5247
> --- /dev/null
> +++ b/drivers/media/video/dw9710.c
> @@ -0,0 +1,569 @@
> +/*
> + * drivers/media/video/dw9710.c
> + *
> + * DW9710 Coil Motor (LENS) driver
> + *
> + * Copyright (C) 2008 Texas Instruments.
> + *
> + * Contributors:
> + * Troy Laramy
> + * Mohit Jalori
> + *
> + * This file is licensed under the terms of the GNU General Public License
> + * version 2. This program is licensed "as is" without any warranty of any
> + * kind, whether express or implied.
> + *
> + */
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +#include "dw9710.h"
> +
> +#define DRIVER_NAME "dw9710"
Here you define DRIVER_NAME. I saw patch(for another module) that
remover MODULE_NAME and places VIVI_MODULE_NAME there. Here it is:
http://linuxtv.org/hg/v4l-dvb/rev/58b95134acb8
May be it's better to define something like this: #define
DW9710_DRIVER_NAME "dw9710" ?
Probably it will help you to avoid possible namespace conflicts and
code become a bit more readible.
> +static int
> +dw9710_probe(struct i2c_client *client, const struct i2c_device_id *id);
> +static int __exit dw9710_remove(struct i2c_client *client);
> +
> +struct dw9710_device {
> + const struct dw9710_platform_data *pdata;
> + struct v4l2_int_device *v4l2_int_device;
> + struct i2c_client *i2c_client;
> + int opened;
> + u16 current_lens_posn;
> + u16 saved_lens_posn;
> + int state;
> + int power_state;
> +};
> +
> +static const struct i2c_device_id dw9710_id[] = {
> + { DW9710_NAME, 0 },
> + { }
> +};
Hmmm, looks like you already defined DW9710_NAME in header-file. Why
didn't you reformat to use _one_ define for this and previous place ?
> +MODULE_DEVICE_TABLE(i2c, dw9710_id);
> +
> +static struct i2c_driver dw9710_i2c_driver = {
> + .driver = {
> + .name = DW9710_NAME,
Actually, the same thing here.
> + .owner = THIS_MODULE,
> + },
> + .probe = dw9710_probe,
> + .remove = __exit_p(dw9710_remove),
> + .id_table = dw9710_id,
> +};
> +
> +static struct dw9710_device dw9710 = {
> + .state = LENS_NOT_DETECTED,
> + .current_lens_posn = DEF_LENS_POSN,
> +};
> +
> +static struct vcontrol {
> + struct v4l2_queryctrl qc;
> + int current_value;
> +} video_control[] = {
> + {
> + {
> + .id = V4L2_CID_FOCUS_ABSOLUTE,
> + .type = V4L2_CTRL_TYPE_INTEGER,
> + .name = "Lens Position",
> + .minimum = 0,
> + .maximum = MAX_FOCUS_POS,
> + .step = LENS_POSN_STEP,
> + .default_value = DEF_LENS_POSN,
> + },
> + .current_value = DEF_LENS_POSN,
> + }
> +};
> +
> +static str
