* [PATCH v3 0/6] V4L2 asynchronous probing + soc-camera example
@ 2012-12-26 17:49 Guennadi Liakhovetski
2012-12-26 17:49 ` [PATCH 1/6] media: V4L2: support asynchronous subdevice registration Guennadi Liakhovetski
` (5 more replies)
0 siblings, 6 replies; 31+ messages in thread
From: Guennadi Liakhovetski @ 2012-12-26 17:49 UTC (permalink / raw)
To: linux-media
Cc: Laurent Pinchart, Hans Verkuil, Sylwester Nawrocki,
Sylwester Nawrocki, linux-sh, Magnus Damm, Sakari Ailus
This is v3 (roughly) of the V4L2 asynchronous probing patch plus an
example soc-camera framework and 1 host, 1 sensor and 1 board conversion
patch set. Logically, this is based on top of my recent patch series
http://thread.gmane.org/gmane.linux.drivers.video-input-infrastructure/58524
and the last version of the v4l2-clock patch. If desired, a git branch can
be provided.
Thanks
Guennadi
Guennadi Liakhovetski (6):
media: V4L2: support asynchronous subdevice registration
media: soc-camera: switch I2C subdevice drivers to use v4l2-clk
soc-camera: add V4L2-async support
sh_mobile_ceu_camera: add asynchronous subdevice probing support
imx074: support asynchronous probing
ARM: shmobile: convert ap4evb to asynchronously register camera
subdevices
arch/arm/mach-shmobile/board-ap4evb.c | 103 ++--
arch/arm/mach-shmobile/clock-sh7372.c | 1 +
drivers/media/i2c/soc_camera/imx074.c | 35 +-
drivers/media/i2c/soc_camera/mt9m001.c | 17 +-
drivers/media/i2c/soc_camera/mt9m111.c | 20 +-
drivers/media/i2c/soc_camera/mt9t031.c | 19 +-
drivers/media/i2c/soc_camera/mt9t112.c | 19 +-
drivers/media/i2c/soc_camera/mt9v022.c | 17 +-
drivers/media/i2c/soc_camera/ov2640.c | 19 +-
drivers/media/i2c/soc_camera/ov5642.c | 20 +-
drivers/media/i2c/soc_camera/ov6650.c | 17 +-
drivers/media/i2c/soc_camera/ov772x.c | 15 +-
drivers/media/i2c/soc_camera/ov9640.c | 17 +-
drivers/media/i2c/soc_camera/ov9640.h | 1 +
drivers/media/i2c/soc_camera/ov9740.c | 18 +-
drivers/media/i2c/soc_camera/rj54n1cb0c.c | 17 +-
drivers/media/i2c/soc_camera/tw9910.c | 18 +-
.../platform/soc_camera/sh_mobile_ceu_camera.c | 135 +++-
drivers/media/platform/soc_camera/sh_mobile_csi2.c | 164 +++--
drivers/media/platform/soc_camera/soc_camera.c | 681 ++++++++++++++++----
.../platform/soc_camera/soc_camera_platform.c | 2 +-
drivers/media/v4l2-core/Makefile | 3 +-
drivers/media/v4l2-core/v4l2-async.c | 284 ++++++++
drivers/media/v4l2-core/v4l2-device.c | 2 +
include/media/sh_mobile_ceu.h | 2 +
include/media/sh_mobile_csi2.h | 2 +-
include/media/soc_camera.h | 35 +-
include/media/v4l2-async.h | 113 ++++
28 files changed, 1489 insertions(+), 307 deletions(-)
create mode 100644 drivers/media/v4l2-core/v4l2-async.c
create mode 100644 include/media/v4l2-async.h
--
1.7.2.5
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH 1/6] media: V4L2: support asynchronous subdevice registration
2012-12-26 17:49 [PATCH v3 0/6] V4L2 asynchronous probing + soc-camera example Guennadi Liakhovetski
@ 2012-12-26 17:49 ` Guennadi Liakhovetski
2013-01-07 10:23 ` [PATCH 1/6 v4] " Guennadi Liakhovetski
2012-12-26 17:49 ` [PATCH 2/6] media: soc-camera: switch I2C subdevice drivers to use v4l2-clk Guennadi Liakhovetski
` (4 subsequent siblings)
5 siblings, 1 reply; 31+ messages in thread
From: Guennadi Liakhovetski @ 2012-12-26 17:49 UTC (permalink / raw)
To: linux-media
Cc: Laurent Pinchart, Hans Verkuil, Sylwester Nawrocki,
Sylwester Nawrocki, linux-sh, Magnus Damm, Sakari Ailus
Currently bridge device drivers register devices for all subdevices
synchronously, tupically, during their probing. E.g. if an I2C CMOS sensor
is attached to a video bridge device, the bridge driver will create an I2C
device and wait for the respective I2C driver to probe. This makes linking
of devices straight forward, but this approach cannot be used with
intrinsically asynchronous and unordered device registration systems like
the Flattened Device Tree. To support such systems this patch adds an
asynchronous subdevice registration framework to V4L2. To use it bridge
drivers register lists of subdevice descriptors and notifier callbacks,
subdevice drivers register subdevices, that they are about to probe or
have successfully probed. The subsystem matches subdevices against
hardware descriptors and calls bridge driver callbacks when matches are
found. Another callback is called, when the subdevice is unregistered.
Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
---
v3: identical to v2, only a kfree() is added, fixing a memory leak.
drivers/media/v4l2-core/Makefile | 3 +-
drivers/media/v4l2-core/v4l2-async.c | 284 +++++++++++++++++++++++++++++++++
drivers/media/v4l2-core/v4l2-device.c | 2 +
include/media/v4l2-async.h | 113 +++++++++++++
4 files changed, 401 insertions(+), 1 deletions(-)
create mode 100644 drivers/media/v4l2-core/v4l2-async.c
create mode 100644 include/media/v4l2-async.h
diff --git a/drivers/media/v4l2-core/Makefile b/drivers/media/v4l2-core/Makefile
index d065c01..b667ced 100644
--- a/drivers/media/v4l2-core/Makefile
+++ b/drivers/media/v4l2-core/Makefile
@@ -5,7 +5,8 @@
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-clk.o
+ v4l2-event.o v4l2-ctrls.o v4l2-subdev.o v4l2-clk.o \
+ v4l2-async.o
ifeq ($(CONFIG_COMPAT),y)
videodev-objs += v4l2-compat-ioctl32.o
endif
diff --git a/drivers/media/v4l2-core/v4l2-async.c b/drivers/media/v4l2-core/v4l2-async.c
new file mode 100644
index 0000000..5e6c2f5
--- /dev/null
+++ b/drivers/media/v4l2-core/v4l2-async.c
@@ -0,0 +1,284 @@
+/*
+ * V4L2 asynchronous subdevice registration API
+ *
+ * Copyright (C) 2012, 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/device.h>
+#include <linux/err.h>
+#include <linux/i2c.h>
+#include <linux/list.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/notifier.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+#include <linux/types.h>
+
+#include <media/v4l2-async.h>
+#include <media/v4l2-device.h>
+#include <media/v4l2-subdev.h>
+
+static bool match_i2c(struct device *dev, struct v4l2_async_hw_device *hw_dev)
+{
+ struct i2c_client *client = to_i2c_client(dev);
+ return hw_dev->bus_type = V4L2_ASYNC_BUS_I2C &&
+ hw_dev->match.i2c.adapter_id = client->adapter->nr &&
+ hw_dev->match.i2c.address = client->addr;
+}
+
+static bool match_platform(struct device *dev, struct v4l2_async_hw_device *hw_dev)
+{
+ return hw_dev->bus_type = V4L2_ASYNC_BUS_PLATFORM &&
+ !strcmp(hw_dev->match.platform.name, dev_name(dev));
+}
+
+static LIST_HEAD(subdev_list);
+static LIST_HEAD(notifier_list);
+static DEFINE_MUTEX(list_lock);
+
+static struct v4l2_async_subdev *v4l2_async_belongs(struct v4l2_async_notifier *notifier,
+ struct v4l2_async_subdev_list *asdl)
+{
+ struct v4l2_async_subdev *asd = NULL;
+ bool (*match)(struct device *,
+ struct v4l2_async_hw_device *);
+
+ list_for_each_entry (asd, ¬ifier->waiting, list) {
+ struct v4l2_async_hw_device *hw = &asd->hw;
+ switch (hw->bus_type) {
+ case V4L2_ASYNC_BUS_SPECIAL:
+ match = hw->match.special.match;
+ if (!match)
+ /* Match always */
+ return asd;
+ break;
+ case V4L2_ASYNC_BUS_PLATFORM:
+ match = match_platform;
+ break;
+ case V4L2_ASYNC_BUS_I2C:
+ match = match_i2c;
+ break;
+ default:
+ /* Oops */
+ match = NULL;
+ dev_err(notifier->v4l2_dev ? notifier->v4l2_dev->dev : NULL,
+ "Invalid bus-type %u on %p\n", hw->bus_type, asd);
+ }
+
+ if (match && match(asdl->dev, hw))
+ break;
+ }
+
+ return asd;
+}
+
+static int v4l2_async_test_notify(struct v4l2_async_notifier *notifier,
+ struct v4l2_async_subdev_list *asdl)
+{
+ struct v4l2_async_subdev *asd = v4l2_async_belongs(notifier, asdl);
+ if (asd) {
+ int ret;
+ /* Remove from the waiting list */
+ list_del(&asd->list);
+ asdl->asd = asd;
+ asdl->notifier = notifier;
+
+ if (notifier->bound) {
+ ret = notifier->bound(notifier, asdl);
+ if (ret < 0)
+ return ret;
+ }
+ /* Move from the global subdevice list to notifier's done */
+ list_move(&asdl->list, ¬ifier->done);
+
+ ret = v4l2_device_register_subdev(notifier->v4l2_dev,
+ asdl->subdev);
+ if (ret < 0) {
+ if (notifier->unbind)
+ notifier->unbind(notifier, asdl);
+ return ret;
+ }
+
+ if (list_empty(¬ifier->waiting) && notifier->complete)
+ return notifier->complete(notifier);
+
+ return 0;
+ }
+
+ return -EPROBE_DEFER;
+}
+
+static struct device *v4l2_async_unbind(struct v4l2_async_subdev_list *asdl)
+{
+ struct device *dev = asdl->dev;
+ v4l2_device_unregister_subdev(asdl->subdev);
+ /* Subdevice driver will reprobe and put asdl back onto the list */
+ list_del(&asdl->list);
+ asdl->asd = NULL;
+ /* If we handled USB devices, we'd have to lock the parent too */
+ device_release_driver(dev);
+ return dev;
+}
+
+int v4l2_async_notifier_register(struct v4l2_device *v4l2_dev,
+ struct v4l2_async_notifier *notifier)
+{
+ struct v4l2_async_subdev_list *asdl;
+ int i;
+
+ notifier->v4l2_dev = v4l2_dev;
+ INIT_LIST_HEAD(¬ifier->waiting);
+ INIT_LIST_HEAD(¬ifier->done);
+
+ for (i = 0; i < notifier->subdev_num; i++)
+ list_add_tail(¬ifier->subdev[i]->list, ¬ifier->waiting);
+
+ mutex_lock(&list_lock);
+
+ /* Keep also completed notifiers on the list */
+ list_add(¬ifier->list, ¬ifier_list);
+
+ list_for_each_entry(asdl, &subdev_list, list) {
+ int ret = v4l2_async_test_notify(notifier, asdl);
+ if (ret != -EPROBE_DEFER) {
+ mutex_unlock(&list_lock);
+ return ret;
+ }
+ }
+
+ mutex_unlock(&list_lock);
+
+ return 0;
+}
+EXPORT_SYMBOL(v4l2_async_notifier_register);
+
+void v4l2_async_notifier_unregister(struct v4l2_async_notifier *notifier)
+{
+ struct v4l2_async_subdev_list *asdl, *tmp;
+ int i = 0;
+ struct device **dev = kcalloc(notifier->subdev_num,
+ sizeof(*dev), GFP_KERNEL);
+ if (!dev)
+ dev_err(notifier->v4l2_dev->dev,
+ "Failed to allocate device cache!\n");
+
+ mutex_lock(&list_lock);
+
+ list_del(¬ifier->list);
+
+ list_for_each_entry_safe(asdl, tmp, ¬ifier->done, list) {
+ if (dev)
+ dev[i++] = get_device(asdl->dev);
+ v4l2_async_unbind(asdl);
+
+ if (notifier->unbind)
+ notifier->unbind(notifier, asdl);
+ }
+
+ mutex_unlock(&list_lock);
+
+ if (dev) {
+ while (i--) {
+ if (dev[i] && device_attach(dev[i]) < 0)
+ dev_err(dev[i], "Failed to re-probe to %s\n",
+ dev[i]->driver ? dev[i]->driver->name : "(none)");
+ put_device(dev[i]);
+ }
+ kfree(dev);
+ }
+ /*
+ * Don't care about the waiting list, it is initialised and populated
+ * upon notifier registration.
+ */
+}
+EXPORT_SYMBOL(v4l2_async_notifier_unregister);
+
+int v4l2_async_subdev_bind(struct v4l2_async_subdev_list *asdl)
+{
+ struct v4l2_async_notifier *notifier;
+ int ret = 0;
+
+ mutex_lock(&list_lock);
+
+ list_for_each_entry(notifier, ¬ifier_list, list) {
+ struct v4l2_async_subdev *asd = v4l2_async_belongs(notifier,
+ asdl);
+ /*
+ * Whether or not probing succeeds - this is the right hardware
+ * subdevice descriptor and we can provide it to the notifier
+ */
+ if (asd) {
+ asdl->asd = asd;
+ if (notifier->bind)
+ ret = notifier->bind(notifier, asdl);
+ break;
+ }
+ }
+
+ mutex_unlock(&list_lock);
+
+ return ret;
+}
+EXPORT_SYMBOL(v4l2_async_subdev_bind);
+
+int v4l2_async_subdev_bound(struct v4l2_async_subdev_list *asdl)
+{
+ struct v4l2_async_notifier *notifier;
+
+ mutex_lock(&list_lock);
+
+ INIT_LIST_HEAD(&asdl->list);
+
+ list_for_each_entry(notifier, ¬ifier_list, list) {
+ int ret = v4l2_async_test_notify(notifier, asdl);
+ if (ret != -EPROBE_DEFER) {
+ mutex_unlock(&list_lock);
+ return ret;
+ }
+ }
+
+ /* None matched, wait for hot-plugging */
+ list_add(&asdl->list, &subdev_list);
+
+ mutex_unlock(&list_lock);
+
+ return 0;
+}
+EXPORT_SYMBOL(v4l2_async_subdev_bound);
+
+void v4l2_async_subdev_unbind(struct v4l2_async_subdev_list *asdl)
+{
+ struct v4l2_async_notifier *notifier = asdl->notifier;
+ struct device *dev;
+
+ if (!asdl->asd)
+ return;
+
+ mutex_lock(&list_lock);
+
+ dev = asdl->dev;
+
+ list_add(&asdl->asd->list, ¬ifier->waiting);
+
+ dev = get_device(asdl->dev);
+
+ v4l2_async_unbind(asdl);
+
+ if (notifier->unbind)
+ notifier->unbind(notifier, asdl);
+
+ mutex_unlock(&list_lock);
+
+ /* Re-probe with lock released - avoid a deadlock */
+ if (dev && device_attach(dev) < 0)
+ dev_err(dev, "Failed to re-probe to %s\n",
+ dev->driver ? dev->driver->name : "(none)");
+
+ put_device(dev);
+}
+EXPORT_SYMBOL(v4l2_async_subdev_unbind);
diff --git a/drivers/media/v4l2-core/v4l2-device.c b/drivers/media/v4l2-core/v4l2-device.c
index 513969f..52faf2f 100644
--- a/drivers/media/v4l2-core/v4l2-device.c
+++ b/drivers/media/v4l2-core/v4l2-device.c
@@ -40,6 +40,8 @@ int v4l2_device_register(struct device *dev, struct v4l2_device *v4l2_dev)
mutex_init(&v4l2_dev->ioctl_lock);
v4l2_prio_init(&v4l2_dev->prio);
kref_init(&v4l2_dev->ref);
+ INIT_LIST_HEAD(&v4l2_dev->group_head);
+ mutex_init(&v4l2_dev->group_lock);
get_device(dev);
v4l2_dev->dev = dev;
if (dev = NULL) {
diff --git a/include/media/v4l2-async.h b/include/media/v4l2-async.h
new file mode 100644
index 0000000..91d436d
--- /dev/null
+++ b/include/media/v4l2-async.h
@@ -0,0 +1,113 @@
+/*
+ * V4L2 asynchronous subdevice registration API
+ *
+ * Copyright (C) 2012, 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.
+ */
+
+#ifndef V4L2_ASYNC_H
+#define V4L2_ASYNC_H
+
+#include <linux/list.h>
+#include <linux/mutex.h>
+#include <linux/notifier.h>
+
+#include <media/v4l2-subdev.h>
+
+struct device;
+struct v4l2_device;
+struct v4l2_async_notifier;
+
+enum v4l2_async_bus_type {
+ V4L2_ASYNC_BUS_SPECIAL,
+ V4L2_ASYNC_BUS_PLATFORM,
+ V4L2_ASYNC_BUS_I2C,
+};
+
+struct v4l2_async_hw_device {
+ enum v4l2_async_bus_type bus_type;
+ union {
+ struct {
+ const char *name;
+ } platform;
+ struct {
+ int adapter_id;
+ unsigned short address;
+ } i2c;
+ struct {
+ bool (*match)(struct device *,
+ struct v4l2_async_hw_device *);
+ void *priv;
+ } special;
+ } match;
+};
+
+/**
+ * struct v4l2_async_subdev - sub-device descriptor, as known to a bridge
+ * @hw: this device descriptor
+ * @list: member in a list of subdevices
+ */
+struct v4l2_async_subdev {
+ struct v4l2_async_hw_device hw;
+ struct list_head list;
+};
+
+/**
+ * v4l2_async_subdev_list - provided by subdevices
+ * @list: member in a list of subdevices
+ * @dev: hardware device
+ * @subdev: V4L2 subdevice
+ * @asd: pointer to respective struct v4l2_async_subdev
+ * @notifier: pointer to managing notifier
+ */
+struct v4l2_async_subdev_list {
+ struct list_head list;
+ struct device *dev;
+ struct v4l2_subdev *subdev;
+ struct v4l2_async_subdev *asd;
+ struct v4l2_async_notifier *notifier;
+};
+
+/**
+ * v4l2_async_notifier - provided by bridges
+ * @subdev_num: number of subdevices
+ * @subdev: array of pointers to subdevices
+ * @v4l2_dev: pointer to sruct v4l2_device
+ * @waiting: list of subdevices, waiting for their drivers
+ * @done: list of subdevices, already probed
+ * @list: member in a global list of notifiers
+ * @bind: a subdevice driver is about to probe one of your subdevices
+ * @bound: a subdevice driver has successfully probed one of your subdevices
+ * @complete: all your subdevices have been probed successfully
+ * @unbind: a subdevice is leaving
+ */
+struct v4l2_async_notifier {
+ int subdev_num;
+ struct v4l2_async_subdev **subdev;
+ struct v4l2_device *v4l2_dev;
+ struct list_head waiting;
+ struct list_head done;
+ struct list_head list;
+ int (*bind)(struct v4l2_async_notifier *notifier,
+ struct v4l2_async_subdev_list *asdl);
+ int (*bound)(struct v4l2_async_notifier *notifier,
+ struct v4l2_async_subdev_list *asdl);
+ int (*complete)(struct v4l2_async_notifier *notifier);
+ void (*unbind)(struct v4l2_async_notifier *notifier,
+ struct v4l2_async_subdev_list *asdl);
+};
+
+int v4l2_async_notifier_register(struct v4l2_device *v4l2_dev,
+ struct v4l2_async_notifier *notifier);
+void v4l2_async_notifier_unregister(struct v4l2_async_notifier *notifier);
+/*
+ * If subdevice probing fails any time after v4l2_async_subdev_bind(), no clean
+ * up must be called. This function is only a message of intention.
+ */
+int v4l2_async_subdev_bind(struct v4l2_async_subdev_list *asdl);
+int v4l2_async_subdev_bound(struct v4l2_async_subdev_list *asdl);
+void v4l2_async_subdev_unbind(struct v4l2_async_subdev_list *asdl);
+#endif
--
1.7.2.5
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [PATCH 2/6] media: soc-camera: switch I2C subdevice drivers to use v4l2-clk
2012-12-26 17:49 [PATCH v3 0/6] V4L2 asynchronous probing + soc-camera example Guennadi Liakhovetski
2012-12-26 17:49 ` [PATCH 1/6] media: V4L2: support asynchronous subdevice registration Guennadi Liakhovetski
@ 2012-12-26 17:49 ` Guennadi Liakhovetski
2013-01-08 8:02 ` Laurent Pinchart
2012-12-26 17:49 ` [PATCH 3/6] soc-camera: add V4L2-async support Guennadi Liakhovetski
` (3 subsequent siblings)
5 siblings, 1 reply; 31+ messages in thread
From: Guennadi Liakhovetski @ 2012-12-26 17:49 UTC (permalink / raw)
To: linux-media
Cc: Laurent Pinchart, Hans Verkuil, Sylwester Nawrocki,
Sylwester Nawrocki, linux-sh, Magnus Damm, Sakari Ailus
Instead of centrally enabling and disabling subdevice master clocks in
soc-camera core, let subdevice drivers do that themselves, using the
V4L2 clock API and soc-camera convenience wrappers.
Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
---
drivers/media/i2c/soc_camera/imx074.c | 18 ++-
drivers/media/i2c/soc_camera/mt9m001.c | 17 ++-
drivers/media/i2c/soc_camera/mt9m111.c | 20 ++-
drivers/media/i2c/soc_camera/mt9t031.c | 19 ++-
drivers/media/i2c/soc_camera/mt9t112.c | 19 ++-
drivers/media/i2c/soc_camera/mt9v022.c | 17 ++-
drivers/media/i2c/soc_camera/ov2640.c | 19 ++-
drivers/media/i2c/soc_camera/ov5642.c | 20 ++-
drivers/media/i2c/soc_camera/ov6650.c | 17 ++-
drivers/media/i2c/soc_camera/ov772x.c | 15 ++-
drivers/media/i2c/soc_camera/ov9640.c | 17 ++-
drivers/media/i2c/soc_camera/ov9640.h | 1 +
drivers/media/i2c/soc_camera/ov9740.c | 18 ++-
drivers/media/i2c/soc_camera/rj54n1cb0c.c | 17 ++-
drivers/media/i2c/soc_camera/tw9910.c | 18 ++-
drivers/media/platform/soc_camera/soc_camera.c | 173 +++++++++++++++-----
.../platform/soc_camera/soc_camera_platform.c | 2 +-
include/media/soc_camera.h | 13 +-
18 files changed, 356 insertions(+), 84 deletions(-)
diff --git a/drivers/media/i2c/soc_camera/imx074.c b/drivers/media/i2c/soc_camera/imx074.c
index a2a5cbb..cee5345 100644
--- a/drivers/media/i2c/soc_camera/imx074.c
+++ b/drivers/media/i2c/soc_camera/imx074.c
@@ -18,6 +18,7 @@
#include <linux/module.h>
#include <media/soc_camera.h>
+#include <media/v4l2-clk.h>
#include <media/v4l2-subdev.h>
#include <media/v4l2-chip-ident.h>
@@ -77,6 +78,7 @@ struct imx074_datafmt {
struct imx074 {
struct v4l2_subdev subdev;
const struct imx074_datafmt *fmt;
+ struct v4l2_clk *clk;
};
static const struct imx074_datafmt imx074_colour_fmts[] = {
@@ -272,8 +274,9 @@ static int imx074_s_power(struct v4l2_subdev *sd, int on)
{
struct i2c_client *client = v4l2_get_subdevdata(sd);
struct soc_camera_subdev_desc *ssdd = soc_camera_i2c_to_desc(client);
+ struct imx074 *priv = to_imx074(client);
- return soc_camera_set_power(&client->dev, ssdd, on);
+ return soc_camera_set_power(&client->dev, ssdd, priv->clk, on);
}
static int imx074_g_mbus_config(struct v4l2_subdev *sd,
@@ -431,6 +434,7 @@ static int imx074_probe(struct i2c_client *client,
struct imx074 *priv;
struct i2c_adapter *adapter = to_i2c_adapter(client->dev.parent);
struct soc_camera_subdev_desc *ssdd = soc_camera_i2c_to_desc(client);
+ int ret;
if (!ssdd) {
dev_err(&client->dev, "IMX074: missing platform data!\n");
@@ -451,13 +455,23 @@ static int imx074_probe(struct i2c_client *client,
priv->fmt = &imx074_colour_fmts[0];
- return imx074_video_probe(client);
+ priv->clk = v4l2_clk_get(&priv->subdev, "mclk");
+ if (IS_ERR(priv->clk))
+ return PTR_ERR(priv->clk);
+
+ ret = imx074_video_probe(client);
+ if (ret < 0)
+ v4l2_clk_put(priv->clk);
+
+ return ret;
}
static int imx074_remove(struct i2c_client *client)
{
struct soc_camera_subdev_desc *ssdd = soc_camera_i2c_to_desc(client);
+ struct imx074 *priv = to_imx074(client);
+ v4l2_clk_put(priv->clk);
if (ssdd->free_bus)
ssdd->free_bus(ssdd);
diff --git a/drivers/media/i2c/soc_camera/mt9m001.c b/drivers/media/i2c/soc_camera/mt9m001.c
index bcdc861..21e6833 100644
--- a/drivers/media/i2c/soc_camera/mt9m001.c
+++ b/drivers/media/i2c/soc_camera/mt9m001.c
@@ -16,6 +16,7 @@
#include <media/soc_camera.h>
#include <media/soc_mediabus.h>
+#include <media/v4l2-clk.h>
#include <media/v4l2-subdev.h>
#include <media/v4l2-chip-ident.h>
#include <media/v4l2-ctrls.h>
@@ -94,6 +95,7 @@ struct mt9m001 {
struct v4l2_ctrl *exposure;
};
struct v4l2_rect rect; /* Sensor window */
+ struct v4l2_clk *clk;
const struct mt9m001_datafmt *fmt;
const struct mt9m001_datafmt *fmts;
int num_fmts;
@@ -381,8 +383,9 @@ static int mt9m001_s_power(struct v4l2_subdev *sd, int on)
{
struct i2c_client *client = v4l2_get_subdevdata(sd);
struct soc_camera_subdev_desc *ssdd = soc_camera_i2c_to_desc(client);
+ struct mt9m001 *mt9m001 = to_mt9m001(client);
- return soc_camera_set_power(&client->dev, ssdd, on);
+ return soc_camera_set_power(&client->dev, ssdd, mt9m001->clk, on);
}
static int mt9m001_g_volatile_ctrl(struct v4l2_ctrl *ctrl)
@@ -710,9 +713,18 @@ static int mt9m001_probe(struct i2c_client *client,
mt9m001->rect.width = MT9M001_MAX_WIDTH;
mt9m001->rect.height = MT9M001_MAX_HEIGHT;
+ mt9m001->clk = v4l2_clk_get(&mt9m001->subdev, "mclk");
+ if (IS_ERR(mt9m001->clk)) {
+ ret = PTR_ERR(mt9m001->clk);
+ goto eclkget;
+ }
+
ret = mt9m001_video_probe(ssdd, client);
- if (ret)
+ if (ret) {
+ v4l2_clk_put(mt9m001->clk);
+eclkget:
v4l2_ctrl_handler_free(&mt9m001->hdl);
+ }
return ret;
}
@@ -722,6 +734,7 @@ static int mt9m001_remove(struct i2c_client *client)
struct mt9m001 *mt9m001 = to_mt9m001(client);
struct soc_camera_subdev_desc *ssdd = soc_camera_i2c_to_desc(client);
+ v4l2_clk_put(mt9m001->clk);
v4l2_device_unregister_subdev(&mt9m001->subdev);
v4l2_ctrl_handler_free(&mt9m001->hdl);
mt9m001_video_remove(ssdd);
diff --git a/drivers/media/i2c/soc_camera/mt9m111.c b/drivers/media/i2c/soc_camera/mt9m111.c
index bbc4ff9..1844b4c 100644
--- a/drivers/media/i2c/soc_camera/mt9m111.c
+++ b/drivers/media/i2c/soc_camera/mt9m111.c
@@ -17,6 +17,7 @@
#include <linux/module.h>
#include <media/soc_camera.h>
+#include <media/v4l2-clk.h>
#include <media/v4l2-common.h>
#include <media/v4l2-ctrls.h>
#include <media/v4l2-chip-ident.h>
@@ -209,6 +210,7 @@ struct mt9m111 {
* from v4l2-chip-ident.h */
struct mt9m111_context *ctx;
struct v4l2_rect rect; /* cropping rectangle */
+ struct v4l2_clk *clk;
int width; /* output */
int height; /* sizes */
struct mutex power_lock; /* lock to protect power_count */
@@ -803,14 +805,14 @@ static int mt9m111_power_on(struct mt9m111 *mt9m111)
struct soc_camera_subdev_desc *ssdd = soc_camera_i2c_to_desc(client);
int ret;
- ret = soc_camera_power_on(&client->dev, ssdd);
+ ret = soc_camera_power_on(&client->dev, ssdd, mt9m111->clk);
if (ret < 0)
return ret;
ret = mt9m111_resume(mt9m111);
if (ret < 0) {
dev_err(&client->dev, "Failed to resume the sensor: %d\n", ret);
- soc_camera_power_off(&client->dev, ssdd);
+ soc_camera_power_off(&client->dev, ssdd, mt9m111->clk);
}
return ret;
@@ -822,7 +824,7 @@ static void mt9m111_power_off(struct mt9m111 *mt9m111)
struct soc_camera_subdev_desc *ssdd = soc_camera_i2c_to_desc(client);
mt9m111_suspend(mt9m111);
- soc_camera_power_off(&client->dev, ssdd);
+ soc_camera_power_off(&client->dev, ssdd, mt9m111->clk);
}
static int mt9m111_s_power(struct v4l2_subdev *sd, int on)
@@ -1001,9 +1003,18 @@ static int mt9m111_probe(struct i2c_client *client,
mt9m111->lastpage = -1;
mutex_init(&mt9m111->power_lock);
+ mt9m111->clk = v4l2_clk_get(&mt9m111->subdev, "mclk");
+ if (IS_ERR(mt9m111->clk)) {
+ ret = PTR_ERR(mt9m111->clk);
+ goto eclkget;
+ }
+
ret = mt9m111_video_probe(client);
- if (ret)
+ if (ret) {
+ v4l2_clk_put(mt9m111->clk);
+eclkget:
v4l2_ctrl_handler_free(&mt9m111->hdl);
+ }
return ret;
}
@@ -1012,6 +1023,7 @@ static int mt9m111_remove(struct i2c_client *client)
{
struct mt9m111 *mt9m111 = to_mt9m111(client);
+ v4l2_clk_put(mt9m111->clk);
v4l2_device_unregister_subdev(&mt9m111->subdev);
v4l2_ctrl_handler_free(&mt9m111->hdl);
diff --git a/drivers/media/i2c/soc_camera/mt9t031.c b/drivers/media/i2c/soc_camera/mt9t031.c
index d80d044..4ab8edb 100644
--- a/drivers/media/i2c/soc_camera/mt9t031.c
+++ b/drivers/media/i2c/soc_camera/mt9t031.c
@@ -19,6 +19,7 @@
#include <media/soc_camera.h>
#include <media/v4l2-chip-ident.h>
+#include <media/v4l2-clk.h>
#include <media/v4l2-subdev.h>
#include <media/v4l2-ctrls.h>
@@ -76,6 +77,7 @@ struct mt9t031 {
struct v4l2_ctrl *exposure;
};
struct v4l2_rect rect; /* Sensor window */
+ struct v4l2_clk *clk;
int model; /* V4L2_IDENT_MT9T031* codes from v4l2-chip-ident.h */
u16 xskip;
u16 yskip;
@@ -610,16 +612,17 @@ static int mt9t031_s_power(struct v4l2_subdev *sd, int on)
struct i2c_client *client = v4l2_get_subdevdata(sd);
struct soc_camera_subdev_desc *ssdd = soc_camera_i2c_to_desc(client);
struct video_device *vdev = soc_camera_i2c_to_vdev(client);
+ struct mt9t031 *mt9t031 = to_mt9t031(client);
int ret;
if (on) {
- ret = soc_camera_power_on(&client->dev, ssdd);
+ ret = soc_camera_power_on(&client->dev, ssdd, mt9t031->clk);
if (ret < 0)
return ret;
vdev->dev.type = &mt9t031_dev_type;
} else {
vdev->dev.type = NULL;
- soc_camera_power_off(&client->dev, ssdd);
+ soc_camera_power_off(&client->dev, ssdd, mt9t031->clk);
}
return 0;
@@ -812,9 +815,18 @@ static int mt9t031_probe(struct i2c_client *client,
mt9t031->xskip = 1;
mt9t031->yskip = 1;
+ mt9t031->clk = v4l2_clk_get(&mt9t031->subdev, "mclk");
+ if (IS_ERR(mt9t031->clk)) {
+ ret = PTR_ERR(mt9t031->clk);
+ goto eclkget;
+ }
+
ret = mt9t031_video_probe(client);
- if (ret)
+ if (ret) {
+ v4l2_clk_put(mt9t031->clk);
+eclkget:
v4l2_ctrl_handler_free(&mt9t031->hdl);
+ }
return ret;
}
@@ -823,6 +835,7 @@ static int mt9t031_remove(struct i2c_client *client)
{
struct mt9t031 *mt9t031 = to_mt9t031(client);
+ v4l2_clk_put(mt9t031->clk);
v4l2_device_unregister_subdev(&mt9t031->subdev);
v4l2_ctrl_handler_free(&mt9t031->hdl);
diff --git a/drivers/media/i2c/soc_camera/mt9t112.c b/drivers/media/i2c/soc_camera/mt9t112.c
index c75d831..1d9e228 100644
--- a/drivers/media/i2c/soc_camera/mt9t112.c
+++ b/drivers/media/i2c/soc_camera/mt9t112.c
@@ -28,6 +28,7 @@
#include <media/mt9t112.h>
#include <media/soc_camera.h>
#include <media/v4l2-chip-ident.h>
+#include <media/v4l2-clk.h>
#include <media/v4l2-common.h>
/* you can check PLL/clock info */
@@ -90,6 +91,7 @@ struct mt9t112_priv {
struct mt9t112_camera_info *info;
struct i2c_client *client;
struct v4l2_rect frame;
+ struct v4l2_clk *clk;
const struct mt9t112_format *format;
int model;
u32 flags;
@@ -780,8 +782,9 @@ static int mt9t112_s_power(struct v4l2_subdev *sd, int on)
{
struct i2c_client *client = v4l2_get_subdevdata(sd);
struct soc_camera_subdev_desc *ssdd = soc_camera_i2c_to_desc(client);
+ struct mt9t112_priv *priv = to_mt9t112(client);
- return soc_camera_set_power(&client->dev, ssdd, on);
+ return soc_camera_set_power(&client->dev, ssdd, priv->clk, on);
}
static struct v4l2_subdev_core_ops mt9t112_subdev_core_ops = {
@@ -1100,18 +1103,26 @@ static int mt9t112_probe(struct i2c_client *client,
v4l2_i2c_subdev_init(&priv->subdev, client, &mt9t112_subdev_ops);
+ priv->clk = v4l2_clk_get(&priv->subdev, "mclk");
+ if (IS_ERR(priv->clk))
+ return PTR_ERR(priv->clk);
+
ret = mt9t112_camera_probe(client);
- if (ret)
- return ret;
/* Cannot fail: using the default supported pixel code */
- mt9t112_set_params(priv, &rect, V4L2_MBUS_FMT_UYVY8_2X8);
+ if (!ret)
+ mt9t112_set_params(priv, &rect, V4L2_MBUS_FMT_UYVY8_2X8);
+ else
+ v4l2_clk_put(priv->clk);
return ret;
}
static int mt9t112_remove(struct i2c_client *client)
{
+ struct mt9t112_priv *priv = to_mt9t112(client);
+
+ v4l2_clk_put(priv->clk);
return 0;
}
diff --git a/drivers/media/i2c/soc_camera/mt9v022.c b/drivers/media/i2c/soc_camera/mt9v022.c
index a5e65d6..3a3c371 100644
--- a/drivers/media/i2c/soc_camera/mt9v022.c
+++ b/drivers/media/i2c/soc_camera/mt9v022.c
@@ -20,6 +20,7 @@
#include <media/soc_mediabus.h>
#include <media/v4l2-subdev.h>
#include <media/v4l2-chip-ident.h>
+#include <media/v4l2-clk.h>
#include <media/v4l2-ctrls.h>
/*
@@ -149,6 +150,7 @@ struct mt9v022 {
struct v4l2_ctrl *hblank;
struct v4l2_ctrl *vblank;
struct v4l2_rect rect; /* Sensor window */
+ struct v4l2_clk *clk;
const struct mt9v022_datafmt *fmt;
const struct mt9v022_datafmt *fmts;
const struct mt9v02x_register *reg;
@@ -509,8 +511,9 @@ static int mt9v022_s_power(struct v4l2_subdev *sd, int on)
{
struct i2c_client *client = v4l2_get_subdevdata(sd);
struct soc_camera_subdev_desc *ssdd = soc_camera_i2c_to_desc(client);
+ struct mt9v022 *mt9v022 = to_mt9v022(client);
- return soc_camera_set_power(&client->dev, ssdd, on);
+ return soc_camera_set_power(&client->dev, ssdd, mt9v022->clk, on);
}
static int mt9v022_g_volatile_ctrl(struct v4l2_ctrl *ctrl)
@@ -948,9 +951,18 @@ static int mt9v022_probe(struct i2c_client *client,
mt9v022->rect.width = MT9V022_MAX_WIDTH;
mt9v022->rect.height = MT9V022_MAX_HEIGHT;
+ mt9v022->clk = v4l2_clk_get(&mt9v022->subdev, "mclk");
+ if (IS_ERR(mt9v022->clk)) {
+ ret = PTR_ERR(mt9v022->clk);
+ goto eclkget;
+ }
+
ret = mt9v022_video_probe(client);
- if (ret)
+ if (ret) {
+ v4l2_clk_put(mt9v022->clk);
+eclkget:
v4l2_ctrl_handler_free(&mt9v022->hdl);
+ }
return ret;
}
@@ -960,6 +972,7 @@ static int mt9v022_remove(struct i2c_client *client)
struct mt9v022 *mt9v022 = to_mt9v022(client);
struct soc_camera_subdev_desc *ssdd = soc_camera_i2c_to_desc(client);
+ v4l2_clk_put(mt9v022->clk);
v4l2_device_unregister_subdev(&mt9v022->subdev);
if (ssdd->free_bus)
ssdd->free_bus(ssdd);
diff --git a/drivers/media/i2c/soc_camera/ov2640.c b/drivers/media/i2c/soc_camera/ov2640.c
index 0f520f6..ad3616e 100644
--- a/drivers/media/i2c/soc_camera/ov2640.c
+++ b/drivers/media/i2c/soc_camera/ov2640.c
@@ -23,6 +23,7 @@
#include <media/soc_camera.h>
#include <media/v4l2-chip-ident.h>
+#include <media/v4l2-clk.h>
#include <media/v4l2-subdev.h>
#include <media/v4l2-ctrls.h>
@@ -303,6 +304,7 @@ struct ov2640_priv {
struct v4l2_subdev subdev;
struct v4l2_ctrl_handler hdl;
enum v4l2_mbus_pixelcode cfmt_code;
+ struct v4l2_clk *clk;
const struct ov2640_win_size *win;
int model;
};
@@ -772,8 +774,9 @@ static int ov2640_s_power(struct v4l2_subdev *sd, int on)
{
struct i2c_client *client = v4l2_get_subdevdata(sd);
struct soc_camera_subdev_desc *ssdd = soc_camera_i2c_to_desc(client);
+ struct ov2640_priv *priv = to_ov2640(client);
- return soc_camera_set_power(&client->dev, ssdd, on);
+ return soc_camera_set_power(&client->dev, ssdd, priv->clk, on);
}
/* Select the nearest higher resolution for capture */
@@ -1113,11 +1116,20 @@ static int ov2640_probe(struct i2c_client *client,
if (priv->hdl.error)
return priv->hdl.error;
+ priv->clk = v4l2_clk_get(&priv->subdev, "mclk");
+ if (IS_ERR(priv->clk)) {
+ ret = PTR_ERR(priv->clk);
+ goto eclkget;
+ }
+
ret = ov2640_video_probe(client);
- if (ret)
+ if (ret) {
+ v4l2_clk_put(priv->clk);
+eclkget:
v4l2_ctrl_handler_free(&priv->hdl);
- else
+ } else {
dev_info(&adapter->dev, "OV2640 Probed\n");
+ }
return ret;
}
@@ -1126,6 +1138,7 @@ static int ov2640_remove(struct i2c_client *client)
{
struct ov2640_priv *priv = to_ov2640(client);
+ v4l2_clk_put(priv->clk);
v4l2_device_unregister_subdev(&priv->subdev);
v4l2_ctrl_handler_free(&priv->hdl);
return 0;
diff --git a/drivers/media/i2c/soc_camera/ov5642.c b/drivers/media/i2c/soc_camera/ov5642.c
index 9d53309..ee7faa3 100644
--- a/drivers/media/i2c/soc_camera/ov5642.c
+++ b/drivers/media/i2c/soc_camera/ov5642.c
@@ -25,6 +25,7 @@
#include <media/soc_camera.h>
#include <media/v4l2-chip-ident.h>
+#include <media/v4l2-clk.h>
#include <media/v4l2-subdev.h>
/* OV5642 registers */
@@ -610,6 +611,7 @@ struct ov5642 {
struct v4l2_subdev subdev;
const struct ov5642_datafmt *fmt;
struct v4l2_rect crop_rect;
+ struct v4l2_clk *clk;
/* blanking information */
int total_width;
@@ -935,12 +937,13 @@ static int ov5642_s_power(struct v4l2_subdev *sd, int on)
{
struct i2c_client *client = v4l2_get_subdevdata(sd);
struct soc_camera_subdev_desc *ssdd = soc_camera_i2c_to_desc(client);
+ struct ov5642 *priv = to_ov5642(client);
int ret;
if (!on)
- return soc_camera_power_off(&client->dev, ssdd);
+ return soc_camera_power_off(&client->dev, ssdd, priv->clk);
- ret = soc_camera_power_on(&client->dev, ssdd);
+ ret = soc_camera_power_on(&client->dev, ssdd, priv->clk);
if (ret < 0)
return ret;
@@ -1021,6 +1024,7 @@ static int ov5642_probe(struct i2c_client *client,
{
struct ov5642 *priv;
struct soc_camera_subdev_desc *ssdd = soc_camera_i2c_to_desc(client);
+ int ret;
if (!ssdd) {
dev_err(&client->dev, "OV5642: missing platform data!\n");
@@ -1042,13 +1046,23 @@ static int ov5642_probe(struct i2c_client *client,
priv->total_width = OV5642_DEFAULT_WIDTH + BLANKING_EXTRA_WIDTH;
priv->total_height = BLANKING_MIN_HEIGHT;
- return ov5642_video_probe(client);
+ priv->clk = v4l2_clk_get(&priv->subdev, "mclk");
+ if (IS_ERR(priv->clk))
+ return PTR_ERR(priv->clk);
+
+ ret = ov5642_video_probe(client);
+ if (ret < 0)
+ v4l2_clk_put(priv->clk);
+
+ return ret;
}
static int ov5642_remove(struct i2c_client *client)
{
struct soc_camera_subdev_desc *ssdd = soc_camera_i2c_to_desc(client);
+ struct ov5642 *priv = to_ov5642(client);
+ v4l2_clk_put(priv->clk);
if (ssdd->free_bus)
ssdd->free_bus(ssdd);
diff --git a/drivers/media/i2c/soc_camera/ov6650.c b/drivers/media/i2c/soc_camera/ov6650.c
index dbe4f56..a33421e 100644
--- a/drivers/media/i2c/soc_camera/ov6650.c
+++ b/drivers/media/i2c/soc_camera/ov6650.c
@@ -33,6 +33,7 @@
#include <media/soc_camera.h>
#include <media/v4l2-chip-ident.h>
+#include <media/v4l2-clk.h>
#include <media/v4l2-ctrls.h>
/* Register definitions */
@@ -196,6 +197,7 @@ struct ov6650 {
struct v4l2_ctrl *blue;
struct v4l2_ctrl *red;
};
+ struct v4l2_clk *clk;
bool half_scale; /* scale down output by 2 */
struct v4l2_rect rect; /* sensor cropping window */
unsigned long pclk_limit; /* from host */
@@ -436,8 +438,9 @@ static int ov6650_s_power(struct v4l2_subdev *sd, int on)
{
struct i2c_client *client = v4l2_get_subdevdata(sd);
struct soc_camera_subdev_desc *ssdd = soc_camera_i2c_to_desc(client);
+ struct ov6650 *priv = to_ov6650(client);
- return soc_camera_set_power(&client->dev, ssdd, on);
+ return soc_camera_set_power(&client->dev, ssdd, priv->clk, on);
}
static int ov6650_g_crop(struct v4l2_subdev *sd, struct v4l2_crop *a)
@@ -1025,9 +1028,18 @@ static int ov6650_probe(struct i2c_client *client,
priv->code = V4L2_MBUS_FMT_YUYV8_2X8;
priv->colorspace = V4L2_COLORSPACE_JPEG;
+ priv->clk = v4l2_clk_get(&priv->subdev, "mclk");
+ if (IS_ERR(priv->clk)) {
+ ret = PTR_ERR(priv->clk);
+ goto eclkget;
+ }
+
ret = ov6650_video_probe(client);
- if (ret)
+ if (ret) {
+ v4l2_clk_put(priv->clk);
+eclkget:
v4l2_ctrl_handler_free(&priv->hdl);
+ }
return ret;
}
@@ -1036,6 +1048,7 @@ static int ov6650_remove(struct i2c_client *client)
{
struct ov6650 *priv = to_ov6650(client);
+ v4l2_clk_put(priv->clk);
v4l2_device_unregister_subdev(&priv->subdev);
v4l2_ctrl_handler_free(&priv->hdl);
return 0;
diff --git a/drivers/media/i2c/soc_camera/ov772x.c b/drivers/media/i2c/soc_camera/ov772x.c
index fbeb5b2..fa1bb2a 100644
--- a/drivers/media/i2c/soc_camera/ov772x.c
+++ b/drivers/media/i2c/soc_camera/ov772x.c
@@ -26,6 +26,7 @@
#include <media/ov772x.h>
#include <media/soc_camera.h>
+#include <media/v4l2-clk.h>
#include <media/v4l2-ctrls.h>
#include <media/v4l2-chip-ident.h>
#include <media/v4l2-subdev.h>
@@ -396,6 +397,7 @@ struct ov772x_win_size {
struct ov772x_priv {
struct v4l2_subdev subdev;
struct v4l2_ctrl_handler hdl;
+ struct v4l2_clk *clk;
struct ov772x_camera_info *info;
const struct ov772x_color_format *cfmt;
const struct ov772x_win_size *win;
@@ -668,8 +670,9 @@ static int ov772x_s_power(struct v4l2_subdev *sd, int on)
{
struct i2c_client *client = v4l2_get_subdevdata(sd);
struct soc_camera_subdev_desc *ssdd = soc_camera_i2c_to_desc(client);
+ struct ov772x_priv *priv = to_ov772x(sd);
- return soc_camera_set_power(&client->dev, ssdd, on);
+ return soc_camera_set_power(&client->dev, ssdd, priv->clk, on);
}
static const struct ov772x_win_size *ov772x_select_win(u32 width, u32 height)
@@ -1088,13 +1091,22 @@ static int ov772x_probe(struct i2c_client *client,
if (priv->hdl.error)
return priv->hdl.error;
+ priv->clk = v4l2_clk_get(&priv->subdev, "mclk");
+ if (IS_ERR(priv->clk)) {
+ ret = PTR_ERR(priv->clk);
+ goto eclkget;
+ }
+
ret = ov772x_video_probe(priv);
if (ret < 0) {
+ v4l2_clk_put(priv->clk);
+eclkget:
v4l2_ctrl_handler_free(&priv->hdl);
} else {
priv->cfmt = &ov772x_cfmts[0];
priv->win = &ov772x_win_sizes[0];
}
+
return ret;
}
@@ -1102,6 +1114,7 @@ static int ov772x_remove(struct i2c_client *client)
{
struct ov772x_priv *priv = to_ov772x(i2c_get_clientdata(client));
+ v4l2_clk_put(priv->clk);
v4l2_device_unregister_subdev(&priv->subdev);
v4l2_ctrl_handler_free(&priv->hdl);
return 0;
diff --git a/drivers/media/i2c/soc_camera/ov9640.c b/drivers/media/i2c/soc_camera/ov9640.c
index 0599304..7c7b844 100644
--- a/drivers/media/i2c/soc_camera/ov9640.c
+++ b/drivers/media/i2c/soc_camera/ov9640.c
@@ -29,6 +29,7 @@
#include <media/soc_camera.h>
#include <media/v4l2-chip-ident.h>
+#include <media/v4l2-clk.h>
#include <media/v4l2-common.h>
#include <media/v4l2-ctrls.h>
@@ -337,8 +338,9 @@ static int ov9640_s_power(struct v4l2_subdev *sd, int on)
{
struct i2c_client *client = v4l2_get_subdevdata(sd);
struct soc_camera_subdev_desc *ssdd = soc_camera_i2c_to_desc(client);
+ struct ov9640_priv *priv = to_ov9640_sensor(sd);
- return soc_camera_set_power(&client->dev, ssdd, on);
+ return soc_camera_set_power(&client->dev, ssdd, priv->clk, on);
}
/* select nearest higher resolution for capture */
@@ -716,10 +718,18 @@ static int ov9640_probe(struct i2c_client *client,
if (priv->hdl.error)
return priv->hdl.error;
- ret = ov9640_video_probe(client);
+ priv->clk = v4l2_clk_get(&priv->subdev, "mclk");
+ if (IS_ERR(priv->clk)) {
+ ret = PTR_ERR(priv->clk);
+ goto eclkget;
+ }
- if (ret)
+ ret = ov9640_video_probe(client);
+ if (ret) {
+ v4l2_clk_put(priv->clk);
+eclkget:
v4l2_ctrl_handler_free(&priv->hdl);
+ }
return ret;
}
@@ -729,6 +739,7 @@ static int ov9640_remove(struct i2c_client *client)
struct v4l2_subdev *sd = i2c_get_clientdata(client);
struct ov9640_priv *priv = to_ov9640_sensor(sd);
+ v4l2_clk_put(priv->clk);
v4l2_device_unregister_subdev(&priv->subdev);
v4l2_ctrl_handler_free(&priv->hdl);
return 0;
diff --git a/drivers/media/i2c/soc_camera/ov9640.h b/drivers/media/i2c/soc_camera/ov9640.h
index 6b33a97..65d13ff 100644
--- a/drivers/media/i2c/soc_camera/ov9640.h
+++ b/drivers/media/i2c/soc_camera/ov9640.h
@@ -199,6 +199,7 @@ struct ov9640_reg {
struct ov9640_priv {
struct v4l2_subdev subdev;
struct v4l2_ctrl_handler hdl;
+ struct v4l2_clk *clk;
int model;
int revision;
diff --git a/drivers/media/i2c/soc_camera/ov9740.c b/drivers/media/i2c/soc_camera/ov9740.c
index 2f236da..953b9e2 100644
--- a/drivers/media/i2c/soc_camera/ov9740.c
+++ b/drivers/media/i2c/soc_camera/ov9740.c
@@ -18,6 +18,7 @@
#include <media/soc_camera.h>
#include <media/v4l2-chip-ident.h>
+#include <media/v4l2-clk.h>
#include <media/v4l2-ctrls.h>
#define to_ov9740(sd) container_of(sd, struct ov9740_priv, subdev)
@@ -196,6 +197,7 @@ struct ov9740_reg {
struct ov9740_priv {
struct v4l2_subdev subdev;
struct v4l2_ctrl_handler hdl;
+ struct v4l2_clk *clk;
int ident;
u16 model;
@@ -792,7 +794,7 @@ static int ov9740_s_power(struct v4l2_subdev *sd, int on)
int ret;
if (on) {
- ret = soc_camera_power_on(&client->dev, ssdd);
+ ret = soc_camera_power_on(&client->dev, ssdd, priv->clk);
if (ret < 0)
return ret;
@@ -806,7 +808,7 @@ static int ov9740_s_power(struct v4l2_subdev *sd, int on)
priv->current_enable = true;
}
- soc_camera_power_off(&client->dev, ssdd);
+ soc_camera_power_off(&client->dev, ssdd, priv->clk);
}
return 0;
@@ -975,9 +977,18 @@ static int ov9740_probe(struct i2c_client *client,
if (priv->hdl.error)
return priv->hdl.error;
+ priv->clk = v4l2_clk_get(&priv->subdev, "mclk");
+ if (IS_ERR(priv->clk)) {
+ ret = PTR_ERR(priv->clk);
+ goto eclkget;
+ }
+
ret = ov9740_video_probe(client);
- if (ret < 0)
+ if (ret < 0) {
+ v4l2_clk_put(priv->clk);
+eclkget:
v4l2_ctrl_handler_free(&priv->hdl);
+ }
return ret;
}
@@ -986,6 +997,7 @@ static int ov9740_remove(struct i2c_client *client)
{
struct ov9740_priv *priv = i2c_get_clientdata(client);
+ v4l2_clk_put(priv->clk);
v4l2_device_unregister_subdev(&priv->subdev);
v4l2_ctrl_handler_free(&priv->hdl);
return 0;
diff --git a/drivers/media/i2c/soc_camera/rj54n1cb0c.c b/drivers/media/i2c/soc_camera/rj54n1cb0c.c
index 5c92679..7b687ed 100644
--- a/drivers/media/i2c/soc_camera/rj54n1cb0c.c
+++ b/drivers/media/i2c/soc_camera/rj54n1cb0c.c
@@ -17,6 +17,7 @@
#include <media/rj54n1cb0c.h>
#include <media/soc_camera.h>
+#include <media/v4l2-clk.h>
#include <media/v4l2-subdev.h>
#include <media/v4l2-chip-ident.h>
#include <media/v4l2-ctrls.h>
@@ -151,6 +152,7 @@ struct rj54n1_clock_div {
struct rj54n1 {
struct v4l2_subdev subdev;
struct v4l2_ctrl_handler hdl;
+ struct v4l2_clk *clk;
struct rj54n1_clock_div clk_div;
const struct rj54n1_datafmt *fmt;
struct v4l2_rect rect; /* Sensor window */
@@ -1184,8 +1186,9 @@ static int rj54n1_s_power(struct v4l2_subdev *sd, int on)
{
struct i2c_client *client = v4l2_get_subdevdata(sd);
struct soc_camera_subdev_desc *ssdd = soc_camera_i2c_to_desc(client);
+ struct rj54n1 *rj54n1 = to_rj54n1(client);
- return soc_camera_set_power(&client->dev, ssdd, on);
+ return soc_camera_set_power(&client->dev, ssdd, rj54n1->clk, on);
}
static int rj54n1_s_ctrl(struct v4l2_ctrl *ctrl)
@@ -1382,9 +1385,18 @@ static int rj54n1_probe(struct i2c_client *client,
rj54n1->tgclk_mhz = (rj54n1_priv->mclk_freq / PLL_L * PLL_N) /
(clk_div.ratio_tg + 1) / (clk_div.ratio_t + 1);
+ rj54n1->clk = v4l2_clk_get(&rj54n1->subdev, "mclk");
+ if (IS_ERR(rj54n1->clk)) {
+ ret = PTR_ERR(rj54n1->clk);
+ goto eclkget;
+ }
+
ret = rj54n1_video_probe(client, rj54n1_priv);
- if (ret < 0)
+ if (ret < 0) {
+ v4l2_clk_put(rj54n1->clk);
+eclkget:
v4l2_ctrl_handler_free(&rj54n1->hdl);
+ }
return ret;
}
@@ -1394,6 +1406,7 @@ static int rj54n1_remove(struct i2c_client *client)
struct rj54n1 *rj54n1 = to_rj54n1(client);
struct soc_camera_subdev_desc *ssdd = soc_camera_i2c_to_desc(client);
+ v4l2_clk_put(rj54n1->clk);
v4l2_device_unregister_subdev(&rj54n1->subdev);
if (ssdd->free_bus)
ssdd->free_bus(ssdd);
diff --git a/drivers/media/i2c/soc_camera/tw9910.c b/drivers/media/i2c/soc_camera/tw9910.c
index 7d20746..3ca1d9e 100644
--- a/drivers/media/i2c/soc_camera/tw9910.c
+++ b/drivers/media/i2c/soc_camera/tw9910.c
@@ -28,6 +28,7 @@
#include <media/soc_camera.h>
#include <media/tw9910.h>
#include <media/v4l2-chip-ident.h>
+#include <media/v4l2-clk.h>
#include <media/v4l2-subdev.h>
#define GET_ID(val) ((val & 0xF8) >> 3)
@@ -228,6 +229,7 @@ struct tw9910_scale_ctrl {
struct tw9910_priv {
struct v4l2_subdev subdev;
+ struct v4l2_clk *clk;
struct tw9910_video_info *info;
const struct tw9910_scale_ctrl *scale;
v4l2_std_id norm;
@@ -570,8 +572,9 @@ static int tw9910_s_power(struct v4l2_subdev *sd, int on)
{
struct i2c_client *client = v4l2_get_subdevdata(sd);
struct soc_camera_subdev_desc *ssdd = soc_camera_i2c_to_desc(client);
+ struct tw9910_priv *priv = to_tw9910(client);
- return soc_camera_set_power(&client->dev, ssdd, on);
+ return soc_camera_set_power(&client->dev, ssdd, priv->clk, on);
}
static int tw9910_set_frame(struct v4l2_subdev *sd, u32 *width, u32 *height)
@@ -912,6 +915,7 @@ static int tw9910_probe(struct i2c_client *client,
struct i2c_adapter *adapter to_i2c_adapter(client->dev.parent);
struct soc_camera_subdev_desc *ssdd = soc_camera_i2c_to_desc(client);
+ int ret;
if (!ssdd || !ssdd->drv_priv) {
dev_err(&client->dev, "TW9910: missing platform data!\n");
@@ -935,11 +939,21 @@ static int tw9910_probe(struct i2c_client *client,
v4l2_i2c_subdev_init(&priv->subdev, client, &tw9910_subdev_ops);
- return tw9910_video_probe(client);
+ priv->clk = v4l2_clk_get(&priv->subdev, "mclk");
+ if (IS_ERR(priv->clk))
+ return PTR_ERR(priv->clk);
+
+ ret = tw9910_video_probe(client);
+ if (ret < 0)
+ v4l2_clk_put(priv->clk);
+
+ return ret;
}
static int tw9910_remove(struct i2c_client *client)
{
+ struct tw9910_priv *priv = to_tw9910(client);
+ v4l2_clk_put(priv->clk);
return 0;
}
diff --git a/drivers/media/platform/soc_camera/soc_camera.c b/drivers/media/platform/soc_camera/soc_camera.c
index 0b6ddff..a9e6f01 100644
--- a/drivers/media/platform/soc_camera/soc_camera.c
+++ b/drivers/media/platform/soc_camera/soc_camera.c
@@ -30,6 +30,7 @@
#include <linux/vmalloc.h>
#include <media/soc_camera.h>
+#include <media/v4l2-clk.h>
#include <media/v4l2-common.h>
#include <media/v4l2-ioctl.h>
#include <media/v4l2-dev.h>
@@ -50,13 +51,19 @@ static LIST_HEAD(hosts);
static LIST_HEAD(devices);
static DEFINE_MUTEX(list_lock); /* Protects the list of hosts */
-int soc_camera_power_on(struct device *dev, struct soc_camera_subdev_desc *ssdd)
+int soc_camera_power_on(struct device *dev, struct soc_camera_subdev_desc *ssdd,
+ struct v4l2_clk *clk)
{
- int ret = regulator_bulk_enable(ssdd->num_regulators,
+ int ret = clk ? v4l2_clk_enable(clk) : 0;
+ if (ret < 0) {
+ dev_err(dev, "Cannot enable clock\n");
+ return ret;
+ }
+ ret = regulator_bulk_enable(ssdd->num_regulators,
ssdd->regulators);
if (ret < 0) {
dev_err(dev, "Cannot enable regulators\n");
- return ret;
+ goto eregenable;;
}
if (ssdd->power) {
@@ -64,16 +71,25 @@ int soc_camera_power_on(struct device *dev, struct soc_camera_subdev_desc *ssdd)
if (ret < 0) {
dev_err(dev,
"Platform failed to power-on the camera.\n");
- regulator_bulk_disable(ssdd->num_regulators,
- ssdd->regulators);
+ goto epwron;
}
}
+ return 0;
+
+epwron:
+ regulator_bulk_disable(ssdd->num_regulators,
+ ssdd->regulators);
+eregenable:
+ if (clk)
+ v4l2_clk_disable(clk);
+
return ret;
}
EXPORT_SYMBOL(soc_camera_power_on);
-int soc_camera_power_off(struct device *dev, struct soc_camera_subdev_desc *ssdd)
+int soc_camera_power_off(struct device *dev, struct soc_camera_subdev_desc *ssdd,
+ struct v4l2_clk *clk)
{
int ret = 0;
int err;
@@ -94,28 +110,44 @@ int soc_camera_power_off(struct device *dev, struct soc_camera_subdev_desc *ssdd
ret = ret ? : err;
}
+ if (clk)
+ v4l2_clk_disable(clk);
+
return ret;
}
EXPORT_SYMBOL(soc_camera_power_off);
static int __soc_camera_power_on(struct soc_camera_device *icd)
{
+ struct soc_camera_host *ici = to_soc_camera_host(icd->parent);
struct v4l2_subdev *sd = soc_camera_to_subdev(icd);
int ret;
+ if (!icd->clk) {
+ ret = ici->ops->add(icd);
+ if (ret < 0)
+ return ret;
+ }
+
ret = v4l2_subdev_call(sd, core, s_power, 1);
- if (ret < 0 && ret != -ENOIOCTLCMD && ret != -ENODEV)
+ if (ret < 0 && ret != -ENOIOCTLCMD && ret != -ENODEV) {
+ if (!icd->clk)
+ ici->ops->remove(icd);
return ret;
+ }
return 0;
}
static int __soc_camera_power_off(struct soc_camera_device *icd)
{
+ struct soc_camera_host *ici = to_soc_camera_host(icd->parent);
struct v4l2_subdev *sd = soc_camera_to_subdev(icd);
int ret;
ret = v4l2_subdev_call(sd, core, s_power, 0);
+ if (!icd->clk)
+ ici->ops->remove(icd);
if (ret < 0 && ret != -ENOIOCTLCMD && ret != -ENODEV)
return ret;
@@ -555,12 +587,6 @@ static int soc_camera_open(struct file *file)
if (sdesc->subdev_desc.reset)
sdesc->subdev_desc.reset(icd->pdev);
- ret = ici->ops->add(icd);
- if (ret < 0) {
- dev_err(icd->pdev, "Couldn't activate the camera: %d\n", ret);
- goto eiciadd;
- }
-
ret = __soc_camera_power_on(icd);
if (ret < 0)
goto epower;
@@ -606,8 +632,6 @@ esfmt:
eresume:
__soc_camera_power_off(icd);
epower:
- ici->ops->remove(icd);
-eiciadd:
icd->use_count--;
module_put(ici->ops->owner);
emodule:
@@ -629,7 +653,6 @@ static int soc_camera_close(struct file *file)
if (ici->ops->init_videobuf2)
vb2_queue_release(&icd->vb2_vidq);
- ici->ops->remove(icd);
__soc_camera_power_off(icd);
}
@@ -1068,6 +1091,57 @@ static void scan_add_host(struct soc_camera_host *ici)
mutex_unlock(&list_lock);
}
+/*
+ * It is invalid to call v4l2_clk_enable() after a successful probing
+ * asynchronously outside of V4L2 operations, i.e. with .host_lock not held.
+ */
+static int soc_camera_clk_enable(struct v4l2_clk *clk)
+{
+ struct soc_camera_device *icd = clk->priv;
+ struct soc_camera_host *ici;
+
+ if (!icd || !icd->parent)
+ return -ENODEV;
+
+ ici = to_soc_camera_host(icd->parent);
+
+ if (!try_module_get(ici->ops->owner))
+ return -ENODEV;
+
+ /*
+ * If a different client is currently being probed, the host will tell
+ * you to go
+ */
+ return ici->ops->add(icd);
+}
+
+static void soc_camera_clk_disable(struct v4l2_clk *clk)
+{
+ struct soc_camera_device *icd = clk->priv;
+ struct soc_camera_host *ici;
+
+ if (!icd || !icd->parent)
+ return;
+
+ ici = to_soc_camera_host(icd->parent);
+
+ ici->ops->remove(icd);
+
+ module_put(ici->ops->owner);
+}
+
+/*
+ * Eventually, it would be more logical to make the respective host the clock
+ * owner, but then we would have to copy this struct for each ici. Besides, it
+ * would introduce the circular dependency problem, unless we port all client
+ * drivers to release the clock, when not in use.
+ */
+static const struct v4l2_clk_ops soc_camera_clk_ops = {
+ .owner = THIS_MODULE,
+ .enable = soc_camera_clk_enable,
+ .disable = soc_camera_clk_disable,
+};
+
#ifdef CONFIG_I2C_BOARDINFO
static int soc_camera_init_i2c(struct soc_camera_device *icd,
struct soc_camera_desc *sdesc)
@@ -1077,19 +1151,33 @@ static int soc_camera_init_i2c(struct soc_camera_device *icd,
struct soc_camera_host_desc *shd = &sdesc->host_desc;
struct i2c_adapter *adap = i2c_get_adapter(shd->i2c_adapter_id);
struct v4l2_subdev *subdev;
+ char clk_name[V4L2_SUBDEV_NAME_SIZE];
+ int ret;
if (!adap) {
dev_err(icd->pdev, "Cannot get I2C adapter #%d. No driver?\n",
shd->i2c_adapter_id);
- goto ei2cga;
+ return -ENODEV;
}
shd->board_info->platform_data = &sdesc->subdev_desc;
+ snprintf(clk_name, sizeof(clk_name), "%s %d-%04x",
+ shd->board_info->type,
+ shd->i2c_adapter_id, shd->board_info->addr);
+
+ icd->clk = v4l2_clk_register(&soc_camera_clk_ops, clk_name, "mclk", icd);
+ if (IS_ERR(icd->clk)) {
+ ret = PTR_ERR(icd->clk);
+ goto eclkreg;
+ }
+
subdev = v4l2_i2c_new_subdev_board(&ici->v4l2_dev, adap,
shd->board_info, NULL);
- if (!subdev)
+ if (!subdev) {
+ ret = -ENODEV;
goto ei2cnd;
+ }
client = v4l2_get_subdevdata(subdev);
@@ -1098,9 +1186,11 @@ static int soc_camera_init_i2c(struct soc_camera_device *icd,
return 0;
ei2cnd:
+ v4l2_clk_unregister(icd->clk);
+ icd->clk = NULL;
+eclkreg:
i2c_put_adapter(adap);
-ei2cga:
- return -ENODEV;
+ return ret;
}
static void soc_camera_free_i2c(struct soc_camera_device *icd)
@@ -1113,6 +1203,8 @@ static void soc_camera_free_i2c(struct soc_camera_device *icd)
v4l2_device_unregister_subdev(i2c_get_clientdata(client));
i2c_unregister_device(client);
i2c_put_adapter(adap);
+ v4l2_clk_unregister(icd->clk);
+ icd->clk = NULL;
}
#else
#define soc_camera_init_i2c(icd, sdesc) (-ENODEV)
@@ -1150,26 +1242,31 @@ static int soc_camera_probe(struct soc_camera_device *icd)
if (ssdd->reset)
ssdd->reset(icd->pdev);
- mutex_lock(&ici->host_lock);
- ret = ici->ops->add(icd);
- mutex_unlock(&ici->host_lock);
- if (ret < 0)
- goto eadd;
-
/* Must have icd->vdev before registering the device */
ret = video_dev_create(icd);
if (ret < 0)
goto evdc;
+ /*
+ * ..._video_start() will create a device node, video_register_device()
+ * itself is protected against concurrent open() calls, but we also have
+ * to protect our data also during client probing.
+ */
+ mutex_lock(&ici->host_lock);
+
/* Non-i2c cameras, e.g., soc_camera_platform, have no board_info */
if (shd->board_info) {
ret = soc_camera_init_i2c(icd, sdesc);
if (ret < 0)
- goto eadddev;
+ goto eadd;
} else if (!ssdd->add_device || !ssdd->del_device) {
ret = -EINVAL;
- goto eadddev;
+ goto eadd;
} else {
+ ret = ici->ops->add(icd);
+ if (ret < 0)
+ goto eadd;
+
if (shd->module_name)
ret = request_module(shd->module_name);
@@ -1205,13 +1302,6 @@ static int soc_camera_probe(struct soc_camera_device *icd)
icd->field = V4L2_FIELD_ANY;
- /*
- * ..._video_start() will create a device node, video_register_device()
- * itself is protected against concurrent open() calls, but we also have
- * to protect our data.
- */
- mutex_lock(&ici->host_lock);
-
ret = soc_camera_video_start(icd);
if (ret < 0)
goto evidstart;
@@ -1224,14 +1314,14 @@ static int soc_camera_probe(struct soc_camera_device *icd)
icd->field = mf.field;
}
- ici->ops->remove(icd);
+ if (!shd->board_info)
+ ici->ops->remove(icd);
mutex_unlock(&ici->host_lock);
return 0;
evidstart:
- mutex_unlock(&ici->host_lock);
soc_camera_free_user_formats(icd);
eiufmt:
ectrl:
@@ -1240,16 +1330,15 @@ ectrl:
} else {
ssdd->del_device(icd);
module_put(control->driver->owner);
- }
enodrv:
eadddev:
+ ici->ops->remove(icd);
+ }
+eadd:
video_device_release(icd->vdev);
icd->vdev = NULL;
-evdc:
- mutex_lock(&ici->host_lock);
- ici->ops->remove(icd);
mutex_unlock(&ici->host_lock);
-eadd:
+evdc:
v4l2_ctrl_handler_free(&icd->ctrl_handler);
return ret;
}
diff --git a/drivers/media/platform/soc_camera/soc_camera_platform.c b/drivers/media/platform/soc_camera/soc_camera_platform.c
index ce3b1d6..8830dfa 100644
--- a/drivers/media/platform/soc_camera/soc_camera_platform.c
+++ b/drivers/media/platform/soc_camera/soc_camera_platform.c
@@ -54,7 +54,7 @@ static int soc_camera_platform_s_power(struct v4l2_subdev *sd, int on)
{
struct soc_camera_platform_info *p = v4l2_get_subdevdata(sd);
- return soc_camera_set_power(p->icd->control, &p->icd->sdesc->subdev_desc, on);
+ return soc_camera_set_power(p->icd->control, &p->icd->sdesc->subdev_desc, NULL, on);
}
static struct v4l2_subdev_core_ops platform_subdev_core_ops = {
diff --git a/include/media/soc_camera.h b/include/media/soc_camera.h
index 2dc3ddd..a9888e9 100644
--- a/include/media/soc_camera.h
+++ b/include/media/soc_camera.h
@@ -49,6 +49,7 @@ struct soc_camera_device {
/* soc_camera.c private count. Only accessed with .host_lock held */
int use_count;
struct file *streamer; /* stream owner */
+ struct v4l2_clk *clk;
union {
struct videobuf_queue vb_vidq;
struct vb2_queue vb2_vidq;
@@ -316,14 +317,16 @@ static inline void soc_camera_limit_side(int *start, int *length,
unsigned long soc_camera_apply_board_flags(struct soc_camera_subdev_desc *ssdd,
const struct v4l2_mbus_config *cfg);
-int soc_camera_power_on(struct device *dev, struct soc_camera_subdev_desc *ssdd);
-int soc_camera_power_off(struct device *dev, struct soc_camera_subdev_desc *ssdd);
+int soc_camera_power_on(struct device *dev, struct soc_camera_subdev_desc *ssdd,
+ struct v4l2_clk *clk);
+int soc_camera_power_off(struct device *dev, struct soc_camera_subdev_desc *ssdd,
+ struct v4l2_clk *clk);
static inline int soc_camera_set_power(struct device *dev,
- struct soc_camera_subdev_desc *ssdd, bool on)
+ struct soc_camera_subdev_desc *ssdd, struct v4l2_clk *clk, bool on)
{
- return on ? soc_camera_power_on(dev, ssdd)
- : soc_camera_power_off(dev, ssdd);
+ return on ? soc_camera_power_on(dev, ssdd, clk)
+ : soc_camera_power_off(dev, ssdd, clk);
}
/* This is only temporary here - until v4l2-subdev begins to link to video_device */
--
1.7.2.5
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [PATCH 3/6] soc-camera: add V4L2-async support
2012-12-26 17:49 [PATCH v3 0/6] V4L2 asynchronous probing + soc-camera example Guennadi Liakhovetski
2012-12-26 17:49 ` [PATCH 1/6] media: V4L2: support asynchronous subdevice registration Guennadi Liakhovetski
2012-12-26 17:49 ` [PATCH 2/6] media: soc-camera: switch I2C subdevice drivers to use v4l2-clk Guennadi Liakhovetski
@ 2012-12-26 17:49 ` Guennadi Liakhovetski
2012-12-26 17:49 ` [PATCH 4/6] sh_mobile_ceu_camera: add asynchronous subdevice probing support Guennadi Liakhovetski
` (2 subsequent siblings)
5 siblings, 0 replies; 31+ messages in thread
From: Guennadi Liakhovetski @ 2012-12-26 17:49 UTC (permalink / raw)
To: linux-media
Cc: Laurent Pinchart, Hans Verkuil, Sylwester Nawrocki,
Sylwester Nawrocki, linux-sh, Magnus Damm, Sakari Ailus
Add support for asynchronous subdevice probing, using the v4l2-async API.
The legacy synchronous mode is still supported too, which allows to
gradually update drivers and platforms. The selected approach adds a
notifier for each struct soc_camera_device instance, i.e. for each video
device node, even when there are multiple such instances registered with a
single soc-camera host simultaneously.
Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
---
drivers/media/platform/soc_camera/soc_camera.c | 532 ++++++++++++++++++++----
include/media/soc_camera.h | 22 +-
2 files changed, 467 insertions(+), 87 deletions(-)
diff --git a/drivers/media/platform/soc_camera/soc_camera.c b/drivers/media/platform/soc_camera/soc_camera.c
index a9e6f01..b962aaa 100644
--- a/drivers/media/platform/soc_camera/soc_camera.c
+++ b/drivers/media/platform/soc_camera/soc_camera.c
@@ -21,22 +21,23 @@
#include <linux/i2c.h>
#include <linux/init.h>
#include <linux/list.h>
-#include <linux/mutex.h>
#include <linux/module.h>
+#include <linux/mutex.h>
#include <linux/platform_device.h>
+#include <linux/pm_runtime.h>
#include <linux/regulator/consumer.h>
#include <linux/slab.h>
-#include <linux/pm_runtime.h>
#include <linux/vmalloc.h>
#include <media/soc_camera.h>
+#include <media/soc_mediabus.h>
+#include <media/v4l2-async.h>
#include <media/v4l2-clk.h>
#include <media/v4l2-common.h>
#include <media/v4l2-ioctl.h>
#include <media/v4l2-dev.h>
#include <media/videobuf-core.h>
#include <media/videobuf2-core.h>
-#include <media/soc_mediabus.h>
/* Default to VGA resolution */
#define DEFAULT_WIDTH 640
@@ -47,23 +48,38 @@
(icd)->vb_vidq.streaming : \
vb2_is_streaming(&(icd)->vb2_vidq))
+#define MAP_MAX_NUM 32
+static DECLARE_BITMAP(device_map, MAP_MAX_NUM);
static LIST_HEAD(hosts);
static LIST_HEAD(devices);
-static DEFINE_MUTEX(list_lock); /* Protects the list of hosts */
+/*
+ * Protects lists and bitmaps of hosts and devices.
+ * Lock nesting: Ok to take ->host_lock under list_lock.
+ */
+static DEFINE_MUTEX(list_lock);
+
+struct soc_camera_async_client {
+ struct v4l2_async_subdev *sensor;
+ struct v4l2_async_notifier notifier;
+ struct platform_device *pdev;
+};
+
+static int soc_camera_video_start(struct soc_camera_device *icd);
+static int video_dev_create(struct soc_camera_device *icd);
int soc_camera_power_on(struct device *dev, struct soc_camera_subdev_desc *ssdd,
struct v4l2_clk *clk)
{
int ret = clk ? v4l2_clk_enable(clk) : 0;
if (ret < 0) {
- dev_err(dev, "Cannot enable clock\n");
+ dev_err(dev, "Cannot enable clock: %d\n", ret);
return ret;
}
ret = regulator_bulk_enable(ssdd->num_regulators,
ssdd->regulators);
if (ret < 0) {
dev_err(dev, "Cannot enable regulators\n");
- goto eregenable;;
+ goto eregenable;
}
if (ssdd->power) {
@@ -124,15 +140,20 @@ static int __soc_camera_power_on(struct soc_camera_device *icd)
int ret;
if (!icd->clk) {
+ mutex_lock(&ici->clk_lock);
ret = ici->ops->add(icd);
+ mutex_unlock(&ici->clk_lock);
if (ret < 0)
return ret;
}
ret = v4l2_subdev_call(sd, core, s_power, 1);
if (ret < 0 && ret != -ENOIOCTLCMD && ret != -ENODEV) {
- if (!icd->clk)
+ if (!icd->clk) {
+ mutex_lock(&ici->clk_lock);
ici->ops->remove(icd);
+ mutex_unlock(&ici->clk_lock);
+ }
return ret;
}
@@ -146,8 +167,11 @@ static int __soc_camera_power_off(struct soc_camera_device *icd)
int ret;
ret = v4l2_subdev_call(sd, core, s_power, 0);
- if (!icd->clk)
+ if (!icd->clk) {
+ mutex_lock(&ici->clk_lock);
ici->ops->remove(icd);
+ mutex_unlock(&ici->clk_lock);
+ }
if (ret < 0 && ret != -ENOIOCTLCMD && ret != -ENODEV)
return ret;
@@ -623,8 +647,8 @@ static int soc_camera_open(struct file *file)
return 0;
/*
- * First four errors are entered with the .host_lock held
- * and use_count = 1
+ * All errors are entered with the .host_lock held, first four also
+ * with use_count = 1
*/
einitvb:
esfmt:
@@ -1072,7 +1096,8 @@ static int soc_camera_s_register(struct file *file, void *fh,
}
#endif
-static int soc_camera_probe(struct soc_camera_device *icd);
+static int soc_camera_probe(struct soc_camera_host *ici,
+ struct soc_camera_device *icd);
/* So far this function cannot fail */
static void scan_add_host(struct soc_camera_host *ici)
@@ -1081,12 +1106,20 @@ static void scan_add_host(struct soc_camera_host *ici)
mutex_lock(&list_lock);
- list_for_each_entry(icd, &devices, list) {
+ list_for_each_entry(icd, &devices, list)
if (icd->iface = ici->nr) {
+ struct soc_camera_desc *sdesc = to_soc_camera_desc(icd);
+ struct soc_camera_subdev_desc *ssdd = &sdesc->subdev_desc;
+
+ /* The camera could have been already on, try to reset */
+ if (ssdd->reset)
+ ssdd->reset(icd->pdev);
+
icd->parent = ici->v4l2_dev.dev;
- soc_camera_probe(icd);
+
+ /* Ignore errors */
+ soc_camera_probe(ici, icd);
}
- }
mutex_unlock(&list_lock);
}
@@ -1099,6 +1132,7 @@ static int soc_camera_clk_enable(struct v4l2_clk *clk)
{
struct soc_camera_device *icd = clk->priv;
struct soc_camera_host *ici;
+ int ret;
if (!icd || !icd->parent)
return -ENODEV;
@@ -1112,7 +1146,10 @@ static int soc_camera_clk_enable(struct v4l2_clk *clk)
* If a different client is currently being probed, the host will tell
* you to go
*/
- return ici->ops->add(icd);
+ mutex_lock(&ici->clk_lock);
+ ret = ici->ops->add(icd);
+ mutex_unlock(&ici->clk_lock);
+ return ret;
}
static void soc_camera_clk_disable(struct v4l2_clk *clk)
@@ -1125,7 +1162,9 @@ static void soc_camera_clk_disable(struct v4l2_clk *clk)
ici = to_soc_camera_host(icd->parent);
+ mutex_lock(&ici->clk_lock);
ici->ops->remove(icd);
+ mutex_unlock(&ici->clk_lock);
module_put(ici->ops->owner);
}
@@ -1142,18 +1181,108 @@ static const struct v4l2_clk_ops soc_camera_clk_ops = {
.disable = soc_camera_clk_disable,
};
+static int soc_camera_dyn_pdev(struct soc_camera_desc *sdesc,
+ struct soc_camera_async_client *sasc)
+{
+ struct platform_device *pdev;
+ int ret, i;
+
+ mutex_lock(&list_lock);
+ i = find_first_zero_bit(device_map, MAP_MAX_NUM);
+ if (i < MAP_MAX_NUM)
+ set_bit(i, device_map);
+ mutex_unlock(&list_lock);
+ if (i >= MAP_MAX_NUM)
+ return -ENOMEM;
+
+ pdev = platform_device_alloc("soc-camera-pdrv", i);
+ if (!pdev)
+ return -ENOMEM;
+
+ ret = platform_device_add_data(pdev, sdesc, sizeof(*sdesc));
+ if (ret < 0) {
+ platform_device_put(pdev);
+ return ret;
+ }
+
+ sasc->pdev = pdev;
+
+ return 0;
+}
+
+static struct soc_camera_device *soc_camera_add_pdev(struct soc_camera_async_client *sasc)
+{
+ struct platform_device *pdev = sasc->pdev;
+ int ret;
+
+ ret = platform_device_add(pdev);
+ if (ret < 0 || !pdev->dev.driver)
+ return NULL;
+
+ return platform_get_drvdata(pdev);
+}
+
+/* Locking: called with .host_lock held */
+static int soc_camera_probe_finish(struct soc_camera_device *icd)
+{
+ struct v4l2_subdev *sd = soc_camera_to_subdev(icd);
+ struct v4l2_mbus_framefmt mf;
+ int ret;
+
+ sd->grp_id = soc_camera_grp_id(icd);
+ v4l2_set_subdev_hostdata(sd, icd);
+
+ ret = v4l2_ctrl_add_handler(&icd->ctrl_handler, sd->ctrl_handler, NULL);
+ if (ret < 0)
+ return ret;
+
+ /* At this point client .probe() should have run already */
+ ret = soc_camera_init_user_formats(icd);
+ if (ret < 0)
+ return ret;
+
+ icd->field = V4L2_FIELD_ANY;
+
+ ret = soc_camera_video_start(icd);
+ if (ret < 0)
+ goto evidstart;
+
+ /* Try to improve our guess of a reasonable window format */
+ if (!v4l2_subdev_call(sd, video, g_mbus_fmt, &mf)) {
+ icd->user_width = mf.width;
+ icd->user_height = mf.height;
+ icd->colorspace = mf.colorspace;
+ icd->field = mf.field;
+ }
+
+ return 0;
+
+evidstart:
+ soc_camera_free_user_formats(icd);
+
+ return ret;
+}
+
#ifdef CONFIG_I2C_BOARDINFO
-static int soc_camera_init_i2c(struct soc_camera_device *icd,
+static int soc_camera_i2c_init(struct soc_camera_device *icd,
struct soc_camera_desc *sdesc)
{
struct i2c_client *client;
- struct soc_camera_host *ici = to_soc_camera_host(icd->parent);
+ struct soc_camera_host *ici;
struct soc_camera_host_desc *shd = &sdesc->host_desc;
- struct i2c_adapter *adap = i2c_get_adapter(shd->i2c_adapter_id);
+ struct i2c_adapter *adap;
struct v4l2_subdev *subdev;
char clk_name[V4L2_SUBDEV_NAME_SIZE];
int ret;
+ /* First find out how we link the main client */
+ if (icd->sasc) {
+ /* Async non-OF probing handled by the subdevice list */
+ return -EPROBE_DEFER;
+ }
+
+ ici = to_soc_camera_host(icd->parent);
+ adap = i2c_get_adapter(shd->i2c_adapter_id);
if (!adap) {
dev_err(icd->pdev, "Cannot get I2C adapter #%d. No driver?\n",
shd->i2c_adapter_id);
@@ -1161,7 +1290,6 @@ static int soc_camera_init_i2c(struct soc_camera_device *icd,
}
shd->board_info->platform_data = &sdesc->subdev_desc;
-
snprintf(clk_name, sizeof(clk_name), "%s %d-%04x",
shd->board_info->type,
shd->i2c_adapter_id, shd->board_info->addr);
@@ -1187,42 +1315,242 @@ static int soc_camera_init_i2c(struct soc_camera_device *icd,
return 0;
ei2cnd:
v4l2_clk_unregister(icd->clk);
- icd->clk = NULL;
eclkreg:
+ icd->clk = NULL;
i2c_put_adapter(adap);
return ret;
}
-static void soc_camera_free_i2c(struct soc_camera_device *icd)
+static void soc_camera_i2c_free(struct soc_camera_device *icd)
{
struct i2c_client *client to_i2c_client(to_soc_camera_control(icd));
struct i2c_adapter *adap = client->adapter;
icd->control = NULL;
+ if (icd->sasc)
+ return;
v4l2_device_unregister_subdev(i2c_get_clientdata(client));
i2c_unregister_device(client);
i2c_put_adapter(adap);
v4l2_clk_unregister(icd->clk);
icd->clk = NULL;
}
+
+/*
+ * V4L2 asynchronous notifier callbacks. They are all called under a v4l2-async
+ * internal global mutex, therefore cannot race against other asynchronous
+ * events. Until notifier->complete() (soc_camera_async_complete()) is called,
+ * the video device node is not registered and no V4L fops can occur. Unloading
+ * of the host driver also calls a v4l2-async function, so also there we're
+ * protected.
+ */
+static int soc_camera_async_bind(struct v4l2_async_notifier *notifier,
+ struct v4l2_async_subdev_list *asdl)
+{
+ struct soc_camera_async_client *sasc = container_of(notifier,
+ struct soc_camera_async_client, notifier);
+ struct i2c_client *client;
+ struct soc_camera_device *icd;
+ struct soc_camera_host *ici;
+ struct soc_camera_subdev_desc *ssdd;
+ char clk_name[V4L2_SUBDEV_NAME_SIZE];
+ int ret;
+
+ /* Only interested in the sensor, run only once */
+ if (sasc->sensor != asdl->asd ||
+ asdl->asd->hw.bus_type != V4L2_ASYNC_BUS_I2C)
+ return 0;
+
+ /* Sensor probing */
+ client = to_i2c_client(asdl->dev);
+ icd = platform_get_drvdata(sasc->pdev);
+
+ if (icd->clk)
+ return 0;
+
+ ici = container_of(notifier->v4l2_dev, struct soc_camera_host, v4l2_dev);
+ ssdd = &to_soc_camera_desc(icd)->subdev_desc;
+
+ snprintf(clk_name, sizeof(clk_name), "%s %d-%04x",
+ asdl->dev->driver->name,
+ i2c_adapter_id(client->adapter), client->addr);
+
+ /*
+ * It is ok to keep the clock for the whole soc_camera_device life-time,
+ * in principle it would be more logical to register the clock on icd
+ * creation, the only problem is, that at that time we don't know the
+ * driver name yet.
+ */
+ icd->clk = v4l2_clk_register(&soc_camera_clk_ops, clk_name, "mclk", icd);
+ if (IS_ERR(icd->clk)) {
+ ret = PTR_ERR(icd->clk);
+ icd->clk = NULL;
+ return ret;
+ }
+
+ /*
+ * Only now we get subdevice-specific information like regulators,
+ * flags, callbacks, etc.
+ */
+ if (asdl->dev->platform_data)
+ memcpy(ssdd, asdl->dev->platform_data, sizeof(*ssdd));
+
+ ret = devm_regulator_bulk_get(asdl->dev, ssdd->num_regulators,
+ ssdd->regulators);
+ if (ret < 0) {
+ v4l2_clk_unregister(icd->clk);
+ icd->clk = NULL;
+ return ret;
+ }
+
+ /* The camera could have been already on, try to reset */
+ if (ssdd->reset)
+ ssdd->reset(icd->pdev);
+
+ icd->parent = ici->v4l2_dev.dev;
+ icd->control = asdl->dev;
+
+ return 0;
+}
+
+static void soc_camera_async_unbind(struct v4l2_async_notifier *notifier,
+ struct v4l2_async_subdev_list *asdl)
+{
+ struct soc_camera_async_client *sasc = container_of(notifier,
+ struct soc_camera_async_client, notifier);
+ struct soc_camera_device *icd = platform_get_drvdata(sasc->pdev);
+
+ if (icd->clk) {
+ v4l2_clk_unregister(icd->clk);
+ icd->clk = NULL;
+ }
+}
+
+static int soc_camera_async_bound(struct v4l2_async_notifier *notifier,
+ struct v4l2_async_subdev_list *asdl)
+{
+ struct soc_camera_async_client *sasc = container_of(notifier,
+ struct soc_camera_async_client, notifier);
+ struct soc_camera_device *icd = platform_get_drvdata(sasc->pdev);
+
+ if (!asdl->subdev) {
+ dev_err(icd->parent,
+ "%s(): Subdevice driver hasn't set subdev pointer!\n",
+ __func__);
+ return -EINVAL;
+ }
+
+ asdl->subdev->grp_id = soc_camera_grp_id(icd);
+ v4l2_set_subdev_hostdata(asdl->subdev, icd);
+
+ return 0;
+}
+
+static int soc_camera_async_complete(struct v4l2_async_notifier *notifier)
+{
+ struct soc_camera_async_client *sasc = container_of(notifier,
+ struct soc_camera_async_client, notifier);
+ struct soc_camera_device *icd = platform_get_drvdata(sasc->pdev);
+
+ if (to_soc_camera_control(icd)) {
+ struct soc_camera_host *ici = to_soc_camera_host(icd->parent);
+ int ret;
+
+ mutex_lock(&list_lock);
+ ret = soc_camera_probe(ici, icd);
+ mutex_unlock(&list_lock);
+ if (ret < 0)
+ return ret;
+ }
+
+ return 0;
+}
+
+static int scan_async_group(struct soc_camera_host *ici,
+ struct v4l2_async_subdev **asd, int size)
+{
+ struct soc_camera_async_subdev *sasd;
+ struct soc_camera_async_client *sasc;
+ struct soc_camera_device *icd;
+ struct soc_camera_desc sdesc = {.host_desc.bus_id = ici->nr,};
+ int ret, i;
+
+ /* First look for a sensor */
+ for (i = 0; i < size; i++) {
+ sasd = container_of(asd[i], struct soc_camera_async_subdev, asd);
+ if (sasd->role = SOCAM_SUBDEV_DATA_SOURCE)
+ break;
+ }
+
+ if (i = size || asd[i]->hw.bus_type != V4L2_ASYNC_BUS_I2C) {
+ /* All useless */
+ dev_err(ici->v4l2_dev.dev, "No I2C data source found!\n");
+ return -ENODEV;
+ }
+
+ /* Or shall this be managed by the soc-camera device? */
+ sasc = devm_kzalloc(ici->v4l2_dev.dev, sizeof(*sasc), GFP_KERNEL);
+ if (!sasc)
+ return -ENOMEM;
+
+ /* HACK: just need a != NULL */
+ sdesc.host_desc.board_info = ERR_PTR(-ENODATA);
+
+ ret = soc_camera_dyn_pdev(&sdesc, sasc);
+ if (ret < 0)
+ return ret;
+
+ sasc->sensor = &sasd->asd;
+
+ icd = soc_camera_add_pdev(sasc);
+ if (!icd) {
+ platform_device_put(sasc->pdev);
+ return -ENOMEM;
+ }
+
+ sasc->notifier.subdev = asd;
+ sasc->notifier.subdev_num = size;
+ sasc->notifier.bind = soc_camera_async_bind;
+ sasc->notifier.bound = soc_camera_async_bound;
+ sasc->notifier.unbind = soc_camera_async_unbind;
+ sasc->notifier.complete = soc_camera_async_complete;
+
+ icd->sasc = sasc;
+
+ ret = v4l2_async_notifier_register(&ici->v4l2_dev, &sasc->notifier);
+ if (ret < 0) {
+ video_device_release(icd->vdev);
+ platform_device_unregister(sasc->pdev);
+ dev_err(ici->v4l2_dev.dev, "group probe failed: %d\n", ret);
+ }
+
+ return ret;
+}
+
+static void scan_async_host(struct soc_camera_host *ici)
+{
+ struct v4l2_async_subdev **asd;
+ int j;
+
+ for (j = 0, asd = ici->asd; ici->asd_sizes[j]; j++) {
+ scan_async_group(ici, asd, ici->asd_sizes[j]);
+ asd += ici->asd_sizes[j];
+ }
+}
#else
-#define soc_camera_init_i2c(icd, sdesc) (-ENODEV)
-#define soc_camera_free_i2c(icd) do {} while (0)
+#define soc_camera_i2c_init(icd, sdesc) (-ENODEV)
+#define soc_camera_i2c_free(icd) do {} while (0)
#endif
-static int soc_camera_video_start(struct soc_camera_device *icd);
-static int video_dev_create(struct soc_camera_device *icd);
/* Called during host-driver probe */
-static int soc_camera_probe(struct soc_camera_device *icd)
+static int soc_camera_probe(struct soc_camera_host *ici,
+ struct soc_camera_device *icd)
{
- struct soc_camera_host *ici = to_soc_camera_host(icd->parent);
struct soc_camera_desc *sdesc = to_soc_camera_desc(icd);
struct soc_camera_host_desc *shd = &sdesc->host_desc;
struct soc_camera_subdev_desc *ssdd = &sdesc->subdev_desc;
struct device *control = NULL;
- struct v4l2_subdev *sd;
- struct v4l2_mbus_framefmt mf;
int ret;
dev_info(icd->pdev, "Probing %s\n", dev_name(icd->pdev));
@@ -1238,10 +1566,6 @@ static int soc_camera_probe(struct soc_camera_device *icd)
if (ret < 0)
return ret;
- /* The camera could have been already on, try to reset */
- if (ssdd->reset)
- ssdd->reset(icd->pdev);
-
/* Must have icd->vdev before registering the device */
ret = video_dev_create(icd);
if (ret < 0)
@@ -1252,18 +1576,19 @@ static int soc_camera_probe(struct soc_camera_device *icd)
* itself is protected against concurrent open() calls, but we also have
* to protect our data also during client probing.
*/
- mutex_lock(&ici->host_lock);
/* Non-i2c cameras, e.g., soc_camera_platform, have no board_info */
if (shd->board_info) {
- ret = soc_camera_init_i2c(icd, sdesc);
- if (ret < 0)
+ ret = soc_camera_i2c_init(icd, sdesc);
+ if (ret < 0 && ret != -EPROBE_DEFER)
goto eadd;
} else if (!ssdd->add_device || !ssdd->del_device) {
ret = -EINVAL;
goto eadd;
} else {
+ mutex_lock(&ici->clk_lock);
ret = ici->ops->add(icd);
+ mutex_unlock(&ici->clk_lock);
if (ret < 0)
goto eadd;
@@ -1287,89 +1612,76 @@ static int soc_camera_probe(struct soc_camera_device *icd)
}
}
- sd = soc_camera_to_subdev(icd);
- sd->grp_id = soc_camera_grp_id(icd);
- v4l2_set_subdev_hostdata(sd, icd);
-
- ret = v4l2_ctrl_add_handler(&icd->ctrl_handler, sd->ctrl_handler, NULL);
- if (ret < 0)
- goto ectrl;
-
- /* At this point client .probe() should have run already */
- ret = soc_camera_init_user_formats(icd);
- if (ret < 0)
- goto eiufmt;
-
- icd->field = V4L2_FIELD_ANY;
-
- ret = soc_camera_video_start(icd);
- if (ret < 0)
- goto evidstart;
-
- /* Try to improve our guess of a reasonable window format */
- if (!v4l2_subdev_call(sd, video, g_mbus_fmt, &mf)) {
- icd->user_width = mf.width;
- icd->user_height = mf.height;
- icd->colorspace = mf.colorspace;
- icd->field = mf.field;
- }
-
- if (!shd->board_info)
- ici->ops->remove(icd);
-
+ mutex_lock(&ici->host_lock);
+ ret = soc_camera_probe_finish(icd);
mutex_unlock(&ici->host_lock);
+ if (ret < 0)
+ goto efinish;
return 0;
-evidstart:
- soc_camera_free_user_formats(icd);
-eiufmt:
-ectrl:
+efinish:
if (shd->board_info) {
- soc_camera_free_i2c(icd);
+ soc_camera_i2c_free(icd);
} else {
ssdd->del_device(icd);
module_put(control->driver->owner);
enodrv:
eadddev:
+ mutex_lock(&ici->clk_lock);
ici->ops->remove(icd);
+ mutex_unlock(&ici->clk_lock);
}
eadd:
- video_device_release(icd->vdev);
- icd->vdev = NULL;
- mutex_unlock(&ici->host_lock);
+ if (icd->vdev) {
+ video_device_release(icd->vdev);
+ icd->vdev = NULL;
+ }
evdc:
v4l2_ctrl_handler_free(&icd->ctrl_handler);
+
return ret;
}
/*
* This is called on device_unregister, which only means we have to disconnect
- * from the host, but not remove ourselves from the device list
+ * from the host, but not remove ourselves from the device list. With
+ * asynchronous client probing this can also be called without
+ * soc_camera_probe_finish() having run. Careful with clean up.
*/
static int soc_camera_remove(struct soc_camera_device *icd)
{
struct soc_camera_desc *sdesc = to_soc_camera_desc(icd);
struct video_device *vdev = icd->vdev;
- BUG_ON(!icd->parent);
-
v4l2_ctrl_handler_free(&icd->ctrl_handler);
if (vdev) {
video_unregister_device(vdev);
icd->vdev = NULL;
}
+ if (icd->sasc)
+ v4l2_async_notifier_unregister(&icd->sasc->notifier);
+
if (sdesc->host_desc.board_info) {
- soc_camera_free_i2c(icd);
+ soc_camera_i2c_free(icd);
} else {
- struct device_driver *drv = to_soc_camera_control(icd)->driver;
+ struct device *dev = to_soc_camera_control(icd);
+ struct device_driver *drv = dev ? dev->driver : NULL;
if (drv) {
sdesc->subdev_desc.del_device(icd);
module_put(drv->owner);
}
}
- soc_camera_free_user_formats(icd);
+ if (icd->num_user_formats)
+ soc_camera_free_user_formats(icd);
+ if (icd->clk) {
+ v4l2_clk_unregister(icd->clk);
+ icd->clk = NULL;
+ }
+
+ if (icd->sasc)
+ platform_device_unregister(icd->sasc->pdev);
return 0;
}
@@ -1480,7 +1792,18 @@ int soc_camera_host_register(struct soc_camera_host *ici)
mutex_unlock(&list_lock);
mutex_init(&ici->host_lock);
- scan_add_host(ici);
+ mutex_init(&ici->clk_lock);
+
+ if (ici->asd_sizes)
+ /*
+ * No OF, host with a list of subdevices. Don't try to mix
+ * modes by initialising some groups statically and some
+ * dynamically!
+ */
+ scan_async_host(ici);
+ else
+ /* Legacy: static platform devices from board data */
+ scan_add_host(ici);
return 0;
@@ -1493,13 +1816,13 @@ EXPORT_SYMBOL(soc_camera_host_register);
/* Unregister all clients! */
void soc_camera_host_unregister(struct soc_camera_host *ici)
{
- struct soc_camera_device *icd;
+ struct soc_camera_device *icd, *tmp;
mutex_lock(&list_lock);
list_del(&ici->list);
- list_for_each_entry(icd, &devices, list)
- if (icd->iface = ici->nr && to_soc_camera_control(icd))
+ list_for_each_entry_safe(icd, tmp, &devices, list)
+ if (icd->iface = ici->nr)
soc_camera_remove(icd);
mutex_unlock(&list_lock);
@@ -1514,6 +1837,7 @@ static int soc_camera_device_register(struct soc_camera_device *icd)
struct soc_camera_device *ix;
int num = -1, i;
+ mutex_lock(&list_lock);
for (i = 0; i < 256 && num < 0; i++) {
num = i;
/* Check if this index is available on this interface */
@@ -1525,18 +1849,34 @@ static int soc_camera_device_register(struct soc_camera_device *icd)
}
}
- if (num < 0)
+ if (num < 0) {
/*
* ok, we have 256 cameras on this host...
* man, stay reasonable...
*/
+ mutex_unlock(&list_lock);
return -ENOMEM;
+ }
icd->devnum = num;
icd->use_count = 0;
icd->host_priv = NULL;
+ /*
+ * Dynamically allocated devices set the bit earlier, but it doesn't hurt setting
+ * it again
+ */
+ i = to_platform_device(icd->pdev)->id;
+ if (i < 0)
+ /* One static (legacy) soc-camera platform device */
+ i = 0;
+ if (i >= MAP_MAX_NUM) {
+ mutex_unlock(&list_lock);
+ return -EBUSY;
+ }
+ set_bit(i, device_map);
list_add_tail(&icd->list, &devices);
+ mutex_unlock(&list_lock);
return 0;
}
@@ -1636,6 +1976,12 @@ static int __devinit soc_camera_pdrv_probe(struct platform_device *pdev)
if (!icd)
return -ENOMEM;
+ /*
+ * In the asynchronous case ssdd->num_regulators = 0 yet, so, the below
+ * regulator allocation is a dummy. They will be really requested later
+ * in soc_camera_async_bind(). Also note, that in that case regulators
+ * are attached to the I2C device and not to the camera platform device.
+ */
ret = devm_regulator_bulk_get(&pdev->dev, ssdd->num_regulators,
ssdd->regulators);
if (ret < 0)
@@ -1660,11 +2006,25 @@ static int __devinit soc_camera_pdrv_probe(struct platform_device *pdev)
static int __devexit soc_camera_pdrv_remove(struct platform_device *pdev)
{
struct soc_camera_device *icd = platform_get_drvdata(pdev);
+ int i;
if (!icd)
return -EINVAL;
- list_del(&icd->list);
+ i = pdev->id;
+ if (i < 0)
+ i = 0;
+
+ /*
+ * In synchronous mode with static platform devices this is called in a
+ * loop from drivers/base/dd.c::driver_detach(), no parallel execution,
+ * no need to lock. In asynchronous case the caller -
+ * soc_camera_host_unregister() - already holds the lock
+ */
+ if (test_bit(i, device_map)) {
+ clear_bit(i, device_map);
+ list_del(&icd->list);
+ }
return 0;
}
diff --git a/include/media/soc_camera.h b/include/media/soc_camera.h
index a9888e9..23f588a 100644
--- a/include/media/soc_camera.h
+++ b/include/media/soc_camera.h
@@ -19,11 +19,13 @@
#include <linux/videodev2.h>
#include <media/videobuf-core.h>
#include <media/videobuf2-core.h>
+#include <media/v4l2-async.h>
#include <media/v4l2-ctrls.h>
#include <media/v4l2-device.h>
struct file;
struct soc_camera_desc;
+struct soc_camera_async_client;
struct soc_camera_device {
struct list_head list; /* list of all registered devices */
@@ -50,6 +52,9 @@ struct soc_camera_device {
int use_count;
struct file *streamer; /* stream owner */
struct v4l2_clk *clk;
+ /* Asynchronous subdevice management */
+ struct soc_camera_async_client *sasc;
+ /* video buffer queue */
union {
struct videobuf_queue vb_vidq;
struct vb2_queue vb2_vidq;
@@ -59,15 +64,29 @@ struct soc_camera_device {
/* Host supports programmable stride */
#define SOCAM_HOST_CAP_STRIDE (1 << 0)
+enum soc_camera_subdev_role {
+ SOCAM_SUBDEV_DATA_SOURCE = 1,
+ SOCAM_SUBDEV_DATA_SINK,
+ SOCAM_SUBDEV_DATA_PROCESSOR,
+};
+
+struct soc_camera_async_subdev {
+ struct v4l2_async_subdev asd;
+ enum soc_camera_subdev_role role;
+};
+
struct soc_camera_host {
struct v4l2_device v4l2_dev;
struct list_head list;
- struct mutex host_lock; /* Protect pipeline modifications */
+ struct mutex host_lock; /* Main synchronisation lock */
+ struct mutex clk_lock; /* Protect pipeline modifications */
unsigned char nr; /* Host number */
u32 capabilities;
void *priv;
const char *drv_name;
struct soc_camera_host_ops *ops;
+ struct v4l2_async_subdev **asd; /* Flat array, arranged in groups */
+ int *asd_sizes; /* 0-terminated array of asd group sizes */
};
struct soc_camera_host_ops {
@@ -158,6 +177,7 @@ struct soc_camera_host_desc {
};
/*
+ * Platform data for "soc-camera-pdrv"
* This MUST be kept binary-identical to struct soc_camera_link below, until
* it is completely replaced by this one, after which we can split it into its
* two components.
--
1.7.2.5
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [PATCH 4/6] sh_mobile_ceu_camera: add asynchronous subdevice probing support
2012-12-26 17:49 [PATCH v3 0/6] V4L2 asynchronous probing + soc-camera example Guennadi Liakhovetski
` (2 preceding siblings ...)
2012-12-26 17:49 ` [PATCH 3/6] soc-camera: add V4L2-async support Guennadi Liakhovetski
@ 2012-12-26 17:49 ` Guennadi Liakhovetski
2012-12-26 17:49 ` [PATCH 5/6] imx074: support asynchronous probing Guennadi Liakhovetski
2012-12-26 17:49 ` [PATCH 6/6] ARM: shmobile: convert ap4evb to asynchronously register camera subdevices Guennadi Liakhovetski
5 siblings, 0 replies; 31+ messages in thread
From: Guennadi Liakhovetski @ 2012-12-26 17:49 UTC (permalink / raw)
To: linux-media
Cc: Laurent Pinchart, Hans Verkuil, Sylwester Nawrocki,
Sylwester Nawrocki, linux-sh, Magnus Damm, Sakari Ailus
Use the v4l2-async API to support asynchronous subdevice probing,
including the CSI2 subdevice. Synchronous probing is still supported too.
Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
---
.../platform/soc_camera/sh_mobile_ceu_camera.c | 135 ++++++++++++----
drivers/media/platform/soc_camera/sh_mobile_csi2.c | 164 +++++++++++--------
include/media/sh_mobile_ceu.h | 2 +
include/media/sh_mobile_csi2.h | 2 +-
4 files changed, 198 insertions(+), 105 deletions(-)
diff --git a/drivers/media/platform/soc_camera/sh_mobile_ceu_camera.c b/drivers/media/platform/soc_camera/sh_mobile_ceu_camera.c
index ba36257..a89f05e 100644
--- a/drivers/media/platform/soc_camera/sh_mobile_ceu_camera.c
+++ b/drivers/media/platform/soc_camera/sh_mobile_ceu_camera.c
@@ -35,6 +35,7 @@
#include <linux/pm_runtime.h>
#include <linux/sched.h>
+#include <media/v4l2-async.h>
#include <media/v4l2-common.h>
#include <media/v4l2-dev.h>
#include <media/soc_camera.h>
@@ -96,6 +97,10 @@ struct sh_mobile_ceu_buffer {
struct sh_mobile_ceu_dev {
struct soc_camera_host ici;
struct soc_camera_device *icd;
+ /* Asynchronous CSI2 linking */
+ struct v4l2_async_subdev *csi2_asd;
+ struct v4l2_subdev *csi2_sd;
+ /* Synchronous probing compatibility */
struct platform_device *csi2_pdev;
unsigned int irq;
@@ -535,16 +540,29 @@ static struct v4l2_subdev *find_csi2(struct sh_mobile_ceu_dev *pcdev)
{
struct v4l2_subdev *sd;
- if (!pcdev->csi2_pdev)
- return NULL;
+ if (pcdev->csi2_sd)
+ return pcdev->csi2_sd;
- v4l2_device_for_each_subdev(sd, &pcdev->ici.v4l2_dev)
- if (&pcdev->csi2_pdev->dev = v4l2_get_subdevdata(sd))
- return sd;
+ if (pcdev->csi2_asd) {
+ char name[] = "sh-mobile-csi2";
+ v4l2_device_for_each_subdev(sd, &pcdev->ici.v4l2_dev)
+ if (!strncmp(name, sd->name, sizeof(name) - 1)) {
+ pcdev->csi2_sd = sd;
+ return sd;
+ }
+ }
return NULL;
}
+static struct v4l2_subdev *csi2_subdev(struct sh_mobile_ceu_dev *pcdev,
+ struct soc_camera_device *icd)
+{
+ struct v4l2_subdev *sd = pcdev->csi2_sd;
+
+ return sd && sd->grp_id = soc_camera_grp_id(icd) ? sd : NULL;
+}
+
/* Called with .host_lock held */
static int sh_mobile_ceu_add_device(struct soc_camera_device *icd)
{
@@ -582,8 +600,13 @@ static int sh_mobile_ceu_add_device(struct soc_camera_device *icd)
* -ENODEV is special: either csi2_sd = NULL or the CSI-2 driver
* has not found this soc-camera device among its clients
*/
- if (ret = -ENODEV && csi2_sd)
- csi2_sd->grp_id = 0;
+ if (csi2_sd) {
+ if (ret = -ENODEV)
+ csi2_sd->grp_id = 0;
+ else
+ dev_info(icd->parent, "Using CSI-2 interface\n");
+ }
+
pcdev->icd = icd;
return 0;
@@ -599,8 +622,7 @@ static void sh_mobile_ceu_remove_device(struct soc_camera_device *icd)
BUG_ON(icd != pcdev->icd);
v4l2_subdev_call(csi2_sd, core, s_power, 0);
- if (csi2_sd)
- csi2_sd->grp_id = 0;
+
/* disable capture, disable interrupts */
ceu_write(pcdev, CEIER, 0);
sh_mobile_ceu_soft_reset(pcdev);
@@ -706,7 +728,7 @@ static void sh_mobile_ceu_set_rect(struct soc_camera_device *icd)
}
/* CSI2 special configuration */
- if (pcdev->csi2_pdev) {
+ if (csi2_subdev(pcdev, icd)) {
in_width = ((in_width - 2) * 2);
left_offset *= 2;
}
@@ -763,13 +785,7 @@ static void capture_restore(struct sh_mobile_ceu_dev *pcdev, u32 capsr)
static struct v4l2_subdev *find_bus_subdev(struct sh_mobile_ceu_dev *pcdev,
struct soc_camera_device *icd)
{
- if (pcdev->csi2_pdev) {
- struct v4l2_subdev *csi2_sd = find_csi2(pcdev);
- if (csi2_sd && csi2_sd->grp_id = soc_camera_grp_id(icd))
- return csi2_sd;
- }
-
- return soc_camera_to_subdev(icd);
+ return csi2_subdev(pcdev, icd) ? : soc_camera_to_subdev(icd);
}
#define CEU_BUS_FLAGS (V4L2_MBUS_MASTER | \
@@ -873,7 +889,7 @@ static int sh_mobile_ceu_set_bus_param(struct soc_camera_device *icd)
value |= common_flags & V4L2_MBUS_VSYNC_ACTIVE_LOW ? 1 << 1 : 0;
value |= common_flags & V4L2_MBUS_HSYNC_ACTIVE_LOW ? 1 << 0 : 0;
- if (pcdev->csi2_pdev) /* CSI2 mode */
+ if (csi2_subdev(pcdev, icd)) /* CSI2 mode */
value |= 3 << 12;
else if (pcdev->is_16bit)
value |= 1 << 12;
@@ -1052,7 +1068,7 @@ static int sh_mobile_ceu_get_formats(struct soc_camera_device *icd, unsigned int
return 0;
}
- if (!pcdev->pdata || !pcdev->pdata->csi2) {
+ if (!csi2_subdev(pcdev, icd)) {
/* Are there any restrictions in the CSI-2 case? */
ret = sh_mobile_ceu_try_bus_param(icd, fmt->bits_per_sample);
if (ret < 0)
@@ -2079,7 +2095,7 @@ static int __devinit sh_mobile_ceu_probe(struct platform_device *pdev)
struct resource *res;
void __iomem *base;
unsigned int irq;
- int err = 0;
+ int err, i;
struct bus_wait wait = {
.completion = COMPLETION_INITIALIZER_ONSTACK(wait.completion),
.notifier.notifier_call = bus_notify,
@@ -2185,31 +2201,60 @@ static int __devinit sh_mobile_ceu_probe(struct platform_device *pdev)
goto exit_free_clk;
}
- err = soc_camera_host_register(&pcdev->ici);
- if (err)
- goto exit_free_ctx;
+ if (pcdev->pdata && pcdev->pdata->asd_sizes) {
+ struct v4l2_async_subdev **asd;
+ char name[] = "sh-mobile-csi2";
+ int j;
+
+ /*
+ * CSI2 interfacing: several groups can use CSI2, pick up the
+ * first one
+ */
+ asd = pcdev->pdata->asd;
+ for (j = 0; pcdev->pdata->asd_sizes[j]; j++) {
+ for (i = 0; i < pcdev->pdata->asd_sizes[j]; i++, asd++) {
+ dev_info(&pdev->dev, "%s(): subdev #%d, type %u\n",
+ __func__, i, (*asd)->hw.bus_type);
+ if ((*asd)->hw.bus_type = V4L2_ASYNC_BUS_PLATFORM &&
+ !strncmp(name, (*asd)->hw.match.platform.name,
+ sizeof(name) - 1)) {
+ pcdev->csi2_asd = *asd;
+ break;
+ }
+ }
+ if (pcdev->csi2_asd)
+ break;
+ }
- /* CSI2 interfacing */
+ pcdev->ici.asd = pcdev->pdata->asd;
+ pcdev->ici.asd_sizes = pcdev->pdata->asd_sizes;
+ }
+
+ /* Legacy CSI2 interfacing */
csi2 = pcdev->pdata ? pcdev->pdata->csi2 : NULL;
if (csi2) {
+ /*
+ * TODO: remove this once all users are converted to
+ * asynchronous CSI2 probing. If it has to be kept, csi2
+ * platform device resources have to be added, using
+ * platform_device_add_resources()
+ */
struct platform_device *csi2_pdev platform_device_alloc("sh-mobile-csi2", csi2->id);
struct sh_csi2_pdata *csi2_pdata = csi2->platform_data;
if (!csi2_pdev) {
err = -ENOMEM;
- goto exit_host_unregister;
+ goto exit_free_ctx;
}
pcdev->csi2_pdev = csi2_pdev;
- err = platform_device_add_data(csi2_pdev, csi2_pdata, sizeof(*csi2_pdata));
+ err = platform_device_add_data(csi2_pdev, csi2_pdata,
+ sizeof(*csi2_pdata));
if (err < 0)
goto exit_pdev_put;
- csi2_pdata = csi2_pdev->dev.platform_data;
- csi2_pdata->v4l2_dev = &pcdev->ici.v4l2_dev;
-
csi2_pdev->resource = csi2->resource;
csi2_pdev->num_resources = csi2->num_resources;
@@ -2251,17 +2296,36 @@ static int __devinit sh_mobile_ceu_probe(struct platform_device *pdev)
err = -ENODEV;
goto exit_pdev_unregister;
}
+
+ pcdev->csi2_sd = platform_get_drvdata(csi2_pdev);
+ }
+
+ err = soc_camera_host_register(&pcdev->ici);
+ if (err)
+ goto exit_csi2_unregister;
+
+ if (csi2) {
+ err = v4l2_device_register_subdev(&pcdev->ici.v4l2_dev,
+ pcdev->csi2_sd);
+ dev_dbg(&pdev->dev, "%s(): ret(register_subdev) = %d\n",
+ __func__, err);
+ if (err < 0)
+ goto exit_host_unregister;
}
return 0;
-exit_pdev_unregister:
- platform_device_del(pcdev->csi2_pdev);
-exit_pdev_put:
- pcdev->csi2_pdev->resource = NULL;
- platform_device_put(pcdev->csi2_pdev);
exit_host_unregister:
soc_camera_host_unregister(&pcdev->ici);
+exit_csi2_unregister:
+ if (csi2) {
+ module_put(pcdev->csi2_pdev->dev.driver->owner);
+exit_pdev_unregister:
+ platform_device_del(pcdev->csi2_pdev);
+exit_pdev_put:
+ pcdev->csi2_pdev->resource = NULL;
+ platform_device_put(pcdev->csi2_pdev);
+ }
exit_free_ctx:
vb2_dma_contig_cleanup_ctx(pcdev->alloc_ctx);
exit_free_clk:
@@ -2321,6 +2385,7 @@ MODULE_DEVICE_TABLE(of, sh_mobile_ceu_of_match);
static struct platform_driver sh_mobile_ceu_driver = {
.driver = {
.name = "sh_mobile_ceu",
+ .owner = THIS_MODULE,
.pm = &sh_mobile_ceu_dev_pm_ops,
.of_match_table = sh_mobile_ceu_of_match,
},
@@ -2346,5 +2411,5 @@ module_exit(sh_mobile_ceu_exit);
MODULE_DESCRIPTION("SuperH Mobile CEU driver");
MODULE_AUTHOR("Magnus Damm");
MODULE_LICENSE("GPL");
-MODULE_VERSION("0.0.6");
+MODULE_VERSION("0.1.0");
MODULE_ALIAS("platform:sh_mobile_ceu");
diff --git a/drivers/media/platform/soc_camera/sh_mobile_csi2.c b/drivers/media/platform/soc_camera/sh_mobile_csi2.c
index c573be7..8472343 100644
--- a/drivers/media/platform/soc_camera/sh_mobile_csi2.c
+++ b/drivers/media/platform/soc_camera/sh_mobile_csi2.c
@@ -35,14 +35,16 @@
struct sh_csi2 {
struct v4l2_subdev subdev;
- struct list_head list;
unsigned int irq;
unsigned long mipi_flags;
void __iomem *base;
struct platform_device *pdev;
struct sh_csi2_client_config *client;
+ struct v4l2_async_subdev_list asdl;
};
+static void sh_csi2_hwinit(struct sh_csi2 *priv);
+
static int sh_csi2_try_fmt(struct v4l2_subdev *sd,
struct v4l2_mbus_framefmt *mf)
{
@@ -131,10 +133,58 @@ static int sh_csi2_s_fmt(struct v4l2_subdev *sd,
static int sh_csi2_g_mbus_config(struct v4l2_subdev *sd,
struct v4l2_mbus_config *cfg)
{
- cfg->flags = V4L2_MBUS_PCLK_SAMPLE_RISING |
- V4L2_MBUS_HSYNC_ACTIVE_HIGH | V4L2_MBUS_VSYNC_ACTIVE_HIGH |
- V4L2_MBUS_MASTER | V4L2_MBUS_DATA_ACTIVE_HIGH;
- cfg->type = V4L2_MBUS_PARALLEL;
+ struct sh_csi2 *priv = container_of(sd, struct sh_csi2, subdev);
+
+ if (!priv->mipi_flags) {
+ struct soc_camera_device *icd = v4l2_get_subdev_hostdata(sd);
+ struct v4l2_subdev *client_sd = soc_camera_to_subdev(icd);
+ struct sh_csi2_pdata *pdata = priv->pdev->dev.platform_data;
+ unsigned long common_flags, csi2_flags;
+ struct v4l2_mbus_config client_cfg = {.type = V4L2_MBUS_CSI2,};
+ int ret;
+
+ /* Check if we can support this camera */
+ csi2_flags = V4L2_MBUS_CSI2_CONTINUOUS_CLOCK |
+ V4L2_MBUS_CSI2_1_LANE;
+
+ switch (pdata->type) {
+ case SH_CSI2C:
+ if (priv->client->lanes != 1)
+ csi2_flags |= V4L2_MBUS_CSI2_2_LANE;
+ break;
+ case SH_CSI2I:
+ switch (priv->client->lanes) {
+ default:
+ csi2_flags |= V4L2_MBUS_CSI2_4_LANE;
+ case 3:
+ csi2_flags |= V4L2_MBUS_CSI2_3_LANE;
+ case 2:
+ csi2_flags |= V4L2_MBUS_CSI2_2_LANE;
+ }
+ }
+
+ ret = v4l2_subdev_call(client_sd, video, g_mbus_config, &client_cfg);
+ if (ret = -ENOIOCTLCMD)
+ common_flags = csi2_flags;
+ else if (!ret)
+ common_flags = soc_mbus_config_compatible(&client_cfg,
+ csi2_flags);
+ else
+ common_flags = 0;
+
+ if (!common_flags)
+ return -EINVAL;
+
+ /* All good: camera MIPI configuration supported */
+ priv->mipi_flags = common_flags;
+ }
+
+ if (cfg) {
+ cfg->flags = V4L2_MBUS_PCLK_SAMPLE_RISING |
+ V4L2_MBUS_HSYNC_ACTIVE_HIGH | V4L2_MBUS_VSYNC_ACTIVE_HIGH |
+ V4L2_MBUS_MASTER | V4L2_MBUS_DATA_ACTIVE_HIGH;
+ cfg->type = V4L2_MBUS_PARALLEL;
+ }
return 0;
}
@@ -145,8 +195,17 @@ static int sh_csi2_s_mbus_config(struct v4l2_subdev *sd,
struct sh_csi2 *priv = container_of(sd, struct sh_csi2, subdev);
struct soc_camera_device *icd = v4l2_get_subdev_hostdata(sd);
struct v4l2_subdev *client_sd = soc_camera_to_subdev(icd);
- struct v4l2_mbus_config client_cfg = {.type = V4L2_MBUS_CSI2,
- .flags = priv->mipi_flags};
+ struct v4l2_mbus_config client_cfg = {.type = V4L2_MBUS_CSI2,};
+ int ret = sh_csi2_g_mbus_config(sd, NULL);
+
+ if (ret < 0)
+ return ret;
+
+ pm_runtime_get_sync(&priv->pdev->dev);
+
+ sh_csi2_hwinit(priv);
+
+ client_cfg.flags = priv->mipi_flags;
return v4l2_subdev_call(client_sd, video, s_mbus_config, &client_cfg);
}
@@ -201,19 +260,19 @@ static void sh_csi2_hwinit(struct sh_csi2 *priv)
static int sh_csi2_client_connect(struct sh_csi2 *priv)
{
- struct sh_csi2_pdata *pdata = priv->pdev->dev.platform_data;
- struct soc_camera_device *icd = v4l2_get_subdev_hostdata(&priv->subdev);
- struct v4l2_subdev *client_sd = soc_camera_to_subdev(icd);
struct device *dev = v4l2_get_subdevdata(&priv->subdev);
- struct v4l2_mbus_config cfg;
- unsigned long common_flags, csi2_flags;
- int i, ret;
+ struct sh_csi2_pdata *pdata = dev->platform_data;
+ struct soc_camera_device *icd = v4l2_get_subdev_hostdata(&priv->subdev);
+ int i;
if (priv->client)
return -EBUSY;
for (i = 0; i < pdata->num_clients; i++)
- if (&pdata->clients[i].pdev->dev = icd->pdev)
+ if ((pdata->clients[i].pdev &&
+ &pdata->clients[i].pdev->dev = icd->pdev) ||
+ (icd->control &&
+ strcmp(pdata->clients[i].name, dev_name(icd->control))))
break;
dev_dbg(dev, "%s(%p): found #%d\n", __func__, dev, i);
@@ -221,46 +280,8 @@ static int sh_csi2_client_connect(struct sh_csi2 *priv)
if (i = pdata->num_clients)
return -ENODEV;
- /* Check if we can support this camera */
- csi2_flags = V4L2_MBUS_CSI2_CONTINUOUS_CLOCK | V4L2_MBUS_CSI2_1_LANE;
-
- switch (pdata->type) {
- case SH_CSI2C:
- if (pdata->clients[i].lanes != 1)
- csi2_flags |= V4L2_MBUS_CSI2_2_LANE;
- break;
- case SH_CSI2I:
- switch (pdata->clients[i].lanes) {
- default:
- csi2_flags |= V4L2_MBUS_CSI2_4_LANE;
- case 3:
- csi2_flags |= V4L2_MBUS_CSI2_3_LANE;
- case 2:
- csi2_flags |= V4L2_MBUS_CSI2_2_LANE;
- }
- }
-
- cfg.type = V4L2_MBUS_CSI2;
- ret = v4l2_subdev_call(client_sd, video, g_mbus_config, &cfg);
- if (ret = -ENOIOCTLCMD)
- common_flags = csi2_flags;
- else if (!ret)
- common_flags = soc_mbus_config_compatible(&cfg,
- csi2_flags);
- else
- common_flags = 0;
-
- if (!common_flags)
- return -EINVAL;
-
- /* All good: camera MIPI configuration supported */
- priv->mipi_flags = common_flags;
priv->client = pdata->clients + i;
- pm_runtime_get_sync(dev);
-
- sh_csi2_hwinit(priv);
-
return 0;
}
@@ -303,11 +324,25 @@ static __devinit int sh_csi2_probe(struct platform_device *pdev)
/* Platform data specify the PHY, lanes, ECC, CRC */
struct sh_csi2_pdata *pdata = pdev->dev.platform_data;
+ if (!pdata)
+ return -EINVAL;
+
+ priv = devm_kzalloc(&pdev->dev, sizeof(struct sh_csi2), GFP_KERNEL);
+ if (!priv)
+ return -ENOMEM;
+
+ priv->asdl.subdev = &priv->subdev;
+ priv->asdl.dev = &pdev->dev;
+
+ ret = v4l2_async_subdev_bind(&priv->asdl);
+ if (ret < 0)
+ return ret;
+
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
/* Interrupt unused so far */
irq = platform_get_irq(pdev, 0);
- if (!res || (int)irq <= 0 || !pdata) {
+ if (!res || (int)irq <= 0) {
dev_err(&pdev->dev, "Not enough CSI2 platform resources.\n");
return -ENODEV;
}
@@ -318,10 +353,6 @@ static __devinit int sh_csi2_probe(struct platform_device *pdev)
return -EINVAL;
}
- priv = devm_kzalloc(&pdev->dev, sizeof(struct sh_csi2), GFP_KERNEL);
- if (!priv)
- return -ENOMEM;
-
priv->irq = irq;
priv->base = devm_request_and_ioremap(&pdev->dev, res);
@@ -331,35 +362,30 @@ static __devinit int sh_csi2_probe(struct platform_device *pdev)
}
priv->pdev = pdev;
- platform_set_drvdata(pdev, priv);
+ platform_set_drvdata(pdev, &priv->subdev);
v4l2_subdev_init(&priv->subdev, &sh_csi2_subdev_ops);
v4l2_set_subdevdata(&priv->subdev, &pdev->dev);
snprintf(priv->subdev.name, V4L2_SUBDEV_NAME_SIZE, "%s.mipi-csi",
- dev_name(pdata->v4l2_dev->dev));
- ret = v4l2_device_register_subdev(pdata->v4l2_dev, &priv->subdev);
- dev_dbg(&pdev->dev, "%s(%p): ret(register_subdev) = %d\n", __func__, priv, ret);
+ dev_name(&pdev->dev));
+
+ ret = v4l2_async_subdev_bound(&priv->asdl);
if (ret < 0)
- goto esdreg;
+ return ret;
pm_runtime_enable(&pdev->dev);
dev_dbg(&pdev->dev, "CSI2 probed.\n");
return 0;
-
-esdreg:
- platform_set_drvdata(pdev, NULL);
-
- return ret;
}
static __devexit int sh_csi2_remove(struct platform_device *pdev)
{
- struct sh_csi2 *priv = platform_get_drvdata(pdev);
+ struct v4l2_subdev *subdev = platform_get_drvdata(pdev);
- v4l2_device_unregister_subdev(&priv->subdev);
+ v4l2_device_unregister_subdev(subdev);
pm_runtime_disable(&pdev->dev);
platform_set_drvdata(pdev, NULL);
diff --git a/include/media/sh_mobile_ceu.h b/include/media/sh_mobile_ceu.h
index 6fdb6ad..8937241 100644
--- a/include/media/sh_mobile_ceu.h
+++ b/include/media/sh_mobile_ceu.h
@@ -22,6 +22,8 @@ struct sh_mobile_ceu_info {
int max_width;
int max_height;
struct sh_mobile_ceu_companion *csi2;
+ struct v4l2_async_subdev **asd; /* Flat array, arranged in groups */
+ int *asd_sizes; /* 0-terminated array pf asd group sizes */
};
#endif /* __ASM_SH_MOBILE_CEU_H__ */
diff --git a/include/media/sh_mobile_csi2.h b/include/media/sh_mobile_csi2.h
index c586c4f..14030db 100644
--- a/include/media/sh_mobile_csi2.h
+++ b/include/media/sh_mobile_csi2.h
@@ -33,6 +33,7 @@ struct sh_csi2_client_config {
unsigned char lanes; /* bitmask[3:0] */
unsigned char channel; /* 0..3 */
struct platform_device *pdev; /* client platform device */
+ const char *name; /* async matching: client name */
};
struct v4l2_device;
@@ -42,7 +43,6 @@ struct sh_csi2_pdata {
unsigned int flags;
struct sh_csi2_client_config *clients;
int num_clients;
- struct v4l2_device *v4l2_dev;
};
#endif
--
1.7.2.5
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [PATCH 5/6] imx074: support asynchronous probing
2012-12-26 17:49 [PATCH v3 0/6] V4L2 asynchronous probing + soc-camera example Guennadi Liakhovetski
` (3 preceding siblings ...)
2012-12-26 17:49 ` [PATCH 4/6] sh_mobile_ceu_camera: add asynchronous subdevice probing support Guennadi Liakhovetski
@ 2012-12-26 17:49 ` Guennadi Liakhovetski
2012-12-26 17:49 ` [PATCH 6/6] ARM: shmobile: convert ap4evb to asynchronously register camera subdevices Guennadi Liakhovetski
5 siblings, 0 replies; 31+ messages in thread
From: Guennadi Liakhovetski @ 2012-12-26 17:49 UTC (permalink / raw)
To: linux-media
Cc: Laurent Pinchart, Hans Verkuil, Sylwester Nawrocki,
Sylwester Nawrocki, linux-sh, Magnus Damm, Sakari Ailus
Both synchronous and asynchronous imx074 subdevice probing is supported by
this patch.
Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
---
drivers/media/i2c/soc_camera/imx074.c | 23 ++++++++++++++++++++---
1 files changed, 20 insertions(+), 3 deletions(-)
diff --git a/drivers/media/i2c/soc_camera/imx074.c b/drivers/media/i2c/soc_camera/imx074.c
index cee5345..a097dd1 100644
--- a/drivers/media/i2c/soc_camera/imx074.c
+++ b/drivers/media/i2c/soc_camera/imx074.c
@@ -18,6 +18,7 @@
#include <linux/module.h>
#include <media/soc_camera.h>
+#include <media/v4l2-async.h>
#include <media/v4l2-clk.h>
#include <media/v4l2-subdev.h>
#include <media/v4l2-chip-ident.h>
@@ -79,6 +80,7 @@ struct imx074 {
struct v4l2_subdev subdev;
const struct imx074_datafmt *fmt;
struct v4l2_clk *clk;
+ struct v4l2_async_subdev_list asdl;
};
static const struct imx074_datafmt imx074_colour_fmts[] = {
@@ -455,14 +457,27 @@ static int imx074_probe(struct i2c_client *client,
priv->fmt = &imx074_colour_fmts[0];
+ priv->asdl.subdev = &priv->subdev;
+ priv->asdl.dev = &client->dev;
+
+ ret = v4l2_async_subdev_bind(&priv->asdl);
+ if (ret < 0)
+ return ret;
+
priv->clk = v4l2_clk_get(&priv->subdev, "mclk");
- if (IS_ERR(priv->clk))
- return PTR_ERR(priv->clk);
+ if (IS_ERR(priv->clk)) {
+ dev_info(&client->dev, "Error %ld getting clock\n", PTR_ERR(priv->clk));
+ return -EPROBE_DEFER;
+ }
ret = imx074_video_probe(client);
if (ret < 0)
- v4l2_clk_put(priv->clk);
+ goto eprobe;
+
+ return v4l2_async_subdev_bound(&priv->asdl);
+eprobe:
+ v4l2_clk_put(priv->clk);
return ret;
}
@@ -471,7 +486,9 @@ static int imx074_remove(struct i2c_client *client)
struct soc_camera_subdev_desc *ssdd = soc_camera_i2c_to_desc(client);
struct imx074 *priv = to_imx074(client);
+ v4l2_async_subdev_unbind(&priv->asdl);
v4l2_clk_put(priv->clk);
+
if (ssdd->free_bus)
ssdd->free_bus(ssdd);
--
1.7.2.5
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [PATCH 6/6] ARM: shmobile: convert ap4evb to asynchronously register camera subdevices
2012-12-26 17:49 [PATCH v3 0/6] V4L2 asynchronous probing + soc-camera example Guennadi Liakhovetski
` (4 preceding siblings ...)
2012-12-26 17:49 ` [PATCH 5/6] imx074: support asynchronous probing Guennadi Liakhovetski
@ 2012-12-26 17:49 ` Guennadi Liakhovetski
2013-01-08 4:27 ` Simon Horman
5 siblings, 1 reply; 31+ messages in thread
From: Guennadi Liakhovetski @ 2012-12-26 17:49 UTC (permalink / raw)
To: linux-media
Cc: Laurent Pinchart, Hans Verkuil, Sylwester Nawrocki,
Sylwester Nawrocki, linux-sh, Magnus Damm, Sakari Ailus
Register the imx074 camera I2C and the CSI-2 platform devices directly
in board platform data instead of letting the sh_mobile_ceu_camera driver
and the soc-camera framework register them at their run-time. This uses
the V4L2 asynchronous subdevice probing capability.
Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
---
arch/arm/mach-shmobile/board-ap4evb.c | 103 +++++++++++++++++++-------------
arch/arm/mach-shmobile/clock-sh7372.c | 1 +
2 files changed, 62 insertions(+), 42 deletions(-)
diff --git a/arch/arm/mach-shmobile/board-ap4evb.c b/arch/arm/mach-shmobile/board-ap4evb.c
index 790dc68..c2cfbc4 100644
--- a/arch/arm/mach-shmobile/board-ap4evb.c
+++ b/arch/arm/mach-shmobile/board-ap4evb.c
@@ -50,6 +50,7 @@
#include <media/sh_mobile_ceu.h>
#include <media/sh_mobile_csi2.h>
#include <media/soc_camera.h>
+#include <media/v4l2-async.h>
#include <sound/sh_fsi.h>
#include <sound/simple_card.h>
@@ -992,22 +993,32 @@ static struct platform_device leds_device = {
},
};
-static struct i2c_board_info imx074_info = {
- I2C_BOARD_INFO("imx074", 0x1a),
+/* I2C */
+static struct soc_camera_subdev_desc imx074_desc;
+static struct i2c_board_info i2c0_devices[] = {
+ {
+ I2C_BOARD_INFO("ak4643", 0x13),
+ }, {
+ I2C_BOARD_INFO("imx074", 0x1a),
+ .platform_data = &imx074_desc,
+ },
};
-static struct soc_camera_link imx074_link = {
- .bus_id = 0,
- .board_info = &imx074_info,
- .i2c_adapter_id = 0,
- .module_name = "imx074",
+static struct i2c_board_info i2c1_devices[] = {
+ {
+ I2C_BOARD_INFO("r2025sd", 0x32),
+ },
};
-static struct platform_device ap4evb_camera = {
- .name = "soc-camera-pdrv",
- .id = 0,
- .dev = {
- .platform_data = &imx074_link,
+static struct resource csi2_resources[] = {
+ {
+ .name = "CSI2",
+ .start = 0xffc90000,
+ .end = 0xffc90fff,
+ .flags = IORESOURCE_MEM,
+ }, {
+ .start = intcs_evt2irq(0x17a0),
+ .flags = IORESOURCE_IRQ,
},
};
@@ -1016,7 +1027,7 @@ static struct sh_csi2_client_config csi2_clients[] = {
.phy = SH_CSI2_PHY_MAIN,
.lanes = 0, /* default: 2 lanes */
.channel = 0,
- .pdev = &ap4evb_camera,
+ .name = "imx074",
},
};
@@ -1027,31 +1038,50 @@ static struct sh_csi2_pdata csi2_info = {
.flags = SH_CSI2_ECC | SH_CSI2_CRC,
};
-static struct resource csi2_resources[] = {
- [0] = {
- .name = "CSI2",
- .start = 0xffc90000,
- .end = 0xffc90fff,
- .flags = IORESOURCE_MEM,
+static struct platform_device csi2_device = {
+ .name = "sh-mobile-csi2",
+ .id = 0,
+ .num_resources = ARRAY_SIZE(csi2_resources),
+ .resource = csi2_resources,
+ .dev = {
+ .platform_data = &csi2_info,
},
- [1] = {
- .start = intcs_evt2irq(0x17a0),
- .flags = IORESOURCE_IRQ,
+};
+
+static struct soc_camera_async_subdev csi2_sd = {
+ .asd.hw = {
+ .bus_type = V4L2_ASYNC_BUS_PLATFORM,
+ .match.platform.name = "sh-mobile-csi2.0",
},
+ .role = SOCAM_SUBDEV_DATA_PROCESSOR,
};
-static struct sh_mobile_ceu_companion csi2 = {
- .id = 0,
- .num_resources = ARRAY_SIZE(csi2_resources),
- .resource = csi2_resources,
- .platform_data = &csi2_info,
+static struct soc_camera_async_subdev imx074_sd = {
+ .asd.hw = {
+ .bus_type = V4L2_ASYNC_BUS_I2C,
+ .match.i2c = {
+ .adapter_id = 0,
+ .address = 0x1a,
+ },
+ },
+ .role = SOCAM_SUBDEV_DATA_SOURCE,
};
+static struct v4l2_async_subdev *ceu_subdevs[] = {
+ /* Single 2-element group */
+ &csi2_sd.asd,
+ &imx074_sd.asd,
+};
+
+/* 0-terminated array of group-sizes */
+static int ceu_subdev_sizes[] = {ARRAY_SIZE(ceu_subdevs), 0};
+
static struct sh_mobile_ceu_info sh_mobile_ceu_info = {
.flags = SH_CEU_FLAG_USE_8BIT_BUS,
.max_width = 8188,
.max_height = 8188,
- .csi2 = &csi2,
+ .asd = ceu_subdevs,
+ .asd_sizes = ceu_subdev_sizes,
};
static struct resource ceu_resources[] = {
@@ -1096,7 +1126,7 @@ static struct platform_device *ap4evb_devices[] __initdata = {
&lcdc_device,
&lcdc1_device,
&ceu_device,
- &ap4evb_camera,
+ &csi2_device,
&meram_device,
};
@@ -1212,19 +1242,6 @@ static struct i2c_board_info tsc_device = {
/*.irq is selected on ap4evb_init */
};
-/* I2C */
-static struct i2c_board_info i2c0_devices[] = {
- {
- I2C_BOARD_INFO("ak4643", 0x13),
- },
-};
-
-static struct i2c_board_info i2c1_devices[] = {
- {
- I2C_BOARD_INFO("r2025sd", 0x32),
- },
-};
-
#define GPIO_PORT9CR IOMEM(0xE6051009)
#define GPIO_PORT10CR IOMEM(0xE605100A)
@@ -1239,6 +1256,7 @@ static void __init ap4evb_init(void)
{ "A3SP", &sdhi0_device, },
{ "A3SP", &sdhi1_device, },
{ "A4R", &ceu_device, },
+ { "A4R", &csi2_device, },
};
u32 srcr4;
struct clk *clk;
@@ -1480,6 +1498,7 @@ static void __init ap4evb_init(void)
sh7372_pm_init();
pm_clk_add(&fsi_device.dev, "spu2");
pm_clk_add(&lcdc1_device.dev, "hdmi");
+ pm_clk_add(&csi2_device.dev, "csir");
}
MACHINE_START(AP4EVB, "ap4evb")
diff --git a/arch/arm/mach-shmobile/clock-sh7372.c b/arch/arm/mach-shmobile/clock-sh7372.c
index 430a90f..e6a4528 100644
--- a/arch/arm/mach-shmobile/clock-sh7372.c
+++ b/arch/arm/mach-shmobile/clock-sh7372.c
@@ -678,6 +678,7 @@ static struct clk_lookup lookups[] = {
CLKDEV_ICK_ID("icka", "sh_fsi2", &div6_reparent_clks[DIV6_FSIA]),
CLKDEV_ICK_ID("ickb", "sh_fsi2", &div6_reparent_clks[DIV6_FSIB]),
CLKDEV_ICK_ID("spu2", "sh_fsi2", &mstp_clks[MSTP223]),
+ CLKDEV_ICK_ID("csir", "sh-mobile-csi2.0", &div4_clks[DIV4_CSIR]),
};
void __init sh7372_clock_init(void)
--
1.7.2.5
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [PATCH 1/6 v4] media: V4L2: support asynchronous subdevice registration
2012-12-26 17:49 ` [PATCH 1/6] media: V4L2: support asynchronous subdevice registration Guennadi Liakhovetski
@ 2013-01-07 10:23 ` Guennadi Liakhovetski
2013-01-08 8:10 ` Laurent Pinchart
2013-01-08 10:06 ` [PATCH 1/6 v5] " Guennadi Liakhovetski
0 siblings, 2 replies; 31+ messages in thread
From: Guennadi Liakhovetski @ 2013-01-07 10:23 UTC (permalink / raw)
To: linux-media
Cc: Laurent Pinchart, Hans Verkuil, Sylwester Nawrocki,
Sylwester Nawrocki, linux-sh, Magnus Damm, Sakari Ailus,
Prabhakar Lad
From 0e1eae338ba898dc25ec60e3dba99e5581edc199 Mon Sep 17 00:00:00 2001
From: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
Date: Fri, 19 Oct 2012 23:40:44 +0200
Subject: [PATCH] media: V4L2: support asynchronous subdevice registration
Currently bridge device drivers register devices for all subdevices
synchronously, tupically, during their probing. E.g. if an I2C CMOS sensor
is attached to a video bridge device, the bridge driver will create an I2C
device and wait for the respective I2C driver to probe. This makes linking
of devices straight forward, but this approach cannot be used with
intrinsically asynchronous and unordered device registration systems like
the Flattened Device Tree. To support such systems this patch adds an
asynchronous subdevice registration framework to V4L2. To use it respective
(e.g. I2C) subdevice drivers must request deferred probing as long as their
bridge driver hasn't probed. The bridge driver during its probing submits a
an arbitrary number of subdevice descriptor groups to the framework to
manage. After that it can add callbacks to each of those groups to be
called at various stages during subdevice probing, e.g. after completion.
Then the bridge driver can request single groups to be probed, finish its
own probing and continue its video subsystem configuration from its
callbacks.
Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
---
v4: Fixed v4l2_async_notifier_register() for the case, when subdevices
probe successfully before the bridge, thanks to Prabhakar for reporting
drivers/media/v4l2-core/Makefile | 3 +-
drivers/media/v4l2-core/v4l2-async.c | 284 ++++++++++++++++++++++++++++++++++
include/media/v4l2-async.h | 113 ++++++++++++++
3 files changed, 399 insertions(+), 1 deletions(-)
create mode 100644 drivers/media/v4l2-core/v4l2-async.c
create mode 100644 include/media/v4l2-async.h
diff --git a/drivers/media/v4l2-core/Makefile b/drivers/media/v4l2-core/Makefile
index d065c01..b667ced 100644
--- a/drivers/media/v4l2-core/Makefile
+++ b/drivers/media/v4l2-core/Makefile
@@ -5,7 +5,8 @@
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-clk.o
+ v4l2-event.o v4l2-ctrls.o v4l2-subdev.o v4l2-clk.o \
+ v4l2-async.o
ifeq ($(CONFIG_COMPAT),y)
videodev-objs += v4l2-compat-ioctl32.o
endif
diff --git a/drivers/media/v4l2-core/v4l2-async.c b/drivers/media/v4l2-core/v4l2-async.c
new file mode 100644
index 0000000..55c2ad0
--- /dev/null
+++ b/drivers/media/v4l2-core/v4l2-async.c
@@ -0,0 +1,284 @@
+/*
+ * V4L2 asynchronous subdevice registration API
+ *
+ * Copyright (C) 2012, 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/device.h>
+#include <linux/err.h>
+#include <linux/i2c.h>
+#include <linux/list.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/notifier.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+#include <linux/types.h>
+
+#include <media/v4l2-async.h>
+#include <media/v4l2-device.h>
+#include <media/v4l2-subdev.h>
+
+static bool match_i2c(struct device *dev, struct v4l2_async_hw_device *hw_dev)
+{
+ struct i2c_client *client = to_i2c_client(dev);
+ return hw_dev->bus_type = V4L2_ASYNC_BUS_I2C &&
+ hw_dev->match.i2c.adapter_id = client->adapter->nr &&
+ hw_dev->match.i2c.address = client->addr;
+}
+
+static bool match_platform(struct device *dev, struct v4l2_async_hw_device *hw_dev)
+{
+ return hw_dev->bus_type = V4L2_ASYNC_BUS_PLATFORM &&
+ !strcmp(hw_dev->match.platform.name, dev_name(dev));
+}
+
+static LIST_HEAD(subdev_list);
+static LIST_HEAD(notifier_list);
+static DEFINE_MUTEX(list_lock);
+
+static struct v4l2_async_subdev *v4l2_async_belongs(struct v4l2_async_notifier *notifier,
+ struct v4l2_async_subdev_list *asdl)
+{
+ struct v4l2_async_subdev *asd = NULL;
+ bool (*match)(struct device *,
+ struct v4l2_async_hw_device *);
+
+ list_for_each_entry (asd, ¬ifier->waiting, list) {
+ struct v4l2_async_hw_device *hw = &asd->hw;
+ switch (hw->bus_type) {
+ case V4L2_ASYNC_BUS_SPECIAL:
+ match = hw->match.special.match;
+ if (!match)
+ /* Match always */
+ return asd;
+ break;
+ case V4L2_ASYNC_BUS_PLATFORM:
+ match = match_platform;
+ break;
+ case V4L2_ASYNC_BUS_I2C:
+ match = match_i2c;
+ break;
+ default:
+ /* Oops */
+ match = NULL;
+ dev_err(notifier->v4l2_dev ? notifier->v4l2_dev->dev : NULL,
+ "Invalid bus-type %u on %p\n", hw->bus_type, asd);
+ }
+
+ if (match && match(asdl->dev, hw))
+ break;
+ }
+
+ return asd;
+}
+
+static int v4l2_async_test_notify(struct v4l2_async_notifier *notifier,
+ struct v4l2_async_subdev_list *asdl)
+{
+ struct v4l2_async_subdev *asd = v4l2_async_belongs(notifier, asdl);
+ if (asd) {
+ int ret;
+ /* Remove from the waiting list */
+ list_del(&asd->list);
+ asdl->asd = asd;
+ asdl->notifier = notifier;
+
+ if (notifier->bound) {
+ ret = notifier->bound(notifier, asdl);
+ if (ret < 0)
+ return ret;
+ }
+ /* Move from the global subdevice list to notifier's done */
+ list_move(&asdl->list, ¬ifier->done);
+
+ ret = v4l2_device_register_subdev(notifier->v4l2_dev,
+ asdl->subdev);
+ if (ret < 0) {
+ if (notifier->unbind)
+ notifier->unbind(notifier, asdl);
+ return ret;
+ }
+
+ if (list_empty(¬ifier->waiting) && notifier->complete)
+ return notifier->complete(notifier);
+
+ return 0;
+ }
+
+ return -EPROBE_DEFER;
+}
+
+static struct device *v4l2_async_unbind(struct v4l2_async_subdev_list *asdl)
+{
+ struct device *dev = asdl->dev;
+ v4l2_device_unregister_subdev(asdl->subdev);
+ /* Subdevice driver will reprobe and put asdl back onto the list */
+ list_del(&asdl->list);
+ asdl->asd = NULL;
+ /* If we handled USB devices, we'd have to lock the parent too */
+ device_release_driver(dev);
+ return dev;
+}
+
+int v4l2_async_notifier_register(struct v4l2_device *v4l2_dev,
+ struct v4l2_async_notifier *notifier)
+{
+ struct v4l2_async_subdev_list *asdl;
+ int i;
+
+ notifier->v4l2_dev = v4l2_dev;
+ INIT_LIST_HEAD(¬ifier->waiting);
+ INIT_LIST_HEAD(¬ifier->done);
+
+ for (i = 0; i < notifier->subdev_num; i++)
+ list_add_tail(¬ifier->subdev[i]->list, ¬ifier->waiting);
+
+ mutex_lock(&list_lock);
+
+ /* Keep also completed notifiers on the list */
+ list_add(¬ifier->list, ¬ifier_list);
+
+ list_for_each_entry(asdl, &subdev_list, list) {
+ int ret = v4l2_async_test_notify(notifier, asdl);
+ if (ret < 0 && ret != -EPROBE_DEFER) {
+ mutex_unlock(&list_lock);
+ return ret;
+ }
+ }
+
+ mutex_unlock(&list_lock);
+
+ return 0;
+}
+EXPORT_SYMBOL(v4l2_async_notifier_register);
+
+void v4l2_async_notifier_unregister(struct v4l2_async_notifier *notifier)
+{
+ struct v4l2_async_subdev_list *asdl, *tmp;
+ int i = 0;
+ struct device **dev = kcalloc(notifier->subdev_num,
+ sizeof(*dev), GFP_KERNEL);
+ if (!dev)
+ dev_err(notifier->v4l2_dev->dev,
+ "Failed to allocate device cache!\n");
+
+ mutex_lock(&list_lock);
+
+ list_del(¬ifier->list);
+
+ list_for_each_entry_safe(asdl, tmp, ¬ifier->done, list) {
+ if (dev)
+ dev[i++] = get_device(asdl->dev);
+ v4l2_async_unbind(asdl);
+
+ if (notifier->unbind)
+ notifier->unbind(notifier, asdl);
+ }
+
+ mutex_unlock(&list_lock);
+
+ if (dev) {
+ while (i--) {
+ if (dev[i] && device_attach(dev[i]) < 0)
+ dev_err(dev[i], "Failed to re-probe to %s\n",
+ dev[i]->driver ? dev[i]->driver->name : "(none)");
+ put_device(dev[i]);
+ }
+ kfree(dev);
+ }
+ /*
+ * Don't care about the waiting list, it is initialised and populated
+ * upon notifier registration.
+ */
+}
+EXPORT_SYMBOL(v4l2_async_notifier_unregister);
+
+int v4l2_async_subdev_bind(struct v4l2_async_subdev_list *asdl)
+{
+ struct v4l2_async_notifier *notifier;
+ int ret = 0;
+
+ mutex_lock(&list_lock);
+
+ list_for_each_entry(notifier, ¬ifier_list, list) {
+ struct v4l2_async_subdev *asd = v4l2_async_belongs(notifier,
+ asdl);
+ /*
+ * Whether or not probing succeeds - this is the right hardware
+ * subdevice descriptor and we can provide it to the notifier
+ */
+ if (asd) {
+ asdl->asd = asd;
+ if (notifier->bind)
+ ret = notifier->bind(notifier, asdl);
+ break;
+ }
+ }
+
+ mutex_unlock(&list_lock);
+
+ return ret;
+}
+EXPORT_SYMBOL(v4l2_async_subdev_bind);
+
+int v4l2_async_subdev_bound(struct v4l2_async_subdev_list *asdl)
+{
+ struct v4l2_async_notifier *notifier;
+
+ mutex_lock(&list_lock);
+
+ INIT_LIST_HEAD(&asdl->list);
+
+ list_for_each_entry(notifier, ¬ifier_list, list) {
+ int ret = v4l2_async_test_notify(notifier, asdl);
+ if (ret != -EPROBE_DEFER) {
+ mutex_unlock(&list_lock);
+ return ret;
+ }
+ }
+
+ /* None matched, wait for hot-plugging */
+ list_add(&asdl->list, &subdev_list);
+
+ mutex_unlock(&list_lock);
+
+ return 0;
+}
+EXPORT_SYMBOL(v4l2_async_subdev_bound);
+
+void v4l2_async_subdev_unbind(struct v4l2_async_subdev_list *asdl)
+{
+ struct v4l2_async_notifier *notifier = asdl->notifier;
+ struct device *dev;
+
+ if (!asdl->asd)
+ return;
+
+ mutex_lock(&list_lock);
+
+ dev = asdl->dev;
+
+ list_add(&asdl->asd->list, ¬ifier->waiting);
+
+ dev = get_device(asdl->dev);
+
+ v4l2_async_unbind(asdl);
+
+ if (notifier->unbind)
+ notifier->unbind(notifier, asdl);
+
+ mutex_unlock(&list_lock);
+
+ /* Re-probe with lock released - avoid a deadlock */
+ if (dev && device_attach(dev) < 0)
+ dev_err(dev, "Failed to re-probe to %s\n",
+ dev->driver ? dev->driver->name : "(none)");
+
+ put_device(dev);
+}
+EXPORT_SYMBOL(v4l2_async_subdev_unbind);
diff --git a/include/media/v4l2-async.h b/include/media/v4l2-async.h
new file mode 100644
index 0000000..91d436d
--- /dev/null
+++ b/include/media/v4l2-async.h
@@ -0,0 +1,113 @@
+/*
+ * V4L2 asynchronous subdevice registration API
+ *
+ * Copyright (C) 2012, 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.
+ */
+
+#ifndef V4L2_ASYNC_H
+#define V4L2_ASYNC_H
+
+#include <linux/list.h>
+#include <linux/mutex.h>
+#include <linux/notifier.h>
+
+#include <media/v4l2-subdev.h>
+
+struct device;
+struct v4l2_device;
+struct v4l2_async_notifier;
+
+enum v4l2_async_bus_type {
+ V4L2_ASYNC_BUS_SPECIAL,
+ V4L2_ASYNC_BUS_PLATFORM,
+ V4L2_ASYNC_BUS_I2C,
+};
+
+struct v4l2_async_hw_device {
+ enum v4l2_async_bus_type bus_type;
+ union {
+ struct {
+ const char *name;
+ } platform;
+ struct {
+ int adapter_id;
+ unsigned short address;
+ } i2c;
+ struct {
+ bool (*match)(struct device *,
+ struct v4l2_async_hw_device *);
+ void *priv;
+ } special;
+ } match;
+};
+
+/**
+ * struct v4l2_async_subdev - sub-device descriptor, as known to a bridge
+ * @hw: this device descriptor
+ * @list: member in a list of subdevices
+ */
+struct v4l2_async_subdev {
+ struct v4l2_async_hw_device hw;
+ struct list_head list;
+};
+
+/**
+ * v4l2_async_subdev_list - provided by subdevices
+ * @list: member in a list of subdevices
+ * @dev: hardware device
+ * @subdev: V4L2 subdevice
+ * @asd: pointer to respective struct v4l2_async_subdev
+ * @notifier: pointer to managing notifier
+ */
+struct v4l2_async_subdev_list {
+ struct list_head list;
+ struct device *dev;
+ struct v4l2_subdev *subdev;
+ struct v4l2_async_subdev *asd;
+ struct v4l2_async_notifier *notifier;
+};
+
+/**
+ * v4l2_async_notifier - provided by bridges
+ * @subdev_num: number of subdevices
+ * @subdev: array of pointers to subdevices
+ * @v4l2_dev: pointer to sruct v4l2_device
+ * @waiting: list of subdevices, waiting for their drivers
+ * @done: list of subdevices, already probed
+ * @list: member in a global list of notifiers
+ * @bind: a subdevice driver is about to probe one of your subdevices
+ * @bound: a subdevice driver has successfully probed one of your subdevices
+ * @complete: all your subdevices have been probed successfully
+ * @unbind: a subdevice is leaving
+ */
+struct v4l2_async_notifier {
+ int subdev_num;
+ struct v4l2_async_subdev **subdev;
+ struct v4l2_device *v4l2_dev;
+ struct list_head waiting;
+ struct list_head done;
+ struct list_head list;
+ int (*bind)(struct v4l2_async_notifier *notifier,
+ struct v4l2_async_subdev_list *asdl);
+ int (*bound)(struct v4l2_async_notifier *notifier,
+ struct v4l2_async_subdev_list *asdl);
+ int (*complete)(struct v4l2_async_notifier *notifier);
+ void (*unbind)(struct v4l2_async_notifier *notifier,
+ struct v4l2_async_subdev_list *asdl);
+};
+
+int v4l2_async_notifier_register(struct v4l2_device *v4l2_dev,
+ struct v4l2_async_notifier *notifier);
+void v4l2_async_notifier_unregister(struct v4l2_async_notifier *notifier);
+/*
+ * If subdevice probing fails any time after v4l2_async_subdev_bind(), no clean
+ * up must be called. This function is only a message of intention.
+ */
+int v4l2_async_subdev_bind(struct v4l2_async_subdev_list *asdl);
+int v4l2_async_subdev_bound(struct v4l2_async_subdev_list *asdl);
+void v4l2_async_subdev_unbind(struct v4l2_async_subdev_list *asdl);
+#endif
--
1.7.2.5
^ permalink raw reply related [flat|nested] 31+ messages in thread
* Re: [PATCH 6/6] ARM: shmobile: convert ap4evb to asynchronously register camera subdevices
2012-12-26 17:49 ` [PATCH 6/6] ARM: shmobile: convert ap4evb to asynchronously register camera subdevices Guennadi Liakhovetski
@ 2013-01-08 4:27 ` Simon Horman
2013-01-08 22:35 ` Guennadi Liakhovetski
0 siblings, 1 reply; 31+ messages in thread
From: Simon Horman @ 2013-01-08 4:27 UTC (permalink / raw)
To: Guennadi Liakhovetski
Cc: linux-media, Laurent Pinchart, Hans Verkuil, Sylwester Nawrocki,
Sylwester Nawrocki, linux-sh, Magnus Damm, Sakari Ailus
On Wed, Dec 26, 2012 at 06:49:11PM +0100, Guennadi Liakhovetski wrote:
> Register the imx074 camera I2C and the CSI-2 platform devices directly
> in board platform data instead of letting the sh_mobile_ceu_camera driver
> and the soc-camera framework register them at their run-time. This uses
> the V4L2 asynchronous subdevice probing capability.
>
> Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
Hi Guennadi,
could you let me know what if any dependencies this patch has.
And the status of any dependencies.
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 2/6] media: soc-camera: switch I2C subdevice drivers to use v4l2-clk
2012-12-26 17:49 ` [PATCH 2/6] media: soc-camera: switch I2C subdevice drivers to use v4l2-clk Guennadi Liakhovetski
@ 2013-01-08 8:02 ` Laurent Pinchart
0 siblings, 0 replies; 31+ messages in thread
From: Laurent Pinchart @ 2013-01-08 8:02 UTC (permalink / raw)
To: Guennadi Liakhovetski
Cc: linux-media, Hans Verkuil, Sylwester Nawrocki, Sylwester Nawrocki,
linux-sh, Magnus Damm, Sakari Ailus
Hi Guennadi,
Thanks for the patch.
On Wednesday 26 December 2012 18:49:07 Guennadi Liakhovetski wrote:
> Instead of centrally enabling and disabling subdevice master clocks in
> soc-camera core, let subdevice drivers do that themselves, using the
> V4L2 clock API and soc-camera convenience wrappers.
>
> Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> ---
> drivers/media/i2c/soc_camera/imx074.c | 18 ++-
> drivers/media/i2c/soc_camera/mt9m001.c | 17 ++-
> drivers/media/i2c/soc_camera/mt9m111.c | 20 ++-
> drivers/media/i2c/soc_camera/mt9t031.c | 19 ++-
> drivers/media/i2c/soc_camera/mt9t112.c | 19 ++-
> drivers/media/i2c/soc_camera/mt9v022.c | 17 ++-
> drivers/media/i2c/soc_camera/ov2640.c | 19 ++-
> drivers/media/i2c/soc_camera/ov5642.c | 20 ++-
> drivers/media/i2c/soc_camera/ov6650.c | 17 ++-
> drivers/media/i2c/soc_camera/ov772x.c | 15 ++-
> drivers/media/i2c/soc_camera/ov9640.c | 17 ++-
> drivers/media/i2c/soc_camera/ov9640.h | 1 +
> drivers/media/i2c/soc_camera/ov9740.c | 18 ++-
> drivers/media/i2c/soc_camera/rj54n1cb0c.c | 17 ++-
> drivers/media/i2c/soc_camera/tw9910.c | 18 ++-
> drivers/media/platform/soc_camera/soc_camera.c | 173 ++++++++++++----
> .../platform/soc_camera/soc_camera_platform.c | 2 +-
> include/media/soc_camera.h | 13 +-
> 18 files changed, 356 insertions(+), 84 deletions(-)
[snip]
> diff --git a/drivers/media/platform/soc_camera/soc_camera.c
> b/drivers/media/platform/soc_camera/soc_camera.c index 0b6ddff..a9e6f01
> 100644
> --- a/drivers/media/platform/soc_camera/soc_camera.c
> +++ b/drivers/media/platform/soc_camera/soc_camera.c
[snip]
> @@ -1068,6 +1091,57 @@ static void scan_add_host(struct soc_camera_host
> *ici) mutex_unlock(&list_lock);
> }
>
> +/*
> + * It is invalid to call v4l2_clk_enable() after a successful probing
> + * asynchronously outside of V4L2 operations, i.e. with .host_lock not
> held.
> + */
> +static int soc_camera_clk_enable(struct v4l2_clk *clk)
> +{
> + struct soc_camera_device *icd = clk->priv;
> + struct soc_camera_host *ici;
> +
> + if (!icd || !icd->parent)
> + return -ENODEV;
> +
> + ici = to_soc_camera_host(icd->parent);
> +
> + if (!try_module_get(ici->ops->owner))
> + return -ENODEV;
> +
> + /*
> + * If a different client is currently being probed, the host will tell
> + * you to go
> + */
> + return ici->ops->add(icd);
> +}
> +
> +static void soc_camera_clk_disable(struct v4l2_clk *clk)
> +{
> + struct soc_camera_device *icd = clk->priv;
> + struct soc_camera_host *ici;
> +
> + if (!icd || !icd->parent)
> + return;
> +
> + ici = to_soc_camera_host(icd->parent);
> +
> + ici->ops->remove(icd);
> +
> + module_put(ici->ops->owner);
> +}
> +
> +/*
> + * Eventually, it would be more logical to make the respective host the
> clock
> + * owner, but then we would have to copy this struct for each ici. Besides,
> it
> + * would introduce the circular dependency problem, unless we port all
> client
> + * drivers to release the clock, when not in use.
> + */
Won't we have to solve this problem eventually ? This should probably be put
on the agenda of the next V4L2 workshop/summit.
> +static const struct v4l2_clk_ops soc_camera_clk_ops = {
> + .owner = THIS_MODULE,
> + .enable = soc_camera_clk_enable,
> + .disable = soc_camera_clk_disable,
> +};
> +
> #ifdef CONFIG_I2C_BOARDINFO
> static int soc_camera_init_i2c(struct soc_camera_device *icd,
> struct soc_camera_desc *sdesc)
> @@ -1077,19 +1151,33 @@ static int soc_camera_init_i2c(struct
> soc_camera_device *icd, struct soc_camera_host_desc *shd > &sdesc->host_desc;
> struct i2c_adapter *adap = i2c_get_adapter(shd->i2c_adapter_id);
> struct v4l2_subdev *subdev;
> + char clk_name[V4L2_SUBDEV_NAME_SIZE];
> + int ret;
>
> if (!adap) {
> dev_err(icd->pdev, "Cannot get I2C adapter #%d. No driver?\n",
> shd->i2c_adapter_id);
> - goto ei2cga;
> + return -ENODEV;
> }
>
> shd->board_info->platform_data = &sdesc->subdev_desc;
>
> + snprintf(clk_name, sizeof(clk_name), "%s %d-%04x",
> + shd->board_info->type,
> + shd->i2c_adapter_id, shd->board_info->addr);
> +
> + icd->clk = v4l2_clk_register(&soc_camera_clk_ops, clk_name, "mclk", icd);
I don't think you should hardcode the clock name to "mclk". The common clock
framework use a client-specific clock name when requesting a clock, it would
be better to use a similar mechanism.
> + if (IS_ERR(icd->clk)) {
> + ret = PTR_ERR(icd->clk);
> + goto eclkreg;
> + }
> +
> subdev = v4l2_i2c_new_subdev_board(&ici->v4l2_dev, adap,
> shd->board_info, NULL);
> - if (!subdev)
> + if (!subdev) {
> + ret = -ENODEV;
> goto ei2cnd;
> + }
>
> client = v4l2_get_subdevdata(subdev);
>
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 1/6 v4] media: V4L2: support asynchronous subdevice registration
2013-01-07 10:23 ` [PATCH 1/6 v4] " Guennadi Liakhovetski
@ 2013-01-08 8:10 ` Laurent Pinchart
2013-01-08 9:25 ` Guennadi Liakhovetski
2013-01-08 14:52 ` Sylwester Nawrocki
2013-01-08 10:06 ` [PATCH 1/6 v5] " Guennadi Liakhovetski
1 sibling, 2 replies; 31+ messages in thread
From: Laurent Pinchart @ 2013-01-08 8:10 UTC (permalink / raw)
To: Guennadi Liakhovetski
Cc: linux-media, Hans Verkuil, Sylwester Nawrocki, Sylwester Nawrocki,
linux-sh, Magnus Damm, Sakari Ailus, Prabhakar Lad
Hi Guennadi,
Thanks for the patch.
On Monday 07 January 2013 11:23:55 Guennadi Liakhovetski wrote:
> >From 0e1eae338ba898dc25ec60e3dba99e5581edc199 Mon Sep 17 00:00:00 2001
>
> From: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> Date: Fri, 19 Oct 2012 23:40:44 +0200
> Subject: [PATCH] media: V4L2: support asynchronous subdevice registration
>
> Currently bridge device drivers register devices for all subdevices
> synchronously, tupically, during their probing. E.g. if an I2C CMOS sensor
> is attached to a video bridge device, the bridge driver will create an I2C
> device and wait for the respective I2C driver to probe. This makes linking
> of devices straight forward, but this approach cannot be used with
> intrinsically asynchronous and unordered device registration systems like
> the Flattened Device Tree. To support such systems this patch adds an
> asynchronous subdevice registration framework to V4L2. To use it respective
> (e.g. I2C) subdevice drivers must request deferred probing as long as their
> bridge driver hasn't probed. The bridge driver during its probing submits a
> an arbitrary number of subdevice descriptor groups to the framework to
> manage. After that it can add callbacks to each of those groups to be
> called at various stages during subdevice probing, e.g. after completion.
> Then the bridge driver can request single groups to be probed, finish its
> own probing and continue its video subsystem configuration from its
> callbacks.
>
> Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> ---
>
> v4: Fixed v4l2_async_notifier_register() for the case, when subdevices
> probe successfully before the bridge, thanks to Prabhakar for reporting
>
> drivers/media/v4l2-core/Makefile | 3 +-
> drivers/media/v4l2-core/v4l2-async.c | 284 +++++++++++++++++++++++++++++++
> include/media/v4l2-async.h | 113 ++++++++++++++
> 3 files changed, 399 insertions(+), 1 deletions(-)
> create mode 100644 drivers/media/v4l2-core/v4l2-async.c
> create mode 100644 include/media/v4l2-async.h
[snip]
> diff --git a/include/media/v4l2-async.h b/include/media/v4l2-async.h
> new file mode 100644
> index 0000000..91d436d
> --- /dev/null
> +++ b/include/media/v4l2-async.h
> @@ -0,0 +1,113 @@
> +/*
> + * V4L2 asynchronous subdevice registration API
> + *
> + * Copyright (C) 2012, 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.
> + */
> +
> +#ifndef V4L2_ASYNC_H
> +#define V4L2_ASYNC_H
> +
> +#include <linux/list.h>
> +#include <linux/mutex.h>
> +#include <linux/notifier.h>
> +
> +#include <media/v4l2-subdev.h>
> +
> +struct device;
> +struct v4l2_device;
> +struct v4l2_async_notifier;
> +
> +enum v4l2_async_bus_type {
> + V4L2_ASYNC_BUS_SPECIAL,
> + V4L2_ASYNC_BUS_PLATFORM,
> + V4L2_ASYNC_BUS_I2C,
> +};
> +
> +struct v4l2_async_hw_device {
> + enum v4l2_async_bus_type bus_type;
> + union {
> + struct {
> + const char *name;
> + } platform;
> + struct {
> + int adapter_id;
> + unsigned short address;
> + } i2c;
> + struct {
> + bool (*match)(struct device *,
> + struct v4l2_async_hw_device *);
> + void *priv;
> + } special;
> + } match;
> +};
> +
> +/**
> + * struct v4l2_async_subdev - sub-device descriptor, as known to a bridge
> + * @hw: this device descriptor
> + * @list: member in a list of subdevices
> + */
> +struct v4l2_async_subdev {
> + struct v4l2_async_hw_device hw;
> + struct list_head list;
> +};
> +
> +/**
> + * v4l2_async_subdev_list - provided by subdevices
> + * @list: member in a list of subdevices
> + * @dev: hardware device
> + * @subdev: V4L2 subdevice
> + * @asd: pointer to respective struct v4l2_async_subdev
> + * @notifier: pointer to managing notifier
> + */
> +struct v4l2_async_subdev_list {
> + struct list_head list;
> + struct device *dev;
> + struct v4l2_subdev *subdev;
> + struct v4l2_async_subdev *asd;
> + struct v4l2_async_notifier *notifier;
> +};
> +
> +/**
> + * v4l2_async_notifier - provided by bridges
> + * @subdev_num: number of subdevices
> + * @subdev: array of pointers to subdevices
> + * @v4l2_dev: pointer to sruct v4l2_device
> + * @waiting: list of subdevices, waiting for their drivers
> + * @done: list of subdevices, already probed
> + * @list: member in a global list of notifiers
> + * @bind: a subdevice driver is about to probe one of your subdevices
> + * @bound: a subdevice driver has successfully probed one of your
> subdevices + * @complete: all your subdevices have been probed successfully
> + * @unbind: a subdevice is leaving
> + */
> +struct v4l2_async_notifier {
> + int subdev_num;
> + struct v4l2_async_subdev **subdev;
> + struct v4l2_device *v4l2_dev;
> + struct list_head waiting;
> + struct list_head done;
> + struct list_head list;
> + int (*bind)(struct v4l2_async_notifier *notifier,
> + struct v4l2_async_subdev_list *asdl);
> + int (*bound)(struct v4l2_async_notifier *notifier,
> + struct v4l2_async_subdev_list *asdl);
> + int (*complete)(struct v4l2_async_notifier *notifier);
> + void (*unbind)(struct v4l2_async_notifier *notifier,
> + struct v4l2_async_subdev_list *asdl);
> +};
> +
> +int v4l2_async_notifier_register(struct v4l2_device *v4l2_dev,
> + struct v4l2_async_notifier *notifier);
> +void v4l2_async_notifier_unregister(struct v4l2_async_notifier *notifier);
> +/*
> + * If subdevice probing fails any time after v4l2_async_subdev_bind(), no
> + * clean up must be called. This function is only a message of intention.
> + */
> +int v4l2_async_subdev_bind(struct v4l2_async_subdev_list *asdl);
> +int v4l2_async_subdev_bound(struct v4l2_async_subdev_list *asdl);
Could you please explain why you need both a bind notifier and a bound
notifier ? I was expecting a single v4l2_async_subdev_register() call in
subdev drivers (and, thinking about it, I would probably name it
v4l2_subdev_register()).
> +void v4l2_async_subdev_unbind(struct v4l2_async_subdev_list *asdl);
> +#endif
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 1/6 v4] media: V4L2: support asynchronous subdevice registration
2013-01-08 8:10 ` Laurent Pinchart
@ 2013-01-08 9:25 ` Guennadi Liakhovetski
2013-01-08 9:41 ` Laurent Pinchart
2013-01-08 14:52 ` Sylwester Nawrocki
1 sibling, 1 reply; 31+ messages in thread
From: Guennadi Liakhovetski @ 2013-01-08 9:25 UTC (permalink / raw)
To: Laurent Pinchart
Cc: linux-media, Hans Verkuil, Sylwester Nawrocki, Sylwester Nawrocki,
linux-sh, Magnus Damm, Sakari Ailus, Prabhakar Lad
Hi Laurent
On Tue, 8 Jan 2013, Laurent Pinchart wrote:
> Hi Guennadi,
>
> Thanks for the patch.
>
> On Monday 07 January 2013 11:23:55 Guennadi Liakhovetski wrote:
> > >From 0e1eae338ba898dc25ec60e3dba99e5581edc199 Mon Sep 17 00:00:00 2001
> >
> > From: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> > Date: Fri, 19 Oct 2012 23:40:44 +0200
> > Subject: [PATCH] media: V4L2: support asynchronous subdevice registration
> >
> > Currently bridge device drivers register devices for all subdevices
> > synchronously, tupically, during their probing. E.g. if an I2C CMOS sensor
> > is attached to a video bridge device, the bridge driver will create an I2C
> > device and wait for the respective I2C driver to probe. This makes linking
> > of devices straight forward, but this approach cannot be used with
> > intrinsically asynchronous and unordered device registration systems like
> > the Flattened Device Tree. To support such systems this patch adds an
> > asynchronous subdevice registration framework to V4L2. To use it respective
> > (e.g. I2C) subdevice drivers must request deferred probing as long as their
> > bridge driver hasn't probed. The bridge driver during its probing submits a
> > an arbitrary number of subdevice descriptor groups to the framework to
> > manage. After that it can add callbacks to each of those groups to be
> > called at various stages during subdevice probing, e.g. after completion.
> > Then the bridge driver can request single groups to be probed, finish its
> > own probing and continue its video subsystem configuration from its
> > callbacks.
> >
> > Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> > ---
> >
> > v4: Fixed v4l2_async_notifier_register() for the case, when subdevices
> > probe successfully before the bridge, thanks to Prabhakar for reporting
> >
> > drivers/media/v4l2-core/Makefile | 3 +-
> > drivers/media/v4l2-core/v4l2-async.c | 284 +++++++++++++++++++++++++++++++
> > include/media/v4l2-async.h | 113 ++++++++++++++
> > 3 files changed, 399 insertions(+), 1 deletions(-)
> > create mode 100644 drivers/media/v4l2-core/v4l2-async.c
> > create mode 100644 include/media/v4l2-async.h
>
> [snip]
>
> > diff --git a/include/media/v4l2-async.h b/include/media/v4l2-async.h
> > new file mode 100644
> > index 0000000..91d436d
> > --- /dev/null
> > +++ b/include/media/v4l2-async.h
> > @@ -0,0 +1,113 @@
> > +/*
> > + * V4L2 asynchronous subdevice registration API
> > + *
> > + * Copyright (C) 2012, 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.
> > + */
> > +
> > +#ifndef V4L2_ASYNC_H
> > +#define V4L2_ASYNC_H
> > +
> > +#include <linux/list.h>
> > +#include <linux/mutex.h>
> > +#include <linux/notifier.h>
> > +
> > +#include <media/v4l2-subdev.h>
> > +
> > +struct device;
> > +struct v4l2_device;
> > +struct v4l2_async_notifier;
> > +
> > +enum v4l2_async_bus_type {
> > + V4L2_ASYNC_BUS_SPECIAL,
> > + V4L2_ASYNC_BUS_PLATFORM,
> > + V4L2_ASYNC_BUS_I2C,
> > +};
> > +
> > +struct v4l2_async_hw_device {
> > + enum v4l2_async_bus_type bus_type;
> > + union {
> > + struct {
> > + const char *name;
> > + } platform;
> > + struct {
> > + int adapter_id;
> > + unsigned short address;
> > + } i2c;
> > + struct {
> > + bool (*match)(struct device *,
> > + struct v4l2_async_hw_device *);
> > + void *priv;
> > + } special;
> > + } match;
> > +};
> > +
> > +/**
> > + * struct v4l2_async_subdev - sub-device descriptor, as known to a bridge
> > + * @hw: this device descriptor
> > + * @list: member in a list of subdevices
> > + */
> > +struct v4l2_async_subdev {
> > + struct v4l2_async_hw_device hw;
> > + struct list_head list;
> > +};
> > +
> > +/**
> > + * v4l2_async_subdev_list - provided by subdevices
> > + * @list: member in a list of subdevices
> > + * @dev: hardware device
> > + * @subdev: V4L2 subdevice
> > + * @asd: pointer to respective struct v4l2_async_subdev
> > + * @notifier: pointer to managing notifier
> > + */
> > +struct v4l2_async_subdev_list {
> > + struct list_head list;
> > + struct device *dev;
> > + struct v4l2_subdev *subdev;
> > + struct v4l2_async_subdev *asd;
> > + struct v4l2_async_notifier *notifier;
> > +};
> > +
> > +/**
> > + * v4l2_async_notifier - provided by bridges
> > + * @subdev_num: number of subdevices
> > + * @subdev: array of pointers to subdevices
> > + * @v4l2_dev: pointer to sruct v4l2_device
> > + * @waiting: list of subdevices, waiting for their drivers
> > + * @done: list of subdevices, already probed
> > + * @list: member in a global list of notifiers
> > + * @bind: a subdevice driver is about to probe one of your subdevices
> > + * @bound: a subdevice driver has successfully probed one of your
> > subdevices + * @complete: all your subdevices have been probed successfully
> > + * @unbind: a subdevice is leaving
> > + */
> > +struct v4l2_async_notifier {
> > + int subdev_num;
> > + struct v4l2_async_subdev **subdev;
> > + struct v4l2_device *v4l2_dev;
> > + struct list_head waiting;
> > + struct list_head done;
> > + struct list_head list;
> > + int (*bind)(struct v4l2_async_notifier *notifier,
> > + struct v4l2_async_subdev_list *asdl);
> > + int (*bound)(struct v4l2_async_notifier *notifier,
> > + struct v4l2_async_subdev_list *asdl);
> > + int (*complete)(struct v4l2_async_notifier *notifier);
> > + void (*unbind)(struct v4l2_async_notifier *notifier,
> > + struct v4l2_async_subdev_list *asdl);
> > +};
> > +
> > +int v4l2_async_notifier_register(struct v4l2_device *v4l2_dev,
> > + struct v4l2_async_notifier *notifier);
> > +void v4l2_async_notifier_unregister(struct v4l2_async_notifier *notifier);
> > +/*
> > + * If subdevice probing fails any time after v4l2_async_subdev_bind(), no
> > + * clean up must be called. This function is only a message of intention.
> > + */
> > +int v4l2_async_subdev_bind(struct v4l2_async_subdev_list *asdl);
> > +int v4l2_async_subdev_bound(struct v4l2_async_subdev_list *asdl);
>
> Could you please explain why you need both a bind notifier and a bound
> notifier ? I was expecting a single v4l2_async_subdev_register() call in
> subdev drivers (and, thinking about it, I would probably name it
> v4l2_subdev_register()).
I think I can, yes. Because between .bind() and .bound() the subdevice
driver does the actual hardware probing. So, .bind() is used to make sure
the hardware can be accessed, most importantly to provide a clock to the
subdevice. You can look at soc_camera_async_bind(). There I'm registering
the clock for the subdevice, about to bind. Why I cannot do it before, is
because I need subdevice name for clock matching. With I2C subdevices the
subdevice name contains the name of the driver, adapter number and i2c
address. The latter 2 I've got from host subdevice list. But not the
driver name. I thought about also passing the driver name there, but that
seemed too limiting to me. I also request regulators there, because before
->bound() the sensor driver, but that could be done on the first call to
soc_camera_power_on(), although doing this "first call" thingie is kind of
hackish too. I could add one more soc-camera-power helper like
soc_camera_prepare() or similar too. So, the main problem is the clock
subdevice name. Also see the comment in soc_camera.c:
/*
* It is ok to keep the clock for the whole soc_camera_device life-time,
* in principle it would be more logical to register the clock on icd
* creation, the only problem is, that at that time we don't know the
* driver name yet.
*/
Thanks
Guennadi
> > +void v4l2_async_subdev_unbind(struct v4l2_async_subdev_list *asdl);
> > +#endif
> --
> Regards,
>
> Laurent Pinchart
>
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 1/6 v4] media: V4L2: support asynchronous subdevice registration
2013-01-08 9:25 ` Guennadi Liakhovetski
@ 2013-01-08 9:41 ` Laurent Pinchart
2013-01-08 9:56 ` Guennadi Liakhovetski
0 siblings, 1 reply; 31+ messages in thread
From: Laurent Pinchart @ 2013-01-08 9:41 UTC (permalink / raw)
To: Guennadi Liakhovetski
Cc: linux-media, Hans Verkuil, Sylwester Nawrocki, Sylwester Nawrocki,
linux-sh, Magnus Damm, Sakari Ailus, Prabhakar Lad
Hi Guennadi,
On Tuesday 08 January 2013 10:25:15 Guennadi Liakhovetski wrote:
> On Tue, 8 Jan 2013, Laurent Pinchart wrote:
> > On Monday 07 January 2013 11:23:55 Guennadi Liakhovetski wrote:
> > > >From 0e1eae338ba898dc25ec60e3dba99e5581edc199 Mon Sep 17 00:00:00 2001
> > >
> > > From: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> > > Date: Fri, 19 Oct 2012 23:40:44 +0200
> > > Subject: [PATCH] media: V4L2: support asynchronous subdevice
> > > registration
> > >
> > > Currently bridge device drivers register devices for all subdevices
> > > synchronously, tupically, during their probing. E.g. if an I2C CMOS
> > > sensor is attached to a video bridge device, the bridge driver will
> > > create an I2C device and wait for the respective I2C driver to probe.
> > > This makes linking of devices straight forward, but this approach cannot
> > > be used with intrinsically asynchronous and unordered device
> > > registration systems like the Flattened Device Tree. To support such
> > > systems this patch adds an asynchronous subdevice registration framework
> > > to V4L2. To use it respective (e.g. I2C) subdevice drivers must request
> > > deferred probing as long as their bridge driver hasn't probed. The
> > > bridge driver during its probing submits a an arbitrary number of
> > > subdevice descriptor groups to the framework to manage. After that it
> > > can add callbacks to each of those groups to be called at various stages
> > > during subdevice probing, e.g. after completion. Then the bridge driver
> > > can request single groups to be probed, finish its own probing and
> > > continue its video subsystem configuration from its callbacks.
> > >
> > > Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> > > ---
> > >
> > > v4: Fixed v4l2_async_notifier_register() for the case, when subdevices
> > > probe successfully before the bridge, thanks to Prabhakar for reporting
> > >
> > > drivers/media/v4l2-core/Makefile | 3 +-
> > > drivers/media/v4l2-core/v4l2-async.c | 284 +++++++++++++++++++++++++++
> > > include/media/v4l2-async.h | 113 ++++++++++++++
> > > 3 files changed, 399 insertions(+), 1 deletions(-)
> > > create mode 100644 drivers/media/v4l2-core/v4l2-async.c
> > > create mode 100644 include/media/v4l2-async.h
> >
> > [snip]
> >
> > > diff --git a/include/media/v4l2-async.h b/include/media/v4l2-async.h
> > > new file mode 100644
> > > index 0000000..91d436d
> > > --- /dev/null
> > > +++ b/include/media/v4l2-async.h
> > > @@ -0,0 +1,113 @@
> > > +/*
> > > + * V4L2 asynchronous subdevice registration API
> > > + *
> > > + * Copyright (C) 2012, 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.
> > > + */
> > > +
> > > +#ifndef V4L2_ASYNC_H
> > > +#define V4L2_ASYNC_H
> > > +
> > > +#include <linux/list.h>
> > > +#include <linux/mutex.h>
> > > +#include <linux/notifier.h>
> > > +
> > > +#include <media/v4l2-subdev.h>
> > > +
> > > +struct device;
> > > +struct v4l2_device;
> > > +struct v4l2_async_notifier;
> > > +
> > > +enum v4l2_async_bus_type {
> > > + V4L2_ASYNC_BUS_SPECIAL,
> > > + V4L2_ASYNC_BUS_PLATFORM,
> > > + V4L2_ASYNC_BUS_I2C,
> > > +};
> > > +
> > > +struct v4l2_async_hw_device {
> > > + enum v4l2_async_bus_type bus_type;
> > > + union {
> > > + struct {
> > > + const char *name;
> > > + } platform;
> > > + struct {
> > > + int adapter_id;
> > > + unsigned short address;
> > > + } i2c;
> > > + struct {
> > > + bool (*match)(struct device *,
> > > + struct v4l2_async_hw_device *);
> > > + void *priv;
> > > + } special;
> > > + } match;
> > > +};
> > > +
> > > +/**
> > > + * struct v4l2_async_subdev - sub-device descriptor, as known to a
> > > bridge
> > > + * @hw: this device descriptor
> > > + * @list: member in a list of subdevices
> > > + */
> > > +struct v4l2_async_subdev {
> > > + struct v4l2_async_hw_device hw;
> > > + struct list_head list;
> > > +};
> > > +
> > > +/**
> > > + * v4l2_async_subdev_list - provided by subdevices
> > > + * @list: member in a list of subdevices
> > > + * @dev: hardware device
> > > + * @subdev: V4L2 subdevice
> > > + * @asd: pointer to respective struct v4l2_async_subdev
> > > + * @notifier: pointer to managing notifier
> > > + */
> > > +struct v4l2_async_subdev_list {
> > > + struct list_head list;
> > > + struct device *dev;
> > > + struct v4l2_subdev *subdev;
> > > + struct v4l2_async_subdev *asd;
> > > + struct v4l2_async_notifier *notifier;
> > > +};
> > > +
> > > +/**
> > > + * v4l2_async_notifier - provided by bridges
> > > + * @subdev_num: number of subdevices
> > > + * @subdev: array of pointers to subdevices
> > > + * @v4l2_dev: pointer to sruct v4l2_device
> > > + * @waiting: list of subdevices, waiting for their drivers
> > > + * @done: list of subdevices, already probed
> > > + * @list: member in a global list of notifiers
> > > + * @bind: a subdevice driver is about to probe one of your subdevices
> > > + * @bound: a subdevice driver has successfully probed one of your
> > > subdevices + * @complete: all your subdevices have been probed
> > > successfully
> > > + * @unbind: a subdevice is leaving
> > > + */
> > > +struct v4l2_async_notifier {
> > > + int subdev_num;
> > > + struct v4l2_async_subdev **subdev;
> > > + struct v4l2_device *v4l2_dev;
> > > + struct list_head waiting;
> > > + struct list_head done;
> > > + struct list_head list;
> > > + int (*bind)(struct v4l2_async_notifier *notifier,
> > > + struct v4l2_async_subdev_list *asdl);
> > > + int (*bound)(struct v4l2_async_notifier *notifier,
> > > + struct v4l2_async_subdev_list *asdl);
> > > + int (*complete)(struct v4l2_async_notifier *notifier);
> > > + void (*unbind)(struct v4l2_async_notifier *notifier,
> > > + struct v4l2_async_subdev_list *asdl);
> > > +};
> > > +
> > > +int v4l2_async_notifier_register(struct v4l2_device *v4l2_dev,
> > > + struct v4l2_async_notifier *notifier);
> > > +void v4l2_async_notifier_unregister(struct v4l2_async_notifier
> > > *notifier);
> > > +/*
> > > + * If subdevice probing fails any time after v4l2_async_subdev_bind(),
> > > no
> > > + * clean up must be called. This function is only a message of
> > > intention.
> > > + */
> > > +int v4l2_async_subdev_bind(struct v4l2_async_subdev_list *asdl);
> > > +int v4l2_async_subdev_bound(struct v4l2_async_subdev_list *asdl);
> >
> > Could you please explain why you need both a bind notifier and a bound
> > notifier ? I was expecting a single v4l2_async_subdev_register() call in
> > subdev drivers (and, thinking about it, I would probably name it
> > v4l2_subdev_register()).
>
> I think I can, yes. Because between .bind() and .bound() the subdevice
> driver does the actual hardware probing. So, .bind() is used to make sure
> the hardware can be accessed, most importantly to provide a clock to the
> subdevice. You can look at soc_camera_async_bind(). There I'm registering
> the clock for the subdevice, about to bind. Why I cannot do it before, is
> because I need subdevice name for clock matching. With I2C subdevices the
> subdevice name contains the name of the driver, adapter number and i2c
> address. The latter 2 I've got from host subdevice list. But not the
> driver name. I thought about also passing the driver name there, but that
> seemed too limiting to me. I also request regulators there, because before
> ->bound() the sensor driver, but that could be done on the first call to
> soc_camera_power_on(), although doing this "first call" thingie is kind of
> hackish too. I could add one more soc-camera-power helper like
> soc_camera_prepare() or similar too.
I think a soc_camera_power_init() function (or similar) would be a good idea,
yes.
> So, the main problem is the clock
> subdevice name. Also see the comment in soc_camera.c:
>
> /*
> * It is ok to keep the clock for the whole soc_camera_device life-time,
> * in principle it would be more logical to register the clock on icd
> * creation, the only problem is, that at that time we don't know the
> * driver name yet.
> */
I think we should fix that problem instead of shaping the async API around a
workaround :-)
From the subdevice point of view, the probe function should request resources,
perform whatever initialization is needed (including verifying that the
hardware is functional when possible), and the register the subdev with the
code if everything succeeded. Splitting registration into bind() and bound()
appears a bit as a workaround to me.
If we need a workaround, I'd rather pass the device name in addition to the
I2C adapter number and address, instead of embedding the workaround in this
new API.
> > > +void v4l2_async_subdev_unbind(struct v4l2_async_subdev_list *asdl);
> > > +#endif
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 1/6 v4] media: V4L2: support asynchronous subdevice registration
2013-01-08 9:41 ` Laurent Pinchart
@ 2013-01-08 9:56 ` Guennadi Liakhovetski
2013-01-08 10:21 ` Laurent Pinchart
0 siblings, 1 reply; 31+ messages in thread
From: Guennadi Liakhovetski @ 2013-01-08 9:56 UTC (permalink / raw)
To: Laurent Pinchart
Cc: linux-media, Hans Verkuil, Sylwester Nawrocki, Sylwester Nawrocki,
linux-sh, Magnus Damm, Sakari Ailus, Prabhakar Lad
On Tue, 8 Jan 2013, Laurent Pinchart wrote:
> Hi Guennadi,
>
> On Tuesday 08 January 2013 10:25:15 Guennadi Liakhovetski wrote:
> > On Tue, 8 Jan 2013, Laurent Pinchart wrote:
> > > On Monday 07 January 2013 11:23:55 Guennadi Liakhovetski wrote:
> > > > >From 0e1eae338ba898dc25ec60e3dba99e5581edc199 Mon Sep 17 00:00:00 2001
[snip]
> > > > +int v4l2_async_notifier_register(struct v4l2_device *v4l2_dev,
> > > > + struct v4l2_async_notifier *notifier);
> > > > +void v4l2_async_notifier_unregister(struct v4l2_async_notifier
> > > > *notifier);
> > > > +/*
> > > > + * If subdevice probing fails any time after v4l2_async_subdev_bind(),
> > > > no
> > > > + * clean up must be called. This function is only a message of
> > > > intention.
> > > > + */
> > > > +int v4l2_async_subdev_bind(struct v4l2_async_subdev_list *asdl);
> > > > +int v4l2_async_subdev_bound(struct v4l2_async_subdev_list *asdl);
> > >
> > > Could you please explain why you need both a bind notifier and a bound
> > > notifier ? I was expecting a single v4l2_async_subdev_register() call in
> > > subdev drivers (and, thinking about it, I would probably name it
> > > v4l2_subdev_register()).
> >
> > I think I can, yes. Because between .bind() and .bound() the subdevice
> > driver does the actual hardware probing. So, .bind() is used to make sure
> > the hardware can be accessed, most importantly to provide a clock to the
> > subdevice. You can look at soc_camera_async_bind(). There I'm registering
> > the clock for the subdevice, about to bind. Why I cannot do it before, is
> > because I need subdevice name for clock matching. With I2C subdevices the
> > subdevice name contains the name of the driver, adapter number and i2c
> > address. The latter 2 I've got from host subdevice list. But not the
> > driver name. I thought about also passing the driver name there, but that
> > seemed too limiting to me. I also request regulators there, because before
> > ->bound() the sensor driver, but that could be done on the first call to
> > soc_camera_power_on(), although doing this "first call" thingie is kind of
> > hackish too. I could add one more soc-camera-power helper like
> > soc_camera_prepare() or similar too.
>
> I think a soc_camera_power_init() function (or similar) would be a good idea,
> yes.
>
> > So, the main problem is the clock
> > subdevice name. Also see the comment in soc_camera.c:
> >
> > /*
> > * It is ok to keep the clock for the whole soc_camera_device life-time,
> > * in principle it would be more logical to register the clock on icd
> > * creation, the only problem is, that at that time we don't know the
> > * driver name yet.
> > */
>
> I think we should fix that problem instead of shaping the async API around a
> workaround :-)
>
> >From the subdevice point of view, the probe function should request resources,
> perform whatever initialization is needed (including verifying that the
> hardware is functional when possible), and the register the subdev with the
> code if everything succeeded. Splitting registration into bind() and bound()
> appears a bit as a workaround to me.
>
> If we need a workaround, I'd rather pass the device name in addition to the
> I2C adapter number and address, instead of embedding the workaround in this
> new API.
...or we can change the I2C subdevice name format. The actual need to do
snprintf(clk_name, sizeof(clk_name), "%s %d-%04x",
asdl->dev->driver->name,
i2c_adapter_id(client->adapter), client->addr);
in soc-camera now to exactly match the subdevice name, as created by
v4l2_i2c_subdev_init(), doesn't make me specifically happy either. What if
the latter changes at some point? Or what if one driver wishes to create
several subdevices for one I2C device?
Thanks
Guennadi
> > > > +void v4l2_async_subdev_unbind(struct v4l2_async_subdev_list *asdl);
> > > > +#endif
>
> --
> Regards,
>
> Laurent Pinchart
>
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH 1/6 v5] media: V4L2: support asynchronous subdevice registration
2013-01-07 10:23 ` [PATCH 1/6 v4] " Guennadi Liakhovetski
2013-01-08 8:10 ` Laurent Pinchart
@ 2013-01-08 10:06 ` Guennadi Liakhovetski
2013-01-10 5:42 ` Prabhakar Lad
1 sibling, 1 reply; 31+ messages in thread
From: Guennadi Liakhovetski @ 2013-01-08 10:06 UTC (permalink / raw)
To: linux-media
Cc: Laurent Pinchart, Hans Verkuil, Sylwester Nawrocki,
Sylwester Nawrocki, linux-sh, Magnus Damm, Sakari Ailus,
Prabhakar Lad
Currently bridge device drivers register devices for all subdevices
synchronously, tupically, during their probing. E.g. if an I2C CMOS sensor
is attached to a video bridge device, the bridge driver will create an I2C
device and wait for the respective I2C driver to probe. This makes linking
of devices straight forward, but this approach cannot be used with
intrinsically asynchronous and unordered device registration systems like
the Flattened Device Tree. To support such systems this patch adds an
asynchronous subdevice registration framework to V4L2. To use it respective
(e.g. I2C) subdevice drivers must request deferred probing as long as their
bridge driver hasn't probed. The bridge driver during its probing submits a
an arbitrary number of subdevice descriptor groups to the framework to
manage. After that it can add callbacks to each of those groups to be
called at various stages during subdevice probing, e.g. after completion.
Then the bridge driver can request single groups to be probed, finish its
own probing and continue its video subsystem configuration from its
callbacks.
Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
---
v5: Now really fix the case, when subdevices probe successfully before the
bridge, thanks to Prabhakar for testing and reporting
drivers/media/v4l2-core/Makefile | 3 +-
drivers/media/v4l2-core/v4l2-async.c | 294 ++++++++++++++++++++++++++++++++++
include/media/v4l2-async.h | 113 +++++++++++++
3 files changed, 409 insertions(+), 1 deletions(-)
create mode 100644 drivers/media/v4l2-core/v4l2-async.c
create mode 100644 include/media/v4l2-async.h
diff --git a/drivers/media/v4l2-core/Makefile b/drivers/media/v4l2-core/Makefile
index d065c01..b667ced 100644
--- a/drivers/media/v4l2-core/Makefile
+++ b/drivers/media/v4l2-core/Makefile
@@ -5,7 +5,8 @@
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-clk.o
+ v4l2-event.o v4l2-ctrls.o v4l2-subdev.o v4l2-clk.o \
+ v4l2-async.o
ifeq ($(CONFIG_COMPAT),y)
videodev-objs += v4l2-compat-ioctl32.o
endif
diff --git a/drivers/media/v4l2-core/v4l2-async.c b/drivers/media/v4l2-core/v4l2-async.c
new file mode 100644
index 0000000..434c53d
--- /dev/null
+++ b/drivers/media/v4l2-core/v4l2-async.c
@@ -0,0 +1,294 @@
+/*
+ * V4L2 asynchronous subdevice registration API
+ *
+ * Copyright (C) 2012, 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/device.h>
+#include <linux/err.h>
+#include <linux/i2c.h>
+#include <linux/list.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/notifier.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+#include <linux/types.h>
+
+#include <media/v4l2-async.h>
+#include <media/v4l2-device.h>
+#include <media/v4l2-subdev.h>
+
+static bool match_i2c(struct device *dev, struct v4l2_async_hw_device *hw_dev)
+{
+ struct i2c_client *client = to_i2c_client(dev);
+ return hw_dev->bus_type = V4L2_ASYNC_BUS_I2C &&
+ hw_dev->match.i2c.adapter_id = client->adapter->nr &&
+ hw_dev->match.i2c.address = client->addr;
+}
+
+static bool match_platform(struct device *dev, struct v4l2_async_hw_device *hw_dev)
+{
+ return hw_dev->bus_type = V4L2_ASYNC_BUS_PLATFORM &&
+ !strcmp(hw_dev->match.platform.name, dev_name(dev));
+}
+
+static LIST_HEAD(subdev_list);
+static LIST_HEAD(notifier_list);
+static DEFINE_MUTEX(list_lock);
+
+static struct v4l2_async_subdev *v4l2_async_belongs(struct v4l2_async_notifier *notifier,
+ struct v4l2_async_subdev_list *asdl)
+{
+ struct v4l2_async_subdev *asd = NULL;
+ bool (*match)(struct device *,
+ struct v4l2_async_hw_device *);
+
+ list_for_each_entry (asd, ¬ifier->waiting, list) {
+ struct v4l2_async_hw_device *hw = &asd->hw;
+ switch (hw->bus_type) {
+ case V4L2_ASYNC_BUS_SPECIAL:
+ match = hw->match.special.match;
+ if (!match)
+ /* Match always */
+ return asd;
+ break;
+ case V4L2_ASYNC_BUS_PLATFORM:
+ match = match_platform;
+ break;
+ case V4L2_ASYNC_BUS_I2C:
+ match = match_i2c;
+ break;
+ default:
+ /* Oops */
+ match = NULL;
+ dev_err(notifier->v4l2_dev ? notifier->v4l2_dev->dev : NULL,
+ "Invalid bus-type %u on %p\n", hw->bus_type, asd);
+ }
+
+ if (match && match(asdl->dev, hw))
+ break;
+ }
+
+ return asd;
+}
+
+static int v4l2_async_test_notify(struct v4l2_async_notifier *notifier,
+ struct v4l2_async_subdev_list *asdl,
+ struct v4l2_async_subdev *asd)
+{
+ int ret;
+
+ /* Remove from the waiting list */
+ list_del(&asd->list);
+ asdl->asd = asd;
+ asdl->notifier = notifier;
+
+ if (notifier->bound) {
+ ret = notifier->bound(notifier, asdl);
+ if (ret < 0)
+ return ret;
+ }
+ /* Move from the global subdevice list to notifier's done */
+ list_move(&asdl->list, ¬ifier->done);
+
+ ret = v4l2_device_register_subdev(notifier->v4l2_dev,
+ asdl->subdev);
+ if (ret < 0) {
+ if (notifier->unbind)
+ notifier->unbind(notifier, asdl);
+ return ret;
+ }
+
+ if (list_empty(¬ifier->waiting) && notifier->complete)
+ return notifier->complete(notifier);
+
+ return 0;
+}
+
+static struct device *v4l2_async_unbind(struct v4l2_async_subdev_list *asdl)
+{
+ struct device *dev = asdl->dev;
+ v4l2_device_unregister_subdev(asdl->subdev);
+ /* Subdevice driver will reprobe and put asdl back onto the list */
+ list_del(&asdl->list);
+ asdl->asd = NULL;
+ /* If we handled USB devices, we'd have to lock the parent too */
+ device_release_driver(dev);
+ return dev;
+}
+
+int v4l2_async_notifier_register(struct v4l2_device *v4l2_dev,
+ struct v4l2_async_notifier *notifier)
+{
+ struct v4l2_async_subdev_list *asdl, *tmp;
+ int i;
+
+ notifier->v4l2_dev = v4l2_dev;
+ INIT_LIST_HEAD(¬ifier->waiting);
+ INIT_LIST_HEAD(¬ifier->done);
+
+ for (i = 0; i < notifier->subdev_num; i++)
+ list_add_tail(¬ifier->subdev[i]->list, ¬ifier->waiting);
+
+ mutex_lock(&list_lock);
+
+ /* Keep also completed notifiers on the list */
+ list_add(¬ifier->list, ¬ifier_list);
+
+ list_for_each_entry_safe(asdl, tmp, &subdev_list, list) {
+ struct v4l2_async_subdev *asd = v4l2_async_belongs(notifier, asdl);
+ int ret;
+
+ if (!asd)
+ continue;
+
+ if (notifier->bind) {
+ ret = notifier->bind(notifier, asdl);
+ if (ret < 0)
+ continue;
+ }
+
+ ret = v4l2_async_test_notify(notifier, asdl, asd);
+ if (ret < 0) {
+ mutex_unlock(&list_lock);
+ return ret;
+ }
+ }
+
+ mutex_unlock(&list_lock);
+
+ return 0;
+}
+EXPORT_SYMBOL(v4l2_async_notifier_register);
+
+void v4l2_async_notifier_unregister(struct v4l2_async_notifier *notifier)
+{
+ struct v4l2_async_subdev_list *asdl, *tmp;
+ int i = 0;
+ struct device **dev = kcalloc(notifier->subdev_num,
+ sizeof(*dev), GFP_KERNEL);
+ if (!dev)
+ dev_err(notifier->v4l2_dev->dev,
+ "Failed to allocate device cache!\n");
+
+ mutex_lock(&list_lock);
+
+ list_del(¬ifier->list);
+
+ list_for_each_entry_safe(asdl, tmp, ¬ifier->done, list) {
+ if (dev)
+ dev[i++] = get_device(asdl->dev);
+ v4l2_async_unbind(asdl);
+
+ if (notifier->unbind)
+ notifier->unbind(notifier, asdl);
+ }
+
+ mutex_unlock(&list_lock);
+
+ if (dev) {
+ while (i--) {
+ if (dev[i] && device_attach(dev[i]) < 0)
+ dev_err(dev[i], "Failed to re-probe to %s\n",
+ dev[i]->driver ? dev[i]->driver->name : "(none)");
+ put_device(dev[i]);
+ }
+ kfree(dev);
+ }
+ /*
+ * Don't care about the waiting list, it is initialised and populated
+ * upon notifier registration.
+ */
+}
+EXPORT_SYMBOL(v4l2_async_notifier_unregister);
+
+int v4l2_async_subdev_bind(struct v4l2_async_subdev_list *asdl)
+{
+ struct v4l2_async_notifier *notifier;
+ int ret = 0;
+
+ mutex_lock(&list_lock);
+
+ list_for_each_entry(notifier, ¬ifier_list, list) {
+ struct v4l2_async_subdev *asd = v4l2_async_belongs(notifier,
+ asdl);
+ /*
+ * Whether or not probing succeeds - this is the right hardware
+ * subdevice descriptor and we can provide it to the notifier
+ */
+ if (asd) {
+ asdl->asd = asd;
+ if (notifier->bind)
+ ret = notifier->bind(notifier, asdl);
+ break;
+ }
+ }
+
+ mutex_unlock(&list_lock);
+
+ return ret;
+}
+EXPORT_SYMBOL(v4l2_async_subdev_bind);
+
+int v4l2_async_subdev_bound(struct v4l2_async_subdev_list *asdl)
+{
+ struct v4l2_async_notifier *notifier;
+
+ mutex_lock(&list_lock);
+
+ INIT_LIST_HEAD(&asdl->list);
+
+ list_for_each_entry(notifier, ¬ifier_list, list) {
+ struct v4l2_async_subdev *asd = v4l2_async_belongs(notifier, asdl);
+ if (asd) {
+ int ret = v4l2_async_test_notify(notifier, asdl, asd);
+ mutex_unlock(&list_lock);
+ return ret;
+ }
+ }
+
+ /* None matched, wait for hot-plugging */
+ list_add(&asdl->list, &subdev_list);
+
+ mutex_unlock(&list_lock);
+
+ return 0;
+}
+EXPORT_SYMBOL(v4l2_async_subdev_bound);
+
+void v4l2_async_subdev_unbind(struct v4l2_async_subdev_list *asdl)
+{
+ struct v4l2_async_notifier *notifier = asdl->notifier;
+ struct device *dev;
+
+ if (!asdl->asd)
+ return;
+
+ mutex_lock(&list_lock);
+
+ dev = asdl->dev;
+
+ list_add(&asdl->asd->list, ¬ifier->waiting);
+
+ dev = get_device(asdl->dev);
+
+ v4l2_async_unbind(asdl);
+
+ if (notifier->unbind)
+ notifier->unbind(notifier, asdl);
+
+ mutex_unlock(&list_lock);
+
+ /* Re-probe with lock released - avoid a deadlock */
+ if (dev && device_attach(dev) < 0)
+ dev_err(dev, "Failed to re-probe to %s\n",
+ dev->driver ? dev->driver->name : "(none)");
+
+ put_device(dev);
+}
+EXPORT_SYMBOL(v4l2_async_subdev_unbind);
diff --git a/include/media/v4l2-async.h b/include/media/v4l2-async.h
new file mode 100644
index 0000000..91d436d
--- /dev/null
+++ b/include/media/v4l2-async.h
@@ -0,0 +1,113 @@
+/*
+ * V4L2 asynchronous subdevice registration API
+ *
+ * Copyright (C) 2012, 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.
+ */
+
+#ifndef V4L2_ASYNC_H
+#define V4L2_ASYNC_H
+
+#include <linux/list.h>
+#include <linux/mutex.h>
+#include <linux/notifier.h>
+
+#include <media/v4l2-subdev.h>
+
+struct device;
+struct v4l2_device;
+struct v4l2_async_notifier;
+
+enum v4l2_async_bus_type {
+ V4L2_ASYNC_BUS_SPECIAL,
+ V4L2_ASYNC_BUS_PLATFORM,
+ V4L2_ASYNC_BUS_I2C,
+};
+
+struct v4l2_async_hw_device {
+ enum v4l2_async_bus_type bus_type;
+ union {
+ struct {
+ const char *name;
+ } platform;
+ struct {
+ int adapter_id;
+ unsigned short address;
+ } i2c;
+ struct {
+ bool (*match)(struct device *,
+ struct v4l2_async_hw_device *);
+ void *priv;
+ } special;
+ } match;
+};
+
+/**
+ * struct v4l2_async_subdev - sub-device descriptor, as known to a bridge
+ * @hw: this device descriptor
+ * @list: member in a list of subdevices
+ */
+struct v4l2_async_subdev {
+ struct v4l2_async_hw_device hw;
+ struct list_head list;
+};
+
+/**
+ * v4l2_async_subdev_list - provided by subdevices
+ * @list: member in a list of subdevices
+ * @dev: hardware device
+ * @subdev: V4L2 subdevice
+ * @asd: pointer to respective struct v4l2_async_subdev
+ * @notifier: pointer to managing notifier
+ */
+struct v4l2_async_subdev_list {
+ struct list_head list;
+ struct device *dev;
+ struct v4l2_subdev *subdev;
+ struct v4l2_async_subdev *asd;
+ struct v4l2_async_notifier *notifier;
+};
+
+/**
+ * v4l2_async_notifier - provided by bridges
+ * @subdev_num: number of subdevices
+ * @subdev: array of pointers to subdevices
+ * @v4l2_dev: pointer to sruct v4l2_device
+ * @waiting: list of subdevices, waiting for their drivers
+ * @done: list of subdevices, already probed
+ * @list: member in a global list of notifiers
+ * @bind: a subdevice driver is about to probe one of your subdevices
+ * @bound: a subdevice driver has successfully probed one of your subdevices
+ * @complete: all your subdevices have been probed successfully
+ * @unbind: a subdevice is leaving
+ */
+struct v4l2_async_notifier {
+ int subdev_num;
+ struct v4l2_async_subdev **subdev;
+ struct v4l2_device *v4l2_dev;
+ struct list_head waiting;
+ struct list_head done;
+ struct list_head list;
+ int (*bind)(struct v4l2_async_notifier *notifier,
+ struct v4l2_async_subdev_list *asdl);
+ int (*bound)(struct v4l2_async_notifier *notifier,
+ struct v4l2_async_subdev_list *asdl);
+ int (*complete)(struct v4l2_async_notifier *notifier);
+ void (*unbind)(struct v4l2_async_notifier *notifier,
+ struct v4l2_async_subdev_list *asdl);
+};
+
+int v4l2_async_notifier_register(struct v4l2_device *v4l2_dev,
+ struct v4l2_async_notifier *notifier);
+void v4l2_async_notifier_unregister(struct v4l2_async_notifier *notifier);
+/*
+ * If subdevice probing fails any time after v4l2_async_subdev_bind(), no clean
+ * up must be called. This function is only a message of intention.
+ */
+int v4l2_async_subdev_bind(struct v4l2_async_subdev_list *asdl);
+int v4l2_async_subdev_bound(struct v4l2_async_subdev_list *asdl);
+void v4l2_async_subdev_unbind(struct v4l2_async_subdev_list *asdl);
+#endif
--
1.7.2.5
^ permalink raw reply related [flat|nested] 31+ messages in thread
* Re: [PATCH 1/6 v4] media: V4L2: support asynchronous subdevice registration
2013-01-08 9:56 ` Guennadi Liakhovetski
@ 2013-01-08 10:21 ` Laurent Pinchart
2013-01-08 10:26 ` Guennadi Liakhovetski
0 siblings, 1 reply; 31+ messages in thread
From: Laurent Pinchart @ 2013-01-08 10:21 UTC (permalink / raw)
To: Guennadi Liakhovetski
Cc: linux-media, Hans Verkuil, Sylwester Nawrocki, Sylwester Nawrocki,
linux-sh, Magnus Damm, Sakari Ailus, Prabhakar Lad
Hi Guennadi,
On Tuesday 08 January 2013 10:56:43 Guennadi Liakhovetski wrote:
> On Tue, 8 Jan 2013, Laurent Pinchart wrote:
> > On Tuesday 08 January 2013 10:25:15 Guennadi Liakhovetski wrote:
> > > On Tue, 8 Jan 2013, Laurent Pinchart wrote:
> > > > On Monday 07 January 2013 11:23:55 Guennadi Liakhovetski wrote:
> > > > > >From 0e1eae338ba898dc25ec60e3dba99e5581edc199 Mon Sep 17 00:00:00
> > > > > >2001
>
> [snip]
>
> > > > > +int v4l2_async_notifier_register(struct v4l2_device *v4l2_dev,
> > > > > + struct v4l2_async_notifier *notifier);
> > > > > +void v4l2_async_notifier_unregister(struct v4l2_async_notifier
> > > > > *notifier);
> > > > > +/*
> > > > > + * If subdevice probing fails any time after
> > > > > v4l2_async_subdev_bind(),
> > > > > no
> > > > > + * clean up must be called. This function is only a message of
> > > > > intention.
> > > > > + */
> > > > > +int v4l2_async_subdev_bind(struct v4l2_async_subdev_list *asdl);
> > > > > +int v4l2_async_subdev_bound(struct v4l2_async_subdev_list *asdl);
> > > >
> > > > Could you please explain why you need both a bind notifier and a bound
> > > > notifier ? I was expecting a single v4l2_async_subdev_register() call
> > > > in subdev drivers (and, thinking about it, I would probably name it
> > > > v4l2_subdev_register()).
> > >
> > > I think I can, yes. Because between .bind() and .bound() the subdevice
> > > driver does the actual hardware probing. So, .bind() is used to make
> > > sure the hardware can be accessed, most importantly to provide a clock
> > > to the subdevice. You can look at soc_camera_async_bind(). There I'm
> > > registering the clock for the subdevice, about to bind. Why I cannot do
> > > it before, is because I need subdevice name for clock matching. With I2C
> > > subdevices the subdevice name contains the name of the driver, adapter
> > > number and i2c address. The latter 2 I've got from host subdevice list.
> > > But not the driver name. I thought about also passing the driver name
> > > there, but that seemed too limiting to me. I also request regulators
> > > there, because before ->bound() the sensor driver, but that could be
> > > done on the first call to soc_camera_power_on(), although doing this
> > > "first call" thingie is kind of hackish too. I could add one more soc-
> > > camera-power helper like soc_camera_prepare() or similar too.
> >
> > I think a soc_camera_power_init() function (or similar) would be a good
> > idea, yes.
> >
> > > So, the main problem is the clock
> > >
> > > subdevice name. Also see the comment in soc_camera.c:
> > > /*
> > > * It is ok to keep the clock for the whole soc_camera_device
> > > life-time,
> > > * in principle it would be more logical to register the clock on icd
> > > * creation, the only problem is, that at that time we don't know the
> > > * driver name yet.
> > > */
> >
> > I think we should fix that problem instead of shaping the async API around
> > a workaround :-)
> >
> > From the subdevice point of view, the probe function should request
> > resources, perform whatever initialization is needed (including verifying
> > that the hardware is functional when possible), and the register the
> > subdev with the code if everything succeeded. Splitting registration into
> > bind() and bound() appears a bit as a workaround to me.
> >
> > If we need a workaround, I'd rather pass the device name in addition to
> > the I2C adapter number and address, instead of embedding the workaround in
> > this new API.
>
> ...or we can change the I2C subdevice name format. The actual need to do
>
> snprintf(clk_name, sizeof(clk_name), "%s %d-%04x",
> asdl->dev->driver->name,
> i2c_adapter_id(client->adapter), client->addr);
>
> in soc-camera now to exactly match the subdevice name, as created by
> v4l2_i2c_subdev_init(), doesn't make me specifically happy either. What if
> the latter changes at some point? Or what if one driver wishes to create
> several subdevices for one I2C device?
The common clock framework uses %d-%04x, maybe we could use that as well for
clock names ?
> > > > > +void v4l2_async_subdev_unbind(struct v4l2_async_subdev_list *asdl);
> > > > > +#endif
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 1/6 v4] media: V4L2: support asynchronous subdevice registration
2013-01-08 10:21 ` Laurent Pinchart
@ 2013-01-08 10:26 ` Guennadi Liakhovetski
2013-01-08 10:35 ` Laurent Pinchart
2013-01-08 19:26 ` Sylwester Nawrocki
0 siblings, 2 replies; 31+ messages in thread
From: Guennadi Liakhovetski @ 2013-01-08 10:26 UTC (permalink / raw)
To: Laurent Pinchart
Cc: linux-media, Hans Verkuil, Sylwester Nawrocki, Sylwester Nawrocki,
linux-sh, Magnus Damm, Sakari Ailus, Prabhakar Lad
On Tue, 8 Jan 2013, Laurent Pinchart wrote:
> Hi Guennadi,
>
> On Tuesday 08 January 2013 10:56:43 Guennadi Liakhovetski wrote:
> > On Tue, 8 Jan 2013, Laurent Pinchart wrote:
> > > On Tuesday 08 January 2013 10:25:15 Guennadi Liakhovetski wrote:
> > > > On Tue, 8 Jan 2013, Laurent Pinchart wrote:
> > > > > On Monday 07 January 2013 11:23:55 Guennadi Liakhovetski wrote:
> > > > > > >From 0e1eae338ba898dc25ec60e3dba99e5581edc199 Mon Sep 17 00:00:00
> > > > > > >2001
> >
> > [snip]
> >
> > > > > > +int v4l2_async_notifier_register(struct v4l2_device *v4l2_dev,
> > > > > > + struct v4l2_async_notifier *notifier);
> > > > > > +void v4l2_async_notifier_unregister(struct v4l2_async_notifier
> > > > > > *notifier);
> > > > > > +/*
> > > > > > + * If subdevice probing fails any time after
> > > > > > v4l2_async_subdev_bind(),
> > > > > > no
> > > > > > + * clean up must be called. This function is only a message of
> > > > > > intention.
> > > > > > + */
> > > > > > +int v4l2_async_subdev_bind(struct v4l2_async_subdev_list *asdl);
> > > > > > +int v4l2_async_subdev_bound(struct v4l2_async_subdev_list *asdl);
> > > > >
> > > > > Could you please explain why you need both a bind notifier and a bound
> > > > > notifier ? I was expecting a single v4l2_async_subdev_register() call
> > > > > in subdev drivers (and, thinking about it, I would probably name it
> > > > > v4l2_subdev_register()).
> > > >
> > > > I think I can, yes. Because between .bind() and .bound() the subdevice
> > > > driver does the actual hardware probing. So, .bind() is used to make
> > > > sure the hardware can be accessed, most importantly to provide a clock
> > > > to the subdevice. You can look at soc_camera_async_bind(). There I'm
> > > > registering the clock for the subdevice, about to bind. Why I cannot do
> > > > it before, is because I need subdevice name for clock matching. With I2C
> > > > subdevices the subdevice name contains the name of the driver, adapter
> > > > number and i2c address. The latter 2 I've got from host subdevice list.
> > > > But not the driver name. I thought about also passing the driver name
> > > > there, but that seemed too limiting to me. I also request regulators
> > > > there, because before ->bound() the sensor driver, but that could be
> > > > done on the first call to soc_camera_power_on(), although doing this
> > > > "first call" thingie is kind of hackish too. I could add one more soc-
> > > > camera-power helper like soc_camera_prepare() or similar too.
> > >
> > > I think a soc_camera_power_init() function (or similar) would be a good
> > > idea, yes.
> > >
> > > > So, the main problem is the clock
> > > >
> > > > subdevice name. Also see the comment in soc_camera.c:
> > > > /*
> > > > * It is ok to keep the clock for the whole soc_camera_device
> > > > life-time,
> > > > * in principle it would be more logical to register the clock on icd
> > > > * creation, the only problem is, that at that time we don't know the
> > > > * driver name yet.
> > > > */
> > >
> > > I think we should fix that problem instead of shaping the async API around
> > > a workaround :-)
> > >
> > > From the subdevice point of view, the probe function should request
> > > resources, perform whatever initialization is needed (including verifying
> > > that the hardware is functional when possible), and the register the
> > > subdev with the code if everything succeeded. Splitting registration into
> > > bind() and bound() appears a bit as a workaround to me.
> > >
> > > If we need a workaround, I'd rather pass the device name in addition to
> > > the I2C adapter number and address, instead of embedding the workaround in
> > > this new API.
> >
> > ...or we can change the I2C subdevice name format. The actual need to do
> >
> > snprintf(clk_name, sizeof(clk_name), "%s %d-%04x",
> > asdl->dev->driver->name,
> > i2c_adapter_id(client->adapter), client->addr);
> >
> > in soc-camera now to exactly match the subdevice name, as created by
> > v4l2_i2c_subdev_init(), doesn't make me specifically happy either. What if
> > the latter changes at some point? Or what if one driver wishes to create
> > several subdevices for one I2C device?
>
> The common clock framework uses %d-%04x, maybe we could use that as well for
> clock names ?
And preserve the subdevice names? Then matching would be more difficult
and less precise. Or change subdevice names too? I think, we can do the
latter, since anyway at any time only one driver can be attached to an I2C
device.
> > > > > > +void v4l2_async_subdev_unbind(struct v4l2_async_subdev_list *asdl);
> > > > > > +#endif
Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 1/6 v4] media: V4L2: support asynchronous subdevice registration
2013-01-08 10:26 ` Guennadi Liakhovetski
@ 2013-01-08 10:35 ` Laurent Pinchart
2013-01-08 11:37 ` Sylwester Nawrocki
2013-01-08 19:26 ` Sylwester Nawrocki
1 sibling, 1 reply; 31+ messages in thread
From: Laurent Pinchart @ 2013-01-08 10:35 UTC (permalink / raw)
To: Guennadi Liakhovetski
Cc: linux-media, Hans Verkuil, Sylwester Nawrocki, Sylwester Nawrocki,
linux-sh, Magnus Damm, Sakari Ailus, Prabhakar Lad
On Tuesday 08 January 2013 11:26:57 Guennadi Liakhovetski wrote:
> On Tue, 8 Jan 2013, Laurent Pinchart wrote:
> > On Tuesday 08 January 2013 10:56:43 Guennadi Liakhovetski wrote:
> > > On Tue, 8 Jan 2013, Laurent Pinchart wrote:
> > > > On Tuesday 08 January 2013 10:25:15 Guennadi Liakhovetski wrote:
> > > > > On Tue, 8 Jan 2013, Laurent Pinchart wrote:
[snip]
> > > > > > Could you please explain why you need both a bind notifier and a
> > > > > > bound notifier ? I was expecting a single
> > > > > > v4l2_async_subdev_register() call in subdev drivers (and, thinking
> > > > > > about it, I would probably name it v4l2_subdev_register()).
> > > > >
> > > > > I think I can, yes. Because between .bind() and .bound() the
> > > > > subdevice driver does the actual hardware probing. So, .bind() is
> > > > > used to make sure the hardware can be accessed, most importantly to
> > > > > provide a clock to the subdevice. You can look at
> > > > > soc_camera_async_bind(). There I'm registering the clock for the
> > > > > subdevice, about to bind. Why I cannot do it before, is because I
> > > > > need subdevice name for clock matching. With I2C subdevices the
> > > > > subdevice name contains the name of the driver, adapter number and
> > > > > i2c address. The latter 2 I've got from host subdevice list. But not
> > > > > the driver name. I thought about also passing the driver name there,
> > > > > but that seemed too limiting to me. I also request regulators there,
> > > > > because before ->bound() the sensor driver, but that could be done
> > > > > on the first call to soc_camera_power_on(), although doing this
> > > > > "first call" thingie is kind of hackish too. I could add one more
> > > > > soc-camera-power helper like soc_camera_prepare() or similar too.
> > > >
> > > > I think a soc_camera_power_init() function (or similar) would be a
> > > > good idea, yes.
> > > >
> > > > > So, the main problem is the clock
> > > > >
> > > > > subdevice name. Also see the comment in soc_camera.c:
> > > > > /*
> > > > > * It is ok to keep the clock for the whole soc_camera_device
> > > > > * life-time, in principle it would be more logical to register
> > > > > * the clock on icd creation, the only problem is, that at that
> > > > > * time we don't know the driver name yet.
> > > > > */
> > > >
> > > > I think we should fix that problem instead of shaping the async API
> > > > around a workaround :-)
> > > >
> > > > From the subdevice point of view, the probe function should request
> > > > resources, perform whatever initialization is needed (including
> > > > verifying that the hardware is functional when possible), and the
> > > > register the subdev with the code if everything succeeded. Splitting
> > > > registration into bind() and bound() appears a bit as a workaround to
> > > > me.
> > > >
> > > > If we need a workaround, I'd rather pass the device name in addition
> > > > to the I2C adapter number and address, instead of embedding the
> > > > workaround in this new API.
> > >
> > > ...or we can change the I2C subdevice name format. The actual need to do
> > >
> > > snprintf(clk_name, sizeof(clk_name), "%s %d-%04x",
> > >
> > > asdl->dev->driver->name,
> > > i2c_adapter_id(client->adapter), client->addr);
> > >
> > > in soc-camera now to exactly match the subdevice name, as created by
> > > v4l2_i2c_subdev_init(), doesn't make me specifically happy either. What
> > > if the latter changes at some point? Or what if one driver wishes to
> > > create several subdevices for one I2C device?
> >
> > The common clock framework uses %d-%04x, maybe we could use that as well
> > for clock names ?
>
> And preserve the subdevice names? Then matching would be more difficult
> and less precise. Or change subdevice names too? I think, we can do the
> latter, since anyway at any time only one driver can be attached to an I2C
> device.
That's right. Where else is the subdev name used ?
> > > > > > > +void v4l2_async_subdev_unbind(struct v4l2_async_subdev_list
> > > > > > > *asdl);
> > > > > > > +#endif
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 1/6 v4] media: V4L2: support asynchronous subdevice registration
2013-01-08 10:35 ` Laurent Pinchart
@ 2013-01-08 11:37 ` Sylwester Nawrocki
2013-01-08 12:41 ` Laurent Pinchart
0 siblings, 1 reply; 31+ messages in thread
From: Sylwester Nawrocki @ 2013-01-08 11:37 UTC (permalink / raw)
To: Laurent Pinchart
Cc: Guennadi Liakhovetski, linux-media, Hans Verkuil,
Sylwester Nawrocki, linux-sh, Magnus Damm, Sakari Ailus,
Prabhakar Lad
Hi,
On 01/08/2013 11:35 AM, Laurent Pinchart wrote:
>>>>> If we need a workaround, I'd rather pass the device name in addition
>>>>> to the I2C adapter number and address, instead of embedding the
>>>>> workaround in this new API.
>>>>
>>>> ...or we can change the I2C subdevice name format. The actual need to do
>>>>
>>>> snprintf(clk_name, sizeof(clk_name), "%s %d-%04x",
>>>>
>>>> asdl->dev->driver->name,
>>>> i2c_adapter_id(client->adapter), client->addr);
>>>>
>>>> in soc-camera now to exactly match the subdevice name, as created by
>>>> v4l2_i2c_subdev_init(), doesn't make me specifically happy either. What
>>>> if the latter changes at some point? Or what if one driver wishes to
>>>> create several subdevices for one I2C device?
>>>
>>> The common clock framework uses %d-%04x, maybe we could use that as well
>>> for clock names ?
>>
>> And preserve the subdevice names? Then matching would be more difficult
>> and less precise. Or change subdevice names too? I think, we can do the
>> latter, since anyway at any time only one driver can be attached to an I2C
>> device.
>
> That's right. Where else is the subdev name used ?
Subdev names are exposed to user space by the media controller API.
So they are really part of an ABI, aren't they ?
Also having I2C bus number or I2C slave address as part of the subdev
name makes it more difficult to write portable applications. Hence
in sensor drivers I used to overwrite subdev name to remove I2C bus
and slave address, as the format used v4l2_i2c_subdev_init() seemed
highly unsuitable..
--
Regards,
Sylwester
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 1/6 v4] media: V4L2: support asynchronous subdevice registration
2013-01-08 11:37 ` Sylwester Nawrocki
@ 2013-01-08 12:41 ` Laurent Pinchart
2013-01-08 13:15 ` Sakari Ailus
2013-01-08 14:27 ` [PATCH 1/6 v4] media: V4L2: support asynchronous subdevice registration Sylwester Nawrocki
0 siblings, 2 replies; 31+ messages in thread
From: Laurent Pinchart @ 2013-01-08 12:41 UTC (permalink / raw)
To: Sylwester Nawrocki
Cc: Guennadi Liakhovetski, linux-media, Hans Verkuil,
Sylwester Nawrocki, linux-sh, Magnus Damm, Sakari Ailus,
Prabhakar Lad
Hi Sylwester,
On Tuesday 08 January 2013 12:37:36 Sylwester Nawrocki wrote:
> On 01/08/2013 11:35 AM, Laurent Pinchart wrote:
> >>>>> If we need a workaround, I'd rather pass the device name in addition
> >>>>> to the I2C adapter number and address, instead of embedding the
> >>>>> workaround in this new API.
> >>>>
> >>>> ...or we can change the I2C subdevice name format. The actual need to
> >>>> do
> >>>>
> >>>> snprintf(clk_name, sizeof(clk_name), "%s %d-%04x",
> >>>> asdl->dev->driver->name,
> >>>> i2c_adapter_id(client->adapter), client->addr);
> >>>>
> >>>> in soc-camera now to exactly match the subdevice name, as created by
> >>>> v4l2_i2c_subdev_init(), doesn't make me specifically happy either. What
> >>>> if the latter changes at some point? Or what if one driver wishes to
> >>>> create several subdevices for one I2C device?
> >>>
> >>> The common clock framework uses %d-%04x, maybe we could use that as well
> >>> for clock names ?
> >>
> >> And preserve the subdevice names? Then matching would be more difficult
> >> and less precise. Or change subdevice names too? I think, we can do the
> >> latter, since anyway at any time only one driver can be attached to an
> >> I2C device.
> >
> > That's right. Where else is the subdev name used ?
>
> Subdev names are exposed to user space by the media controller API.
> So they are really part of an ABI, aren't they ?
They're used to construct the name exposed to userspace, but the media
controller core could probably handle that internally by concatenating the
driver name and the subdev name.
> Also having I2C bus number or I2C slave address as part of the subdev
> name makes it more difficult to write portable applications. Hence
> in sensor drivers I used to overwrite subdev name to remove I2C bus
> and slave address, as the format used v4l2_i2c_subdev_init() seemed
> highly unsuitable..
This clearly shows that we need to discuss the matter and agree on a common
mode of operation.
Aren't applications that use the subdev name directly inherently non-portable
anyway ? If you want your application to support different boards/sensors/SoCs
you should discover the pipeline and find the sensor by iterating over
entities, instead of using the sensor entity name.
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 1/6 v4] media: V4L2: support asynchronous subdevice registration
2013-01-08 12:41 ` Laurent Pinchart
@ 2013-01-08 13:15 ` Sakari Ailus
2013-01-08 21:17 ` Laurent Pinchart
2013-01-08 14:27 ` [PATCH 1/6 v4] media: V4L2: support asynchronous subdevice registration Sylwester Nawrocki
1 sibling, 1 reply; 31+ messages in thread
From: Sakari Ailus @ 2013-01-08 13:15 UTC (permalink / raw)
To: Laurent Pinchart
Cc: Sylwester Nawrocki, Guennadi Liakhovetski, linux-media,
Hans Verkuil, Sylwester Nawrocki, linux-sh, Magnus Damm,
Prabhakar Lad
Hi Laurent,
On Tue, Jan 08, 2013 at 01:41:59PM +0100, Laurent Pinchart wrote:
> Hi Sylwester,
>
> On Tuesday 08 January 2013 12:37:36 Sylwester Nawrocki wrote:
> > On 01/08/2013 11:35 AM, Laurent Pinchart wrote:
> > >>>>> If we need a workaround, I'd rather pass the device name in addition
> > >>>>> to the I2C adapter number and address, instead of embedding the
> > >>>>> workaround in this new API.
> > >>>>
> > >>>> ...or we can change the I2C subdevice name format. The actual need to
> > >>>> do
> > >>>>
> > >>>> snprintf(clk_name, sizeof(clk_name), "%s %d-%04x",
> > >>>> asdl->dev->driver->name,
> > >>>> i2c_adapter_id(client->adapter), client->addr);
> > >>>>
> > >>>> in soc-camera now to exactly match the subdevice name, as created by
> > >>>> v4l2_i2c_subdev_init(), doesn't make me specifically happy either. What
> > >>>> if the latter changes at some point? Or what if one driver wishes to
> > >>>> create several subdevices for one I2C device?
> > >>>
> > >>> The common clock framework uses %d-%04x, maybe we could use that as well
> > >>> for clock names ?
> > >>
> > >> And preserve the subdevice names? Then matching would be more difficult
> > >> and less precise. Or change subdevice names too? I think, we can do the
> > >> latter, since anyway at any time only one driver can be attached to an
> > >> I2C device.
> > >
> > > That's right. Where else is the subdev name used ?
> >
> > Subdev names are exposed to user space by the media controller API.
> > So they are really part of an ABI, aren't they ?
>
> They're used to construct the name exposed to userspace, but the media
> controller core could probably handle that internally by concatenating the
> driver name and the subdev name.
>
> > Also having I2C bus number or I2C slave address as part of the subdev
> > name makes it more difficult to write portable applications. Hence
> > in sensor drivers I used to overwrite subdev name to remove I2C bus
> > and slave address, as the format used v4l2_i2c_subdev_init() seemed
> > highly unsuitable..
>
> This clearly shows that we need to discuss the matter and agree on a common
> mode of operation.
>
> Aren't applications that use the subdev name directly inherently non-portable
> anyway ? If you want your application to support different boards/sensors/SoCs
Well, the name could come from a configuration file to distinguish e.g.
between primary and secondary sensors.
For what it's worth, the SMIA++ driver uses the actual name of the sensor
since there are about 10 sensors supported at the moment, and calling them
all smiapp-xxxx looks a bit insipid. So one has to talk to the sensor to
know what it's called.
This isn't strictly mandatory but a nice feature.
> you should discover the pipeline and find the sensor by iterating over
> entities, instead of using the sensor entity name.
To be fully generic, yes.
--
Cheers,
Sakari Ailus
e-mail: sakari.ailus@iki.fi XMPP: sailus@retiisi.org.uk
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 1/6 v4] media: V4L2: support asynchronous subdevice registration
2013-01-08 12:41 ` Laurent Pinchart
2013-01-08 13:15 ` Sakari Ailus
@ 2013-01-08 14:27 ` Sylwester Nawrocki
2013-01-08 21:20 ` Laurent Pinchart
1 sibling, 1 reply; 31+ messages in thread
From: Sylwester Nawrocki @ 2013-01-08 14:27 UTC (permalink / raw)
To: Laurent Pinchart
Cc: Sylwester Nawrocki, Guennadi Liakhovetski, linux-media,
Hans Verkuil, Sylwester Nawrocki, linux-sh, Magnus Damm,
Sakari Ailus, Prabhakar Lad
Hi Laurent,
On 01/08/2013 01:41 PM, Laurent Pinchart wrote:
>> Subdev names are exposed to user space by the media controller API.
>> So they are really part of an ABI, aren't they ?
>
> They're used to construct the name exposed to userspace, but the media
> controller core could probably handle that internally by concatenating the
> driver name and the subdev name.
>
>> Also having I2C bus number or I2C slave address as part of the subdev
>> name makes it more difficult to write portable applications. Hence
>> in sensor drivers I used to overwrite subdev name to remove I2C bus
>> and slave address, as the format used v4l2_i2c_subdev_init() seemed
>> highly unsuitable..
>
> This clearly shows that we need to discuss the matter and agree on a common
> mode of operation.
>
> Aren't applications that use the subdev name directly inherently non-portable
> anyway ? If you want your application to support different boards/sensors/SoCs
> you should discover the pipeline and find the sensor by iterating over
> entities, instead of using the sensor entity name.
It depends on how we define the entity names :) It the names change from
board
to board and are completely unreliable then user space applications
using them
have no any chance to be generic. Nevertheless, struct
media_entity_desc::name
[1] has currently no specific semantics defined, e.g. for V4L2.
It's likely way better for the kernel space to be no constrained by the
subdev
user space name requirement. But having no clear definition of the
entity names
brings more trouble to user space. E.g. when a sensor exposes multiple
subdevs.
User space library/application could then reference them by entity name. It
seems difficult to me to handle such multiple subdev devices without
somehow
reliable subdev names.
I imagine a system with multiple sensors of same type sitting on
different I2C
busses, then appending I2C bus number/slave address to the name would be
useful.
And is it always possible to discover the pipeline, as opposite to e.g.
using
configuration file to activate all required links ? Configurations could be
of course per board file, but then at least we should keep subdev names
constant through the kernel releases.
[1] http://linuxtv.org/downloads/v4l-dvb-apis/media-ioc-enum-entities.html
--
Regards,
Sylwester
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 1/6 v4] media: V4L2: support asynchronous subdevice registration
2013-01-08 8:10 ` Laurent Pinchart
2013-01-08 9:25 ` Guennadi Liakhovetski
@ 2013-01-08 14:52 ` Sylwester Nawrocki
2013-01-08 21:23 ` Laurent Pinchart
1 sibling, 1 reply; 31+ messages in thread
From: Sylwester Nawrocki @ 2013-01-08 14:52 UTC (permalink / raw)
To: Laurent Pinchart, Guennadi Liakhovetski
Cc: linux-media, Hans Verkuil, Sylwester Nawrocki, Sylwester Nawrocki,
linux-sh, Magnus Damm, Sakari Ailus, Prabhakar Lad
Hi,
On 01/08/2013 09:10 AM, Laurent Pinchart wrote:
>> +/*
>> + * If subdevice probing fails any time after v4l2_async_subdev_bind(), no
>> + * clean up must be called. This function is only a message of intention.
>> + */
>> +int v4l2_async_subdev_bind(struct v4l2_async_subdev_list *asdl);
>> +int v4l2_async_subdev_bound(struct v4l2_async_subdev_list *asdl);
>
> Could you please explain why you need both a bind notifier and a bound
> notifier ? I was expecting a single v4l2_async_subdev_register() call in
> subdev drivers (and, thinking about it, I would probably name it
> v4l2_subdev_register()).
I expected it to be done this way too, and I also used
v4l2_subdev_register()
name in my early version of the subdev registration code where subdevs
were registering themselves to the v4l2 core.
BTW, this might not be most important thing here, but do we need separate
file, i.e. v4l2-async.c, instead of for example putting it in
v4l2-device.c ?
>> +void v4l2_async_subdev_unbind(struct v4l2_async_subdev_list *asdl);
>> +#endif
--
Regards,
Sylwester
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 1/6 v4] media: V4L2: support asynchronous subdevice registration
2013-01-08 10:26 ` Guennadi Liakhovetski
2013-01-08 10:35 ` Laurent Pinchart
@ 2013-01-08 19:26 ` Sylwester Nawrocki
1 sibling, 0 replies; 31+ messages in thread
From: Sylwester Nawrocki @ 2013-01-08 19:26 UTC (permalink / raw)
To: Guennadi Liakhovetski
Cc: Laurent Pinchart, linux-media, Hans Verkuil, Sylwester Nawrocki,
linux-sh, Magnus Damm, Sakari Ailus, Prabhakar Lad, LKML
Hi Guennadi,
Cc: LKML
On 01/08/2013 11:26 AM, Guennadi Liakhovetski wrote:
> On Tue, 8 Jan 2013, Laurent Pinchart wrote:
>> On Tuesday 08 January 2013 10:56:43 Guennadi Liakhovetski wrote:
>>> On Tue, 8 Jan 2013, Laurent Pinchart wrote:
>>>> On Tuesday 08 January 2013 10:25:15 Guennadi Liakhovetski wrote:
>>>>> On Tue, 8 Jan 2013, Laurent Pinchart wrote:
>>>>>> On Monday 07 January 2013 11:23:55 Guennadi Liakhovetski wrote:
>>>>>>> > From 0e1eae338ba898dc25ec60e3dba99e5581edc199 Mon Sep 17 00:00:00
>>>>>>>> 2001
>>>
>>> [snip]
>>>
>>>>>>> +int v4l2_async_notifier_register(struct v4l2_device *v4l2_dev,
>>>>>>> + struct v4l2_async_notifier *notifier);
>>>>>>> +void v4l2_async_notifier_unregister(struct v4l2_async_notifier
>>>>>>> *notifier);
>>>>>>> +/*
>>>>>>> + * If subdevice probing fails any time after
>>>>>>> v4l2_async_subdev_bind(),
>>>>>>> no
>>>>>>> + * clean up must be called. This function is only a message of
>>>>>>> intention.
>>>>>>> + */
>>>>>>> +int v4l2_async_subdev_bind(struct v4l2_async_subdev_list *asdl);
>>>>>>> +int v4l2_async_subdev_bound(struct v4l2_async_subdev_list *asdl);
>>>>>>
>>>>>> Could you please explain why you need both a bind notifier and a bound
>>>>>> notifier ? I was expecting a single v4l2_async_subdev_register() call
>>>>>> in subdev drivers (and, thinking about it, I would probably name it
>>>>>> v4l2_subdev_register()).
>>>>>
>>>>> I think I can, yes. Because between .bind() and .bound() the subdevice
>>>>> driver does the actual hardware probing. So, .bind() is used to make
>>>>> sure the hardware can be accessed, most importantly to provide a clock
>>>>> to the subdevice. You can look at soc_camera_async_bind(). There I'm
>>>>> registering the clock for the subdevice, about to bind. Why I cannot do
>>>>> it before, is because I need subdevice name for clock matching. With I2C
>>>>> subdevices the subdevice name contains the name of the driver, adapter
>>>>> number and i2c address. The latter 2 I've got from host subdevice list.
>>>>> But not the driver name. I thought about also passing the driver name
>>>>> there, but that seemed too limiting to me. I also request regulators
>>>>> there, because before ->bound() the sensor driver, but that could be
>>>>> done on the first call to soc_camera_power_on(), although doing this
>>>>> "first call" thingie is kind of hackish too. I could add one more soc-
>>>>> camera-power helper like soc_camera_prepare() or similar too.
>>>>
>>>> I think a soc_camera_power_init() function (or similar) would be a good
>>>> idea, yes.
>>>>
>>>>> So, the main problem is the clock
>>>>>
>>>>> subdevice name. Also see the comment in soc_camera.c:
>>>>> /*
>>>>> * It is ok to keep the clock for the whole soc_camera_device
>>>>> life-time,
>>>>> * in principle it would be more logical to register the clock on icd
>>>>> * creation, the only problem is, that at that time we don't know the
>>>>> * driver name yet.
>>>>> */
>>>>
>>>> I think we should fix that problem instead of shaping the async API around
>>>> a workaround :-)
>>>>
>>>> From the subdevice point of view, the probe function should request
>>>> resources, perform whatever initialization is needed (including verifying
>>>> that the hardware is functional when possible), and the register the
>>>> subdev with the code if everything succeeded. Splitting registration into
>>>> bind() and bound() appears a bit as a workaround to me.
>>>>
>>>> If we need a workaround, I'd rather pass the device name in addition to
>>>> the I2C adapter number and address, instead of embedding the workaround in
>>>> this new API.
>>>
>>> ...or we can change the I2C subdevice name format. The actual need to do
>>>
>>> snprintf(clk_name, sizeof(clk_name), "%s %d-%04x",
>>> asdl->dev->driver->name,
>>> i2c_adapter_id(client->adapter), client->addr);
>>>
>>> in soc-camera now to exactly match the subdevice name, as created by
>>> v4l2_i2c_subdev_init(), doesn't make me specifically happy either. What if
>>> the latter changes at some point? Or what if one driver wishes to create
>>> several subdevices for one I2C device?
>>
>> The common clock framework uses %d-%04x, maybe we could use that as well for
>> clock names ?
>
> And preserve the subdevice names? Then matching would be more difficult
> and less precise. Or change subdevice names too? I think, we can do the
> latter, since anyway at any time only one driver can be attached to an I2C
> device.
I'm just wondering why we can't associate the clock with relevant device,
rather than its driver ? This could eliminate the problem of unknown
sub-device name at the host driver, before sub-device driver is actually
probed, couldn't it ?
--
Thanks,
Sylwester
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 1/6 v4] media: V4L2: support asynchronous subdevice registration
2013-01-08 13:15 ` Sakari Ailus
@ 2013-01-08 21:17 ` Laurent Pinchart
2013-03-12 16:59 ` V4L2 subdevice naming (was Re: [PATCH 1/6 v4] media: V4L2: support asynchronous subdevice registrati Guennadi Liakhovetski
0 siblings, 1 reply; 31+ messages in thread
From: Laurent Pinchart @ 2013-01-08 21:17 UTC (permalink / raw)
To: Sakari Ailus
Cc: Sylwester Nawrocki, Guennadi Liakhovetski, linux-media,
Hans Verkuil, Sylwester Nawrocki, linux-sh, Magnus Damm,
Prabhakar Lad
Hi Sakari,
On Tuesday 08 January 2013 15:15:44 Sakari Ailus wrote:
> On Tue, Jan 08, 2013 at 01:41:59PM +0100, Laurent Pinchart wrote:
> > On Tuesday 08 January 2013 12:37:36 Sylwester Nawrocki wrote:
> > > On 01/08/2013 11:35 AM, Laurent Pinchart wrote:
> > > >>>>> If we need a workaround, I'd rather pass the device name in
> > > >>>>> addition to the I2C adapter number and address, instead of
> > > >>>>> embedding the workaround in this new API.
> > > >>>>
> > > >>>> ...or we can change the I2C subdevice name format. The actual need
> > > >>>> to do
> > > >>>>
> > > >>>> snprintf(clk_name, sizeof(clk_name), "%s %d-%04x",
> > > >>>> asdl->dev->driver->name,
> > > >>>> i2c_adapter_id(client->adapter), client->addr);
> > > >>>>
> > > >>>> in soc-camera now to exactly match the subdevice name, as created
> > > >>>> by v4l2_i2c_subdev_init(), doesn't make me specifically happy
> > > >>>> either. What if the latter changes at some point? Or what if one
> > > >>>> driver wishes to create several subdevices for one I2C device?
> > > >>>
> > > >>> The common clock framework uses %d-%04x, maybe we could use that as
> > > >>> well for clock names ?
> > > >>
> > > >> And preserve the subdevice names? Then matching would be more
> > > >> difficult and less precise. Or change subdevice names too? I think,
> > > >> we can do the latter, since anyway at any time only one driver can be
> > > >> attached to an I2C device.
> > > >
> > > > That's right. Where else is the subdev name used ?
> > >
> > > Subdev names are exposed to user space by the media controller API.
> > > So they are really part of an ABI, aren't they ?
> >
> > They're used to construct the name exposed to userspace, but the media
> > controller core could probably handle that internally by concatenating the
> > driver name and the subdev name.
> >
> > > Also having I2C bus number or I2C slave address as part of the subdev
> > > name makes it more difficult to write portable applications. Hence
> > > in sensor drivers I used to overwrite subdev name to remove I2C bus
> > > and slave address, as the format used v4l2_i2c_subdev_init() seemed
> > > highly unsuitable..
> >
> > This clearly shows that we need to discuss the matter and agree on a
> > common mode of operation.
> >
> > Aren't applications that use the subdev name directly inherently
> > non-portable anyway ? If you want your application to support different
> > boards/sensors/SoCs
>
> Well, the name could come from a configuration file to distinguish e.g.
> between primary and secondary sensors.
In that case having the I2C bus number and address in the name doesn't create
an extra portability issue, does it ?
> For what it's worth, the SMIA++ driver uses the actual name of the sensor
> since there are about 10 sensors supported at the moment, and calling them
> all smiapp-xxxx looks a bit insipid. So one has to talk to the sensor to
> know what it's called.
>
> This isn't strictly mandatory but a nice feature.
>
> > you should discover the pipeline and find the sensor by iterating over
> > entities, instead of using the sensor entity name.
>
> To be fully generic, yes.
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 1/6 v4] media: V4L2: support asynchronous subdevice registration
2013-01-08 14:27 ` [PATCH 1/6 v4] media: V4L2: support asynchronous subdevice registration Sylwester Nawrocki
@ 2013-01-08 21:20 ` Laurent Pinchart
0 siblings, 0 replies; 31+ messages in thread
From: Laurent Pinchart @ 2013-01-08 21:20 UTC (permalink / raw)
To: Sylwester Nawrocki
Cc: Guennadi Liakhovetski, linux-media, Hans Verkuil,
Sylwester Nawrocki, linux-sh, Magnus Damm, Sakari Ailus,
Prabhakar Lad
Hi Sylwester,
On Tuesday 08 January 2013 15:27:09 Sylwester Nawrocki wrote:
> On 01/08/2013 01:41 PM, Laurent Pinchart wrote:
> >> Subdev names are exposed to user space by the media controller API.
> >> So they are really part of an ABI, aren't they ?
> >
> > They're used to construct the name exposed to userspace, but the media
> > controller core could probably handle that internally by concatenating the
> > driver name and the subdev name.
> >
> >> Also having I2C bus number or I2C slave address as part of the subdev
> >> name makes it more difficult to write portable applications. Hence
> >> in sensor drivers I used to overwrite subdev name to remove I2C bus
> >> and slave address, as the format used v4l2_i2c_subdev_init() seemed
> >> highly unsuitable..
> >
> > This clearly shows that we need to discuss the matter and agree on a
> > common mode of operation.
> >
> > Aren't applications that use the subdev name directly inherently
> > non-portable anyway ? If you want your application to support different
> > boards/sensors/SoCs you should discover the pipeline and find the sensor
> > by iterating over entities, instead of using the sensor entity name.
>
> It depends on how we define the entity names :) It the names change from
> board to board and are completely unreliable then user space applications
> using them have no any chance to be generic. Nevertheless, struct
> media_entity_desc::name [1] has currently no specific semantics defined,
> e.g. for V4L2.
>
> It's likely way better for the kernel space to be no constrained by the
> subdev user space name requirement. But having no clear definition of the
> entity names brings more trouble to user space. E.g. when a sensor exposes
> multiple subdevs. User space library/application could then reference them
> by entity name. It seems difficult to me to handle such multiple subdev
> devices without somehow reliable subdev names.
I agree, I think naming rules are required.
> I imagine a system with multiple sensors of same type sitting on different
> I2C busses, then appending I2C bus number/slave address to the name would be
> useful.
And an application can always strip off the I2C bus number and address and use
the sensor name only if needed (or use the sensor name in addition to the I2C
information).
> And is it always possible to discover the pipeline, as opposite to e.g.
> using configuration file to activate all required links ? Configurations
> could be of course per board file, but then at least we should keep subdev
> names constant through the kernel releases.
I'm fine with applications more or less hardcoding the pipeline, and I agree
that subdev names need to be kept constant across kernel releases (and, very
importantly, well-defined). Now we "just" need to agree on naming rules :-)
> [1] http://linuxtv.org/downloads/v4l-dvb-apis/media-ioc-enum-entities.html
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 1/6 v4] media: V4L2: support asynchronous subdevice registration
2013-01-08 14:52 ` Sylwester Nawrocki
@ 2013-01-08 21:23 ` Laurent Pinchart
0 siblings, 0 replies; 31+ messages in thread
From: Laurent Pinchart @ 2013-01-08 21:23 UTC (permalink / raw)
To: Sylwester Nawrocki
Cc: Guennadi Liakhovetski, linux-media, Hans Verkuil,
Sylwester Nawrocki, linux-sh, Magnus Damm, Sakari Ailus,
Prabhakar Lad
Hi Sylwester,
On Tuesday 08 January 2013 15:52:21 Sylwester Nawrocki wrote:
> On 01/08/2013 09:10 AM, Laurent Pinchart wrote:
> >> +/*
> >> + * If subdevice probing fails any time after v4l2_async_subdev_bind(),
> >> + * no clean up must be called. This function is only a message of
> >> + * intention.
> >> + */
> >> +int v4l2_async_subdev_bind(struct v4l2_async_subdev_list *asdl);
> >> +int v4l2_async_subdev_bound(struct v4l2_async_subdev_list *asdl);
> >
> > Could you please explain why you need both a bind notifier and a bound
> > notifier ? I was expecting a single v4l2_async_subdev_register() call in
> > subdev drivers (and, thinking about it, I would probably name it
> > v4l2_subdev_register()).
>
> I expected it to be done this way too, and I also used
> v4l2_subdev_register() name in my early version of the subdev registration
> code where subdevs were registering themselves to the v4l2 core.
I think we can switch back to v4l2_subdev_register() if we can solve the clock
name issue. This doesn't seem impossible at first sight.
> BTW, this might not be most important thing here, but do we need separate
> file, i.e. v4l2-async.c, instead of for example putting it in v4l2-device.c
> ?
I'm fine with both, but I tend to try and keep source files not too large for
ease of reading. Depending on the amount of code we end up adding, moving the
functions to v4l2-device.c might be a good idea.
> >> +void v4l2_async_subdev_unbind(struct v4l2_async_subdev_list *asdl);
> >> +#endif
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 6/6] ARM: shmobile: convert ap4evb to asynchronously register camera subdevices
2013-01-08 4:27 ` Simon Horman
@ 2013-01-08 22:35 ` Guennadi Liakhovetski
2013-01-09 0:04 ` Simon Horman
0 siblings, 1 reply; 31+ messages in thread
From: Guennadi Liakhovetski @ 2013-01-08 22:35 UTC (permalink / raw)
To: Simon Horman
Cc: linux-media, Laurent Pinchart, Hans Verkuil, Sylwester Nawrocki,
Sylwester Nawrocki, linux-sh, Magnus Damm, Sakari Ailus
Hi Simon
On Tue, 8 Jan 2013, Simon Horman wrote:
> On Wed, Dec 26, 2012 at 06:49:11PM +0100, Guennadi Liakhovetski wrote:
> > Register the imx074 camera I2C and the CSI-2 platform devices directly
> > in board platform data instead of letting the sh_mobile_ceu_camera driver
> > and the soc-camera framework register them at their run-time. This uses
> > the V4L2 asynchronous subdevice probing capability.
> >
> > Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
>
> Hi Guennadi,
>
> could you let me know what if any dependencies this patch has.
> And the status of any dependencies.
This patch depends on the other 5 patches in this series. Since the other
patches are still in work, this patch cannot be applied either yet. Sorry,
I should have marked it as RFC.
Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 6/6] ARM: shmobile: convert ap4evb to asynchronously register camera subdevices
2013-01-08 22:35 ` Guennadi Liakhovetski
@ 2013-01-09 0:04 ` Simon Horman
0 siblings, 0 replies; 31+ messages in thread
From: Simon Horman @ 2013-01-09 0:04 UTC (permalink / raw)
To: Guennadi Liakhovetski
Cc: linux-media, Laurent Pinchart, Hans Verkuil, Sylwester Nawrocki,
Sylwester Nawrocki, linux-sh, Magnus Damm, Sakari Ailus
On Tue, Jan 08, 2013 at 11:35:21PM +0100, Guennadi Liakhovetski wrote:
> Hi Simon
>
> On Tue, 8 Jan 2013, Simon Horman wrote:
>
> > On Wed, Dec 26, 2012 at 06:49:11PM +0100, Guennadi Liakhovetski wrote:
> > > Register the imx074 camera I2C and the CSI-2 platform devices directly
> > > in board platform data instead of letting the sh_mobile_ceu_camera driver
> > > and the soc-camera framework register them at their run-time. This uses
> > > the V4L2 asynchronous subdevice probing capability.
> > >
> > > Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> >
> > Hi Guennadi,
> >
> > could you let me know what if any dependencies this patch has.
> > And the status of any dependencies.
>
> This patch depends on the other 5 patches in this series. Since the other
> patches are still in work, this patch cannot be applied either yet. Sorry,
> I should have marked it as RFC.
Thanks, got it.
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 1/6 v5] media: V4L2: support asynchronous subdevice registration
2013-01-08 10:06 ` [PATCH 1/6 v5] " Guennadi Liakhovetski
@ 2013-01-10 5:42 ` Prabhakar Lad
0 siblings, 0 replies; 31+ messages in thread
From: Prabhakar Lad @ 2013-01-10 5:42 UTC (permalink / raw)
To: Guennadi Liakhovetski
Cc: linux-media, Laurent Pinchart, Hans Verkuil, Sylwester Nawrocki,
Sylwester Nawrocki, linux-sh, Magnus Damm, Sakari Ailus,
Prabhakar Lad
Hi Guennadi,
Nice Work, Thanks for the patch.
On Tue, Jan 8, 2013 at 3:36 PM, Guennadi Liakhovetski
<g.liakhovetski@gmx.de> wrote:
> Currently bridge device drivers register devices for all subdevices
> synchronously, tupically, during their probing. E.g. if an I2C CMOS sensor
> is attached to a video bridge device, the bridge driver will create an I2C
> device and wait for the respective I2C driver to probe. This makes linking
> of devices straight forward, but this approach cannot be used with
> intrinsically asynchronous and unordered device registration systems like
> the Flattened Device Tree. To support such systems this patch adds an
> asynchronous subdevice registration framework to V4L2. To use it respective
> (e.g. I2C) subdevice drivers must request deferred probing as long as their
> bridge driver hasn't probed. The bridge driver during its probing submits a
> an arbitrary number of subdevice descriptor groups to the framework to
> manage. After that it can add callbacks to each of those groups to be
> called at various stages during subdevice probing, e.g. after completion.
> Then the bridge driver can request single groups to be probed, finish its
> own probing and continue its video subsystem configuration from its
> callbacks.
>
> Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
Tested-by: Lad, Prabhakar <prabhakar.lad@ti.com>
Regards,
--Prabhakar Lad
> ---
> v5: Now really fix the case, when subdevices probe successfully before the
> bridge, thanks to Prabhakar for testing and reporting
>
> drivers/media/v4l2-core/Makefile | 3 +-
> drivers/media/v4l2-core/v4l2-async.c | 294 ++++++++++++++++++++++++++++++++++
> include/media/v4l2-async.h | 113 +++++++++++++
> 3 files changed, 409 insertions(+), 1 deletions(-)
> create mode 100644 drivers/media/v4l2-core/v4l2-async.c
> create mode 100644 include/media/v4l2-async.h
>
> diff --git a/drivers/media/v4l2-core/Makefile b/drivers/media/v4l2-core/Makefile
> index d065c01..b667ced 100644
> --- a/drivers/media/v4l2-core/Makefile
> +++ b/drivers/media/v4l2-core/Makefile
> @@ -5,7 +5,8 @@
> 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-clk.o
> + v4l2-event.o v4l2-ctrls.o v4l2-subdev.o v4l2-clk.o \
> + v4l2-async.o
> ifeq ($(CONFIG_COMPAT),y)
> videodev-objs += v4l2-compat-ioctl32.o
> endif
> diff --git a/drivers/media/v4l2-core/v4l2-async.c b/drivers/media/v4l2-core/v4l2-async.c
> new file mode 100644
> index 0000000..434c53d
> --- /dev/null
> +++ b/drivers/media/v4l2-core/v4l2-async.c
> @@ -0,0 +1,294 @@
> +/*
> + * V4L2 asynchronous subdevice registration API
> + *
> + * Copyright (C) 2012, 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/device.h>
> +#include <linux/err.h>
> +#include <linux/i2c.h>
> +#include <linux/list.h>
> +#include <linux/module.h>
> +#include <linux/mutex.h>
> +#include <linux/notifier.h>
> +#include <linux/platform_device.h>
> +#include <linux/slab.h>
> +#include <linux/types.h>
> +
> +#include <media/v4l2-async.h>
> +#include <media/v4l2-device.h>
> +#include <media/v4l2-subdev.h>
> +
> +static bool match_i2c(struct device *dev, struct v4l2_async_hw_device *hw_dev)
> +{
> + struct i2c_client *client = to_i2c_client(dev);
> + return hw_dev->bus_type = V4L2_ASYNC_BUS_I2C &&
> + hw_dev->match.i2c.adapter_id = client->adapter->nr &&
> + hw_dev->match.i2c.address = client->addr;
> +}
> +
> +static bool match_platform(struct device *dev, struct v4l2_async_hw_device *hw_dev)
> +{
> + return hw_dev->bus_type = V4L2_ASYNC_BUS_PLATFORM &&
> + !strcmp(hw_dev->match.platform.name, dev_name(dev));
> +}
> +
> +static LIST_HEAD(subdev_list);
> +static LIST_HEAD(notifier_list);
> +static DEFINE_MUTEX(list_lock);
> +
> +static struct v4l2_async_subdev *v4l2_async_belongs(struct v4l2_async_notifier *notifier,
> + struct v4l2_async_subdev_list *asdl)
> +{
> + struct v4l2_async_subdev *asd = NULL;
> + bool (*match)(struct device *,
> + struct v4l2_async_hw_device *);
> +
> + list_for_each_entry (asd, ¬ifier->waiting, list) {
> + struct v4l2_async_hw_device *hw = &asd->hw;
> + switch (hw->bus_type) {
> + case V4L2_ASYNC_BUS_SPECIAL:
> + match = hw->match.special.match;
> + if (!match)
> + /* Match always */
> + return asd;
> + break;
> + case V4L2_ASYNC_BUS_PLATFORM:
> + match = match_platform;
> + break;
> + case V4L2_ASYNC_BUS_I2C:
> + match = match_i2c;
> + break;
> + default:
> + /* Oops */
> + match = NULL;
> + dev_err(notifier->v4l2_dev ? notifier->v4l2_dev->dev : NULL,
> + "Invalid bus-type %u on %p\n", hw->bus_type, asd);
> + }
> +
> + if (match && match(asdl->dev, hw))
> + break;
> + }
> +
> + return asd;
> +}
> +
> +static int v4l2_async_test_notify(struct v4l2_async_notifier *notifier,
> + struct v4l2_async_subdev_list *asdl,
> + struct v4l2_async_subdev *asd)
> +{
> + int ret;
> +
> + /* Remove from the waiting list */
> + list_del(&asd->list);
> + asdl->asd = asd;
> + asdl->notifier = notifier;
> +
> + if (notifier->bound) {
> + ret = notifier->bound(notifier, asdl);
> + if (ret < 0)
> + return ret;
> + }
> + /* Move from the global subdevice list to notifier's done */
> + list_move(&asdl->list, ¬ifier->done);
> +
> + ret = v4l2_device_register_subdev(notifier->v4l2_dev,
> + asdl->subdev);
> + if (ret < 0) {
> + if (notifier->unbind)
> + notifier->unbind(notifier, asdl);
> + return ret;
> + }
> +
> + if (list_empty(¬ifier->waiting) && notifier->complete)
> + return notifier->complete(notifier);
> +
> + return 0;
> +}
> +
> +static struct device *v4l2_async_unbind(struct v4l2_async_subdev_list *asdl)
> +{
> + struct device *dev = asdl->dev;
> + v4l2_device_unregister_subdev(asdl->subdev);
> + /* Subdevice driver will reprobe and put asdl back onto the list */
> + list_del(&asdl->list);
> + asdl->asd = NULL;
> + /* If we handled USB devices, we'd have to lock the parent too */
> + device_release_driver(dev);
> + return dev;
> +}
> +
> +int v4l2_async_notifier_register(struct v4l2_device *v4l2_dev,
> + struct v4l2_async_notifier *notifier)
> +{
> + struct v4l2_async_subdev_list *asdl, *tmp;
> + int i;
> +
> + notifier->v4l2_dev = v4l2_dev;
> + INIT_LIST_HEAD(¬ifier->waiting);
> + INIT_LIST_HEAD(¬ifier->done);
> +
> + for (i = 0; i < notifier->subdev_num; i++)
> + list_add_tail(¬ifier->subdev[i]->list, ¬ifier->waiting);
> +
> + mutex_lock(&list_lock);
> +
> + /* Keep also completed notifiers on the list */
> + list_add(¬ifier->list, ¬ifier_list);
> +
> + list_for_each_entry_safe(asdl, tmp, &subdev_list, list) {
> + struct v4l2_async_subdev *asd = v4l2_async_belongs(notifier, asdl);
> + int ret;
> +
> + if (!asd)
> + continue;
> +
> + if (notifier->bind) {
> + ret = notifier->bind(notifier, asdl);
> + if (ret < 0)
> + continue;
> + }
> +
> + ret = v4l2_async_test_notify(notifier, asdl, asd);
> + if (ret < 0) {
> + mutex_unlock(&list_lock);
> + return ret;
> + }
> + }
> +
> + mutex_unlock(&list_lock);
> +
> + return 0;
> +}
> +EXPORT_SYMBOL(v4l2_async_notifier_register);
> +
> +void v4l2_async_notifier_unregister(struct v4l2_async_notifier *notifier)
> +{
> + struct v4l2_async_subdev_list *asdl, *tmp;
> + int i = 0;
> + struct device **dev = kcalloc(notifier->subdev_num,
> + sizeof(*dev), GFP_KERNEL);
> + if (!dev)
> + dev_err(notifier->v4l2_dev->dev,
> + "Failed to allocate device cache!\n");
> +
> + mutex_lock(&list_lock);
> +
> + list_del(¬ifier->list);
> +
> + list_for_each_entry_safe(asdl, tmp, ¬ifier->done, list) {
> + if (dev)
> + dev[i++] = get_device(asdl->dev);
> + v4l2_async_unbind(asdl);
> +
> + if (notifier->unbind)
> + notifier->unbind(notifier, asdl);
> + }
> +
> + mutex_unlock(&list_lock);
> +
> + if (dev) {
> + while (i--) {
> + if (dev[i] && device_attach(dev[i]) < 0)
> + dev_err(dev[i], "Failed to re-probe to %s\n",
> + dev[i]->driver ? dev[i]->driver->name : "(none)");
> + put_device(dev[i]);
> + }
> + kfree(dev);
> + }
> + /*
> + * Don't care about the waiting list, it is initialised and populated
> + * upon notifier registration.
> + */
> +}
> +EXPORT_SYMBOL(v4l2_async_notifier_unregister);
> +
> +int v4l2_async_subdev_bind(struct v4l2_async_subdev_list *asdl)
> +{
> + struct v4l2_async_notifier *notifier;
> + int ret = 0;
> +
> + mutex_lock(&list_lock);
> +
> + list_for_each_entry(notifier, ¬ifier_list, list) {
> + struct v4l2_async_subdev *asd = v4l2_async_belongs(notifier,
> + asdl);
> + /*
> + * Whether or not probing succeeds - this is the right hardware
> + * subdevice descriptor and we can provide it to the notifier
> + */
> + if (asd) {
> + asdl->asd = asd;
> + if (notifier->bind)
> + ret = notifier->bind(notifier, asdl);
> + break;
> + }
> + }
> +
> + mutex_unlock(&list_lock);
> +
> + return ret;
> +}
> +EXPORT_SYMBOL(v4l2_async_subdev_bind);
> +
> +int v4l2_async_subdev_bound(struct v4l2_async_subdev_list *asdl)
> +{
> + struct v4l2_async_notifier *notifier;
> +
> + mutex_lock(&list_lock);
> +
> + INIT_LIST_HEAD(&asdl->list);
> +
> + list_for_each_entry(notifier, ¬ifier_list, list) {
> + struct v4l2_async_subdev *asd = v4l2_async_belongs(notifier, asdl);
> + if (asd) {
> + int ret = v4l2_async_test_notify(notifier, asdl, asd);
> + mutex_unlock(&list_lock);
> + return ret;
> + }
> + }
> +
> + /* None matched, wait for hot-plugging */
> + list_add(&asdl->list, &subdev_list);
> +
> + mutex_unlock(&list_lock);
> +
> + return 0;
> +}
> +EXPORT_SYMBOL(v4l2_async_subdev_bound);
> +
> +void v4l2_async_subdev_unbind(struct v4l2_async_subdev_list *asdl)
> +{
> + struct v4l2_async_notifier *notifier = asdl->notifier;
> + struct device *dev;
> +
> + if (!asdl->asd)
> + return;
> +
> + mutex_lock(&list_lock);
> +
> + dev = asdl->dev;
> +
> + list_add(&asdl->asd->list, ¬ifier->waiting);
> +
> + dev = get_device(asdl->dev);
> +
> + v4l2_async_unbind(asdl);
> +
> + if (notifier->unbind)
> + notifier->unbind(notifier, asdl);
> +
> + mutex_unlock(&list_lock);
> +
> + /* Re-probe with lock released - avoid a deadlock */
> + if (dev && device_attach(dev) < 0)
> + dev_err(dev, "Failed to re-probe to %s\n",
> + dev->driver ? dev->driver->name : "(none)");
> +
> + put_device(dev);
> +}
> +EXPORT_SYMBOL(v4l2_async_subdev_unbind);
> diff --git a/include/media/v4l2-async.h b/include/media/v4l2-async.h
> new file mode 100644
> index 0000000..91d436d
> --- /dev/null
> +++ b/include/media/v4l2-async.h
> @@ -0,0 +1,113 @@
> +/*
> + * V4L2 asynchronous subdevice registration API
> + *
> + * Copyright (C) 2012, 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.
> + */
> +
> +#ifndef V4L2_ASYNC_H
> +#define V4L2_ASYNC_H
> +
> +#include <linux/list.h>
> +#include <linux/mutex.h>
> +#include <linux/notifier.h>
> +
> +#include <media/v4l2-subdev.h>
> +
> +struct device;
> +struct v4l2_device;
> +struct v4l2_async_notifier;
> +
> +enum v4l2_async_bus_type {
> + V4L2_ASYNC_BUS_SPECIAL,
> + V4L2_ASYNC_BUS_PLATFORM,
> + V4L2_ASYNC_BUS_I2C,
> +};
> +
> +struct v4l2_async_hw_device {
> + enum v4l2_async_bus_type bus_type;
> + union {
> + struct {
> + const char *name;
> + } platform;
> + struct {
> + int adapter_id;
> + unsigned short address;
> + } i2c;
> + struct {
> + bool (*match)(struct device *,
> + struct v4l2_async_hw_device *);
> + void *priv;
> + } special;
> + } match;
> +};
> +
> +/**
> + * struct v4l2_async_subdev - sub-device descriptor, as known to a bridge
> + * @hw: this device descriptor
> + * @list: member in a list of subdevices
> + */
> +struct v4l2_async_subdev {
> + struct v4l2_async_hw_device hw;
> + struct list_head list;
> +};
> +
> +/**
> + * v4l2_async_subdev_list - provided by subdevices
> + * @list: member in a list of subdevices
> + * @dev: hardware device
> + * @subdev: V4L2 subdevice
> + * @asd: pointer to respective struct v4l2_async_subdev
> + * @notifier: pointer to managing notifier
> + */
> +struct v4l2_async_subdev_list {
> + struct list_head list;
> + struct device *dev;
> + struct v4l2_subdev *subdev;
> + struct v4l2_async_subdev *asd;
> + struct v4l2_async_notifier *notifier;
> +};
> +
> +/**
> + * v4l2_async_notifier - provided by bridges
> + * @subdev_num: number of subdevices
> + * @subdev: array of pointers to subdevices
> + * @v4l2_dev: pointer to sruct v4l2_device
> + * @waiting: list of subdevices, waiting for their drivers
> + * @done: list of subdevices, already probed
> + * @list: member in a global list of notifiers
> + * @bind: a subdevice driver is about to probe one of your subdevices
> + * @bound: a subdevice driver has successfully probed one of your subdevices
> + * @complete: all your subdevices have been probed successfully
> + * @unbind: a subdevice is leaving
> + */
> +struct v4l2_async_notifier {
> + int subdev_num;
> + struct v4l2_async_subdev **subdev;
> + struct v4l2_device *v4l2_dev;
> + struct list_head waiting;
> + struct list_head done;
> + struct list_head list;
> + int (*bind)(struct v4l2_async_notifier *notifier,
> + struct v4l2_async_subdev_list *asdl);
> + int (*bound)(struct v4l2_async_notifier *notifier,
> + struct v4l2_async_subdev_list *asdl);
> + int (*complete)(struct v4l2_async_notifier *notifier);
> + void (*unbind)(struct v4l2_async_notifier *notifier,
> + struct v4l2_async_subdev_list *asdl);
> +};
> +
> +int v4l2_async_notifier_register(struct v4l2_device *v4l2_dev,
> + struct v4l2_async_notifier *notifier);
> +void v4l2_async_notifier_unregister(struct v4l2_async_notifier *notifier);
> +/*
> + * If subdevice probing fails any time after v4l2_async_subdev_bind(), no clean
> + * up must be called. This function is only a message of intention.
> + */
> +int v4l2_async_subdev_bind(struct v4l2_async_subdev_list *asdl);
> +int v4l2_async_subdev_bound(struct v4l2_async_subdev_list *asdl);
> +void v4l2_async_subdev_unbind(struct v4l2_async_subdev_list *asdl);
> +#endif
> --
> 1.7.2.5
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-media" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 31+ messages in thread
* V4L2 subdevice naming (was Re: [PATCH 1/6 v4] media: V4L2: support asynchronous subdevice registrati
2013-01-08 21:17 ` Laurent Pinchart
@ 2013-03-12 16:59 ` Guennadi Liakhovetski
0 siblings, 0 replies; 31+ messages in thread
From: Guennadi Liakhovetski @ 2013-03-12 16:59 UTC (permalink / raw)
To: Laurent Pinchart
Cc: Sakari Ailus, Sylwester Nawrocki, linux-media, Hans Verkuil,
Sylwester Nawrocki, linux-sh, Magnus Damm, Prabhakar Lad
Hi all
(this is a _conscious_ case of top-posting, hopefully a justified one :-) )
The discussion, I'd like to continue here is pretty old (2 months), so,
instead of asking everyone to go and re-read the thread, I'll try to
summarise the problem here and encourage everyone to contribute to a final
solution :-)
The last version of my "asynchronous V4L2 subdevice registration" patch
stumbled upon a problem of V4L2 clock naming, which has to match a
subdevice name. Currently on I2C subdevice names are produced per
snprintf(clk_name, sizeof(clk_name), "%s %d-%04x",
client->dev.driver->name,
i2c_adapter_id(client->adapter), client->addr);
The problem with this is, that to match this name in a V4L2 clock, the
clock provider (typically a V4L2 bridge driver) has to know the name of a
driver, that is going to bind to the I2C device. Normally bridge drivers
don't have that information, so, it either has to be provided to them
additionally beforehand, or they have to wait until an I2C driver has
been bound to the I2C device. Both these options are inconvenient.
We can now decide to drop the driver name from the subdevice name, but
subdevice names are exposed to the user-space. It is possible, that
user-space software exists, that relies on specific subdevice names. If we
change them, that software can break. A possible solution would be to
remove the driver name from the subdevice name for internal purposes, but
to re-add it, when exposing to the user-space.
We can also leave the name as is and relax the clock matching criteria:
instead of a full string match we can match a substring and verify, that
the matching part is at the end of the name. We could even actually
produce the complete name by prepending the driver name in the V4L2 clock
helpers, but that would make the helpers aware of V4L2 subdevice
internals, which isn't very good either.
Also note, that non-standard subdevice names are possible, there are also
non-I2C subdevices, multiple subdevices per I2C device are also a
possibility. So, maybe explicitly passing a device name from board data,
similarly to how the clock API does that, is indeed the best option.
I'll keep an original message quoted below for now, in case someone wants
to through a glance, but feel free to remove it when replying.
Thanks
Guennadi
On Tue, 8 Jan 2013, Laurent Pinchart wrote:
> Hi Sakari,
>
> On Tuesday 08 January 2013 15:15:44 Sakari Ailus wrote:
> > On Tue, Jan 08, 2013 at 01:41:59PM +0100, Laurent Pinchart wrote:
> > > On Tuesday 08 January 2013 12:37:36 Sylwester Nawrocki wrote:
> > > > On 01/08/2013 11:35 AM, Laurent Pinchart wrote:
> > > > >>>>> If we need a workaround, I'd rather pass the device name in
> > > > >>>>> addition to the I2C adapter number and address, instead of
> > > > >>>>> embedding the workaround in this new API.
> > > > >>>>
> > > > >>>> ...or we can change the I2C subdevice name format. The actual need
> > > > >>>> to do
> > > > >>>>
> > > > >>>> snprintf(clk_name, sizeof(clk_name), "%s %d-%04x",
> > > > >>>> asdl->dev->driver->name,
> > > > >>>> i2c_adapter_id(client->adapter), client->addr);
> > > > >>>>
> > > > >>>> in soc-camera now to exactly match the subdevice name, as created
> > > > >>>> by v4l2_i2c_subdev_init(), doesn't make me specifically happy
> > > > >>>> either. What if the latter changes at some point? Or what if one
> > > > >>>> driver wishes to create several subdevices for one I2C device?
> > > > >>>
> > > > >>> The common clock framework uses %d-%04x, maybe we could use that as
> > > > >>> well for clock names ?
> > > > >>
> > > > >> And preserve the subdevice names? Then matching would be more
> > > > >> difficult and less precise. Or change subdevice names too? I think,
> > > > >> we can do the latter, since anyway at any time only one driver can be
> > > > >> attached to an I2C device.
> > > > >
> > > > > That's right. Where else is the subdev name used ?
> > > >
> > > > Subdev names are exposed to user space by the media controller API.
> > > > So they are really part of an ABI, aren't they ?
> > >
> > > They're used to construct the name exposed to userspace, but the media
> > > controller core could probably handle that internally by concatenating the
> > > driver name and the subdev name.
> > >
> > > > Also having I2C bus number or I2C slave address as part of the subdev
> > > > name makes it more difficult to write portable applications. Hence
> > > > in sensor drivers I used to overwrite subdev name to remove I2C bus
> > > > and slave address, as the format used v4l2_i2c_subdev_init() seemed
> > > > highly unsuitable..
> > >
> > > This clearly shows that we need to discuss the matter and agree on a
> > > common mode of operation.
> > >
> > > Aren't applications that use the subdev name directly inherently
> > > non-portable anyway ? If you want your application to support different
> > > boards/sensors/SoCs
> >
> > Well, the name could come from a configuration file to distinguish e.g.
> > between primary and secondary sensors.
>
> In that case having the I2C bus number and address in the name doesn't create
> an extra portability issue, does it ?
>
> > For what it's worth, the SMIA++ driver uses the actual name of the sensor
> > since there are about 10 sensors supported at the moment, and calling them
> > all smiapp-xxxx looks a bit insipid. So one has to talk to the sensor to
> > know what it's called.
> >
> > This isn't strictly mandatory but a nice feature.
> >
> > > you should discover the pipeline and find the sensor by iterating over
> > > entities, instead of using the sensor entity name.
> >
> > To be fully generic, yes.
>
> --
> Regards,
>
> Laurent Pinchart
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-sh" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
^ permalink raw reply [flat|nested] 31+ messages in thread
end of thread, other threads:[~2013-03-12 16:59 UTC | newest]
Thread overview: 31+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-12-26 17:49 [PATCH v3 0/6] V4L2 asynchronous probing + soc-camera example Guennadi Liakhovetski
2012-12-26 17:49 ` [PATCH 1/6] media: V4L2: support asynchronous subdevice registration Guennadi Liakhovetski
2013-01-07 10:23 ` [PATCH 1/6 v4] " Guennadi Liakhovetski
2013-01-08 8:10 ` Laurent Pinchart
2013-01-08 9:25 ` Guennadi Liakhovetski
2013-01-08 9:41 ` Laurent Pinchart
2013-01-08 9:56 ` Guennadi Liakhovetski
2013-01-08 10:21 ` Laurent Pinchart
2013-01-08 10:26 ` Guennadi Liakhovetski
2013-01-08 10:35 ` Laurent Pinchart
2013-01-08 11:37 ` Sylwester Nawrocki
2013-01-08 12:41 ` Laurent Pinchart
2013-01-08 13:15 ` Sakari Ailus
2013-01-08 21:17 ` Laurent Pinchart
2013-03-12 16:59 ` V4L2 subdevice naming (was Re: [PATCH 1/6 v4] media: V4L2: support asynchronous subdevice registrati Guennadi Liakhovetski
2013-01-08 14:27 ` [PATCH 1/6 v4] media: V4L2: support asynchronous subdevice registration Sylwester Nawrocki
2013-01-08 21:20 ` Laurent Pinchart
2013-01-08 19:26 ` Sylwester Nawrocki
2013-01-08 14:52 ` Sylwester Nawrocki
2013-01-08 21:23 ` Laurent Pinchart
2013-01-08 10:06 ` [PATCH 1/6 v5] " Guennadi Liakhovetski
2013-01-10 5:42 ` Prabhakar Lad
2012-12-26 17:49 ` [PATCH 2/6] media: soc-camera: switch I2C subdevice drivers to use v4l2-clk Guennadi Liakhovetski
2013-01-08 8:02 ` Laurent Pinchart
2012-12-26 17:49 ` [PATCH 3/6] soc-camera: add V4L2-async support Guennadi Liakhovetski
2012-12-26 17:49 ` [PATCH 4/6] sh_mobile_ceu_camera: add asynchronous subdevice probing support Guennadi Liakhovetski
2012-12-26 17:49 ` [PATCH 5/6] imx074: support asynchronous probing Guennadi Liakhovetski
2012-12-26 17:49 ` [PATCH 6/6] ARM: shmobile: convert ap4evb to asynchronously register camera subdevices Guennadi Liakhovetski
2013-01-08 4:27 ` Simon Horman
2013-01-08 22:35 ` Guennadi Liakhovetski
2013-01-09 0:04 ` Simon Horman
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).