linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 0/3] Davinci VPSS helper functions for VPFE
@ 2012-11-28 10:55 Prabhakar Lad
  2012-11-28 10:55 ` [PATCH v3 1/3] davinci: vpss: dm365: enable ISP registers Prabhakar Lad
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Prabhakar Lad @ 2012-11-28 10:55 UTC (permalink / raw)
  To: LMML, Mauro Carvalho Chehab
  Cc: LKML, DLOS, Manjunath Hadli, Prabhakar Lad, Laurent Pinchart,
	Sakari Ailus, Hans Verkuil

From: Manjunath Hadli <manjunath.hadli@ti.com>

Hi Mauro,

This patchset, was part of the DM365 capture driver series, I have split
them up and made as separate series as the capture patches will now be going
under staging and made as version V3. This patches have undergone
several reviews along with the capture driver.

I am combining the patches with the pull request so we can get them into the
3.8 kernel. Please pull these patches.If you want a separate pull request,
please let me know.

The following changes since commit c6c22955f80f2db9614b01fe5a3d1cfcd8b3d848:

  [media] dma-mapping: fix dma_common_get_sgtable() conditional compilation (2012-11-27 09:42:31 -0200)

are available in the git repository at:
  git://linuxtv.org/mhadli/v4l-dvb-davinci_devices.git for_mauro

Manjunath Hadli (3):
      davinci: vpss: dm365: enable ISP registers
      davinci: vpss: dm365: set vpss clk ctrl
      davinci: vpss: dm365: add vpss helper functions to be used in the main driver for setting hardware parameters

 drivers/media/platform/davinci/vpss.c |   59 +++++++++++++++++++++++++++++++++
 include/media/davinci/vpss.h          |   16 +++++++++
 2 files changed, 75 insertions(+), 0 deletions(-)


-- 
1.7.4.1


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v3 1/3] davinci: vpss: dm365: enable ISP registers
  2012-11-28 10:55 [PATCH v3 0/3] Davinci VPSS helper functions for VPFE Prabhakar Lad
@ 2012-11-28 10:55 ` Prabhakar Lad
  2012-11-28 20:08   ` Sakari Ailus
  2012-11-28 10:55 ` [PATCH v3 2/3] davinci: vpss: dm365: set vpss clk ctrl Prabhakar Lad
  2012-11-28 10:55 ` [PATCH v3 3/3] davinci: vpss: dm365: add vpss helper functions to be used in the main driver for setting hardware parameters Prabhakar Lad
  2 siblings, 1 reply; 8+ messages in thread
From: Prabhakar Lad @ 2012-11-28 10:55 UTC (permalink / raw)
  To: LMML, Mauro Carvalho Chehab
  Cc: LKML, DLOS, Manjunath Hadli, Prabhakar Lad, Laurent Pinchart,
	Sakari Ailus, Hans Verkuil

From: Manjunath Hadli <manjunath.hadli@ti.com>

enable PPCR, enbale ISIF out on BCR and disable all events to
get the correct operation from ISIF.

Signed-off-by: Manjunath Hadli <manjunath.hadli@ti.com>
Signed-off-by: Lad, Prabhakar <prabhakar.lad@ti.com>
---
 drivers/media/platform/davinci/vpss.c |   13 +++++++++++++
 1 files changed, 13 insertions(+), 0 deletions(-)

diff --git a/drivers/media/platform/davinci/vpss.c b/drivers/media/platform/davinci/vpss.c
index 146e4b0..34ad7bd 100644
--- a/drivers/media/platform/davinci/vpss.c
+++ b/drivers/media/platform/davinci/vpss.c
@@ -52,9 +52,11 @@ MODULE_AUTHOR("Texas Instruments");
 #define DM355_VPSSBL_EVTSEL_DEFAULT	0x4
 
 #define DM365_ISP5_PCCR 		0x04
+#define DM365_ISP5_BCR			0x08
 #define DM365_ISP5_INTSEL1		0x10
 #define DM365_ISP5_INTSEL2		0x14
 #define DM365_ISP5_INTSEL3		0x18
+#define DM365_ISP5_EVTSEL		0x1c
 #define DM365_ISP5_CCDCMUX 		0x20
 #define DM365_ISP5_PG_FRAME_SIZE 	0x28
 #define DM365_VPBE_CLK_CTRL 		0x00
@@ -357,6 +359,10 @@ void dm365_vpss_set_pg_frame_size(struct vpss_pg_frame_size frame_size)
 }
 EXPORT_SYMBOL(dm365_vpss_set_pg_frame_size);
 
+#define DM365_ISP5_EVTSEL_EVT_DISABLE	0x00000000
+#define DM365_ISP5_BCR_ISIF_OUT_ENABLE	0x00000002
+#define DM365_ISP5_PCCR_CLK_ENABLE	0x0000007f
+
 static int __devinit vpss_probe(struct platform_device *pdev)
 {
 	struct resource		*r1, *r2;
@@ -426,9 +432,16 @@ static int __devinit vpss_probe(struct platform_device *pdev)
 		oper_cfg.hw_ops.enable_clock = dm365_enable_clock;
 		oper_cfg.hw_ops.select_ccdc_source = dm365_select_ccdc_source;
 		/* Setup vpss interrupts */
+		isp5_write((isp5_read(DM365_ISP5_PCCR) |
+				DM365_ISP5_PCCR_CLK_ENABLE), DM365_ISP5_PCCR);
+		isp5_write((isp5_read(DM365_ISP5_BCR) |
+			     DM365_ISP5_BCR_ISIF_OUT_ENABLE), DM365_ISP5_BCR);
 		isp5_write(DM365_ISP5_INTSEL1_DEFAULT, DM365_ISP5_INTSEL1);
 		isp5_write(DM365_ISP5_INTSEL2_DEFAULT, DM365_ISP5_INTSEL2);
 		isp5_write(DM365_ISP5_INTSEL3_DEFAULT, DM365_ISP5_INTSEL3);
+		/* No event selected */
+		isp5_write((isp5_read(DM365_ISP5_EVTSEL) |
+			DM365_ISP5_EVTSEL_EVT_DISABLE), DM365_ISP5_EVTSEL);
 	} else
 		oper_cfg.hw_ops.clear_wbl_overflow = dm644x_clear_wbl_overflow;
 
-- 
1.7.4.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH v3 2/3] davinci: vpss: dm365: set vpss clk ctrl
  2012-11-28 10:55 [PATCH v3 0/3] Davinci VPSS helper functions for VPFE Prabhakar Lad
  2012-11-28 10:55 ` [PATCH v3 1/3] davinci: vpss: dm365: enable ISP registers Prabhakar Lad
@ 2012-11-28 10:55 ` Prabhakar Lad
  2012-11-28 20:18   ` Sakari Ailus
  2012-11-28 10:55 ` [PATCH v3 3/3] davinci: vpss: dm365: add vpss helper functions to be used in the main driver for setting hardware parameters Prabhakar Lad
  2 siblings, 1 reply; 8+ messages in thread
From: Prabhakar Lad @ 2012-11-28 10:55 UTC (permalink / raw)
  To: LMML, Mauro Carvalho Chehab
  Cc: LKML, DLOS, Manjunath Hadli, Prabhakar Lad, Laurent Pinchart,
	Sakari Ailus, Hans Verkuil

From: Manjunath Hadli <manjunath.hadli@ti.com>

request_mem_region for VPSS_CLK_CTRL register and ioremap.
and enable clocks appropriately.

Signed-off-by: Manjunath Hadli <manjunath.hadli@ti.com>
Signed-off-by: Lad, Prabhakar <prabhakar.lad@ti.com>
---
 drivers/media/platform/davinci/vpss.c |   14 ++++++++++++++
 1 files changed, 14 insertions(+), 0 deletions(-)

diff --git a/drivers/media/platform/davinci/vpss.c b/drivers/media/platform/davinci/vpss.c
index 34ad7bd..a36d694 100644
--- a/drivers/media/platform/davinci/vpss.c
+++ b/drivers/media/platform/davinci/vpss.c
@@ -103,6 +103,7 @@ struct vpss_hw_ops {
 struct vpss_oper_config {
 	__iomem void *vpss_regs_base0;
 	__iomem void *vpss_regs_base1;
+	resource_size_t *vpss_regs_base2;
 	enum vpss_platform_type platform;
 	spinlock_t vpss_lock;
 	struct vpss_hw_ops hw_ops;
@@ -484,11 +485,24 @@ static struct platform_driver vpss_driver = {
 
 static void vpss_exit(void)
 {
+	iounmap(oper_cfg.vpss_regs_base2);
+	release_mem_region(*oper_cfg.vpss_regs_base2, 4);
 	platform_driver_unregister(&vpss_driver);
 }
 
+#define VPSS_CLK_CTRL			0x01c40044
+#define VPSS_CLK_CTRL_VENCCLKEN		BIT(3)
+#define VPSS_CLK_CTRL_DACCLKEN		BIT(4)
+
 static int __init vpss_init(void)
 {
+	if (request_mem_region(VPSS_CLK_CTRL, 4, "vpss_clock_control")) {
+		oper_cfg.vpss_regs_base2 = ioremap(VPSS_CLK_CTRL, 4);
+		__raw_writel(VPSS_CLK_CTRL_VENCCLKEN |
+			     VPSS_CLK_CTRL_DACCLKEN, oper_cfg.vpss_regs_base2);
+	} else {
+		return -EBUSY;
+	}
 	return platform_driver_register(&vpss_driver);
 }
 subsys_initcall(vpss_init);
-- 
1.7.4.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH v3 3/3] davinci: vpss: dm365: add vpss helper functions to be used in the main driver for setting hardware parameters
  2012-11-28 10:55 [PATCH v3 0/3] Davinci VPSS helper functions for VPFE Prabhakar Lad
  2012-11-28 10:55 ` [PATCH v3 1/3] davinci: vpss: dm365: enable ISP registers Prabhakar Lad
  2012-11-28 10:55 ` [PATCH v3 2/3] davinci: vpss: dm365: set vpss clk ctrl Prabhakar Lad
@ 2012-11-28 10:55 ` Prabhakar Lad
  2 siblings, 0 replies; 8+ messages in thread
From: Prabhakar Lad @ 2012-11-28 10:55 UTC (permalink / raw)
  To: LMML, Mauro Carvalho Chehab
  Cc: LKML, DLOS, Manjunath Hadli, Prabhakar Lad, Laurent Pinchart,
	Sakari Ailus, Hans Verkuil

From: Manjunath Hadli <manjunath.hadli@ti.com>

add interface functions to set sync polarity, interrupt
completion and pageframe size in vpss to be used by the main driver.

Signed-off-by: Manjunath Hadli <manjunath.hadli@ti.com>
Signed-off-by: Lad, Prabhakar <prabhakar.lad@ti.com>
---
 drivers/media/platform/davinci/vpss.c |   32 ++++++++++++++++++++++++++++++++
 include/media/davinci/vpss.h          |   16 ++++++++++++++++
 2 files changed, 48 insertions(+), 0 deletions(-)

diff --git a/drivers/media/platform/davinci/vpss.c b/drivers/media/platform/davinci/vpss.c
index a36d694..4b7c7ecc 100644
--- a/drivers/media/platform/davinci/vpss.c
+++ b/drivers/media/platform/davinci/vpss.c
@@ -97,6 +97,12 @@ struct vpss_hw_ops {
 	void (*select_ccdc_source)(enum vpss_ccdc_source_sel src_sel);
 	/* clear wbl overflow bit */
 	int (*clear_wbl_overflow)(enum vpss_wbl_sel wbl_sel);
+	/* set sync polarity */
+	void (*set_sync_pol)(struct vpss_sync_pol);
+	/* set the PG_FRAME_SIZE register*/
+	void (*set_pg_frame_size)(struct vpss_pg_frame_size);
+	/* check and clear interrupt if occured */
+	int (*dma_complete_interrupt)(void);
 };
 
 /* vpss configuration */
@@ -161,6 +167,14 @@ static void dm355_select_ccdc_source(enum vpss_ccdc_source_sel src_sel)
 	bl_regw(src_sel << VPSS_HSSISEL_SHIFT, DM355_VPSSBL_CCDCMUX);
 }
 
+int vpss_dma_complete_interrupt(void)
+{
+	if (!oper_cfg.hw_ops.dma_complete_interrupt)
+		return 2;
+	return oper_cfg.hw_ops.dma_complete_interrupt();
+}
+EXPORT_SYMBOL(vpss_dma_complete_interrupt);
+
 int vpss_select_ccdc_source(enum vpss_ccdc_source_sel src_sel)
 {
 	if (!oper_cfg.hw_ops.select_ccdc_source)
@@ -186,6 +200,15 @@ static int dm644x_clear_wbl_overflow(enum vpss_wbl_sel wbl_sel)
 	return 0;
 }
 
+void vpss_set_sync_pol(struct vpss_sync_pol sync)
+{
+	if (!oper_cfg.hw_ops.set_sync_pol)
+		return;
+
+	oper_cfg.hw_ops.set_sync_pol(sync);
+}
+EXPORT_SYMBOL(vpss_set_sync_pol);
+
 int vpss_clear_wbl_overflow(enum vpss_wbl_sel wbl_sel)
 {
 	if (!oper_cfg.hw_ops.clear_wbl_overflow)
@@ -351,6 +374,15 @@ void dm365_vpss_set_sync_pol(struct vpss_sync_pol sync)
 }
 EXPORT_SYMBOL(dm365_vpss_set_sync_pol);
 
+void vpss_set_pg_frame_size(struct vpss_pg_frame_size frame_size)
+{
+	if (!oper_cfg.hw_ops.set_pg_frame_size)
+		return;
+
+	oper_cfg.hw_ops.set_pg_frame_size(frame_size);
+}
+EXPORT_SYMBOL(vpss_set_pg_frame_size);
+
 void dm365_vpss_set_pg_frame_size(struct vpss_pg_frame_size frame_size)
 {
 	int current_reg = ((frame_size.hlpfr >> 1) - 1) << 16;
diff --git a/include/media/davinci/vpss.h b/include/media/davinci/vpss.h
index b586495..153473d 100644
--- a/include/media/davinci/vpss.h
+++ b/include/media/davinci/vpss.h
@@ -105,4 +105,20 @@ enum vpss_wbl_sel {
 };
 /* clear wbl overflow flag for DM6446 */
 int vpss_clear_wbl_overflow(enum vpss_wbl_sel wbl_sel);
+
+/* set sync polarity*/
+void vpss_set_sync_pol(struct vpss_sync_pol sync);
+/* set the PG_FRAME_SIZE register */
+void vpss_set_pg_frame_size(struct vpss_pg_frame_size frame_size);
+/*
+ * vpss_check_and_clear_interrupt - check and clear interrupt
+ * @irq - common enumerator for IRQ
+ *
+ * Following return values used:-
+ * 0 - interrupt occurred and cleared
+ * 1 - interrupt not occurred
+ * 2 - interrupt status not available
+ */
+int vpss_dma_complete_interrupt(void);
+
 #endif
-- 
1.7.4.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH v3 1/3] davinci: vpss: dm365: enable ISP registers
  2012-11-28 10:55 ` [PATCH v3 1/3] davinci: vpss: dm365: enable ISP registers Prabhakar Lad
@ 2012-11-28 20:08   ` Sakari Ailus
  2012-11-29  2:46     ` Prabhakar Lad
  0 siblings, 1 reply; 8+ messages in thread
From: Sakari Ailus @ 2012-11-28 20:08 UTC (permalink / raw)
  To: Prabhakar Lad
  Cc: LMML, Mauro Carvalho Chehab, LKML, DLOS, Manjunath Hadli,
	Prabhakar Lad, Laurent Pinchart, Hans Verkuil

On Wed, Nov 28, 2012 at 04:25:32PM +0530, Prabhakar Lad wrote:
> From: Manjunath Hadli <manjunath.hadli@ti.com>
> 
> enable PPCR, enbale ISIF out on BCR and disable all events to
> get the correct operation from ISIF.
> 
> Signed-off-by: Manjunath Hadli <manjunath.hadli@ti.com>
> Signed-off-by: Lad, Prabhakar <prabhakar.lad@ti.com>
> ---
>  drivers/media/platform/davinci/vpss.c |   13 +++++++++++++
>  1 files changed, 13 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/media/platform/davinci/vpss.c b/drivers/media/platform/davinci/vpss.c
> index 146e4b0..34ad7bd 100644
> --- a/drivers/media/platform/davinci/vpss.c
> +++ b/drivers/media/platform/davinci/vpss.c
> @@ -52,9 +52,11 @@ MODULE_AUTHOR("Texas Instruments");
>  #define DM355_VPSSBL_EVTSEL_DEFAULT	0x4
>  
>  #define DM365_ISP5_PCCR 		0x04
> +#define DM365_ISP5_BCR			0x08
>  #define DM365_ISP5_INTSEL1		0x10
>  #define DM365_ISP5_INTSEL2		0x14
>  #define DM365_ISP5_INTSEL3		0x18
> +#define DM365_ISP5_EVTSEL		0x1c
>  #define DM365_ISP5_CCDCMUX 		0x20
>  #define DM365_ISP5_PG_FRAME_SIZE 	0x28
>  #define DM365_VPBE_CLK_CTRL 		0x00
> @@ -357,6 +359,10 @@ void dm365_vpss_set_pg_frame_size(struct vpss_pg_frame_size frame_size)
>  }
>  EXPORT_SYMBOL(dm365_vpss_set_pg_frame_size);
>  
> +#define DM365_ISP5_EVTSEL_EVT_DISABLE	0x00000000
> +#define DM365_ISP5_BCR_ISIF_OUT_ENABLE	0x00000002
> +#define DM365_ISP5_PCCR_CLK_ENABLE	0x0000007f
> +

How about defining these next to the register definitions themselves? There
also seems to be one EVTSEL bit defined in the beginning of the first chunk.
Please group these.

For that matter --- IMO you could just write zero to the register, without
defining it a name, if the purpose of the register is to select something
based on bits that are set.

>  static int __devinit vpss_probe(struct platform_device *pdev)
>  {
>  	struct resource		*r1, *r2;
> @@ -426,9 +432,16 @@ static int __devinit vpss_probe(struct platform_device *pdev)
>  		oper_cfg.hw_ops.enable_clock = dm365_enable_clock;
>  		oper_cfg.hw_ops.select_ccdc_source = dm365_select_ccdc_source;
>  		/* Setup vpss interrupts */
> +		isp5_write((isp5_read(DM365_ISP5_PCCR) |
> +				DM365_ISP5_PCCR_CLK_ENABLE), DM365_ISP5_PCCR);
> +		isp5_write((isp5_read(DM365_ISP5_BCR) |
> +			     DM365_ISP5_BCR_ISIF_OUT_ENABLE), DM365_ISP5_BCR);
>  		isp5_write(DM365_ISP5_INTSEL1_DEFAULT, DM365_ISP5_INTSEL1);
>  		isp5_write(DM365_ISP5_INTSEL2_DEFAULT, DM365_ISP5_INTSEL2);
>  		isp5_write(DM365_ISP5_INTSEL3_DEFAULT, DM365_ISP5_INTSEL3);
> +		/* No event selected */
> +		isp5_write((isp5_read(DM365_ISP5_EVTSEL) |
> +			DM365_ISP5_EVTSEL_EVT_DISABLE), DM365_ISP5_EVTSEL);

What's this? You're reading the value of the register, orring that with zero
and writing it back? :-)

>  	} else
>  		oper_cfg.hw_ops.clear_wbl_overflow = dm644x_clear_wbl_overflow;
>  

-- 
Regards,

Sakari Ailus
e-mail: sakari.ailus@iki.fi	XMPP: sailus@retiisi.org.uk

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v3 2/3] davinci: vpss: dm365: set vpss clk ctrl
  2012-11-28 10:55 ` [PATCH v3 2/3] davinci: vpss: dm365: set vpss clk ctrl Prabhakar Lad
@ 2012-11-28 20:18   ` Sakari Ailus
  2012-11-29  1:38     ` Prabhakar Lad
  0 siblings, 1 reply; 8+ messages in thread
From: Sakari Ailus @ 2012-11-28 20:18 UTC (permalink / raw)
  To: Prabhakar Lad
  Cc: LMML, Mauro Carvalho Chehab, LKML, DLOS, Manjunath Hadli,
	Prabhakar Lad, Laurent Pinchart, Hans Verkuil

Hi Prabhakar,

On Wed, Nov 28, 2012 at 04:25:33PM +0530, Prabhakar Lad wrote:
> From: Manjunath Hadli <manjunath.hadli@ti.com>
> 
> request_mem_region for VPSS_CLK_CTRL register and ioremap.
> and enable clocks appropriately.
> 
> Signed-off-by: Manjunath Hadli <manjunath.hadli@ti.com>
> Signed-off-by: Lad, Prabhakar <prabhakar.lad@ti.com>
> ---
>  drivers/media/platform/davinci/vpss.c |   14 ++++++++++++++
>  1 files changed, 14 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/media/platform/davinci/vpss.c b/drivers/media/platform/davinci/vpss.c
> index 34ad7bd..a36d694 100644
> --- a/drivers/media/platform/davinci/vpss.c
> +++ b/drivers/media/platform/davinci/vpss.c
> @@ -103,6 +103,7 @@ struct vpss_hw_ops {
>  struct vpss_oper_config {
>  	__iomem void *vpss_regs_base0;
>  	__iomem void *vpss_regs_base1;
> +	resource_size_t *vpss_regs_base2;
>  	enum vpss_platform_type platform;
>  	spinlock_t vpss_lock;
>  	struct vpss_hw_ops hw_ops;
> @@ -484,11 +485,24 @@ static struct platform_driver vpss_driver = {
>  
>  static void vpss_exit(void)
>  {
> +	iounmap(oper_cfg.vpss_regs_base2);
> +	release_mem_region(*oper_cfg.vpss_regs_base2, 4);

release_mem_region(VPSS_CLK_CTRL, 4);?

>  	platform_driver_unregister(&vpss_driver);
>  }
>  
> +#define VPSS_CLK_CTRL			0x01c40044
> +#define VPSS_CLK_CTRL_VENCCLKEN		BIT(3)
> +#define VPSS_CLK_CTRL_DACCLKEN		BIT(4)
> +
>  static int __init vpss_init(void)
>  {
> +	if (request_mem_region(VPSS_CLK_CTRL, 4, "vpss_clock_control")) {

if (!request_mem_region())
	return -EBUSY;

...

> +		oper_cfg.vpss_regs_base2 = ioremap(VPSS_CLK_CTRL, 4);
> +		__raw_writel(VPSS_CLK_CTRL_VENCCLKEN |
> +			     VPSS_CLK_CTRL_DACCLKEN, oper_cfg.vpss_regs_base2);
> +	} else {
> +		return -EBUSY;
> +	}
>  	return platform_driver_register(&vpss_driver);
>  }
>  subsys_initcall(vpss_init);

-- 
Sakari Ailus
e-mail: sakari.ailus@iki.fi	XMPP: sailus@retiisi.org.uk

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v3 2/3] davinci: vpss: dm365: set vpss clk ctrl
  2012-11-28 20:18   ` Sakari Ailus
@ 2012-11-29  1:38     ` Prabhakar Lad
  0 siblings, 0 replies; 8+ messages in thread
From: Prabhakar Lad @ 2012-11-29  1:38 UTC (permalink / raw)
  To: Sakari Ailus
  Cc: LMML, Mauro Carvalho Chehab, LKML, DLOS, Manjunath Hadli,
	Prabhakar Lad, Laurent Pinchart, Hans Verkuil

Hi Sakari,

Thanks for the quick review.

On Thu, Nov 29, 2012 at 1:48 AM, Sakari Ailus <sakari.ailus@iki.fi> wrote:
> Hi Prabhakar,
>
> On Wed, Nov 28, 2012 at 04:25:33PM +0530, Prabhakar Lad wrote:
>> From: Manjunath Hadli <manjunath.hadli@ti.com>
>>
>> request_mem_region for VPSS_CLK_CTRL register and ioremap.
>> and enable clocks appropriately.
>>
>> Signed-off-by: Manjunath Hadli <manjunath.hadli@ti.com>
>> Signed-off-by: Lad, Prabhakar <prabhakar.lad@ti.com>
>> ---
>>  drivers/media/platform/davinci/vpss.c |   14 ++++++++++++++
>>  1 files changed, 14 insertions(+), 0 deletions(-)
>>
>> diff --git a/drivers/media/platform/davinci/vpss.c b/drivers/media/platform/davinci/vpss.c
>> index 34ad7bd..a36d694 100644
>> --- a/drivers/media/platform/davinci/vpss.c
>> +++ b/drivers/media/platform/davinci/vpss.c
>> @@ -103,6 +103,7 @@ struct vpss_hw_ops {
>>  struct vpss_oper_config {
>>       __iomem void *vpss_regs_base0;
>>       __iomem void *vpss_regs_base1;
>> +     resource_size_t *vpss_regs_base2;
>>       enum vpss_platform_type platform;
>>       spinlock_t vpss_lock;
>>       struct vpss_hw_ops hw_ops;
>> @@ -484,11 +485,24 @@ static struct platform_driver vpss_driver = {
>>
>>  static void vpss_exit(void)
>>  {
>> +     iounmap(oper_cfg.vpss_regs_base2);
>> +     release_mem_region(*oper_cfg.vpss_regs_base2, 4);
>
> release_mem_region(VPSS_CLK_CTRL, 4);?
>
Ok.

>>       platform_driver_unregister(&vpss_driver);
>>  }
>>
>> +#define VPSS_CLK_CTRL                        0x01c40044
>> +#define VPSS_CLK_CTRL_VENCCLKEN              BIT(3)
>> +#define VPSS_CLK_CTRL_DACCLKEN               BIT(4)
>> +
>>  static int __init vpss_init(void)
>>  {
>> +     if (request_mem_region(VPSS_CLK_CTRL, 4, "vpss_clock_control")) {
>
> if (!request_mem_region())
>         return -EBUSY;
>
Ok makes sense returning early on failure.

Regards,
--Prabhakar

> ...
>
>> +             oper_cfg.vpss_regs_base2 = ioremap(VPSS_CLK_CTRL, 4);
>> +             __raw_writel(VPSS_CLK_CTRL_VENCCLKEN |
>> +                          VPSS_CLK_CTRL_DACCLKEN, oper_cfg.vpss_regs_base2);
>> +     } else {
>> +             return -EBUSY;
>> +     }
>>       return platform_driver_register(&vpss_driver);
>>  }
>>  subsys_initcall(vpss_init);
>
> --
> Sakari Ailus
> e-mail: sakari.ailus@iki.fi     XMPP: sailus@retiisi.org.uk

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v3 1/3] davinci: vpss: dm365: enable ISP registers
  2012-11-28 20:08   ` Sakari Ailus
@ 2012-11-29  2:46     ` Prabhakar Lad
  0 siblings, 0 replies; 8+ messages in thread
From: Prabhakar Lad @ 2012-11-29  2:46 UTC (permalink / raw)
  To: Sakari Ailus
  Cc: LMML, Mauro Carvalho Chehab, LKML, DLOS, Manjunath Hadli,
	Prabhakar Lad, Laurent Pinchart, Hans Verkuil

Hi Sakari,

Thanks for the quick review.

On Thu, Nov 29, 2012 at 1:38 AM, Sakari Ailus <sakari.ailus@iki.fi> wrote:
> On Wed, Nov 28, 2012 at 04:25:32PM +0530, Prabhakar Lad wrote:
>> From: Manjunath Hadli <manjunath.hadli@ti.com>
>>
>> enable PPCR, enbale ISIF out on BCR and disable all events to
>> get the correct operation from ISIF.
>>
>> Signed-off-by: Manjunath Hadli <manjunath.hadli@ti.com>
>> Signed-off-by: Lad, Prabhakar <prabhakar.lad@ti.com>
>> ---
>>  drivers/media/platform/davinci/vpss.c |   13 +++++++++++++
>>  1 files changed, 13 insertions(+), 0 deletions(-)
>>
>> diff --git a/drivers/media/platform/davinci/vpss.c b/drivers/media/platform/davinci/vpss.c
>> index 146e4b0..34ad7bd 100644
>> --- a/drivers/media/platform/davinci/vpss.c
>> +++ b/drivers/media/platform/davinci/vpss.c
>> @@ -52,9 +52,11 @@ MODULE_AUTHOR("Texas Instruments");
>>  #define DM355_VPSSBL_EVTSEL_DEFAULT  0x4
>>
>>  #define DM365_ISP5_PCCR              0x04
>> +#define DM365_ISP5_BCR                       0x08
>>  #define DM365_ISP5_INTSEL1           0x10
>>  #define DM365_ISP5_INTSEL2           0x14
>>  #define DM365_ISP5_INTSEL3           0x18
>> +#define DM365_ISP5_EVTSEL            0x1c
>>  #define DM365_ISP5_CCDCMUX           0x20
>>  #define DM365_ISP5_PG_FRAME_SIZE     0x28
>>  #define DM365_VPBE_CLK_CTRL          0x00
>> @@ -357,6 +359,10 @@ void dm365_vpss_set_pg_frame_size(struct vpss_pg_frame_size frame_size)
>>  }
>>  EXPORT_SYMBOL(dm365_vpss_set_pg_frame_size);
>>
>> +#define DM365_ISP5_EVTSEL_EVT_DISABLE        0x00000000
>> +#define DM365_ISP5_BCR_ISIF_OUT_ENABLE       0x00000002
>> +#define DM365_ISP5_PCCR_CLK_ENABLE   0x0000007f
>> +
>
> How about defining these next to the register definitions themselves? There
> also seems to be one EVTSEL bit defined in the beginning of the first chunk.
> Please group these.
>
Ok I'll move them to the top. These are bit definitions, maybe using BIT()
could be better ?

> For that matter --- IMO you could just write zero to the register, without
> defining it a name, if the purpose of the register is to select something
> based on bits that are set.
>
Ok.

>>  static int __devinit vpss_probe(struct platform_device *pdev)
>>  {
>>       struct resource         *r1, *r2;
>> @@ -426,9 +432,16 @@ static int __devinit vpss_probe(struct platform_device *pdev)
>>               oper_cfg.hw_ops.enable_clock = dm365_enable_clock;
>>               oper_cfg.hw_ops.select_ccdc_source = dm365_select_ccdc_source;
>>               /* Setup vpss interrupts */
>> +             isp5_write((isp5_read(DM365_ISP5_PCCR) |
>> +                             DM365_ISP5_PCCR_CLK_ENABLE), DM365_ISP5_PCCR);
>> +             isp5_write((isp5_read(DM365_ISP5_BCR) |
>> +                          DM365_ISP5_BCR_ISIF_OUT_ENABLE), DM365_ISP5_BCR);
>>               isp5_write(DM365_ISP5_INTSEL1_DEFAULT, DM365_ISP5_INTSEL1);
>>               isp5_write(DM365_ISP5_INTSEL2_DEFAULT, DM365_ISP5_INTSEL2);
>>               isp5_write(DM365_ISP5_INTSEL3_DEFAULT, DM365_ISP5_INTSEL3);
>> +             /* No event selected */
>> +             isp5_write((isp5_read(DM365_ISP5_EVTSEL) |
>> +                     DM365_ISP5_EVTSEL_EVT_DISABLE), DM365_ISP5_EVTSEL);
>
> What's this? You're reading the value of the register, orring that with zero
> and writing it back? :-)
>
My Bad I'll remove it.

Regards,
--Prabhakar

>>       } else
>>               oper_cfg.hw_ops.clear_wbl_overflow = dm644x_clear_wbl_overflow;
>>
>
> --
> Regards,
>
> Sakari Ailus
> e-mail: sakari.ailus@iki.fi     XMPP: sailus@retiisi.org.uk

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2012-11-29  2:46 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-11-28 10:55 [PATCH v3 0/3] Davinci VPSS helper functions for VPFE Prabhakar Lad
2012-11-28 10:55 ` [PATCH v3 1/3] davinci: vpss: dm365: enable ISP registers Prabhakar Lad
2012-11-28 20:08   ` Sakari Ailus
2012-11-29  2:46     ` Prabhakar Lad
2012-11-28 10:55 ` [PATCH v3 2/3] davinci: vpss: dm365: set vpss clk ctrl Prabhakar Lad
2012-11-28 20:18   ` Sakari Ailus
2012-11-29  1:38     ` Prabhakar Lad
2012-11-28 10:55 ` [PATCH v3 3/3] davinci: vpss: dm365: add vpss helper functions to be used in the main driver for setting hardware parameters Prabhakar Lad

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).