On 30 October 2012 15:57, Guennadi Liakhovetski <g.liakhovetski@xxxxxx> wrote: > On Tue, 30 Oct 2012, javier Martin wrote: > >> Hi Guennadi, Fabio, >> >> On 30 October 2012 13:29, Guennadi Liakhovetski <g.liakhovetski@xxxxxx> wrote: >> > On Tue, 30 Oct 2012, Fabio Estevam wrote: >> > >> >> Javier, >> >> >> >> On Tue, Oct 30, 2012 at 10:16 AM, Javier Martin >> >> <javier.martin@xxxxxxxxxxxxxxxxx> 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 majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
