Re: [PATCH] clk: let mxs specific clk-div clock type be a generic clock type

2013-03-18 Thread Shawn Guo
On Sat, Mar 16, 2013 at 06:20:01PM +0530, Thomas Abraham wrote:
 The mxs platform specific clk-div clock is an extended version of the
 basic integer divider clock type that supports checking the stability
 status of the divider clock output. This type of clock is found on
 some of the Samsung platforms as well. So let the mxs specfic clk-div
 clock type be a generic clock type that all platforms can utilize.
 
 Cc: Shawn Guo shawn@linaro.org
 Cc: Mike Turquette mturque...@linaro.org
 Signed-off-by: Thomas Abraham thomas.abra...@linaro.org
 ---
  drivers/clk/Makefile |1 +
  drivers/clk/clk-divider-status.c |  119 
 ++
  drivers/clk/mxs/Makefile |2 +-
  drivers/clk/mxs/clk-div.c|  110 ---
  drivers/clk/mxs/clk.h|   12 +++-
  include/linux/clk-provider.h |   21 +++
  6 files changed, 151 insertions(+), 114 deletions(-)
  create mode 100644 drivers/clk/clk-divider-status.c
  delete mode 100644 drivers/clk/mxs/clk-div.c

From my quick testing, it seems working for mxs platform.  But it's hard
to review the changes.  Making it two steps might be helpful for
reviewer:

1) git mv drivers/clk/mxs/clk-div.c drivers/clk/clk-divider-status.c
2) make changes on drivers/clk/clk-divider-status.c

Shawn

 
 diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
 index 0147022..0ac851a 100644
 --- a/drivers/clk/Makefile
 +++ b/drivers/clk/Makefile
 @@ -7,6 +7,7 @@ obj-$(CONFIG_COMMON_CLK)  += clk-fixed-factor.o
  obj-$(CONFIG_COMMON_CLK) += clk-fixed-rate.o
  obj-$(CONFIG_COMMON_CLK) += clk-gate.o
  obj-$(CONFIG_COMMON_CLK) += clk-mux.o
 +obj-$(CONFIG_COMMON_CLK) += clk-divider-status.o
  
  # SoCs specific
  obj-$(CONFIG_ARCH_BCM2835)   += clk-bcm2835.o
 diff --git a/drivers/clk/clk-divider-status.c 
 b/drivers/clk/clk-divider-status.c
 new file mode 100644
 index 000..1d66059
 --- /dev/null
 +++ b/drivers/clk/clk-divider-status.c
 @@ -0,0 +1,119 @@
 +/*
 + * Copyright 2012 Freescale Semiconductor, Inc.
 + *
 + * The code contained herein is licensed under the GNU General Public
 + * License. You may obtain a copy of the GNU General Public License
 + * Version 2 or later at the following locations:
 + *
 + * http://www.opensource.org/licenses/gpl-license.html
 + * http://www.gnu.org/copyleft/gpl.html
 + *
 + * Extension to the adjustable divider clock implementation with support for
 + * divider clock stability checks.
 + */
 +
 +#include linux/clk.h
 +#include linux/module.h
 +#include linux/clk-provider.h
 +#include linux/err.h
 +#include linux/slab.h
 +#include linux/io.h
 +#include linux/jiffies.h
 +
 +/*
 + * DOC: Adjustable divider clock with support for divider stability check
 + *
 + * Traits of this clock:
 + * prepare - clk_prepare only ensures that parents are prepared
 + * enable - clk_enable only ensures that parents are enabled
 + * rate - rate is adjustable.  clk-rate = parent-rate / divisor
 + * parent - fixed parent.  No clk_set_parent support
 + */
 +
 +static inline struct clk_divider_status *to_clk_div(struct clk_hw *hw)
 +{
 + struct clk_divider *divider = container_of(hw, struct clk_divider, hw);
 +
 + return container_of(divider, struct clk_divider_status, divider);
 +}
 +
 +static unsigned long clk_divider_status_recalc_rate(struct clk_hw *hw,
 +  unsigned long parent_rate)
 +{
 + struct clk_divider_status *div = to_clk_div(hw);
 +
 + return div-ops-recalc_rate(div-divider.hw, parent_rate);
 +}
 +
 +static long clk_divider_status_round_rate(struct clk_hw *hw, unsigned long 
 rate,
 +unsigned long *prate)
 +{
 + struct clk_divider_status *div = to_clk_div(hw);
 +
 + return div-ops-round_rate(div-divider.hw, rate, prate);
 +}
 +
 +static int clk_divider_status_set_rate(struct clk_hw *hw, unsigned long rate,
 + unsigned long parent_rate)
 +{
 + struct clk_divider_status *div = to_clk_div(hw);
 + int ret;
 +
 + ret = div-ops-set_rate(div-divider.hw, rate, parent_rate);
 + if (!ret) {
 + unsigned long timeout = jiffies + msecs_to_jiffies(10);
 +
 + while (readl_relaxed(div-reg)  (1  div-busy)) {
 + if (time_after(jiffies, timeout))
 + return -ETIMEDOUT;
 + }
 + }
 +
 + return ret;
 +}
 +
 +static struct clk_ops clk_divider_status_ops = {
 + .recalc_rate = clk_divider_status_recalc_rate,
 + .round_rate = clk_divider_status_round_rate,
 + .set_rate = clk_divider_status_set_rate,
 +};
 +EXPORT_SYMBOL_GPL(clk_divider_status_ops);
 +
 +struct clk *clk_register_divider_status(struct device *dev, const char *name,
 + const char *parent_name, unsigned long flags, void __iomem *reg,
 + u8 shift, u8 width, u8 clk_divider_flags,
 + void __iomem *reg_status, u8 shift_status, spinlock_t *lock)
 +{
 + 

Re: [PATCH] ARM: SAMSUNG: Set clock parent if provided

2013-03-18 Thread Sylwester Nawrocki
Hi Shaik,

On 03/15/2013 10:00 AM, Shaik Ameer Basha wrote:
 On Thu, Mar 7, 2013 at 9:05 PM, Sylwester Nawrocki
 s.nawro...@samsung.com wrote:
...
 May I ask what do you need this for ? This code won't be used for
 Exynos4 and Exynos5 SoCs starting from 3.10. And it is going to be
 removed once other platforms are converted to the new Samsung clocks
 driver.
 
 I had some issues in exynos5 with out this implementation.
 But yes... you are right, once we move to common clock framework (CCF)
 we don't require this change, as CCF doesn't use this (for exynos 4/5).
 
 What about all old non-dt based platforms?
 Cant they use this change until they move to DT and CCF?

I have nothing against the patch, I was just curious if you need
it for the new SOCs where the new clocks driver is supposed to be used.
Of course other Samsung platforms could switch to CCF before having
support for the device tree.

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


Re: [PATCH] clk: let mxs specific clk-div clock type be a generic clock type

2013-03-18 Thread Thomas Abraham
On 18 March 2013 14:12, Shawn Guo shawn@linaro.org wrote:
 On Sat, Mar 16, 2013 at 06:20:01PM +0530, Thomas Abraham wrote:
 The mxs platform specific clk-div clock is an extended version of the
 basic integer divider clock type that supports checking the stability
 status of the divider clock output. This type of clock is found on
 some of the Samsung platforms as well. So let the mxs specfic clk-div
 clock type be a generic clock type that all platforms can utilize.

 Cc: Shawn Guo shawn@linaro.org
 Cc: Mike Turquette mturque...@linaro.org
 Signed-off-by: Thomas Abraham thomas.abra...@linaro.org
 ---
  drivers/clk/Makefile |1 +
  drivers/clk/clk-divider-status.c |  119 
 ++
  drivers/clk/mxs/Makefile |2 +-
  drivers/clk/mxs/clk-div.c|  110 ---
  drivers/clk/mxs/clk.h|   12 +++-
  include/linux/clk-provider.h |   21 +++
  6 files changed, 151 insertions(+), 114 deletions(-)
  create mode 100644 drivers/clk/clk-divider-status.c
  delete mode 100644 drivers/clk/mxs/clk-div.c

 From my quick testing, it seems working for mxs platform.  But it's hard
 to review the changes.  Making it two steps might be helpful for
 reviewer:

 1) git mv drivers/clk/mxs/clk-div.c drivers/clk/clk-divider-status.c
 2) make changes on drivers/clk/clk-divider-status.c

Thanks Shawn for your comments. I will split this patch as you
suggested and post again.

Thomas.


 Shawn


 diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
 index 0147022..0ac851a 100644
 --- a/drivers/clk/Makefile
 +++ b/drivers/clk/Makefile
 @@ -7,6 +7,7 @@ obj-$(CONFIG_COMMON_CLK)  += clk-fixed-factor.o
  obj-$(CONFIG_COMMON_CLK) += clk-fixed-rate.o
  obj-$(CONFIG_COMMON_CLK) += clk-gate.o
  obj-$(CONFIG_COMMON_CLK) += clk-mux.o
 +obj-$(CONFIG_COMMON_CLK) += clk-divider-status.o

  # SoCs specific
  obj-$(CONFIG_ARCH_BCM2835)   += clk-bcm2835.o
 diff --git a/drivers/clk/clk-divider-status.c 
 b/drivers/clk/clk-divider-status.c
 new file mode 100644
 index 000..1d66059
 --- /dev/null
 +++ b/drivers/clk/clk-divider-status.c
 @@ -0,0 +1,119 @@
 +/*
 + * Copyright 2012 Freescale Semiconductor, Inc.
 + *
 + * The code contained herein is licensed under the GNU General Public
 + * License. You may obtain a copy of the GNU General Public License
 + * Version 2 or later at the following locations:
 + *
 + * http://www.opensource.org/licenses/gpl-license.html
 + * http://www.gnu.org/copyleft/gpl.html
 + *
 + * Extension to the adjustable divider clock implementation with support for
 + * divider clock stability checks.
 + */
 +
 +#include linux/clk.h
 +#include linux/module.h
 +#include linux/clk-provider.h
 +#include linux/err.h
 +#include linux/slab.h
 +#include linux/io.h
 +#include linux/jiffies.h
 +
 +/*
 + * DOC: Adjustable divider clock with support for divider stability check
 + *
 + * Traits of this clock:
 + * prepare - clk_prepare only ensures that parents are prepared
 + * enable - clk_enable only ensures that parents are enabled
 + * rate - rate is adjustable.  clk-rate = parent-rate / divisor
 + * parent - fixed parent.  No clk_set_parent support
 + */
 +
 +static inline struct clk_divider_status *to_clk_div(struct clk_hw *hw)
 +{
 + struct clk_divider *divider = container_of(hw, struct clk_divider, hw);
 +
 + return container_of(divider, struct clk_divider_status, divider);
 +}
 +
 +static unsigned long clk_divider_status_recalc_rate(struct clk_hw *hw,
 +  unsigned long parent_rate)
 +{
 + struct clk_divider_status *div = to_clk_div(hw);
 +
 + return div-ops-recalc_rate(div-divider.hw, parent_rate);
 +}
 +
 +static long clk_divider_status_round_rate(struct clk_hw *hw, unsigned long 
 rate,
 +unsigned long *prate)
 +{
 + struct clk_divider_status *div = to_clk_div(hw);
 +
 + return div-ops-round_rate(div-divider.hw, rate, prate);
 +}
 +
 +static int clk_divider_status_set_rate(struct clk_hw *hw, unsigned long 
 rate,
 + unsigned long parent_rate)
 +{
 + struct clk_divider_status *div = to_clk_div(hw);
 + int ret;
 +
 + ret = div-ops-set_rate(div-divider.hw, rate, parent_rate);
 + if (!ret) {
 + unsigned long timeout = jiffies + msecs_to_jiffies(10);
 +
 + while (readl_relaxed(div-reg)  (1  div-busy)) {
 + if (time_after(jiffies, timeout))
 + return -ETIMEDOUT;
 + }
 + }
 +
 + return ret;
 +}
 +
 +static struct clk_ops clk_divider_status_ops = {
 + .recalc_rate = clk_divider_status_recalc_rate,
 + .round_rate = clk_divider_status_round_rate,
 + .set_rate = clk_divider_status_set_rate,
 +};
 +EXPORT_SYMBOL_GPL(clk_divider_status_ops);
 +
 +struct clk *clk_register_divider_status(struct device *dev, const char 
 *name,
 + const char *parent_name, unsigned 

Re: [PATCH] clk: let mxs specific clk-div clock type be a generic clock type

2013-03-18 Thread Uwe Kleine-König
Hello,

On Mon, Mar 18, 2013 at 03:48:24PM +0530, Thomas Abraham wrote:
 On 18 March 2013 14:12, Shawn Guo shawn@linaro.org wrote:
  On Sat, Mar 16, 2013 at 06:20:01PM +0530, Thomas Abraham wrote:
  The mxs platform specific clk-div clock is an extended version of the
  basic integer divider clock type that supports checking the stability
  status of the divider clock output. This type of clock is found on
  some of the Samsung platforms as well. So let the mxs specfic clk-div
  clock type be a generic clock type that all platforms can utilize.
 
  Cc: Shawn Guo shawn@linaro.org
  Cc: Mike Turquette mturque...@linaro.org
  Signed-off-by: Thomas Abraham thomas.abra...@linaro.org
  ---
   drivers/clk/Makefile |1 +
   drivers/clk/clk-divider-status.c |  119 
  ++
   drivers/clk/mxs/Makefile |2 +-
   drivers/clk/mxs/clk-div.c|  110 
  ---
   drivers/clk/mxs/clk.h|   12 +++-
   include/linux/clk-provider.h |   21 +++
   6 files changed, 151 insertions(+), 114 deletions(-)
   create mode 100644 drivers/clk/clk-divider-status.c
   delete mode 100644 drivers/clk/mxs/clk-div.c
 
  From my quick testing, it seems working for mxs platform.  But it's hard
  to review the changes.  Making it two steps might be helpful for
  reviewer:
 
  1) git mv drivers/clk/mxs/clk-div.c drivers/clk/clk-divider-status.c
  2) make changes on drivers/clk/clk-divider-status.c
 
 Thanks Shawn for your comments. I will split this patch as you
 suggested and post again.
