public inbox for linux-media@vger.kernel.org
 help / color / mirror / Atom feed
* omap3 camera driver
@ 2008-06-26 22:53 Jalori, Mohit
  2008-06-27 10:16 ` Trilok Soni
  0 siblings, 1 reply; 11+ messages in thread
From: Jalori, Mohit @ 2008-06-26 22:53 UTC (permalink / raw)
  To: video4linux-list@redhat.com

Hi,

Here is the high level OMAP 3 Camera driver design and link to source code for review.

OMAP3 camera subsystem integrates two serial (CCP2, CSI2) and one parallel camera interface, a ccdc module, a preview module, a resizer module, a statistics collection module and a dedicated MMU.

OMAP camera driver (omap34xxcam.c, omap34xxcam.h): OMAP3 camera driver allows capturing and previewing images through a camera sensor. It is a V4L2-compliant driver with additions targeting an OMAP3 ISP hardware features. It interfaces with V4L2 sensor drivers using the V4L2 master slave i/f. Camera driver is also capable of handling multiple slaves (it can have a sensor as a slave and an associate lens actuator and flash as other slaves.
Each sensor driver exports the type of interface, serial or parallel, and type of sensor, raw or smart sensor. Sensor driver also makes call to ISP library to setup the interface specific parameters and i/p clocks.

ISP Library (isp.c, isp.h, ispreg.h): The ISP library exports APIs to configure ISP module and clocks to the sensor. It is the central interrupt handler where callback routines for ISP interrupts are handled.

CCDC library (ispccdc.c, ispccdc.h): CCDC is a HW block in Camera ISP which acts as a data input port. It receives data from the sensor through parallel or serial interface. The CCDC library exports API to configure CCDC. It is configured by the camera driver based on the sensor attached and desired output from the camera driver.

Preview library (isppreview.c, isppreview.h): Preview is a HW block in Camera ISP which is responsible for image processing and color conversion. It has HW blocks for image processing algorithms. Preview library allows camera driver to configure, enable and disable the individual HW blocks in the preview module.

Resizer library (ispresizer.c, ispresizer.h): Resizer is a HW block in Camera ISP which is responsible for image downscaling and upscaling. It has HW filters which resize the input image based on input/output configuration. Resizer library allows camera driver to query and configure the resizer module. Resizer in OMAP3 ISP supports on the fly resizing with ratios from 1/4 to 4. When used with camera driver resizer only supports on the fly mode of operation. In this mode image is taken from sensor and passed to application without any memory to memory operations in ISP and so multipass resizer operations are not supported. Resizer also has standalone, multipass resizer driver which can be used to overcome this limitation.

H3A library (isph3a.c, isph3a.h, isphist.c, isphist.h): H3A is a HW block in Camera ISP which is responsible for collecting image statistics that can be used by other algorithms. It generates auto focus, auto white balance, auto exposure and histogram statistics. H3A library allows user space algorithms to configure and request these statistics through private IOCTLs.

MMU Library (ispmmu.c, ispmmu.h): The ISP MMU library exports APIs that allow mapping of buffer to ISP MMU. This allows camera to work with discontiguous memory.

Table files (noise filter, gamma tables): Default values for these tables are maintained in the header files.

Private ioctls: We have exported private ioctls through which user space applications can request statistics for the images and apply feedback to OMAP3 ISP or sensor modules. Typically user space applications can request Auto-exposure, Auto white balance and Auto Focus statistics. They can use the same private IOCTL to update the parameters or use V4L2 specific controls to do so.

Private CIDs. One private CID is used to set color effects. Currently supported ones are B&W and Sepia.


Changes in V4L2
Following changes have been made to linux/mediav4l2-int-device.h file:
Additions of enums for power states. There are 4 possible power states defined which can be used with the s_power ioctls. Most sensor support standby mode so this allows us to put the sensors in standby mode.
Addition of CCP2 as one of the i/f types to v4l2_if_types and then we have the structures for supporting this interface.
Adding crop capability in the vidioc_int_ioctls.

Added defines for RAW10 bayer and Bayer DPCM data format in include/linux/videodev2.h


File organization
Camera driver is maintained under drivers/media/video directory
ISP files are under driver/media/video/isp directory
One include file is maintained under include/asm-arm/arch-omap/isp_user.h. this file is needed for private IOCTL implementation for data structure definitions


All files are available at http://linux.omap.com/pub/patches/rfc/34xxcamisp/omap34xx_camera_files.tar.gz. The files are available for review and will be shared as patches once review is complete.


Contributors:
Mohit Jalori
Leonides Martinez de la Cruz
Sameer Venkatraman
Senthilvadidu Guruswamy
Sakari Alius
Thara Gopinath
Tukka Toivanen
Toni Leinonen

Regards
Mohit


--
video4linux-list mailing list
Unsubscribe mailto:video4linux-list-request@redhat.com?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: omap3 camera driver
  2008-06-26 22:53 omap3 camera driver Jalori, Mohit
@ 2008-06-27 10:16 ` Trilok Soni
  2008-06-27 21:57   ` Daniel Glöckner
  2008-07-01  3:36   ` Jalori, Mohit
  0 siblings, 2 replies; 11+ messages in thread
