public inbox for linux-media@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH v8 1/7] media: V4L2: add temporary clock helpers
  2013-04-08 15:05 [PATCH v8 0/7] V4L2 clock and async patches and soc-camera example Guennadi Liakhovetski
@ 2013-04-08 15:05 ` Guennadi Liakhovetski
  0 siblings, 0 replies; 10+ messages in thread
From: Guennadi Liakhovetski @ 2013-04-08 15:05 UTC (permalink / raw)
  To: linux-media
  Cc: Laurent Pinchart, Hans Verkuil, Sylwester Nawrocki,
	Sylwester Nawrocki, linux-sh, Magnus Damm, Sakari Ailus,
	Prabhakar Lad, Guennadi Liakhovetski

Typical video devices like camera sensors require an external clock source.
Many such devices cannot even access their hardware registers without a
running clock. These clock sources should be controlled by their consumers.
This should be performed, using the generic clock framework. Unfortunately
so far only very few systems have been ported to that framework. This patch
adds a set of temporary helpers, mimicking the generic clock API, to V4L2.
Platforms, adopting the clock API, should switch to using it. Eventually
this temporary API should be removed.

Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
---

v8: Updated both (C) dates

 drivers/media/v4l2-core/Makefile   |    2 +-
 drivers/media/v4l2-core/v4l2-clk.c |  177 ++++++++++++++++++++++++++++++++++++
 include/media/v4l2-clk.h           |   54 +++++++++++
 3 files changed, 232 insertions(+), 1 deletions(-)
 create mode 100644 drivers/media/v4l2-core/v4l2-clk.c
 create mode 100644 include/media/v4l2-clk.h

diff --git a/drivers/media/v4l2-core/Makefile b/drivers/media/v4l2-core/Makefile
index aa50c46..628c630 100644
--- a/drivers/media/v4l2-core/Makefile
+++ b/drivers/media/v4l2-core/Makefile
@@ -5,7 +5,7 @@
 tuner-objs	:=	tuner-core.o
 
 videodev-objs	:=	v4l2-dev.o v4l2-ioctl.o v4l2-device.o v4l2-fh.o \
-			v4l2-event.o v4l2-ctrls.o v4l2-subdev.o
+			v4l2-event.o v4l2-ctrls.o v4l2-subdev.o v4l2-clk.o
 ifeq ($(CONFIG_COMPAT),y)
   videodev-objs += v4l2-compat-ioctl32.o
 endif
diff --git a/drivers/media/v4l2-core/v4l2-clk.c b/drivers/media/v4l2-core/v4l2-clk.c
new file mode 100644
index 0000000..d7cc13e
--- /dev/null
+++ b/drivers/media/v4l2-core/v4l2-clk.c
@@ -0,0 +1,177 @@
+/*
+ * V4L2 clock service
+ *
+ * Copyright (C) 2012-2013, Guennadi Liakhovetski <g.liakhovetski@gmx.de>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/atomic.h>
+#include <linux/device.h>
+#include <linux/errno.h>
+#include <linux/list.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/slab.h>
+#include <linux/string.h>
+
+#include <media/v4l2-clk.h>
+#include <media/v4l2-subdev.h>
+
+static DEFINE_MUTEX(clk_lock);
+static LIST_HEAD(clk_list);
+
+static struct v4l2_clk *v4l2_clk_find(const char *dev_id, const char *id)
+{
+	struct v4l2_clk *clk;
+
+	list_for_each_entry(clk, &clk_list, list) {
+		if (strcmp(dev_id, clk->dev_id))
+			continue;
+
+		if (!id || !clk->id || !strcmp(clk->id, id))
+			return clk;
+	}
+
+	return ERR_PTR(-ENODEV);
+}
+
+struct v4l2_clk *v4l2_clk_get(struct device *dev, const char *id)
+{
+	struct v4l2_clk *clk;
+
+	mutex_lock(&clk_lock);
+	clk = v4l2_clk_find(dev_name(dev), id);
+
+	if (!IS_ERR(clk) && !try_module_get(clk->ops->owner))
+		clk = ERR_PTR(-ENODEV);
+	mutex_unlock(&clk_lock);
+
+	if (!IS_ERR(clk))
+		atomic_inc(&clk->use_count);
+
+	return clk;
+}
+EXPORT_SYMBOL(v4l2_clk_get);
+
+void v4l2_clk_put(struct v4l2_clk *clk)
+{
+	if (!IS_ERR(clk)) {
+		atomic_dec(&clk->use_count);
+		module_put(clk->ops->owner);
+	}
+}
+EXPORT_SYMBOL(v4l2_clk_put);
+
+int v4l2_clk_enable(struct v4l2_clk *clk)
+{
+	int ret;
+	mutex_lock(&clk->lock);
+	if (++clk->enable == 1 && clk->ops->enable) {
+		ret = clk->ops->enable(clk);
+		if (ret < 0)
+			clk->enable--;
+	} else {
+		ret = 0;
+	}
+	mutex_unlock(&clk->lock);
+	return ret;
+}
+EXPORT_SYMBOL(v4l2_clk_enable);
+
+void v4l2_clk_disable(struct v4l2_clk *clk)
+{
+	int enable;
+
+	mutex_lock(&clk->lock);
+	enable = --clk->enable;
+	if (WARN(enable < 0, "Unbalanced %s() on %s:%s!\n", __func__,
+		 clk->dev_id, clk->id))
+		clk->enable++;
+	else if (!enable && clk->ops->disable)
+		clk->ops->disable(clk);
+	mutex_unlock(&clk->lock);
+}
+EXPORT_SYMBOL(v4l2_clk_disable);
+
+unsigned long v4l2_clk_get_rate(struct v4l2_clk *clk)
+{
+	if (!clk->ops->get_rate)
+		return -ENOSYS;
+
+	return clk->ops->get_rate(clk);
+}
+EXPORT_SYMBOL(v4l2_clk_get_rate);
+
+int v4l2_clk_set_rate(struct v4l2_clk *clk, unsigned long rate)
+{
+	if (!clk->ops->set_rate)
+		return -ENOSYS;
+
+	return clk->ops->set_rate(clk, rate);
+}
+EXPORT_SYMBOL(v4l2_clk_set_rate);
+
+struct v4l2_clk *v4l2_clk_register(const struct v4l2_clk_ops *ops,
+				   const char *dev_id,
+				   const char *id, void *priv)
+{
+	struct v4l2_clk *clk;
+	int ret;
+
+	if (!ops || !dev_id)
+		return ERR_PTR(-EINVAL);
+
+	clk = kzalloc(sizeof(struct v4l2_clk), GFP_KERNEL);
+	if (!clk)
+		return ERR_PTR(-ENOMEM);
+
+	clk->id = kstrdup(id, GFP_KERNEL);
+	clk->dev_id = kstrdup(dev_id, GFP_KERNEL);
+	if ((id && !clk->id) || !clk->dev_id) {
+		ret = -ENOMEM;
+		goto ealloc;
+	}
+	clk->ops = ops;
+	clk->priv = priv;
+	atomic_set(&clk->use_count, 0);
+	mutex_init(&clk->lock);
+
+	mutex_lock(&clk_lock);
+	if (!IS_ERR(v4l2_clk_find(dev_id, id))) {
+		mutex_unlock(&clk_lock);
+		ret = -EEXIST;
+		goto eexist;
+	}
+	list_add_tail(&clk->list, &clk_list);
+	mutex_unlock(&clk_lock);
+
+	return clk;
+
+eexist:
+ealloc:
+	kfree(clk->id);
+	kfree(clk->dev_id);
+	kfree(clk);
+	return ERR_PTR(ret);
+}
+EXPORT_SYMBOL(v4l2_clk_register);
+
+void v4l2_clk_unregister(struct v4l2_clk *clk)
+{
+	if (WARN(atomic_read(&clk->use_count),
+		 "%s(): Refusing to unregister ref-counted %s:%s clock!\n",
+		 __func__, clk->dev_id, clk->id))
+		return;
+
+	mutex_lock(&clk_lock);
+	list_del(&clk->list);
+	mutex_unlock(&clk_lock);
+
+	kfree(clk->id);
+	kfree(clk->dev_id);
+	kfree(clk);
+}
+EXPORT_SYMBOL(v4l2_clk_unregister);
diff --git a/include/media/v4l2-clk.h b/include/media/v4l2-clk.h
new file mode 100644
index 0000000..89efbd7
--- /dev/null
+++ b/include/media/v4l2-clk.h
@@ -0,0 +1,54 @@
+/*
+ * V4L2 clock service
+ *
+ * Copyright (C) 2012-2013, Guennadi Liakhovetski <g.liakhovetski@gmx.de>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * ATTENTION: This is a temporary API and it shall be replaced by the generic
+ * clock API, when the latter becomes widely available.
+ */
+
+#ifndef MEDIA_V4L2_CLK_H
+#define MEDIA_V4L2_CLK_H
+
+#include <linux/atomic.h>
+#include <linux/list.h>
+#include <linux/mutex.h>
+
+struct module;
+struct device;
+
+struct v4l2_clk {
+	struct list_head list;
+	const struct v4l2_clk_ops *ops;
+	const char *dev_id;
+	const char *id;
+	int enable;
+	struct mutex lock; /* Protect the enable count */
+	atomic_t use_count;
+	void *priv;
+};
+
+struct v4l2_clk_ops {
+	struct module	*owner;
+	int		(*enable)(struct v4l2_clk *clk);
+	void		(*disable)(struct v4l2_clk *clk);
+	unsigned long	(*get_rate)(struct v4l2_clk *clk);
+	int		(*set_rate)(struct v4l2_clk *clk, unsigned long);
+};
+
+struct v4l2_clk *v4l2_clk_register(const struct v4l2_clk_ops *ops,
+				   const char *dev_name,
+				   const char *name, void *priv);
+void v4l2_clk_unregister(struct v4l2_clk *clk);
+struct v4l2_clk *v4l2_clk_get(struct device *dev, const char *id);
+void v4l2_clk_put(struct v4l2_clk *clk);
+int v4l2_clk_enable(struct v4l2_clk *clk);
+void v4l2_clk_disable(struct v4l2_clk *clk);
+unsigned long v4l2_clk_get_rate(struct v4l2_clk *clk);
+int v4l2_clk_set_rate(struct v4l2_clk *clk, unsigned long rate);
+
+#endif

-- 
1.7.2.5


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

* Re: [PATCH v8 1/7] media: V4L2: add temporary clock helpers
@ 2013-04-11  3:40 Barry Song
  2013-04-11  7:22 ` Guennadi Liakhovetski
  0 siblings, 1 reply; 10+ messages in thread