I didn't try to look at your patch, but maybe format-patch -M is enough
to make the patch easier to parse (for humans).

Best regards
Uwe

-- 
Pengutronix e.K.   | Uwe Kleine-König|
Industrial Linux Solutions | http://www.pengutronix.de/  |
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH V4 1/4] cpufreq: exynos: Adding cpufreq driver for exynos5440

2013-03-18 Thread amit kachhap
Hi Viresh,

On Tue, Mar 12, 2013 at 4:19 PM, Viresh Kumar viresh.ku...@linaro.org wrote:
 This is what Russell told me a long time back:
 Don't use Adding, Fixing, etc words as this work is not something, which is
 already done.

 So your subject should have been: cpufreq: exynos: Add cpufreq driver
 for exynos5440
ok right.

 Fix it if you need another version, which i believe you do :)
yes no escape now :)

 On Tue, Mar 12, 2013 at 5:58 PM, Amit Daniel Kachhap
 amit.dan...@samsung.com wrote:
 This patch adds dvfs support for exynos5440 SOC. This soc has 4 cores and
 they scale at same frequency. The nature of exynos5440 clock controller is
 different from previous exynos controllers so not using the common exynos
 cpufreq framework. The major difference being interrupt notfication for

 s/notfication/notification
ok

 frequency change. Also, OPP library is used for device tree parsing to get
 different parameters like frequency, voltage etc. Since the opp library sorts
 the frequency table in ascending order so they are again re-arranged in
 descending order. This will have one-to-one mapping with the clock controller
 state management logic.

 Signed-off-by: Amit Daniel Kachhap amit.dan...@samsung.com
 ---
  .../bindings/cpufreq/cpufreq-exynos5440.txt|   29 ++
  drivers/cpufreq/Kconfig.arm|9 +
  drivers/cpufreq/Makefile   |1 +
  drivers/cpufreq/exynos5440-cpufreq.c   |  466 
 
  4 files changed, 505 insertions(+), 0 deletions(-)
  create mode 100644 
 Documentation/devicetree/bindings/cpufreq/cpufreq-exynos5440.txt
  create mode 100644 drivers/cpufreq/exynos5440-cpufreq.c

 diff --git 
 a/Documentation/devicetree/bindings/cpufreq/cpufreq-exynos5440.txt 
 b/Documentation/devicetree/bindings/cpufreq/cpufreq-exynos5440.txt
 new file mode 100644
 index 000..a0dbe0b
 --- /dev/null
 +++ b/Documentation/devicetree/bindings/cpufreq/cpufreq-exynos5440.txt
 @@ -0,0 +1,29 @@
 +
 +Exynos5440 cpufreq driver
 +---
 +
 +Exynos5440 SoC cpufreq driver for CPU frequency scaling.
 +
 +Required properties:
 +- interrupts: Interrupt to know the completion of cpu frequency change.
 +- operating-points: Table of frequencies and voltage CPU could be 
 transitioned into,
 +   in the decreasing order. Frequency should be in KHZ units and voltage

 s/KHZ/KHz
ok

 +   should be in microvolts.

 probably s/microvolts/micro-volts ??

 +
 +Optional properties:
 +- clock-latency: Clock monitor latency in microsecond.
 +
 +All the required listed above must be defined under node cpufreq.
 +
 +Example:
 +
 +   cpufreq@16 {
 +   compatible = samsung,exynos5440-cpufreq;
 +   reg = 0x16 0x1000;
 +   interrupts = 0 57 0;
 +   operating-points = 
 +   100 975000
 +   80  925000;
 +   clock-latency = 10;
 +   };
 +

 diff --git a/drivers/cpufreq/exynos5440-cpufreq.c 
 b/drivers/cpufreq/exynos5440-cpufreq.c
 +static void exynos_enable_dvfs(void)
 +{

 +   /* Set initial performance index */
 +   for (i = 0; freq_table[i].frequency != CPUFREQ_TABLE_END; i++)
 +   if (freq_table[i].frequency == dvfs_info-cur_frequency)
 +   break;
 +
 +   if (freq_table[i].frequency == CPUFREQ_TABLE_END) {
 +   dev_crit(dvfs_info-dev, Boot up frequency not 
 supported\n);
 +   /* Assign the highest frequency */
 +   i = 0;
 +   dvfs_info-cur_frequency = freq_table[i].frequency;

 What about:

 dvfs_info-cur_frequency = freq_table[0].frequency;

 as i don't see i being used again?
No It is used below for frequency setting.

 +   }

 +}

 +static int exynos_target(struct cpufreq_policy *policy,
 + unsigned int target_freq,
 + unsigned int relation)
 +{

 +   if (cpufreq_frequency_table_target(policy, freq_table,
 +  target_freq, relation, index)) {
 +   ret = -EINVAL;

 Use the error value returned by called functions, probably i gave this
 comment last time too?
yes my mistake.

 +   goto out;
 +   }

 +}

 +static void exynos_sort_descend_freq_table(void)
 +{
 +   struct cpufreq_frequency_table *freq_tbl = dvfs_info-freq_table;
 +   int i = 0, index;
 +   unsigned int tmp_freq;
 +
 +   /*
 +* Freq table is already in ascending order as it is created from
 +* OPP library, so just swap the elements to make it descending.

 why??
I explained this requirement in the patch commit. Will explain it here again.

 +*/
 +   for (i = 0; i  dvfs_info-freq_count / 2; i++) {
 +   index = dvfs_info-freq_count - i - 1;
 +   tmp_freq = freq_tbl[i].frequency;
 +   freq_tbl[i].frequency = freq_tbl[index].frequency;
 +  

Re: [RFC PATCH 5/8] s5p-fimc: Add ISP video capture driver stubs

2013-03-18 Thread Hans Verkuil
On Sun March 17 2013 22:03:38 Sylwester Nawrocki wrote:
 On 03/12/2013 03:44 PM, Hans Verkuil wrote:
  On Mon 11 March 2013 20:44:49 Sylwester Nawrocki wrote:
 [...]
  +static int isp_video_capture_open(struct file *file)
  +{
  +  struct fimc_isp *isp = video_drvdata(file);
  +  int ret = 0;
  +
  +  if (mutex_lock_interruptible(isp-video_lock))
  +  return -ERESTARTSYS;
  +
  +  /* ret = pm_runtime_get_sync(isp-pdev-dev); */
  +  if (ret  0)
  +  goto done;
  +
  +  ret = v4l2_fh_open(file);
  +  if (ret  0)
  +  goto done;
  +
  +  /* TODO: prepare video pipeline */
  +done:
  +  mutex_unlock(isp-video_lock);
  +  return ret;
  +}
  +
  +static int isp_video_capture_close(struct file *file)
  +{
  +  struct fimc_isp *isp = video_drvdata(file);
  +  int ret = 0;
  +
  +  mutex_lock(isp-video_lock);
  +
  +  if (isp-out_path == FIMC_IO_DMA) {
  +  /* TODO: stop capture, cleanup */
  +  }
  +
  +  /* pm_runtime_put(isp-pdev-dev); */
  +
  +  if (isp-ref_count == 0)
  +  vb2_queue_release(isp-capture_vb_queue);
  +
  +  ret = v4l2_fh_release(file);
  +
  +  mutex_unlock(isp-video_lock);
  +  return ret;
  +}
  +
  +static unsigned int isp_video_capture_poll(struct file *file,
  + struct poll_table_struct *wait)
  +{
  +  struct fimc_isp *isp = video_drvdata(file);
  +  int ret;
  +
  +  mutex_lock(isp-video_lock);
  +  ret = vb2_poll(isp-capture_vb_queue, file, wait);
  +  mutex_unlock(isp-video_lock);
  +  return ret;
  +}
  +
  +static int isp_video_capture_mmap(struct file *file, struct 
  vm_area_struct *vma)
  +{
  +  struct fimc_isp *isp = video_drvdata(file);
  +  int ret;
  +
  +  if (mutex_lock_interruptible(isp-video_lock))
  +  return -ERESTARTSYS;
  +
  +  ret = vb2_mmap(isp-capture_vb_queue, vma);
  +  mutex_unlock(isp-video_lock);
  +
  +  return ret;
  +}
  +
  +static const struct v4l2_file_operations isp_video_capture_fops = {
  +  .owner  = THIS_MODULE,
  +  .open   = isp_video_capture_open,
  +  .release= isp_video_capture_close,
  +  .poll   = isp_video_capture_poll,
  +  .unlocked_ioctl = video_ioctl2,
  +  .mmap   = isp_video_capture_mmap,
 
  Can't you use the helper functions vb2_fop_open/release/poll/mmap here?
 
 It seems vb2_fop_mmap/poll can be used directly, open(), release() are
 a bit more complicated as some media pipeline operations need to
 additionally be done within these callbacks. There is no vb2_fop_open(),
 and AFAICS v4l2_fh_open() is sufficient and intended as open() helper.

That's correct. Sorry for the misinformation about the non-existant
vb2_fop_open.

 For the next iteration I have used vb2_fop_release(), called indirectly,
 as it nicely simplifies things a bit.
 
 BTW, shouldn't vb2_fop_release() also be taking the lock ? Actually it is
 more useful for me in current form, but the drivers that directly assign
 it to struct v4l2_file_operations::open might be in trouble, unless I'm
 missing something.

I don't see where a lock would be needed. I don't see any concurrency here.
Nobody else can mess with the queue as long as they are not the owner.

 
  +};
  +
  +/*
  + * Video node ioctl operations
  + */
 [...]
  +static int fimc_isp_capture_streamon(struct file *file, void *priv,
  +   enum v4l2_buf_type type)
  +{
  +  struct fimc_isp *isp = video_drvdata(file);
  +  struct v4l2_subdev *sensor = isp-pipeline.subdevs[IDX_SENSOR];
  +  struct fimc_pipeline *p =isp-pipeline;
  +  int ret;
  +
  +  /* TODO: check if the OTF interface is not running */
  +
  +  ret = media_entity_pipeline_start(sensor-entity, p-m_pipeline);
  +  if (ret  0)
  +  return ret;
  +
  +  ret = fimc_isp_pipeline_validate(isp);
  +  if (ret) {
  +  media_entity_pipeline_stop(sensor-entity);
  +  return ret;
  +  }
  +
  +  return vb2_streamon(isp-capture_vb_queue, type);
  +}
  +
  +static int fimc_isp_capture_streamoff(struct file *file, void *priv,
  +enum v4l2_buf_type type)
  +{
  +  struct fimc_isp *isp = video_drvdata(file);
  +  struct v4l2_subdev *sd = isp-pipeline.subdevs[IDX_SENSOR];
  +  int ret;
  +
  +  ret = vb2_streamoff(isp-capture_vb_queue, type);
  +  if (ret == 0)
  +  media_entity_pipeline_stop(sd-entity);
  +  return ret;
  +}
  +
  +static int fimc_isp_capture_reqbufs(struct file *file, void *priv,
  +  struct v4l2_requestbuffers *reqbufs)
  +{
  +  struct fimc_isp *isp = video_drvdata(file);
  +  int ret;
  +
  +  reqbufs-count = max_t(u32, FIMC_IS_REQ_BUFS_MIN, reqbufs-count);
  +  ret = vb2_reqbufs(isp-capture_vb_queue, reqbufs);
 
  You probably want to call vb2_ioctl_reqbufs here since that does additional
  ownership checks that vb2_reqbufs doesn't.
 
 Yes, thanks for the suggestion. That was actually helpful, previously
 it wasn't immediately clear to me one can still take advantage of those
 vb2_ioctl_* helpers, mainly have 

Re: [PATCH] ARM: dts: add interrupt-names property to get interrupt resource by name

2013-03-18 Thread Rob Herring
On 03/13/2013 05:42 PM, Sylwester Nawrocki wrote:
 Rob,
 
 On 03/13/2013 03:39 PM, Rob Herring wrote:
 I fail to see what the hack is. The order of interrupt properties must
 be defined by the binding. interrupt-names is auxiliary data and must
 not be required by an OS.
 
 It is clear that the order of the interrupts must be defined by the
 bindings. But how useful resource-names properties are when we
 cannot define them as required ? If an OS cannot rely on them then
 it must use some other, reliable, method to identify the resources,
 e.g. by hard coding the indexes. If we have to do it then why even
 bother with the resource-names properties ? I can see interrupt-names
 property specified as required in at least 2 bindings' documentation
 and all bindings having reg-names property define it as required.
 Are they wrong them ?

You can require the name for a binding definition, but that does not
remove the requirement that the order and index of interrupts also be
defined by the binding. Then it is up to the OS to use names or
hard-coded indexes. Hard-coded indexes are not a hack. This is how FDT
and OF are defined to work.

I'm still not clear how changing the order of the interrupts removes a hack.

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


Re: [PATCH v3 6/6] irqchip: s3c24xx: add s3c2450 interrupt definitions

2013-03-18 Thread Rob Herring
On 03/17/2013 08:09 AM, Heiko Stübner wrote:
 The s3c2450 is special in that it shares the cpu identification with the
 s3c2416 but provides more interrupts for its additional components.
 
 It also shares the layout of the main interrupt register with the s3c2443
 and therefore reuses this definition.
 
 As no non-dt boards are present, the s3c2450 irqs will only be
 accessible thru devicetree.
 
 Signed-off-by: Heiko Stuebner he...@sntech.de
 ---
  drivers/irqchip/irq-s3c24xx.c |   62 +++-
  1 files changed, 60 insertions(+), 2 deletions(-)
 
 diff --git a/drivers/irqchip/irq-s3c24xx.c b/drivers/irqchip/irq-s3c24xx.c
 index 55cb363..7ddf8e8 100644
 --- a/drivers/irqchip/irq-s3c24xx.c
 +++ b/drivers/irqchip/irq-s3c24xx.c
 @@ -852,6 +852,51 @@ static struct s3c_irq_data init_s3c2416_second[32] = {
   { .type = S3C_IRQTYPE_EDGE }, /* I2S0 */
  };
  
 +static struct s3c_irq_data init_s3c2450subint[32] = {
 + { .type = S3C_IRQTYPE_LEVEL, .parent_irq = 28 }, /* UART0-RX */
 + { .type = S3C_IRQTYPE_LEVEL, .parent_irq = 28 }, /* UART0-TX */
 + { .type = S3C_IRQTYPE_LEVEL, .parent_irq = 28 }, /* UART0-ERR */
 + { .type = S3C_IRQTYPE_LEVEL, .parent_irq = 23 }, /* UART1-RX */
 + { .type = S3C_IRQTYPE_LEVEL, .parent_irq = 23 }, /* UART1-TX */
 + { .type = S3C_IRQTYPE_LEVEL, .parent_irq = 23 }, /* UART1-ERR */
 + { .type = S3C_IRQTYPE_LEVEL, .parent_irq = 15 }, /* UART2-RX */
 + { .type = S3C_IRQTYPE_LEVEL, .parent_irq = 15 }, /* UART2-TX */
 + { .type = S3C_IRQTYPE_LEVEL, .parent_irq = 15 }, /* UART2-ERR */
 + { .type = S3C_IRQTYPE_EDGE, .parent_irq = 31 }, /* TC */
 + { .type = S3C_IRQTYPE_EDGE, .parent_irq = 31 }, /* ADC */
 + { .type = S3C_IRQTYPE_LEVEL, .parent_irq = 6 }, /* CAM_C */
 + { .type = S3C_IRQTYPE_LEVEL, .parent_irq = 6 }, /* CAM_P */
 + { .type = S3C_IRQTYPE_NONE }, /* reserved */
 + { .type = S3C_IRQTYPE_NONE }, /* reserved */
 + { .type = S3C_IRQTYPE_LEVEL, .parent_irq = 16 }, /* LCD2 */
 + { .type = S3C_IRQTYPE_LEVEL, .parent_irq = 16 }, /* LCD3 */
 + { .type = S3C_IRQTYPE_LEVEL, .parent_irq = 16 }, /* LCD4 */
 + { .type = S3C_IRQTYPE_LEVEL, .parent_irq = 17 }, /* DMA0 */
 + { .type = S3C_IRQTYPE_LEVEL, .parent_irq = 17 }, /* DMA1 */
 + { .type = S3C_IRQTYPE_LEVEL, .parent_irq = 17 }, /* DMA2 */
 + { .type = S3C_IRQTYPE_LEVEL, .parent_irq = 17 }, /* DMA3 */
 + { .type = S3C_IRQTYPE_LEVEL, .parent_irq = 17 }, /* DMA4 */
 + { .type = S3C_IRQTYPE_LEVEL, .parent_irq = 17 }, /* DMA5 */
 + { .type = S3C_IRQTYPE_LEVEL, .parent_irq = 18 }, /* UART3-RX */
 + { .type = S3C_IRQTYPE_LEVEL, .parent_irq = 18 }, /* UART3-TX */
 + { .type = S3C_IRQTYPE_LEVEL, .parent_irq = 18 }, /* UART3-ERR */
 + { .type = S3C_IRQTYPE_LEVEL, .parent_irq = 9 }, /* WDT */
 + { .type = S3C_IRQTYPE_LEVEL, .parent_irq = 9 }, /* AC97 */
 + { .type = S3C_IRQTYPE_LEVEL, .parent_irq = 17 }, /* DMA6 */
 + { .type = S3C_IRQTYPE_LEVEL, .parent_irq = 17 }, /* DMA7 */

This all seems like information that should come from DT.

 +};
 +
 +static struct s3c_irq_data init_s3c2450second[32] = {
 + { .type = S3C_IRQTYPE_EDGE }, /* 2D */
 + { .type = S3C_IRQTYPE_EDGE }, /* IIC1 */
 + { .type = S3C_IRQTYPE_NONE }, /* reserved */
 + { .type = S3C_IRQTYPE_NONE }, /* reserved */
 + { .type = S3C_IRQTYPE_EDGE }, /* PCM0 */
 + { .type = S3C_IRQTYPE_EDGE }, /* PCM1 */
 + { .type = S3C_IRQTYPE_EDGE }, /* I2S0 */
 + { .type = S3C_IRQTYPE_EDGE }, /* I2S1 */
 +};
 +
  void __init s3c2416_init_irq(void)
  {
   struct s3c_irq_intc *main_intc;
 @@ -1024,7 +1069,7 @@ void __init s3c2442_init_irq(void)
  }
  #endif
  
 -#ifdef CONFIG_CPU_S3C2443
 +#if defined(CONFIG_CPU_S3C2443) || defined(CONFIG_CPU_S3C2416)
  static struct s3c_irq_data init_s3c2443base[32] = {
   { .type = S3C_IRQTYPE_EINT, }, /* EINT0 */
   { .type = S3C_IRQTYPE_EINT, }, /* EINT1 */
 @@ -1059,8 +1104,9 @@ static struct s3c_irq_data init_s3c2443base[32] = {
   { .type = S3C_IRQTYPE_EDGE, }, /* RTC */
   { .type = S3C_IRQTYPE_LEVEL, }, /* ADCPARENT */
  };
 +#endif
  
 -
 +#ifdef CONFIG_CPU_S3C2443
  static struct s3c_irq_data init_s3c2443subint[32] = {
   { .type = S3C_IRQTYPE_LEVEL, .parent_irq = 28 }, /* UART0-RX */
   { .type = S3C_IRQTYPE_LEVEL, .parent_irq = 28 }, /* UART0-TX */
 @@ -1172,6 +1218,17 @@ struct s3c24xx_irq_of_data s3c2416_irq_data = {
   .irq_ctrl = s3c2416_ctrl,
   .num_ctrl = ARRAY_SIZE(s3c2416_ctrl),
  };
 +
 +static struct s3c24xx_irq_of_ctrl s3c2450_ctrl[] = {
 + S3C24XX_IRQCTRL(intc, 0, init_s3c2443base, main_intc, NULL),
 + S3C24XX_IRQCTRL(subintc, 0x18, init_s3c2450subint, NULL, main_intc),
 + S3C24XX_IRQCTRL(intc2, 0x40, init_s3c2450second, main_intc2, NULL),
 +};
 +
 +static struct s3c24xx_irq_of_data s3c2450_irq_data = {
 + .irq_ctrl = s3c2450_ctrl,
 + .num_ctrl = ARRAY_SIZE(s3c2450_ctrl),
 +};
  #endif
  
  #ifdef 

Re: [PATCH v3 6/6] irqchip: s3c24xx: add s3c2450 interrupt definitions

2013-03-18 Thread Heiko Stübner
Hi Rob,

Am Montag, 18. März 2013, 16:54:03 schrieb Rob Herring:
 On 03/17/2013 08:09 AM, Heiko Stübner wrote:
  The s3c2450 is special in that it shares the cpu identification with the
  s3c2416 but provides more interrupts for its additional components.
  
  It also shares the layout of the main interrupt register with the s3c2443
  and therefore reuses this definition.
  
  As no non-dt boards are present, the s3c2450 irqs will only be
  accessible thru devicetree.
  
  Signed-off-by: Heiko Stuebner he...@sntech.de
  ---
  
   drivers/irqchip/irq-s3c24xx.c |   62
   +++- 1 files changed, 60
   insertions(+), 2 deletions(-)
  
  diff --git a/drivers/irqchip/irq-s3c24xx.c
  b/drivers/irqchip/irq-s3c24xx.c index 55cb363..7ddf8e8 100644
  --- a/drivers/irqchip/irq-s3c24xx.c
  +++ b/drivers/irqchip/irq-s3c24xx.c
  @@ -852,6 +852,51 @@ static struct s3c_irq_data init_s3c2416_second[32] =
  {
  
  { .type = S3C_IRQTYPE_EDGE }, /* I2S0 */
   
   };
  
  +static struct s3c_irq_data init_s3c2450subint[32] = {
  +   { .type = S3C_IRQTYPE_LEVEL, .parent_irq = 28 }, /* UART0-RX */
  +   { .type = S3C_IRQTYPE_LEVEL, .parent_irq = 28 }, /* UART0-TX */
  +   { .type = S3C_IRQTYPE_LEVEL, .parent_irq = 28 }, /* UART0-ERR */
  +   { .type = S3C_IRQTYPE_LEVEL, .parent_irq = 23 }, /* UART1-RX */
  +   { .type = S3C_IRQTYPE_LEVEL, .parent_irq = 23 }, /* UART1-TX */
  +   { .type = S3C_IRQTYPE_LEVEL, .parent_irq = 23 }, /* UART1-ERR */
  +   { .type = S3C_IRQTYPE_LEVEL, .parent_irq = 15 }, /* UART2-RX */
  +   { .type = S3C_IRQTYPE_LEVEL, .parent_irq = 15 }, /* UART2-TX */
  +   { .type = S3C_IRQTYPE_LEVEL, .parent_irq = 15 }, /* UART2-ERR */
  +   { .type = S3C_IRQTYPE_EDGE, .parent_irq = 31 }, /* TC */
  +   { .type = S3C_IRQTYPE_EDGE, .parent_irq = 31 }, /* ADC */
  +   { .type = S3C_IRQTYPE_LEVEL, .parent_irq = 6 }, /* CAM_C */
  +   { .type = S3C_IRQTYPE_LEVEL, .parent_irq = 6 }, /* CAM_P */
  +   { .type = S3C_IRQTYPE_NONE }, /* reserved */
  +   { .type = S3C_IRQTYPE_NONE }, /* reserved */
  +   { .type = S3C_IRQTYPE_LEVEL, .parent_irq = 16 }, /* LCD2 */
  +   { .type = S3C_IRQTYPE_LEVEL, .parent_irq = 16 }, /* LCD3 */
  +   { .type = S3C_IRQTYPE_LEVEL, .parent_irq = 16 }, /* LCD4 */
  +   { .type = S3C_IRQTYPE_LEVEL, .parent_irq = 17 }, /* DMA0 */
  +   { .type = S3C_IRQTYPE_LEVEL, .parent_irq = 17 }, /* DMA1 */
  +   { .type = S3C_IRQTYPE_LEVEL, .parent_irq = 17 }, /* DMA2 */
  +   { .type = S3C_IRQTYPE_LEVEL, .parent_irq = 17 }, /* DMA3 */
  +   { .type = S3C_IRQTYPE_LEVEL, .parent_irq = 17 }, /* DMA4 */
  +   { .type = S3C_IRQTYPE_LEVEL, .parent_irq = 17 }, /* DMA5 */
  +   { .type = S3C_IRQTYPE_LEVEL, .parent_irq = 18 }, /* UART3-RX */
  +   { .type = S3C_IRQTYPE_LEVEL, .parent_irq = 18 }, /* UART3-TX */
  +   { .type = S3C_IRQTYPE_LEVEL, .parent_irq = 18 }, /* UART3-ERR */
  +   { .type = S3C_IRQTYPE_LEVEL, .parent_irq = 9 }, /* WDT */
  +   { .type = S3C_IRQTYPE_LEVEL, .parent_irq = 9 }, /* AC97 */
  +   { .type = S3C_IRQTYPE_LEVEL, .parent_irq = 17 }, /* DMA6 */
  +   { .type = S3C_IRQTYPE_LEVEL, .parent_irq = 17 }, /* DMA7 */
 
 This all seems like information that should come from DT.

In the first iterations [0] of theis series it was done this way, but was 
suggested that these informations _might_ be implementation specific and not 
describing the hardware.

As I didn't get any final feedback on the matter, I tried it this way this 
time. Personally I also did like the previous variant better, as the driver 
could loose all the declaration stuff when platforms move to dt.

I would be glad for a hint if the first approach was the correct one.


[0] irqchip: irq-s3c24xx: add devicetree support from 2013-02-18, also with 
you in the recipient list

 
  +};
  +
  +static struct s3c_irq_data init_s3c2450second[32] = {
  +   { .type = S3C_IRQTYPE_EDGE }, /* 2D */
  +   { .type = S3C_IRQTYPE_EDGE }, /* IIC1 */
  +   { .type = S3C_IRQTYPE_NONE }, /* reserved */
  +   { .type = S3C_IRQTYPE_NONE }, /* reserved */
  +   { .type = S3C_IRQTYPE_EDGE }, /* PCM0 */
  +   { .type = S3C_IRQTYPE_EDGE }, /* PCM1 */
  +   { .type = S3C_IRQTYPE_EDGE }, /* I2S0 */
  +   { .type = S3C_IRQTYPE_EDGE }, /* I2S1 */
  +};
  +
  
   void __init s3c2416_init_irq(void)
   {
   
  struct s3c_irq_intc *main_intc;
  
  @@ -1024,7 +1069,7 @@ void __init s3c2442_init_irq(void)
  
   }
   #endif
  
  -#ifdef CONFIG_CPU_S3C2443
  +#if defined(CONFIG_CPU_S3C2443) || defined(CONFIG_CPU_S3C2416)
  
   static struct s3c_irq_data init_s3c2443base[32] = {
   
  { .type = S3C_IRQTYPE_EINT, }, /* EINT0 */
  { .type = S3C_IRQTYPE_EINT, }, /* EINT1 */
  
  @@ -1059,8 +1104,9 @@ static struct s3c_irq_data init_s3c2443base[32] = {
  
  { .type = S3C_IRQTYPE_EDGE, }, /* RTC */
  { .type = S3C_IRQTYPE_LEVEL, }, /* ADCPARENT */
   
   };
  
  +#endif
  
  -
  +#ifdef CONFIG_CPU_S3C2443
  
   static struct s3c_irq_data init_s3c2443subint[32] = {
   
  { .type = S3C_IRQTYPE_LEVEL, .parent_irq = 28 

Re: [PATCH] ARM: dts: add interrupt-names property to get interrupt resource by name

2013-03-18 Thread Stephen Warren
On 03/18/2013 09:50 AM, Rob Herring wrote:
 On 03/13/2013 05:42 PM, Sylwester Nawrocki wrote:
 Rob,

 On 03/13/2013 03:39 PM, Rob Herring wrote:
 I fail to see what the hack is. The order of interrupt properties must
 be defined by the binding. interrupt-names is auxiliary data and must
 not be required by an OS.

Is that true for all foo-names properties, or only for interrupt-names?
I was under the impression that foo-names was specifically invented so
that the order of the entries didn't matter, and instead they could be
requested by name.
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v3 5/6] irqchip: s3c24xx: add devicetree support

2013-03-18 Thread Rob Herring
On 03/17/2013 08:07 AM, Heiko Stübner wrote:
 Add the necessary code to initialize the interrupt controller
 thru devicetree data using the irqchip infrastructure.
 
 On dt machines the eint-type interrupts in the main interrupt controller
 get mapped as regular edge-types, as their wakeup and interrupt type
 properties will be handled by the upcoming pinctrl driver.
 
 Signed-off-by: Heiko Stuebner he...@sntech.de
 ---
  .../interrupt-controller/samsung,s3c24xx-irq.txt   |   54 +
  drivers/irqchip/irq-s3c24xx.c  |  222 
 
  2 files changed, 276 insertions(+), 0 deletions(-)
  create mode 100644 
 Documentation/devicetree/bindings/interrupt-controller/samsung,s3c24xx-irq.txt
 
 diff --git 
 a/Documentation/devicetree/bindings/interrupt-controller/samsung,s3c24xx-irq.txt
  
 b/Documentation/devicetree/bindings/interrupt-controller/samsung,s3c24xx-irq.txt
 new file mode 100644
 index 000..be5dead
 --- /dev/null
 +++ 
 b/Documentation/devicetree/bindings/interrupt-controller/samsung,s3c24xx-irq.txt
 @@ -0,0 +1,54 @@
 +Samsung S3C24XX Interrupt Controllers
 +
 +The S3C24XX SoCs contain a custom set of interrupt controllers providing a
 +varying number of interrupt sources. The set consists of a main- and sub-
 +controller and on newer SoCs even a second main controller.
 +
 +Required properties:
 +- compatible: Compatible property value should be one of 
 samsung,s3c2410-irq,
 +  samsung,s3c2412-irq, samsung,s3c2416-irq, samsung,s3c2440-irq,
 +  samsung,s3c2442-irq, samsung,s3c2443-irq depending on the SoC variant.
 +
 +- reg: Physical base address of the controller and length of memory mapped
 +  region.
 +
 +- interrupt-controller : Identifies the node as an interrupt controller
 +
 +Sub-controllers as child nodes:
 +  The interrupt controllers that should be referenced by device nodes are
 +  represented by child nodes. Valid names are intc, subintc and intc2.
 +  The interrupt values in device nodes are then mapped directly to the
 +  bit-numbers of the pending register of the named interrupt controller.
 +
 +Required properties:
 +- interrupt-controller : Identifies the node as an interrupt controller
 +
 +- #interrupt-cells : Specifies the number of cells needed to encode an
 +  interrupt source. The value shall be 2.
 +
 +Example:
 +
 + interrupt-controller@4a00 {
 + compatible = samsung,s3c2416-irq;
 + reg = 0x4a00 0x100;
 + interrupt-controller;
 +
 + intc:intc {
 + interrupt-controller;
 + #interrupt-cells = 2;
 + };
 +
 + subintc:subintc {
 + interrupt-controller;
 + #interrupt-cells = 2;
 + };
 + };
 +
 + [...]
 +
 + serial@5000 {
 + compatible = samsung,s3c2410-uart;
 + reg = 0x5000 0x4000;
 + interrupt-parent = subintc;
 + interrupts = 0 0, 1 0;
 + };
 diff --git a/drivers/irqchip/irq-s3c24xx.c b/drivers/irqchip/irq-s3c24xx.c
 index 1eba289..55cb363 100644
 --- a/drivers/irqchip/irq-s3c24xx.c
 +++ b/drivers/irqchip/irq-s3c24xx.c
 @@ -25,6 +25,9 @@
  #include linux/ioport.h
  #include linux/device.h
  #include linux/irqdomain.h
 +#include linux/of.h
 +#include linux/of_irq.h
 +#include linux/of_address.h
  
  #include asm/exception.h
  #include asm/mach/irq.h
 @@ -36,6 +39,8 @@
  #include plat/regs-irqtype.h
  #include plat/pm.h
  
 +#include irqchip.h
 +
  #define S3C_IRQTYPE_NONE 0
  #define S3C_IRQTYPE_EINT 1
  #define S3C_IRQTYPE_EDGE 2
 @@ -380,6 +385,10 @@ static int s3c24xx_irq_map(struct irq_domain *h, 
 unsigned int virq,
  
   parent_intc = intc-parent;
  
 + /* on dt platforms the extints get handled by the pinctrl driver */
 + if (h-of_node  irq_data-type == S3C_IRQTYPE_EINT)
 + irq_data-type = S3C_IRQTYPE_EDGE;
 +
   /* set handler and flags */
   switch (irq_data-type) {
   case S3C_IRQTYPE_NONE:
 @@ -1104,3 +1113,216 @@ void __init s3c2443_init_irq(void)
   s3c24xx_init_intc(NULL, init_s3c2443subint[0], main_intc, 0x4a18);
  }
  #endif
 +
 +#ifdef CONFIG_OF
 +struct s3c24xx_irq_of_ctrl {
 + char*name;
 + unsigned long   offset;
 + struct s3c_irq_data *irq_data;
 + struct s3c_irq_intc **handle;
 + struct s3c_irq_intc **parent;
 +};
 +
 +#define S3C24XX_IRQCTRL(n, o, d, h, p)   \
 +{\
 + .name   = n,\
 + .offset = o,\
 + .irq_data   = d,\
 + .handle = h,\
 + .parent = p,\
 +}
 +
 +struct s3c24xx_irq_of_data {
 + struct s3c24xx_irq_of_ctrl  *irq_ctrl;
 + int num_ctrl;
 +};
 +
 +#ifdef CONFIG_CPU_S3C2410
 +static struct 

[PATCH] [media] Add a V4L2 OF parser

2013-03-18 Thread Sylwester Nawrocki
From: Guennadi Liakhovetski g.liakhovet...@gmx.de

Add a V4L2 OF parser, implementing bindings documented in
Documentation/devicetree/bindings/media/video-interfaces.txt.

Signed-off-by: Guennadi Liakhovetski g.liakhovet...@gmx.de
[s.nawro...@samsung.com: various corrections and improvements
since the initial version]
Signed-off-by: Sylwester Nawrocki s.nawro...@samsung.com
Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com

---
Geunnadi,

I have made couple further changes to this parsing library, mostly
addressing your review comments. Hopefully it looks better now.

It would be good to get more reviews and opinions of other media
sub-maintainers. Any Acked/Tested-by are also of course most
welcome.

I have a feeling this is about ready for upstream ;-) We have
used it in couple subsystems already and the overall design
looks good and seems to be proving itself in practice.

Thanks,
Sylwester

The last version of the bindings documentation patch can be found
at: https://patchwork.kernel.org/patch/2074951

Changes since v7 include mostly corrections of issues pointed
out by Guennadi, and few others:

 - v4l2_of_parse_parallel_bus() and v4l2_of_parse_csi_bus() are
   made static and are not exported anymore;
 - swapped order of parsing the serial and parallel bus properties
   in v4l2_of_parse_endpoint() function; the rationale is that
   MIPI CSI-2 has less properties and an overall number of searches
   of non-existent properties is less this way;
 - v4l2_mbus_mipi_csi2, v4l2_mbus_parallel structs renamed to
   v4l2_of_bus_mipi_csi2, v4l2_of_bus_parallel respectively;
 - added kernel-doc for the above two data structures;
 - the flags field split into to fields - separate fields for the
   parallel and serial bus, V4L2_MBUS_* bit flags are defined
   separately for MIPI CSI-2 and parallel busses so they were
   clashing when used together in single data structure member!;
 - the head member of struct v4l2_endpoint made last member of
   this data structure, it is now not cleared with memset() at the
   beginning of v4l2_of_parse_endpoint() function; I believe
   clearing list head in this function is not expected;
 - fixed bug in v4l2_of_get_remote_port_parent() function;
 - reworked first port node searching in v4l2_of_get_next_endpoint();
 - moved code setting V4L2_MBUS_MASTER and V4L2_MBUS_CSI2_CONTINUOUS_CLOCK
   out from v4l2_of_parse_endpoint() to v4l2_of_parse_mipi_csi2/
   parallel_bus() functions;
 - ensured we recognize presence of the clock-lanes property regardless
   of the clock lane index being 0;
 - added v4l2_of_get_remote_port() function.

Changes since v6:
 - minor v4l2_of_get_remote_port_parent() function cleanup.

Changes since v5:
 - renamed v4l2_of_parse_mipi_csi2 - v4l2_of_parse_csi_bus;
 - corrected v4l2_of_get_remote_port_parent() function declaration
   for !CONFIG_OF;
 - reworked v4l2_of_get_next_endpoint() function to consider the
   'port' nodes can be grouped under optional 'ports' node;
 - added kerneldoc description for v4l2_of_get_next_endpoint()
   function.

Changes since v4:
 - reworked v4l2_of_get_remote_port() function to consider cases
   where 'port' nodes are grouped in a parent 'ports' node;
 - rearranged struct v4l2_of_endpoint and related changes added
   in the parser code;
 - added kerneldoc description for struct v4l2_of_endpoint;
 - s/link/endpoint in the comments.
---
 drivers/media/v4l2-core/Makefile  |3 +
 drivers/media/v4l2-core/v4l2-of.c |  267 +
 include/media/v4l2-of.h   |  111 +++
 3 files changed, 381 insertions(+)
 create mode 100644 drivers/media/v4l2-core/v4l2-of.c
 create mode 100644 include/media/v4l2-of.h

diff --git a/drivers/media/v4l2-core/Makefile b/drivers/media/v4l2-core/Makefile
index c2d61d4..00f64d6 100644
--- a/drivers/media/v4l2-core/Makefile
+++ b/drivers/media/v4l2-core/Makefile
@@ -9,6 +9,9 @@ videodev-objs   :=  v4l2-dev.o v4l2-ioctl.o v4l2-device.o 
v4l2-fh.o \
 ifeq ($(CONFIG_COMPAT),y)
   videodev-objs += v4l2-compat-ioctl32.o
 endif
+ifeq ($(CONFIG_OF),y)
+  videodev-objs += v4l2-of.o
+endif

 obj-$(CONFIG_VIDEO_DEV) += videodev.o v4l2-int-device.o
 obj-$(CONFIG_VIDEO_V4L2) += v4l2-common.o
diff --git a/drivers/media/v4l2-core/v4l2-of.c 
b/drivers/media/v4l2-core/v4l2-of.c
new file mode 100644
index 000..e38e210
--- /dev/null
+++ b/drivers/media/v4l2-core/v4l2-of.c
@@ -0,0 +1,267 @@
+/*
+ * V4L2 OF binding parsing library
+ *
+ * Copyright (C) 2012 - 2013 Samsung Electronics Co., Ltd.
+ * Sylwester Nawrocki s.nawro...@samsung.com
+ *
+ * Copyright (C) 2012 Renesas Electronics Corp.
+ * Author: Guennadi Liakhovetski g.liakhovet...@gmx.de
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of version 2 of the GNU General Public License as
+ * published by the Free Software Foundation.
+ */
+#include linux/kernel.h
+#include linux/module.h
+#include linux/of.h
+#include linux/string.h
+#include 

[PATCH 0/6] pinctrl: Add support for pin control on S3C64xx

2013-03-18 Thread Tomasz Figa
This series makes necessary preparations to add support for pin controller
available on Samsung S3C64xx using pinctrl-samsung driver and then adds
pinctrl-s3c64xx driver which implements SoC-specific part of the code.

It has been tested on a tiny6410 (mini6410-compatible) board with my patches
for samsung-time cleanup and S3C64xx Device Tree support:
 - http://thread.gmane.org/gmane.linux.kernel.samsung-soc/16664
 - http://www.mail-archive.com/linux-samsung-soc@vger.kernel.org/msg16325.html

Just the driver is added for now, as this is the part that can safely go
through Linus' pinctrl tree. I will post appropriate enablement patches
after all the dependencies get merged to Kgene's tree.

See particular patches for more detailed description of all changes.

Tomasz Figa (6):
  pinctrl: samsung: Protect bank registers with a spinlock
  pinctrl: samsung: Include pinctrl-exynos driver data conditionally
  pinctrl: samsung: Split pin bank description into two structures
  pinctrl: samsung: Remove hardcoded register offsets
  pinctrl: samsung: Handle banks with two configuration registers
  pinctrl: Add pinctrl-s3c64xx driver

 .../bindings/pinctrl/samsung-pinctrl.txt   |   3 +
 drivers/pinctrl/Kconfig|   5 +
 drivers/pinctrl/Makefile   |   1 +
 drivers/pinctrl/pinctrl-exynos.c   |  36 +-
 drivers/pinctrl/pinctrl-exynos.h   |  16 +-
 drivers/pinctrl/pinctrl-s3c64xx.c  | 817 +
 drivers/pinctrl/pinctrl-samsung.c  |  99 ++-
 drivers/pinctrl/pinctrl-samsung.h  |  42 +-
 8 files changed, 947 insertions(+), 72 deletions(-)
 create mode 100644 drivers/pinctrl/pinctrl-s3c64xx.c

-- 
1.8.1.5

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


[PATCH 5/6] pinctrl: samsung: Handle banks with two configuration registers

2013-03-18 Thread Tomasz Figa
This patch adds support for banks that have more than one function
configuration registers, e.g. some of the banks of S3C64xx SoCs.

Signed-off-by: Tomasz Figa tomasz.f...@gmail.com
---
 drivers/pinctrl/pinctrl-samsung.c | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/drivers/pinctrl/pinctrl-samsung.c 
b/drivers/pinctrl/pinctrl-samsung.c
index 45a9939..b738b7b 100644
--- a/drivers/pinctrl/pinctrl-samsung.c
+++ b/drivers/pinctrl/pinctrl-samsung.c
@@ -303,6 +303,11 @@ static void samsung_pinmux_setup(struct pinctrl_dev 
*pctldev, unsigned selector,
type = bank-type;
mask = (1  type-fld_width[PINCFG_TYPE_FUNC]) - 1;
shift = pin_offset * type-fld_width[PINCFG_TYPE_FUNC];
+   if (shift = 32) {
+   /* Some banks have two config registers */
+   shift -= 32;
+   reg += 4;
+   }
 
spin_lock_irqsave(bank-slock, flags);
 
@@ -356,6 +361,11 @@ static int samsung_pinmux_gpio_set_direction(struct 
pinctrl_dev *pctldev,
 
mask = (1  type-fld_width[PINCFG_TYPE_FUNC]) - 1;
shift = pin_offset * type-fld_width[PINCFG_TYPE_FUNC];
+   if (shift = 32) {
+   /* Some banks have two config registers */
+   shift -= 32;
+   reg += 4;
+   }
 
spin_lock_irqsave(bank-slock, flags);
 
-- 
1.8.1.5

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


[PATCH 6/6] pinctrl: Add pinctrl-s3c64xx driver

2013-03-18 Thread Tomasz Figa
This patch adds pinctrl-s3c64xx driver which implements pin control
interface for Samsung S3C64xx SoCs.

Signed-off-by: Tomasz Figa tomasz.f...@gmail.com
---
 .../bindings/pinctrl/samsung-pinctrl.txt   |   3 +
 drivers/pinctrl/Kconfig|   5 +
 drivers/pinctrl/Makefile   |   1 +
 drivers/pinctrl/pinctrl-s3c64xx.c  | 817 +
 drivers/pinctrl/pinctrl-samsung.c  |   4 +
 drivers/pinctrl/pinctrl-samsung.h  |   5 +
 6 files changed, 835 insertions(+)
 create mode 100644 drivers/pinctrl/pinctrl-s3c64xx.c

diff --git a/Documentation/devicetree/bindings/pinctrl/samsung-pinctrl.txt 
b/Documentation/devicetree/bindings/pinctrl/samsung-pinctrl.txt
index 4598a47..c70fca1 100644
--- a/Documentation/devicetree/bindings/pinctrl/samsung-pinctrl.txt
+++ b/Documentation/devicetree/bindings/pinctrl/samsung-pinctrl.txt
@@ -7,6 +7,7 @@ on-chip controllers onto these pads.
 
 Required Properties:
 - compatible: should be one of the following.
+  - samsung,s3c64xx-pinctrl: for S3C64xx-compatible pin-controller,
   - samsung,exynos4210-pinctrl: for Exynos4210 compatible pin-controller.
   - samsung,exynos4x12-pinctrl: for Exynos4x12 compatible pin-controller.
   - samsung,exynos5250-pinctrl: for Exynos5250 compatible pin-controller.
@@ -105,6 +106,8 @@ B. External Wakeup Interrupts: For supporting external 
wakeup interrupts, a
 
- compatible: identifies the type of the external wakeup interrupt 
controller
  The possible values are:
+ - samsung,s3c64xx-wakeup-eint: represents wakeup interrupt controller
+   found on Samsung S3C64xx SoCs,
  - samsung,exynos4210-wakeup-eint: represents wakeup interrupt controller
found on Samsung Exynos4210 SoC.
- interrupt-parent: phandle of the interrupt parent to which the external
diff --git a/drivers/pinctrl/Kconfig b/drivers/pinctrl/Kconfig
index 5a690ce..b1cc5dc 100644
--- a/drivers/pinctrl/Kconfig
+++ b/drivers/pinctrl/Kconfig
@@ -227,6 +227,11 @@ config PINCTRL_EXYNOS5440
select PINMUX
select PINCONF
 
+config PINCTRL_S3C64XX
+   bool Samsung S3C64XX SoC pinctrl driver
+   depends on ARCH_S3C64XX
+   select PINCTRL_SAMSUNG
+
 source drivers/pinctrl/mvebu/Kconfig
 source drivers/pinctrl/sh-pfc/Kconfig
 source drivers/pinctrl/spear/Kconfig
diff --git a/drivers/pinctrl/Makefile b/drivers/pinctrl/Makefile
index f82cc5b..21d34c2 100644
--- a/drivers/pinctrl/Makefile
+++ b/drivers/pinctrl/Makefile
@@ -45,6 +45,7 @@ obj-$(CONFIG_PINCTRL_COH901)  += pinctrl-coh901.o
 obj-$(CONFIG_PINCTRL_SAMSUNG)  += pinctrl-samsung.o
 obj-$(CONFIG_PINCTRL_EXYNOS)   += pinctrl-exynos.o
 obj-$(CONFIG_PINCTRL_EXYNOS5440)   += pinctrl-exynos5440.o
+obj-$(CONFIG_PINCTRL_S3C64XX)  += pinctrl-s3c64xx.o
 obj-$(CONFIG_PINCTRL_XWAY) += pinctrl-xway.o
 obj-$(CONFIG_PINCTRL_LANTIQ)   += pinctrl-lantiq.o
 
diff --git a/drivers/pinctrl/pinctrl-s3c64xx.c 
b/drivers/pinctrl/pinctrl-s3c64xx.c
new file mode 100644
index 000..b5d1c4a
--- /dev/null
+++ b/drivers/pinctrl/pinctrl-s3c64xx.c
@@ -0,0 +1,817 @@
+/*
+ * S3C64xx specific support for pinctrl-samsung driver.
+ *
+ * Copyright (c) 2013 Tomasz Figa tomasz.f...@gmail.com
+ *
+ * Based on pinctrl-exynos.c, please see the file for original copyrights.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This file contains the Samsung S3C64xx specific information required by the
+ * the Samsung pinctrl/gpiolib driver. It also includes the implementation of
+ * external gpio and wakeup interrupt support.
+ */
+
+#include linux/module.h
+#include linux/device.h
+#include linux/interrupt.h
+#include linux/irqdomain.h
+#include linux/irq.h
+#include linux/of_irq.h
+#include linux/io.h
+#include linux/slab.h
+#include linux/err.h
+
+#include asm/mach/irq.h
+
+#include pinctrl-samsung.h
+
+#define NUM_EINT0  28
+#define NUM_EINT0_IRQ  4
+#define EINT_MAX_PER_REG   16
+#define EINT_MAX_PER_GROUP 16
+
+/* External GPIO and wakeup interrupt related definitions */
+#define SVC_GROUP_SHIFT4
+#define SVC_GROUP_MASK 0xf
+#define SVC_NUM_MASK   0xf
+#define SVC_GROUP(x)   ((x  SVC_GROUP_SHIFT)  \
+   SVC_GROUP_MASK)
+
+#define EINT12CON_REG  0x200
+#define EINT12MASK_REG 0x240
+#define EINT12PEND_REG 0x260
+
+#define EINT_OFFS(i)   ((i) % (2 * EINT_MAX_PER_GROUP))
+#define EINT_GROUP(i)  ((i) / EINT_MAX_PER_GROUP)
+#define EINT_REG(g)(4 * ((g) / 2))
+
+#define EINTCON_REG(i) (EINT12CON_REG + EINT_REG(EINT_GROUP(i)))
+#define EINTMASK_REG(i)(EINT12MASK_REG + 
EINT_REG(EINT_GROUP(i)))
+#define 

[PATCH 4/6] pinctrl: samsung: Remove hardcoded register offsets

2013-03-18 Thread Tomasz Figa
This patch replaces statically hardcoded register offsets of Exynos SoCs
with an array of register offsets in samsung_pin_bank_type struct.

Thanks to this change, support for SoCs with other set and order of
registers can be added (e.g. S3C24xx and S3C64xx).

Signed-off-by: Tomasz Figa tomasz.f...@gmail.com
---
 drivers/pinctrl/pinctrl-exynos.c  |  6 --
 drivers/pinctrl/pinctrl-samsung.c | 37 +
 drivers/pinctrl/pinctrl-samsung.h |  9 ++---
 3 files changed, 15 insertions(+), 37 deletions(-)

diff --git a/drivers/pinctrl/pinctrl-exynos.c b/drivers/pinctrl/pinctrl-exynos.c
index b5dbb87..8b10b1a 100644
--- a/drivers/pinctrl/pinctrl-exynos.c
+++ b/drivers/pinctrl/pinctrl-exynos.c
@@ -37,10 +37,12 @@
 
 static struct samsung_pin_bank_type bank_type_off = {
.fld_width = { 4, 1, 2, 2, 2, 2, },
+   .reg_offset = { 0x00, 0x04, 0x08, 0x0c, 0x10, 0x14, },
 };
 
 static struct samsung_pin_bank_type bank_type_alive = {
.fld_width = { 4, 1, 2, 2, },
+   .reg_offset = { 0x00, 0x04, 0x08, 0x0c, },
 };
 
 /* list of external wakeup controllers supported */
@@ -126,7 +128,7 @@ static int exynos_gpio_irq_set_type(struct irq_data *irqd, 
unsigned int type)
con |= trig_type  shift;
writel(con, d-virt_base + reg_con);
 
-   reg_con = bank-pctl_offset;
+   reg_con = bank-pctl_offset + bank_type-reg_offset[PINCFG_TYPE_FUNC];
shift = pin * bank_type-fld_width[PINCFG_TYPE_FUNC];
mask = (1  bank_type-fld_width[PINCFG_TYPE_FUNC]) - 1;
 
@@ -309,7 +311,7 @@ static int exynos_wkup_irq_set_type(struct irq_data *irqd, 
unsigned int type)
con |= trig_type  shift;
writel(con, d-virt_base + reg_con);
 
-   reg_con = bank-pctl_offset;
+   reg_con = bank-pctl_offset + bank_type-reg_offset[PINCFG_TYPE_FUNC];
shift = pin * bank_type-fld_width[PINCFG_TYPE_FUNC];
mask = (1  bank_type-fld_width[PINCFG_TYPE_FUNC]) - 1;
 
diff --git a/drivers/pinctrl/pinctrl-samsung.c 
b/drivers/pinctrl/pinctrl-samsung.c
index ac428ff..45a9939 100644
--- a/drivers/pinctrl/pinctrl-samsung.c
+++ b/drivers/pinctrl/pinctrl-samsung.c
@@ -275,10 +275,6 @@ static void pin_to_reg_bank(struct 
samsung_pinctrl_drv_data *drvdata,
*offset = pin - b-pin_base;
if (bank)
*bank = b;
-
-   /* some banks have two config registers in a single bank */
-   if (*offset * b-func_width  BITS_PER_LONG)
-   *reg += 4;
 }
 
 /* enable or disable a pinmux function */
@@ -310,11 +306,11 @@ static void samsung_pinmux_setup(struct pinctrl_dev 
*pctldev, unsigned selector,
 
spin_lock_irqsave(bank-slock, flags);
 
-   data = readl(reg);
+   data = readl(reg + type-reg_offset[PINCFG_TYPE_FUNC]);
data = ~(mask  shift);
if (enable)
data |= drvdata-pin_groups[group].func  shift;
-   writel(data, reg);
+   writel(data, reg + type-reg_offset[PINCFG_TYPE_FUNC]);
 
spin_unlock_irqrestore(bank-slock, flags);
}
@@ -355,7 +351,8 @@ static int samsung_pinmux_gpio_set_direction(struct 
pinctrl_dev *pctldev,
drvdata = pinctrl_dev_get_drvdata(pctldev);
 
pin_offset = offset - bank-pin_base;
-   reg = drvdata-virt_base + bank-pctl_offset;
+   reg = drvdata-virt_base + bank-pctl_offset +
+   type-reg_offset[PINCFG_TYPE_FUNC];
 
mask = (1  type-fld_width[PINCFG_TYPE_FUNC]) - 1;
shift = pin_offset * type-fld_width[PINCFG_TYPE_FUNC];
@@ -401,28 +398,11 @@ static int samsung_pinconf_rw(struct pinctrl_dev 
*pctldev, unsigned int pin,
pin_offset, bank);
type = bank-type;
 
-   switch (cfg_type) {
-   case PINCFG_TYPE_PUD:
-   cfg_reg = PUD_REG;
-   break;
-   case PINCFG_TYPE_DRV:
-   cfg_reg = DRV_REG;
-   break;
-   case PINCFG_TYPE_CON_PDN:
-   cfg_reg = CONPDN_REG;
-   break;
-   case PINCFG_TYPE_PUD_PDN:
-   cfg_reg = PUDPDN_REG;
-   break;
-   default:
-   WARN_ON(1);
-   return -EINVAL;
-   }
-
if (cfg_type = PINCFG_TYPE_NUM || !type-fld_width[cfg_type])
return -EINVAL;
 
width = type-fld_width[cfg_type];
+   cfg_reg = type-reg_offset[cfg_type];
 
spin_lock_irqsave(bank-slock, flags);
 
@@ -511,11 +491,11 @@ static void samsung_gpio_set(struct gpio_chip *gc, 
unsigned offset, int value)
 
spin_lock_irqsave(bank-slock, flags);
 
-   data = readl(reg + DAT_REG);
+   data = readl(reg + type-reg_offset[PINCFG_TYPE_DAT]);
data = ~(1  offset);
if (value)
data |= 1  offset;
-   writel(data, reg + DAT_REG);
+   writel(data, reg + type-reg_offset[PINCFG_TYPE_DAT]);
 
spin_unlock_irqrestore(bank-slock, flags);

[PATCH 3/6] pinctrl: samsung: Split pin bank description into two structures

2013-03-18 Thread Tomasz Figa
This patch splits pin bank description into two structures, one
describing bank type (currently only bitfield widths), which can be
shared across multiple banks and second containing bank-specific
parameters including a pointer to a bank type struct.

It is a prerequisite for further patch removing the statically hardcoded
register offsets, making it impossible to support SoCs with different
set and order of pin control registers.

Signed-off-by: Tomasz Figa tomasz.f...@gmail.com
---
 drivers/pinctrl/pinctrl-exynos.c  | 19 +++
 drivers/pinctrl/pinctrl-exynos.h  | 16 +++-
 drivers/pinctrl/pinctrl-samsung.c | 24 +++-
 drivers/pinctrl/pinctrl-samsung.h | 26 --
 4 files changed, 49 insertions(+), 36 deletions(-)

diff --git a/drivers/pinctrl/pinctrl-exynos.c b/drivers/pinctrl/pinctrl-exynos.c
index cf7700e..b5dbb87 100644
--- a/drivers/pinctrl/pinctrl-exynos.c
+++ b/drivers/pinctrl/pinctrl-exynos.c
@@ -34,6 +34,15 @@
 #include pinctrl-samsung.h
 #include pinctrl-exynos.h
 
+
+static struct samsung_pin_bank_type bank_type_off = {
+   .fld_width = { 4, 1, 2, 2, 2, 2, },
+};
+
+static struct samsung_pin_bank_type bank_type_alive = {
+   .fld_width = { 4, 1, 2, 2, },
+};
+
 /* list of external wakeup controllers supported */
 static const struct of_device_id exynos_wkup_irq_ids[] = {
{ .compatible = samsung,exynos4210-wakeup-eint, },
@@ -76,6 +85,7 @@ static void exynos_gpio_irq_ack(struct irq_data *irqd)
 static int exynos_gpio_irq_set_type(struct irq_data *irqd, unsigned int type)
 {
struct samsung_pin_bank *bank = irq_data_get_irq_chip_data(irqd);
+   struct samsung_pin_bank_type *bank_type = bank-type;
struct samsung_pinctrl_drv_data *d = bank-drvdata;
struct samsung_pin_ctrl *ctrl = d-ctrl;
unsigned int pin = irqd-hwirq;
@@ -117,8 +127,8 @@ static int exynos_gpio_irq_set_type(struct irq_data *irqd, 
unsigned int type)
writel(con, d-virt_base + reg_con);
 
reg_con = bank-pctl_offset;
-   shift = pin * bank-func_width;
-   mask = (1  bank-func_width) - 1;
+   shift = pin * bank_type-fld_width[PINCFG_TYPE_FUNC];
+   mask = (1  bank_type-fld_width[PINCFG_TYPE_FUNC]) - 1;
 
spin_lock_irqsave(bank-slock, flags);
 
@@ -259,6 +269,7 @@ static void exynos_wkup_irq_ack(struct irq_data *irqd)
 static int exynos_wkup_irq_set_type(struct irq_data *irqd, unsigned int type)
 {
struct samsung_pin_bank *bank = irq_data_get_irq_chip_data(irqd);
+   struct samsung_pin_bank_type *bank_type = bank-type;
struct samsung_pinctrl_drv_data *d = bank-drvdata;
unsigned int pin = irqd-hwirq;
unsigned long reg_con = d-ctrl-weint_con + bank-eint_offset;
@@ -299,8 +310,8 @@ static int exynos_wkup_irq_set_type(struct irq_data *irqd, 
unsigned int type)
writel(con, d-virt_base + reg_con);
 
reg_con = bank-pctl_offset;
-   shift = pin * bank-func_width;
-   mask = (1  bank-func_width) - 1;
+   shift = pin * bank_type-fld_width[PINCFG_TYPE_FUNC];
+   mask = (1  bank_type-fld_width[PINCFG_TYPE_FUNC]) - 1;
 
spin_lock_irqsave(bank-slock, flags);
 
diff --git a/drivers/pinctrl/pinctrl-exynos.h b/drivers/pinctrl/pinctrl-exynos.h
index 0a70889..9b1f77a 100644
--- a/drivers/pinctrl/pinctrl-exynos.h
+++ b/drivers/pinctrl/pinctrl-exynos.h
@@ -48,26 +48,18 @@
 
 #define EXYNOS_PIN_BANK_EINTN(pins, reg, id)   \
{   \
+   .type   = bank_type_off,   \
.pctl_offset= reg,  \
.nr_pins= pins, \
-   .func_width = 4,\
-   .pud_width  = 2,\
-   .drv_width  = 2,\
-   .conpdn_width   = 2,\
-   .pudpdn_width   = 2,\
.eint_type  = EINT_TYPE_NONE,   \
.name   = id\
}
 
 #define EXYNOS_PIN_BANK_EINTG(pins, reg, id, offs) \
{   \
+   .type   = bank_type_off,   \
.pctl_offset= reg,  \
.nr_pins= pins, \
-   .func_width = 4,\
-   .pud_width  = 2,\
-   .drv_width  = 2,\
-   .conpdn_width   = 2,\
-   .pudpdn_width   = 2,\
.eint_type  = EINT_TYPE_GPIO,   \
.eint_offset= offs, \
.name   = id\
@@ -75,11 +67,9 @@
 
 #define EXYNOS_PIN_BANK_EINTW(pins, reg, id, offs) \
{   

[PATCH 2/6] pinctrl: samsung: Include pinctrl-exynos driver data conditionally

2013-03-18 Thread Tomasz Figa
Since pinctrl-samsung is a common part of the pin control support for
several Samsung SoCs, it can be compiled without Exynos support enabled.

This patch surrounds Exynos-specific driver data with ifdefs to include
them only when support for Exynos is enabled.

Signed-off-by: Tomasz Figa tomasz.f...@gmail.com
---
 drivers/pinctrl/pinctrl-samsung.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/pinctrl/pinctrl-samsung.c 
b/drivers/pinctrl/pinctrl-samsung.c
index b1d4ac8..f1fb562 100644
--- a/drivers/pinctrl/pinctrl-samsung.c
+++ b/drivers/pinctrl/pinctrl-samsung.c
@@ -968,10 +968,12 @@ static int samsung_pinctrl_probe(struct platform_device 
*pdev)
 }
 
 static const struct of_device_id samsung_pinctrl_dt_match[] = {
+#ifdef CONFIG_PINCTRL_EXYNOS4
{ .compatible = samsung,exynos4210-pinctrl,
.data = (void *)exynos4210_pin_ctrl },
{ .compatible = samsung,exynos4x12-pinctrl,
.data = (void *)exynos4x12_pin_ctrl },
+#endif
{},
 };
 MODULE_DEVICE_TABLE(of, samsung_pinctrl_dt_match);
-- 
1.8.1.5

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


Re: [PATCH 2/6] pinctrl: samsung: Include pinctrl-exynos driver data conditionally

2013-03-18 Thread Tomasz Figa
On Monday 18 of March 2013 22:31:51 Tomasz Figa wrote:
 Since pinctrl-samsung is a common part of the pin control support for
 several Samsung SoCs, it can be compiled without Exynos support enabled.
 
 This patch surrounds Exynos-specific driver data with ifdefs to include
 them only when support for Exynos is enabled.
 
 Signed-off-by: Tomasz Figa tomasz.f...@gmail.com
 ---
  drivers/pinctrl/pinctrl-samsung.c | 2 ++
  1 file changed, 2 insertions(+)
 
 diff --git a/drivers/pinctrl/pinctrl-samsung.c
 b/drivers/pinctrl/pinctrl-samsung.c index b1d4ac8..f1fb562 100644
 --- a/drivers/pinctrl/pinctrl-samsung.c
 +++ b/drivers/pinctrl/pinctrl-samsung.c
 @@ -968,10 +968,12 @@ static int samsung_pinctrl_probe(struct
 platform_device *pdev) }
 
  static const struct of_device_id samsung_pinctrl_dt_match[] = {
 +#ifdef CONFIG_PINCTRL_EXYNOS4

Oops... This should be CONFIG_PINCTRL_EXYNOS. It used to be EXYNOS4 in 
Kgene's tree (on which I usually base my patches) for some time, but it no 
longer is, but I forgot to change this during rebae.

Linus, if remaining patches turned out to be OK, would you fix this when 
applying?

Best regards,
Tomasz

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


[PATCH 1/6] pinctrl: samsung: Protect bank registers with a spinlock

2013-03-18 Thread Tomasz Figa
Certain pin control registers can be accessed from different contexts,
i.e. pinctrl, gpio and irq functions. This makes the locking provided by
pin control core insufficient.

This patch adds necessary locking using a per bank spinlock as it was
done in the old Samsung GPIO driver.

Signed-off-by: Tomasz Figa tomasz.f...@gmail.com
---
 drivers/pinctrl/pinctrl-exynos.c  | 11 +++
 drivers/pinctrl/pinctrl-samsung.c | 24 
 drivers/pinctrl/pinctrl-samsung.h |  2 ++
 3 files changed, 37 insertions(+)

diff --git a/drivers/pinctrl/pinctrl-exynos.c b/drivers/pinctrl/pinctrl-exynos.c
index 538b9dd..cf7700e 100644
--- a/drivers/pinctrl/pinctrl-exynos.c
+++ b/drivers/pinctrl/pinctrl-exynos.c
@@ -26,6 +26,7 @@
 #include linux/of_irq.h
 #include linux/io.h
 #include linux/slab.h
+#include linux/spinlock.h
 #include linux/err.h
 
 #include asm/mach/irq.h
@@ -81,6 +82,7 @@ static int exynos_gpio_irq_set_type(struct irq_data *irqd, 
unsigned int type)
unsigned int shift = EXYNOS_EINT_CON_LEN * pin;
unsigned int con, trig_type;
unsigned long reg_con = ctrl-geint_con + bank-eint_offset;
+   unsigned long flags;
unsigned int mask;
 
switch (type) {
@@ -118,11 +120,15 @@ static int exynos_gpio_irq_set_type(struct irq_data 
*irqd, unsigned int type)
shift = pin * bank-func_width;
mask = (1  bank-func_width) - 1;
 
+   spin_lock_irqsave(bank-slock, flags);
+
con = readl(d-virt_base + reg_con);
con = ~(mask  shift);
con |= EXYNOS_EINT_FUNC  shift;
writel(con, d-virt_base + reg_con);
 
+   spin_unlock_irqrestore(bank-slock, flags);
+
return 0;
 }
 
@@ -258,6 +264,7 @@ static int exynos_wkup_irq_set_type(struct irq_data *irqd, 
unsigned int type)
unsigned long reg_con = d-ctrl-weint_con + bank-eint_offset;
unsigned long shift = EXYNOS_EINT_CON_LEN * pin;
unsigned long con, trig_type;
+   unsigned long flags;
unsigned int mask;
 
switch (type) {
@@ -295,11 +302,15 @@ static int exynos_wkup_irq_set_type(struct irq_data 
*irqd, unsigned int type)
shift = pin * bank-func_width;
mask = (1  bank-func_width) - 1;
 
+   spin_lock_irqsave(bank-slock, flags);
+
con = readl(d-virt_base + reg_con);
con = ~(mask  shift);
con |= EXYNOS_EINT_FUNC  shift;
writel(con, d-virt_base + reg_con);
 
+   spin_unlock_irqrestore(bank-slock, flags);
+
return 0;
 }
 
diff --git a/drivers/pinctrl/pinctrl-samsung.c 
b/drivers/pinctrl/pinctrl-samsung.c
index 3475b92..b1d4ac8 100644
--- a/drivers/pinctrl/pinctrl-samsung.c
+++ b/drivers/pinctrl/pinctrl-samsung.c
@@ -27,6 +27,7 @@
 #include linux/err.h
 #include linux/gpio.h
 #include linux/irqdomain.h
+#include linux/spinlock.h
 
 #include core.h
 #include pinctrl-samsung.h
@@ -289,6 +290,7 @@ static void samsung_pinmux_setup(struct pinctrl_dev 
*pctldev, unsigned selector,
struct samsung_pin_bank *bank;
void __iomem *reg;
u32 mask, shift, data, pin_offset, cnt;
+   unsigned long flags;
 
drvdata = pinctrl_dev_get_drvdata(pctldev);
pins = drvdata-pin_groups[group].pins;
@@ -303,11 +305,15 @@ static void samsung_pinmux_setup(struct pinctrl_dev 
*pctldev, unsigned selector,
mask = (1  bank-func_width) - 1;
shift = pin_offset * bank-func_width;
 
+   spin_lock_irqsave(bank-slock, flags);
+
data = readl(reg);
data = ~(mask  shift);
if (enable)
data |= drvdata-pin_groups[group].func  shift;
writel(data, reg);
+
+   spin_unlock_irqrestore(bank-slock, flags);
}
 }
 
@@ -338,6 +344,7 @@ static int samsung_pinmux_gpio_set_direction(struct 
pinctrl_dev *pctldev,
struct samsung_pinctrl_drv_data *drvdata;
void __iomem *reg;
u32 data, pin_offset, mask, shift;
+   unsigned long flags;
 
bank = gc_to_pin_bank(range-gc);
drvdata = pinctrl_dev_get_drvdata(pctldev);
@@ -348,11 +355,16 @@ static int samsung_pinmux_gpio_set_direction(struct 
pinctrl_dev *pctldev,
mask = (1  bank-func_width) - 1;
shift = pin_offset * bank-func_width;
 
+   spin_lock_irqsave(bank-slock, flags);
+
data = readl(reg);
data = ~(mask  shift);
if (!input)
data |= FUNC_OUTPUT  shift;
writel(data, reg);
+
+   spin_unlock_irqrestore(bank-slock, flags);
+
return 0;
 }
 
@@ -376,6 +388,7 @@ static int samsung_pinconf_rw(struct pinctrl_dev *pctldev, 
unsigned int pin,
enum pincfg_type cfg_type = PINCFG_UNPACK_TYPE(*config);
u32 data, width, pin_offset, mask, shift;
u32 cfg_value, cfg_reg;
+   unsigned long flags;
 
drvdata = pinctrl_dev_get_drvdata(pctldev);
pin_to_reg_bank(drvdata, pin - drvdata-ctrl-base, reg_base,
@@ -406,6 +419,8 @@ static int 

Re: [PATCH v3 6/6] irqchip: s3c24xx: add s3c2450 interrupt definitions

2013-03-18 Thread Rob Herring
On 03/18/2013 11:53 AM, Heiko Stübner wrote:
 Hi Rob,
 
 Am Montag, 18. März 2013, 16:54:03 schrieb Rob Herring:
 On 03/17/2013 08:09 AM, Heiko Stübner wrote:
 The s3c2450 is special in that it shares the cpu identification with the
 s3c2416 but provides more interrupts for its additional components.

 It also shares the layout of the main interrupt register with the s3c2443
 and therefore reuses this definition.

 As no non-dt boards are present, the s3c2450 irqs will only be
 accessible thru devicetree.

[snip]

 +   { .type = S3C_IRQTYPE_LEVEL, .parent_irq = 17 }, /* DMA4 */
 +   { .type = S3C_IRQTYPE_LEVEL, .parent_irq = 17 }, /* DMA5 */
 +   { .type = S3C_IRQTYPE_LEVEL, .parent_irq = 18 }, /* UART3-RX */
 +   { .type = S3C_IRQTYPE_LEVEL, .parent_irq = 18 }, /* UART3-TX */
 +   { .type = S3C_IRQTYPE_LEVEL, .parent_irq = 18 }, /* UART3-ERR */
 +   { .type = S3C_IRQTYPE_LEVEL, .parent_irq = 9 }, /* WDT */
 +   { .type = S3C_IRQTYPE_LEVEL, .parent_irq = 9 }, /* AC97 */
 +   { .type = S3C_IRQTYPE_LEVEL, .parent_irq = 17 }, /* DMA6 */
 +   { .type = S3C_IRQTYPE_LEVEL, .parent_irq = 17 }, /* DMA7 */

 This all seems like information that should come from DT.
 
 In the first iterations [0] of theis series it was done this way, but was 
 suggested that these informations _might_ be implementation specific and not 
 describing the hardware.
 
 As I didn't get any final feedback on the matter, I tried it this way this 
 time. Personally I also did like the previous variant better, as the driver 
 could loose all the declaration stuff when platforms move to dt.
 
 I would be glad for a hint if the first approach was the correct one.
 
 
 [0] irqchip: irq-s3c24xx: add devicetree support from 2013-02-18, also with 
 you in the recipient list

I'm inclined to say the prior way is more in the right direction.
However, I'm not really clear what you are trying to describe.

 + s3c24xx,irqlist = 2 0 /* 2D */
 +2 0 /* IIC1 */
 +0 0 /* reserved */
 +0 0 /* reserved */
 +2 0 /* PCM0 */
 +2 0 /* PCM1 */
 +2 0 /* I2S0 */
 +2 0; /* I2S1 */

My first thought here is this information should not be centralized in
the controller node, but placed with each source node (2D, I2C1, etc).

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


Re: [PATCH v3 6/6] irqchip: s3c24xx: add s3c2450 interrupt definitions

2013-03-18 Thread Arnd Bergmann
On Monday 18 March 2013 17:14:52 Rob Herring wrote:
 
  + s3c24xx,irqlist = 2 0 /* 2D */
  +2 0 /* IIC1 */
  +0 0 /* reserved */
  +0 0 /* reserved */
  +2 0 /* PCM0 */
  +2 0 /* PCM1 */
  +2 0 /* I2S0 */
  +2 0; /* I2S1 */
 
 My first thought here is this information should not be centralized in
 the controller node, but placed with each source node (2D, I2C1, etc).

Seconded. Let's do this the same way we have it for all other irq chips
and put it all into the irq descriptor referring to the controller, not
the controller.

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


Re: [PATCH] ARM: dts: add interrupt-names property to get interrupt resource by name

2013-03-18 Thread Rob Herring
On 03/18/2013 01:11 PM, Stephen Warren wrote:
 On 03/18/2013 09:50 AM, Rob Herring wrote:
 On 03/13/2013 05:42 PM, Sylwester Nawrocki wrote:
 Rob,

 On 03/13/2013 03:39 PM, Rob Herring wrote:
 I fail to see what the hack is. The order of interrupt properties must
 be defined by the binding. interrupt-names is auxiliary data and must
 not be required by an OS.
 
 Is that true for all foo-names properties, or only for interrupt-names?
 I was under the impression that foo-names was specifically invented so
 that the order of the entries didn't matter, and instead they could be
 requested by name.

I think it depends on the specific name the property is tied too. For
interrupt and reg properties which have a long history and convention,
the order should be defined. IIRC, this was Grant's position too. For
new bindings, perhaps we can be more lenient.

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


Re: [PATCH] ARM: dts: add interrupt-names property to get interrupt resource by name

2013-03-18 Thread Arnd Bergmann
On Monday 18 March 2013 17:27:35 Rob Herring wrote:
 
 I think it depends on the specific name the property is tied too. For
 interrupt and reg properties which have a long history and convention,
 the order should be defined. IIRC, this was Grant's position too. For
 new bindings, perhaps we can be more lenient.

At least in case of the dmaengine binding, order is not significant
and cannot be, because you may have multiple DMA specifiers referring
to the a output of the slave device connected to multiple dma engines,
and we require that they all use the same name in that case.

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


Re: [PATCH] ARM: dts: add interrupt-names property to get interrupt resource by name

2013-03-18 Thread Stephen Warren
On 03/18/2013 04:27 PM, Rob Herring wrote:
 On 03/18/2013 01:11 PM, Stephen Warren wrote:
 On 03/18/2013 09:50 AM, Rob Herring wrote:
 On 03/13/2013 05:42 PM, Sylwester Nawrocki wrote:
 Rob,

 On 03/13/2013 03:39 PM, Rob Herring wrote:
 I fail to see what the hack is. The order of interrupt properties must
 be defined by the binding. interrupt-names is auxiliary data and must
 not be required by an OS.

 Is that true for all foo-names properties, or only for interrupt-names?
 I was under the impression that foo-names was specifically invented so
 that the order of the entries didn't matter, and instead they could be
 requested by name.
 
 I think it depends on the specific name the property is tied too. For
 interrupt and reg properties which have a long history and convention,
 the order should be defined. IIRC, this was Grant's position too. For
 new bindings, perhaps we can be more lenient.

OK, that makes sense for interrupts/reg. Can we decide that clock-namess
are new-style and that order is not significant? I guess gpio-names too?

I guess this should be documented in whatever binding describes the core
interrupts/reg-names/gpio-names/clock-names/dma-names properties.
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v3 6/6] irqchip: s3c24xx: add s3c2450 interrupt definitions

2013-03-18 Thread Heiko Stübner
Am Montag, 18. März 2013, 23:14:52 schrieb Rob Herring:
 On 03/18/2013 11:53 AM, Heiko Stübner wrote:
  Hi Rob,
  
  Am Montag, 18. März 2013, 16:54:03 schrieb Rob Herring:
  On 03/17/2013 08:09 AM, Heiko Stübner wrote:
  The s3c2450 is special in that it shares the cpu identification with
  the s3c2416 but provides more interrupts for its additional
  components.
  
  It also shares the layout of the main interrupt register with the
  s3c2443 and therefore reuses this definition.
  
  As no non-dt boards are present, the s3c2450 irqs will only be
  accessible thru devicetree.
 
 [snip]
 
  + { .type = S3C_IRQTYPE_LEVEL, .parent_irq = 17 }, /* DMA4 */
  + { .type = S3C_IRQTYPE_LEVEL, .parent_irq = 17 }, /* DMA5 */
  + { .type = S3C_IRQTYPE_LEVEL, .parent_irq = 18 }, /* UART3-RX */
  + { .type = S3C_IRQTYPE_LEVEL, .parent_irq = 18 }, /* UART3-TX */
  + { .type = S3C_IRQTYPE_LEVEL, .parent_irq = 18 }, /* UART3-ERR */
  + { .type = S3C_IRQTYPE_LEVEL, .parent_irq = 9 }, /* WDT */
  + { .type = S3C_IRQTYPE_LEVEL, .parent_irq = 9 }, /* AC97 */
  + { .type = S3C_IRQTYPE_LEVEL, .parent_irq = 17 }, /* DMA6 */
  + { .type = S3C_IRQTYPE_LEVEL, .parent_irq = 17 }, /* DMA7 */
  
  This all seems like information that should come from DT.
  
  In the first iterations [0] of theis series it was done this way, but was
  suggested that these informations _might_ be implementation specific and
  not describing the hardware.
  
  As I didn't get any final feedback on the matter, I tried it this way
  this time. Personally I also did like the previous variant better, as
  the driver could loose all the declaration stuff when platforms move to
  dt.
  
  I would be glad for a hint if the first approach was the correct one.
  
  
  [0] irqchip: irq-s3c24xx: add devicetree support from 2013-02-18, also
  with you in the recipient list
 
 I'm inclined to say the prior way is more in the right direction.
 However, I'm not really clear what you are trying to describe.
 
  +   s3c24xx,irqlist = 2 0 /* 2D */
  +  2 0 /* IIC1 */
  +  0 0 /* reserved */
  +  0 0 /* reserved */
  +  2 0 /* PCM0 */
  +  2 0 /* PCM1 */
  +  2 0 /* I2S0 */
  +  2 0; /* I2S1 */

The s3c24xx SoCs contain one (or two) main interrupt controllers. That are the 
nodes that gets checked by handle_irq. Additionally they contain something 
called a sub-register which provides more specific irq for some of them.

The list you quoted, is meant to describe which bits of the interrupt register 
contain a valid interrupt at all (!= 0), which handler should be used (2 for 
the edge handler, 3 for level handler). The second argument contains the bit 
in the parent interrupt register that contains the parent interrupt, that gets 
checked in the irq_entry function.

The original code (which I reworked into this more declarative form) 
distinguished very much between using the edge handler for some and the level 
handler for other interrupts.

This list is essentially the same representation of the list you quoted above 
and which also can be found in [0]

Going this way makes it easy to map datasheet values to interrupt number. When 
I read the ac97 interrupt is in bit 28 of the sub-register I can simply 
reference it as
interrupt-parent = subintc;
interrupts = 28;


So these lists only desribe the internal structure of the interrupt controller 
registers, which vary for each s3c24xx variant.


 My first thought here is this information should not be centralized in
 the controller node, but placed with each source node (2D, I2C1, etc).

I'm not sure I can follow currently :-)

I'll try an example:

The s3c serial driver expects the interrupts for uart tx and rx and the dt 
entry would look like:

serial@5000 {
compatible = samsung,s3c2410-uart;
reg = 0x5000 0x4000;
interrupt-parent = subintc;
interrupts = 0, 1;
};

the map for these in the subintc looks like
   s3c24xx,irqlist = 3 28 /* UART0-RX */
  3 28 /* UART0-TX */
  3 28 /* UART0-ERR */

marking them as using the level-handler and being children of the interrupt in 
bit 28 of the intc handler.

So the irq_entry would check the intc, see the waiting interrupt in bit 28, 
jump to the demux function which would then handle which ever of rx,tx or err 
would be waiting, which then get sent to the serial driver.

These mappings between sub- and parent irqs also vary very much between the 
different s3c24xx variants, as can be seen by the multitude of lists in [0] 
and the parent interrupts are only used for demuxing purposes.

-
A notable speciality are the AC97 (sound) and watchdog interrupts (bits 27 and 
28 of the 

Re: [PATCH v3 6/6] irqchip: s3c24xx: add s3c2450 interrupt definitions

2013-03-18 Thread Rob Herring
On 03/18/2013 06:34 PM, Heiko Stübner wrote:
 Am Montag, 18. März 2013, 23:14:52 schrieb Rob Herring:
 On 03/18/2013 11:53 AM, Heiko Stübner wrote:
 Hi Rob,

 Am Montag, 18. März 2013, 16:54:03 schrieb Rob Herring:
 On 03/17/2013 08:09 AM, Heiko Stübner wrote:
 The s3c2450 is special in that it shares the cpu identification with
 the s3c2416 but provides more interrupts for its additional
 components.

 It also shares the layout of the main interrupt register with the
 s3c2443 and therefore reuses this definition.

 As no non-dt boards are present, the s3c2450 irqs will only be
 accessible thru devicetree.

 [snip]

 + { .type = S3C_IRQTYPE_LEVEL, .parent_irq = 17 }, /* DMA4 */
 + { .type = S3C_IRQTYPE_LEVEL, .parent_irq = 17 }, /* DMA5 */
 + { .type = S3C_IRQTYPE_LEVEL, .parent_irq = 18 }, /* UART3-RX */
 + { .type = S3C_IRQTYPE_LEVEL, .parent_irq = 18 }, /* UART3-TX */
 + { .type = S3C_IRQTYPE_LEVEL, .parent_irq = 18 }, /* UART3-ERR */
 + { .type = S3C_IRQTYPE_LEVEL, .parent_irq = 9 }, /* WDT */
 + { .type = S3C_IRQTYPE_LEVEL, .parent_irq = 9 }, /* AC97 */
 + { .type = S3C_IRQTYPE_LEVEL, .parent_irq = 17 }, /* DMA6 */
 + { .type = S3C_IRQTYPE_LEVEL, .parent_irq = 17 }, /* DMA7 */

 This all seems like information that should come from DT.

 In the first iterations [0] of theis series it was done this way, but was
 suggested that these informations _might_ be implementation specific and
 not describing the hardware.

 As I didn't get any final feedback on the matter, I tried it this way
 this time. Personally I also did like the previous variant better, as
 the driver could loose all the declaration stuff when platforms move to
 dt.

 I would be glad for a hint if the first approach was the correct one.


 [0] irqchip: irq-s3c24xx: add devicetree support from 2013-02-18, also
 with you in the recipient list

 I'm inclined to say the prior way is more in the right direction.
 However, I'm not really clear what you are trying to describe.

 +   s3c24xx,irqlist = 2 0 /* 2D */
 +  2 0 /* IIC1 */
 +  0 0 /* reserved */
 +  0 0 /* reserved */
 +  2 0 /* PCM0 */
 +  2 0 /* PCM1 */
 +  2 0 /* I2S0 */
 +  2 0; /* I2S1 */
 
 The s3c24xx SoCs contain one (or two) main interrupt controllers. That are 
 the 
 nodes that gets checked by handle_irq. Additionally they contain something 
 called a sub-register which provides more specific irq for some of them.
 
 The list you quoted, is meant to describe which bits of the interrupt 
 register 
 contain a valid interrupt at all (!= 0), which handler should be used (2 for 
 the edge handler, 3 for level handler). The second argument contains the bit 
 in the parent interrupt register that contains the parent interrupt, that 
 gets 
 checked in the irq_entry function.
 
 The original code (which I reworked into this more declarative form) 
 distinguished very much between using the edge handler for some and the level 
 handler for other interrupts.
 
 This list is essentially the same representation of the list you quoted above 
 and which also can be found in [0]
 
 Going this way makes it easy to map datasheet values to interrupt number. 
 When 
 I read the ac97 interrupt is in bit 28 of the sub-register I can simply 
 reference it as
   interrupt-parent = subintc;
   interrupts = 28;
 
 
 So these lists only desribe the internal structure of the interrupt 
 controller 
 registers, which vary for each s3c24xx variant.
 
 
 My first thought here is this information should not be centralized in
 the controller node, but placed with each source node (2D, I2C1, etc).
 
 I'm not sure I can follow currently :-)
 
 I'll try an example:
 
 The s3c serial driver expects the interrupts for uart tx and rx and the dt 
 entry would look like:
 
   serial@5000 {
   compatible = samsung,s3c2410-uart;
   reg = 0x5000 0x4000;
   interrupt-parent = subintc;
   interrupts = 0, 1;

So what does 0 and 1 correspond to? These are the bits in the subintc?

   };
 
 the map for these in the subintc looks like
s3c24xx,irqlist = 3 28 /* UART0-RX */
   3 28 /* UART0-TX */
   3 28 /* UART0-ERR */
 
 marking them as using the level-handler and being children of the interrupt 
 in 
 bit 28 of the intc handler.
 
 So the irq_entry would check the intc, see the waiting interrupt in bit 28, 
 jump to the demux function which would then handle which ever of rx,tx or err 
 would be waiting, which then get sent to the serial driver.
 
 These mappings between sub- and parent irqs also vary very much between the 
 different s3c24xx variants, as can be seen by the multitude of lists in [0] 
 and the parent interrupts are only used for demuxing