From: Trilok Soni @ 2008-06-27 10:16 UTC (permalink / raw)
  To: Jalori, Mohit; +Cc: video4linux-list@redhat.com

Hi Mohit,

On Fri, Jun 27, 2008 at 4:23 AM, Jalori, Mohit <mjalori@ti.com> wrote:
> Hi,
>

...
>
>
> All files are available at http://linux.omap.com/pub/patches/rfc/34xxcamisp/omap34xx_camera_files.tar.gz. The files are available for review and will be shared as patches once review is complete.
>


This way of distribution of code doesn't help in review. Please
break-up this code into multiple patches as per the functionality, so
that we can review it.


-- 
---Trilok Soni
http://triloksoni.wordpress.com

--
video4linux-list mailing list
Unsubscribe mailto:video4linux-list-request@redhat.com?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: omap3 camera driver
  2008-06-27 10:16 ` Trilok Soni
@ 2008-06-27 21:57   ` Daniel Glöckner
  2008-06-27 22:08     ` hermann pitton
  2008-07-01  3:36   ` Jalori, Mohit
  1 sibling, 1 reply; 11+ messages in thread
From: Daniel Glöckner @ 2008-06-27 21:57 UTC (permalink / raw)
  To: Jalori, Mohit; +Cc: video4linux-list@redhat.com

On Fri, Jun 27, 2008 at 03:46:41PM +0530, Trilok Soni wrote:
> On Fri, Jun 27, 2008 at 4:23 AM, Jalori, Mohit <mjalori@ti.com> wrote:
> > All files are available at http://linux.omap.com/pub/patches/rfc/34xxcamisp/omap34xx_camera_files.tar.gz. The files are available for review and will be shared as patches once review is complete.
> 
> This way of distribution of code doesn't help in review. Please
> break-up this code into multiple patches as per the functionality, so
> that we can review it.

And by the way, the link doesn't work.

Yes, I tried without the dot.

  Daniel

--
video4linux-list mailing list
Unsubscribe mailto:video4linux-list-request@redhat.com?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: omap3 camera driver
  2008-06-27 21:57   ` Daniel Glöckner
@ 2008-06-27 22:08     ` hermann pitton
  2008-06-28 16:27       ` Jalori, Mohit
  0 siblings, 1 reply; 11+ messages in thread
From: hermann pitton @ 2008-06-27 22:08 UTC (permalink / raw)
  To: Daniel Glöckner; +Cc: video4linux-list@redhat.com

Hi!

Am Freitag, den 27.06.2008, 23:57 +0200 schrieb Daniel Glöckner:
> On Fri, Jun 27, 2008 at 03:46:41PM +0530, Trilok Soni wrote:
> > On Fri, Jun 27, 2008 at 4:23 AM, Jalori, Mohit <mjalori@ti.com> wrote:
> > > All files are available at http://linux.omap.com/pub/patches/rfc/34xxcamisp/omap34xx_camera_files.tar.gz. The files are available for review and will be shared as patches once review is complete.
> > 
> > This way of distribution of code doesn't help in review. Please
> > break-up this code into multiple patches as per the functionality, so
> > that we can review it.
> 
> And by the way, the link doesn't work.
> 
> Yes, I tried without the dot.
> 
>   Daniel
> 

Maybe like that?

http://linux.omap.com/pub/patches/rfc/34xxcamisp

It is .tar.bz.

Greetings,

Hermann


--
video4linux-list mailing list
Unsubscribe mailto:video4linux-list-request@redhat.com?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list

^ permalink raw reply	[flat|nested] 11+ messages in thread

* RE: omap3 camera driver
  2008-06-27 22:08     ` hermann pitton
@ 2008-06-28 16:27       ` Jalori, Mohit
  2008-06-30  8:12         ` Paulius Zaleckas
  0 siblings, 1 reply; 11+ messages in thread
From: Jalori, Mohit @ 2008-06-28 16:27 UTC (permalink / raw)
  To: hermann pitton, Daniel Glöckner; +Cc: video4linux-list@redhat.com



> -----Original Message-----
> From: hermann pitton [mailto:hermann-pitton@arcor.de]
> Sent: Friday, June 27, 2008 5:09 PM
> To: Daniel Glöckner
> Cc: Jalori, Mohit; video4linux-list@redhat.com
> Subject: Re: omap3 camera driver
>
> Hi!
>
> Am Freitag, den 27.06.2008, 23:57 +0200 schrieb Daniel Glöckner:
> > On Fri, Jun 27, 2008 at 03:46:41PM +0530, Trilok Soni wrote:
> > > On Fri, Jun 27, 2008 at 4:23 AM, Jalori, Mohit <mjalori@ti.com> wrote:
> > > > All files are available at
> http://linux.omap.com/pub/patches/rfc/34xxcamisp/omap34xx_camera_files.tar
> .gz. The files are available for review and will be shared as patches once
> review is complete.
> > >
> > > This way of distribution of code doesn't help in review. Please
> > > break-up this code into multiple patches as per the functionality, so
> > > that we can review it.
> >
> > And by the way, the link doesn't work.
> >
> > Yes, I tried without the dot.
> >
> >   Daniel
> >
>
> Maybe like that?
>
> http://linux.omap.com/pub/patches/rfc/34xxcamisp
>
> It is .tar.bz.
>

