Hi, Guennadi >-----Original Message----- >From: Guennadi Liakhovetski [mailto:g.liakhovetski@xxxxxx] >Sent: Tuesday, 05 March, 2013 21:33 >To: Albert Wang >Cc: corbet@xxxxxxx; Linux Media Mailing List; Libin Yang >Subject: Re: [REVIEW PATCH V4 10/12] [media] marvell-ccic: add soc_camera support >for marvell-ccic driver > >Ok, can we test existing systems, have they been tested? It is hard to >review this, as you might imagine. At least I don't think I can in any >reasonably way verify by just looking at this whether it's breaking >anything... I think, people, really using / developing this driver for >other platforms would have to just extensively test this and verify the >final result (I think I know one such person ;-)). A couple of minor >comments below. In general - it does look quite good to me! So, provided >relevant testing is done and, possibly, my comments addressed: > Actually, we have tested it on all Marvell platforms which we have. But we have no OLPC board on hand. You are right and we still need test it on OLPC. >Acked-by: Guennadi Liakhovetski <g.liakhovetski@xxxxxx> > >Thanks >Guennadi > >On Thu, 7 Feb 2013, Albert Wang wrote: > >> This patch adds the soc_camera mode support in marvell-ccic driver. >> It also removes the old mode for maintaining single mode in the future. >> >> Signed-off-by: Libin Yang <lbyang@xxxxxxxxxxx> >> Signed-off-by: Albert Wang <twang13@xxxxxxxxxxx> >> --- >> drivers/media/platform/Makefile | 4 +- >> drivers/media/platform/marvell-ccic/Kconfig | 6 +- >> drivers/media/platform/marvell-ccic/mcam-core.c | 1084 +++++++--------------- >> drivers/media/platform/marvell-ccic/mcam-core.h | 27 +- >> drivers/media/platform/marvell-ccic/mmp-driver.c | 73 +- >> include/media/mmp-camera.h | 5 + >> 6 files changed, 402 insertions(+), 797 deletions(-) > >At least the stats look good ;-) > >> >> diff --git a/drivers/media/platform/Makefile b/drivers/media/platform/Makefile >> index 4817d28..1a345c8 100644 >> --- a/drivers/media/platform/Makefile >> +++ b/drivers/media/platform/Makefile >> @@ -11,8 +11,6 @@ obj-$(CONFIG_VIDEO_TIMBERDALE) += timblogiw.o >> obj-$(CONFIG_VIDEO_M32R_AR_M64278) += arv.o >> >> obj-$(CONFIG_VIDEO_VIA_CAMERA) += via-camera.o >> -obj-$(CONFIG_VIDEO_CAFE_CCIC) += marvell-ccic/ >> -obj-$(CONFIG_VIDEO_MMP_CAMERA) += marvell-ccic/ >> >> obj-$(CONFIG_VIDEO_OMAP2) += omap2cam.o >> obj-$(CONFIG_VIDEO_OMAP3) += omap3isp/ >> @@ -43,6 +41,8 @@ obj-$(CONFIG_ARCH_DAVINCI) += davinci/ >> obj-$(CONFIG_VIDEO_SH_VOU) += sh_vou.o >> >> obj-$(CONFIG_SOC_CAMERA) += soc_camera/ >> +obj-$(CONFIG_VIDEO_CAFE_CCIC) += marvell-ccic/ >> +obj-$(CONFIG_VIDEO_MMP_CAMERA) += marvell-ccic/ >> >> obj-y += davinci/ >> >> diff --git a/drivers/media/platform/marvell-ccic/Kconfig >b/drivers/media/platform/marvell-ccic/Kconfig >> index bf739e3..7fa3c62 100755 >> --- a/drivers/media/platform/marvell-ccic/Kconfig >> +++ b/drivers/media/platform/marvell-ccic/Kconfig >> @@ -10,14 +10,14 @@ config VIDEO_CAFE_CCIC >> generation OLPC systems. >> >> config VIDEO_MMP_CAMERA >> - tristate "Marvell Armada 610 integrated camera controller support" >> + tristate "Marvell Armada integrated camera controller support" >> depends on ARCH_MMP && I2C && VIDEO_V4L2 > >Perhaps, you want to add "&& SOC_CAMERA" above > Yes, we missed it. Thanks! >> - select VIDEO_OV7670 >> select I2C_GPIO >> + select VIDEOBUF2_DMA_CONTIG >> select VIDEOBUF2_DMA_SG >> ---help--- >> This is a Video4Linux2 driver for the integrated camera >> - controller found on Marvell Armada 610 application >> + controller found on Marvell Armada application >> processors (and likely beyond). This is the controller found >> in OLPC XO 1.75 systems. >> > >[snip] > >> diff --git a/drivers/media/platform/marvell-ccic/mmp-driver.c >b/drivers/media/platform/marvell-ccic/mmp-driver.c >> index 732af16..3d5db24 100755 >> --- a/drivers/media/platform/marvell-ccic/mmp-driver.c >> +++ b/drivers/media/platform/marvell-ccic/mmp-driver.c >> @@ -28,6 +28,10 @@ >> #include <linux/list.h> >> #include <linux/pm.h> >> #include <linux/clk.h> >> +#include <linux/regulator/consumer.h> > >Looks like you don't need this header. > Ok, will remove it. >Thanks >Guennadi >--- >Guennadi Liakhovetski, Ph.D. >Freelance Open-Source Software Developer >http://www.open-technology.de/ Thanks Albert Wang 86-21-61092656 -- 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