From: Barry Song @ 2013-04-11  3:40 UTC (permalink / raw)
  To: Guennadi Liakhovetski
  Cc: Sylwester Nawrocki, Mark Brown, linux-media, linux-sh,
	Magnus Damm, Hans Verkuil, Laurent Pinchart, renwei.wu,
	DL-SHA-WorkGroupLinux, xiaomeng.hou, zilong.wu

Hi Guennadi,

> Typical video devices like camera sensors require an external clock source.
> Many such devices cannot even access their hardware registers without a
> running clock. These clock sources should be controlled by their consumers.
> This should be performed, using the generic clock framework. Unfortunately
> so far only very few systems have been ported to that framework. This patch
> adds a set of temporary helpers, mimicking the generic clock API, to V4L2.
> Platforms, adopting the clock API, should switch to using it. Eventually
> this temporary API should be removed.

> Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@xxxxxx>
> ---

for your patch 1/8 and 3/8, i think it makes a lot of senses to let
the object manages its own clock by itself.
is it possible for us to implement v4l2-clk.c directly as an instance
of standard clk driver for those systems which don't have generic
clock,  and remove the V4L2 clock APIs like v4l2_clk_get,
v4l2_clk_enable from the first day? i mean v4l2-clk.c becomes a temp
and fake clock controller driver. finally, after people have
generically clk, remove it.

