Re: [PATCH 12/15] [media] marvell-ccic: add soc_camera support in mmp driver

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, 23 Nov 2012, Albert Wang wrote:

> This patch adds the soc_camera support in the platform driver: mmp-driver.c.
> Specified board driver also should be modified to support soc_camera by passing
> some platform datas to platform driver.
> 
> Currently the soc_camera mode in mmp driver only supports B_DMA_contig mode.
> 
> 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      |   22 ++++++
>  drivers/media/platform/marvell-ccic/mmp-driver.c |   80 +++++++++++++++++++---
>  include/media/mmp-camera.h                       |    2 +
>  4 files changed, 97 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/media/platform/Makefile b/drivers/media/platform/Makefile
> index baaa550..feae003 100644
> --- a/drivers/media/platform/Makefile
> +++ b/drivers/media/platform/Makefile
> @@ -11,8 +11,8 @@ 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_MARVELL_CCIC)       += marvell-ccic/
>  
>  obj-$(CONFIG_VIDEO_OMAP2)		+= omap2cam.o
>  obj-$(CONFIG_VIDEO_OMAP3)	+= omap3isp/
> diff --git a/drivers/media/platform/marvell-ccic/Kconfig b/drivers/media/platform/marvell-ccic/Kconfig
> index bf739e3..6e3eaa0 100755
> --- a/drivers/media/platform/marvell-ccic/Kconfig
> +++ b/drivers/media/platform/marvell-ccic/Kconfig
> @@ -1,23 +1,45 @@
> +config VIDEO_MARVELL_CCIC
> +       tristate
> +config VIDEO_MRVL_SOC_CAMERA
> +       tristate

The latter you can make a "bool"

> +
>  config VIDEO_CAFE_CCIC
>  	tristate "Marvell 88ALP01 (Cafe) CMOS Camera Controller support"
>  	depends on PCI && I2C && VIDEO_V4L2
>  	select VIDEO_OV7670
>  	select VIDEOBUF2_VMALLOC
>  	select VIDEOBUF2_DMA_CONTIG
> +	select VIDEO_MARVELL_CCIC
>  	---help---
>  	  This is a video4linux2 driver for the Marvell 88ALP01 integrated
>  	  CMOS camera controller.  This is the controller found on first-
>  	  generation OLPC systems.
>  
> +choice
> +	prompt "Camera support on Marvell MMP"
> +	depends on ARCH_MMP && VIDEO_V4L2
> +	optional
>  config VIDEO_MMP_CAMERA
>  	tristate "Marvell Armada 610 integrated camera controller support"
>  	depends on ARCH_MMP && I2C && VIDEO_V4L2
>  	select VIDEO_OV7670
>  	select I2C_GPIO
>  	select VIDEOBUF2_DMA_SG
> +	select VIDEO_MARVELL_CCIC
>  	---help---
>  	  This is a Video4Linux2 driver for the integrated camera
>  	  controller found on Marvell Armada 610 application
>  	  processors (and likely beyond).  This is the controller found
>  	  in OLPC XO 1.75 systems.
>  
> +config VIDEO_MMP_SOC_CAMERA
> +	bool "Marvell MMP camera driver based on SOC_CAMERA"
> +	depends on VIDEO_DEV && SOC_CAMERA && ARCH_MMP && VIDEO_V4L2
> +	select VIDEOBUF2_DMA_CONTIG
> +	select VIDEO_MARVELL_CCIC
> +	select VIDEO_MRVL_SOC_CAMERA
> +	---help---
> +	  This is a Video4Linux2 driver for the Marvell Mobile Soc
> +	  PXA910/PXA688/PXA2128/PXA988 CCIC
> +	  (CMOS Camera Interface Controller)
> +endchoice
> diff --git a/drivers/media/platform/marvell-ccic/mmp-driver.c b/drivers/media/platform/marvell-ccic/mmp-driver.c
> index c3031e7..bea7224 100755
> --- a/drivers/media/platform/marvell-ccic/mmp-driver.c
> +++ b/drivers/media/platform/marvell-ccic/mmp-driver.c
> @@ -4,6 +4,12 @@
>   *
>   * Copyright 2011 Jonathan Corbet <corbet@xxxxxxx>
>   *
> + * History:
> + * Support Soc Camera
> + * Support MIPI interface and Dual CCICs in Soc Camera mode
> + * Sep-2012: Libin Yang <lbyang@xxxxxxxxxxx>
> + *           Albert Wang <twang13@xxxxxxxxxxx>
> + *
>   * This file may be distributed under the terms of the GNU General
>   * Public License, version 2.
>   */
> @@ -28,6 +34,10 @@
>  #include <linux/list.h>
>  #include <linux/pm.h>
>  #include <linux/clk.h>
> +#include <linux/regulator/consumer.h>
> +#include <media/videobuf2-dma-contig.h>
> +#include <media/soc_camera.h>
> +#include <media/soc_mediabus.h>
>  
>  #include "mcam-core.h"
>  
> @@ -40,6 +50,8 @@ struct mmp_camera {
>  	struct platform_device *pdev;
>  	struct mcam_camera mcam;
>  	struct list_head devlist;
> +	/* will change here */
> +	struct clk *clk[3];	/* CCIC_GATE, CCIC_RST, CCIC_DBG clocks */
>  	int irq;
>  };
>  
> @@ -135,7 +147,9 @@ static void mmpcam_power_up_ctlr(struct mmp_camera *cam)
>  static void mmpcam_power_up(struct mcam_camera *mcam)
>  {
>  	struct mmp_camera *cam = mcam_to_cam(mcam);
> +#ifndef CONFIG_VIDEO_MMP_SOC_CAMERA
>  	struct mmp_camera_platform_data *pdata;
> +#endif
>  /*
>   * Turn on power and clocks to the controller.
>   */
> @@ -144,34 +158,40 @@ static void mmpcam_power_up(struct mcam_camera *mcam)
>   * Provide power to the sensor.
>   */
>  	mcam_reg_write(mcam, REG_CLKCTRL, 0x60000002);
> +#ifndef CONFIG_VIDEO_MMP_SOC_CAMERA
>  	pdata = cam->pdev->dev.platform_data;
>  	gpio_set_value(pdata->sensor_power_gpio, 1);
>  	mdelay(5);
> +#endif
>  	mcam_reg_clear_bit(mcam, REG_CTRL1, 0x10000000);
> +#ifndef CONFIG_VIDEO_MMP_SOC_CAMERA
>  	gpio_set_value(pdata->sensor_reset_gpio, 0); /* reset is active low */
>  	mdelay(5);
>  	gpio_set_value(pdata->sensor_reset_gpio, 1); /* reset is active low */
>  	mdelay(5);
> -
> +#endif
>  	mcam_clk_set(mcam, 1);
>  }
>  
>  static void mmpcam_power_down(struct mcam_camera *mcam)
>  {
>  	struct mmp_camera *cam = mcam_to_cam(mcam);
> +#ifndef CONFIG_VIDEO_MMP_SOC_CAMERA
>  	struct mmp_camera_platform_data *pdata;
> +#endif
>  /*
>   * Turn off clocks and set reset lines
>   */
>  	iowrite32(0, cam->power_regs + REG_CCIC_DCGCR);
>  	iowrite32(0, cam->power_regs + REG_CCIC_CRCR);
> +#ifndef CONFIG_VIDEO_MMP_SOC_CAMERA
>  /*
>   * Shut down the sensor.
>   */
>  	pdata = cam->pdev->dev.platform_data;
>  	gpio_set_value(pdata->sensor_power_gpio, 0);
>  	gpio_set_value(pdata->sensor_reset_gpio, 0);
> -
> +#endif
>  	mcam_clk_set(mcam, 0);
>  }
>  
> @@ -322,20 +342,31 @@ static int mmpcam_probe(struct platform_device *pdev)
>  	INIT_LIST_HEAD(&cam->devlist);
>  
>  	mcam = &cam->mcam;
> +	spin_lock_init(&mcam->dev_lock);
>  	mcam->plat_power_up = mmpcam_power_up;
>  	mcam->plat_power_down = mmpcam_power_down;
>  	mcam->ctlr_reset = mcam_ctlr_reset;
>  	mcam->calc_dphy = mmpcam_calc_dphy;
>  	mcam->dev = &pdev->dev;
>  	mcam->use_smbus = 0;
> +	mcam->card_name = pdata->name;
> +	mcam->mclk_min = pdata->mclk_min;
> +	mcam->mclk_src = pdata->mclk_src;
> +	mcam->mclk_div = pdata->mclk_div;

Actually you don't really have to copy everything from platform data to 
your private driver object during probing. You can access your platform 
data also at run-time. So, maybe you can survive without adding these 
.mclk_* struct members?

> +#ifdef CONFIG_VIDEO_MMP_SOC_CAMERA

Maybe you could add a run-time context detection to avoid preprocessor 
conditionals?

> +	mcam->chip_id = pdata->chip_id;
> +	mcam->buffer_mode = B_DMA_contig;
> +#else
> +	mcam->chip_id = V4L2_IDENT_ARMADA610;
> +	mcam->buffer_mode = B_DMA_sg;
> +#endif
>  	mcam->ccic_id = pdev->id;
>  	mcam->bus_type = pdata->bus_type;
>  	mcam->dphy = &(pdata->dphy);
>  	mcam->mipi_enabled = 0;
>  	mcam->lane = pdata->lane;
> -	mcam->chip_id = V4L2_IDENT_ARMADA610;
> -	mcam->buffer_mode = B_DMA_sg;
> -	spin_lock_init(&mcam->dev_lock);
> +	INIT_LIST_HEAD(&mcam->buffers);
> +
>  	/*
>  	 * Get our I/O memory.
>  	 */
> @@ -365,9 +396,22 @@ static int mmpcam_probe(struct platform_device *pdev)
>  	}
>  
>  	mcam_init_clk(mcam, pdata, 1);
> +#ifdef CONFIG_VIDEO_MMP_SOC_CAMERA
> +	mcam->vb_alloc_ctx =
> +		vb2_dma_contig_init_ctx(&pdev->dev);
> +	if (IS_ERR(mcam->vb_alloc_ctx))
> +		return PTR_ERR(mcam->vb_alloc_ctx);

Also the choice of the videobuf2 implementation can be a runtime 
parameter.

> +
> +	ret = mcam_soc_camera_host_register(mcam);
> +	if (ret)
> +		goto out_free_ctx;
> +#endif
> +
> +#ifdef CONFIG_VIDEO_MMP_CAMERA
>  	/*
>  	 * Find the i2c adapter.  This assumes, of course, that the
>  	 * i2c bus is already up and functioning.
> +	 * soc-camera manages i2c interface in sensor side
>  	 */
>  	mcam->i2c_adapter = platform_get_drvdata(pdata->i2c_device);
>  	if (mcam->i2c_adapter == NULL) {
> @@ -396,9 +440,10 @@ static int mmpcam_probe(struct platform_device *pdev)
>  	 * Power the device up and hand it off to the core.
>  	 */
>  	mmpcam_power_up(mcam);
> +#endif
>  	ret = mccic_register(mcam);
>  	if (ret)
> -		goto out_gpio2;
> +		goto ccic_register_fail;
>  	/*
>  	 * Finally, set up our IRQ now that the core is ready to
>  	 * deal with it.

Have you considered splitting the .probe() function into 3: common, MMP 
and SOC? Like

static int probe()
{
	/* Common probing */
	...
	if (mmp)
		ret = mmp_probe();
	else if (soc)
		ret = soc_probe();
	if (ret < 0)
		goto out_plat_probe;

> @@ -418,12 +463,21 @@ static int mmpcam_probe(struct platform_device *pdev)
>  
>  out_unregister:
>  	mccic_shutdown(mcam);
> -out_gpio2:
> +ccic_register_fail:
> +#ifdef CONFIG_VIDEO_MMP_CAMERA
>  	mmpcam_power_down(mcam);
> +	mcam_init_clk(mcam, pdata, 0);
>  	gpio_free(pdata->sensor_reset_gpio);
>  out_gpio:
>  	gpio_free(pdata->sensor_power_gpio);
> +#endif
> +#ifdef CONFIG_VIDEO_MMP_SOC_CAMERA
> +	soc_camera_host_unregister(&mcam->soc_host);
>  	mcam_init_clk(mcam, pdata, 0);
> +out_free_ctx:
> +	vb2_dma_contig_cleanup_ctx(mcam->vb_alloc_ctx);
> +	mcam->vb_alloc_ctx = NULL;
> +#endif
>  	return ret;
>  }
>  
> @@ -431,14 +485,22 @@ out_gpio:
>  static int mmpcam_remove(struct mmp_camera *cam)
>  {
>  	struct mcam_camera *mcam = &cam->mcam;
> -	struct mmp_camera_platform_data *pdata;
> +#ifdef CONFIG_VIDEO_MMP_SOC_CAMERA
> +	struct soc_camera_host *soc_host = &mcam->soc_host;
> +#endif
> +	struct mmp_camera_platform_data *pdata = cam->pdev->dev.platform_data;
>  
>  	mmpcam_remove_device(cam);
>  	mccic_shutdown(mcam);
>  	mmpcam_power_down(mcam);
> -	pdata = cam->pdev->dev.platform_data;
> +#ifdef CONFIG_VIDEO_MMP_SOC_CAMERA
> +	soc_camera_host_unregister(soc_host);
> +	vb2_dma_contig_cleanup_ctx(mcam->vb_alloc_ctx);
> +	mcam->vb_alloc_ctx = NULL;
> +#else
>  	gpio_free(pdata->sensor_reset_gpio);
>  	gpio_free(pdata->sensor_power_gpio);
> +#endif

Same here, a runtime detection would look much nicer!

>  	mcam_init_clk(mcam, pdata, 0);
>  	return 0;
>  }
> diff --git a/include/media/mmp-camera.h b/include/media/mmp-camera.h
> index 36891ed..731f81f 100755
> --- a/include/media/mmp-camera.h
> +++ b/include/media/mmp-camera.h
> @@ -6,9 +6,11 @@ struct mmp_camera_platform_data {
>  	struct platform_device *i2c_device;
>  	int sensor_power_gpio;
>  	int sensor_reset_gpio;
> +	char name[16];
>  	int mclk_min;
>  	int mclk_src;
>  	int mclk_div;
> +	int chip_id;
>  	/*
>  	 * MIPI support
>  	 */
> -- 
> 1.7.9.5
> 

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 majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux