Hi Greg On Sat, 20 Oct 2012, Guennadi Liakhovetski 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@xxxxxx> Sorry, I did indeed forget to include you on CC for this patch as promised in https://lkml.org/lkml/2012/10/17/306. In the patch below just look for the only occurrance of the device_release_driver() / device_attach(). In your mail you said, that only bus drivers should do this. In fact this is indeed what is happening here. A device is attached to two busses: (typically) I2C and "media." And the code below is called when the device is detached from the media bus. Thanks Guennadi > --- > > One more thing to note about this patch. Subdevice drivers, supporting > asynchronous probing, and using this framework, need a unified way to > detect, whether their probing should succeed or they should request > deferred probing. I implement this using device platform data. This means, > that all subdevice drivers, wishing to use this API will have to use the > same platform data struct. I don't think this is a major inconvenience, > but if we decide against this, we'll have to add a V4L2 function to verify > "are you ready for me or not." The latter would be inconvenient, because > then we would have to look through all registered subdevice descriptor > groups for this specific subdevice. > > drivers/media/v4l2-core/Makefile | 3 +- > drivers/media/v4l2-core/v4l2-async.c | 249 +++++++++++++++++++++++++++++++++ > drivers/media/v4l2-core/v4l2-device.c | 2 + > include/media/v4l2-async.h | 88 ++++++++++++ > include/media/v4l2-device.h | 6 + > include/media/v4l2-subdev.h | 16 ++ > 6 files changed, 363 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 cb5fede..074e01c 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..871f116 > --- /dev/null > +++ b/drivers/media/v4l2-core/v4l2-async.c > @@ -0,0 +1,249 @@ > +/* > + * V4L2 asynchronous subdevice registration API > + * > + * Copyright (C) 2012, Guennadi Liakhovetski <g.liakhovetski@xxxxxx> > + * > + * 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)); > +} > + > +/* > + * I think, notifiers on different busses can run concurrently, so, we have to > + * protect common data, e.g. sub-device lists. > + */ > +static int async_notifier_cb(struct v4l2_async_group *group, > + unsigned long action, struct device *dev, > + bool (*match)(struct device *, struct v4l2_async_hw_device *)) > +{ > + struct v4l2_device *v4l2_dev = group->v4l2_dev; > + struct v4l2_async_subdev *asd; > + bool done; > + int ret; > + > + if (action != BUS_NOTIFY_BOUND_DRIVER && > + action != BUS_NOTIFY_BIND_DRIVER) > + return NOTIFY_DONE; > + > + /* Asynchronous: have to lock */ > + mutex_lock(&v4l2_dev->group_lock); > + > + list_for_each_entry(asd, &group->group, list) { > + if (match(dev, &asd->hw)) > + break; > + } > + > + if (&asd->list == &group->group) { > + /* Not our device */ > + mutex_unlock(&v4l2_dev->group_lock); > + return NOTIFY_DONE; > + } > + > + asd->dev = dev; > + > + if (action == BUS_NOTIFY_BIND_DRIVER) { > + /* > + * Provide platform data to the driver: it can complete probing > + * now. > + */ > + dev->platform_data = &asd->sdpd; > + mutex_unlock(&v4l2_dev->group_lock); > + if (group->bind_cb) > + group->bind_cb(group, asd); > + return NOTIFY_OK; > + } > + > + /* BUS_NOTIFY_BOUND_DRIVER */ > + if (asd->hw.bus_type == V4L2_ASYNC_BUS_I2C) > + asd->sdpd.subdev = i2c_get_clientdata(to_i2c_client(dev)); > + /* > + * Non-I2C subdevice drivers should take care to assign their subdevice > + * pointers > + */ > + ret = v4l2_device_register_subdev(v4l2_dev, > + asd->sdpd.subdev); > + if (ret < 0) { > + mutex_unlock(&v4l2_dev->group_lock); > + /* FIXME: error, clean up world? */ > + dev_err(dev, "Failed registering a subdev: %d\n", ret); > + return NOTIFY_OK; > + } > + list_move(&asd->list, &group->done); > + > + /* Client probed & all subdev drivers collected */ > + done = list_empty(&group->group); > + > + mutex_unlock(&v4l2_dev->group_lock); > + > + if (group->bound_cb) > + group->bound_cb(group, asd); > + > + if (done && group->complete_cb) > + group->complete_cb(group); > + > + return NOTIFY_OK; > +} > + > +static int platform_cb(struct notifier_block *nb, > + unsigned long action, void *data) > +{ > + struct device *dev = data; > + struct v4l2_async_group *group = container_of(nb, struct v4l2_async_group, > + platform_notifier); > + > + return async_notifier_cb(group, action, dev, match_platform); > +} > + > +static int i2c_cb(struct notifier_block *nb, > + unsigned long action, void *data) > +{ > + struct device *dev = data; > + struct v4l2_async_group *group = container_of(nb, struct v4l2_async_group, > + i2c_notifier); > + > + return async_notifier_cb(group, action, dev, match_i2c); > +} > + > +/* > + * Typically this function will be called during bridge driver probing. It > + * installs bus notifiers to handle asynchronously probing subdevice drivers. > + * Once the bridge driver probing completes, subdevice drivers, waiting in > + * EPROBE_DEFER state are re-probed, at which point they get their platform > + * data, which allows them to complete probing. > + */ > +int v4l2_async_group_probe(struct v4l2_async_group *group) > +{ > + struct v4l2_async_subdev *asd, *tmp; > + bool i2c_used = false, platform_used = false; > + int ret; > + > + /* This group is inactive so far - no notifiers yet */ > + list_for_each_entry_safe(asd, tmp, &group->group, list) { > + if (asd->sdpd.subdev) { > + /* Simulate a BIND event */ > + if (group->bind_cb) > + group->bind_cb(group, asd); > + > + /* Already probed, don't wait for it */ > + ret = v4l2_device_register_subdev(group->v4l2_dev, > + asd->sdpd.subdev); > + > + if (ret < 0) > + return ret; > + > + list_move(&asd->list, &group->done); > + continue; > + } > + > + switch (asd->hw.bus_type) { > + case V4L2_ASYNC_BUS_PLATFORM: > + platform_used = true; > + break; > + case V4L2_ASYNC_BUS_I2C: > + i2c_used = true; > + } > + } > + > + if (list_empty(&group->group)) { > + if (group->complete_cb) > + group->complete_cb(group); > + return 0; > + } > + > + /* TODO: so far bus_register_notifier() never fails */ > + if (platform_used) { > + group->platform_notifier.notifier_call = platform_cb; > + bus_register_notifier(&platform_bus_type, > + &group->platform_notifier); > + } > + > + if (i2c_used) { > + group->i2c_notifier.notifier_call = i2c_cb; > + bus_register_notifier(&i2c_bus_type, > + &group->i2c_notifier); > + } > + > + return 0; > +} > +EXPORT_SYMBOL(v4l2_async_group_probe); > + > +int v4l2_async_group_init(struct v4l2_device *v4l2_dev, > + struct v4l2_async_group *group, > + struct v4l2_async_subdev *asd, int cnt) > +{ > + int i; > + > + if (!group) > + return -EINVAL; > + > + INIT_LIST_HEAD(&group->group); > + INIT_LIST_HEAD(&group->done); > + group->v4l2_dev = v4l2_dev; > + > + for (i = 0; i < cnt; i++) > + list_add_tail(&asd[i].list, &group->group); > + > + /* Protect the global V4L2 device group list */ > + mutex_lock(&v4l2_dev->group_lock); > + list_add_tail(&group->list, &v4l2_dev->group_head); > + mutex_unlock(&v4l2_dev->group_lock); > + > + return 0; > +} > +EXPORT_SYMBOL(v4l2_async_group_init); > + > +void v4l2_async_group_release(struct v4l2_async_group *group) > +{ > + struct v4l2_async_subdev *asd, *tmp; > + > + /* Also no problem, if notifiers haven't been registered */ > + bus_unregister_notifier(&platform_bus_type, > + &group->platform_notifier); > + bus_unregister_notifier(&i2c_bus_type, > + &group->i2c_notifier); > + > + mutex_lock(&group->v4l2_dev->group_lock); > + list_del(&group->list); > + mutex_unlock(&group->v4l2_dev->group_lock); > + > + list_for_each_entry_safe(asd, tmp, &group->done, list) { > + v4l2_device_unregister_subdev(asd->sdpd.subdev); > + /* If we handled USB devices, we'd have to lock the parent too */ > + device_release_driver(asd->dev); > + asd->dev->platform_data = NULL; > + if (device_attach(asd->dev) <= 0) > + dev_dbg(asd->dev, "Failed to re-probe to %s\n", asd->dev->driver ? > + asd->dev->driver->name : "(none)"); > + list_del(&asd->list); > + } > +} > +EXPORT_SYMBOL(v4l2_async_group_release); > 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..8f7bc06 > --- /dev/null > +++ b/include/media/v4l2-async.h > @@ -0,0 +1,88 @@ > +/* > + * V4L2 asynchronous subdevice registration API > + * > + * Copyright (C) 2012, Guennadi Liakhovetski <g.liakhovetski@xxxxxx> > + * > + * 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; > + > +enum v4l2_async_bus_type { > + 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; > + } match; > +}; > + > +/** > + * struct v4l2_async_subdev - device descriptor > + * @hw: this device descriptor > + * @list: member in the group > + * @dev: corresponding hardware device (I2C, platform,...) > + * @sdpd: embedded subdevice platform data > + * @role: this subdevice role in the video pipeline > + */ > +struct v4l2_async_subdev { > + struct v4l2_async_hw_device hw; > + struct list_head list; > + struct device *dev; > + struct v4l2_subdev_platform_data sdpd; > + enum v4l2_subdev_role role; > +}; > + > +/** > + * struct v4l2_async_group - list of device descriptors > + * @list: member in the v4l2 group list > + * @group: head of device list > + * @done: head of probed device list > + * @platform_notifier: platform bus notifier block > + * @i2c_notifier: I2C bus notifier block > + * @v4l2_dev: link to the respective struct v4l2_device > + * @bind: callback, called upon BUS_NOTIFY_BIND_DRIVER for each > + * subdevice > + * @complete: callback, called once after all subdevices in the group > + * have been bound > + */ > +struct v4l2_async_group { > + struct list_head list; > + struct list_head group; > + struct list_head done; > + struct notifier_block platform_notifier; > + struct notifier_block i2c_notifier; > + struct v4l2_device *v4l2_dev; > + int (*bind)(struct v4l2_async_group *group, > + struct v4l2_async_subdev *asd); > + int (*complete)(struct v4l2_async_group *group); > +}; > + > +int v4l2_async_group_init(struct v4l2_device *v4l2_dev, > + struct v4l2_async_group *group, > + struct v4l2_async_subdev *asd, int cnt); > +int v4l2_async_group_probe(struct v4l2_async_group *group); > +void v4l2_async_group_release(struct v4l2_async_group *group); > + > +#endif > diff --git a/include/media/v4l2-device.h b/include/media/v4l2-device.h > index d61febf..84b18c9 100644 > --- a/include/media/v4l2-device.h > +++ b/include/media/v4l2-device.h > @@ -21,6 +21,9 @@ > #ifndef _V4L2_DEVICE_H > #define _V4L2_DEVICE_H > > +#include <linux/list.h> > +#include <linux/mutex.h> > + > #include <media/media-device.h> > #include <media/v4l2-subdev.h> > #include <media/v4l2-dev.h> > @@ -60,6 +63,9 @@ struct v4l2_device { > struct v4l2_prio_state prio; > /* BKL replacement mutex. Temporary solution only. */ > struct mutex ioctl_lock; > + /* Subdevice group handling */ > + struct mutex group_lock; > + struct list_head group_head; > /* Keep track of the references to this struct. */ > struct kref ref; > /* Release function that is called when the ref count goes to 0. */ > diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h > index 2ecd737..1756c6c 100644 > --- a/include/media/v4l2-subdev.h > +++ b/include/media/v4l2-subdev.h > @@ -574,6 +574,22 @@ struct v4l2_subdev_fh { > #endif > }; > > +enum v4l2_subdev_role { > + V4L2_SUBDEV_DATA_SOURCE = 1, > + V4L2_SUBDEV_DATA_SINK, > + V4L2_SUBDEV_DATA_PROCESSOR, > +}; > + > +/** > + * struct v4l2_subdev_platform_data - subdev platform data > + * @subdev: subdevice > + * @platform_data: subdevice driver platform data > + */ > +struct v4l2_subdev_platform_data { > + struct v4l2_subdev *subdev; > + void *platform_data; > +}; > + > #define to_v4l2_subdev_fh(fh) \ > container_of(fh, struct v4l2_subdev_fh, vfh) > > -- > 1.7.2.5 > > --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/ -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