> v8: Updated both (C) dates

>  drivers/media/v4l2-core/Makefile   |    2 +-
>  drivers/media/v4l2-core/v4l2-clk.c |  177 ++++++++++++++++++++++++++++++++++++
>  include/media/v4l2-clk.h           |   54 +++++++++++
>  3 files changed, 232 insertions(+), 1 deletions(-)
>  create mode 100644 drivers/media/v4l2-core/v4l2-clk.c
>  create mode 100644 include/media/v4l2-clk.h

> diff --git a/drivers/media/v4l2-core/Makefile b/drivers/media/v4l2-core/Makefile
> index aa50c46..628c630 100644
> --- a/drivers/media/v4l2-core/Makefile
> +++ b/drivers/media/v4l2-core/Makefile
> @@ -5,7 +5,7 @@
>  tuner-objs	:=	tuner-core.o

>  videodev-objs	:=	v4l2-dev.o v4l2-ioctl.o v4l2-device.o v4l2-fh.o \
> -			v4l2-event.o v4l2-ctrls.o v4l2-subdev.o
> +			v4l2-event.o v4l2-ctrls.o v4l2-subdev.o v4l2-clk.o
> ifeq ($(CONFIG_COMPAT),y)
>    videodev-objs += v4l2-compat-ioctl32.o
>  endif
> diff --git a/drivers/media/v4l2-core/v4l2-clk.c b/drivers/media/v4l2-core/v4l2-clk.c
> new file mode 100644
> index 0000000..d7cc13e
> --- /dev/null
> +++ b/drivers/media/v4l2-core/v4l2-clk.c
> @@ -0,0 +1,177 @@

-barry

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

* Re: [PATCH v8 1/7] media: V4L2: add temporary clock helpers
  2013-04-11  3:40 [PATCH v8 1/7] media: V4L2: add temporary clock helpers Barry Song
@ 2013-04-11  7:22 ` Guennadi Liakhovetski
  2013-04-11  8:22   ` Barry Song
  0 siblings, 1 reply; 10+ messages in thread
From: Guennadi Liakhovetski @ 2013-04-11  7:22 UTC (permalink / raw)
  To: Barry Song
  Cc: Sylwester Nawrocki, Mark Brown, linux-media, linux-sh,
	Magnus Damm, Hans Verkuil, Laurent Pinchart, renwei.wu,
	DL-SHA-WorkGroupLinux, xiaomeng.hou, zilong.wu

Hi Barry

On Thu, 11 Apr 2013, Barry Song wrote:

> Hi Guennadi,
> 
> > Typical video devices like camera sensors require an external clock source.
> > Many such devices cannot even access their hardware registers without a
> > running clock. These clock sources should be controlled by their consumers.
> > This should be performed, using the generic clock framework. Unfortunately
> > so far only very few systems have been ported to that framework. This patch
> > adds a set of temporary helpers, mimicking the generic clock API, to V4L2.
> > Platforms, adopting the clock API, should switch to using it. Eventually
> > this temporary API should be removed.
> 
> > Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@xxxxxx>
> > ---
> 
> for your patch 1/8 and 3/8, i think it makes a lot of senses to let
> the object manages its own clock by itself.
> is it possible for us to implement v4l2-clk.c directly as an instance
> of standard clk driver for those systems which don't have generic
> clock,  and remove the V4L2 clock APIs like v4l2_clk_get,
> v4l2_clk_enable from the first day? i mean v4l2-clk.c becomes a temp
> and fake clock controller driver. finally, after people have
> generically clk, remove it.

I don't think you can force-enable the CFF on systems, that don't support 
it, e.g. PXA.

Thanks
Guennadi

> > v8: Updated both (C) dates
> 
> >  drivers/media/v4l2-core/Makefile   |    2 +-
> >  drivers/media/v4l2-core/v4l2-clk.c |  177 ++++++++++++++++++++++++++++++++++++
> >  include/media/v4l2-clk.h           |   54 +++++++++++
> >  3 files changed, 232 insertions(+), 1 deletions(-)
> >  create mode 100644 drivers/media/v4l2-core/v4l2-clk.c
> >  create mode 100644 include/media/v4l2-clk.h
> 
> > diff --git a/drivers/media/v4l2-core/Makefile b/drivers/media/v4l2-core/Makefile
> > index aa50c46..628c630 100644
> > --- a/drivers/media/v4l2-core/Makefile
> > +++ b/drivers/media/v4l2-core/Makefile
> > @@ -5,7 +5,7 @@
> >  tuner-objs	:=	tuner-core.o
> 
> >  videodev-objs	:=	v4l2-dev.o v4l2-ioctl.o v4l2-device.o v4l2-fh.o \
> > -			v4l2-event.o v4l2-ctrls.o v4l2-subdev.o
> > +			v4l2-event.o v4l2-ctrls.o v4l2-subdev.o v4l2-clk.o
> > ifeq ($(CONFIG_COMPAT),y)
> >    videodev-objs += v4l2-compat-ioctl32.o
> >  endif
> > diff --git a/drivers/media/v4l2-core/v4l2-clk.c b/drivers/media/v4l2-core/v4l2-clk.c
> > new file mode 100644
> > index 0000000..d7cc13e
> > --- /dev/null
> > +++ b/drivers/media/v4l2-core/v4l2-clk.c
> > @@ -0,0 +1,177 @@
> 
> -barry
> 

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

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

