Re: [PATCH 1/4] media: mx2_camera: Remove i.mx25 support.
Hi Javier, On Wed, Jan 2, 2013 at 10:18 AM, javier Martin javier.mar...@vista-silicon.com wrote: That's great. Did you need to change anything in the mx2 camera driver for mx25 to work? Have you already submitted the patches? I only touched board file code: http://www.spinics.net/lists/arm-kernel/msg210216.html ,and have only verified that camera probe worked on mx25pdk. Regards, Fabio Estevam -- To unsubscribe from this list: send the line unsubscribe linux-media 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/4] media: mx2_camera: Remove i.mx25 support.
Hi Fabio, Guennadi, sorry for the long delay but I've been out of the office for a month and without internet access. On 27 November 2012 14:05, Fabio Estevam feste...@gmail.com wrote: I just added the camera support to mach-mx25_3ds.c (which I will submit it soon to arm kernel list) and it works fine: soc-camera-pdrv soc-camera-pdrv.0: Probing soc-camera-pdrv.0 mx2-camera imx25-camera.0: Camera driver attached to camera 0 ov2640 0-0030: ov2640 Product ID 26:42 Manufacturer ID 7f:a2 i2c i2c-0: OV2640 Probed mx2-camera imx25-camera.0: Camera driver detached from camera 0 mx2-camera imx25-camera.0: MX2 Camera (CSI) driver probed, clock frequency: 2216 Could we please keep the mx25 support? That's great. Did you need to change anything in the mx2 camera driver for mx25 to work? Have you already submitted the patches? Regards. -- Javier Martin Vista Silicon S.L. CDTUC - FASE C - Oficina S-345 Avda de los Castros s/n 39005- Santander. Cantabria. Spain +34 942 25 32 60 www.vista-silicon.com -- To unsubscribe from this list: send the line unsubscribe linux-media 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/4] media: mx2_camera: Remove i.mx25 support.
Hi Fabio, On 2 January 2013 13:25, Fabio Estevam feste...@gmail.com wrote: Hi Javier, On Wed, Jan 2, 2013 at 10:18 AM, javier Martin javier.mar...@vista-silicon.com wrote: That's great. Did you need to change anything in the mx2 camera driver for mx25 to work? Have you already submitted the patches? I only touched board file code: http://www.spinics.net/lists/arm-kernel/msg210216.html ,and have only verified that camera probe worked on mx25pdk. Sorry Fabio but IMHO that's not enough. The probe() callback may work properly but it doesn't mean that buffer and HW handling for i.MX25 are functional. The condition to keep i.MX25 support in this file was that you were going to fix it but instead you have just added board support for a broken driver. I'd like to hear Guennadi's view on the matter but I think we've given plenty of time for this. Regards. -- Javier Martin Vista Silicon S.L. CDTUC - FASE C - Oficina S-345 Avda de los Castros s/n 39005- Santander. Cantabria. Spain +34 942 25 32 60 www.vista-silicon.com -- To unsubscribe from this list: send the line unsubscribe linux-media 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/4] media: mx2_camera: Remove i.mx25 support.
On Wed, Jan 2, 2013 at 10:35 AM, javier Martin javier.mar...@vista-silicon.com wrote: Sorry Fabio but IMHO that's not enough. The probe() callback may work properly but it doesn't mean that buffer and HW handling for i.MX25 are functional. The condition to keep i.MX25 support in this file was that you were going to fix it but instead you have just added board support for a broken driver. I'd like to hear Guennadi's view on the matter but I think we've given plenty of time for this. Sounds fair. Go ahead and remove mx25 camera support then. I would not be able to further debug this driver. Regards, Fabio Estevam -- To unsubscribe from this list: send the line unsubscribe linux-media 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/4] media: mx2_camera: Remove i.mx25 support.
Hi Guennadi/Javier, On Tue, Oct 30, 2012 at 12:57 PM, Guennadi Liakhovetski g.liakhovet...@gmx.de wrote: Fabio, I wasn't in favour of removing mx25 support initially and I still don't quite fancy it, but the delta is getting too large. If we remove it now you still have the git history, so, you'll be able to restore the latest state before removal. OTOH, it would be easier for me to review a 50-line fix patch, than a 400-line revert-and-fix patch, so, this has an adbantage too. Sorry for the delay. I just added the camera support to mach-mx25_3ds.c (which I will submit it soon to arm kernel list) and it works fine: soc-camera-pdrv soc-camera-pdrv.0: Probing soc-camera-pdrv.0 mx2-camera imx25-camera.0: Camera driver attached to camera 0 ov2640 0-0030: ov2640 Product ID 26:42 Manufacturer ID 7f:a2 i2c i2c-0: OV2640 Probed mx2-camera imx25-camera.0: Camera driver detached from camera 0 mx2-camera imx25-camera.0: MX2 Camera (CSI) driver probed, clock frequency: 2216 Could we please keep the mx25 support? Thanks, Fabio Estevam -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/4] media: mx2_camera: Remove i.mx25 support.
i.MX25 support has been broken for several releases
now and nobody seems to care about it.
Signed-off-by: Javier Martin javier.mar...@vista-silicon.com
---
drivers/media/platform/soc_camera/mx2_camera.c | 405 ++--
1 file changed, 100 insertions(+), 305 deletions(-)
diff --git a/drivers/media/platform/soc_camera/mx2_camera.c
b/drivers/media/platform/soc_camera/mx2_camera.c
index 9fd9d1c..e1b44b1 100644
--- a/drivers/media/platform/soc_camera/mx2_camera.c
+++ b/drivers/media/platform/soc_camera/mx2_camera.c
@@ -1,5 +1,5 @@
/*
- * V4L2 Driver for i.MX27/i.MX25 camera host
+ * V4L2 Driver for i.MX27 camera host
*
* Copyright (C) 2008, Sascha Hauer, Pengutronix
* Copyright (C) 2010, Baruch Siach, Orex Computed Radiography
@@ -65,9 +65,7 @@
#define CSICR1_RF_OR_INTEN (1 24)
#define CSICR1_STATFF_LEVEL(3 22)
#define CSICR1_STATFF_INTEN(1 21)
-#define CSICR1_RXFF_LEVEL(l) (((l) 3) 19) /* MX27 */
-#define CSICR1_FB2_DMA_INTEN (1 20) /* MX25 */
-#define CSICR1_FB1_DMA_INTEN (1 19) /* MX25 */
+#define CSICR1_RXFF_LEVEL(l) (((l) 3) 19)
#define CSICR1_RXFF_INTEN (1 18)
#define CSICR1_SOF_POL (1 17)
#define CSICR1_SOF_INTEN (1 16)
@@ -92,11 +90,6 @@
/* control reg 3 */
#define CSICR3_FRMCNT (0x 16)
#define CSICR3_FRMCNT_RST (1 15)
-#define CSICR3_DMA_REFLASH_RFF (1 14)
-#define CSICR3_DMA_REFLASH_SFF (1 13)
-#define CSICR3_DMA_REQ_EN_RFF (1 12)
-#define CSICR3_DMA_REQ_EN_SFF (1 11)
-#define CSICR3_RXFF_LEVEL(l) (((l) 7) 4)/* MX25 */
#define CSICR3_CSI_SUP (1 3)
#define CSICR3_ZERO_PACK_EN(1 2)
#define CSICR3_ECC_INT_EN (1 1)
@@ -108,8 +101,6 @@
#define CSISR_SFF_OR_INT (1 25)
#define CSISR_RFF_OR_INT (1 24)
#define CSISR_STATFF_INT (1 21)
-#define CSISR_DMA_TSF_FB2_INT (1 20) /* MX25 */
-#define CSISR_DMA_TSF_FB1_INT (1 19) /* MX25 */
#define CSISR_RXFF_INT (1 18)
#define CSISR_EOF_INT (1 17)
#define CSISR_SOF_INT (1 16)
@@ -121,17 +112,11 @@
#define CSICR1 0x00
#define CSICR2 0x04
-#define CSISR (cpu_is_mx27() ? 0x08 : 0x18)
+#define CSISR 0x08
#define CSISTATFIFO0x0c
#define CSIRFIFO 0x10
#define CSIRXCNT 0x14
-#define CSICR3 (cpu_is_mx27() ? 0x1C : 0x08)
-#define CSIDMASA_STATFIFO 0x20
-#define CSIDMATA_STATFIFO 0x24
-#define CSIDMASA_FB1 0x28
-#define CSIDMASA_FB2 0x2c
-#define CSIFBUF_PARA 0x30
-#define CSIIMAG_PARA 0x34
+#define CSICR3 0x1C
/* EMMA PrP */
#define PRP_CNTL 0x00
@@ -430,20 +415,9 @@ static void mx27_update_emma_buf(struct mx2_camera_dev
*pcdev,
static void mx2_camera_deactivate(struct mx2_camera_dev *pcdev)
{
- unsigned long flags;
-
clk_disable_unprepare(pcdev-clk_csi);
writel(0, pcdev-base_csi + CSICR1);
- if (cpu_is_mx27()) {
- writel(0, pcdev-base_emma + PRP_CNTL);
- } else if (cpu_is_mx25()) {
- spin_lock_irqsave(pcdev-lock, flags);
- pcdev-fb1_active = NULL;
- pcdev-fb2_active = NULL;
- writel(0, pcdev-base_csi + CSIDMASA_FB1);
- writel(0, pcdev-base_csi + CSIDMASA_FB2);
- spin_unlock_irqrestore(pcdev-lock, flags);
- }
+ writel(0, pcdev-base_emma + PRP_CNTL);
}
/*
@@ -464,10 +438,7 @@ static int mx2_camera_add_device(struct soc_camera_device
*icd)
if (ret 0)
return ret;
- csicr1 = CSICR1_MCLKEN;
-
- if (cpu_is_mx27())
- csicr1 |= CSICR1_PRP_IF_EN | CSICR1_FCC |
+ csicr1 = CSICR1_MCLKEN | CSICR1_PRP_IF_EN | CSICR1_FCC |
CSICR1_RXFF_LEVEL(0);
pcdev-csicr1 = csicr1;
@@ -497,65 +468,6 @@ static void mx2_camera_remove_device(struct
soc_camera_device *icd)
pcdev-icd = NULL;
}
-static void mx25_camera_frame_done(struct mx2_camera_dev *pcdev, int fb,
- int state)
-{
- struct vb2_buffer *vb;
- struct mx2_buffer *buf;
- struct mx2_buffer **fb_active = fb == 1 ? pcdev-fb1_active :
- pcdev-fb2_active;
- u32 fb_reg = fb == 1 ? CSIDMASA_FB1 : CSIDMASA_FB2;
- unsigned long flags;
-
- spin_lock_irqsave(pcdev-lock, flags);
-
- if (*fb_active == NULL)
- goto out;
-
- vb = (*fb_active)-vb;
- dev_dbg(pcdev-dev, %s (vb=0x%p) 0x%p %lu\n, __func__,
- vb, vb2_plane_vaddr(vb, 0), vb2_get_plane_payload(vb, 0));
-
- do_gettimeofday(vb-v4l2_buf.timestamp);
- vb-v4l2_buf.sequence++;
- vb2_buffer_done(vb, VB2_BUF_STATE_DONE);
-
- if (list_empty(pcdev-capture)) {
- buf = NULL;
- writel(0, pcdev-base_csi + fb_reg);
- } else {
-
Re: [PATCH 1/4] media: mx2_camera: Remove i.mx25 support.
Javier, On Tue, Oct 30, 2012 at 10:16 AM, Javier Martin javier.mar...@vista-silicon.com wrote: i.MX25 support has been broken for several releases now and nobody seems to care about it. I will work on fixing camera support for mx25. Please do not remove its support. Thanks, Fabio Estevam -- To unsubscribe from this list: send the line unsubscribe linux-media 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/4] media: mx2_camera: Remove i.mx25 support.
On Tue, 30 Oct 2012, Fabio Estevam wrote: Javier, On Tue, Oct 30, 2012 at 10:16 AM, Javier Martin javier.mar...@vista-silicon.com wrote: i.MX25 support has been broken for several releases now and nobody seems to care about it. I will work on fixing camera support for mx25. Please do not remove its support. This is good to hear, thanks for doing this! But we also don't want to slow down Javier's work, if he works on features, only available on i.MX27 or that he can only test there. How about separating parts of code, specific to each platform more cleanly? Maybe add an mx27_camera.c file to build the final driver from both files and mx27 and only from one on mx25? Or something similar? Would this be difficult or make sense at all? Thanks Guennadi --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/ -- To unsubscribe from this list: send the line unsubscribe linux-media 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/4] media: mx2_camera: Remove i.mx25 support.
Hi Guennadi, Fabio, On 30 October 2012 13:29, Guennadi Liakhovetski g.liakhovet...@gmx.de wrote: On Tue, 30 Oct 2012, Fabio Estevam wrote: Javier, On Tue, Oct 30, 2012 at 10:16 AM, Javier Martin javier.mar...@vista-silicon.com wrote: i.MX25 support has been broken for several releases now and nobody seems to care about it. I will work on fixing camera support for mx25. Please do not remove its support. This is good to hear, thanks for doing this! But we also don't want to slow down Javier's work, if he works on features, only available on i.MX27 or that he can only test there. How about separating parts of code, specific to each platform more cleanly? Maybe add an mx27_camera.c file to build the final driver from both files and mx27 and only from one on mx25? Or something similar? Would this be difficult or make sense at all? It's pretty good that you want to provide proper support for video capture on mx25 but I am still in favour of applying this patch for several reasons: 1. i.mx25 support is so broken now that it would be better to start from scratch IMHO. 2. AFAIK mx25 is not provided with an eMMa-PrP module. The current mx2_camera driver relies on this module to perform DMA transfers. If we added support for i.MX25 in this file, we'd have to use generic DMAs again, which is something we already removed in the past. 3. CSI provided in i.mx25 has more features than the one in the i.MX27, so the code they possibly share is even more reduced. By the way, removal of all i.mx25 traces in this file was announced several times in the past: 9 Jul 2012 [PATCH] [v3] i.MX27: Fix emma-prp clocks in mx2_camera.c 26 Jul 2012 [PATCH 2/4] media: mx2_camera: Mark i.MX25 support as BROKEN. [PATCH 3/4] Schedule removal of i.MX25 support in mx2_camera.c In my opinion. i.mx25 video capture support should be added in a separate file later. Though some CSI features are common, the lack of eMMa-PrP in i.mx25 will make the driver be very different. Please, expect a v2 of this patch soon. I've just remembered that I missed removing i.MX25 traces from the Kconfig file too. Regards. -- Javier Martin Vista Silicon S.L. CDTUC - FASE C - Oficina S-345 Avda de los Castros s/n 39005- Santander. Cantabria. Spain +34 942 25 32 60 www.vista-silicon.com -- To unsubscribe from this list: send the line unsubscribe linux-media 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/4] media: mx2_camera: Remove i.mx25 support.
On Tue, 30 Oct 2012, javier Martin wrote: Hi Guennadi, Fabio, On 30 October 2012 13:29, Guennadi Liakhovetski g.liakhovet...@gmx.de wrote: On Tue, 30 Oct 2012, Fabio Estevam wrote: Javier, On Tue, Oct 30, 2012 at 10:16 AM, Javier Martin javier.mar...@vista-silicon.com wrote: i.MX25 support has been broken for several releases now and nobody seems to care about it. I will work on fixing camera support for mx25. Please do not remove its support. This is good to hear, thanks for doing this! But we also don't want to slow down Javier's work, if he works on features, only available on i.MX27 or that he can only test there. How about separating parts of code, specific to each platform more cleanly? Maybe add an mx27_camera.c file to build the final driver from both files and mx27 and only from one on mx25? Or something similar? Would this be difficult or make sense at all? It's pretty good that you want to provide proper support for video capture on mx25 but I am still in favour of applying this patch for several reasons: Fabio, I wasn't in favour of removing mx25 support initially and I still don't quite fancy it, but the delta is getting too large. If we remove it now you still have the git history, so, you'll be able to restore the latest state before removal. OTOH, it would be easier for me to review a 50-line fix patch, than a 400-line revert-and-fix patch, so, this has an adbantage too. Let's try the following: I'm away the whole next week, so, I'll wait for your patches for almost 2 weeks until around 10.11. If you don't manage to fix the driver until then, we remove mx25 support to have it re-added later. What do you think? Thanks Guennadi 1. i.mx25 support is so broken now that it would be better to start from scratch IMHO. 2. AFAIK mx25 is not provided with an eMMa-PrP module. The current mx2_camera driver relies on this module to perform DMA transfers. If we added support for i.MX25 in this file, we'd have to use generic DMAs again, which is something we already removed in the past. 3. CSI provided in i.mx25 has more features than the one in the i.MX27, so the code they possibly share is even more reduced. By the way, removal of all i.mx25 traces in this file was announced several times in the past: 9 Jul 2012 [PATCH] [v3] i.MX27: Fix emma-prp clocks in mx2_camera.c 26 Jul 2012 [PATCH 2/4] media: mx2_camera: Mark i.MX25 support as BROKEN. [PATCH 3/4] Schedule removal of i.MX25 support in mx2_camera.c In my opinion. i.mx25 video capture support should be added in a separate file later. Though some CSI features are common, the lack of eMMa-PrP in i.mx25 will make the driver be very different. Please, expect a v2 of this patch soon. I've just remembered that I missed removing i.MX25 traces from the Kconfig file too. Regards. -- Javier Martin Vista Silicon S.L. CDTUC - FASE C - Oficina S-345 Avda de los Castros s/n 39005- Santander. Cantabria. Spain +34 942 25 32 60 www.vista-silicon.com --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/ -- To unsubscribe from this list: send the line unsubscribe linux-media 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/4] media: mx2_camera: Remove i.mx25 support.
On 30 October 2012 15:57, Guennadi Liakhovetski g.liakhovet...@gmx.de wrote: On Tue, 30 Oct 2012, javier Martin wrote: Hi Guennadi, Fabio, On 30 October 2012 13:29, Guennadi Liakhovetski g.liakhovet...@gmx.de wrote: On Tue, 30 Oct 2012, Fabio Estevam wrote: Javier, On Tue, Oct 30, 2012 at 10:16 AM, Javier Martin javier.mar...@vista-silicon.com wrote: i.MX25 support has been broken for several releases now and nobody seems to care about it. I will work on fixing camera support for mx25. Please do not remove its support. This is good to hear, thanks for doing this! But we also don't want to slow down Javier's work, if he works on features, only available on i.MX27 or that he can only test there. How about separating parts of code, specific to each platform more cleanly? Maybe add an mx27_camera.c file to build the final driver from both files and mx27 and only from one on mx25? Or something similar? Would this be difficult or make sense at all? It's pretty good that you want to provide proper support for video capture on mx25 but I am still in favour of applying this patch for several reasons: Fabio, I wasn't in favour of removing mx25 support initially and I still don't quite fancy it, but the delta is getting too large. If we remove it now you still have the git history, so, you'll be able to restore the latest state before removal. OTOH, it would be easier for me to review a 50-line fix patch, than a 400-line revert-and-fix patch, so, this has an adbantage too. Let's try the following: I'm away the whole next week, so, I'll wait for your patches for almost 2 weeks until around 10.11. If you don't manage to fix the driver until then, we remove mx25 support to have it re-added later. What do you think? Please, consider adding support for i.MX25 in a separate file. Just take a look at the mess we had in 3.1: http://lxr.linux.no/#linux+v3.1/drivers/media/video/mx2_camera.c Regards. Thanks Guennadi 1. i.mx25 support is so broken now that it would be better to start from scratch IMHO. 2. AFAIK mx25 is not provided with an eMMa-PrP module. The current mx2_camera driver relies on this module to perform DMA transfers. If we added support for i.MX25 in this file, we'd have to use generic DMAs again, which is something we already removed in the past. 3. CSI provided in i.mx25 has more features than the one in the i.MX27, so the code they possibly share is even more reduced. By the way, removal of all i.mx25 traces in this file was announced several times in the past: 9 Jul 2012 [PATCH] [v3] i.MX27: Fix emma-prp clocks in mx2_camera.c 26 Jul 2012 [PATCH 2/4] media: mx2_camera: Mark i.MX25 support as BROKEN. [PATCH 3/4] Schedule removal of i.MX25 support in mx2_camera.c In my opinion. i.mx25 video capture support should be added in a separate file later. Though some CSI features are common, the lack of eMMa-PrP in i.mx25 will make the driver be very different. Please, expect a v2 of this patch soon. I've just remembered that I missed removing i.MX25 traces from the Kconfig file too. Regards. -- Javier Martin Vista Silicon S.L. CDTUC - FASE C - Oficina S-345 Avda de los Castros s/n 39005- Santander. Cantabria. Spain +34 942 25 32 60 www.vista-silicon.com --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/ -- Javier Martin Vista Silicon S.L. CDTUC - FASE C - Oficina S-345 Avda de los Castros s/n 39005- Santander. Cantabria. Spain +34 942 25 32 60 www.vista-silicon.com -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
