Re: [PATCH 1/2] media: V4L2: add temporary clock helpers
Hello, On Sunday 21 October 2012 20:52:21 Sylwester Nawrocki wrote: On 10/20/2012 12:20 AM, Guennadi Liakhovetski wrote: 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. So I gave this patch a try this weekend. I would have a few comments/ questions. Thank you for sharing this! I've finally found time to give it a try, and I can report successful results. My development target here is a Beagleboard-xM with an MT9P031 sensor. With this patch and Sylwester's additional patches [1], I've been able to remove the board code callback from the mt9p031 driver platform data, as well as the last omap3-isp platform callback. The result is available in the devel/v4l2-clock branch of http://git.linuxtv.org/pinchartl/media.git. Sylwester, that branch includes a minor fix titled v4l2-clk: Fix clock id matching for your v4l2-clk: Rework to accept more than one clock with null clock id patch. Could you please have a look at it ? On the downside, there's now a circular dependency between the mt9p031 and omap3-isp drivers, so neither of them can be removed. That will need to be fixed. [1] http://git.linuxtv.org/snawrocki/media.git/shortlog/refs/heads/s3c-camif- devel -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] media: V4L2: add temporary clock helpers
Hi Sylwester
On Sun, 21 Oct 2012, Sylwester Nawrocki wrote:
Hi Guennadi,
On 10/20/2012 12:20 AM, Guennadi Liakhovetski wrote:
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.
So I gave this patch a try this weekend. I would have a few comments/
questions. Thank you for sharing this!
You mean you actually tried to use it? Wow, impressive! :-)
Signed-off-by: Guennadi Liakhovetskig.liakhovet...@gmx.de
---
drivers/media/v4l2-core/Makefile |2 +-
drivers/media/v4l2-core/v4l2-clk.c | 126
include/media/v4l2-clk.h | 48 ++
3 files changed, 175 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 00f64d6..cb5fede 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 000..7d457e4
--- /dev/null
+++ b/drivers/media/v4l2-core/v4l2-clk.c
@@ -0,0 +1,126 @@
+/*
+ * V4L2 clock service
A like the name :-D
+ *
+ * Copyright (C) 2012, Guennadi Liakhovetskig.liakhovet...@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.
+ */
+
+#includelinux/errno.h
+#includelinux/list.h
+#includelinux/module.h
+#includelinux/mutex.h
+#includelinux/string.h
+
+#includemedia/v4l2-clk.h
+#includemedia/v4l2-subdev.h
+
+static DEFINE_MUTEX(clk_lock);
+static LIST_HEAD(v4l2_clk);
nit: how about naming this lists v4l2_clks ?
+
+struct v4l2_clk *v4l2_clk_get(struct v4l2_subdev *sd, const char *id)
+{
+ struct v4l2_clk *clk = NULL;
+
+ mutex_lock(clk_lock);
+ if (!id) {
+ if (list_is_singular(v4l2_clk)) {
Hmm, the clock list is global, why should we assume there will be only one
entry with NULL v4l2_clk::id ?
This is testing for a case, when the user is trying to obtain a clock
without providing an ID, similar to how with real clocks you can do
clk = clk_get(dev, NULL);
It would be useful to not provide the per
subdev clock id when there is only one clock used per a sub-device, which
is a majority of cases AFAICT.
+ clk = list_entry(v4l2_clk, struct v4l2_clk, list);
+ if (!strstr(sd-name, clk-dev_id))
Ok, then clk-dev_id is supposed to be a sub-string of sd-name,
looks good...
Looks like the no-ID case hasn't been tested... Make it
+ clk = list_first_entry(v4l2_clk, struct v4l2_clk,
list);
+ if (strcmp(sd-name, clk-dev_id))
+ clk = ERR_PTR(-ENODEV);
+ } else {
+ clk = ERR_PTR(-EINVAL);
+ }
+ } else {
+ list_for_each_entry(clk,v4l2_clk, list) {
+ if (!strcmp(id, clk-id)
+ !strcmp(sd-name, clk-dev_id))
but why we are doing a strong check here ? Couldn't the second strcmp()
be just strstr() ?
I prefer both to be strcmp to avoid degenerate cases with just one letter
etc.
+ break;
+ }
+ if (clk-list ==v4l2_clk)
+ clk = ERR_PTR(-ENODEV);
+ }
+ mutex_unlock(clk_lock);
+
+ if (!IS_ERR(clk)
+ !try_module_get(clk-ops-owner))
+ clk = ERR_PTR(-ENODEV);
+
+ return clk;
+}
+EXPORT_SYMBOL(v4l2_clk_get);
+void v4l2_clk_put(struct v4l2_clk *clk)
+{
+ if (!IS_ERR(clk))
+ module_put(clk-ops-owner);
+}
+EXPORT_SYMBOL(v4l2_clk_put);
+
+int v4l2_clk_enable(struct v4l2_clk *clk)
+{
+ if (!clk-ops-enable)
+ return -ENOSYS;
+ return clk-ops-enable(clk);
+}
+EXPORT_SYMBOL(v4l2_clk_enable);
+
+void
Re: [PATCH 1/2] media: V4L2: add temporary clock helpers
On 10/22/2012 11:14 AM, Guennadi Liakhovetski wrote:
On Sun, 21 Oct 2012, Sylwester Nawrocki wrote:
On 10/20/2012 12:20 AM, Guennadi Liakhovetski wrote:
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.
So I gave this patch a try this weekend. I would have a few comments/
questions. Thank you for sharing this!
You mean you actually tried to use it? Wow, impressive! :-)
Signed-off-by: Guennadi Liakhovetskig.liakhovet...@gmx.de
---
drivers/media/v4l2-core/Makefile |2 +-
drivers/media/v4l2-core/v4l2-clk.c | 126
include/media/v4l2-clk.h | 48 ++
3 files changed, 175 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 00f64d6..cb5fede 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 000..7d457e4
--- /dev/null
+++ b/drivers/media/v4l2-core/v4l2-clk.c
@@ -0,0 +1,126 @@
+/*
+ * V4L2 clock service
A like the name :-D
+ *
+ * Copyright (C) 2012, Guennadi Liakhovetskig.liakhovet...@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.
+ */
+
+#includelinux/errno.h
+#includelinux/list.h
+#includelinux/module.h
+#includelinux/mutex.h
+#includelinux/string.h
+
+#includemedia/v4l2-clk.h
+#includemedia/v4l2-subdev.h
+
+static DEFINE_MUTEX(clk_lock);
+static LIST_HEAD(v4l2_clk);
nit: how about naming this lists v4l2_clks ?
+
+struct v4l2_clk *v4l2_clk_get(struct v4l2_subdev *sd, const char *id)
+{
+ struct v4l2_clk *clk = NULL;
+
+ mutex_lock(clk_lock);
+ if (!id) {
+ if (list_is_singular(v4l2_clk)) {
Hmm, the clock list is global, why should we assume there will be only one
entry with NULL v4l2_clk::id ?
This is testing for a case, when the user is trying to obtain a clock
without providing an ID, similar to how with real clocks you can do
clk = clk_get(dev, NULL);
Right, but there may be a need to handle more than one clock like this.
In Samsung Exynos SoC there are two clocks, for each physical video input
bus. I will like not use this temporary clock API, as I seem to have common
clock framework and DT support available there (just need to sort out a few
crashes yet ;). But still why not to allow more than one clock ?
It would be useful to not provide the per
subdev clock id when there is only one clock used per a sub-device, which
is a majority of cases AFAICT.
+ clk = list_entry(v4l2_clk, struct v4l2_clk, list);
+ if (!strstr(sd-name, clk-dev_id))
Ok, then clk-dev_id is supposed to be a sub-string of sd-name,
looks good...
Looks like the no-ID case hasn't been tested... Make it
+ clk = list_first_entry(v4l2_clk, struct v4l2_clk,
list);
+ if (strcmp(sd-name, clk-dev_id))
Right, I noticed it too, and fixed this list handling temporarily like that.
As for sd-name, this is supposed to be subdev's name as created by subdev
driver during its' probing ? And the clock may need to be registered before
subdev has been probed or the host gets hold of the subdev ?
How would we make sure the host knows _subdev's_, i.e. not it's driver's
name ?
I know there are standard functions like v4l2_i2c_subdev_init() and
v4l2_spi_subdev_init() that initialize sd-name using standar pattern, but
subdev driver can overwrite the name. I.e. to avoid I2C adapter and I2C
slave address numbers creeping in into subdev names, which are then exposed
to user space through Media Controller API.
+ clk = ERR_PTR(-ENODEV);
+ } else {
+ clk = ERR_PTR(-EINVAL);
+ }
+ } else {
+
Re: [PATCH 1/2] media: V4L2: add temporary clock helpers
Hi Guennadi,
Thanks for the patch. Here are a few comments in addition to what Sylwester
already mentioned.
On Saturday 20 October 2012 00:20:20 Guennadi Liakhovetski wrote:
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.liakhovet...@gmx.de
---
drivers/media/v4l2-core/Makefile |2 +-
drivers/media/v4l2-core/v4l2-clk.c | 126 +
include/media/v4l2-clk.h | 48 ++
3 files changed, 175 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 00f64d6..cb5fede 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 000..7d457e4
--- /dev/null
+++ b/drivers/media/v4l2-core/v4l2-clk.c
@@ -0,0 +1,126 @@
+/*
+ * V4L2 clock service
+ *
+ * Copyright (C) 2012, Guennadi Liakhovetski g.liakhovet...@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/errno.h
+#include linux/list.h
+#include linux/module.h
+#include linux/mutex.h
+#include linux/string.h
+
+#include media/v4l2-clk.h
+#include media/v4l2-subdev.h
+
+static DEFINE_MUTEX(clk_lock);
+static LIST_HEAD(v4l2_clk);
+
+struct v4l2_clk *v4l2_clk_get(struct v4l2_subdev *sd, const char *id)
+{
+ struct v4l2_clk *clk = NULL;
+
+ mutex_lock(clk_lock);
+ if (!id) {
+ if (list_is_singular(v4l2_clk)) {
+ clk = list_entry(v4l2_clk, struct v4l2_clk, list);
+ if (!strstr(sd-name, clk-dev_id))
+ clk = ERR_PTR(-ENODEV);
+ } else {
+ clk = ERR_PTR(-EINVAL);
+ }
+ } else {
+ list_for_each_entry(clk, v4l2_clk, list) {
+ if (!strcmp(id, clk-id)
+ !strcmp(sd-name, clk-dev_id))
+ break;
+ }
+ if (clk-list == v4l2_clk)
+ clk = ERR_PTR(-ENODEV);
+ }
+ mutex_unlock(clk_lock);
+
+ if (!IS_ERR(clk)
+ !try_module_get(clk-ops-owner))
+ clk = ERR_PTR(-ENODEV);
+
+ return clk;
+}
+EXPORT_SYMBOL(v4l2_clk_get);
+
+void v4l2_clk_put(struct v4l2_clk *clk)
+{
+ if (!IS_ERR(clk))
+ module_put(clk-ops-owner);
+}
+EXPORT_SYMBOL(v4l2_clk_put);
+
+int v4l2_clk_enable(struct v4l2_clk *clk)
+{
+ if (!clk-ops-enable)
+ return -ENOSYS;
+ return clk-ops-enable(clk);
+}
+EXPORT_SYMBOL(v4l2_clk_enable);
+
+void v4l2_clk_disable(struct v4l2_clk *clk)
+{
+ if (clk-ops-disable)
+ clk-ops-disable(clk);
+}
+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_name,
+const char *name)
+{
+ struct v4l2_clk *clk;
+
+ if (!ops || !ops-owner || (!list_empty(v4l2_clk) !name))
+ return ERR_PTR(-EINVAL);
+
+ clk = kzalloc(sizeof(struct v4l2_clk), GFP_KERNEL);
+ if (!clk)
+ return ERR_PTR(-ENOMEM);
+
+ clk-ops = ops;
+ clk-id = name;
+ clk-dev_id = dev_name;
What about kstrdup() ing name and dev_name here ? Otherwise callers would need
Re: [PATCH 1/2] media: V4L2: add temporary clock helpers
Hi Guennadi,
A couple more comments.
On Saturday 20 October 2012 00:20:20 Guennadi Liakhovetski wrote:
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.liakhovet...@gmx.de
---
drivers/media/v4l2-core/Makefile |2 +-
drivers/media/v4l2-core/v4l2-clk.c | 126 +
include/media/v4l2-clk.h | 48 ++
3 files changed, 175 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 00f64d6..cb5fede 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 000..7d457e4
--- /dev/null
+++ b/drivers/media/v4l2-core/v4l2-clk.c
@@ -0,0 +1,126 @@
+/*
+ * V4L2 clock service
+ *
+ * Copyright (C) 2012, Guennadi Liakhovetski g.liakhovet...@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/errno.h
+#include linux/list.h
+#include linux/module.h
+#include linux/mutex.h
+#include linux/string.h
+
+#include media/v4l2-clk.h
+#include media/v4l2-subdev.h
+
+static DEFINE_MUTEX(clk_lock);
+static LIST_HEAD(v4l2_clk);
+
+struct v4l2_clk *v4l2_clk_get(struct v4l2_subdev *sd, const char *id)
+{
+ struct v4l2_clk *clk = NULL;
+
+ mutex_lock(clk_lock);
+ if (!id) {
+ if (list_is_singular(v4l2_clk)) {
+ clk = list_entry(v4l2_clk, struct v4l2_clk, list);
+ if (!strstr(sd-name, clk-dev_id))
+ clk = ERR_PTR(-ENODEV);
+ } else {
+ clk = ERR_PTR(-EINVAL);
+ }
+ } else {
+ list_for_each_entry(clk, v4l2_clk, list) {
+ if (!strcmp(id, clk-id)
+ !strcmp(sd-name, clk-dev_id))
+ break;
+ }
+ if (clk-list == v4l2_clk)
+ clk = ERR_PTR(-ENODEV);
+ }
+ mutex_unlock(clk_lock);
+
+ if (!IS_ERR(clk)
+ !try_module_get(clk-ops-owner))
+ clk = ERR_PTR(-ENODEV);
+
+ return clk;
+}
+EXPORT_SYMBOL(v4l2_clk_get);
+
+void v4l2_clk_put(struct v4l2_clk *clk)
+{
+ if (!IS_ERR(clk))
+ module_put(clk-ops-owner);
+}
+EXPORT_SYMBOL(v4l2_clk_put);
+
+int v4l2_clk_enable(struct v4l2_clk *clk)
+{
+ if (!clk-ops-enable)
+ return -ENOSYS;
Are enable/disable supposed to be refcounted ? What about adding the refcount
(or a boolean enable field) to struct v4l2_clk and handling it here and in
v4l2_clk_disable() ?
+ return clk-ops-enable(clk);
+}
+EXPORT_SYMBOL(v4l2_clk_enable);
+
+void v4l2_clk_disable(struct v4l2_clk *clk)
+{
+ if (clk-ops-disable)
+ clk-ops-disable(clk);
+}
+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);
What's the expected behaviour when calling v4l2_clk_get/set_rate on a disabled
clock ?
+}
+EXPORT_SYMBOL(v4l2_clk_set_rate);
+
+struct v4l2_clk *v4l2_clk_register(const struct v4l2_clk_ops *ops,
+const char *dev_name,
+const char *name)
+{
+ struct v4l2_clk *clk;
+
+ if (!ops || !ops-owner || (!list_empty(v4l2_clk) !name))
+ return ERR_PTR(-EINVAL);
+
+ clk = kzalloc(sizeof(struct v4l2_clk), GFP_KERNEL);
+ if (!clk)
+
Re: [PATCH 1/2] media: V4L2: add temporary clock helpers
Hi Guennadi,
On 10/20/2012 12:20 AM, Guennadi Liakhovetski wrote:
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.
So I gave this patch a try this weekend. I would have a few comments/
questions. Thank you for sharing this!
Signed-off-by: Guennadi Liakhovetskig.liakhovet...@gmx.de
---
drivers/media/v4l2-core/Makefile |2 +-
drivers/media/v4l2-core/v4l2-clk.c | 126
include/media/v4l2-clk.h | 48 ++
3 files changed, 175 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 00f64d6..cb5fede 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 000..7d457e4
--- /dev/null
+++ b/drivers/media/v4l2-core/v4l2-clk.c
@@ -0,0 +1,126 @@
+/*
+ * V4L2 clock service
A like the name :-D
+ *
+ * Copyright (C) 2012, Guennadi Liakhovetskig.liakhovet...@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.
+ */
+
+#includelinux/errno.h
+#includelinux/list.h
+#includelinux/module.h
+#includelinux/mutex.h
+#includelinux/string.h
+
+#includemedia/v4l2-clk.h
+#includemedia/v4l2-subdev.h
+
+static DEFINE_MUTEX(clk_lock);
+static LIST_HEAD(v4l2_clk);
nit: how about naming this lists v4l2_clks ?
+
+struct v4l2_clk *v4l2_clk_get(struct v4l2_subdev *sd, const char *id)
+{
+ struct v4l2_clk *clk = NULL;
+
+ mutex_lock(clk_lock);
+ if (!id) {
+ if (list_is_singular(v4l2_clk)) {
Hmm, the clock list is global, why should we assume there will be only one
entry with NULL v4l2_clk::id ? It would be useful to not provide the per
subdev clock id when there is only one clock used per a sub-device, which
is a majority of cases AFAICT.
+ clk = list_entry(v4l2_clk, struct v4l2_clk, list);
+ if (!strstr(sd-name, clk-dev_id))
Ok, then clk-dev_id is supposed to be a sub-string of sd-name,
looks good...
+ clk = ERR_PTR(-ENODEV);
+ } else {
+ clk = ERR_PTR(-EINVAL);
+ }
+ } else {
+ list_for_each_entry(clk,v4l2_clk, list) {
+ if (!strcmp(id, clk-id)
+ !strcmp(sd-name, clk-dev_id))
but why we are doing a strong check here ? Couldn't the second strcmp()
be just strstr() ?
+ break;
+ }
+ if (clk-list ==v4l2_clk)
+ clk = ERR_PTR(-ENODEV);
+ }
+ mutex_unlock(clk_lock);
+
+ if (!IS_ERR(clk)
+ !try_module_get(clk-ops-owner))
+ clk = ERR_PTR(-ENODEV);
+
+ return clk;
+}
+EXPORT_SYMBOL(v4l2_clk_get);
+void v4l2_clk_put(struct v4l2_clk *clk)
+{
+ if (!IS_ERR(clk))
+ module_put(clk-ops-owner);
+}
+EXPORT_SYMBOL(v4l2_clk_put);
+
+int v4l2_clk_enable(struct v4l2_clk *clk)
+{
+ if (!clk-ops-enable)
+ return -ENOSYS;
+ return clk-ops-enable(clk);
+}
+EXPORT_SYMBOL(v4l2_clk_enable);
+
+void v4l2_clk_disable(struct v4l2_clk *clk)
+{
+ if (clk-ops-disable)
+ clk-ops-disable(clk);
+}
+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,
+
[PATCH 1/2] media: V4L2: add temporary clock helpers
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.liakhovet...@gmx.de
---
drivers/media/v4l2-core/Makefile |2 +-
drivers/media/v4l2-core/v4l2-clk.c | 126
include/media/v4l2-clk.h | 48 ++
3 files changed, 175 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 00f64d6..cb5fede 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 000..7d457e4
--- /dev/null
+++ b/drivers/media/v4l2-core/v4l2-clk.c
@@ -0,0 +1,126 @@
+/*
+ * V4L2 clock service
+ *
+ * Copyright (C) 2012, Guennadi Liakhovetski g.liakhovet...@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/errno.h
+#include linux/list.h
+#include linux/module.h
+#include linux/mutex.h
+#include linux/string.h
+
+#include media/v4l2-clk.h
+#include media/v4l2-subdev.h
+
+static DEFINE_MUTEX(clk_lock);
+static LIST_HEAD(v4l2_clk);
+
+struct v4l2_clk *v4l2_clk_get(struct v4l2_subdev *sd, const char *id)
+{
+ struct v4l2_clk *clk = NULL;
+
+ mutex_lock(clk_lock);
+ if (!id) {
+ if (list_is_singular(v4l2_clk)) {
+ clk = list_entry(v4l2_clk, struct v4l2_clk, list);
+ if (!strstr(sd-name, clk-dev_id))
+ clk = ERR_PTR(-ENODEV);
+ } else {
+ clk = ERR_PTR(-EINVAL);
+ }
+ } else {
+ list_for_each_entry(clk, v4l2_clk, list) {
+ if (!strcmp(id, clk-id)
+ !strcmp(sd-name, clk-dev_id))
+ break;
+ }
+ if (clk-list == v4l2_clk)
+ clk = ERR_PTR(-ENODEV);
+ }
+ mutex_unlock(clk_lock);
+
+ if (!IS_ERR(clk)
+ !try_module_get(clk-ops-owner))
+ clk = ERR_PTR(-ENODEV);
+
+ return clk;
+}
+EXPORT_SYMBOL(v4l2_clk_get);
+
+void v4l2_clk_put(struct v4l2_clk *clk)
+{
+ if (!IS_ERR(clk))
+ module_put(clk-ops-owner);
+}
+EXPORT_SYMBOL(v4l2_clk_put);
+
+int v4l2_clk_enable(struct v4l2_clk *clk)
+{
+ if (!clk-ops-enable)
+ return -ENOSYS;
+ return clk-ops-enable(clk);
+}
+EXPORT_SYMBOL(v4l2_clk_enable);
+
+void v4l2_clk_disable(struct v4l2_clk *clk)
+{
+ if (clk-ops-disable)
+ clk-ops-disable(clk);
+}
+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_name,
+ const char *name)
+{
+ struct v4l2_clk *clk;
+
+ if (!ops || !ops-owner || (!list_empty(v4l2_clk) !name))
+ return ERR_PTR(-EINVAL);
+
+ clk = kzalloc(sizeof(struct v4l2_clk), GFP_KERNEL);
+ if (!clk)
+ return ERR_PTR(-ENOMEM);
+
+ clk-ops = ops;
+ clk-id = name;
+ clk-dev_id = dev_name;
+
+ mutex_lock(clk_lock);
+ list_add_tail(clk-list, v4l2_clk);
+ mutex_unlock(clk_lock);
+
+ return clk;
+}
+EXPORT_SYMBOL(v4l2_clk_register);
+
+void v4l2_clk_unregister(struct v4l2_clk *clk)
+{
+ mutex_lock(clk_lock);
+ list_del(clk-list);
+