* Re: [PATCH v8 1/7] media: V4L2: add temporary clock helpers
  2013-04-11  7:22 ` Guennadi Liakhovetski
@ 2013-04-11  8:22   ` Barry Song
  2013-04-11  8:36     ` Guennadi Liakhovetski
  0 siblings, 1 reply; 10+ messages in thread
From: Barry Song @ 2013-04-11  8:22 UTC (permalink / raw)
  To: Guennadi Liakhovetski
  Cc: Sylwester Nawrocki, Mark Brown, linux-media, linux-sh,
	Magnus Damm, Hans Verkuil, Laurent Pinchart, renwei.wu,
	DL-SHA-WorkGroupLinux, xiaomeng.hou, zilong.wu, linux-arm-kernel,
	Russell King

2013/4/11 Guennadi Liakhovetski <g.liakhovetski@gmx.de>:
> Hi Barry
>
> On Thu, 11 Apr 2013, Barry Song wrote:
>
>> Hi Guennadi,
>>
>> > Typical video devices like camera sensors require an external clock source.
>> > Many such devices cannot even access their hardware registers without a
>> > running clock. These clock sources should be controlled by their consumers.
>> > This should be performed, using the generic clock framework. Unfortunately
>> > so far only very few systems have been ported to that framework. This patch
>> > adds a set of temporary helpers, mimicking the generic clock API, to V4L2.
>> > Platforms, adopting the clock API, should switch to using it. Eventually
>> > this temporary API should be removed.
>>
>> > Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@xxxxxx>
>> > ---
>>
>> for your patch 1/8 and 3/8, i think it makes a lot of senses to let
>> the object manages its own clock by itself.
>> is it possible for us to implement v4l2-clk.c directly as an instance
>> of standard clk driver for those systems which don't have generic
>> clock,  and remove the V4L2 clock APIs like v4l2_clk_get,
>> v4l2_clk_enable from the first day? i mean v4l2-clk.c becomes a temp
>> and fake clock controller driver. finally, after people have
>> generically clk, remove it.
>
> I don't think you can force-enable the CFF on systems, that don't support
> it, e.g. PXA.

yes. we can. clock is only a framework, has it any limitation to
implement a driver instance on any platform?
people have tried to move to common clk and generic framework for a
long time, now you still try to provide a v4l2 specific clock APIs, it
just makes v4l2 unacceptable and much complex.

>
> Thanks
> Guennadi
>
>> > v8: Updated both (C) dates
>>
>> >  drivers/media/v4l2-core/Makefile   |    2 +-
>> >  drivers/media/v4l2-core/v4l2-clk.c |  177 ++++++++++++++++++++++++++++++++++++
>> >  include/media/v4l2-clk.h           |   54 +++++++++++
>> >  3 files changed, 232 insertions(+), 1 deletions(-)
>> >  create mode 100644 drivers/media/v4l2-core/v4l2-clk.c
>> >  create mode 100644 include/media/v4l2-clk.h
>>
>> > diff --git a/drivers/media/v4l2-core/Makefile b/drivers/media/v4l2-core/Makefile
>> > index aa50c46..628c630 100644
>> > --- a/drivers/media/v4l2-core/Makefile
>> > +++ b/drivers/media/v4l2-core/Makefile
>> > @@ -5,7 +5,7 @@
>> >  tuner-objs :=      tuner-core.o
>>
>> >  videodev-objs      :=      v4l2-dev.o v4l2-ioctl.o v4l2-device.o v4l2-fh.o \
>> > -                   v4l2-event.o v4l2-ctrls.o v4l2-subdev.o
>> > +                   v4l2-event.o v4l2-ctrls.o v4l2-subdev.o v4l2-clk.o
>> > ifeq ($(CONFIG_COMPAT),y)
>> >    videodev-objs += v4l2-compat-ioctl32.o
>> >  endif
>> > diff --git a/drivers/media/v4l2-core/v4l2-clk.c b/drivers/media/v4l2-core/v4l2-clk.c
>> > new file mode 100644
>> > index 0000000..d7cc13e
>> > --- /dev/null
>> > +++ b/drivers/media/v4l2-core/v4l2-clk.c
>> > @@ -0,0 +1,177 @@
>>
>> -barry

-barry

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

* Re: [PATCH v8 1/7] media: V4L2: add temporary clock helpers
  2013-04-11  8:22   ` Barry Song
@ 2013-04-11  8:36     ` Guennadi Liakhovetski
  2013-04-11  8:59       ` Barry Song
  0 siblings, 1 reply; 10+ messages in thread
From: Guennadi Liakhovetski @ 2013-04-11  8:36 UTC (permalink / raw)
  To: Barry Song
  Cc: Sylwester Nawrocki, Mark Brown, linux-media, linux-sh,
	Magnus Damm, Hans Verkuil, Laurent Pinchart, renwei.wu,
	DL-SHA-WorkGroupLinux, xiaomeng.hou, zilong.wu, linux-arm-kernel,
	Russell King

On Thu, 11 Apr 2013, Barry Song wrote:

> 2013/4/11 Guennadi Liakhovetski <g.liakhovetski@gmx.de>:
> > Hi Barry
> >
> > On Thu, 11 Apr 2013, Barry Song wrote:
> >
> >> Hi Guennadi,
> >>
> >> > Typical video devices like camera sensors require an external clock source.
> >> > Many such devices cannot even access their hardware registers without a
> >> > running clock. These clock sources should be controlled by their consumers.
> >> > This should be performed, using the generic clock framework. Unfortunately
> >> > so far only very few systems have been ported to that framework. This patch
> >> > adds a set of temporary helpers, mimicking the generic clock API, to V4L2.
> >> > Platforms, adopting the clock API, should switch to using it. Eventually
> >> > this temporary API should be removed.
> >>
> >> > Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@xxxxxx>
> >> > ---
> >>
> >> for your patch 1/8 and 3/8, i think it makes a lot of senses to let
> >> the object manages its own clock by itself.
> >> is it possible for us to implement v4l2-clk.c directly as an instance
> >> of standard clk driver for those systems which don't have generic
> >> clock,  and remove the V4L2 clock APIs like v4l2_clk_get,
> >> v4l2_clk_enable from the first day? i mean v4l2-clk.c becomes a temp
> >> and fake clock controller driver. finally, after people have
> >> generically clk, remove it.
> >
> > I don't think you can force-enable the CFF on systems, that don't support
> > it, e.g. PXA.
> 
> yes. we can. clock is only a framework, has it any limitation to
> implement a driver instance on any platform?

So, you enable CFF, it provides its own clk_* implementation like 
clk_get_rate() etc. Now, PXA already has it defined in 
arch/arm/mach-pxa/clock.c. Don't think this is going to fly.

Thanks
Guennadi

> people have tried to move to common clk and generic framework for a
> long time, now you still try to provide a v4l2 specific clock APIs, it
> just makes v4l2 unacceptable and much complex.
> 
> >
> > Thanks
> > Guennadi
> >
> >> > v8: Updated both (C) dates
> >>
> >> >  drivers/media/v4l2-core/Makefile   |    2 +-
> >> >  drivers/media/v4l2-core/v4l2-clk.c |  177 ++++++++++++++++++++++++++++++++++++
> >> >  include/media/v4l2-clk.h           |   54 +++++++++++
> >> >  3 files changed, 232 insertions(+), 1 deletions(-)
> >> >  create mode 100644 drivers/media/v4l2-core/v4l2-clk.c
> >> >  create mode 100644 include/media/v4l2-clk.h
> >>
> >> > diff --git a/drivers/media/v4l2-core/Makefile b/drivers/media/v4l2-core/Makefile
> >> > index aa50c46..628c630 100644
> >> > --- a/drivers/media/v4l2-core/Makefile
> >> > +++ b/drivers/media/v4l2-core/Makefile
> >> > @@ -5,7 +5,7 @@
> >> >  tuner-objs :=      tuner-core.o
> >>
> >> >  videodev-objs      :=      v4l2-dev.o v4l2-ioctl.o v4l2-device.o v4l2-fh.o \
> >> > -                   v4l2-event.o v4l2-ctrls.o v4l2-subdev.o
> >> > +                   v4l2-event.o v4l2-ctrls.o v4l2-subdev.o v4l2-clk.o
> >> > ifeq ($(CONFIG_COMPAT),y)
> >> >    videodev-objs += v4l2-compat-ioctl32.o
> >> >  endif
> >> > diff --git a/drivers/media/v4l2-core/v4l2-clk.c b/drivers/media/v4l2-core/v4l2-clk.c
> >> > new file mode 100644
> >> > index 0000000..d7cc13e
> >> > --- /dev/null
> >> > +++ b/drivers/media/v4l2-core/v4l2-clk.c
> >> > @@ -0,0 +1,177 @@
> >>
> >> -barry
> 
> -barry
> 

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

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

* Re: [PATCH v8 1/7] media: V4L2: add temporary clock helpers
  2013-04-11  8:36     ` Guennadi Liakhovetski
@ 2013-04-11  8:59       ` Barry Song
  2013-04-11 18:52         ` Mike Turquette
  0 siblings, 1 reply; 10+ messages in thread
From: Barry Song @ 2013-04-11  8:59 UTC (permalink / raw)
  To: Guennadi Liakhovetski
  Cc: Sylwester Nawrocki, Mark Brown, linux-media, linux-sh,
	Magnus Damm, Hans Verkuil, Laurent Pinchart, renwei.wu,
	DL-SHA-WorkGroupLinux, xiaomeng.hou, zilong.wu, linux-arm-kernel,
	Russell King

2013/4/11 Guennadi Liakhovetski <g.liakhovetski@gmx.de>:
> On Thu, 11 Apr 2013, Barry Song wrote:
>
>> 2013/4/11 Guennadi Liakhovetski <g.liakhovetski@gmx.de>:
>> > Hi Barry
>> >
>> > On Thu, 11 Apr 2013, Barry Song wrote:
>> >
>> >> Hi Guennadi,
>> >>
>> >> > Typical video devices like camera sensors require an external clock source.
>> >> > Many such devices cannot even access their hardware registers without a
>> >> > running clock. These clock sources should be controlled by their consumers.
>> >> > This should be performed, using the generic clock framework. Unfortunately
>> >> > so far only very few systems have been ported to that framework. This patch
>> >> > adds a set of temporary helpers, mimicking the generic clock API, to V4L2.
>> >> > Platforms, adopting the clock API, should switch to using it. Eventually
>> >> > this temporary API should be removed.
>> >>
>> >> > Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@xxxxxx>
>> >> > ---
>> >>
>> >> for your patch 1/8 and 3/8, i think it makes a lot of senses to let
>> >> the object manages its own clock by itself.
>> >> is it possible for us to implement v4l2-clk.c directly as an instance
>> >> of standard clk driver for those systems which don't have generic
>> >> clock,  and remove the V4L2 clock APIs like v4l2_clk_get,
>> >> v4l2_clk_enable from the first day? i mean v4l2-clk.c becomes a temp
>> >> and fake clock controller driver. finally, after people have
>> >> generically clk, remove it.
>> >
>> > I don't think you can force-enable the CFF on systems, that don't support
>> > it, e.g. PXA.
>>
>> yes. we can. clock is only a framework, has it any limitation to
>> implement a driver instance on any platform?
>
> So, you enable CFF, it provides its own clk_* implementation like
> clk_get_rate() etc. Now, PXA already has it defined in
> arch/arm/mach-pxa/clock.c. Don't think this is going to fly.

agree.

>
> Thanks
> Guennadi
>
>> people have tried to move to common clk and generic framework for a
>> long time, now you still try to provide a v4l2 specific clock APIs, it
>> just makes v4l2 unacceptable and much complex.
>>
>> >
>> > Thanks
>> > Guennadi
>> >
>> >> > v8: Updated both (C) dates
>> >>
>> >> >  drivers/media/v4l2-core/Makefile   |    2 +-
>> >> >  drivers/media/v4l2-core/v4l2-clk.c |  177 ++++++++++++++++++++++++++++++++++++
>> >> >  include/media/v4l2-clk.h           |   54 +++++++++++
>> >> >  3 files changed, 232 insertions(+), 1 deletions(-)
>> >> >  create mode 100644 drivers/media/v4l2-core/v4l2-clk.c
>> >> >  create mode 100644 include/media/v4l2-clk.h
>> >>
>> >> > diff --git a/drivers/media/v4l2-core/Makefile b/drivers/media/v4l2-core/Makefile
>> >> > index aa50c46..628c630 100644
>> >> > --- a/drivers/media/v4l2-core/Makefile
>> >> > +++ b/drivers/media/v4l2-core/Makefile
>> >> > @@ -5,7 +5,7 @@
>> >> >  tuner-objs :=      tuner-core.o
>> >>
>> >> >  videodev-objs      :=      v4l2-dev.o v4l2-ioctl.o v4l2-device.o v4l2-fh.o \
>> >> > -                   v4l2-event.o v4l2-ctrls.o v4l2-subdev.o
>> >> > +                   v4l2-event.o v4l2-ctrls.o v4l2-subdev.o v4l2-clk.o
>> >> > ifeq ($(CONFIG_COMPAT),y)
>> >> >    videodev-objs += v4l2-compat-ioctl32.o
>> >> >  endif
>> >> > diff --git a/drivers/media/v4l2-core/v4l2-clk.c b/drivers/media/v4l2-core/v4l2-clk.c
>> >> > new file mode 100644
>> >> > index 0000000..d7cc13e
>> >> > --- /dev/null
>> >> > +++ b/drivers/media/v4l2-core/v4l2-clk.c
>> >> > @@ -0,0 +1,177 @@
>> >>
>> >> -barry

-barry

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

* Re: [PATCH v8 1/7] media: V4L2: add temporary clock helpers
  2013-04-11  8:59       ` Barry Song
@ 2013-04-11 18:52         ` Mike Turquette
  2013-04-11 20:14           ` Sylwester Nawrocki
  2013-04-11 22:35           ` Laurent Pinchart
  0 siblings, 2 replies; 10+ messages in thread
From: Mike Turquette @ 2013-04-11 18:52 UTC (permalink / raw)
  To: Barry Song, Guennadi Liakhovetski
  Cc: linux-arm-kernel, renwei.wu, linux-sh, Mark Brown, Magnus Damm,
	DL-SHA-WorkGroupLinux, Hans Verkuil, Laurent Pinchart,
	Russell King, Sylwester Nawrocki, zilong.wu, xiaomeng.hou,
	linux-media

Quoting Barry Song (2013-04-11 01:59:28)
> 2013/4/11 Guennadi Liakhovetski <g.liakhovetski@gmx.de>:
> > On Thu, 11 Apr 2013, Barry Song wrote:
> >
> >> 2013/4/11 Guennadi Liakhovetski <g.liakhovetski@gmx.de>:
> >> > Hi Barry
> >> >
> >> > On Thu, 11 Apr 2013, Barry Song wrote:
> >> >
> >> >> Hi Guennadi,
> >> >>
> >> >> > Typical video devices like camera sensors require an external clock source.
> >> >> > Many such devices cannot even access their hardware registers without a
> >> >> > running clock. These clock sources should be controlled by their consumers.
> >> >> > This should be performed, using the generic clock framework. Unfortunately
> >> >> > so far only very few systems have been ported to that framework. This patch
> >> >> > adds a set of temporary helpers, mimicking the generic clock API, to V4L2.
> >> >> > Platforms, adopting the clock API, should switch to using it. Eventually
> >> >> > this temporary API should be removed.
> >> >>
> >> >> > Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@xxxxxx>
> >> >> > ---
> >> >>
> >> >> for your patch 1/8 and 3/8, i think it makes a lot of senses to let
> >> >> the object manages its own clock by itself.
> >> >> is it possible for us to implement v4l2-clk.c directly as an instance
> >> >> of standard clk driver for those systems which don't have generic
> >> >> clock,  and remove the V4L2 clock APIs like v4l2_clk_get,
> >> >> v4l2_clk_enable from the first day? i mean v4l2-clk.c becomes a temp
> >> >> and fake clock controller driver. finally, after people have
> >> >> generically clk, remove it.
> >> >
> >> > I don't think you can force-enable the CFF on systems, that don't support
> >> > it, e.g. PXA.
> >>
> >> yes. we can. clock is only a framework, has it any limitation to
> >> implement a driver instance on any platform?
> >
> > So, you enable CFF, it provides its own clk_* implementation like
> > clk_get_rate() etc. Now, PXA already has it defined in
> > arch/arm/mach-pxa/clock.c. Don't think this is going to fly.
> 
> agree.
> 

Hi,

I came into this thread late and don't have the actual patches in my
inbox for review.  That said, I don't understand why V4L2 cares about
the clk framework *implementation*?  The clk.h api is the same for
platforms using the common struct clk and those still using the legacy
method of defining their own struct clk.  If drivers are only consumers
of the clk.h api then the implementation underneath should not matter.

Regards,
Mike

> >
> > Thanks
> > Guennadi
> >
> >> people have tried to move to common clk and generic framework for a
> >> long time, now you still try to provide a v4l2 specific clock APIs, it
> >> just makes v4l2 unacceptable and much complex.
> >>
> >> >
> >> > Thanks
> >> > Guennadi
> >> >
> >> >> > v8: Updated both (C) dates
> >> >>
> >> >> >  drivers/media/v4l2-core/Makefile   |    2 +-
> >> >> >  drivers/media/v4l2-core/v4l2-clk.c |  177 ++++++++++++++++++++++++++++++++++++
> >> >> >  include/media/v4l2-clk.h           |   54 +++++++++++
> >> >> >  3 files changed, 232 insertions(+), 1 deletions(-)
> >> >> >  create mode 100644 drivers/media/v4l2-core/v4l2-clk.c
> >> >> >  create mode 100644 include/media/v4l2-clk.h
> >> >>
> >> >> > diff --git a/drivers/media/v4l2-core/Makefile b/drivers/media/v4l2-core/Makefile
> >> >> > index aa50c46..628c630 100644
> >> >> > --- a/drivers/media/v4l2-core/Makefile
> >> >> > +++ b/drivers/media/v4l2-core/Makefile
> >> >> > @@ -5,7 +5,7 @@
> >> >> >  tuner-objs :=      tuner-core.o
> >> >>
> >> >> >  videodev-objs      :=      v4l2-dev.o v4l2-ioctl.o v4l2-device.o v4l2-fh.o \
> >> >> > -                   v4l2-event.o v4l2-ctrls.o v4l2-subdev.o
> >> >> > +                   v4l2-event.o v4l2-ctrls.o v4l2-subdev.o v4l2-clk.o
> >> >> > ifeq ($(CONFIG_COMPAT),y)
> >> >> >    videodev-objs += v4l2-compat-ioctl32.o
> >> >> >  endif
> >> >> > diff --git a/drivers/media/v4l2-core/v4l2-clk.c b/drivers/media/v4l2-core/v4l2-clk.c
> >> >> > new file mode 100644
> >> >> > index 0000000..d7cc13e
> >> >> > --- /dev/null
> >> >> > +++ b/drivers/media/v4l2-core/v4l2-clk.c
> >> >> > @@ -0,0 +1,177 @@
> >> >>
> >> >> -barry
> 
> -barry
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [PATCH v8 1/7] media: V4L2: add temporary clock helpers
  2013-04-11 18:52         ` Mike Turquette
@ 2013-04-11 20:14           ` Sylwester Nawrocki
  2013-04-11 22:35           ` Laurent Pinchart
  1 sibling, 0 replies; 10+ messages in thread
From: Sylwester Nawrocki @ 2013-04-11 20:14 UTC (permalink / raw)
  To: Mike Turquette
  Cc: Barry Song, Guennadi Liakhovetski, linux-arm-kernel, renwei.wu,
	linux-sh, Mark Brown, Magnus Damm, DL-SHA-WorkGroupLinux,
	Hans Verkuil, Laurent Pinchart, Russell King, zilong.wu,
	xiaomeng.hou, linux-media

Hi,

On 04/11/2013 08:52 PM, Mike Turquette wrote:
[...]
>>> So, you enable CFF, it provides its own clk_* implementation like
>>> clk_get_rate() etc. Now, PXA already has it defined in
>>> arch/arm/mach-pxa/clock.c. Don't think this is going to fly.
>>
>> agree.
>
> Hi,
>
> I came into this thread late and don't have the actual patches in my
> inbox for review.  That said, I don't understand why V4L2 cares about
> the clk framework *implementation*?  The clk.h api is the same for
> platforms using the common struct clk and those still using the legacy
> method of defining their own struct clk.  If drivers are only consumers
> of the clk.h api then the implementation underneath should not matter.

I came to similar conclusions previously, but in case when one of the two
drivers is the clock provider I think there is still an issue there.

The drivers are supposed to be platform agnostic, but the clock provider
would have to include mach specific declarations of struct clk, wouldn't
it ?

Regards,
Sylwester

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

* Re: [PATCH v8 1/7] media: V4L2: add temporary clock helpers
  2013-04-11 18:52         ` Mike Turquette
  2013-04-11 20:14           ` Sylwester Nawrocki
@ 2013-04-11 22:35           ` Laurent Pinchart
       [not found]             ` <20130411231923.7915.17215@quantum>
  1 sibling, 1 reply; 10+ messages in thread
From: Laurent Pinchart @ 2013-04-11 22:35 UTC (permalink / raw)
  To: Mike Turquette
  Cc: Barry Song, Guennadi Liakhovetski, linux-arm-kernel, renwei.wu,
	linux-sh, Mark Brown, Magnus Damm, DL-SHA-WorkGroupLinux,
	Hans Verkuil, Russell King, Sylwester Nawrocki, zilong.wu,
	xiaomeng.hou, linux-media

Hi Mike,