Sorry about that, yes it is .tar.bz. We will separate out the patches and post them to the list. Some of the files are more than 200kB (LSC table), hopefully list will allow us to send those across.

> Greetings,
>
> Hermann
>


--
video4linux-list mailing list
Unsubscribe mailto:video4linux-list-request@redhat.com?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: omap3 camera driver
  2008-06-28 16:27       ` Jalori, Mohit
@ 2008-06-30  8:12         ` Paulius Zaleckas
  0 siblings, 0 replies; 11+ messages in thread
From: Paulius Zaleckas @ 2008-06-30  8:12 UTC (permalink / raw)
  To: video4linux-list

Have you considered using soc_camera framework?
That would make thing a little bit easier and more generic.

BR,
Paulius Zaleckas

--
video4linux-list mailing list
Unsubscribe mailto:video4linux-list-request@redhat.com?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list

^ permalink raw reply	[flat|nested] 11+ messages in thread

* RE: omap3 camera driver
  2008-06-27 10:16 ` Trilok Soni
  2008-06-27 21:57   ` Daniel Glöckner
@ 2008-07-01  3:36   ` Jalori, Mohit
  1 sibling, 0 replies; 11+ messages in thread
From: Jalori, Mohit @ 2008-07-01  3:36 UTC (permalink / raw)
  To: Trilok Soni; +Cc: video4linux-list@redhat.com

Hi

I am sending out the files as separate patches in next set of emails. There are a total of 16 patches. 2 of these patches are big tables which are available for review at the site below.

http://linux.omap.com/pub/patches/rfc/34xxcamisp/omap34xx_camera_patches.tar.bz

If you want me send those as well in email let me know.

Regards
Mohit


> -----Original Message-----
> From: Trilok Soni [mailto:soni.trilok@gmail.com]
> Sent: Friday, June 27, 2008 5:17 AM
> To: Jalori, Mohit
> Cc: video4linux-list@redhat.com
> Subject: Re: omap3 camera driver
>
> Hi Mohit,
>
> On Fri, Jun 27, 2008 at 4:23 AM, Jalori, Mohit <mjalori@ti.com> wrote:
> > Hi,
> >
>
> ...
> >
> >
> > All files are available at
> http://linux.omap.com/pub/patches/rfc/34xxcamisp/omap34xx_camera_files.tar
> .gz. The files are available for review and will be shared as patches once
> review is complete.
> >
>
>
> This way of distribution of code doesn't help in review. Please
> break-up this code into multiple patches as per the functionality, so
> that we can review it.
>
>
> --
> ---Trilok Soni
> http://triloksoni.wordpress.com

--
video4linux-list mailing list
Unsubscribe mailto:video4linux-list-request@redhat.com?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list

^ permalink raw reply	[flat|nested] 11+ messages in thread

* omap3 camera driver
@ 2008-07-12 10:11 John Smith
  2008-07-12 15:10 ` Jalori, Mohit
  0 siblings, 1 reply; 11+ messages in thread
From: John Smith @ 2008-07-12 10:11 UTC (permalink / raw)
  To: video4linux-list, mjalori

Hi Mohit,
I was just looking to your patch and the first observation I could get
immediately is, checkpatch.pl script is failing on some of patches. I
am not sure, was that missed or intentional? Below is checkpatch
output for the one which I have tired -



./scripts/checkpatch.pl scripts/camera_patches/004_v4l2_sgrbg10_format.patch
need space after that ',' (ctx:VxV)
#19: FILE: include/linux/videodev2.h:312:
+#define V4L2_PIX_FMT_SGRBG10 v4l2_fourcc('B','A','1','0') /* 10bit
raw bayer  */
                                             ^

need space after that ',' (ctx:VxV)
#19: FILE: include/linux/videodev2.h:312:
+#define V4L2_PIX_FMT_SGRBG10 v4l2_fourcc('B','A','1','0') /* 10bit
raw bayer  */
                                                 ^

need space after that ',' (ctx:VxV)
#19: FILE: include/linux/videodev2.h:312:
+#define V4L2_PIX_FMT_SGRBG10 v4l2_fourcc('B','A','1','0') /* 10bit
raw bayer  */
                                                     ^

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

#----------------------------------------------------------------------------------

./scripts/checkpatch.pl
scripts/camera_patches/007_v4l2_sgrbg10dpcm8_format.patch
line over 80 characters
#20: FILE: include/linux/videodev2.h:313:
+#define V4L2_PIX_FMT_SGRBG10DPCM8 v4l2_fourcc('B','D','1','0') /*
10bit raw bayer DPCM compressed to 8 bits */

need space after that ',' (ctx:VxV)
#20: FILE: include/linux/videodev2.h:313:
+#define V4L2_PIX_FMT_SGRBG10DPCM8 v4l2_fourcc('B','D','1','0') /*
10bit raw bayer DPCM compressed to 8 bits */
                                                  ^

need space after that ',' (ctx:VxV)
#20: FILE: include/linux/videodev2.h:313:
+#define V4L2_PIX_FMT_SGRBG10DPCM8 v4l2_fourcc('B','D','1','0') /*
10bit raw bayer DPCM compressed to 8 bits */
                                                      ^

need space after that ',' (ctx:VxV)
#20: FILE: include/linux/videodev2.h:313:
+#define V4L2_PIX_FMT_SGRBG10DPCM8 v4l2_fourcc('B','D','1','0') /*
10bit raw bayer DPCM compressed to 8 bits */
                                                          ^

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.


#----------------------------------------------------------------------------------

./scripts/checkpatch.pl
scripts/camera_patches/010_omap_34xx_camera_driver_isp_basic_blocks.patch
struct mutex definition without comment
#1762: FILE: drivers/media/video/omap34xxcam.h:97:
+       struct mutex mutex;

struct mutex definition without comment
#1816: FILE: drivers/media/video/omap34xxcam.h:151:
+       struct mutex mutex;

spinlock_t definition without comment
#1846: FILE: drivers/media/video/omap34xxcam.h:181:
+       spinlock_t vbq_lock;

Use #include <linux/irq.h> instead of <asm/irq.h>
#1901: FILE: drivers/media/video/isp/isp.c:33:
+#include <asm/irq.h>

spinlock_t definition without comment
#1979: FILE: drivers/media/video/isp/isp.c:111:
+       spinlock_t lock;

spinlock_t definition without comment
#1980: FILE: drivers/media/video/isp/isp.c:112:
+       spinlock_t isp_temp_buf_lock;

struct mutex definition without comment
#1981: FILE: drivers/media/video/isp/isp.c:113:
+       struct mutex isp_mutex;

need consistent spacing around '/' (ctx:WxV)
#3760: FILE: drivers/media/video/isp/isp.h:87:
+#define NUM_ISP_CAPTURE_FORMATS        (sizeof(isp_formats) /\
                                                             ^

do not add new typedefs
#3763: FILE: drivers/media/video/isp/isp.h:90:
+typedef int (*isp_vbq_callback_ptr) (struct videobuf_buffer *vb);

do not add new typedefs
#3764: FILE: drivers/media/video/isp/isp.h:91:
+typedef void (*isp_callback_t) (unsigned long status,

spinlock_t definition without comment
#3836: FILE: drivers/media/video/isp/isp.h:163:
+       spinlock_t lock;

struct mutex definition without comment
#4085: FILE: drivers/media/video/isp/ispccdc.c:83:
+       struct mutex mutexlock;

Use #include <linux/irq.h> instead of <asm/irq.h>
#5544: FILE: drivers/media/video/isp/ispmmu.c:38:
+#include <asm/irq.h>

#if 0 -- if this code redundant remove it
#6397: FILE: drivers/media/video/isp/ispreg.h:26:
+#if 0

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

#-------------------------------------------------------------------------------------------


I would like to understand the design actually, will take some time.

Thanks,
John

--
video4linux-list mailing list
Unsubscribe mailto:video4linux-list-request@redhat.com?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list

^ permalink raw reply	[flat|nested] 11+ messages in thread

* RE: omap3 camera driver
  2008-07-12 10:11 John Smith
@ 2008-07-12 15:10 ` Jalori, Mohit
  2008-07-16 16:06   ` John
  0 siblings, 1 reply; 11+ messages in thread
From: Jalori, Mohit @ 2008-07-12 15:10 UTC (permalink / raw)
  To: John Smith, video4linux-list@redhat.com

Yes it was failing for this for us as well.

I kept the same formatting since other formats were defined that way.
Should I modify all formats with the space in between or keep it this way?



> -----Original Message-----
> From: John Smith [mailto:john.v4l2@gmail.com]
> Sent: Saturday, July 12, 2008 5:12 AM
> To: video4linux-list@redhat.com; Jalori, Mohit
> Subject: omap3 camera driver
>
> Hi Mohit,
> I was just looking to your patch and the first observation I could get
> immediately is, checkpatch.pl script is failing on some of patches. I
> am not sure, was that missed or intentional? Below is checkpatch
> output for the one which I have tired -
>
>
>
> ./scripts/checkpatch.pl
> scripts/camera_patches/004_v4l2_sgrbg10_format.patch
> need space after that ',' (ctx:VxV)
> #19: FILE: include/linux/videodev2.h:312:
> +#define V4L2_PIX_FMT_SGRBG10 v4l2_fourcc('B','A','1','0') /* 10bit
> raw bayer  */
>                                              ^
>
> need space after that ',' (ctx:VxV)
> #19: FILE: include/linux/videodev2.h:312:
> +#define V4L2_PIX_FMT_SGRBG10 v4l2_fourcc('B','A','1','0') /* 10bit
> raw bayer  */
>                                                  ^
>
> need space after that ',' (ctx:VxV)
> #19: FILE: include/linux/videodev2.h:312:
> +#define V4L2_PIX_FMT_SGRBG10 v4l2_fourcc('B','A','1','0') /* 10bit
> raw bayer  */
>                                                      ^
>
> Your patch has style problems, please review.  If any of these errors
> are false positives report them to the maintainer, see
> CHECKPATCH in MAINTAINERS.
>
> #---------------------------------------------------------------------
> -------------
>
> ./scripts/checkpatch.pl
> scripts/camera_patches/007_v4l2_sgrbg10dpcm8_format.patch
> line over 80 characters
> #20: FILE: include/linux/videodev2.h:313:
> +#define V4L2_PIX_FMT_SGRBG10DPCM8 v4l2_fourcc('B','D','1','0') /*
> 10bit raw bayer DPCM compressed to 8 bits */
>
> need space after that ',' (ctx:VxV)
> #20: FILE: include/linux/videodev2.h:313:
> +#define V4L2_PIX_FMT_SGRBG10DPCM8 v4l2_fourcc('B','D','1','0') /*
> 10bit raw bayer DPCM compressed to 8 bits */
>                                                   ^
>
> need space after that ',' (ctx:VxV)
> #20: FILE: include/linux/videodev2.h:313:
> +#define V4L2_PIX_FMT_SGRBG10DPCM8 v4l2_fourcc('B','D','1','0') /*
> 10bit raw bayer DPCM compressed to 8 bits */
>                                                       ^
>
> need space after that ',' (ctx:VxV)
> #20: FILE: include/linux/videodev2.h:313:
> +#define V4L2_PIX_FMT_SGRBG10DPCM8 v4l2_fourcc('B','D','1','0') /*
> 10bit raw bayer DPCM compressed to 8 bits */
>                                                           ^
>
> Your patch has style problems, please review.  If any of these errors
> are false positives report them to the maintainer, see
> CHECKPATCH in MAINTAINERS.
>
>
> #---------------------------------------------------------------------
> -------------
>
> ./scripts/checkpatch.pl
> scripts/camera_patches/010_omap_34xx_camera_driver_isp_basic_blocks.pa
> tch
> struct mutex definition without comment
> #1762: FILE: drivers/media/video/omap34xxcam.h:97:
> +       struct mutex mutex;
>
> struct mutex definition without comment
> #1816: FILE: drivers/media/video/omap34xxcam.h:151:
> +       struct mutex mutex;
>
> spinlock_t definition without comment
> #1846: FILE: drivers/media/video/omap34xxcam.h:181:
> +       spinlock_t vbq_lock;
>
> Use #include <linux/irq.h> instead of <asm/irq.h>
> #1901: FILE: drivers/media/video/isp/isp.c:33:
> +#include <asm/irq.h>
>
> spinlock_t definition without comment
> #1979: FILE: drivers/media/video/isp/isp.c:111:
> +       spinlock_t lock;
>
> spinlock_t definition without comment
> #1980: FILE: drivers/media/video/isp/isp.c:112:
> +       spinlock_t isp_temp_buf_lock;
>
> struct mutex definition without comment
> #1981: FILE: drivers/media/video/isp/isp.c:113:
> +       struct mutex isp_mutex;
>
> need consistent spacing around '/' (ctx:WxV)
> #3760: FILE: drivers/media/video/isp/isp.h:87:
> +#define NUM_ISP_CAPTURE_FORMATS        (sizeof(isp_formats) /\
>                                                              ^
>
> do not add new typedefs
> #3763: FILE: drivers/media/video/isp/isp.h:90:
> +typedef int (*isp_vbq_callback_ptr) (struct videobuf_buffer *vb);
>
> do not add new typedefs
> #3764: FILE: drivers/media/video/isp/isp.h:91:
> +typedef void (*isp_callback_t) (unsigned long status,
>
> spinlock_t definition without comment
> #3836: FILE: drivers/media/video/isp/isp.h:163:
> +       spinlock_t lock;
>
> struct mutex definition without comment
> #4085: FILE: drivers/media/video/isp/ispccdc.c:83:
> +       struct mutex mutexlock;
>
> Use #include <linux/irq.h> instead of <asm/irq.h>
> #5544: FILE: drivers/media/video/isp/ispmmu.c:38:
> +#include <asm/irq.h>
>
> #if 0 -- if this code redundant remove it
> #6397: FILE: drivers/media/video/isp/ispreg.h:26:
> +#if 0
>
> Your patch has style problems, please review.  If any of these errors
> are false positives report them to the maintainer, see
> CHECKPATCH in MAINTAINERS.
>
> #---------------------------------------------------------------------
> ----------------------
>
>
> I would like to understand the design actually, will take some time.
>
> Thanks,
> John

--
video4linux-list mailing list
Unsubscribe mailto:video4linux-list-request@redhat.com?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: omap3 camera driver
  2008-07-12 15:10 ` Jalori, Mohit
@ 2008-07-16 16:06   ` John
  2008-07-18 17:44     ` Jalori, Mohit
  0 siblings, 1 reply; 11+ messages in thread
From: John @ 2008-07-16 16:06 UTC (permalink / raw)
  To: Jalori, Mohit; +Cc: video4linux-list

Hi Mohit,
This patch is fails to apply against the labrador kernel 2.6.22.

Kindly provide details of the kernel against which this patch is
generated and tested.

Regards,

John



On Sat, Jul 12, 2008 at 8:40 PM, Jalori, Mohit <mjalori@ti.com> wrote:
> Yes it was failing for this for us as well.
>
> I kept the same formatting since other formats were defined that way.
> Should I modify all formats with the space in between or keep it this way?
>
>
>
>> -----Original Message-----
>> From: John Smith [mailto:john.v4l2@gmail.com]
>> Sent: Saturday, July 12, 2008 5:12 AM
>> To: video4linux-list@redhat.com; Jalori, Mohit
>> Subject: omap3 camera driver
>>
>> Hi Mohit,
>> I was just looking to your patch and the first observation I could get
>> immediately is, checkpatch.pl script is failing on some of patches. I
>> am not sure, was that missed or intentional? Below is checkpatch
>> output for the one which I have tired -
>>
>>
>>
>> ./scripts/checkpatch.pl
>> scripts/camera_patches/004_v4l2_sgrbg10_format.patch
>> need space after that ',' (ctx:VxV)
>> #19: FILE: include/linux/videodev2.h:312:
>> +#define V4L2_PIX_FMT_SGRBG10 v4l2_fourcc('B','A','1','0') /* 10bit
>> raw bayer  */
>>                                              ^
>>
>> need space after that ',' (ctx:VxV)
>> #19: FILE: include/linux/videodev2.h:312:
>> +#define V4L2_PIX_FMT_SGRBG10 v4l2_fourcc('B','A','1','0') /* 10bit
>> raw bayer  */
>>                                                  ^
>>
>> need space after that ',' (ctx:VxV)
>> #19: FILE: include/linux/videodev2.h:312:
>> +#define V4L2_PIX_FMT_SGRBG10 v4l2_fourcc('B','A','1','0') /* 10bit
>> raw bayer  */
>>                                                      ^
>>
>> Your patch has style problems, please review.  If any of these errors
>> are false positives report them to the maintainer, see
>> CHECKPATCH in MAINTAINERS.
>>
>> #---------------------------------------------------------------------
>> -------------
>>
>> ./scripts/checkpatch.pl
>> scripts/camera_patches/007_v4l2_sgrbg10dpcm8_format.patch
>> line over 80 characters
>> #20: FILE: include/linux/videodev2.h:313:
>> +#define V4L2_PIX_FMT_SGRBG10DPCM8 v4l2_fourcc('B','D','1','0') /*
>> 10bit raw bayer DPCM compressed to 8 bits */
>>
>> need space after that ',' (ctx:VxV)
>> #20: FILE: include/linux/videodev2.h:313:
>> +#define V4L2_PIX_FMT_SGRBG10DPCM8 v4l2_fourcc('B','D','1','0') /*
>> 10bit raw bayer DPCM compressed to 8 bits */
>>                                                   ^
>>
>> need space after that ',' (ctx:VxV)
>> #20: FILE: include/linux/videodev2.h:313:
>> +#define V4L2_PIX_FMT_SGRBG10DPCM8 v4l2_fourcc('B','D','1','0') /*
>> 10bit raw bayer DPCM compressed to 8 bits */
>>                                                       ^
>>
>> need space after that ',' (ctx:VxV)
>> #20: FILE: include/linux/videodev2.h:313:
>> +#define V4L2_PIX_FMT_SGRBG10DPCM8 v4l2_fourcc('B','D','1','0') /*
>> 10bit raw bayer DPCM compressed to 8 bits */
>>                                                           ^
>>
>> Your patch has style problems, please review.  If any of these errors
>> are false positives report them to the maintainer, see
>> CHECKPATCH in MAINTAINERS.
>>
>>
>> #---------------------------------------------------------------------
>> -------------
>>
>> ./scripts/checkpatch.pl
>> scripts/camera_patches/010_omap_34xx_camera_driver_isp_basic_blocks.pa
>> tch
>> struct mutex definition without comment
>> #1762: FILE: drivers/media/video/omap34xxcam.h:97:
>> +       struct mutex mutex;
>>
>> struct mutex definition without comment
>> #1816: FILE: drivers/media/video/omap34xxcam.h:151:
>> +       struct mutex mutex;
>>
>> spinlock_t definition without comment
>> #1846: FILE: drivers/media/video/omap34xxcam.h:181:
>> +       spinlock_t vbq_lock;
>>
>> Use #include <linux/irq.h> instead of <asm/irq.h>
>> #1901: FILE: drivers/media/video/isp/isp.c:33:
>> +#include <asm/irq.h>
>>
>> spinlock_t definition without comment
>> #1979: FILE: drivers/media/video/isp/isp.c:111:
>> +       spinlock_t lock;
>>
>> spinlock_t definition without comment
>> #1980: FILE: drivers/media/video/isp/isp.c:112:
>> +       spinlock_t isp_temp_buf_lock;
>>
>> struct mutex definition without comment
>> #1981: FILE: drivers/media/video/isp/isp.c:113:
>> +       struct mutex isp_mutex;
>>
>> need consistent spacing around '/' (ctx:WxV)
>> #3760: FILE: drivers/media/video/isp/isp.h:87:
>> +#define NUM_ISP_CAPTURE_FORMATS        (sizeof(isp_formats) /\
>>                                                              ^
>>
>> do not add new typedefs
>> #3763: FILE: drivers/media/video/isp/isp.h:90:
>> +typedef int (*isp_vbq_callback_ptr) (struct videobuf_buffer *vb);
>>
>> do not add new typedefs
>> #3764: FILE: drivers/media/video/isp/isp.h:91:
>> +typedef void (*isp_callback_t) (unsigned long status,
>>
>> spinlock_t definition without comment
>> #3836: FILE: drivers/media/video/isp/isp.h:163:
>> +       spinlock_t lock;
>>
>> struct mutex definition without comment
>> #4085: FILE: drivers/media/video/isp/ispccdc.c:83:
>> +       struct mutex mutexlock;
>>
>> Use #include <linux/irq.h> instead of <asm/irq.h>
>> #5544: FILE: drivers/media/video/isp/ispmmu.c:38:
>> +#include <asm/irq.h>
>>
>> #if 0 -- if this code redundant remove it
>> #6397: FILE: drivers/media/video/isp/ispreg.h:26:
>> +#if 0
>>
>> Your patch has style problems, please review.  If any of these errors
>> are false positives report them to the maintainer, see
>> CHECKPATCH in MAINTAINERS.
>>
>> #---------------------------------------------------------------------
>> ----------------------
>>
>>
>> I would like to understand the design actually, will take some time.
>>
>> Thanks,
>> John
>
> --
> video4linux-list mailing list
> Unsubscribe mailto:video4linux-list-request@redhat.com?subject=unsubscribe
> https://www.redhat.com/mailman/listinfo/video4linux-list
>

--
video4linux-list mailing list
Unsubscribe mailto:video4linux-list-request@redhat.com?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list

^ permalink raw reply	[flat|nested] 11+ messages in thread

* RE: omap3 camera driver
  2008-07-16 16:06   ` John
@ 2008-07-18 17:44     ` Jalori, Mohit
  0 siblings, 0 replies; 11+ messages in thread
From: Jalori, Mohit @ 2008-07-18 17:44 UTC (permalink / raw)
  To: John; +Cc: video4linux-list@redhat.com



> -----Original Message-----
> From: John [mailto:john.maximus@gmail.com]
> Sent: Wednesday, July 16, 2008 11:06 AM
> To: Jalori, Mohit
> Cc: video4linux-list@redhat.com
> Subject: Re: omap3 camera driver
>
> Hi Mohit,
> This patch is fails to apply against the labrador kernel 2.6.22.
>
> Kindly provide details of the kernel against which this patch is
> generated and tested.


The patch was generated against 2.6.26-rc6 version of the kernel. We will be posting sensor driver and lens driver that works with this camera driver next week.

>
> Regards,
>
> John
>
>
>
> On Sat, Jul 12, 2008 at 8:40 PM, Jalori, Mohit <mjalori@ti.com> wrote:
> > Yes it was failing for this for us as well.
> >
> > I kept the same formatting since other formats were defined that
> way.
> > Should I modify all formats with the space in between or keep it
> this way?
> >
> >
> >
> >> -----Original Message-----
> >> From: John Smith [mailto:john.v4l2@gmail.com]
> >> Sent: Saturday, July 12, 2008 5:12 AM
> >> To: video4linux-list@redhat.com; Jalori, Mohit
> >> Subject: omap3 camera driver
> >>
> >> Hi Mohit,
> >> I was just looking to your patch and the first observation I could
> get
> >> immediately is, checkpatch.pl script is failing on some of patches.
> I
> >> am not sure, was that missed or intentional? Below is checkpatch
> >> output for the one which I have tired -
> >>
> >>
> >>
> >> ./scripts/checkpatch.pl
> >> scripts/camera_patches/004_v4l2_sgrbg10_format.patch
> >> need space after that ',' (ctx:VxV)
> >> #19: FILE: include/linux/videodev2.h:312:
> >> +#define V4L2_PIX_FMT_SGRBG10 v4l2_fourcc('B','A','1','0') /* 10bit
> >> raw bayer  */
> >>                                              ^
> >>
> >> need space after that ',' (ctx:VxV)
> >> #19: FILE: include/linux/videodev2.h:312:
> >> +#define V4L2_PIX_FMT_SGRBG10 v4l2_fourcc('B','A','1','0') /* 10bit
> >> raw bayer  */
> >>                                                  ^
> >>
> >> need space after that ',' (ctx:VxV)
> >> #19: FILE: include/linux/videodev2.h:312:
> >> +#define V4L2_PIX_FMT_SGRBG10 v4l2_fourcc('B','A','1','0') /* 10bit
> >> raw bayer  */
> >>                                                      ^
> >>
> >> Your patch has style problems, please review.  If any of these
> errors
> >> are false positives report them to the maintainer, see
> >> CHECKPATCH in MAINTAINERS.
> >>
> >> #------------------------------------------------------------------
> ---
> >> -------------
> >>
> >> ./scripts/checkpatch.pl
> >> scripts/camera_patches/007_v4l2_sgrbg10dpcm8_format.patch
> >> line over 80 characters
> >> #20: FILE: include/linux/videodev2.h:313:
> >> +#define V4L2_PIX_FMT_SGRBG10DPCM8 v4l2_fourcc('B','D','1','0') /*
> >> 10bit raw bayer DPCM compressed to 8 bits */
> >>
> >> need space after that ',' (ctx:VxV)
> >> #20: FILE: include/linux/videodev2.h:313:
> >> +#define V4L2_PIX_FMT_SGRBG10DPCM8 v4l2_fourcc('B','D','1','0') /*
> >> 10bit raw bayer DPCM compressed to 8 bits */
> >>                                                   ^
> >>
> >> need space after that ',' (ctx:VxV)
> >> #20: FILE: include/linux/videodev2.h:313:
> >> +#define V4L2_PIX_FMT_SGRBG10DPCM8 v4l2_fourcc('B','D','1','0') /*
> >> 10bit raw bayer DPCM compressed to 8 bits */
> >>                                                       ^
> >>
> >> need space after that ',' (ctx:VxV)
> >> #20: FILE: include/linux/videodev2.h:313:
> >> +#define V4L2_PIX_FMT_SGRBG10DPCM8 v4l2_fourcc('B','D','1','0') /*
> >> 10bit raw bayer DPCM compressed to 8 bits */
> >>                                                           ^
> >>
> >> Your patch has style problems, please review.  If any of these
> errors
> >> are false positives report them to the maintainer, see
> >> CHECKPATCH in MAINTAINERS.
> >>
> >>
> >> #------------------------------------------------------------------
> ---
> >> -------------
> >>
> >> ./scripts/checkpatch.pl
> >>
> scripts/camera_patches/010_omap_34xx_camera_driver_isp_basic_blocks.pa
> >> tch
> >> struct mutex definition without comment
> >> #1762: FILE: drivers/media/video/omap34xxcam.h:97:
> >> +       struct mutex mutex;
> >>
> >> struct mutex definition without comment
> >> #1816: FILE: drivers/media/video/omap34xxcam.h:151:
> >> +       struct mutex mutex;
> >>
> >> spinlock_t definition without comment
> >> #1846: FILE: drivers/media/video/omap34xxcam.h:181:
> >> +       spinlock_t vbq_lock;
> >>
> >> Use #include <linux/irq.h> instead of <asm/irq.h>
> >> #1901: FILE: drivers/media/video/isp/isp.c:33:
> >> +#include <asm/irq.h>
> >>
> >> spinlock_t definition without comment
> >> #1979: FILE: drivers/media/video/isp/isp.c:111:
> >> +       spinlock_t lock;
> >>
> >> spinlock_t definition without comment
> >> #1980: FILE: drivers/media/video/isp/isp.c:112:
> >> +       spinlock_t isp_temp_buf_lock;
> >>
> >> struct mutex definition without comment
> >> #1981: FILE: drivers/media/video/isp/isp.c:113:
> >> +       struct mutex isp_mutex;
> >>
> >> need consistent spacing around '/' (ctx:WxV)
> >> #3760: FILE: drivers/media/video/isp/isp.h:87:
> >> +#define NUM_ISP_CAPTURE_FORMATS        (sizeof(isp_formats) /\
> >>                                                              ^
> >>
> >> do not add new typedefs
> >> #3763: FILE: drivers/media/video/isp/isp.h:90:
> >> +typedef int (*isp_vbq_callback_ptr) (struct videobuf_buffer *vb);
> >>
> >> do not add new typedefs
> >> #3764: FILE: drivers/media/video/isp/isp.h:91:
> >> +typedef void (*isp_callback_t) (unsigned long status,
> >>
> >> spinlock_t definition without comment
> >> #3836: FILE: drivers/media/video/isp/isp.h:163:
> >> +       spinlock_t lock;
> >>
> >> struct mutex definition without comment
> >> #4085: FILE: drivers/media/video/isp/ispccdc.c:83:
> >> +       struct mutex mutexlock;
> >>
> >> Use #include <linux/irq.h> instead of <asm/irq.h>
> >> #5544: FILE: drivers/media/video/isp/ispmmu.c:38:
> >> +#include <asm/irq.h>
> >>
> >> #if 0 -- if this code redundant remove it
> >> #6397: FILE: drivers/media/video/isp/ispreg.h:26:
> >> +#if 0
> >>
> >> Your patch has style problems, please review.  If any of these
> errors
> >> are false positives report them to the maintainer, see
> >> CHECKPATCH in MAINTAINERS.
> >>
> >> #------------------------------------------------------------------
> ---
> >> ----------------------
> >>
> >>
> >> I would like to understand the design actually, will take some
> time.
> >>
> >> Thanks,
> >> John
> >
> > --
> > video4linux-list mailing list
> > Unsubscribe mailto:video4linux-list-
> request@redhat.com?subject=unsubscribe
> > https://www.redhat.com/mailman/listinfo/video4linux-list
> >

--
video4linux-list mailing list
Unsubscribe mailto:video4linux-list-request@redhat.com?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2008-07-18 17:45 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-06-26 22:53 omap3 camera driver Jalori, Mohit
2008-06-27 10:16 ` Trilok Soni
2008-06-27 21:57   ` Daniel Glöckner
2008-06-27 22:08     ` hermann pitton
2008-06-28 16:27       ` Jalori, Mohit
2008-06-30  8:12         ` Paulius Zaleckas
2008-07-01  3:36   ` Jalori, Mohit
  -- strict thread matches above, loose matches on Subject: below --
2008-07-12 10:11 John Smith
2008-07-12 15:10 ` Jalori, Mohit
2008-07-16 16:06   ` John
2008-07-18 17:44     ` Jalori, Mohit

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox