[PATCH v4 0/3] add Atmel ISI, ov2640 support for sam9m10/sam9g45

[PATCH v4 0/3] add Atmel ISI, ov2640 support for sam9m10/sam9g45

[Josh Wu]

Hi, all

Accroding to the comments from Guennadi, please see below the changes since V3:
1. Move isi_mck registration to at91_add_device_isi, but the user has a chance to or not to use the Programmable clock of the SoC.
2. In board file, e.g. board-sam9m10g45ek.c, user can pass a boolean to at91_add_device_isi, so that the SoC programmable clock can be used or not.

To summary, programmable clock is managed in SoC level. The user can decide to use or not to use SoC clock as the sensor MCK at board level. 
In later case, user has to provide a clock source named "isi_mck"

Best Regards,
Josh Wu

[PATCH v4 1/3] [media] at91: add code to enable/disable ISI_MCK clock

[Josh Wu]

This patch
- add ISI_MCK clock enable/disable code.
- change field name in isi_platform_data structure

Signed-off-by: Josh Wu <josh.wu@atmel.com>
---
 drivers/media/video/atmel-isi.c |   34 +++++++++++++++++++++++++++++++---
 include/media/atmel-isi.h       |    4 +++-
 2 files changed, 34 insertions(+), 4 deletions(-)

diff --git a/drivers/media/video/atmel-isi.c b/drivers/media/video/atmel-isi.c
index 774715d..0617163 100644
--- a/drivers/media/video/atmel-isi.c
+++ b/drivers/media/video/atmel-isi.c
@@ -90,7 +90,10 @@ struct atmel_isi {
 	struct isi_dma_desc		dma_desc[MAX_BUFFER_NUM];
 
 	struct completion		complete;
+	/* ISI peripherial clock */
 	struct clk			*pclk;
+	/* ISI_MCK, feed to camera sensor to generate pixel clock */
+	struct clk			*mck;
 	unsigned int			irq;
 
 	struct isi_platform_data	*pdata;
@@ -773,6 +776,12 @@ static int isi_camera_add_device(struct soc_camera_device *icd)
 	if (ret)
 		return ret;
 
+	ret = clk_enable(isi->mck);
+	if (ret) {
+		clk_disable(isi->pclk);
+		return ret;
+	}
+
 	isi->icd = icd;
 	dev_dbg(icd->parent, "Atmel ISI Camera driver attached to camera %d\n",
 		 icd->devnum);
@@ -786,6 +795,7 @@ static void isi_camera_remove_device(struct soc_camera_device *icd)
 
 	BUG_ON(icd != isi->icd);
 
+	clk_disable(isi->mck);
 	clk_disable(isi->pclk);
 	isi->icd = NULL;
 
@@ -869,7 +879,7 @@ static int isi_camera_set_bus_param(struct soc_camera_device *icd, u32 pixfmt)
 
 	if (isi->pdata->has_emb_sync)
 		cfg1 |= ISI_CFG1_EMB_SYNC;
-	if (isi->pdata->isi_full_mode)
+	if (isi->pdata->full_mode)
 		cfg1 |= ISI_CFG1_FULL_MODE;
 
 	isi_writel(isi, ISI_CTRL, ISI_CTRL_DIS);
@@ -907,6 +917,7 @@ static int __devexit atmel_isi_remove(struct platform_device *pdev)
 			isi->fb_descriptors_phys);
 
 	iounmap(isi->regs);
+	clk_put(isi->mck);
 	clk_put(isi->pclk);
 	kfree(isi);
 
@@ -925,7 +936,7 @@ static int __devinit atmel_isi_probe(struct platform_device *pdev)
 	struct isi_platform_data *pdata;
 
 	pdata = dev->platform_data;
-	if (!pdata || !pdata->data_width_flags) {
+	if (!pdata || !pdata->data_width_flags || !pdata->mck_hz) {
 		dev_err(&pdev->dev,
 			"No config available for Atmel ISI\n");
 		return -EINVAL;
@@ -954,6 +965,21 @@ static int __devinit atmel_isi_probe(struct platform_device *pdev)
 	INIT_LIST_HEAD(&isi->video_buffer_list);
 	INIT_LIST_HEAD(&isi->dma_desc_head);
 
+	/* Get ISI_MCK, provided by programmable clock or external clock */
+	isi->mck = clk_get(dev, "isi_mck");
+	if (IS_ERR_OR_NULL(isi->mck)) {
+		dev_err(dev, "Failed to get isi_mck\n");
+		ret = PTR_ERR(isi->mck);
+		if (isi->mck == NULL)
+			ret = -EINVAL;
+		goto err_alloc_descriptors;
+	}
+
+	/* Set ISI_MCK's frequency, it should be faster than pixel clock */
+	ret = clk_set_rate(isi->mck, pdata->mck_hz);
+	if (ret < 0)
+		goto err_set_mck_rate;
+
 	isi->p_fb_descriptors = dma_alloc_coherent(&pdev->dev,
 				sizeof(struct fbd) * MAX_BUFFER_NUM,
 				&isi->fb_descriptors_phys,
@@ -961,7 +987,7 @@ static int __devinit atmel_isi_probe(struct platform_device *pdev)
 	if (!isi->p_fb_descriptors) {
 		ret = -ENOMEM;
 		dev_err(&pdev->dev, "Can't allocate descriptors!\n");
-		goto err_alloc_descriptors;
+		goto err_set_mck_rate;
 	}
 
 	for (i = 0; i < MAX_BUFFER_NUM; i++) {
@@ -1023,6 +1049,8 @@ err_alloc_ctx:
 			sizeof(struct fbd) * MAX_BUFFER_NUM,
 			isi->p_fb_descriptors,
 			isi->fb_descriptors_phys);
+err_set_mck_rate:
+	clk_put(isi->mck);
 err_alloc_descriptors:
 	kfree(isi);
 err_alloc_isi:
diff --git a/include/media/atmel-isi.h b/include/media/atmel-isi.h
index 26cece5..6568230 100644
--- a/include/media/atmel-isi.h
+++ b/include/media/atmel-isi.h
@@ -110,10 +110,12 @@ struct isi_platform_data {
 	u8 hsync_act_low;
 	u8 vsync_act_low;
 	u8 pclk_act_falling;
-	u8 isi_full_mode;
+	u8 full_mode;
 	u32 data_width_flags;
 	/* Using for ISI_CFG1 */
 	u32 frate;
+	/* Using for ISI_MCK */
+	u32 mck_hz;
 };
 
 #endif /* __ATMEL_ISI_H__ */
-- 
1.6.3.3

[PATCH v4 2/3] at91: add parameters for at91_add_device_isi function

[Josh Wu]

This patch add parameters for at91_add_device_isi function

Signed-off-by: Josh Wu <josh.wu@atmel.com>
---
 arch/arm/mach-at91/at91sam9263_devices.c |   13 ++++++++++---
 arch/arm/mach-at91/include/mach/board.h  |    4 +++-
 2 files changed, 13 insertions(+), 4 deletions(-)

diff --git a/arch/arm/mach-at91/at91sam9263_devices.c b/arch/arm/mach-at91/at91sam9263_devices.c
index a050f41..7990e8e 100644
--- a/arch/arm/mach-at91/at91sam9263_devices.c
+++ b/arch/arm/mach-at91/at91sam9263_devices.c
@@ -885,7 +885,8 @@ static struct platform_device at91sam9263_isi_device = {
 	.num_resources	= ARRAY_SIZE(isi_resources),
 };
 
-void __init at91_add_device_isi(void)
+void __init at91_add_device_isi(struct isi_platform_data * data,
+		bool use_pck_as_mck)
 {
 	at91_set_A_periph(AT91_PIN_PE0, 0);	/* ISI_D0 */
 	at91_set_A_periph(AT91_PIN_PE1, 0);	/* ISI_D1 */
@@ -898,14 +899,20 @@ void __init at91_add_device_isi(void)
 	at91_set_A_periph(AT91_PIN_PE8, 0);	/* ISI_PCK */
 	at91_set_A_periph(AT91_PIN_PE9, 0);	/* ISI_HSYNC */
 	at91_set_A_periph(AT91_PIN_PE10, 0);	/* ISI_VSYNC */
-	at91_set_B_periph(AT91_PIN_PE11, 0);	/* ISI_MCK (PCK3) */
 	at91_set_B_periph(AT91_PIN_PE12, 0);	/* ISI_PD8 */
 	at91_set_B_periph(AT91_PIN_PE13, 0);	/* ISI_PD9 */
 	at91_set_B_periph(AT91_PIN_PE14, 0);	/* ISI_PD10 */
 	at91_set_B_periph(AT91_PIN_PE15, 0);	/* ISI_PD11 */
+
+	if (use_pck_as_mck) {
+		at91_set_B_periph(AT91_PIN_PE11, 0);	/* ISI_MCK (PCK3) */
+
+		/* TODO: register the PCK for ISI_MCK and set its parent */
+	}
 }
 #else
-void __init at91_add_device_isi(void) {}
+void __init at91_add_device_isi(struct isi_platform_data * data,
+		int use_pck_as_mck) {}
 #endif
 
 
diff --git a/arch/arm/mach-at91/include/mach/board.h b/arch/arm/mach-at91/include/mach/board.h
index ed544a0..731c449 100644
--- a/arch/arm/mach-at91/include/mach/board.h
+++ b/arch/arm/mach-at91/include/mach/board.h
@@ -183,7 +183,9 @@ extern void __init at91_add_device_lcdc(struct atmel_lcdfb_info *data);
 extern void __init at91_add_device_ac97(struct ac97c_platform_data *data);
 
  /* ISI */
-extern void __init at91_add_device_isi(void);
+struct isi_platform_data;
+extern void __init at91_add_device_isi(struct isi_platform_data *data,
+		bool use_pck_as_mck);
 
  /* Touchscreen Controller */
 struct at91_tsadcc_data {
-- 
1.6.3.3

[PATCH v4 3/3] at91: add Atmel ISI and ov2640 support on sam9m10/sam9g45 board

[Josh Wu]

This patch add:
- ov2640 sensor in sam9m10/sam9g45 board.
- support to use PCK as ISI_MCK. PCK's parent is managed in SoC level, e.g. at91sam9g45_devices.c

Signed-off-by: Josh Wu <josh.wu@atmel.com>
---
 arch/arm/mach-at91/at91sam9g45_devices.c |   98 +++++++++++++++++++++++++++++-
 arch/arm/mach-at91/board-sam9m10g45ek.c  |   85 +++++++++++++++++++++++++-
 2 files changed, 180 insertions(+), 3 deletions(-)

diff --git a/arch/arm/mach-at91/at91sam9g45_devices.c b/arch/arm/mach-at91/at91sam9g45_devices.c
index 600bffb..00e6a7f 100644
--- a/arch/arm/mach-at91/at91sam9g45_devices.c
+++ b/arch/arm/mach-at91/at91sam9g45_devices.c
@@ -16,7 +16,7 @@
 #include <linux/platform_device.h>
 #include <linux/i2c-gpio.h>
 #include <linux/atmel-mci.h>
-
+#include <linux/clk.h>
 #include <linux/fb.h>
 #include <video/atmel_lcdc.h>
 
@@ -28,7 +28,10 @@
 #include <mach/at_hdmac.h>
 #include <mach/atmel-mci.h>
 
+#include <media/atmel-isi.h>
+
 #include "generic.h"
+#include "clock.h"
 
 
 /* --------------------------------------------------------------------
@@ -863,6 +866,99 @@ void __init at91_add_device_ac97(struct ac97c_platform_data *data)
 void __init at91_add_device_ac97(struct ac97c_platform_data *data) {}
 #endif
 
+/* --------------------------------------------------------------------
+ *  Image Sensor Interface
+ * -------------------------------------------------------------------- */
+#if defined(CONFIG_VIDEO_ATMEL_ISI) || defined(CONFIG_VIDEO_ATMEL_ISI_MODULE)
+static u64 isi_dmamask = DMA_BIT_MASK(32);
+static struct isi_platform_data isi_data;
+
+struct resource isi_resources[] = {
+	[0] = {
+		.start	= AT91SAM9G45_BASE_ISI,
+		.end	= AT91SAM9G45_BASE_ISI + SZ_16K - 1,
+		.flags	= IORESOURCE_MEM,
+	},
+	[1] = {
+		.start	= AT91SAM9G45_ID_ISI,
+		.end	= AT91SAM9G45_ID_ISI,
+		.flags	= IORESOURCE_IRQ,
+	},
+};
+
+static struct platform_device at91sam9g45_isi_device = {
+	.name		= "atmel_isi",
+	.id		= 0,
+	.dev		= {
+			.dma_mask		= &isi_dmamask,
+			.coherent_dma_mask	= DMA_BIT_MASK(32),
+			.platform_data		= &isi_data,
+	},
+	.resource	= isi_resources,
+	.num_resources	= ARRAY_SIZE(isi_resources),
+};
+
+static struct clk_lookup isi_mck_lookups[] = {
+	CLKDEV_CON_DEV_ID("isi_mck", "atmel_isi.0", NULL),
+};
+
+void __init at91_add_device_isi(struct isi_platform_data * data,
+		bool use_pck_as_mck)
+{
+	struct clk *pck;
+	struct clk *parent;
+
+	if (!data)
+		return;
+	isi_data = *data;
+
+	at91_set_A_periph(AT91_PIN_PB20, 0);	/* ISI_D0 */
+	at91_set_A_periph(AT91_PIN_PB21, 0);	/* ISI_D1 */
+	at91_set_A_periph(AT91_PIN_PB22, 0);	/* ISI_D2 */
+	at91_set_A_periph(AT91_PIN_PB23, 0);	/* ISI_D3 */
+	at91_set_A_periph(AT91_PIN_PB24, 0);	/* ISI_D4 */
+	at91_set_A_periph(AT91_PIN_PB25, 0);	/* ISI_D5 */
+	at91_set_A_periph(AT91_PIN_PB26, 0);	/* ISI_D6 */
+	at91_set_A_periph(AT91_PIN_PB27, 0);	/* ISI_D7 */
+	at91_set_A_periph(AT91_PIN_PB28, 0);	/* ISI_PCK */
+	at91_set_A_periph(AT91_PIN_PB30, 0);	/* ISI_HSYNC */
+	at91_set_A_periph(AT91_PIN_PB29, 0);	/* ISI_VSYNC */
+	at91_set_B_periph(AT91_PIN_PB8, 0);	/* ISI_PD8 */
+	at91_set_B_periph(AT91_PIN_PB9, 0);	/* ISI_PD9 */
+	at91_set_B_periph(AT91_PIN_PB10, 0);	/* ISI_PD10 */
+	at91_set_B_periph(AT91_PIN_PB11, 0);	/* ISI_PD11 */
+
+	platform_device_register(&at91sam9g45_isi_device);
+
+	if (use_pck_as_mck) {
+		at91_set_B_periph(AT91_PIN_PB31, 0);	/* ISI_MCK (PCK1) */
+
+		pck = clk_get(NULL, "pck1");
+		parent = clk_get(NULL, "plla");
+
+		if (IS_ERR(pck) || IS_ERR(parent))
+			BUG();
+
+		if (clk_set_parent(pck, parent))
+			printk(KERN_ERR "Failed to set PCK's parent\n");
+		else {
+			/* Register PCK as ISI_MCK */
+			isi_mck_lookups->clk = pck;
+			clkdev_add_table(isi_mck_lookups,
+					ARRAY_SIZE(isi_mck_lookups));
+		}
+
+		clk_put(pck);
+		clk_put(parent);
+	}
+
+	return;
+}
+#else
+void __init at91_add_device_isi(struct isi_platform_data * data,
+		int use_pck_as_mck) {}
+#endif
+
 
 /* --------------------------------------------------------------------
  *  LCD Controller
diff --git a/arch/arm/mach-at91/board-sam9m10g45ek.c b/arch/arm/mach-at91/board-sam9m10g45ek.c
index ad234cc..068464c 100644
--- a/arch/arm/mach-at91/board-sam9m10g45ek.c
+++ b/arch/arm/mach-at91/board-sam9m10g45ek.c
@@ -23,11 +23,13 @@
 #include <linux/gpio_keys.h>
 #include <linux/input.h>
 #include <linux/leds.h>
-#include <linux/clk.h>
 #include <linux/atmel-mci.h>
+#include <linux/delay.h>
 
 #include <mach/hardware.h>
 #include <video/atmel_lcdc.h>
+#include <media/soc_camera.h>
+#include <media/atmel-isi.h>
 
 #include <asm/setup.h>
 #include <asm/mach-types.h>
@@ -187,6 +189,76 @@ static void __init ek_add_device_nand(void)
 
 
 /*
+ *  ISI
+ */
+static struct isi_platform_data __initdata isi_data = {
+	.frate			= ISI_CFG1_FRATE_CAPTURE_ALL,
+	.has_emb_sync		= 0,
+	.emb_crc_sync		= 0,
+	.hsync_act_low		= 0,
+	.vsync_act_low		= 0,
+	.pclk_act_falling	= 0,
+	/* to use codec and preview path simultaneously */
+	.full_mode		= 1,
+	.data_width_flags	= ISI_DATAWIDTH_8 | ISI_DATAWIDTH_10,
+	/* ISI_MCK is provided by programmable clock or external clock  */
+	.mck_hz			= 25000000,
+};
+
+
+/*
+ * soc-camera OV2640
+ */
+#if defined(CONFIG_SOC_CAMERA_OV2640) || \
+	defined(CONFIG_SOC_CAMERA_OV2640_MODULE)
+static unsigned long isi_camera_query_bus_param(struct soc_camera_link *link)
+{
+	/* ISI board for ek using default 8-bits connection */
+	return SOCAM_DATAWIDTH_8;
+}
+
+static int i2c_camera_power(struct device *dev, int on)
+{
+	/* enable or disable the camera */
+	pr_debug("%s: %s the camera\n", __func__, on ? "ENABLE" : "DISABLE");
+	at91_set_gpio_output(AT91_PIN_PD13, !on);
+
+	if (!on)
+		goto out;
+
+	/* If enabled, give a reset impulse */
+	at91_set_gpio_output(AT91_PIN_PD12, 0);
+	msleep(20);
+	at91_set_gpio_output(AT91_PIN_PD12, 1);
+	msleep(100);
+
+out:
+	return 0;
+}
+
+static struct i2c_board_info i2c_camera = {
+	I2C_BOARD_INFO("ov2640", 0x30),
+};
+
+static struct soc_camera_link iclink_ov2640 = {
+	.bus_id			= 0,
+	.board_info		= &i2c_camera,
+	.i2c_adapter_id		= 0,
+	.power			= i2c_camera_power,
+	.query_bus_param	= isi_camera_query_bus_param,
+};
+
+static struct platform_device isi_ov2640 = {
+	.name	= "soc-camera-pdrv",
+	.id	= 0,
+	.dev	= {
+		.platform_data = &iclink_ov2640,
+	},
+};
+#endif
+
+
+/*
  * LCD Controller
  */
 #if defined(CONFIG_FB_ATMEL) || defined(CONFIG_FB_ATMEL_MODULE)
@@ -378,7 +450,12 @@ static struct gpio_led ek_pwm_led[] = {
 #endif
 };
 
-
+static struct platform_device *devices[] __initdata = {
+#if defined(CONFIG_SOC_CAMERA_OV2640) || \
+	defined(CONFIG_SOC_CAMERA_OV2640_MODULE)
+	&isi_ov2640,
+#endif
+};
 
 static void __init ek_board_init(void)
 {
@@ -400,6 +477,8 @@ static void __init ek_board_init(void)
 	ek_add_device_nand();
 	/* I2C */
 	at91_add_device_i2c(0, NULL, 0);
+	/* ISI, using programmable clock as ISI_MCK */
+	at91_add_device_isi(&isi_data, 1);
 	/* LCD Controller */
 	at91_add_device_lcdc(&ek_lcdc_data);
 	/* Touch Screen */
@@ -411,6 +490,8 @@ static void __init ek_board_init(void)
 	/* LEDs */
 	at91_gpio_leds(ek_leds, ARRAY_SIZE(ek_leds));
 	at91_pwm_leds(ek_pwm_led, ARRAY_SIZE(ek_pwm_led));
+	/* Other platform devices */
+	platform_add_devices(devices, ARRAY_SIZE(devices));
 }
 
 MACHINE_START(AT91SAM9M10G45EK, "Atmel AT91SAM9M10G45-EK")
-- 
1.6.3.3

[PATCH v4 2/3] at91: add parameters for at91_add_device_isi function

[Guennadi Liakhovetski]

On Tue, 11 Oct 2011, Josh Wu wrote:

> This patch add parameters for at91_add_device_isi function
> 
> Signed-off-by: Josh Wu <josh.wu@atmel.com>
> ---
>  arch/arm/mach-at91/at91sam9263_devices.c |   13 ++++++++++---
>  arch/arm/mach-at91/include/mach/board.h  |    4 +++-
>  2 files changed, 13 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/arm/mach-at91/at91sam9263_devices.c b/arch/arm/mach-at91/at91sam9263_devices.c
> index a050f41..7990e8e 100644
> --- a/arch/arm/mach-at91/at91sam9263_devices.c
> +++ b/arch/arm/mach-at91/at91sam9263_devices.c
> @@ -885,7 +885,8 @@ static struct platform_device at91sam9263_isi_device = {
>  	.num_resources	= ARRAY_SIZE(isi_resources),
>  };
>  
> -void __init at91_add_device_isi(void)
> +void __init at91_add_device_isi(struct isi_platform_data * data,

Not sure if anything in CodingStyle prohibits this, but the rest of this 
file uses

static type fn(struct s *p)

i.e., without an extra space after the '*'. Would be better to keep that 
style.

> +		bool use_pck_as_mck)
>  {
>  	at91_set_A_periph(AT91_PIN_PE0, 0);	/* ISI_D0 */
>  	at91_set_A_periph(AT91_PIN_PE1, 0);	/* ISI_D1 */
> @@ -898,14 +899,20 @@ void __init at91_add_device_isi(void)
>  	at91_set_A_periph(AT91_PIN_PE8, 0);	/* ISI_PCK */
>  	at91_set_A_periph(AT91_PIN_PE9, 0);	/* ISI_HSYNC */
>  	at91_set_A_periph(AT91_PIN_PE10, 0);	/* ISI_VSYNC */
> -	at91_set_B_periph(AT91_PIN_PE11, 0);	/* ISI_MCK (PCK3) */
>  	at91_set_B_periph(AT91_PIN_PE12, 0);	/* ISI_PD8 */
>  	at91_set_B_periph(AT91_PIN_PE13, 0);	/* ISI_PD9 */
>  	at91_set_B_periph(AT91_PIN_PE14, 0);	/* ISI_PD10 */
>  	at91_set_B_periph(AT91_PIN_PE15, 0);	/* ISI_PD11 */
> +
> +	if (use_pck_as_mck) {
> +		at91_set_B_periph(AT91_PIN_PE11, 0);	/* ISI_MCK (PCK3) */
> +
> +		/* TODO: register the PCK for ISI_MCK and set its parent */



> +	}
>  }
>  #else
> -void __init at91_add_device_isi(void) {}
> +void __init at91_add_device_isi(struct isi_platform_data * data,

same - no extra space.

> +		int use_pck_as_mck) {}
>  #endif
>  
>  
> diff --git a/arch/arm/mach-at91/include/mach/board.h b/arch/arm/mach-at91/include/mach/board.h
> index ed544a0..731c449 100644
> --- a/arch/arm/mach-at91/include/mach/board.h
> +++ b/arch/arm/mach-at91/include/mach/board.h
> @@ -183,7 +183,9 @@ extern void __init at91_add_device_lcdc(struct atmel_lcdfb_info *data);
>  extern void __init at91_add_device_ac97(struct ac97c_platform_data *data);
>  
>   /* ISI */
> -extern void __init at91_add_device_isi(void);
> +struct isi_platform_data;
> +extern void __init at91_add_device_isi(struct isi_platform_data *data,
> +		bool use_pck_as_mck);
>  
>   /* Touchscreen Controller */
>  struct at91_tsadcc_data {
> -- 
> 1.6.3.3

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

[PATCH v4 3/3] at91: add Atmel ISI and ov2640 support on sam9m10/sam9g45 board

[Guennadi Liakhovetski]

On Tue, 11 Oct 2011, Josh Wu wrote:

> This patch add:
> - ov2640 sensor in sam9m10/sam9g45 board.
> - support to use PCK as ISI_MCK. PCK's parent is managed in SoC level, e.g. at91sam9g45_devices.c
> 
> Signed-off-by: Josh Wu <josh.wu@atmel.com>
> ---
>  arch/arm/mach-at91/at91sam9g45_devices.c |   98 +++++++++++++++++++++++++++++-
>  arch/arm/mach-at91/board-sam9m10g45ek.c  |   85 +++++++++++++++++++++++++-
>  2 files changed, 180 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/arm/mach-at91/at91sam9g45_devices.c b/arch/arm/mach-at91/at91sam9g45_devices.c
> index 600bffb..00e6a7f 100644
> --- a/arch/arm/mach-at91/at91sam9g45_devices.c
> +++ b/arch/arm/mach-at91/at91sam9g45_devices.c
> @@ -16,7 +16,7 @@
>  #include <linux/platform_device.h>
>  #include <linux/i2c-gpio.h>
>  #include <linux/atmel-mci.h>
> -
> +#include <linux/clk.h>
>  #include <linux/fb.h>
>  #include <video/atmel_lcdc.h>
>  
> @@ -28,7 +28,10 @@
>  #include <mach/at_hdmac.h>
>  #include <mach/atmel-mci.h>
>  
> +#include <media/atmel-isi.h>
> +
>  #include "generic.h"
> +#include "clock.h"
>  
>  
>  /* --------------------------------------------------------------------
> @@ -863,6 +866,99 @@ void __init at91_add_device_ac97(struct ac97c_platform_data *data)
>  void __init at91_add_device_ac97(struct ac97c_platform_data *data) {}
>  #endif
>  
> +/* --------------------------------------------------------------------
> + *  Image Sensor Interface
> + * -------------------------------------------------------------------- */
> +#if defined(CONFIG_VIDEO_ATMEL_ISI) || defined(CONFIG_VIDEO_ATMEL_ISI_MODULE)
> +static u64 isi_dmamask = DMA_BIT_MASK(32);
> +static struct isi_platform_data isi_data;
> +
> +struct resource isi_resources[] = {
> +	[0] = {
> +		.start	= AT91SAM9G45_BASE_ISI,
> +		.end	= AT91SAM9G45_BASE_ISI + SZ_16K - 1,
> +		.flags	= IORESOURCE_MEM,
> +	},
> +	[1] = {
> +		.start	= AT91SAM9G45_ID_ISI,
> +		.end	= AT91SAM9G45_ID_ISI,
> +		.flags	= IORESOURCE_IRQ,
> +	},
> +};
> +
> +static struct platform_device at91sam9g45_isi_device = {
> +	.name		= "atmel_isi",
> +	.id		= 0,
> +	.dev		= {
> +			.dma_mask		= &isi_dmamask,
> +			.coherent_dma_mask	= DMA_BIT_MASK(32),
> +			.platform_data		= &isi_data,
> +	},
> +	.resource	= isi_resources,
> +	.num_resources	= ARRAY_SIZE(isi_resources),
> +};
> +
> +static struct clk_lookup isi_mck_lookups[] = {
> +	CLKDEV_CON_DEV_ID("isi_mck", "atmel_isi.0", NULL),
> +};
> +
> +void __init at91_add_device_isi(struct isi_platform_data * data,
> +		bool use_pck_as_mck)

Extra space.

> +{
> +	struct clk *pck;
> +	struct clk *parent;
> +
> +	if (!data)
> +		return;
> +	isi_data = *data;
> +
> +	at91_set_A_periph(AT91_PIN_PB20, 0);	/* ISI_D0 */
> +	at91_set_A_periph(AT91_PIN_PB21, 0);	/* ISI_D1 */
> +	at91_set_A_periph(AT91_PIN_PB22, 0);	/* ISI_D2 */
> +	at91_set_A_periph(AT91_PIN_PB23, 0);	/* ISI_D3 */
> +	at91_set_A_periph(AT91_PIN_PB24, 0);	/* ISI_D4 */
> +	at91_set_A_periph(AT91_PIN_PB25, 0);	/* ISI_D5 */
> +	at91_set_A_periph(AT91_PIN_PB26, 0);	/* ISI_D6 */
> +	at91_set_A_periph(AT91_PIN_PB27, 0);	/* ISI_D7 */
> +	at91_set_A_periph(AT91_PIN_PB28, 0);	/* ISI_PCK */
> +	at91_set_A_periph(AT91_PIN_PB30, 0);	/* ISI_HSYNC */
> +	at91_set_A_periph(AT91_PIN_PB29, 0);	/* ISI_VSYNC */
> +	at91_set_B_periph(AT91_PIN_PB8, 0);	/* ISI_PD8 */
> +	at91_set_B_periph(AT91_PIN_PB9, 0);	/* ISI_PD9 */
> +	at91_set_B_periph(AT91_PIN_PB10, 0);	/* ISI_PD10 */
> +	at91_set_B_periph(AT91_PIN_PB11, 0);	/* ISI_PD11 */
> +
> +	platform_device_register(&at91sam9g45_isi_device);
> +
> +	if (use_pck_as_mck) {
> +		at91_set_B_periph(AT91_PIN_PB31, 0);	/* ISI_MCK (PCK1) */
> +
> +		pck = clk_get(NULL, "pck1");
> +		parent = clk_get(NULL, "plla");
> +
> +		if (IS_ERR(pck) || IS_ERR(parent))
> +			BUG();

Use BUG_ON().

> +
> +		if (clk_set_parent(pck, parent))
> +			printk(KERN_ERR "Failed to set PCK's parent\n");

pr_err()

> +		else {

Missing braces before "else."

> +			/* Register PCK as ISI_MCK */
> +			isi_mck_lookups->clk = pck;

If you're certain, it'll always be a single-element array, you might 
remove the array completely. If you want to have the array, better do

+			isi_mck_lookups[0].clk = pck;

Or even:

+ #define CLK_IDX_ISI_MCK 0

...

+	[CLK_IDX_ISI_MCK] = CLKDEV_CON_DEV_ID("isi_mck", "atmel_isi.0", NULL),

...

+			isi_mck_lookups[CLK_IDX_ISI_MCK].clk = pck;

But in any case I wouldn't use "isi_mck_lookups->clk" with an array.

> +			clkdev_add_table(isi_mck_lookups,
> +					ARRAY_SIZE(isi_mck_lookups));
> +		}
> +
> +		clk_put(pck);
> +		clk_put(parent);
> +	}
> +
> +	return;

No need for "return."

> +}
> +#else
> +void __init at91_add_device_isi(struct isi_platform_data * data,
> +		int use_pck_as_mck) {}

Please use the same prototype - bool. Looks like you haven't tried to 
compile this with CONFIG_VIDEO_ATMEL_ISI(_MODULE) unset. Extra space.

> +#endif
> +
>  
>  /* --------------------------------------------------------------------
>   *  LCD Controller
> diff --git a/arch/arm/mach-at91/board-sam9m10g45ek.c b/arch/arm/mach-at91/board-sam9m10g45ek.c
> index ad234cc..068464c 100644
> --- a/arch/arm/mach-at91/board-sam9m10g45ek.c
> +++ b/arch/arm/mach-at91/board-sam9m10g45ek.c
> @@ -23,11 +23,13 @@
>  #include <linux/gpio_keys.h>
>  #include <linux/input.h>
>  #include <linux/leds.h>
> -#include <linux/clk.h>
>  #include <linux/atmel-mci.h>
> +#include <linux/delay.h>
>  
>  #include <mach/hardware.h>
>  #include <video/atmel_lcdc.h>
> +#include <media/soc_camera.h>
> +#include <media/atmel-isi.h>
>  
>  #include <asm/setup.h>
>  #include <asm/mach-types.h>
> @@ -187,6 +189,76 @@ static void __init ek_add_device_nand(void)
>  
>  
>  /*
> + *  ISI
> + */
> +static struct isi_platform_data __initdata isi_data = {
> +	.frate			= ISI_CFG1_FRATE_CAPTURE_ALL,
> +	.has_emb_sync		= 0,
> +	.emb_crc_sync		= 0,
> +	.hsync_act_low		= 0,
> +	.vsync_act_low		= 0,
> +	.pclk_act_falling	= 0,

I would drop all "= 0"

> +	/* to use codec and preview path simultaneously */
> +	.full_mode		= 1,
> +	.data_width_flags	= ISI_DATAWIDTH_8 | ISI_DATAWIDTH_10,
> +	/* ISI_MCK is provided by programmable clock or external clock  */

Superfluous space before '*/'

> +	.mck_hz			= 25000000,
> +};
> +
> +
> +/*
> + * soc-camera OV2640
> + */
> +#if defined(CONFIG_SOC_CAMERA_OV2640) || \
> +	defined(CONFIG_SOC_CAMERA_OV2640_MODULE)
> +static unsigned long isi_camera_query_bus_param(struct soc_camera_link *link)
> +{
> +	/* ISI board for ek using default 8-bits connection */
> +	return SOCAM_DATAWIDTH_8;
> +}
> +
> +static int i2c_camera_power(struct device *dev, int on)
> +{
> +	/* enable or disable the camera */
> +	pr_debug("%s: %s the camera\n", __func__, on ? "ENABLE" : "DISABLE");
> +	at91_set_gpio_output(AT91_PIN_PD13, !on);
> +
> +	if (!on)
> +		goto out;
> +
> +	/* If enabled, give a reset impulse */
> +	at91_set_gpio_output(AT91_PIN_PD12, 0);
> +	msleep(20);
> +	at91_set_gpio_output(AT91_PIN_PD12, 1);
> +	msleep(100);
> +
> +out:
> +	return 0;
> +}
> +
> +static struct i2c_board_info i2c_camera = {
> +	I2C_BOARD_INFO("ov2640", 0x30),
> +};
> +
> +static struct soc_camera_link iclink_ov2640 = {
> +	.bus_id			= 0,
> +	.board_info		= &i2c_camera,
> +	.i2c_adapter_id		= 0,
> +	.power			= i2c_camera_power,
> +	.query_bus_param	= isi_camera_query_bus_param,
> +};
> +
> +static struct platform_device isi_ov2640 = {
> +	.name	= "soc-camera-pdrv",
> +	.id	= 0,
> +	.dev	= {
> +		.platform_data = &iclink_ov2640,
> +	},
> +};
> +#endif
> +
> +
> +/*
>   * LCD Controller
>   */
>  #if defined(CONFIG_FB_ATMEL) || defined(CONFIG_FB_ATMEL_MODULE)
> @@ -378,7 +450,12 @@ static struct gpio_led ek_pwm_led[] = {
>  #endif
>  };
>  
> -
> +static struct platform_device *devices[] __initdata = {
> +#if defined(CONFIG_SOC_CAMERA_OV2640) || \
> +	defined(CONFIG_SOC_CAMERA_OV2640_MODULE)
> +	&isi_ov2640,
> +#endif
> +};
>  
>  static void __init ek_board_init(void)
>  {
> @@ -400,6 +477,8 @@ static void __init ek_board_init(void)
>  	ek_add_device_nand();
>  	/* I2C */
>  	at91_add_device_i2c(0, NULL, 0);
> +	/* ISI, using programmable clock as ISI_MCK */
> +	at91_add_device_isi(&isi_data, 1);

s/1/true/

>  	/* LCD Controller */
>  	at91_add_device_lcdc(&ek_lcdc_data);
>  	/* Touch Screen */
> @@ -411,6 +490,8 @@ static void __init ek_board_init(void)
>  	/* LEDs */
>  	at91_gpio_leds(ek_leds, ARRAY_SIZE(ek_leds));
>  	at91_pwm_leds(ek_pwm_led, ARRAY_SIZE(ek_pwm_led));
> +	/* Other platform devices */
> +	platform_add_devices(devices, ARRAY_SIZE(devices));
>  }
>  
>  MACHINE_START(AT91SAM9M10G45EK, "Atmel AT91SAM9M10G45-EK")
> -- 
> 1.6.3.3
> 

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

[PATCH v4 1/3] [media] at91: add code to enable/disable ISI_MCK clock

[Guennadi Liakhovetski]

From: Josh Wu <josh.wu@atmel.com>

This patch
- add ISI_MCK clock enable/disable code.
- change field name in isi_platform_data structure

Signed-off-by: Josh Wu <josh.wu@atmel.com>
[g.liakhovetski at gmx.de: fix label names]
Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
---

Josh, I missed a slight hick-up in the previous version of this your 
patch, so, I fixed it myself. Please confirm, that this version is ok with 
you.

Thanks
Guennadi

 drivers/media/video/atmel-isi.c |   31 +++++++++++++++++++++++++++++--
 include/media/atmel-isi.h       |    4 +++-
 2 files changed, 32 insertions(+), 3 deletions(-)

diff --git a/drivers/media/video/atmel-isi.c b/drivers/media/video/atmel-isi.c
index 7b89f00..c42ec6e 100644
--- a/drivers/media/video/atmel-isi.c
+++ b/drivers/media/video/atmel-isi.c
@@ -90,7 +90,10 @@ struct atmel_isi {
 	struct isi_dma_desc		dma_desc[MAX_BUFFER_NUM];
 
 	struct completion		complete;
+	/* ISI peripherial clock */
 	struct clk			*pclk;
+	/* ISI_MCK, feed to camera sensor to generate pixel clock */
+	struct clk			*mck;
 	unsigned int			irq;
 
 	struct isi_platform_data	*pdata;
@@ -763,6 +766,12 @@ static int isi_camera_add_device(struct soc_camera_device *icd)
 	if (ret)
 		return ret;
 
+	ret = clk_enable(isi->mck);
+	if (ret) {
+		clk_disable(isi->pclk);
+		return ret;
+	}
+
 	isi->icd = icd;
 	dev_dbg(icd->parent, "Atmel ISI Camera driver attached to camera %d\n",
 		 icd->devnum);
@@ -776,6 +785,7 @@ static void isi_camera_remove_device(struct soc_camera_device *icd)
 
 	BUG_ON(icd != isi->icd);
 
+	clk_disable(isi->mck);
 	clk_disable(isi->pclk);
 	isi->icd = NULL;
 
@@ -859,7 +869,7 @@ static int isi_camera_set_bus_param(struct soc_camera_device *icd, u32 pixfmt)
 
 	if (isi->pdata->has_emb_sync)
 		cfg1 |= ISI_CFG1_EMB_SYNC;
-	if (isi->pdata->isi_full_mode)
+	if (isi->pdata->full_mode)
 		cfg1 |= ISI_CFG1_FULL_MODE;
 
 	isi_writel(isi, ISI_CTRL, ISI_CTRL_DIS);
@@ -897,6 +907,7 @@ static int __devexit atmel_isi_remove(struct platform_device *pdev)
 			isi->fb_descriptors_phys);
 
 	iounmap(isi->regs);
+	clk_put(isi->mck);
 	clk_put(isi->pclk);
 	kfree(isi);
 
@@ -915,7 +926,7 @@ static int __devinit atmel_isi_probe(struct platform_device *pdev)
 	struct isi_platform_data *pdata;
 
 	pdata = dev->platform_data;
-	if (!pdata || !pdata->data_width_flags) {
+	if (!pdata || !pdata->data_width_flags || !pdata->mck_hz) {
 		dev_err(&pdev->dev,
 			"No config available for Atmel ISI\n");
 		return -EINVAL;
@@ -944,6 +955,19 @@ static int __devinit atmel_isi_probe(struct platform_device *pdev)
 	INIT_LIST_HEAD(&isi->video_buffer_list);
 	INIT_LIST_HEAD(&isi->dma_desc_head);
 
+	/* Get ISI_MCK, provided by programmable clock or external clock */
+	isi->mck = clk_get(dev, "isi_mck");
+	if (IS_ERR_OR_NULL(isi->mck)) {
+		dev_err(dev, "Failed to get isi_mck\n");
+		ret = isi->mck ? PTR_ERR(isi->mck) : -EINVAL;
+		goto err_clk_get;
+	}
+
+	/* Set ISI_MCK's frequency, it should be faster than pixel clock */
+	ret = clk_set_rate(isi->mck, pdata->mck_hz);
+	if (ret < 0)
+		goto err_set_mck_rate;
+
 	isi->p_fb_descriptors = dma_alloc_coherent(&pdev->dev,
 				sizeof(struct fbd) * MAX_BUFFER_NUM,
 				&isi->fb_descriptors_phys,
@@ -1014,6 +1038,9 @@ err_alloc_ctx:
 			isi->p_fb_descriptors,
 			isi->fb_descriptors_phys);
 err_alloc_descriptors:
+err_set_mck_rate:
+	clk_put(isi->mck);
+err_clk_get:
 	kfree(isi);
 err_alloc_isi:
 	clk_put(isi->pclk);
diff --git a/include/media/atmel-isi.h b/include/media/atmel-isi.h
index 26cece5..6568230 100644
--- a/include/media/atmel-isi.h
+++ b/include/media/atmel-isi.h
@@ -110,10 +110,12 @@ struct isi_platform_data {
 	u8 hsync_act_low;
 	u8 vsync_act_low;
 	u8 pclk_act_falling;
-	u8 isi_full_mode;
+	u8 full_mode;
 	u32 data_width_flags;
 	/* Using for ISI_CFG1 */
 	u32 frate;
+	/* Using for ISI_MCK */
+	u32 mck_hz;
 };
 
 #endif /* __ATMEL_ISI_H__ */
-- 
1.7.2.5

[PATCH v4 1/3] [media] at91: add code to enable/disable ISI_MCK clock

[Wu, Josh]

Hi, Guennadi

On Monday, October 17, 2011 11:04 PM, Guennadi Liakhovetski wrote:

> From: Josh Wu <josh.wu@atmel.com>
>
> This patch
> - add ISI_MCK clock enable/disable code.
> - change field name in isi_platform_data structure
>
> Signed-off-by: Josh Wu <josh.wu@atmel.com>
> [g.liakhovetski at gmx.de: fix label names]
> Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> ---

> Josh, I missed a slight hick-up in the previous version of this your 
> patch, so, I fixed it myself. Please confirm, that this version is ok
with 
> you.

> Thanks
> Guennadi

I confirm that the patch is ok for me. Thank you.

Best Regards,
Josh Wu