On Thursday 11 April 2013 11:52:58 Mike Turquette wrote:
> Quoting Barry Song (2013-04-11 01:59:28)
> > 2013/4/11 Guennadi Liakhovetski <g.liakhovetski@gmx.de>:
> > > On Thu, 11 Apr 2013, Barry Song wrote:
> > >> 2013/4/11 Guennadi Liakhovetski <g.liakhovetski@gmx.de>:
> > >> > On Thu, 11 Apr 2013, Barry Song wrote:
> > >> >> Hi Guennadi,
> > >> >> 
> > >> >> > Typical video devices like camera sensors require an external
> > >> >> > clock source. Many such devices cannot even access their hardware
> > >> >> > registers without a running clock. These clock sources should be
> > >> >> > controlled by their consumers. This should be performed, using the
> > >> >> > generic clock framework. Unfortunately so far only very few
> > >> >> > systems have been ported to that framework. This patch adds a set
> > >> >> > of temporary helpers, mimicking the generic clock API, to V4L2.
> > >> >> > Platforms, adopting the clock API, should switch to using it.
> > >> >> > Eventually this temporary API should be removed.
> > >> >> > 
> > >> >> > Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@xxxxxx>
> > >> >> > ---
> > >> >> 
> > >> >> for your patch 1/8 and 3/8, i think it makes a lot of senses to let
> > >> >> the object manages its own clock by itself.
> > >> >> is it possible for us to implement v4l2-clk.c directly as an
> > >> >> instance of standard clk driver for those systems which don't have
> > >> >> generic clock,  and remove the V4L2 clock APIs like v4l2_clk_get,
> > >> >> v4l2_clk_enable from the first day? i mean v4l2-clk.c becomes a temp
> > >> >> and fake clock controller driver. finally, after people have
> > >> >> generically clk, remove it.
> > >> > 
> > >> > I don't think you can force-enable the CFF on systems, that don't
> > >> > support it, e.g. PXA.
> > >> 
> > >> yes. we can. clock is only a framework, has it any limitation to
> > >> implement a driver instance on any platform?
> > > 
> > > So, you enable CFF, it provides its own clk_* implementation like
> > > clk_get_rate() etc. Now, PXA already has it defined in
> > > arch/arm/mach-pxa/clock.c. Don't think this is going to fly.
> > 
> > agree.
> 
> I came into this thread late and don't have the actual patches in my inbox
> for review.  That said, I don't understand why V4L2 cares about the clk
> framework *implementation*?  The clk.h api is the same for platforms using
> the common struct clk and those still using the legacy method of defining
> their own struct clk.  If drivers are only consumers of the clk.h api then
> the implementation underneath should not matter.

The issue on non-CCF systems is that devices usually can't register clocks 
dynamically. (Most of) those systems provide system clocks only through their 
clock API, without a way for the camera IP core to hook up the clock(s) it can 
provide to the camera sensor. On the consumer side we don't care much about 
the clock framework implementation, but on the provider side we need a 
framework that allows registering non-system clocks at runtime.

-- 
Regards,

Laurent Pinchart


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

* Re: [PATCH v8 1/7] media: V4L2: add temporary clock helpers
       [not found]             ` <20130411231923.7915.17215@quantum>
@ 2013-04-12  8:22               ` Laurent Pinchart
  0 siblings, 0 replies; 10+ messages in thread
From: Laurent Pinchart @ 2013-04-12  8:22 UTC (permalink / raw)
  To: Mike Turquette
  Cc: Barry Song, Guennadi Liakhovetski, linux-arm-kernel, renwei.wu,
	linux-sh, Mark Brown, Magnus Damm, DL-SHA-WorkGroupLinux,
	Hans Verkuil, Russell King, Sylwester Nawrocki, zilong.wu,
	xiaomeng.hou, linux-media

Hi Mike,

On Thursday 11 April 2013 16:19:23 Mike Turquette wrote:
> Quoting Laurent Pinchart (2013-04-11 15:35:38)
> > On Thursday 11 April 2013 11:52:58 Mike Turquette wrote:

[snip]

> > > I came into this thread late and don't have the actual patches in my
> > > inbox for review.  That said, I don't understand why V4L2 cares about
> > > the clk framework *implementation*?  The clk.h api is the same for
> > > platforms using the common struct clk and those still using the legacy
> > > method of defining their own struct clk.  If drivers are only consumers
> > > of the clk.h api then the implementation underneath should not matter.
> > 
> > The issue on non-CCF systems is that devices usually can't register clocks
> > dynamically. (Most of) those systems provide system clocks only through
> > their clock API, without a way for the camera IP core to hook up the
> > clock(s) it can provide to the camera sensor. On the consumer side we
> > don't care much about the clock framework implementation, but on the
> > provider side we need a framework that allows registering non-system
> > clocks at runtime.
> 
> Yes, you do care about the clock framework implementation if you are a clock
> provider.  I still haven't gone through the archives to find these patches
> but I hope that any dependency on CONFIG_COMMON_CLK is conditionalized to
> have the smallest impact possible. Making v4l2 as a whole depend on
> COMMON_CLK might be a bit overkill compared to just making individual camera
> drivers depend on it.

The basic idea is to push the dependency on CONFIG_COMMON_CLK to individual 
drivers, and provide a V4L2-specific clock framework (that looks like a 
stripped-down version of CCF) for platforms that don't implement CCF yet.

-- 
Regards,

Laurent Pinchart


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

end of thread, other threads:[~2013-04-12  8:22 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-04-11  3:40 [PATCH v8 1/7] media: V4L2: add temporary clock helpers Barry Song
2013-04-11  7:22 ` Guennadi Liakhovetski
2013-04-11  8:22   ` Barry Song
2013-04-11  8:36     ` Guennadi Liakhovetski
2013-04-11  8:59       ` Barry Song
2013-04-11 18:52         ` Mike Turquette
2013-04-11 20:14           ` Sylwester Nawrocki
2013-04-11 22:35           ` Laurent Pinchart
     [not found]             ` <20130411231923.7915.17215@quantum>
2013-04-12  8:22               ` Laurent Pinchart
  -- strict thread matches above, loose matches on Subject: below --
2013-04-08 15:05 [PATCH v8 0/7] V4L2 clock and async patches and soc-camera example Guennadi Liakhovetski
2013-04-08 15:05 ` [PATCH v8 1/7] media: V4L2: add temporary clock helpers Guennadi Liakhovetski

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox