Skip to contentSkip to conversation

mx2_camera: Add soc_camera support for i.MX25/i.MX27

PATCH Superseded Baruch Siach 3 participants 5 messages Jul 1, 2010

Subsystems (4)
Arm (32-bit) Media (V4L/DVB) Arm sub-architectures Arm i.MX and Layerscape

v5 of 6 Superseded Latest is v6 →

lore
Share & tools
lore

Baruch Siach

author
#0 lore raw

PATCH v5 1/1 mx2_camera: Add soc_camera support for i.MX25/i.MX27

This is the soc_camera support developed by Sascha Hauer for the i.MX27. Alan Carvalho de Assis modified the original driver to get it working on more recent kernels. I modified it further to add support for i.MX25. This driver has been tested on i.MX25 and i.MX27 based platforms.

Signed-off-by Baruch Siach <[email protected]>
Acked-by Guennadi Liakhovetski <[email protected]>

---
Changes v4 -> v5
Comments from Uwe Kleine-K??nig:

Enclose mx27 DMA related stuff in #ifdefs since the dma-mx1-mx2.h is no longer accessible to mx25 builds

s/MX2_VIDEO/VIDEO_MX2_HOSTSUPPORT/

Changes v3 -> v4
Address more comments from Guennadi Liakhovetski, including:

* Fix the double trigger handling of mx27 eMMA

* Add a FIXME comment in the code of eMMA overflow handling

Changes v2 -> v3
Address more comments from Guennadi Liakhovetski.

Applied part of Sashca's patch that I forgot in v2.

Changes v1 -> v2
Addressed the comments of Guennadi Liakhovetski except from the following:

1. The mclk_get_divisor implementation, since I don't know what this code is good for

2. mx2_videobuf_release should not set pcdev->active on i.MX27, because mx27_camera_frame_done needs this pointer

3. In mx27_camera_emma_buf_init I don't know the meaning of those hard coded magic numbers

Applied i.MX27 fixes from Sascha.

 arch/arm/plat-mxc/include/mach/memory.h  |    4 +-
 arch/arm/plat-mxc/include/mach/mx2_cam.h |   46 +
 drivers/media/video/Kconfig              |   13 +
 drivers/media/video/Makefile             |    1 +
 drivers/media/video/mx2_camera.c         | 1513 ++++++++++++++++++++++++++++++
 5 files changed, 1575 insertions(+), 2 deletions(-)
 create mode 100644 arch/arm/plat-mxc/include/mach/mx2_cam.h
 create mode 100644 drivers/media/video/mx2_camera.c
arch/arm/plat-mxc/include/mach/memory.h +2 -2
index c4b40c3..849d891 100644
@@ -44,12 +44,12 @@
  */
 #define CONSISTENT_DMA_SIZE SZ_8M
 
-#elif defined(CONFIG_MX1_VIDEO)
+#elif defined(CONFIG_MX1_VIDEO) || defined(CONFIG_VIDEO_MX2_HOSTSUPPORT)
 /*
  * Increase size of DMA-consistent memory region.
  * This is required for i.MX camera driver to capture at least four VGA frames.
  */
 #define CONSISTENT_DMA_SIZE SZ_4M
-#endif /* CONFIG_MX1_VIDEO */
+#endif /* CONFIG_MX1_VIDEO || CONFIG_MX2_VIDEO */
 
 #endif /* __ASM_ARCH_MXC_MEMORY_H__ */
arch/arm/plat-mxc/include/mach/mx2_cam.h new file +46 -0

46 lines not on this page · raw message

drivers/media/video/Kconfig +13 -0
index bdbc9d3..27e2acc 100644
@@ -969,6 +969,19 @@ config VIDEO_OMAP2
 	---help---
 	  This is a v4l2 driver for the TI OMAP2 camera capture interface
 
+config VIDEO_MX2_HOSTSUPPORT
+        bool
+
+config VIDEO_MX2
+	tristate "i.MX27/i.MX25 Camera Sensor Interface driver"
+	depends on VIDEO_DEV && SOC_CAMERA && (MACH_MX27 || ARCH_MX25)
+	select VIDEOBUF_DMA_CONTIG
+	select VIDEO_MX2_HOSTSUPPORT
+	---help---
+	  This is a v4l2 driver for the i.MX27 and the i.MX25 Camera Sensor
+	  Interface
+
+
 #
 # USB Multimedia device configuration
 #
drivers/media/video/Makefile +1 -0
index cc93859..b08bd2b 100644
@@ -162,6 +162,7 @@ obj-$(CONFIG_SOC_CAMERA)		+= soc_camera.o soc_mediabus.o
 obj-$(CONFIG_SOC_CAMERA_PLATFORM)	+= soc_camera_platform.o
 # soc-camera host drivers have to be linked after camera drivers
 obj-$(CONFIG_VIDEO_MX1)			+= mx1_camera.o
+obj-$(CONFIG_VIDEO_MX2)			+= mx2_camera.o
 obj-$(CONFIG_VIDEO_MX3)			+= mx3_camera.o
 obj-$(CONFIG_VIDEO_PXA27x)		+= pxa_camera.o
 obj-$(CONFIG_VIDEO_SH_MOBILE_CEU)	+= sh_mobile_ceu_camera.o
drivers/media/video/mx2_camera.c new file +1513 -0

1,513 lines not on this page · raw message

Signature · 1 line
1.7.1
#1 lore raw

PATCH v5 1/1 mx2_camera: Add soc_camera support for i.MX25/i.MX27

Quoted 58 lines · Baruch Siach

This is the soc_camera support developed by Sascha Hauer for the i.MX27. Alan Carvalho de Assis modified the original driver to get it working on more recent kernels. I modified it further to add support for i.MX25. This driver has been tested on i.MX25 and i.MX27 based platforms.

Signed-off-by Baruch Siach <[email protected]>
Acked-by Guennadi Liakhovetski <[email protected]>

---
Changes v4 -> v5
Comments from Uwe Kleine-K?nig:

Enclose mx27 DMA related stuff in #ifdefs since the dma-mx1-mx2.h is no longer accessible to mx25 builds

s/MX2_VIDEO/VIDEO_MX2_HOSTSUPPORT/

Changes v3 -> v4
Address more comments from Guennadi Liakhovetski, including:

* Fix the double trigger handling of mx27 eMMA

* Add a FIXME comment in the code of eMMA overflow handling

Changes v2 -> v3
Address more comments from Guennadi Liakhovetski.

Applied part of Sashca's patch that I forgot in v2.

Changes v1 -> v2
Addressed the comments of Guennadi Liakhovetski except from the following:

1. The mclk_get_divisor implementation, since I don't know what this code is good for

2. mx2_videobuf_release should not set pcdev->active on i.MX27, because mx27_camera_frame_done needs this pointer

3. In mx27_camera_emma_buf_init I don't know the meaning of those hard coded magic numbers

Applied i.MX27 fixes from Sascha.

 arch/arm/plat-mxc/include/mach/memory.h  |    4 +-
 arch/arm/plat-mxc/include/mach/mx2_cam.h |   46 +
 drivers/media/video/Kconfig              |   13 +
 drivers/media/video/Makefile             |    1 +
 drivers/media/video/mx2_camera.c         | 1513 ++++++++++++++++++++++++++++++
 5 files changed, 1575 insertions(+), 2 deletions(-)
 create mode 100644 arch/arm/plat-mxc/include/mach/mx2_cam.h
 create mode 100644 drivers/media/video/mx2_camera.c
arch/arm/plat-mxc/include/mach/memory.h +0 -0
index c4b40c3..849d891 100644
@@ -44,12 +44,12 @@
  */
 #define CONSISTENT_DMA_SIZE SZ_8M

arch/arm/plat-mxc/include/mach/memory.h line 53

Quoted 10 lines
arch/arm/plat-mxc/include/mach/memory.h +2 -2
 
-#elif defined(CONFIG_MX1_VIDEO)
+#elif defined(CONFIG_MX1_VIDEO) || defined(CONFIG_VIDEO_MX2_HOSTSUPPORT)
 /*
  * Increase size of DMA-consistent memory region.
  * This is required for i.MX camera driver to capture at least four VGA frames.
  */
 #define CONSISTENT_DMA_SIZE SZ_4M
-#endif /* CONFIG_MX1_VIDEO */
+#endif /* CONFIG_MX1_VIDEO || CONFIG_MX2_VIDEO */

s/CONFIG_MX2_VIDEO/CONFIG_VIDEO_MX2_HOSTSUPPORT/ please

arch/arm/plat-mxc/include/mach/mx2_cam.h

Quoted 6 lines
arch/arm/plat-mxc/include/mach/memory.h +0 -0
 #endif /* __ASM_ARCH_MXC_MEMORY_H__ */
arch/arm/plat-mxc/include/mach/mx2_cam.h new file +0 -0
new file mode 100644
index 0000000..3c080a3

Do you expect users of this header other than your driver? If not you can fold it into the latter.

[...snip...]

Quoted 5 lines
drivers/media/video/Kconfig +0 -0
index bdbc9d3..27e2acc 100644

drivers/media/video/Kconfig config VIDEO_OMAP2 · line 978

Quoted 10 lines
drivers/media/video/Kconfig +7 -0
@@ -969,6 +969,19 @@ config VIDEO_OMAP2
 	---help---
 	  This is a v4l2 driver for the TI OMAP2 camera capture interface
 
+config VIDEO_MX2_HOSTSUPPORT
+        bool
+
+config VIDEO_MX2
+	tristate "i.MX27/i.MX25 Camera Sensor Interface driver"
+	depends on VIDEO_DEV && SOC_CAMERA && (MACH_MX27 || ARCH_MX25)
+	select VIDEOBUF_DMA_CONTIG

CONTIG?

[...snap...]

Quoted 6 lines
drivers/media/video/mx2_camera.c new file +0 -0
new file mode 100644
index 0000000..98c93fa
@@ -0,0 +1,1513 @@

[...snip...]

Quoted 31 lines
drivers/media/video/mx2_camera.c new file +31 -0
+static int __devexit mx2_camera_remove(struct platform_device *pdev)
+{
+	struct soc_camera_host *soc_host = to_soc_camera_host(&pdev->dev);
+	struct mx2_camera_dev *pcdev = container_of(soc_host,
+			struct mx2_camera_dev, soc_host);
+	struct resource *res;
+
+	clk_put(pcdev->clk_csi);
+#ifdef CONFIG_MACH_MX27
+	if (cpu_is_mx27())
+		imx_dma_free(pcdev->dma);
+#endif /* CONFIG_MACH_MX27 */
+	free_irq(pcdev->irq_csi, pcdev);
+	if (mx27_camera_emma(pcdev))
+		free_irq(pcdev->irq_emma, pcdev);
+
+	soc_camera_host_unregister(&pcdev->soc_host);
+
+	iounmap(pcdev->base_csi);
+
+	if (mx27_camera_emma(pcdev)) {
+		clk_disable(pcdev->clk_emma);
+		clk_put(pcdev->clk_emma);
+		iounmap(pcdev->base_emma);
+		res = pcdev->res_emma;
+		release_mem_region(res->start, resource_size(res));
+	}
+
+	res = pcdev->res_csi;
+	release_mem_region(res->start, resource_size(res));
+

drivers/media/video/mx2_camera.c line 1492

Quoted 10 lines
drivers/media/video/mx2_camera.c new file +10 -0
+	kfree(pcdev);
+
+	dev_info(&pdev->dev, "MX2 Camera driver unloaded\n");
+
+	return 0;
+}
+
+static struct platform_driver mx2_camera_driver = {
+	.driver 	= {
+		.name	= MX2_CAM_DRV_NAME,

I'm always unsure if you need

.owner = THIS_MODULE,

here.

Best regards
Uwe

Signature · 2 lines
Pengutronix e.K.                           | Uwe Kleine-K?nig            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
author
#2 lore raw

PATCH v5 1/1 mx2_camera: Add soc_camera support for i.MX25/i.MX27

Hi Uwe,

Quoted 60 lines · Uwe Kleine-K?nig
Quoted 59 lines · Baruch Siach

This is the soc_camera support developed by Sascha Hauer for the i.MX27. Alan Carvalho de Assis modified the original driver to get it working on more recent kernels. I modified it further to add support for i.MX25. This driver has been tested on i.MX25 and i.MX27 based platforms.

Signed-off-by Baruch Siach <[email protected]>
Acked-by Guennadi Liakhovetski <[email protected]>

---
Changes v4 -> v5
Comments from Uwe Kleine-K?nig:

Enclose mx27 DMA related stuff in #ifdefs since the dma-mx1-mx2.h is no longer accessible to mx25 builds

s/MX2_VIDEO/VIDEO_MX2_HOSTSUPPORT/

Changes v3 -> v4
Address more comments from Guennadi Liakhovetski, including:

* Fix the double trigger handling of mx27 eMMA

* Add a FIXME comment in the code of eMMA overflow handling

Changes v2 -> v3
Address more comments from Guennadi Liakhovetski.

Applied part of Sashca's patch that I forgot in v2.

Changes v1 -> v2
Addressed the comments of Guennadi Liakhovetski except from the following:

1. The mclk_get_divisor implementation, since I don't know what this code is good for

2. mx2_videobuf_release should not set pcdev->active on i.MX27, because mx27_camera_frame_done needs this pointer

3. In mx27_camera_emma_buf_init I don't know the meaning of those hard coded magic numbers

Applied i.MX27 fixes from Sascha.

 arch/arm/plat-mxc/include/mach/memory.h  |    4 +-
 arch/arm/plat-mxc/include/mach/mx2_cam.h |   46 +
 drivers/media/video/Kconfig              |   13 +
 drivers/media/video/Makefile             |    1 +
 drivers/media/video/mx2_camera.c         | 1513 ++++++++++++++++++++++++++++++
 5 files changed, 1575 insertions(+), 2 deletions(-)
 create mode 100644 arch/arm/plat-mxc/include/mach/mx2_cam.h
 create mode 100644 drivers/media/video/mx2_camera.c
arch/arm/plat-mxc/include/mach/memory.h +0 -0
index c4b40c3..849d891 100644
@@ -44,12 +44,12 @@
  */
 #define CONSISTENT_DMA_SIZE SZ_8M
Quoted 10 lines

arch/arm/plat-mxc/include/mach/memory.h line 53

Quoted 9 lines
arch/arm/plat-mxc/include/mach/memory.h +2 -2
-#elif defined(CONFIG_MX1_VIDEO)
+#elif defined(CONFIG_MX1_VIDEO) || defined(CONFIG_VIDEO_MX2_HOSTSUPPORT)
 /*
  * Increase size of DMA-consistent memory region.
  * This is required for i.MX camera driver to capture at least four VGA frames.
  */
 #define CONSISTENT_DMA_SIZE SZ_4M
-#endif /* CONFIG_MX1_VIDEO */
+#endif /* CONFIG_MX1_VIDEO || CONFIG_MX2_VIDEO */

s/CONFIG_MX2_VIDEO/CONFIG_VIDEO_MX2_HOSTSUPPORT/ please

Oops. Will fix.

Quoted 8 lines

arch/arm/plat-mxc/include/mach/mx2_cam.h

Quoted 6 lines
arch/arm/plat-mxc/include/mach/memory.h +0 -0
 #endif /* __ASM_ARCH_MXC_MEMORY_H__ */
arch/arm/plat-mxc/include/mach/mx2_cam.h new file +0 -0
new file mode 100644
index 0000000..3c080a3

Do you expect users of this header other than your driver? If not you can fold it into the latter.

This header contains mx2_camera_platform_data. Platform code needs this.

Quoted 10 lines

[...snip...]

Quoted 8 lines
drivers/media/video/Kconfig +0 -0
index bdbc9d3..27e2acc 100644
@@ -969,6 +969,19 @@ config VIDEO_OMAP2
 	---help---
 	  This is a v4l2 driver for the TI OMAP2 camera capture interface
Quoted 8 lines

drivers/media/video/Kconfig config VIDEO_OMAP2 · line 978

Quoted 7 lines
drivers/media/video/Kconfig +7 -0
+config VIDEO_MX2_HOSTSUPPORT
+        bool
+
+config VIDEO_MX2
+	tristate "i.MX27/i.MX25 Camera Sensor Interface driver"
+	depends on VIDEO_DEV && SOC_CAMERA && (MACH_MX27 || ARCH_MX25)
+	select VIDEOBUF_DMA_CONTIG

CONTIG?

This selects the physically contiguous DMA implementation of the videobuf API. See drivers/media/video/videobuf-dma-contig.c.

Quoted 54 lines

[...snap...]

Quoted 6 lines
drivers/media/video/mx2_camera.c new file +0 -0
new file mode 100644
index 0000000..98c93fa
@@ -0,0 +1,1513 @@

[...snip...]

Quoted 31 lines
drivers/media/video/mx2_camera.c new file +31 -0
+static int __devexit mx2_camera_remove(struct platform_device *pdev)
+{
+	struct soc_camera_host *soc_host = to_soc_camera_host(&pdev->dev);
+	struct mx2_camera_dev *pcdev = container_of(soc_host,
+			struct mx2_camera_dev, soc_host);
+	struct resource *res;
+
+	clk_put(pcdev->clk_csi);
+#ifdef CONFIG_MACH_MX27
+	if (cpu_is_mx27())
+		imx_dma_free(pcdev->dma);
+#endif /* CONFIG_MACH_MX27 */
+	free_irq(pcdev->irq_csi, pcdev);
+	if (mx27_camera_emma(pcdev))
+		free_irq(pcdev->irq_emma, pcdev);
+
+	soc_camera_host_unregister(&pcdev->soc_host);
+
+	iounmap(pcdev->base_csi);
+
+	if (mx27_camera_emma(pcdev)) {
+		clk_disable(pcdev->clk_emma);
+		clk_put(pcdev->clk_emma);
+		iounmap(pcdev->base_emma);
+		res = pcdev->res_emma;
+		release_mem_region(res->start, resource_size(res));
+	}
+
+	res = pcdev->res_csi;
+	release_mem_region(res->start, resource_size(res));
+

drivers/media/video/mx2_camera.c line 1492

Quoted 10 lines
drivers/media/video/mx2_camera.c new file +10 -0
+	kfree(pcdev);
+
+	dev_info(&pdev->dev, "MX2 Camera driver unloaded\n");
+
+	return 0;
+}
+
+static struct platform_driver mx2_camera_driver = {
+	.driver 	= {
+		.name	= MX2_CAM_DRV_NAME,

I'm always unsure if you need

Quoted 3 lines

.owner = THIS_MODULE,

here.

It shouldn't heart. Will add.

baruch

Signature · 3 lines
                                                     ~. .~   Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
   - baruch at tkos.co.il - tel: +972.2.679.5364, http://www.tkos.co.il -
#3 lore raw

PATCH v5 1/1 mx2_camera: Add soc_camera support for i.MX25/i.MX27

In reply to Uwe Kleine-König

Hi Uwe

Quoted 8 lines · Uwe Kleine-König
Quoted 8 lines
drivers/media/video/Kconfig +0 -0
index bdbc9d3..27e2acc 100644
@@ -969,6 +969,19 @@ config VIDEO_OMAP2
 	---help---
 	  This is a v4l2 driver for the TI OMAP2 camera capture interface
Quoted 8 lines

drivers/media/video/Kconfig config VIDEO_OMAP2 · line 978

Quoted 7 lines
drivers/media/video/Kconfig +7 -0
+config VIDEO_MX2_HOSTSUPPORT
+        bool
+
+config VIDEO_MX2
+	tristate "i.MX27/i.MX25 Camera Sensor Interface driver"
+	depends on VIDEO_DEV && SOC_CAMERA && (MACH_MX27 || ARCH_MX25)
+	select VIDEOBUF_DMA_CONTIG

CONTIG?

What exactly was your question here?

Quoted 9 lines
Quoted 6 lines
drivers/media/video/mx2_camera.c new file +0 -0
new file mode 100644
index 0000000..98c93fa
@@ -0,0 +1,1513 @@

[...snip...]

Quoted 8 lines

drivers/media/video/mx2_camera.c line 1492

Quoted 3 lines
drivers/media/video/mx2_camera.c new file +3 -0
+static struct platform_driver mx2_camera_driver = {
+	.driver 	= {
+		.name	= MX2_CAM_DRV_NAME,

I'm always unsure if you need

.owner = THIS_MODULE,

here.

It is not needed in this case. See the "owner" field in struct soc_camera_host_ops mx2_soc_camera_host_ops.

But that's not the reason why I'm replying. What I didn't like in these your reviews, was the fact, that this driver has been submitted a number of times to the arm-kernel ML, it has "mx2" in its subject, so, you had enough chances to review it, just like Sascha did. Instead, you review it now, making the author create new versions of his patch. You have been asked for advise, because this patch potentially collided with other your patches, your help in resolving this question is appreciated. But then you suddenly decide to review the whole patch... Several of my patches have been treated similarly in the past, so, I know how annoying it is to have to re-iterate them because at v5 someone suddenly decided to take part in the patch review process too...

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

#4 lore raw

PATCH v5 1/1 mx2_camera: Add soc_camera support for i.MX25/i.MX27

Hi Guennadi,

Quoted 7 lines · Guennadi Liakhovetski
Quoted 5 lines

drivers/media/video/Kconfig config VIDEO_OMAP2 · line 978

Quoted 4 lines
drivers/media/video/Kconfig +4 -0
+config VIDEO_MX2
+	tristate "i.MX27/i.MX25 Camera Sensor Interface driver"
+	depends on VIDEO_DEV && SOC_CAMERA && (MACH_MX27 || ARCH_MX25)
+	select VIDEOBUF_DMA_CONTIG

CONTIG?

What exactly was your question here?

I thought it to be a mistyped "CONFIG", I was wrong.

Quoted 21 lines
Quoted 9 lines
Quoted 6 lines
drivers/media/video/mx2_camera.c new file +0 -0
new file mode 100644
index 0000000..98c93fa
@@ -0,0 +1,1513 @@

[...snip...]

Quoted 8 lines

drivers/media/video/mx2_camera.c line 1492

Quoted 3 lines
drivers/media/video/mx2_camera.c new file +3 -0
+static struct platform_driver mx2_camera_driver = {
+	.driver 	= {
+		.name	= MX2_CAM_DRV_NAME,

I'm always unsure if you need

.owner = THIS_MODULE,

here.

It is not needed in this case. See the "owner" field in struct soc_camera_host_ops mx2_soc_camera_host_ops.

Quoted 11 lines

But that's not the reason why I'm replying. What I didn't like in these your reviews, was the fact, that this driver has been submitted a number of times to the arm-kernel ML, it has "mx2" in its subject, so, you had enough chances to review it, just like Sascha did. Instead, you review it now, making the author create new versions of his patch. You have been asked for advise, because this patch potentially collided with other your patches, your help in resolving this question is appreciated. But then you suddenly decide to review the whole patch... Several of my patches have been treated similarly in the past, so, I know how annoying it is to have to re-iterate them because at v5 someone suddenly decided to take part in the patch review process too...

OK it might annoying but still I cannot understand why this is a reason to lament. I don't necessarily feel part of the intended audience for each patch on LAKML that contains mx2 in it's subject, still less if the patch changes very little in arch/arm/{plat-mxc,arch-mx*} as Baruch's patch does. Now he cc:d me and I had a look and even took the time to point out the things that I noticed. From my POV a late reviewer is better than getting code in that is less optimal. And let me point out that reviewing sequentially is more efficient in the sum---at least for the reviewers.

So should I ignore patches that are say already > v3?

Best regards
Uwe

Signature · 2 lines
Pengutronix e.K.                           | Uwe Kleine-K?nig            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |