Re: [PATCH 1/2] [media] atmel-isc: add the Image Sensor Controller code

2016-04-19 Thread Wu, Songjun 



On 4/15/2016 00:21, Laurent Pinchart wrote:

>+   return -EINVAL;
>+
>+   parent_names = kcalloc(num_parents, sizeof(char *), GFP_KERNEL);
>+   if (!parent_names)
>+   return -ENOMEM;
>+
>+   of_clk_parent_fill(np, parent_names, num_parents);
>+
>+   init.parent_names   = parent_names;
>+   init.num_parents= num_parents;
>+   init.name   = clk_name;
>+   init.ops= _clk_ops;
>+   init.flags  = CLK_SET_RATE_GATE | CLK_SET_PARENT_GATE;
>+
>+   isc_clk = >isc_clks[id];
>+   isc_clk->hw.init = 
>+   isc_clk->regmap  = regmap;
>+   isc_clk->lock= lock;
>+   isc_clk->id  = id;
>+
>+   isc_clk->clk = clk_register(NULL, _clk->hw);
>+   if (!IS_ERR(isc_clk->clk))
>+   of_clk_add_provider(np, of_clk_src_simple_get, isc_clk->clk);
>+   else {
>+   dev_err(isc->dev, "%s: clock register fail\n", clk_name);
>+   ret = PTR_ERR(isc_clk->clk);
>+   goto free_parent_names;
>+   }
>+
>+free_parent_names:
>+   kfree(parent_names);
>+   return ret;
>+}
>+
>+static int isc_clk_init(struct isc_device *isc)
>+{
>+   struct device_node *np = of_get_child_by_name(isc->dev->of_node,
>+ "clk_in_isc");

Do you really need the clk_in_isc DT node ? I would have assumed that the
clock topology inside the ISC is fixed, and that it would be enough to just
specify the three parent clocks in the ISC DT node and create the two internal
clocks in the driver without needing a DT description.


Hi Laurent,

I think more, and the clk_in_isc DT node should be needed. The clock 
topology inside the ISC is fixed, but isc will provide the clock to 
sensor, we need create the corresponding clock node int DT file, then 
the sensor will get this clock and set the clock rate in DT file.


--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] [media] atmel-isc: add the Image Sensor Controller code

2016-04-19 Thread Nicolas Ferre 
Le 19/04/2016 09:46, Wu, Songjun a écrit :
> 
> 
> On 4/15/2016 00:21, Laurent Pinchart wrote:
>> Hello Songjun,
>>
>> Thank you for the patch.
>>
>> On Wednesday 13 Apr 2016 15:44:19 Songjun Wu wrote:
>>> Add driver for the Image Sensor Controller. It manages
>>> incoming data from a parallel based CMOS/CCD sensor.
>>> It has an internal image processor, also integrates a
>>> triple channel direct memory access controller master
>>> interface.
>>>
>>> Signed-off-by: Songjun Wu 

I add some tiny comments above...


>>> ---
>>>
>>>   drivers/media/platform/Kconfig|1 +
>>>   drivers/media/platform/Makefile   |2 +
>>>   drivers/media/platform/atmel/Kconfig  |9 +
>>>   drivers/media/platform/atmel/Makefile |3 +
>>>   drivers/media/platform/atmel/atmel-isc-regs.h |  280 +
>>>   drivers/media/platform/atmel/atmel-isc.c  | 1537 
>>> ++
>>>   6 files changed, 1832 insertions(+)
>>>   create mode 100644 drivers/media/platform/atmel/Kconfig
>>>   create mode 100644 drivers/media/platform/atmel/Makefile
>>>   create mode 100644 drivers/media/platform/atmel/atmel-isc-regs.h
>>>   create mode 100644 drivers/media/platform/atmel/atmel-isc.c

[..]

>>> +static int isc_clk_enable(struct clk_hw *hw)
>>> +{
>>> +   struct isc_clk *isc_clk = to_isc_clk(hw);
>>> +   u32 id = isc_clk->id;
>>> +   struct regmap *regmap = isc_clk->regmap;
>>> +   unsigned long flags;
>>> +   u32 sr_val;
>>> +
>>> +   pr_debug("ISC CLK: %s, div = %d, parent id = %d\n",
>>> +__func__, isc_clk->div, isc_clk->parent_id);
>>
>> Please use dev_dbg() instead of pr_debug(). Same comment for all the other
>> pr_*() calls below.
>>
> Accept, thank you.
> 
>>> +   spin_lock_irqsave(isc_clk->lock, flags);
>>> +
>>> +   regmap_update_bits(regmap, ISC_CLKCFG,
>>> +  ISC_CLKCFG_DIV_MASK(id) | ISC_CLKCFG_SEL_MASK(id),
>>> +  (isc_clk->div << ISC_CLKCFG_DIV_SHIFT(id)) |
>>> +  (isc_clk->parent_id << ISC_CLKCFG_SEL_SHIFT(id)));
>>> +
>>> +   regmap_read(regmap, ISC_CLKSR, _val);
>>> +   while (sr_val & ISC_CLKSR_SIP_PROGRESS) {
>>> +   cpu_relax();
>>> +   regmap_read(regmap, ISC_CLKSR, _val);
>>> +   }
>>
>> A busy loop while holding a spinlock ? Ouch... You should at least set a
>> higher bound on the number of iterations.
>>
> Accept, after the clock register is written, we need wait the SIP flag 
> before the clock register is written again, the time is very short.
> I think the number of iterations will be added when the loop is under 
> spinlock.
> Thank you.

Yes, I have the same feeling as Laurent, busy loop must be avoided while
holding a spinlock. Even outside a critical section, a way-out must
always be added (timeout, number of iterations, etc.).

There is a nice conference summary by Wolfram about that here:

http://elinux.org/Session:_Developer's_Diary:_It's_About_Time_ELCE_2011

Slides here:
http://elinux.org/images/5/54/Elce11_sang.pdf
The slide named "timeout #4" gives a comprehensive timeout function with
all the needed tricks to make it rock solid. You can also check my use
of this code template here:

http://lxr.free-electrons.com/source/drivers/net/ethernet/cadence/macb.c#L503


>>> +   regmap_update_bits(regmap, ISC_CLKEN,
>>> +  ISC_CLKEN_EN_MASK(id),
>>> +  ISC_CLKEN_EN << ISC_CLKEN_EN_SHIFT(id));
>>> +
>>> +   spin_unlock_irqrestore(isc_clk->lock, flags);
>>> +
>>> +   return 0;
>>> +}

[..]

>>> +static void isc_stop_streaming(struct vb2_queue *vq)
>>> +{
>>> +   struct isc_device *isc = vb2_get_drv_priv(vq);
>>> +   struct regmap *regmap = isc->regmap;
>>> +   unsigned long flags;
>>> +   struct isc_buffer *buf, *tmp;
>>> +   int ret;
>>> +   u32 val;
>>> +
>>> +   isc->stop = true;
>>> +
>>> +   /* Wait until the end of the current frame */
>>> +   regmap_read(regmap, ISC_CTRLSR, );
>>> +   while (val & ISC_CTRLSR_CAPTURE) {
>>> +   usleep_range(1000, 2000);
>>> +   regmap_read(regmap, ISC_CTRLSR, );
>>> +   }
>>
>> Can't you synchronize with the frame end interrupt  instead of using a busy
>> loop ? The code here doesn't guarantee that your frame end interrupt won't
>> race you.
>>
> The capture will be disabled automatically when a frame capture is 
> completed. When 'isc->stop' is set to true, the new frame capture will 
> not be enabled in the frame end interrupt, then the capture is disabled 
> automatically, we can synchronize the capture status by loop reading the 
> capture status register in stop_streaming function. If the frame list is 
> empty, the frame end interrupt will not be triggered, if we want to 
> synchronize with the frame end interrupt, it will not be synchronized 
> forever.
> 
> Maybe my understanding is not correct. What's your opinion?

Without having an opinion on this precise case, still, here again, no
busy loop without way-out.

>>> +   /* Disable DMA

Re: [PATCH 1/2] [media] atmel-isc: add the Image Sensor Controller code

2016-04-19 Thread Wu, Songjun 



On 4/18/2016 15:24, Hans Verkuil wrote:

On 04/13/2016 09:44 AM, Songjun Wu wrote:

Add driver for the Image Sensor Controller. It manages
incoming data from a parallel based CMOS/CCD sensor.
It has an internal image processor, also integrates a
triple channel direct memory access controller master
interface.

Signed-off-by: Songjun Wu 


Hi Songjun,

Before this driver can be accepted it has to pass the v4l2-compliance test.
The v4l2-compliance utility is here:

git://linuxtv.org/v4l-utils.git

Compile the utility straight from this repository so you're up to date.

First fix any failures you get when running 'v4l2-compliance', then do the
same when running 'v4l2-compliance -s' (now it is streaming as well) and
finally when running 'v4l2-compliance -f' (streaming and testing all available
formats).

I would like to see the output of 'v4l2-compliance -f' as part of the cover
letter of the next patch series.

Looking at the code I see that it will definitely fail a few tests :-)

Certainly the VIDIOC_CREATE_BUFS support in the queue_setup function is missing.
Read the comments for queue_setup in videobuf2-core.h for more information.

Thank you very much, I will pass the v4l2-compliance test before I send 
the version 2 patch.



Regards,

Hans


--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] [media] atmel-isc: add the Image Sensor Controller code

2016-04-19 Thread Wu, Songjun 



On 4/15/2016 00:21, Laurent Pinchart wrote:

Hello Songjun,

Thank you for the patch.

On Wednesday 13 Apr 2016 15:44:19 Songjun Wu wrote:

Add driver for the Image Sensor Controller. It manages
incoming data from a parallel based CMOS/CCD sensor.
It has an internal image processor, also integrates a
triple channel direct memory access controller master
interface.

Signed-off-by: Songjun Wu 
---

  drivers/media/platform/Kconfig|1 +
  drivers/media/platform/Makefile   |2 +
  drivers/media/platform/atmel/Kconfig  |9 +
  drivers/media/platform/atmel/Makefile |3 +
  drivers/media/platform/atmel/atmel-isc-regs.h |  280 +
  drivers/media/platform/atmel/atmel-isc.c  | 1537 ++
  6 files changed, 1832 insertions(+)
  create mode 100644 drivers/media/platform/atmel/Kconfig
  create mode 100644 drivers/media/platform/atmel/Makefile
  create mode 100644 drivers/media/platform/atmel/atmel-isc-regs.h
  create mode 100644 drivers/media/platform/atmel/atmel-isc.c

diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig
index 201f5c2..1b50ed1 100644
--- a/drivers/media/platform/Kconfig
+++ b/drivers/media/platform/Kconfig
@@ -110,6 +110,7 @@ source "drivers/media/platform/exynos4-is/Kconfig"
  source "drivers/media/platform/s5p-tv/Kconfig"
  source "drivers/media/platform/am437x/Kconfig"
  source "drivers/media/platform/xilinx/Kconfig"
+source "drivers/media/platform/atmel/Kconfig"

  config VIDEO_TI_CAL
tristate "TI CAL (Camera Adaptation Layer) driver"
diff --git a/drivers/media/platform/Makefile
b/drivers/media/platform/Makefile index bbb7bd1..ad8f471 100644
--- a/drivers/media/platform/Makefile
+++ b/drivers/media/platform/Makefile
@@ -55,4 +55,6 @@ obj-$(CONFIG_VIDEO_AM437X_VPFE)   += am437x/

  obj-$(CONFIG_VIDEO_XILINX)+= xilinx/

+obj-$(CONFIG_VIDEO_ATMEL_ISC)  += atmel/
+
  ccflags-y += -I$(srctree)/drivers/media/i2c
diff --git a/drivers/media/platform/atmel/Kconfig
b/drivers/media/platform/atmel/Kconfig new file mode 100644
index 000..5ebc4a6
--- /dev/null
+++ b/drivers/media/platform/atmel/Kconfig
@@ -0,0 +1,9 @@
+config VIDEO_ATMEL_ISC
+   tristate "ATMEL Image Sensor Controller (ISC) support"
+   depends on VIDEO_V4L2 && HAS_DMA
+   depends on ARCH_AT91 || COMPILE_TEST


As commented separately, you're missing "depends on COMMON_CLK".


Accept, thank you.


+   select VIDEOBUF2_DMA_CONTIG
+   select REGMAP_MMIO
+   help
+  This module makes the ATMEL Image Sensor Controller available
+  as a v4l2 device.
\ No newline at end of file
diff --git a/drivers/media/platform/atmel/Makefile
b/drivers/media/platform/atmel/Makefile new file mode 100644
index 000..eb8cdbb
--- /dev/null
+++ b/drivers/media/platform/atmel/Makefile
@@ -0,0 +1,3 @@
+# Makefile for ATMEL ISC driver


The makefile isn't limited to the ISC driver, even if that's the only one
currently located in the atmel directory. The atmel-isi driver should be
placed here when it will move away from soc-camera. I would just write
"Makefile for Atmel drivers", or even remove the comment completely.


Accept.
atmel-isi driver will be added into this folder later.
I think "Makefile for Atmel drivers" is better.
Thank you.


+obj-$(CONFIG_VIDEO_ATMEL_ISC) += atmel-isc.o
diff --git a/drivers/media/platform/atmel/atmel-isc-regs.h
b/drivers/media/platform/atmel/atmel-isc-regs.h new file mode 100644
index 000..8be9e4a
--- /dev/null
+++ b/drivers/media/platform/atmel/atmel-isc-regs.h
@@ -0,0 +1,280 @@
+


No need for a blank line here.


Accept, thank you.


+#ifndef __ATMEL_ISC_REGS_H
+#define __ATMEL_ISC_REGS_H
+
+#include 


[snip]


+/* ISC Clock Configuration Register */
+#define ISC_CLKCFG  0x0024
+#define ISC_CLKCFG_DIV_SHIFT(n) (n*16)


As n can be an expression, you should enclose it in parentheses, ((n)*16).
Same for tall the macros below.


Accept, thank you.


+#define ISC_CLKCFG_DIV_MASK(n)  GENMASK((n*16 + 7), n*16)
+#define ISC_CLKCFG_SEL_SHIFT(n) (n*16 + 8)
+#define ISC_CLKCFG_SEL_MASK(n)  GENMASK((n*17 + 8), (n*16 + 8))
+


[snip]


diff --git a/drivers/media/platform/atmel/atmel-isc.c
b/drivers/media/platform/atmel/atmel-isc.c new file mode 100644
index 000..4ffbfc9
--- /dev/null
+++ b/drivers/media/platform/atmel/atmel-isc.c
@@ -0,0 +1,1537 @@
+/*
+ * Atmel Image Sensor Controller (ISC) driver
+ *
+ * Copyright (C) 2016 Atmel
+ *
+ * Author: Songjun Wu 
+ *
+ * This program is free software; you may redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; version 2 of the License.
+ *
+ * Sensor-->PFE-->WB-->CFA-->CC-->GAM-->CSC-->CBC-->SUB-->RLP-->DMA
+ *
+ * ISC video pipeline integrates the following submodules:
+ * PFE: Parallel Front End to sample the camera sensor input stream
+ * WB:  Programmable

Re: [PATCH 1/2] [media] atmel-isc: add the Image Sensor Controller code

2016-04-18 Thread Wu, Songjun 



On 4/14/2016 22:14, Laurent Pinchart wrote:

Hello Songjun,

On Thursday 14 Apr 2016 13:44:27 Wu, Songjun wrote:

The option 'CONFIG_COMMON_CLK=y' is needed to add to '.config'.
But I do not validate, '.config' will be generated automatically and
overwritten when it is changed.


Your driver's Kconfig entry should then contain "depends on COMMON_CLK".


Accept.
Thank you.

On 4/14/2016 00:01, kbuild test robot wrote:

Hi Songjun,

[auto build test ERROR on linuxtv-media/master]
[also build test ERROR on v4.6-rc3 next-20160413]
[if your patch is applied to the wrong git tree, please drop us a note to
help improving the system]

url:
https://github.com/0day-ci/linux/commits/Songjun-Wu/atmel-isc-add-driver-> > 
for-Atmel-ISC/20160413-155337 base:   git://linuxtv.org/media_tree.git
master
config: powerpc-allyesconfig (attached as .config)

reproduce:
  wget
  https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/p
  lain/sbin/make.cross -O ~/bin/make.cross chmod +x
  ~/bin/make.cross
  # save the attached .config to linux build tree
  make.cross ARCH=powerpc

All errors (new ones prefixed by >>):
  from include/linux/of.h:21,

  from drivers/media/platform/atmel/atmel-isc.c:27:
 drivers/media/platform/atmel/atmel-isc.c: In function
 'isc_clk_enable':
 include/linux/kernel.h:824:48: error: initialization from incompatible
 pointer type [-Werror=incompatible-pointer-types]>
   const typeof( ((type *)0)->member ) *__mptr = (ptr); \

 ^

 drivers/media/platform/atmel/atmel-isc.c:55:24: note: in expansion of
 macro 'container_of'>
  #define to_isc_clk(hw) container_of(hw, struct isc_clk, hw)

 ^

 drivers/media/platform/atmel/atmel-isc.c:247:28: note: in expansion of
 macro 'to_isc_clk'>
   struct isc_clk *isc_clk = to_isc_clk(hw);

 ^

 include/linux/kernel.h:824:48: note: (near initialization for
 'isc_clk')

   const typeof( ((type *)0)->member ) *__mptr = (ptr); \

 ^

 drivers/media/platform/atmel/atmel-isc.c:55:24: note: in expansion of
 macro 'container_of'>
  #define to_isc_clk(hw) container_of(hw, struct isc_clk, hw)

 ^

 drivers/media/platform/atmel/atmel-isc.c:247:28: note: in expansion of
 macro 'to_isc_clk'>
   struct isc_clk *isc_clk = to_isc_clk(hw);

 ^

 drivers/media/platform/atmel/atmel-isc.c: In function
 'isc_clk_disable':
 include/linux/kernel.h:824:48: error: initialization from incompatible
 pointer type [-Werror=incompatible-pointer-types]>
   const typeof( ((type *)0)->member ) *__mptr = (ptr); \

 ^

 drivers/media/platform/atmel/atmel-isc.c:55:24: note: in expansion of
 macro 'container_of'>
  #define to_isc_clk(hw) container_of(hw, struct isc_clk, hw)

 ^

 drivers/media/platform/atmel/atmel-isc.c:280:28: note: in expansion of
 macro 'to_isc_clk'>
   struct isc_clk *isc_clk = to_isc_clk(hw);

 ^

 include/linux/kernel.h:824:48: note: (near initialization for
 'isc_clk')

   const typeof( ((type *)0)->member ) *__mptr = (ptr); \

 ^

 drivers/media/platform/atmel/atmel-isc.c:55:24: note: in expansion of
 macro 'container_of'>
  #define to_isc_clk(hw) container_of(hw, struct isc_clk, hw)

 ^

 drivers/media/platform/atmel/atmel-isc.c:280:28: note: in expansion of
 macro 'to_isc_clk'>
   struct isc_clk *isc_clk = to_isc_clk(hw);

 ^

 drivers/media/platform/atmel/atmel-isc.c: In function
 'isc_clk_is_enabled':
 include/linux/kernel.h:824:48: error: initialization from incompatible
 pointer type [-Werror=incompatible-pointer-types]>
   const typeof( ((type *)0)->member ) *__mptr = (ptr); \

 ^

 drivers/media/platform/atmel/atmel-isc.c:55:24: note: in expansion of
 macro 'container_of'>
  #define to_isc_clk(hw) container_of(hw, struct isc_clk, hw)

 ^

 drivers/media/platform/atmel/atmel-isc.c:295:28: note: in expansion of
 macro 'to_isc_clk'>
   struct isc_clk *isc_clk = to_isc_clk(hw);

 ^

 include/linux/kernel.h:824:48: note: (near initialization for
 'isc_clk')

   const typeof( ((type *)0)->member ) *__mptr = (ptr); \

 ^

 drivers/media/platform/atmel/atmel-isc.c:55:24: note: in expansion of
 macro 'container_of'>
  #define to_isc_clk(hw) container_of(hw,

Re: [PATCH 1/2] [media] atmel-isc: add the Image Sensor Controller code

2016-04-18 Thread Hans Verkuil 
On 04/13/2016 09:44 AM, Songjun Wu wrote:
> Add driver for the Image Sensor Controller. It manages
> incoming data from a parallel based CMOS/CCD sensor.
> It has an internal image processor, also integrates a
> triple channel direct memory access controller master
> interface.
> 
> Signed-off-by: Songjun Wu 

Hi Songjun,

Before this driver can be accepted it has to pass the v4l2-compliance test.
The v4l2-compliance utility is here:

git://linuxtv.org/v4l-utils.git

Compile the utility straight from this repository so you're up to date.

First fix any failures you get when running 'v4l2-compliance', then do the
same when running 'v4l2-compliance -s' (now it is streaming as well) and
finally when running 'v4l2-compliance -f' (streaming and testing all available
formats).

I would like to see the output of 'v4l2-compliance -f' as part of the cover
letter of the next patch series.

Looking at the code I see that it will definitely fail a few tests :-)

Certainly the VIDIOC_CREATE_BUFS support in the queue_setup function is missing.
Read the comments for queue_setup in videobuf2-core.h for more information.

Regards,

Hans
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] [media] atmel-isc: add the Image Sensor Controller code

2016-04-14 Thread Laurent Pinchart 
Hello Songjun,

Thank you for the patch.

On Wednesday 13 Apr 2016 15:44:19 Songjun Wu wrote:
> Add driver for the Image Sensor Controller. It manages
> incoming data from a parallel based CMOS/CCD sensor.
> It has an internal image processor, also integrates a
> triple channel direct memory access controller master
> interface.
> 
> Signed-off-by: Songjun Wu 
> ---
> 
>  drivers/media/platform/Kconfig|1 +
>  drivers/media/platform/Makefile   |2 +
>  drivers/media/platform/atmel/Kconfig  |9 +
>  drivers/media/platform/atmel/Makefile |3 +
>  drivers/media/platform/atmel/atmel-isc-regs.h |  280 +
>  drivers/media/platform/atmel/atmel-isc.c  | 1537 ++
>  6 files changed, 1832 insertions(+)
>  create mode 100644 drivers/media/platform/atmel/Kconfig
>  create mode 100644 drivers/media/platform/atmel/Makefile
>  create mode 100644 drivers/media/platform/atmel/atmel-isc-regs.h
>  create mode 100644 drivers/media/platform/atmel/atmel-isc.c
> 
> diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig
> index 201f5c2..1b50ed1 100644
> --- a/drivers/media/platform/Kconfig
> +++ b/drivers/media/platform/Kconfig
> @@ -110,6 +110,7 @@ source "drivers/media/platform/exynos4-is/Kconfig"
>  source "drivers/media/platform/s5p-tv/Kconfig"
>  source "drivers/media/platform/am437x/Kconfig"
>  source "drivers/media/platform/xilinx/Kconfig"
> +source "drivers/media/platform/atmel/Kconfig"
> 
>  config VIDEO_TI_CAL
>   tristate "TI CAL (Camera Adaptation Layer) driver"
> diff --git a/drivers/media/platform/Makefile
> b/drivers/media/platform/Makefile index bbb7bd1..ad8f471 100644
> --- a/drivers/media/platform/Makefile
> +++ b/drivers/media/platform/Makefile
> @@ -55,4 +55,6 @@ obj-$(CONFIG_VIDEO_AM437X_VPFE) += am437x/
> 
>  obj-$(CONFIG_VIDEO_XILINX)   += xilinx/
> 
> +obj-$(CONFIG_VIDEO_ATMEL_ISC)+= atmel/
> +
>  ccflags-y += -I$(srctree)/drivers/media/i2c
> diff --git a/drivers/media/platform/atmel/Kconfig
> b/drivers/media/platform/atmel/Kconfig new file mode 100644
> index 000..5ebc4a6
> --- /dev/null
> +++ b/drivers/media/platform/atmel/Kconfig
> @@ -0,0 +1,9 @@
> +config VIDEO_ATMEL_ISC
> + tristate "ATMEL Image Sensor Controller (ISC) support"
> + depends on VIDEO_V4L2 && HAS_DMA
> + depends on ARCH_AT91 || COMPILE_TEST

As commented separately, you're missing "depends on COMMON_CLK".

> + select VIDEOBUF2_DMA_CONTIG
> + select REGMAP_MMIO
> + help
> +This module makes the ATMEL Image Sensor Controller available
> +as a v4l2 device.
> \ No newline at end of file
> diff --git a/drivers/media/platform/atmel/Makefile
> b/drivers/media/platform/atmel/Makefile new file mode 100644
> index 000..eb8cdbb
> --- /dev/null
> +++ b/drivers/media/platform/atmel/Makefile
> @@ -0,0 +1,3 @@
> +# Makefile for ATMEL ISC driver

The makefile isn't limited to the ISC driver, even if that's the only one 
currently located in the atmel directory. The atmel-isi driver should be 
placed here when it will move away from soc-camera. I would just write 
"Makefile for Atmel drivers", or even remove the comment completely.

> +obj-$(CONFIG_VIDEO_ATMEL_ISC) += atmel-isc.o
> diff --git a/drivers/media/platform/atmel/atmel-isc-regs.h
> b/drivers/media/platform/atmel/atmel-isc-regs.h new file mode 100644
> index 000..8be9e4a
> --- /dev/null
> +++ b/drivers/media/platform/atmel/atmel-isc-regs.h
> @@ -0,0 +1,280 @@
> +

No need for a blank line here.

> +#ifndef __ATMEL_ISC_REGS_H
> +#define __ATMEL_ISC_REGS_H
> +
> +#include 

[snip]

> +/* ISC Clock Configuration Register */
> +#define ISC_CLKCFG  0x0024
> +#define ISC_CLKCFG_DIV_SHIFT(n) (n*16)

As n can be an expression, you should enclose it in parentheses, ((n)*16). 
Same for tall the macros below.

> +#define ISC_CLKCFG_DIV_MASK(n)  GENMASK((n*16 + 7), n*16)
> +#define ISC_CLKCFG_SEL_SHIFT(n) (n*16 + 8)
> +#define ISC_CLKCFG_SEL_MASK(n)  GENMASK((n*17 + 8), (n*16 + 8))
> +

[snip]

> diff --git a/drivers/media/platform/atmel/atmel-isc.c
> b/drivers/media/platform/atmel/atmel-isc.c new file mode 100644
> index 000..4ffbfc9
> --- /dev/null
> +++ b/drivers/media/platform/atmel/atmel-isc.c
> @@ -0,0 +1,1537 @@
> +/*
> + * Atmel Image Sensor Controller (ISC) driver
> + *
> + * Copyright (C) 2016 Atmel
> + *
> + * Author: Songjun Wu 
> + *
> + * This program is free software; you may redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; version 2 of the License.
> + *
> + * Sensor-->PFE-->WB-->CFA-->CC-->GAM-->CSC-->CBC-->SUB-->RLP-->DMA
> + *
> + * ISC video pipeline integrates the following submodules:
> + * PFE: Parallel Front End to sample the camera sensor input stream
> + * WB:  Programmable white balance in the Bayer domain
> + * CFA: Color filter

Re: [PATCH 1/2] [media] atmel-isc: add the Image Sensor Controller code

2016-04-14 Thread Laurent Pinchart 
Hello Songjun,

On Thursday 14 Apr 2016 13:44:27 Wu, Songjun wrote:
> The option 'CONFIG_COMMON_CLK=y' is needed to add to '.config'.
> But I do not validate, '.config' will be generated automatically and
> overwritten when it is changed.

Your driver's Kconfig entry should then contain "depends on COMMON_CLK".

> On 4/14/2016 00:01, kbuild test robot wrote:
> > Hi Songjun,
> > 
> > [auto build test ERROR on linuxtv-media/master]
> > [also build test ERROR on v4.6-rc3 next-20160413]
> > [if your patch is applied to the wrong git tree, please drop us a note to
> > help improving the system]
> > 
> > url:   
> > https://github.com/0day-ci/linux/commits/Songjun-Wu/atmel-isc-add-driver-> 
> > > for-Atmel-ISC/20160413-155337 base:   git://linuxtv.org/media_tree.git
> > master
> > config: powerpc-allyesconfig (attached as .config)
> > 
> > reproduce:
> >  wget
> >  https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/p
> >  lain/sbin/make.cross -O ~/bin/make.cross chmod +x
> >  ~/bin/make.cross
> >  # save the attached .config to linux build tree
> >  make.cross ARCH=powerpc
> > 
> > All errors (new ones prefixed by >>):
> >  from include/linux/of.h:21,
> > 
> >  from drivers/media/platform/atmel/atmel-isc.c:27:
> > drivers/media/platform/atmel/atmel-isc.c: In function
> > 'isc_clk_enable':
> > include/linux/kernel.h:824:48: error: initialization from incompatible
> > pointer type [-Werror=incompatible-pointer-types]> 
> >   const typeof( ((type *)0)->member ) *__mptr = (ptr); \
> >   
> > ^
> > 
> > drivers/media/platform/atmel/atmel-isc.c:55:24: note: in expansion of
> > macro 'container_of'> 
> >  #define to_isc_clk(hw) container_of(hw, struct isc_clk, hw)
> >  
> > ^
> > 
> > drivers/media/platform/atmel/atmel-isc.c:247:28: note: in expansion of
> > macro 'to_isc_clk'> 
> >   struct isc_clk *isc_clk = to_isc_clk(hw);
> >   
> > ^
> > 
> > include/linux/kernel.h:824:48: note: (near initialization for
> > 'isc_clk')
> > 
> >   const typeof( ((type *)0)->member ) *__mptr = (ptr); \
> >   
> > ^
> > 
> > drivers/media/platform/atmel/atmel-isc.c:55:24: note: in expansion of
> > macro 'container_of'> 
> >  #define to_isc_clk(hw) container_of(hw, struct isc_clk, hw)
> >  
> > ^
> > 
> > drivers/media/platform/atmel/atmel-isc.c:247:28: note: in expansion of
> > macro 'to_isc_clk'> 
> >   struct isc_clk *isc_clk = to_isc_clk(hw);
> >   
> > ^
> > 
> > drivers/media/platform/atmel/atmel-isc.c: In function
> > 'isc_clk_disable':
> > include/linux/kernel.h:824:48: error: initialization from incompatible
> > pointer type [-Werror=incompatible-pointer-types]> 
> >   const typeof( ((type *)0)->member ) *__mptr = (ptr); \
> >   
> > ^
> > 
> > drivers/media/platform/atmel/atmel-isc.c:55:24: note: in expansion of
> > macro 'container_of'> 
> >  #define to_isc_clk(hw) container_of(hw, struct isc_clk, hw)
> >  
> > ^
> > 
> > drivers/media/platform/atmel/atmel-isc.c:280:28: note: in expansion of
> > macro 'to_isc_clk'> 
> >   struct isc_clk *isc_clk = to_isc_clk(hw);
> >   
> > ^
> > 
> > include/linux/kernel.h:824:48: note: (near initialization for
> > 'isc_clk')
> > 
> >   const typeof( ((type *)0)->member ) *__mptr = (ptr); \
> >   
> > ^
> > 
> > drivers/media/platform/atmel/atmel-isc.c:55:24: note: in expansion of
> > macro 'container_of'> 
> >  #define to_isc_clk(hw) container_of(hw, struct isc_clk, hw)
> >  
> > ^
> > 
> > drivers/media/platform/atmel/atmel-isc.c:280:28: note: in expansion of
> > macro 'to_isc_clk'> 
> >   struct isc_clk *isc_clk = to_isc_clk(hw);
> >   
> > ^
> > 
> > drivers/media/platform/atmel/atmel-isc.c: In function
> > 'isc_clk_is_enabled':
> > include/linux/kernel.h:824:48: error: initialization from incompatible
> > pointer type [-Werror=incompatible-pointer-types]> 
> >   const typeof( ((type *)0)->member ) *__mptr = (ptr); \
> >   
> > ^
> > 
> > drivers/media/platform/atmel/atmel-isc.c:55:24: note: in expansion of
> > macro 'container_of'> 
> >  #define to_isc_clk(hw) container_of(hw, struct isc_clk, hw)
> >  
> > ^
> >

Re: [PATCH 1/2] [media] atmel-isc: add the Image Sensor Controller code

2016-04-13 Thread Wu, Songjun 

The option 'CONFIG_COMMON_CLK=y' is needed to add to '.config'.
But I do not validate, '.config' will be generated automatically and 
overwritten when it is changed.


On 4/14/2016 00:01, kbuild test robot wrote:

Hi Songjun,

[auto build test ERROR on linuxtv-media/master]
[also build test ERROR on v4.6-rc3 next-20160413]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improving the system]

url:
https://github.com/0day-ci/linux/commits/Songjun-Wu/atmel-isc-add-driver-for-Atmel-ISC/20160413-155337
base:   git://linuxtv.org/media_tree.git master
config: powerpc-allyesconfig (attached as .config)
reproduce:
 wget 
https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross
 -O ~/bin/make.cross
 chmod +x ~/bin/make.cross
 # save the attached .config to linux build tree
 make.cross ARCH=powerpc

All errors (new ones prefixed by >>):

 from include/linux/of.h:21,
 from drivers/media/platform/atmel/atmel-isc.c:27:
drivers/media/platform/atmel/atmel-isc.c: In function 'isc_clk_enable':
include/linux/kernel.h:824:48: error: initialization from incompatible 
pointer type [-Werror=incompatible-pointer-types]
  const typeof( ((type *)0)->member ) *__mptr = (ptr); \
^
drivers/media/platform/atmel/atmel-isc.c:55:24: note: in expansion of macro 
'container_of'
 #define to_isc_clk(hw) container_of(hw, struct isc_clk, hw)
^
drivers/media/platform/atmel/atmel-isc.c:247:28: note: in expansion of 
macro 'to_isc_clk'
  struct isc_clk *isc_clk = to_isc_clk(hw);
^
include/linux/kernel.h:824:48: note: (near initialization for 'isc_clk')
  const typeof( ((type *)0)->member ) *__mptr = (ptr); \
^
drivers/media/platform/atmel/atmel-isc.c:55:24: note: in expansion of macro 
'container_of'
 #define to_isc_clk(hw) container_of(hw, struct isc_clk, hw)
^
drivers/media/platform/atmel/atmel-isc.c:247:28: note: in expansion of 
macro 'to_isc_clk'
  struct isc_clk *isc_clk = to_isc_clk(hw);
^
drivers/media/platform/atmel/atmel-isc.c: In function 'isc_clk_disable':
include/linux/kernel.h:824:48: error: initialization from incompatible 
pointer type [-Werror=incompatible-pointer-types]
  const typeof( ((type *)0)->member ) *__mptr = (ptr); \
^
drivers/media/platform/atmel/atmel-isc.c:55:24: note: in expansion of macro 
'container_of'
 #define to_isc_clk(hw) container_of(hw, struct isc_clk, hw)
^
drivers/media/platform/atmel/atmel-isc.c:280:28: note: in expansion of 
macro 'to_isc_clk'
  struct isc_clk *isc_clk = to_isc_clk(hw);
^
include/linux/kernel.h:824:48: note: (near initialization for 'isc_clk')
  const typeof( ((type *)0)->member ) *__mptr = (ptr); \
^
drivers/media/platform/atmel/atmel-isc.c:55:24: note: in expansion of macro 
'container_of'
 #define to_isc_clk(hw) container_of(hw, struct isc_clk, hw)
^
drivers/media/platform/atmel/atmel-isc.c:280:28: note: in expansion of 
macro 'to_isc_clk'
  struct isc_clk *isc_clk = to_isc_clk(hw);
^
drivers/media/platform/atmel/atmel-isc.c: In function 'isc_clk_is_enabled':
include/linux/kernel.h:824:48: error: initialization from incompatible 
pointer type [-Werror=incompatible-pointer-types]
  const typeof( ((type *)0)->member ) *__mptr = (ptr); \
^
drivers/media/platform/atmel/atmel-isc.c:55:24: note: in expansion of macro 
'container_of'
 #define to_isc_clk(hw) container_of(hw, struct isc_clk, hw)
^
drivers/media/platform/atmel/atmel-isc.c:295:28: note: in expansion of 
macro 'to_isc_clk'
  struct isc_clk *isc_clk = to_isc_clk(hw);
^
include/linux/kernel.h:824:48: note: (near initialization for 'isc_clk')
  const typeof( ((type *)0)->member ) *__mptr = (ptr); \
^
drivers/media/platform/atmel/atmel-isc.c:55:24: note: in expansion of macro 
'container_of'
 #define to_isc_clk(hw) container_of(hw, struct isc_clk, hw)
^
drivers/media/platform/atmel/atmel-isc.c:295:28: note: in expansion of 
macro 'to_isc_clk'
  struct isc_clk *isc_clk = to_isc_clk(hw);
^
drivers/media/platform/atmel/atmel-isc.c: In function 'isc_clk_recalc_rate':
include/linux/kernel.h:824:48: error: initialization from incompatible 
pointer type

Re: [PATCH 1/2] [media] atmel-isc: add the Image Sensor Controller code

2016-04-13 Thread kbuild test robot 
Hi Songjun,

[auto build test ERROR on linuxtv-media/master]
[also build test ERROR on v4.6-rc3 next-20160413]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improving the system]

url:
https://github.com/0day-ci/linux/commits/Songjun-Wu/atmel-isc-add-driver-for-Atmel-ISC/20160413-155337
base:   git://linuxtv.org/media_tree.git master
config: powerpc-allyesconfig (attached as .config)
reproduce:
wget 
https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross
 -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=powerpc 

All errors (new ones prefixed by >>):

from include/linux/of.h:21,
from drivers/media/platform/atmel/atmel-isc.c:27:
   drivers/media/platform/atmel/atmel-isc.c: In function 'isc_clk_enable':
   include/linux/kernel.h:824:48: error: initialization from incompatible 
pointer type [-Werror=incompatible-pointer-types]
 const typeof( ((type *)0)->member ) *__mptr = (ptr); \
   ^
   drivers/media/platform/atmel/atmel-isc.c:55:24: note: in expansion of macro 
'container_of'
#define to_isc_clk(hw) container_of(hw, struct isc_clk, hw)
   ^
   drivers/media/platform/atmel/atmel-isc.c:247:28: note: in expansion of macro 
'to_isc_clk'
 struct isc_clk *isc_clk = to_isc_clk(hw);
   ^
   include/linux/kernel.h:824:48: note: (near initialization for 'isc_clk')
 const typeof( ((type *)0)->member ) *__mptr = (ptr); \
   ^
   drivers/media/platform/atmel/atmel-isc.c:55:24: note: in expansion of macro 
'container_of'
#define to_isc_clk(hw) container_of(hw, struct isc_clk, hw)
   ^
   drivers/media/platform/atmel/atmel-isc.c:247:28: note: in expansion of macro 
'to_isc_clk'
 struct isc_clk *isc_clk = to_isc_clk(hw);
   ^
   drivers/media/platform/atmel/atmel-isc.c: In function 'isc_clk_disable':
   include/linux/kernel.h:824:48: error: initialization from incompatible 
pointer type [-Werror=incompatible-pointer-types]
 const typeof( ((type *)0)->member ) *__mptr = (ptr); \
   ^
   drivers/media/platform/atmel/atmel-isc.c:55:24: note: in expansion of macro 
'container_of'
#define to_isc_clk(hw) container_of(hw, struct isc_clk, hw)
   ^
   drivers/media/platform/atmel/atmel-isc.c:280:28: note: in expansion of macro 
'to_isc_clk'
 struct isc_clk *isc_clk = to_isc_clk(hw);
   ^
   include/linux/kernel.h:824:48: note: (near initialization for 'isc_clk')
 const typeof( ((type *)0)->member ) *__mptr = (ptr); \
   ^
   drivers/media/platform/atmel/atmel-isc.c:55:24: note: in expansion of macro 
'container_of'
#define to_isc_clk(hw) container_of(hw, struct isc_clk, hw)
   ^
   drivers/media/platform/atmel/atmel-isc.c:280:28: note: in expansion of macro 
'to_isc_clk'
 struct isc_clk *isc_clk = to_isc_clk(hw);
   ^
   drivers/media/platform/atmel/atmel-isc.c: In function 'isc_clk_is_enabled':
   include/linux/kernel.h:824:48: error: initialization from incompatible 
pointer type [-Werror=incompatible-pointer-types]
 const typeof( ((type *)0)->member ) *__mptr = (ptr); \
   ^
   drivers/media/platform/atmel/atmel-isc.c:55:24: note: in expansion of macro 
'container_of'
#define to_isc_clk(hw) container_of(hw, struct isc_clk, hw)
   ^
   drivers/media/platform/atmel/atmel-isc.c:295:28: note: in expansion of macro 
'to_isc_clk'
 struct isc_clk *isc_clk = to_isc_clk(hw);
   ^
   include/linux/kernel.h:824:48: note: (near initialization for 'isc_clk')
 const typeof( ((type *)0)->member ) *__mptr = (ptr); \
   ^
   drivers/media/platform/atmel/atmel-isc.c:55:24: note: in expansion of macro 
'container_of'
#define to_isc_clk(hw) container_of(hw, struct isc_clk, hw)
   ^
   drivers/media/platform/atmel/atmel-isc.c:295:28: note: in expansion of macro 
'to_isc_clk'
 struct isc_clk *isc_clk = to_isc_clk(hw);
   ^
   drivers/media/platform/atmel/atmel-isc.c: In function 'isc_clk_recalc_rate':
   include/linux/kernel.h:824:48: error: initialization from incompatible 
pointer type [-Werror=incompatible-pointer-types]
 const typeof( ((type *)0)->member ) *__mptr = (ptr); \
   ^
   drivers/media/platform/atmel/atmel-isc.c:55:24: note: in expansion of macro 
'container_of'
#define to_isc_clk(hw) container_of(hw, struct isc_clk, hw)

Re: [PATCH 1/2] [media] atmel-isc: add the Image Sensor Controller code

2016-04-13 Thread kbuild test robot 
Hi Songjun,

[auto build test WARNING on linuxtv-media/master]
[also build test WARNING on v4.6-rc3 next-20160413]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improving the system]

url:
https://github.com/0day-ci/linux/commits/Songjun-Wu/atmel-isc-add-driver-for-Atmel-ISC/20160413-155337
base:   git://linuxtv.org/media_tree.git master
config: tile-allmodconfig (attached as .config)
reproduce:
wget 
https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross
 -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=tile 

All warnings (new ones prefixed by >>):

   drivers/media/platform/atmel/atmel-isc.c:67:18: error: field 'hw' has 
incomplete type
   drivers/media/platform/atmel/atmel-isc.c: In function 'isc_clk_enable':
>> drivers/media/platform/atmel/atmel-isc.c:247:28: warning: initialization 
>> from incompatible pointer type [enabled by default]
   drivers/media/platform/atmel/atmel-isc.c:247:28: warning: (near 
initialization for 'isc_clk') [enabled by default]
   drivers/media/platform/atmel/atmel-isc.c: In function 'isc_clk_disable':
   drivers/media/platform/atmel/atmel-isc.c:280:28: warning: initialization 
from incompatible pointer type [enabled by default]
   drivers/media/platform/atmel/atmel-isc.c:280:28: warning: (near 
initialization for 'isc_clk') [enabled by default]
   drivers/media/platform/atmel/atmel-isc.c: In function 'isc_clk_is_enabled':
   drivers/media/platform/atmel/atmel-isc.c:295:28: warning: initialization 
from incompatible pointer type [enabled by default]
   drivers/media/platform/atmel/atmel-isc.c:295:28: warning: (near 
initialization for 'isc_clk') [enabled by default]
   drivers/media/platform/atmel/atmel-isc.c: In function 'isc_clk_recalc_rate':
   drivers/media/platform/atmel/atmel-isc.c:309:28: warning: initialization 
from incompatible pointer type [enabled by default]
   drivers/media/platform/atmel/atmel-isc.c:309:28: warning: (near 
initialization for 'isc_clk') [enabled by default]
   drivers/media/platform/atmel/atmel-isc.c: At top level:
   drivers/media/platform/atmel/atmel-isc.c:315:14: warning: 'struct 
clk_rate_request' declared inside parameter list [enabled by default]
   drivers/media/platform/atmel/atmel-isc.c:315:14: warning: its scope is only 
this definition or declaration, which is probably not what you want [enabled by 
default]
   drivers/media/platform/atmel/atmel-isc.c: In function 
'isc_clk_determine_rate':
   drivers/media/platform/atmel/atmel-isc.c:324:2: error: implicit declaration 
of function 'clk_hw_get_num_parents' [-Werror=implicit-function-declaration]
   drivers/media/platform/atmel/atmel-isc.c:325:3: error: implicit declaration 
of function 'clk_hw_get_parent_by_index' [-Werror=implicit-function-declaration]
   drivers/media/platform/atmel/atmel-isc.c:325:10: warning: assignment makes 
pointer from integer without a cast [enabled by default]
   drivers/media/platform/atmel/atmel-isc.c:329:3: error: implicit declaration 
of function 'clk_hw_get_rate' [-Werror=implicit-function-declaration]
   drivers/media/platform/atmel/atmel-isc.c:335:15: error: dereferencing 
pointer to incomplete type
   drivers/media/platform/atmel/atmel-isc.c:335: confused by earlier errors, 
bailing out

vim +247 drivers/media/platform/atmel/atmel-isc.c

61  enum isc_clk_id {
62  ISC_ISPCK = 0,
63  ISC_MCK = 1,
64  };
65  
66  struct isc_clk {
  > 67  struct clk_hw   hw;
68  struct clk  *clk;
69  struct regmap   *regmap;
70  spinlock_t  *lock;
71  enum isc_clk_id id;
72  u32 div;
73  u8  parent_id;
74  };
75  
76  struct isc_buffer {
77  struct vb2_v4l2_buffer  vb;
78  struct list_headlist;
79  };
80  
81  struct isc_subdev_entity {
82  struct v4l2_subdev  *sd;
83  struct v4l2_async_subdev*asd;
84  struct v4l2_async_notifier  notifier;
85  
86  u32 hsync_active;
87  u32 vsync_active;
88  u32 pclk_sample;
89  
90  struct list_head list;
91  };
92  
93  /*
94   * struct isc_format - ISC media bus format information
95   * @fourcc: Fourcc code for this format
96   * @isc_mbus_code:  V4L2 media bus format code if ISC is preferred
97   * @sd_mbus_code:   V4L2 media bus format code if subdev is 
preferred
98   * @bpp:Bytes per pixel (when stored in memory)
99   * @reg_sd_bps: reg value for bits per sample if subdev is 
preferred
   100   *  (when transferred over a bus)
   101   * @reg_isc_bps:reg value for bits per sample if ISC is 
preferred
   102   *  (when

Re: [PATCH 1/2] [media] atmel-isc: add the Image Sensor Controller code

2016-04-13 Thread kbuild test robot 
Hi Songjun,

[auto build test ERROR on linuxtv-media/master]
[also build test ERROR on v4.6-rc3 next-20160413]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improving the system]

url:
https://github.com/0day-ci/linux/commits/Songjun-Wu/atmel-isc-add-driver-for-Atmel-ISC/20160413-155337
base:   git://linuxtv.org/media_tree.git master
config: sparc64-allmodconfig (attached as .config)
reproduce:
wget 
https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross
 -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=sparc64 

All error/warnings (new ones prefixed by >>):

>> drivers/media/platform/atmel/atmel-isc.c:67:18: error: field 'hw' has 
>> incomplete type
 struct clk_hw   hw;
 ^
   In file included from include/linux/list.h:8:0,
from include/linux/kobject.h:20,
from include/linux/of.h:21,
from drivers/media/platform/atmel/atmel-isc.c:27:
   drivers/media/platform/atmel/atmel-isc.c: In function 'isc_clk_enable':
   include/linux/kernel.h:824:48: warning: initialization from incompatible 
pointer type
 const typeof( ((type *)0)->member ) *__mptr = (ptr); \
   ^
>> drivers/media/platform/atmel/atmel-isc.c:55:24: note: in expansion of macro 
>> 'container_of'
#define to_isc_clk(hw) container_of(hw, struct isc_clk, hw)
   ^
>> drivers/media/platform/atmel/atmel-isc.c:247:28: note: in expansion of macro 
>> 'to_isc_clk'
 struct isc_clk *isc_clk = to_isc_clk(hw);
   ^
   include/linux/kernel.h:824:48: warning: (near initialization for 'isc_clk')
 const typeof( ((type *)0)->member ) *__mptr = (ptr); \
   ^
>> drivers/media/platform/atmel/atmel-isc.c:55:24: note: in expansion of macro 
>> 'container_of'
#define to_isc_clk(hw) container_of(hw, struct isc_clk, hw)
   ^
>> drivers/media/platform/atmel/atmel-isc.c:247:28: note: in expansion of macro 
>> 'to_isc_clk'
 struct isc_clk *isc_clk = to_isc_clk(hw);
   ^
   drivers/media/platform/atmel/atmel-isc.c: In function 'isc_clk_disable':
   include/linux/kernel.h:824:48: warning: initialization from incompatible 
pointer type
 const typeof( ((type *)0)->member ) *__mptr = (ptr); \
   ^
>> drivers/media/platform/atmel/atmel-isc.c:55:24: note: in expansion of macro 
>> 'container_of'
#define to_isc_clk(hw) container_of(hw, struct isc_clk, hw)
   ^
   drivers/media/platform/atmel/atmel-isc.c:280:28: note: in expansion of macro 
'to_isc_clk'
 struct isc_clk *isc_clk = to_isc_clk(hw);
   ^
   include/linux/kernel.h:824:48: warning: (near initialization for 'isc_clk')
 const typeof( ((type *)0)->member ) *__mptr = (ptr); \
   ^
>> drivers/media/platform/atmel/atmel-isc.c:55:24: note: in expansion of macro 
>> 'container_of'
#define to_isc_clk(hw) container_of(hw, struct isc_clk, hw)
   ^
   drivers/media/platform/atmel/atmel-isc.c:280:28: note: in expansion of macro 
'to_isc_clk'
 struct isc_clk *isc_clk = to_isc_clk(hw);
   ^
   drivers/media/platform/atmel/atmel-isc.c: In function 'isc_clk_is_enabled':
   include/linux/kernel.h:824:48: warning: initialization from incompatible 
pointer type
 const typeof( ((type *)0)->member ) *__mptr = (ptr); \
   ^
>> drivers/media/platform/atmel/atmel-isc.c:55:24: note: in expansion of macro 
>> 'container_of'
#define to_isc_clk(hw) container_of(hw, struct isc_clk, hw)
   ^
   drivers/media/platform/atmel/atmel-isc.c:295:28: note: in expansion of macro 
'to_isc_clk'
 struct isc_clk *isc_clk = to_isc_clk(hw);
   ^
   include/linux/kernel.h:824:48: warning: (near initialization for 'isc_clk')
 const typeof( ((type *)0)->member ) *__mptr = (ptr); \
   ^
>> drivers/media/platform/atmel/atmel-isc.c:55:24: note: in expansion of macro 
>> 'container_of'
#define to_isc_clk(hw) container_of(hw, struct isc_clk, hw)
   ^
   drivers/media/platform/atmel/atmel-isc.c:295:28: note: in expansion of macro 
'to_isc_clk'
 struct isc_clk *isc_clk = to_isc_clk(hw);
   ^
   drivers/media/platform/atmel/atmel-isc.c: In function 'isc_clk_recalc_rate':
   include/linux/kernel.h:824:48: warning: initialization from incompatible 
pointer type
 const typeof( ((type *)0)->member ) *__mptr = (ptr); \
   ^
>>

[PATCH 1/2] [media] atmel-isc: add the Image Sensor Controller code

2016-04-13 Thread Songjun Wu 
Add driver for the Image Sensor Controller. It manages
incoming data from a parallel based CMOS/CCD sensor.
It has an internal image processor, also integrates a
triple channel direct memory access controller master
interface.

Signed-off-by: Songjun Wu 
---

 drivers/media/platform/Kconfig|1 +
 drivers/media/platform/Makefile   |2 +
 drivers/media/platform/atmel/Kconfig  |9 +
 drivers/media/platform/atmel/Makefile |3 +
 drivers/media/platform/atmel/atmel-isc-regs.h |  280 +
 drivers/media/platform/atmel/atmel-isc.c  | 1537 +
 6 files changed, 1832 insertions(+)
 create mode 100644 drivers/media/platform/atmel/Kconfig
 create mode 100644 drivers/media/platform/atmel/Makefile
 create mode 100644 drivers/media/platform/atmel/atmel-isc-regs.h
 create mode 100644 drivers/media/platform/atmel/atmel-isc.c

diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig
index 201f5c2..1b50ed1 100644
--- a/drivers/media/platform/Kconfig
+++ b/drivers/media/platform/Kconfig
@@ -110,6 +110,7 @@ source "drivers/media/platform/exynos4-is/Kconfig"
 source "drivers/media/platform/s5p-tv/Kconfig"
 source "drivers/media/platform/am437x/Kconfig"
 source "drivers/media/platform/xilinx/Kconfig"
+source "drivers/media/platform/atmel/Kconfig"
 
 config VIDEO_TI_CAL
tristate "TI CAL (Camera Adaptation Layer) driver"
diff --git a/drivers/media/platform/Makefile b/drivers/media/platform/Makefile
index bbb7bd1..ad8f471 100644
--- a/drivers/media/platform/Makefile
+++ b/drivers/media/platform/Makefile
@@ -55,4 +55,6 @@ obj-$(CONFIG_VIDEO_AM437X_VPFE)   += am437x/
 
 obj-$(CONFIG_VIDEO_XILINX) += xilinx/
 
+obj-$(CONFIG_VIDEO_ATMEL_ISC)  += atmel/
+
 ccflags-y += -I$(srctree)/drivers/media/i2c
diff --git a/drivers/media/platform/atmel/Kconfig 
b/drivers/media/platform/atmel/Kconfig
new file mode 100644
index 000..5ebc4a6
--- /dev/null
+++ b/drivers/media/platform/atmel/Kconfig
@@ -0,0 +1,9 @@
+config VIDEO_ATMEL_ISC
+   tristate "ATMEL Image Sensor Controller (ISC) support"
+   depends on VIDEO_V4L2 && HAS_DMA
+   depends on ARCH_AT91 || COMPILE_TEST
+   select VIDEOBUF2_DMA_CONTIG
+   select REGMAP_MMIO
+   help
+  This module makes the ATMEL Image Sensor Controller available
+  as a v4l2 device.
\ No newline at end of file
diff --git a/drivers/media/platform/atmel/Makefile 
b/drivers/media/platform/atmel/Makefile
new file mode 100644
index 000..eb8cdbb
--- /dev/null
+++ b/drivers/media/platform/atmel/Makefile
@@ -0,0 +1,3 @@
+# Makefile for ATMEL ISC driver
+
+obj-$(CONFIG_VIDEO_ATMEL_ISC) += atmel-isc.o
diff --git a/drivers/media/platform/atmel/atmel-isc-regs.h 
b/drivers/media/platform/atmel/atmel-isc-regs.h
new file mode 100644
index 000..8be9e4a
--- /dev/null
+++ b/drivers/media/platform/atmel/atmel-isc-regs.h
@@ -0,0 +1,280 @@
+
+#ifndef __ATMEL_ISC_REGS_H
+#define __ATMEL_ISC_REGS_H
+
+#include 
+
+/* ISC Control Enable Register 0 */
+#define ISC_CTRLEN  0x
+
+#define ISC_CTRLEN_CAPTURE  BIT(0)
+#define ISC_CTRLEN_CAPTURE_MASK BIT(0)
+
+#define ISC_CTRLEN_UPPROBIT(1)
+#define ISC_CTRLEN_UPPRO_MASK   BIT(1)
+
+#define ISC_CTRLEN_HISREQ   BIT(2)
+#define ISC_CTRLEN_HISREQ_MASK  BIT(2)
+
+#define ISC_CTRLEN_HISCLR   BIT(3)
+#define ISC_CTRLEN_HISCLR_MASK  BIT(3)
+
+/* ISC Control Disable Register 0 */
+#define ISC_CTRLDIS 0x0004
+
+#define ISC_CTRLDIS_DISABLE BIT(0)
+#define ISC_CTRLDIS_DISABLE_MASKBIT(0)
+
+#define ISC_CTRLDIS_SWRST   BIT(8)
+#define ISC_CTRLDIS_SWRST_MASK  BIT(8)
+
+/* ISC Control Status Register 0 */
+#define ISC_CTRLSR  0x0008
+
+#define ISC_CTRLSR_CAPTURE  BIT(0)
+#define ISC_CTRLSR_UPPROBIT(1)
+#define ISC_CTRLSR_HISREQ   BIT(2)
+#define ISC_CTRLSR_FIELDBIT(4)
+#define ISC_CTRLSR_SIP  BIT(31)
+
+/* ISC Parallel Front End Configuration 0 Register */
+#define ISC_PFE_CFG00x000c
+
+#define ISC_PFE_CFG0_HPOL_LOW   BIT(0)
+#define ISC_PFE_CFG0_HPOL_HIGH  0x0
+#define ISC_PFE_CFG0_HPOL_MASK  BIT(0)
+
+#define ISC_PFE_CFG0_VPOL_LOW   BIT(1)
+#define ISC_PFE_CFG0_VPOL_HIGH  0x0
+#define ISC_PFE_CFG0_VPOL_MASK  BIT(1)
+
+#define ISC_PFE_CFG0_PPOL_LOW   BIT(2)
+#define ISC_PFE_CFG0_PPOL_HIGH  0x0
+#define ISC_PFE_CFG0_PPOL_MASK  BIT(2)
+
+#define ISC_PFE_CFG0_MODE_PROGRESSIVE   0x0
+#define ISC_PFE_CFG0_MODE_MASK  GENMASK(6, 4)
+
+#define ISC_PFE_CFG0_BPS_EIGHT  (0x4 << 28)
+#define ISC_PFG_CFG0_BPS_NINE   (0x3 << 28)
+#define ISC_PFG_CFG0_BPS_TEN(0x2 << 28)
+#define ISC_PFG_CFG0_BPS_ELEVEN (0x1 << 28)
+#define ISC_PFG_CFG0_BPS_TWELVE 0x0
+#define ISC_PFE_CFG0_BPS_MASK   GENMASK(30, 28)
+
+/* ISC Clock Enable Register */
+#define ISC_CLKEN   0x0018
+#define ISC_CLKEN_EN