* [PATCH v5 1/5] ARM: OMAP3: Update Camera ISP definitions for OMAP3630
2011-01-27 12:32 [PATCH v5 0/5] OMAP3 ISP driver Laurent Pinchart
@ 2011-01-27 12:32 ` Laurent Pinchart
2011-01-27 12:32 ` [PATCH v5 2/5] omap3: Remove unusued ISP CBUFF resource Laurent Pinchart
` (3 subsequent siblings)
4 siblings, 0 replies; 9+ messages in thread
From: Laurent Pinchart @ 2011-01-27 12:32 UTC (permalink / raw)
To: linux-media, linux-omap; +Cc: sakari.ailus
From: Tuukka Toivonen <tuukka.o.toivonen@nokia.com>
Add new/changed base address definitions and resources for
OMAP3630 ISP.
The OMAP3430 CSI2PHY block is same as the OMAP3630 CSIPHY2
block. But the later name is chosen as it gives more symmetry
to the names.
Signed-off-by: Tuukka Toivonen <tuukka.o.toivonen@nokia.com>
Signed-off-by: Vimarsh Zutshi <vimarsh.zutshi@nokia.com>
Acked-by: Tony Lindgren <tony@atomide.com>
---
arch/arm/mach-omap2/devices.c | 28 ++++++++++++++++++++++++----
arch/arm/plat-omap/include/plat/omap34xx.h | 16 ++++++++++++----
2 files changed, 36 insertions(+), 8 deletions(-)
diff --git a/arch/arm/mach-omap2/devices.c b/arch/arm/mach-omap2/devices.c
index 5a0c148..d5da345 100644
--- a/arch/arm/mach-omap2/devices.c
+++ b/arch/arm/mach-omap2/devices.c
@@ -109,13 +109,33 @@ static struct resource omap3isp_resources[] = {
.flags = IORESOURCE_MEM,
},
{
- .start = OMAP3430_ISP_CSI2A_BASE,
- .end = OMAP3430_ISP_CSI2A_END,
+ .start = OMAP3430_ISP_CSI2A_REGS1_BASE,
+ .end = OMAP3430_ISP_CSI2A_REGS1_END,
.flags = IORESOURCE_MEM,
},
{
- .start = OMAP3430_ISP_CSI2PHY_BASE,
- .end = OMAP3430_ISP_CSI2PHY_END,
+ .start = OMAP3430_ISP_CSIPHY2_BASE,
+ .end = OMAP3430_ISP_CSIPHY2_END,
+ .flags = IORESOURCE_MEM,
+ },
+ {
+ .start = OMAP3630_ISP_CSI2A_REGS2_BASE,
+ .end = OMAP3630_ISP_CSI2A_REGS2_END,
+ .flags = IORESOURCE_MEM,
+ },
+ {
+ .start = OMAP3630_ISP_CSI2C_REGS1_BASE,
+ .end = OMAP3630_ISP_CSI2C_REGS1_END,
+ .flags = IORESOURCE_MEM,
+ },
+ {
+ .start = OMAP3630_ISP_CSIPHY1_BASE,
+ .end = OMAP3630_ISP_CSIPHY1_END,
+ .flags = IORESOURCE_MEM,
+ },
+ {
+ .start = OMAP3630_ISP_CSI2C_REGS2_BASE,
+ .end = OMAP3630_ISP_CSI2C_REGS2_END,
.flags = IORESOURCE_MEM,
},
{
diff --git a/arch/arm/plat-omap/include/plat/omap34xx.h b/arch/arm/plat-omap/include/plat/omap34xx.h
index 98fc8b4..b9e8588 100644
--- a/arch/arm/plat-omap/include/plat/omap34xx.h
+++ b/arch/arm/plat-omap/include/plat/omap34xx.h
@@ -56,8 +56,12 @@
#define OMAP3430_ISP_RESZ_BASE (OMAP3430_ISP_BASE + 0x1000)
#define OMAP3430_ISP_SBL_BASE (OMAP3430_ISP_BASE + 0x1200)
#define OMAP3430_ISP_MMU_BASE (OMAP3430_ISP_BASE + 0x1400)
-#define OMAP3430_ISP_CSI2A_BASE (OMAP3430_ISP_BASE + 0x1800)
-#define OMAP3430_ISP_CSI2PHY_BASE (OMAP3430_ISP_BASE + 0x1970)
+#define OMAP3430_ISP_CSI2A_REGS1_BASE (OMAP3430_ISP_BASE + 0x1800)
+#define OMAP3430_ISP_CSIPHY2_BASE (OMAP3430_ISP_BASE + 0x1970)
+#define OMAP3630_ISP_CSI2A_REGS2_BASE (OMAP3430_ISP_BASE + 0x19C0)
+#define OMAP3630_ISP_CSI2C_REGS1_BASE (OMAP3430_ISP_BASE + 0x1C00)
+#define OMAP3630_ISP_CSIPHY1_BASE (OMAP3430_ISP_BASE + 0x1D70)
+#define OMAP3630_ISP_CSI2C_REGS2_BASE (OMAP3430_ISP_BASE + 0x1DC0)
#define OMAP3430_ISP_END (OMAP3430_ISP_BASE + 0x06F)
#define OMAP3430_ISP_CBUFF_END (OMAP3430_ISP_CBUFF_BASE + 0x077)
@@ -69,8 +73,12 @@
#define OMAP3430_ISP_RESZ_END (OMAP3430_ISP_RESZ_BASE + 0x0AB)
#define OMAP3430_ISP_SBL_END (OMAP3430_ISP_SBL_BASE + 0x0FB)
#define OMAP3430_ISP_MMU_END (OMAP3430_ISP_MMU_BASE + 0x06F)
-#define OMAP3430_ISP_CSI2A_END (OMAP3430_ISP_CSI2A_BASE + 0x16F)
-#define OMAP3430_ISP_CSI2PHY_END (OMAP3430_ISP_CSI2PHY_BASE + 0x007)
+#define OMAP3430_ISP_CSI2A_REGS1_END (OMAP3430_ISP_CSI2A_REGS1_BASE + 0x16F)
+#define OMAP3430_ISP_CSIPHY2_END (OMAP3430_ISP_CSIPHY2_BASE + 0x00B)
+#define OMAP3630_ISP_CSI2A_REGS2_END (OMAP3630_ISP_CSI2A_REGS2_BASE + 0x3F)
+#define OMAP3630_ISP_CSI2C_REGS1_END (OMAP3630_ISP_CSI2C_REGS1_BASE + 0x16F)
+#define OMAP3630_ISP_CSIPHY1_END (OMAP3630_ISP_CSIPHY1_BASE + 0x00B)
+#define OMAP3630_ISP_CSI2C_REGS2_END (OMAP3630_ISP_CSI2C_REGS2_BASE + 0x3F)
#define OMAP34XX_HSUSB_OTG_BASE (L4_34XX_BASE + 0xAB000)
#define OMAP34XX_USBTLL_BASE (L4_34XX_BASE + 0x62000)
--
1.7.3.4
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH v5 2/5] omap3: Remove unusued ISP CBUFF resource
2011-01-27 12:32 [PATCH v5 0/5] OMAP3 ISP driver Laurent Pinchart
2011-01-27 12:32 ` [PATCH v5 1/5] ARM: OMAP3: Update Camera ISP definitions for OMAP3630 Laurent Pinchart
@ 2011-01-27 12:32 ` Laurent Pinchart
2011-02-01 23:36 ` Tony Lindgren
2011-01-27 12:32 ` [PATCH v5 3/5] omap3: Add function to register omap3isp platform device structure Laurent Pinchart
` (2 subsequent siblings)
4 siblings, 1 reply; 9+ messages in thread
From: Laurent Pinchart @ 2011-01-27 12:32 UTC (permalink / raw)
To: linux-media, linux-omap; +Cc: sakari.ailus
From: Sergio Aguirre <saaguirre@ti.com>
The ISP CBUFF module isn't use, its resource isn't needed.
Signed-off-by: Sergio Aguirre <saaguirre@ti.com>
Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
arch/arm/mach-omap2/devices.c | 5 -----
1 files changed, 0 insertions(+), 5 deletions(-)
diff --git a/arch/arm/mach-omap2/devices.c b/arch/arm/mach-omap2/devices.c
index d5da345..f16268d 100644
--- a/arch/arm/mach-omap2/devices.c
+++ b/arch/arm/mach-omap2/devices.c
@@ -69,11 +69,6 @@ static struct resource omap3isp_resources[] = {
.flags = IORESOURCE_MEM,
},
{
- .start = OMAP3430_ISP_CBUFF_BASE,
- .end = OMAP3430_ISP_CBUFF_END,
- .flags = IORESOURCE_MEM,
- },
- {
.start = OMAP3430_ISP_CCP2_BASE,
.end = OMAP3430_ISP_CCP2_END,
.flags = IORESOURCE_MEM,
--
1.7.3.4
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH v5 2/5] omap3: Remove unusued ISP CBUFF resource
2011-01-27 12:32 ` [PATCH v5 2/5] omap3: Remove unusued ISP CBUFF resource Laurent Pinchart
@ 2011-02-01 23:36 ` Tony Lindgren
0 siblings, 0 replies; 9+ messages in thread
From: Tony Lindgren @ 2011-02-01 23:36 UTC (permalink / raw)
To: Laurent Pinchart; +Cc: linux-media, linux-omap, sakari.ailus
* Laurent Pinchart <laurent.pinchart@ideasonboard.com> [110127 04:38]:
> From: Sergio Aguirre <saaguirre@ti.com>
>
> The ISP CBUFF module isn't use, its resource isn't needed.
>
> Signed-off-by: Sergio Aguirre <saaguirre@ti.com>
> Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Assuming you want to queue all of these via linux-media:
Acked-by: Tony Lindgren <tony@atomide.com>
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v5 3/5] omap3: Add function to register omap3isp platform device structure
2011-01-27 12:32 [PATCH v5 0/5] OMAP3 ISP driver Laurent Pinchart
2011-01-27 12:32 ` [PATCH v5 1/5] ARM: OMAP3: Update Camera ISP definitions for OMAP3630 Laurent Pinchart
2011-01-27 12:32 ` [PATCH v5 2/5] omap3: Remove unusued ISP CBUFF resource Laurent Pinchart
@ 2011-01-27 12:32 ` Laurent Pinchart
2011-01-27 12:32 ` [PATCH v5 4/5] omap2: Fix camera resources for multiomap Laurent Pinchart
2011-02-04 11:55 ` [PATCH v5 0/5] OMAP3 ISP driver Hans Verkuil
4 siblings, 0 replies; 9+ messages in thread
From: Laurent Pinchart @ 2011-01-27 12:32 UTC (permalink / raw)
To: linux-media, linux-omap; +Cc: sakari.ailus
The omap3isp platform device requires platform data. Instead of
registering the device in omap2_init_devices(), export an
omap3_init_camera() function to fill the device structure with the
platform data pointer and register the device.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Acked-by: Tony Lindgren <tony@atomide.com>
---
arch/arm/mach-omap2/devices.c | 20 +++++++++++---------
arch/arm/mach-omap2/devices.h | 17 +++++++++++++++++
2 files changed, 28 insertions(+), 9 deletions(-)
create mode 100644 arch/arm/mach-omap2/devices.h
diff --git a/arch/arm/mach-omap2/devices.c b/arch/arm/mach-omap2/devices.c
index f16268d..cc400c7 100644
--- a/arch/arm/mach-omap2/devices.c
+++ b/arch/arm/mach-omap2/devices.c
@@ -34,6 +34,8 @@
#include "mux.h"
#include "control.h"
+#include "devices.h"
+
#if defined(CONFIG_VIDEO_OMAP2) || defined(CONFIG_VIDEO_OMAP2_MODULE)
static struct resource cam_resources[] = {
@@ -59,8 +61,11 @@ static inline void omap_init_camera(void)
{
platform_device_register(&omap_cam_device);
}
-
-#elif defined(CONFIG_VIDEO_OMAP3) || defined(CONFIG_VIDEO_OMAP3_MODULE)
+#else
+static inline void omap_init_camera(void)
+{
+}
+#endif
static struct resource omap3isp_resources[] = {
{
@@ -146,15 +151,12 @@ static struct platform_device omap3isp_device = {
.resource = omap3isp_resources,
};
-static inline void omap_init_camera(void)
-{
- platform_device_register(&omap3isp_device);
-}
-#else
-static inline void omap_init_camera(void)
+int omap3_init_camera(void *pdata)
{
+ omap3isp_device.dev.platform_data = pdata;
+ return platform_device_register(&omap3isp_device);
}
-#endif
+EXPORT_SYMBOL_GPL(omap3_init_camera);
#if defined(CONFIG_OMAP_MBOX_FWK) || defined(CONFIG_OMAP_MBOX_FWK_MODULE)
diff --git a/arch/arm/mach-omap2/devices.h b/arch/arm/mach-omap2/devices.h
new file mode 100644
index 0000000..12ddb8a
--- /dev/null
+++ b/arch/arm/mach-omap2/devices.h
@@ -0,0 +1,17 @@
+/*
+ * arch/arm/mach-omap2/devices.h
+ *
+ * OMAP2 platform device setup/initialization
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+
+#ifndef __ARCH_ARM_MACH_OMAP_DEVICES_H
+#define __ARCH_ARM_MACH_OMAP_DEVICES_H
+
+int omap3_init_camera(void *pdata);
+
+#endif
--
1.7.3.4
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH v5 4/5] omap2: Fix camera resources for multiomap
2011-01-27 12:32 [PATCH v5 0/5] OMAP3 ISP driver Laurent Pinchart
` (2 preceding siblings ...)
2011-01-27 12:32 ` [PATCH v5 3/5] omap3: Add function to register omap3isp platform device structure Laurent Pinchart
@ 2011-01-27 12:32 ` Laurent Pinchart
2011-02-01 23:37 ` Tony Lindgren
2011-02-04 11:55 ` [PATCH v5 0/5] OMAP3 ISP driver Hans Verkuil
4 siblings, 1 reply; 9+ messages in thread
From: Laurent Pinchart @ 2011-01-27 12:32 UTC (permalink / raw)
To: linux-media, linux-omap; +Cc: sakari.ailus
From: Sergio Aguirre <saaguirre@ti.com>
Make sure the kernel can be compiled with both OMAP2 and OMAP3 camera
support linked in, and give public symbols proper omap2/omap3 prefixes.
Signed-off-by: Sergio Aguirre <saaguirre@ti.com>
Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
arch/arm/mach-omap2/devices.c | 25 ++++++++++++-------------
1 files changed, 12 insertions(+), 13 deletions(-)
diff --git a/arch/arm/mach-omap2/devices.c b/arch/arm/mach-omap2/devices.c
index cc400c7..e799303 100644
--- a/arch/arm/mach-omap2/devices.c
+++ b/arch/arm/mach-omap2/devices.c
@@ -38,7 +38,7 @@
#if defined(CONFIG_VIDEO_OMAP2) || defined(CONFIG_VIDEO_OMAP2_MODULE)
-static struct resource cam_resources[] = {
+static struct resource omap2cam_resources[] = {
{
.start = OMAP24XX_CAMERA_BASE,
.end = OMAP24XX_CAMERA_BASE + 0xfff,
@@ -50,21 +50,12 @@ static struct resource cam_resources[] = {
}
};
-static struct platform_device omap_cam_device = {
+static struct platform_device omap2cam_device = {
.name = "omap24xxcam",
.id = -1,
- .num_resources = ARRAY_SIZE(cam_resources),
- .resource = cam_resources,
+ .num_resources = ARRAY_SIZE(omap2cam_resources),
+ .resource = omap2cam_resources,
};
-
-static inline void omap_init_camera(void)
-{
- platform_device_register(&omap_cam_device);
-}
-#else
-static inline void omap_init_camera(void)
-{
-}
#endif
static struct resource omap3isp_resources[] = {
@@ -158,6 +149,14 @@ int omap3_init_camera(void *pdata)
}
EXPORT_SYMBOL_GPL(omap3_init_camera);
+static inline void omap_init_camera(void)
+{
+#if defined(CONFIG_VIDEO_OMAP2) || defined(CONFIG_VIDEO_OMAP2_MODULE)
+ if (cpu_is_omap24xx())
+ platform_device_register(&omap2cam_device);
+#endif
+}
+
#if defined(CONFIG_OMAP_MBOX_FWK) || defined(CONFIG_OMAP_MBOX_FWK_MODULE)
#define MBOX_REG_SIZE 0x120
--
1.7.3.4
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH v5 4/5] omap2: Fix camera resources for multiomap
2011-01-27 12:32 ` [PATCH v5 4/5] omap2: Fix camera resources for multiomap Laurent Pinchart
@ 2011-02-01 23:37 ` Tony Lindgren
0 siblings, 0 replies; 9+ messages in thread
From: Tony Lindgren @ 2011-02-01 23:37 UTC (permalink / raw)
To: Laurent Pinchart; +Cc: linux-media, linux-omap, sakari.ailus
* Laurent Pinchart <laurent.pinchart@ideasonboard.com> [110127 04:38]:
> From: Sergio Aguirre <saaguirre@ti.com>
>
> Make sure the kernel can be compiled with both OMAP2 and OMAP3 camera
> support linked in, and give public symbols proper omap2/omap3 prefixes.
>
> Signed-off-by: Sergio Aguirre <saaguirre@ti.com>
> Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Acked-by: Tony Lindgren <tony@atomide.com>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v5 0/5] OMAP3 ISP driver
2011-01-27 12:32 [PATCH v5 0/5] OMAP3 ISP driver Laurent Pinchart
` (3 preceding siblings ...)
2011-01-27 12:32 ` [PATCH v5 4/5] omap2: Fix camera resources for multiomap Laurent Pinchart
@ 2011-02-04 11:55 ` Hans Verkuil
2011-02-08 12:15 ` Laurent Pinchart
4 siblings, 1 reply; 9+ messages in thread
From: Hans Verkuil @ 2011-02-04 11:55 UTC (permalink / raw)
To: Laurent Pinchart; +Cc: linux-media, linux-omap, sakari.ailus
On Thursday, January 27, 2011 13:32:16 Laurent Pinchart wrote:
> Hi everybody,
>
> Here's the fifth version of the OMAP3 ISP driver patches, updated to
> 2.6.37 and the latest changes in the media controller and sub-device APIs.
Hmm, patch 5/5 is missing. It's probably too big.
Anyway, I got the patch from your git tree and did a review. It's always hard
to review over 21000 lines of driver code :-), so I limited myself to the V4L2
API parts. I can't really comment on the OMAP3 specific parts anyway.
The first issue I found was related to controls: it seems you set up control
handlers for subdevs that don't have any controls. You can just leave
sd->ctrl_handler to NULL in that case and you don't need to use a control handler
at all.
There is also no need to set the core ctrl ops:
+ .queryctrl = v4l2_subdev_queryctrl,
+ .querymenu = v4l2_subdev_querymenu,
+ .g_ctrl = v4l2_subdev_g_ctrl,
+ .s_ctrl = v4l2_subdev_s_ctrl,
+ .g_ext_ctrls = v4l2_subdev_g_ext_ctrls,
+ .try_ext_ctrls = v4l2_subdev_try_ext_ctrls,
+ .s_ext_ctrls = v4l2_subdev_s_ext_ctrls,
These are only necessary if the master driver doesn't use the control
framework but called core.queryctrl directly. That shouldn't be the case
for this driver.
What isn't clear to me is whether the /dev/videoX nodes should give access
to the subdev controls as well. As far as I can see the ctrl_handler pointer
of neither v4l2_device nor video_device is ever set, so that means that the
controls are only accessible through /dev/v4l-subdevX.
I'm not sure whether that is intentional or not.
The other comment I have is regarding include/linux/omap3isp.h: both the
ioctls and the events need to be documented there. A one-liner for each is
probably enough. I also see that struct omap3isp_stat_data has a deprecated
field: perhaps when creating the final pull request the time is right to
remove it?
Finally, I noticed that OMAP3 has its own implementation of videobuf. Are
there plans to move to vb2?
Regards,
Hans
>
> You can find the patches in http://git.linuxtv.org/pinchartl/media.git as
> usual (media-0005-omap3isp).
>
> Laurent Pinchart (2):
> omap3: Add function to register omap3isp platform device structure
> OMAP3 ISP driver
>
> Sergio Aguirre (2):
> omap3: Remove unusued ISP CBUFF resource
> omap2: Fix camera resources for multiomap
>
> Tuukka Toivonen (1):
> ARM: OMAP3: Update Camera ISP definitions for OMAP3630
>
> arch/arm/mach-omap2/devices.c | 64 +-
> arch/arm/mach-omap2/devices.h | 17 +
> arch/arm/plat-omap/include/plat/omap34xx.h | 16 +-
> drivers/media/video/Kconfig | 13 +
> drivers/media/video/Makefile | 2 +
> drivers/media/video/isp/Makefile | 13 +
> drivers/media/video/isp/cfa_coef_table.h | 601 +++++++
> drivers/media/video/isp/gamma_table.h | 90 +
> drivers/media/video/isp/isp.c | 2221 +++++++++++++++++++++++++
> drivers/media/video/isp/isp.h | 427 +++++
> drivers/media/video/isp/ispccdc.c | 2280 ++++++++++++++++++++++++++
> drivers/media/video/isp/ispccdc.h | 223 +++
> drivers/media/video/isp/ispccp2.c | 1189 ++++++++++++++
> drivers/media/video/isp/ispccp2.h | 101 ++
> drivers/media/video/isp/ispcsi2.c | 1332 +++++++++++++++
> drivers/media/video/isp/ispcsi2.h | 169 ++
> drivers/media/video/isp/ispcsiphy.c | 247 +++
> drivers/media/video/isp/ispcsiphy.h | 74 +
> drivers/media/video/isp/isph3a.h | 117 ++
> drivers/media/video/isp/isph3a_aewb.c | 374 +++++
> drivers/media/video/isp/isph3a_af.c | 429 +++++
> drivers/media/video/isp/isphist.c | 520 ++++++
> drivers/media/video/isp/isphist.h | 40 +
> drivers/media/video/isp/isppreview.c | 2120 ++++++++++++++++++++++++
> drivers/media/video/isp/isppreview.h | 214 +++
> drivers/media/video/isp/ispqueue.c | 1136 +++++++++++++
> drivers/media/video/isp/ispqueue.h | 185 +++
> drivers/media/video/isp/ispreg.h | 1589 ++++++++++++++++++
> drivers/media/video/isp/ispresizer.c | 1710 +++++++++++++++++++
> drivers/media/video/isp/ispresizer.h | 150 ++
> drivers/media/video/isp/ispstat.c | 1100 +++++++++++++
> drivers/media/video/isp/ispstat.h | 169 ++
> drivers/media/video/isp/ispvideo.c | 1264 ++++++++++++++
> drivers/media/video/isp/ispvideo.h | 202 +++
> drivers/media/video/isp/luma_enhance_table.h | 154 ++
> drivers/media/video/isp/noise_filter_table.h | 90 +
> include/linux/Kbuild | 1 +
> include/linux/omap3isp.h | 631 +++++++
> 38 files changed, 21246 insertions(+), 28 deletions(-)
> create mode 100644 arch/arm/mach-omap2/devices.h
> create mode 100644 drivers/media/video/isp/Makefile
> create mode 100644 drivers/media/video/isp/cfa_coef_table.h
> create mode 100644 drivers/media/video/isp/gamma_table.h
> create mode 100644 drivers/media/video/isp/isp.c
> create mode 100644 drivers/media/video/isp/isp.h
> create mode 100644 drivers/media/video/isp/ispccdc.c
> create mode 100644 drivers/media/video/isp/ispccdc.h
> create mode 100644 drivers/media/video/isp/ispccp2.c
> create mode 100644 drivers/media/video/isp/ispccp2.h
> create mode 100644 drivers/media/video/isp/ispcsi2.c
> create mode 100644 drivers/media/video/isp/ispcsi2.h
> create mode 100644 drivers/media/video/isp/ispcsiphy.c
> create mode 100644 drivers/media/video/isp/ispcsiphy.h
> create mode 100644 drivers/media/video/isp/isph3a.h
> create mode 100644 drivers/media/video/isp/isph3a_aewb.c
> create mode 100644 drivers/media/video/isp/isph3a_af.c
> create mode 100644 drivers/media/video/isp/isphist.c
> create mode 100644 drivers/media/video/isp/isphist.h
> 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/ispqueue.c
> create mode 100644 drivers/media/video/isp/ispqueue.h
> create mode 100644 drivers/media/video/isp/ispreg.h
> create mode 100644 drivers/media/video/isp/ispresizer.c
> create mode 100644 drivers/media/video/isp/ispresizer.h
> create mode 100644 drivers/media/video/isp/ispstat.c
> create mode 100644 drivers/media/video/isp/ispstat.h
> create mode 100644 drivers/media/video/isp/ispvideo.c
> create mode 100644 drivers/media/video/isp/ispvideo.h
> create mode 100644 drivers/media/video/isp/luma_enhance_table.h
> create mode 100644 drivers/media/video/isp/noise_filter_table.h
> create mode 100644 include/linux/omap3isp.h
>
>
--
Hans Verkuil - video4linux developer - sponsored by Cisco
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH v5 0/5] OMAP3 ISP driver
2011-02-04 11:55 ` [PATCH v5 0/5] OMAP3 ISP driver Hans Verkuil
@ 2011-02-08 12:15 ` Laurent Pinchart
0 siblings, 0 replies; 9+ messages in thread
From: Laurent Pinchart @ 2011-02-08 12:15 UTC (permalink / raw)
To: Hans Verkuil; +Cc: linux-media, linux-omap, sakari.ailus
Hi Hans,
Thanks for the review.
On Friday 04 February 2011 12:55:50 Hans Verkuil wrote:
> On Thursday, January 27, 2011 13:32:16 Laurent Pinchart wrote:
> > Hi everybody,
> >
> > Here's the fifth version of the OMAP3 ISP driver patches, updated to
> > 2.6.37 and the latest changes in the media controller and sub-device
> > APIs.
>
> Hmm, patch 5/5 is missing. It's probably too big.
Yes it is. I forgot to mention that in the cover letter.
> Anyway, I got the patch from your git tree and did a review. It's always
> hard to review over 21000 lines of driver code :-), so I limited myself to
> the V4L2 API parts. I can't really comment on the OMAP3 specific parts
> anyway.
>
> The first issue I found was related to controls: it seems you set up
> control handlers for subdevs that don't have any controls. You can just
> leave sd->ctrl_handler to NULL in that case and you don't need to use a
> control handler at all.
OK. It was a leftover.
> There is also no need to set the core ctrl ops:
>
> + .queryctrl = v4l2_subdev_queryctrl,
> + .querymenu = v4l2_subdev_querymenu,
> + .g_ctrl = v4l2_subdev_g_ctrl,
> + .s_ctrl = v4l2_subdev_s_ctrl,
> + .g_ext_ctrls = v4l2_subdev_g_ext_ctrls,
> + .try_ext_ctrls = v4l2_subdev_try_ext_ctrls,
> + .s_ext_ctrls = v4l2_subdev_s_ext_ctrls,
>
> These are only necessary if the master driver doesn't use the control
> framework but called core.queryctrl directly. That shouldn't be the case
> for this driver.
OK.
> What isn't clear to me is whether the /dev/videoX nodes should give access
> to the subdev controls as well. As far as I can see the ctrl_handler
> pointer of neither v4l2_device nor video_device is ever set, so that means
> that the controls are only accessible through /dev/v4l-subdevX.
>
> I'm not sure whether that is intentional or not.
It's intentional.
> The other comment I have is regarding include/linux/omap3isp.h: both the
> ioctls and the events need to be documented there. A one-liner for each is
> probably enough. I also see that struct omap3isp_stat_data has a deprecated
> field: perhaps when creating the final pull request the time is right to
> remove it?
OK.
> Finally, I noticed that OMAP3 has its own implementation of videobuf. Are
> there plans to move to vb2?
Yes, but not before 2.6.39 :-)
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 9+ messages in thread