* [PATCH v3] media: v4l2-async: Add waiting subdevices debugfs
2021-01-08 17:17 [PATCH] media: v4l2-async: Remove V4L2_ASYNC_MATCH_CUSTOM Ezequiel Garcia
@ 2021-01-08 17:17 ` Ezequiel Garcia
2021-01-14 23:36 ` Sakari Ailus
` (2 more replies)
2021-01-09 0:27 ` [PATCH] media: v4l2-async: Remove V4L2_ASYNC_MATCH_CUSTOM Laurent Pinchart
2021-01-15 19:40 ` Kieran Bingham
2 siblings, 3 replies; 12+ messages in thread
From: Ezequiel Garcia @ 2021-01-08 17:17 UTC (permalink / raw)
To: linux-media, Hans Verkuil
Cc: kernel, Laurent Pinchart, Sakari Ailus, Ezequiel Garcia
There is currently little to no information available
about the reasons why a v4l2-async device hasn't
probed completely.
Inspired by the "devices_deferred" debugfs file,
add a file to list information about the subdevices
that are on waiting lists, for each notifier.
This is useful to debug v4l2-async subdevices
and notifiers, for instance when doing device bring-up.
For instance, a typical output would be:
$ cat /sys/kernel/debug/video4linux/pending_async_subdevices
ipu1_csi1:
[fwnode] dev=20e0000.iomuxc-gpr:ipu1_csi1_mux, node=/soc/bus@2000000/iomuxc-gpr@20e0000/ipu1_csi1_mux
ipu1_csi0:
[fwnode] dev=20e0000.iomuxc-gpr:ipu1_csi0_mux, node=/soc/bus@2000000/iomuxc-gpr@20e0000/ipu1_csi0_mux
imx6-mipi-csi2:
[fwnode] dev=1-003c, node=/soc/bus@2100000/i2c@21a4000/camera@3c
imx-media:
Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
--
v3:
* Drop MATCH_CUSTOM
* Use the actual fwnode, to print the real full path. endpoint name.
* Fix refcounted handle leak.
v2:
* Print fwnode path, as suggested by Sakari.
* Print the subdevices under their corresponding notifier.
* Rename the file as suggested by Laurent.
---
drivers/media/v4l2-core/v4l2-async.c | 66 ++++++++++++++++++++++++++++
drivers/media/v4l2-core/v4l2-dev.c | 5 +++
include/media/v4l2-async.h | 8 ++++
3 files changed, 79 insertions(+)
diff --git a/drivers/media/v4l2-core/v4l2-async.c b/drivers/media/v4l2-core/v4l2-async.c
index 3faf1d12d49d..7c8a97205b3e 100644
--- a/drivers/media/v4l2-core/v4l2-async.c
+++ b/drivers/media/v4l2-core/v4l2-async.c
@@ -5,6 +5,7 @@
* Copyright (C) 2012-2013, Guennadi Liakhovetski <g.liakhovetski@gmx.de>
*/
+#include <linux/debugfs.h>
#include <linux/device.h>
#include <linux/err.h>
#include <linux/i2c.h>
@@ -14,6 +15,7 @@
#include <linux/mutex.h>
#include <linux/of.h>
#include <linux/platform_device.h>
+#include <linux/seq_file.h>
#include <linux/slab.h>
#include <linux/types.h>
@@ -823,3 +825,67 @@ void v4l2_async_unregister_subdev(struct v4l2_subdev *sd)
mutex_unlock(&list_lock);
}
EXPORT_SYMBOL(v4l2_async_unregister_subdev);
+
+static void print_waiting_subdev(struct seq_file *s,
+ struct v4l2_async_subdev *asd)
+{
+ switch (asd->match_type) {
+ case V4L2_ASYNC_MATCH_DEVNAME:
+ seq_printf(s, " [devname] dev=%s\n", asd->match.device_name);
+ break;
+ case V4L2_ASYNC_MATCH_I2C:
+ seq_printf(s, " [i2c] dev=%d-%04x\n", asd->match.i2c.adapter_id,
+ asd->match.i2c.address);
+ break;
+ case V4L2_ASYNC_MATCH_FWNODE: {
+ struct fwnode_handle *devnode, *fwnode = asd->match.fwnode;
+
+ devnode = fwnode_graph_is_endpoint(fwnode) ?
+ fwnode_graph_get_port_parent(fwnode) :
+ fwnode_handle_get(fwnode);
+
+ seq_printf(s, " [fwnode] dev=%s, node=%pfw\n",
+ devnode->dev ? dev_name(devnode->dev) : "nil",
+ fwnode);
+
+ fwnode_handle_put(devnode);
+ break;
+ }
+ }
+}
+
+static const char *
+v4l2_async_notifier_name(struct v4l2_async_notifier *notifier)
+{
+ if (notifier->v4l2_dev)
+ return notifier->v4l2_dev->name;
+ else if (notifier->sd)
+ return notifier->sd->name;
+ else
+ return "nil";
+}
+
+static int pending_subdevs_show(struct seq_file *s, void *data)
+{
+ struct v4l2_async_notifier *notif;
+ struct v4l2_async_subdev *asd;
+
+ mutex_lock(&list_lock);
+
+ list_for_each_entry(notif, ¬ifier_list, list) {
+ seq_printf(s, "%s:\n", v4l2_async_notifier_name(notif));
+ list_for_each_entry(asd, ¬if->waiting, list)
+ print_waiting_subdev(s, asd);
+ }
+
+ mutex_unlock(&list_lock);
+
+ return 0;
+}
+DEFINE_SHOW_ATTRIBUTE(pending_subdevs);
+
+void v4l2_async_debug_init(struct dentry *debugfs_dir)
+{
+ debugfs_create_file("pending_async_subdevices", 0444, debugfs_dir, NULL,
+ &pending_subdevs_fops);
+}
diff --git a/drivers/media/v4l2-core/v4l2-dev.c b/drivers/media/v4l2-core/v4l2-dev.c
index a593ea0598b5..8d3813e6ab56 100644
--- a/drivers/media/v4l2-core/v4l2-dev.c
+++ b/drivers/media/v4l2-core/v4l2-dev.c
@@ -14,6 +14,7 @@
#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+#include <linux/debugfs.h>
#include <linux/module.h>
#include <linux/types.h>
#include <linux/kernel.h>
@@ -37,6 +38,7 @@
__func__, ##arg); \
} while (0)
+static struct dentry *v4l2_debugfs_dir;
/*
* sysfs stuff
@@ -1113,6 +1115,8 @@ static int __init videodev_init(void)
return -EIO;
}
+ v4l2_debugfs_dir = debugfs_create_dir("video4linux", NULL);
+ v4l2_async_debug_init(v4l2_debugfs_dir);
return 0;
}
@@ -1120,6 +1124,7 @@ static void __exit videodev_exit(void)
{
dev_t dev = MKDEV(VIDEO_MAJOR, 0);
+ debugfs_remove_recursive(v4l2_debugfs_dir);
class_unregister(&video_class);
unregister_chrdev_region(dev, VIDEO_NUM_DEVICES);
}
diff --git a/include/media/v4l2-async.h b/include/media/v4l2-async.h
index 8ed42188e7c9..26296d4cb660 100644
--- a/include/media/v4l2-async.h
+++ b/include/media/v4l2-async.h
@@ -11,6 +11,7 @@
#include <linux/list.h>
#include <linux/mutex.h>
+struct dentry;
struct device;
struct device_node;
struct v4l2_device;
@@ -120,6 +121,13 @@ struct v4l2_async_notifier {
struct list_head list;
};
+/**
+ * v4l2_async_debug_init - Initialize debugging tools.
+ *
+ * @debugfs_dir: pointer to the parent debugfs &struct dentry
+ */
+void v4l2_async_debug_init(struct dentry *debugfs_dir);
+
/**
* v4l2_async_notifier_init - Initialize a notifier.
*
--
2.29.2
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH v3] media: v4l2-async: Add waiting subdevices debugfs
2021-01-08 17:17 ` [PATCH v3] media: v4l2-async: Add waiting subdevices debugfs Ezequiel Garcia
@ 2021-01-14 23:36 ` Sakari Ailus
2021-01-15 14:08 ` Ezequiel Garcia
2021-01-15 10:34 ` Hans Verkuil
2021-01-15 19:14 ` [PATCH v4] " Ezequiel Garcia
2 siblings, 1 reply; 12+ messages in thread
From: Sakari Ailus @ 2021-01-14 23:36 UTC (permalink / raw)
To: Ezequiel Garcia; +Cc: linux-media, Hans Verkuil, kernel, Laurent Pinchart
Hi Ezequiel,
On Fri, Jan 08, 2021 at 02:17:28PM -0300, Ezequiel Garcia wrote:
> There is currently little to no information available
> about the reasons why a v4l2-async device hasn't
> probed completely.
>
> Inspired by the "devices_deferred" debugfs file,
> add a file to list information about the subdevices
> that are on waiting lists, for each notifier.
>
> This is useful to debug v4l2-async subdevices
> and notifiers, for instance when doing device bring-up.
>
> For instance, a typical output would be:
>
> $ cat /sys/kernel/debug/video4linux/pending_async_subdevices
> ipu1_csi1:
> [fwnode] dev=20e0000.iomuxc-gpr:ipu1_csi1_mux, node=/soc/bus@2000000/iomuxc-gpr@20e0000/ipu1_csi1_mux
> ipu1_csi0:
> [fwnode] dev=20e0000.iomuxc-gpr:ipu1_csi0_mux, node=/soc/bus@2000000/iomuxc-gpr@20e0000/ipu1_csi0_mux
> imx6-mipi-csi2:
> [fwnode] dev=1-003c, node=/soc/bus@2100000/i2c@21a4000/camera@3c
> imx-media:
>
> Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> --
> v3:
> * Drop MATCH_CUSTOM
> * Use the actual fwnode, to print the real full path. endpoint name.
> * Fix refcounted handle leak.
> v2:
> * Print fwnode path, as suggested by Sakari.
> * Print the subdevices under their corresponding notifier.
> * Rename the file as suggested by Laurent.
The patch itself seems fine. But lease put such notes below the '---' line
below going forward. What's above it will be part of the commit message.
> ---
> drivers/media/v4l2-core/v4l2-async.c | 66 ++++++++++++++++++++++++++++
> drivers/media/v4l2-core/v4l2-dev.c | 5 +++
> include/media/v4l2-async.h | 8 ++++
> 3 files changed, 79 insertions(+)
>
--
Kind regards,
Sakari Ailus
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v3] media: v4l2-async: Add waiting subdevices debugfs
2021-01-14 23:36 ` Sakari Ailus
@ 2021-01-15 14:08 ` Ezequiel Garcia
2021-01-15 15:05 ` Sakari Ailus
0 siblings, 1 reply; 12+ messages in thread
From: Ezequiel Garcia @ 2021-01-15 14:08 UTC (permalink / raw)
To: Sakari Ailus; +Cc: linux-media, Hans Verkuil, kernel, Laurent Pinchart
On Fri, 2021-01-15 at 01:36 +0200, Sakari Ailus wrote:
> Hi Ezequiel,
>
> On Fri, Jan 08, 2021 at 02:17:28PM -0300, Ezequiel Garcia wrote:
> > There is currently little to no information available
> > about the reasons why a v4l2-async device hasn't
> > probed completely.
> >
> > Inspired by the "devices_deferred" debugfs file,
> > add a file to list information about the subdevices
> > that are on waiting lists, for each notifier.
> >
> > This is useful to debug v4l2-async subdevices
> > and notifiers, for instance when doing device bring-up.
> >
> > For instance, a typical output would be:
> >
> > $ cat /sys/kernel/debug/video4linux/pending_async_subdevices
> > ipu1_csi1:
> > [fwnode] dev=20e0000.iomuxc-gpr:ipu1_csi1_mux, node=/soc/bus@2000000/iomuxc-gpr@20e0000/ipu1_csi1_mux
> > ipu1_csi0:
> > [fwnode] dev=20e0000.iomuxc-gpr:ipu1_csi0_mux, node=/soc/bus@2000000/iomuxc-gpr@20e0000/ipu1_csi0_mux
> > imx6-mipi-csi2:
> > [fwnode] dev=1-003c, node=/soc/bus@2100000/i2c@21a4000/camera@3c
> > imx-media:
> >
> > Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
> > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > --
> > v3:
> > * Drop MATCH_CUSTOM
> > * Use the actual fwnode, to print the real full path. endpoint name.
> > * Fix refcounted handle leak.
> > v2:
> > * Print fwnode path, as suggested by Sakari.
> > * Print the subdevices under their corresponding notifier.
> > * Rename the file as suggested by Laurent.
>
> The patch itself seems fine. But lease put such notes below the '---' line
> below going forward. What's above it will be part of the commit message.
>
Oops, sorry for the typo.
I have a v4 for this one, dropping MATCH_DEVNAME too.
Thanks,
Ezequiel
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v3] media: v4l2-async: Add waiting subdevices debugfs
2021-01-15 14:08 ` Ezequiel Garcia
@ 2021-01-15 15:05 ` Sakari Ailus
2021-01-15 15:17 ` Ezequiel Garcia
0 siblings, 1 reply; 12+ messages in thread
From: Sakari Ailus @ 2021-01-15 15:05 UTC (permalink / raw)
To: Ezequiel Garcia; +Cc: linux-media, Hans Verkuil, kernel, Laurent Pinchart
On Fri, Jan 15, 2021 at 11:08:21AM -0300, Ezequiel Garcia wrote:
> On Fri, 2021-01-15 at 01:36 +0200, Sakari Ailus wrote:
> > Hi Ezequiel,
> >
> > On Fri, Jan 08, 2021 at 02:17:28PM -0300, Ezequiel Garcia wrote:
> > > There is currently little to no information available
> > > about the reasons why a v4l2-async device hasn't
> > > probed completely.
> > >
> > > Inspired by the "devices_deferred" debugfs file,
> > > add a file to list information about the subdevices
> > > that are on waiting lists, for each notifier.
> > >
> > > This is useful to debug v4l2-async subdevices
> > > and notifiers, for instance when doing device bring-up.
> > >
> > > For instance, a typical output would be:
> > >
> > > $ cat /sys/kernel/debug/video4linux/pending_async_subdevices
> > > ipu1_csi1:
> > > [fwnode] dev=20e0000.iomuxc-gpr:ipu1_csi1_mux, node=/soc/bus@2000000/iomuxc-gpr@20e0000/ipu1_csi1_mux
> > > ipu1_csi0:
> > > [fwnode] dev=20e0000.iomuxc-gpr:ipu1_csi0_mux, node=/soc/bus@2000000/iomuxc-gpr@20e0000/ipu1_csi0_mux
> > > imx6-mipi-csi2:
> > > [fwnode] dev=1-003c, node=/soc/bus@2100000/i2c@21a4000/camera@3c
> > > imx-media:
> > >
> > > Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
> > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > > --
> > > v3:
> > > * Drop MATCH_CUSTOM
> > > * Use the actual fwnode, to print the real full path. endpoint name.
> > > * Fix refcounted handle leak.
> > > v2:
> > > * Print fwnode path, as suggested by Sakari.
> > > * Print the subdevices under their corresponding notifier.
> > > * Rename the file as suggested by Laurent.
> >
> > The patch itself seems fine. But lease put such notes below the '---' line
> > below going forward. What's above it will be part of the commit message.
> >
>
> Oops, sorry for the typo.
>
> I have a v4 for this one, dropping MATCH_DEVNAME too.
Could you remove it as a separate patch? It's unrelated to debugging.
--
Sakari Ailus
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v3] media: v4l2-async: Add waiting subdevices debugfs
2021-01-15 15:05 ` Sakari Ailus
@ 2021-01-15 15:17 ` Ezequiel Garcia
0 siblings, 0 replies; 12+ messages in thread
From: Ezequiel Garcia @ 2021-01-15 15:17 UTC (permalink / raw)
To: Sakari Ailus; +Cc: linux-media, Hans Verkuil, kernel, Laurent Pinchart
On Fri, 2021-01-15 at 17:05 +0200, Sakari Ailus wrote:
> On Fri, Jan 15, 2021 at 11:08:21AM -0300, Ezequiel Garcia wrote:
> > On Fri, 2021-01-15 at 01:36 +0200, Sakari Ailus wrote:
> > > Hi Ezequiel,
> > >
> > > On Fri, Jan 08, 2021 at 02:17:28PM -0300, Ezequiel Garcia wrote:
> > > > There is currently little to no information available
> > > > about the reasons why a v4l2-async device hasn't
> > > > probed completely.
> > > >
> > > > Inspired by the "devices_deferred" debugfs file,
> > > > add a file to list information about the subdevices
> > > > that are on waiting lists, for each notifier.
> > > >
> > > > This is useful to debug v4l2-async subdevices
> > > > and notifiers, for instance when doing device bring-up.
> > > >
> > > > For instance, a typical output would be:
> > > >
> > > > $ cat /sys/kernel/debug/video4linux/pending_async_subdevices
> > > > ipu1_csi1:
> > > > [fwnode] dev=20e0000.iomuxc-gpr:ipu1_csi1_mux, node=/soc/bus@2000000/iomuxc-gpr@20e0000/ipu1_csi1_mux
> > > > ipu1_csi0:
> > > > [fwnode] dev=20e0000.iomuxc-gpr:ipu1_csi0_mux, node=/soc/bus@2000000/iomuxc-gpr@20e0000/ipu1_csi0_mux
> > > > imx6-mipi-csi2:
> > > > [fwnode] dev=1-003c, node=/soc/bus@2100000/i2c@21a4000/camera@3c
> > > > imx-media:
> > > >
> > > > Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
> > > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > > > --
> > > > v3:
> > > > * Drop MATCH_CUSTOM
> > > > * Use the actual fwnode, to print the real full path. endpoint name.
> > > > * Fix refcounted handle leak.
> > > > v2:
> > > > * Print fwnode path, as suggested by Sakari.
> > > > * Print the subdevices under their corresponding notifier.
> > > > * Rename the file as suggested by Laurent.
> > >
> > > The patch itself seems fine. But lease put such notes below the '---' line
> > > below going forward. What's above it will be part of the commit message.
> > >
> >
> > Oops, sorry for the typo.
> >
> > I have a v4 for this one, dropping MATCH_DEVNAME too.
>
> Could you remove it as a separate patch? It's unrelated to debugging.
>
Yes, but still we won't be printing MATCH_DEVNAME types
so this patch needs a v4 dropping that.
I think it's worth it as it simplifies the patch a bit.
Thanks,
Ezequiel
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v3] media: v4l2-async: Add waiting subdevices debugfs
2021-01-08 17:17 ` [PATCH v3] media: v4l2-async: Add waiting subdevices debugfs Ezequiel Garcia
2021-01-14 23:36 ` Sakari Ailus
@ 2021-01-15 10:34 ` Hans Verkuil
2021-01-15 19:14 ` [PATCH v4] " Ezequiel Garcia
2 siblings, 0 replies; 12+ messages in thread
From: Hans Verkuil @ 2021-01-15 10:34 UTC (permalink / raw)
To: Ezequiel Garcia, linux-media; +Cc: kernel, Laurent Pinchart, Sakari Ailus
On 08/01/2021 18:17, Ezequiel Garcia wrote:
> There is currently little to no information available
> about the reasons why a v4l2-async device hasn't
> probed completely.
>
> Inspired by the "devices_deferred" debugfs file,
> add a file to list information about the subdevices
> that are on waiting lists, for each notifier.
>
> This is useful to debug v4l2-async subdevices
> and notifiers, for instance when doing device bring-up.
>
> For instance, a typical output would be:
>
> $ cat /sys/kernel/debug/video4linux/pending_async_subdevices
> ipu1_csi1:
> [fwnode] dev=20e0000.iomuxc-gpr:ipu1_csi1_mux, node=/soc/bus@2000000/iomuxc-gpr@20e0000/ipu1_csi1_mux
> ipu1_csi0:
> [fwnode] dev=20e0000.iomuxc-gpr:ipu1_csi0_mux, node=/soc/bus@2000000/iomuxc-gpr@20e0000/ipu1_csi0_mux
> imx6-mipi-csi2:
> [fwnode] dev=1-003c, node=/soc/bus@2100000/i2c@21a4000/camera@3c
> imx-media:
>
> Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
Nice, this should be quite useful.
Regards,
Hans
> --
> v3:
> * Drop MATCH_CUSTOM
> * Use the actual fwnode, to print the real full path. endpoint name.
> * Fix refcounted handle leak.
> v2:
> * Print fwnode path, as suggested by Sakari.
> * Print the subdevices under their corresponding notifier.
> * Rename the file as suggested by Laurent.
> ---
> drivers/media/v4l2-core/v4l2-async.c | 66 ++++++++++++++++++++++++++++
> drivers/media/v4l2-core/v4l2-dev.c | 5 +++
> include/media/v4l2-async.h | 8 ++++
> 3 files changed, 79 insertions(+)
>
> diff --git a/drivers/media/v4l2-core/v4l2-async.c b/drivers/media/v4l2-core/v4l2-async.c
> index 3faf1d12d49d..7c8a97205b3e 100644
> --- a/drivers/media/v4l2-core/v4l2-async.c
> +++ b/drivers/media/v4l2-core/v4l2-async.c
> @@ -5,6 +5,7 @@
> * Copyright (C) 2012-2013, Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> */
>
> +#include <linux/debugfs.h>
> #include <linux/device.h>
> #include <linux/err.h>
> #include <linux/i2c.h>
> @@ -14,6 +15,7 @@
> #include <linux/mutex.h>
> #include <linux/of.h>
> #include <linux/platform_device.h>
> +#include <linux/seq_file.h>
> #include <linux/slab.h>
> #include <linux/types.h>
>
> @@ -823,3 +825,67 @@ void v4l2_async_unregister_subdev(struct v4l2_subdev *sd)
> mutex_unlock(&list_lock);
> }
> EXPORT_SYMBOL(v4l2_async_unregister_subdev);
> +
> +static void print_waiting_subdev(struct seq_file *s,
> + struct v4l2_async_subdev *asd)
> +{
> + switch (asd->match_type) {
> + case V4L2_ASYNC_MATCH_DEVNAME:
> + seq_printf(s, " [devname] dev=%s\n", asd->match.device_name);
> + break;
> + case V4L2_ASYNC_MATCH_I2C:
> + seq_printf(s, " [i2c] dev=%d-%04x\n", asd->match.i2c.adapter_id,
> + asd->match.i2c.address);
> + break;
> + case V4L2_ASYNC_MATCH_FWNODE: {
> + struct fwnode_handle *devnode, *fwnode = asd->match.fwnode;
> +
> + devnode = fwnode_graph_is_endpoint(fwnode) ?
> + fwnode_graph_get_port_parent(fwnode) :
> + fwnode_handle_get(fwnode);
> +
> + seq_printf(s, " [fwnode] dev=%s, node=%pfw\n",
> + devnode->dev ? dev_name(devnode->dev) : "nil",
> + fwnode);
> +
> + fwnode_handle_put(devnode);
> + break;
> + }
> + }
> +}
> +
> +static const char *
> +v4l2_async_notifier_name(struct v4l2_async_notifier *notifier)
> +{
> + if (notifier->v4l2_dev)
> + return notifier->v4l2_dev->name;
> + else if (notifier->sd)
> + return notifier->sd->name;
> + else
> + return "nil";
> +}
> +
> +static int pending_subdevs_show(struct seq_file *s, void *data)
> +{
> + struct v4l2_async_notifier *notif;
> + struct v4l2_async_subdev *asd;
> +
> + mutex_lock(&list_lock);
> +
> + list_for_each_entry(notif, ¬ifier_list, list) {
> + seq_printf(s, "%s:\n", v4l2_async_notifier_name(notif));
> + list_for_each_entry(asd, ¬if->waiting, list)
> + print_waiting_subdev(s, asd);
> + }
> +
> + mutex_unlock(&list_lock);
> +
> + return 0;
> +}
> +DEFINE_SHOW_ATTRIBUTE(pending_subdevs);
> +
> +void v4l2_async_debug_init(struct dentry *debugfs_dir)
> +{
> + debugfs_create_file("pending_async_subdevices", 0444, debugfs_dir, NULL,
> + &pending_subdevs_fops);
> +}
> diff --git a/drivers/media/v4l2-core/v4l2-dev.c b/drivers/media/v4l2-core/v4l2-dev.c
> index a593ea0598b5..8d3813e6ab56 100644
> --- a/drivers/media/v4l2-core/v4l2-dev.c
> +++ b/drivers/media/v4l2-core/v4l2-dev.c
> @@ -14,6 +14,7 @@
>
> #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
>
> +#include <linux/debugfs.h>
> #include <linux/module.h>
> #include <linux/types.h>
> #include <linux/kernel.h>
> @@ -37,6 +38,7 @@
> __func__, ##arg); \
> } while (0)
>
> +static struct dentry *v4l2_debugfs_dir;
>
> /*
> * sysfs stuff
> @@ -1113,6 +1115,8 @@ static int __init videodev_init(void)
> return -EIO;
> }
>
> + v4l2_debugfs_dir = debugfs_create_dir("video4linux", NULL);
> + v4l2_async_debug_init(v4l2_debugfs_dir);
> return 0;
> }
>
> @@ -1120,6 +1124,7 @@ static void __exit videodev_exit(void)
> {
> dev_t dev = MKDEV(VIDEO_MAJOR, 0);
>
> + debugfs_remove_recursive(v4l2_debugfs_dir);
> class_unregister(&video_class);
> unregister_chrdev_region(dev, VIDEO_NUM_DEVICES);
> }
> diff --git a/include/media/v4l2-async.h b/include/media/v4l2-async.h
> index 8ed42188e7c9..26296d4cb660 100644
> --- a/include/media/v4l2-async.h
> +++ b/include/media/v4l2-async.h
> @@ -11,6 +11,7 @@
> #include <linux/list.h>
> #include <linux/mutex.h>
>
> +struct dentry;
> struct device;
> struct device_node;
> struct v4l2_device;
> @@ -120,6 +121,13 @@ struct v4l2_async_notifier {
> struct list_head list;
> };
>
> +/**
> + * v4l2_async_debug_init - Initialize debugging tools.
> + *
> + * @debugfs_dir: pointer to the parent debugfs &struct dentry
> + */
> +void v4l2_async_debug_init(struct dentry *debugfs_dir);
> +
> /**
> * v4l2_async_notifier_init - Initialize a notifier.
> *
>
^ permalink raw reply [flat|nested] 12+ messages in thread* [PATCH v4] media: v4l2-async: Add waiting subdevices debugfs
2021-01-08 17:17 ` [PATCH v3] media: v4l2-async: Add waiting subdevices debugfs Ezequiel Garcia
2021-01-14 23:36 ` Sakari Ailus
2021-01-15 10:34 ` Hans Verkuil
@ 2021-01-15 19:14 ` Ezequiel Garcia
2021-01-15 19:47 ` Kieran Bingham
2 siblings, 1 reply; 12+ messages in thread
From: Ezequiel Garcia @ 2021-01-15 19:14 UTC (permalink / raw)
To: linux-media, Hans Verkuil
Cc: kernel, Laurent Pinchart, Sakari Ailus, Ezequiel Garcia, Hans Verkuil
There is currently little to no information available
about the reasons why a v4l2-async device hasn't
probed completely.
Inspired by the "devices_deferred" debugfs file,
add a file to list information about the subdevices
that are on waiting lists, for each notifier.
This is useful to debug v4l2-async subdevices
and notifiers, for instance when doing device bring-up.
For instance, a typical output would be:
$ cat /sys/kernel/debug/video4linux/pending_async_subdevices
ipu1_csi1:
[fwnode] dev=20e0000.iomuxc-gpr:ipu1_csi1_mux, node=/soc/bus@2000000/iomuxc-gpr@20e0000/ipu1_csi1_mux
ipu1_csi0:
[fwnode] dev=20e0000.iomuxc-gpr:ipu1_csi0_mux, node=/soc/bus@2000000/iomuxc-gpr@20e0000/ipu1_csi0_mux
imx6-mipi-csi2:
[fwnode] dev=1-003c, node=/soc/bus@2100000/i2c@21a4000/camera@3c
imx-media:
Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
---
drivers/media/v4l2-core/v4l2-async.c | 63 ++++++++++++++++++++++++++++
drivers/media/v4l2-core/v4l2-dev.c | 5 +++
include/media/v4l2-async.h | 8 ++++
3 files changed, 76 insertions(+)
diff --git a/drivers/media/v4l2-core/v4l2-async.c b/drivers/media/v4l2-core/v4l2-async.c
index e3ab003a6c85..e35f18706792 100644
--- a/drivers/media/v4l2-core/v4l2-async.c
+++ b/drivers/media/v4l2-core/v4l2-async.c
@@ -5,6 +5,7 @@
* Copyright (C) 2012-2013, Guennadi Liakhovetski <g.liakhovetski@gmx.de>
*/
+#include <linux/debugfs.h>
#include <linux/device.h>
#include <linux/err.h>
#include <linux/i2c.h>
@@ -14,6 +15,7 @@
#include <linux/mutex.h>
#include <linux/of.h>
#include <linux/platform_device.h>
+#include <linux/seq_file.h>
#include <linux/slab.h>
#include <linux/types.h>
@@ -837,3 +839,64 @@ void v4l2_async_unregister_subdev(struct v4l2_subdev *sd)
mutex_unlock(&list_lock);
}
EXPORT_SYMBOL(v4l2_async_unregister_subdev);
+
+static void print_waiting_subdev(struct seq_file *s,
+ struct v4l2_async_subdev *asd)
+{
+ switch (asd->match_type) {
+ case V4L2_ASYNC_MATCH_I2C:
+ seq_printf(s, " [i2c] dev=%d-%04x\n", asd->match.i2c.adapter_id,
+ asd->match.i2c.address);
+ break;
+ case V4L2_ASYNC_MATCH_FWNODE: {
+ struct fwnode_handle *devnode, *fwnode = asd->match.fwnode;
+
+ devnode = fwnode_graph_is_endpoint(fwnode) ?
+ fwnode_graph_get_port_parent(fwnode) :
+ fwnode_handle_get(fwnode);
+
+ seq_printf(s, " [fwnode] dev=%s, node=%pfw\n",
+ devnode->dev ? dev_name(devnode->dev) : "nil",
+ fwnode);
+
+ fwnode_handle_put(devnode);
+ break;
+ }
+ }
+}
+
+static const char *
+v4l2_async_notifier_name(struct v4l2_async_notifier *notifier)
+{
+ if (notifier->v4l2_dev)
+ return notifier->v4l2_dev->name;
+ else if (notifier->sd)
+ return notifier->sd->name;
+ else
+ return "nil";
+}
+
+static int pending_subdevs_show(struct seq_file *s, void *data)
+{
+ struct v4l2_async_notifier *notif;
+ struct v4l2_async_subdev *asd;
+
+ mutex_lock(&list_lock);
+
+ list_for_each_entry(notif, ¬ifier_list, list) {
+ seq_printf(s, "%s:\n", v4l2_async_notifier_name(notif));
+ list_for_each_entry(asd, ¬if->waiting, list)
+ print_waiting_subdev(s, asd);
+ }
+
+ mutex_unlock(&list_lock);
+
+ return 0;
+}
+DEFINE_SHOW_ATTRIBUTE(pending_subdevs);
+
+void v4l2_async_debug_init(struct dentry *debugfs_dir)
+{
+ debugfs_create_file("pending_async_subdevices", 0444, debugfs_dir, NULL,
+ &pending_subdevs_fops);
+}
diff --git a/drivers/media/v4l2-core/v4l2-dev.c b/drivers/media/v4l2-core/v4l2-dev.c
index f9cff033d0dc..b6a72d297775 100644
--- a/drivers/media/v4l2-core/v4l2-dev.c
+++ b/drivers/media/v4l2-core/v4l2-dev.c
@@ -14,6 +14,7 @@
#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+#include <linux/debugfs.h>
#include <linux/module.h>
#include <linux/types.h>
#include <linux/kernel.h>
@@ -38,6 +39,7 @@
__func__, ##arg); \
} while (0)
+static struct dentry *v4l2_debugfs_dir;
/*
* sysfs stuff
@@ -1118,6 +1120,8 @@ static int __init videodev_init(void)
return -EIO;
}
+ v4l2_debugfs_dir = debugfs_create_dir("video4linux", NULL);
+ v4l2_async_debug_init(v4l2_debugfs_dir);
return 0;
}
@@ -1125,6 +1129,7 @@ static void __exit videodev_exit(void)
{
dev_t dev = MKDEV(VIDEO_MAJOR, 0);
+ debugfs_remove_recursive(v4l2_debugfs_dir);
class_unregister(&video_class);
unregister_chrdev_region(dev, VIDEO_NUM_DEVICES);
}
diff --git a/include/media/v4l2-async.h b/include/media/v4l2-async.h
index 0e04b5b2ebb0..243ac10a53c6 100644
--- a/include/media/v4l2-async.h
+++ b/include/media/v4l2-async.h
@@ -11,6 +11,7 @@
#include <linux/list.h>
#include <linux/mutex.h>
+struct dentry;
struct device;
struct device_node;
struct v4l2_device;
@@ -137,6 +138,13 @@ struct v4l2_async_notifier {
struct list_head list;
};
+/**
+ * v4l2_async_debug_init - Initialize debugging tools.
+ *
+ * @debugfs_dir: pointer to the parent debugfs &struct dentry
+ */
+void v4l2_async_debug_init(struct dentry *debugfs_dir);
+
/**
* v4l2_async_notifier_init - Initialize a notifier.
*
--
2.29.2
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH v4] media: v4l2-async: Add waiting subdevices debugfs
2021-01-15 19:14 ` [PATCH v4] " Ezequiel Garcia
@ 2021-01-15 19:47 ` Kieran Bingham
2021-01-20 12:12 ` Kieran Bingham
0 siblings, 1 reply; 12+ messages in thread
From: Kieran Bingham @ 2021-01-15 19:47 UTC (permalink / raw)
To: Ezequiel Garcia, linux-media, Hans Verkuil
Cc: kernel, Laurent Pinchart, Sakari Ailus, Hans Verkuil
Hi Ezequiel,
On 15/01/2021 19:14, Ezequiel Garcia wrote:
> There is currently little to no information available
> about the reasons why a v4l2-async device hasn't
> probed completely.
>
> Inspired by the "devices_deferred" debugfs file,
> add a file to list information about the subdevices
> that are on waiting lists, for each notifier.
>
> This is useful to debug v4l2-async subdevices
> and notifiers, for instance when doing device bring-up.
>
> For instance, a typical output would be:
>
> $ cat /sys/kernel/debug/video4linux/pending_async_subdevices
> ipu1_csi1:
> [fwnode] dev=20e0000.iomuxc-gpr:ipu1_csi1_mux, node=/soc/bus@2000000/iomuxc-gpr@20e0000/ipu1_csi1_mux
> ipu1_csi0:
> [fwnode] dev=20e0000.iomuxc-gpr:ipu1_csi0_mux, node=/soc/bus@2000000/iomuxc-gpr@20e0000/ipu1_csi0_mux
> imx6-mipi-csi2:
> [fwnode] dev=1-003c, node=/soc/bus@2100000/i2c@21a4000/camera@3c
> imx-media:
Oh this is very exciting. I started looking at something like this
recently, hitting async failures, and this already looks better.
In other words, - Thank you!
Reviewed-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
> Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Reviewed-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
> ---
> drivers/media/v4l2-core/v4l2-async.c | 63 ++++++++++++++++++++++++++++
> drivers/media/v4l2-core/v4l2-dev.c | 5 +++
> include/media/v4l2-async.h | 8 ++++
> 3 files changed, 76 insertions(+)
>
> diff --git a/drivers/media/v4l2-core/v4l2-async.c b/drivers/media/v4l2-core/v4l2-async.c
> index e3ab003a6c85..e35f18706792 100644
> --- a/drivers/media/v4l2-core/v4l2-async.c
> +++ b/drivers/media/v4l2-core/v4l2-async.c
> @@ -5,6 +5,7 @@
> * Copyright (C) 2012-2013, Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> */
>
> +#include <linux/debugfs.h>
> #include <linux/device.h>
> #include <linux/err.h>
> #include <linux/i2c.h>
> @@ -14,6 +15,7 @@
> #include <linux/mutex.h>
> #include <linux/of.h>
> #include <linux/platform_device.h>
> +#include <linux/seq_file.h>
> #include <linux/slab.h>
> #include <linux/types.h>
>
> @@ -837,3 +839,64 @@ void v4l2_async_unregister_subdev(struct v4l2_subdev *sd)
> mutex_unlock(&list_lock);
> }
> EXPORT_SYMBOL(v4l2_async_unregister_subdev);
> +
> +static void print_waiting_subdev(struct seq_file *s,
> + struct v4l2_async_subdev *asd)
> +{
> + switch (asd->match_type) {
> + case V4L2_ASYNC_MATCH_I2C:
> + seq_printf(s, " [i2c] dev=%d-%04x\n", asd->match.i2c.adapter_id,
> + asd->match.i2c.address);
> + break;
> + case V4L2_ASYNC_MATCH_FWNODE: {
> + struct fwnode_handle *devnode, *fwnode = asd->match.fwnode;
> +
> + devnode = fwnode_graph_is_endpoint(fwnode) ?
> + fwnode_graph_get_port_parent(fwnode) :
> + fwnode_handle_get(fwnode);
> +
> + seq_printf(s, " [fwnode] dev=%s, node=%pfw\n",
> + devnode->dev ? dev_name(devnode->dev) : "nil",
> + fwnode);
> +
> + fwnode_handle_put(devnode);
> + break;
> + }
> + }
> +}
> +
> +static const char *
> +v4l2_async_notifier_name(struct v4l2_async_notifier *notifier)
> +{
> + if (notifier->v4l2_dev)
> + return notifier->v4l2_dev->name;
> + else if (notifier->sd)
> + return notifier->sd->name;
> + else
> + return "nil";
> +}
> +
> +static int pending_subdevs_show(struct seq_file *s, void *data)
> +{
> + struct v4l2_async_notifier *notif;
> + struct v4l2_async_subdev *asd;
> +
> + mutex_lock(&list_lock);
> +
> + list_for_each_entry(notif, ¬ifier_list, list) {
> + seq_printf(s, "%s:\n", v4l2_async_notifier_name(notif));
> + list_for_each_entry(asd, ¬if->waiting, list)
> + print_waiting_subdev(s, asd);
> + }
> +
> + mutex_unlock(&list_lock);
> +
> + return 0;
> +}
> +DEFINE_SHOW_ATTRIBUTE(pending_subdevs);
> +
> +void v4l2_async_debug_init(struct dentry *debugfs_dir)
> +{
> + debugfs_create_file("pending_async_subdevices", 0444, debugfs_dir, NULL,
> + &pending_subdevs_fops);
> +}
> diff --git a/drivers/media/v4l2-core/v4l2-dev.c b/drivers/media/v4l2-core/v4l2-dev.c
> index f9cff033d0dc..b6a72d297775 100644
> --- a/drivers/media/v4l2-core/v4l2-dev.c
> +++ b/drivers/media/v4l2-core/v4l2-dev.c
> @@ -14,6 +14,7 @@
>
> #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
>
> +#include <linux/debugfs.h>
> #include <linux/module.h>
> #include <linux/types.h>
> #include <linux/kernel.h>
> @@ -38,6 +39,7 @@
> __func__, ##arg); \
> } while (0)
>
> +static struct dentry *v4l2_debugfs_dir;
>
> /*
> * sysfs stuff
> @@ -1118,6 +1120,8 @@ static int __init videodev_init(void)
> return -EIO;
> }
>
> + v4l2_debugfs_dir = debugfs_create_dir("video4linux", NULL);
> + v4l2_async_debug_init(v4l2_debugfs_dir);
> return 0;
> }
>
> @@ -1125,6 +1129,7 @@ static void __exit videodev_exit(void)
> {
> dev_t dev = MKDEV(VIDEO_MAJOR, 0);
>
> + debugfs_remove_recursive(v4l2_debugfs_dir);
> class_unregister(&video_class);
> unregister_chrdev_region(dev, VIDEO_NUM_DEVICES);
> }
> diff --git a/include/media/v4l2-async.h b/include/media/v4l2-async.h
> index 0e04b5b2ebb0..243ac10a53c6 100644
> --- a/include/media/v4l2-async.h
> +++ b/include/media/v4l2-async.h
> @@ -11,6 +11,7 @@
> #include <linux/list.h>
> #include <linux/mutex.h>
>
> +struct dentry;
> struct device;
> struct device_node;
> struct v4l2_device;
> @@ -137,6 +138,13 @@ struct v4l2_async_notifier {
> struct list_head list;
> };
>
> +/**
> + * v4l2_async_debug_init - Initialize debugging tools.
> + *
> + * @debugfs_dir: pointer to the parent debugfs &struct dentry
> + */
> +void v4l2_async_debug_init(struct dentry *debugfs_dir);
> +
> /**
> * v4l2_async_notifier_init - Initialize a notifier.
> *
>
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH v4] media: v4l2-async: Add waiting subdevices debugfs
2021-01-15 19:47 ` Kieran Bingham
@ 2021-01-20 12:12 ` Kieran Bingham
0 siblings, 0 replies; 12+ messages in thread
From: Kieran Bingham @ 2021-01-20 12:12 UTC (permalink / raw)
To: Ezequiel Garcia, linux-media, Hans Verkuil
Cc: kernel, Laurent Pinchart, Sakari Ailus, Hans Verkuil
Hi Ezequiel,
On 15/01/2021 19:47, Kieran Bingham wrote:
> Hi Ezequiel,
>
> On 15/01/2021 19:14, Ezequiel Garcia wrote:
>> There is currently little to no information available
>> about the reasons why a v4l2-async device hasn't
>> probed completely.
>>
>> Inspired by the "devices_deferred" debugfs file,
>> add a file to list information about the subdevices
>> that are on waiting lists, for each notifier.
>>
>> This is useful to debug v4l2-async subdevices
>> and notifiers, for instance when doing device bring-up.
>>
>> For instance, a typical output would be:
>>
>> $ cat /sys/kernel/debug/video4linux/pending_async_subdevices
>> ipu1_csi1:
>> [fwnode] dev=20e0000.iomuxc-gpr:ipu1_csi1_mux, node=/soc/bus@2000000/iomuxc-gpr@20e0000/ipu1_csi1_mux
>> ipu1_csi0:
>> [fwnode] dev=20e0000.iomuxc-gpr:ipu1_csi0_mux, node=/soc/bus@2000000/iomuxc-gpr@20e0000/ipu1_csi0_mux
>> imx6-mipi-csi2:
>> [fwnode] dev=1-003c, node=/soc/bus@2100000/i2c@21a4000/camera@3c
>> imx-media:
>
> Oh this is very exciting. I started looking at something like this
> recently, hitting async failures, and this already looks better.
>
> In other words, - Thank you!
>
> Reviewed-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
Throwing in an extra tag:
> root@salvator-x:/home/kbingham# cat /sys/kernel/debug/video4linux/pending_async_subdevices
> max9286 4-006c:
> [fwnode] dev=15-0035, node=/soc/i2c@e66d8000/gmsl-deserializer@6c/i2c-mux/i2c@0/camera@35/port/endpoint
> max9286 4-004c:
> [fwnode] dev=10-0031, node=/soc/i2c@e66d8000/gmsl-deserializer@4c/i2c-mux/i2c@0/camera@31/port/endpoint
> [fwnode] dev=11-0032, node=/soc/i2c@e66d8000/gmsl-deserializer@4c/i2c-mux/i2c@1/camera@32/port/endpoint
> [fwnode] dev=12-0033, node=/soc/i2c@e66d8000/gmsl-deserializer@4c/i2c-mux/i2c@2/camera@33/port/endpoint
> [fwnode] dev=13-0034, node=/soc/i2c@e66d8000/gmsl-deserializer@4c/i2c-mux/i2c@3/camera@34/port/endpoint
> rcar-vin e6ef7000.video:
> rcar_csi2 feab0000.csi2:
> rcar_csi2 feaa0000.csi2:
> rcar_csi2 fea80000.csi2:
That looks good and really helpful to me ;-)
Tested-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
>
>> Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
>> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>> Reviewed-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
>> ---
>> drivers/media/v4l2-core/v4l2-async.c | 63 ++++++++++++++++++++++++++++
>> drivers/media/v4l2-core/v4l2-dev.c | 5 +++
>> include/media/v4l2-async.h | 8 ++++
>> 3 files changed, 76 insertions(+)
>>
>> diff --git a/drivers/media/v4l2-core/v4l2-async.c b/drivers/media/v4l2-core/v4l2-async.c
>> index e3ab003a6c85..e35f18706792 100644
>> --- a/drivers/media/v4l2-core/v4l2-async.c
>> +++ b/drivers/media/v4l2-core/v4l2-async.c
>> @@ -5,6 +5,7 @@
>> * Copyright (C) 2012-2013, Guennadi Liakhovetski <g.liakhovetski@gmx.de>
>> */
>>
>> +#include <linux/debugfs.h>
>> #include <linux/device.h>
>> #include <linux/err.h>
>> #include <linux/i2c.h>
>> @@ -14,6 +15,7 @@
>> #include <linux/mutex.h>
>> #include <linux/of.h>
>> #include <linux/platform_device.h>
>> +#include <linux/seq_file.h>
>> #include <linux/slab.h>
>> #include <linux/types.h>
>>
>> @@ -837,3 +839,64 @@ void v4l2_async_unregister_subdev(struct v4l2_subdev *sd)
>> mutex_unlock(&list_lock);
>> }
>> EXPORT_SYMBOL(v4l2_async_unregister_subdev);
>> +
>> +static void print_waiting_subdev(struct seq_file *s,
>> + struct v4l2_async_subdev *asd)
>> +{
>> + switch (asd->match_type) {
>> + case V4L2_ASYNC_MATCH_I2C:
>> + seq_printf(s, " [i2c] dev=%d-%04x\n", asd->match.i2c.adapter_id,
>> + asd->match.i2c.address);
>> + break;
>> + case V4L2_ASYNC_MATCH_FWNODE: {
>> + struct fwnode_handle *devnode, *fwnode = asd->match.fwnode;
>> +
>> + devnode = fwnode_graph_is_endpoint(fwnode) ?
>> + fwnode_graph_get_port_parent(fwnode) :
>> + fwnode_handle_get(fwnode);
>> +
>> + seq_printf(s, " [fwnode] dev=%s, node=%pfw\n",
>> + devnode->dev ? dev_name(devnode->dev) : "nil",
>> + fwnode);
>> +
>> + fwnode_handle_put(devnode);
>> + break;
>> + }
>> + }
>> +}
>> +
>> +static const char *
>> +v4l2_async_notifier_name(struct v4l2_async_notifier *notifier)
>> +{
>> + if (notifier->v4l2_dev)
>> + return notifier->v4l2_dev->name;
>> + else if (notifier->sd)
>> + return notifier->sd->name;
>> + else
>> + return "nil";
>> +}
>> +
>> +static int pending_subdevs_show(struct seq_file *s, void *data)
>> +{
>> + struct v4l2_async_notifier *notif;
>> + struct v4l2_async_subdev *asd;
>> +
>> + mutex_lock(&list_lock);
>> +
>> + list_for_each_entry(notif, ¬ifier_list, list) {
>> + seq_printf(s, "%s:\n", v4l2_async_notifier_name(notif));
>> + list_for_each_entry(asd, ¬if->waiting, list)
>> + print_waiting_subdev(s, asd);
>> + }
>> +
>> + mutex_unlock(&list_lock);
>> +
>> + return 0;
>> +}
>> +DEFINE_SHOW_ATTRIBUTE(pending_subdevs);
>> +
>> +void v4l2_async_debug_init(struct dentry *debugfs_dir)
>> +{
>> + debugfs_create_file("pending_async_subdevices", 0444, debugfs_dir, NULL,
>> + &pending_subdevs_fops);
>> +}
>> diff --git a/drivers/media/v4l2-core/v4l2-dev.c b/drivers/media/v4l2-core/v4l2-dev.c
>> index f9cff033d0dc..b6a72d297775 100644
>> --- a/drivers/media/v4l2-core/v4l2-dev.c
>> +++ b/drivers/media/v4l2-core/v4l2-dev.c
>> @@ -14,6 +14,7 @@
>>
>> #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
>>
>> +#include <linux/debugfs.h>
>> #include <linux/module.h>
>> #include <linux/types.h>
>> #include <linux/kernel.h>
>> @@ -38,6 +39,7 @@
>> __func__, ##arg); \
>> } while (0)
>>
>> +static struct dentry *v4l2_debugfs_dir;
>>
>> /*
>> * sysfs stuff
>> @@ -1118,6 +1120,8 @@ static int __init videodev_init(void)
>> return -EIO;
>> }
>>
>> + v4l2_debugfs_dir = debugfs_create_dir("video4linux", NULL);
>> + v4l2_async_debug_init(v4l2_debugfs_dir);
>> return 0;
>> }
>>
>> @@ -1125,6 +1129,7 @@ static void __exit videodev_exit(void)
>> {
>> dev_t dev = MKDEV(VIDEO_MAJOR, 0);
>>
>> + debugfs_remove_recursive(v4l2_debugfs_dir);
>> class_unregister(&video_class);
>> unregister_chrdev_region(dev, VIDEO_NUM_DEVICES);
>> }
>> diff --git a/include/media/v4l2-async.h b/include/media/v4l2-async.h
>> index 0e04b5b2ebb0..243ac10a53c6 100644
>> --- a/include/media/v4l2-async.h
>> +++ b/include/media/v4l2-async.h
>> @@ -11,6 +11,7 @@
>> #include <linux/list.h>
>> #include <linux/mutex.h>
>>
>> +struct dentry;
>> struct device;
>> struct device_node;
>> struct v4l2_device;
>> @@ -137,6 +138,13 @@ struct v4l2_async_notifier {
>> struct list_head list;
>> };
>>
>> +/**
>> + * v4l2_async_debug_init - Initialize debugging tools.
>> + *
>> + * @debugfs_dir: pointer to the parent debugfs &struct dentry
>> + */
>> +void v4l2_async_debug_init(struct dentry *debugfs_dir);
>> +
>> /**
>> * v4l2_async_notifier_init - Initialize a notifier.
>> *
>>
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] media: v4l2-async: Remove V4L2_ASYNC_MATCH_CUSTOM
2021-01-08 17:17 [PATCH] media: v4l2-async: Remove V4L2_ASYNC_MATCH_CUSTOM Ezequiel Garcia
2021-01-08 17:17 ` [PATCH v3] media: v4l2-async: Add waiting subdevices debugfs Ezequiel Garcia
@ 2021-01-09 0:27 ` Laurent Pinchart
2021-01-15 19:40 ` Kieran Bingham
2 siblings, 0 replies; 12+ messages in thread
From: Laurent Pinchart @ 2021-01-09 0:27 UTC (permalink / raw)
To: Ezequiel Garcia; +Cc: linux-media, Hans Verkuil, kernel, Sakari Ailus
Hi Ezequiel,
Thank you for the patch.
On Fri, Jan 08, 2021 at 02:17:27PM -0300, Ezequiel Garcia wrote:
> Custom/driver-specific v4l2-async match support was introduced
> in 2013, as V4L2_ASYNC_BUS_CUSTOM.
>
> This type of match never had any user, so it's fair
> to conclude it's not required and that safe for removal.
> If the support is ever needed, it can always be restored.
>
> Suggested-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
> drivers/media/v4l2-core/v4l2-async.c | 14 --------------
> include/media/v4l2-async.h | 17 -----------------
> 2 files changed, 31 deletions(-)
>
> diff --git a/drivers/media/v4l2-core/v4l2-async.c b/drivers/media/v4l2-core/v4l2-async.c
> index e3ab003a6c85..3faf1d12d49d 100644
> --- a/drivers/media/v4l2-core/v4l2-async.c
> +++ b/drivers/media/v4l2-core/v4l2-async.c
> @@ -139,16 +139,6 @@ static bool match_fwnode(struct v4l2_async_notifier *notifier,
> return true;
> }
>
> -static bool match_custom(struct v4l2_async_notifier *notifier,
> - struct v4l2_subdev *sd, struct v4l2_async_subdev *asd)
> -{
> - if (!asd->match.custom.match)
> - /* Match always */
> - return true;
> -
> - return asd->match.custom.match(sd->dev, asd);
> -}
> -
> static LIST_HEAD(subdev_list);
> static LIST_HEAD(notifier_list);
> static DEFINE_MUTEX(list_lock);
> @@ -164,9 +154,6 @@ v4l2_async_find_match(struct v4l2_async_notifier *notifier,
> list_for_each_entry(asd, ¬ifier->waiting, list) {
> /* bus_type has been verified valid before */
> switch (asd->match_type) {
> - case V4L2_ASYNC_MATCH_CUSTOM:
> - match = match_custom;
> - break;
> case V4L2_ASYNC_MATCH_DEVNAME:
> match = match_devname;
> break;
> @@ -467,7 +454,6 @@ static int v4l2_async_notifier_asd_valid(struct v4l2_async_notifier *notifier,
> return -EINVAL;
>
> switch (asd->match_type) {
> - case V4L2_ASYNC_MATCH_CUSTOM:
> case V4L2_ASYNC_MATCH_DEVNAME:
> case V4L2_ASYNC_MATCH_I2C:
> case V4L2_ASYNC_MATCH_FWNODE:
> diff --git a/include/media/v4l2-async.h b/include/media/v4l2-async.h
> index 0e04b5b2ebb0..8ed42188e7c9 100644
> --- a/include/media/v4l2-async.h
> +++ b/include/media/v4l2-async.h
> @@ -21,8 +21,6 @@ struct v4l2_async_notifier;
> * enum v4l2_async_match_type - type of asynchronous subdevice logic to be used
> * in order to identify a match
> *
> - * @V4L2_ASYNC_MATCH_CUSTOM: Match will use the logic provided by &struct
> - * v4l2_async_subdev.match ops
> * @V4L2_ASYNC_MATCH_DEVNAME: Match will use the device name
> * @V4L2_ASYNC_MATCH_I2C: Match will check for I2C adapter ID and address
> * @V4L2_ASYNC_MATCH_FWNODE: Match will use firmware node
> @@ -31,7 +29,6 @@ struct v4l2_async_notifier;
> * algorithm that will be used to match an asynchronous device.
> */
> enum v4l2_async_match_type {
> - V4L2_ASYNC_MATCH_CUSTOM,
> V4L2_ASYNC_MATCH_DEVNAME,
> V4L2_ASYNC_MATCH_I2C,
> V4L2_ASYNC_MATCH_FWNODE,
> @@ -58,15 +55,6 @@ enum v4l2_async_match_type {
> * @match.i2c.address:
> * I2C address to be matched.
> * Used if @match_type is %V4L2_ASYNC_MATCH_I2C.
> - * @match.custom:
> - * Driver-specific match criteria.
> - * Used if @match_type is %V4L2_ASYNC_MATCH_CUSTOM.
> - * @match.custom.match:
> - * Driver-specific match function to be used if
> - * %V4L2_ASYNC_MATCH_CUSTOM.
> - * @match.custom.priv:
> - * Driver-specific private struct with match parameters
> - * to be used if %V4L2_ASYNC_MATCH_CUSTOM.
> * @asd_list: used to add struct v4l2_async_subdev objects to the
> * master notifier @asd_list
> * @list: used to link struct v4l2_async_subdev objects, waiting to be
> @@ -85,11 +73,6 @@ struct v4l2_async_subdev {
> int adapter_id;
> unsigned short address;
> } i2c;
> - struct {
> - bool (*match)(struct device *dev,
> - struct v4l2_async_subdev *sd);
> - void *priv;
> - } custom;
> } match;
>
> /* v4l2-async core private: not to be used by drivers */
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH] media: v4l2-async: Remove V4L2_ASYNC_MATCH_CUSTOM
2021-01-08 17:17 [PATCH] media: v4l2-async: Remove V4L2_ASYNC_MATCH_CUSTOM Ezequiel Garcia
2021-01-08 17:17 ` [PATCH v3] media: v4l2-async: Add waiting subdevices debugfs Ezequiel Garcia
2021-01-09 0:27 ` [PATCH] media: v4l2-async: Remove V4L2_ASYNC_MATCH_CUSTOM Laurent Pinchart
@ 2021-01-15 19:40 ` Kieran Bingham
2 siblings, 0 replies; 12+ messages in thread
From: Kieran Bingham @ 2021-01-15 19:40 UTC (permalink / raw)
To: Ezequiel Garcia, linux-media, Hans Verkuil
Cc: kernel, Laurent Pinchart, Sakari Ailus
Hi Ezequiel,
On 08/01/2021 17:17, Ezequiel Garcia wrote:
> Custom/driver-specific v4l2-async match support was introduced
> in 2013, as V4L2_ASYNC_BUS_CUSTOM.
>
> This type of match never had any user, so it's fair
> to conclude it's not required and that safe for removal.
> If the support is ever needed, it can always be restored.
>
Simplify v4l2-async? Yes please!
Reviewed-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
> Suggested-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
> ---
> drivers/media/v4l2-core/v4l2-async.c | 14 --------------
> include/media/v4l2-async.h | 17 -----------------
> 2 files changed, 31 deletions(-)
>
> diff --git a/drivers/media/v4l2-core/v4l2-async.c b/drivers/media/v4l2-core/v4l2-async.c
> index e3ab003a6c85..3faf1d12d49d 100644
> --- a/drivers/media/v4l2-core/v4l2-async.c
> +++ b/drivers/media/v4l2-core/v4l2-async.c
> @@ -139,16 +139,6 @@ static bool match_fwnode(struct v4l2_async_notifier *notifier,
> return true;
> }
>
> -static bool match_custom(struct v4l2_async_notifier *notifier,
> - struct v4l2_subdev *sd, struct v4l2_async_subdev *asd)
> -{
> - if (!asd->match.custom.match)
> - /* Match always */
> - return true;
> -
> - return asd->match.custom.match(sd->dev, asd);
> -}
> -
> static LIST_HEAD(subdev_list);
> static LIST_HEAD(notifier_list);
> static DEFINE_MUTEX(list_lock);
> @@ -164,9 +154,6 @@ v4l2_async_find_match(struct v4l2_async_notifier *notifier,
> list_for_each_entry(asd, ¬ifier->waiting, list) {
> /* bus_type has been verified valid before */
> switch (asd->match_type) {
> - case V4L2_ASYNC_MATCH_CUSTOM:
> - match = match_custom;
> - break;
> case V4L2_ASYNC_MATCH_DEVNAME:
> match = match_devname;
> break;
> @@ -467,7 +454,6 @@ static int v4l2_async_notifier_asd_valid(struct v4l2_async_notifier *notifier,
> return -EINVAL;
>
> switch (asd->match_type) {
> - case V4L2_ASYNC_MATCH_CUSTOM:
> case V4L2_ASYNC_MATCH_DEVNAME:
> case V4L2_ASYNC_MATCH_I2C:
> case V4L2_ASYNC_MATCH_FWNODE:
> diff --git a/include/media/v4l2-async.h b/include/media/v4l2-async.h
> index 0e04b5b2ebb0..8ed42188e7c9 100644
> --- a/include/media/v4l2-async.h
> +++ b/include/media/v4l2-async.h
> @@ -21,8 +21,6 @@ struct v4l2_async_notifier;
> * enum v4l2_async_match_type - type of asynchronous subdevice logic to be used
> * in order to identify a match
> *
> - * @V4L2_ASYNC_MATCH_CUSTOM: Match will use the logic provided by &struct
> - * v4l2_async_subdev.match ops
> * @V4L2_ASYNC_MATCH_DEVNAME: Match will use the device name
> * @V4L2_ASYNC_MATCH_I2C: Match will check for I2C adapter ID and address
> * @V4L2_ASYNC_MATCH_FWNODE: Match will use firmware node
> @@ -31,7 +29,6 @@ struct v4l2_async_notifier;
> * algorithm that will be used to match an asynchronous device.
> */
> enum v4l2_async_match_type {
> - V4L2_ASYNC_MATCH_CUSTOM,
> V4L2_ASYNC_MATCH_DEVNAME,
> V4L2_ASYNC_MATCH_I2C,
> V4L2_ASYNC_MATCH_FWNODE,
> @@ -58,15 +55,6 @@ enum v4l2_async_match_type {
> * @match.i2c.address:
> * I2C address to be matched.
> * Used if @match_type is %V4L2_ASYNC_MATCH_I2C.
> - * @match.custom:
> - * Driver-specific match criteria.
> - * Used if @match_type is %V4L2_ASYNC_MATCH_CUSTOM.
> - * @match.custom.match:
> - * Driver-specific match function to be used if
> - * %V4L2_ASYNC_MATCH_CUSTOM.
> - * @match.custom.priv:
> - * Driver-specific private struct with match parameters
> - * to be used if %V4L2_ASYNC_MATCH_CUSTOM.
> * @asd_list: used to add struct v4l2_async_subdev objects to the
> * master notifier @asd_list
> * @list: used to link struct v4l2_async_subdev objects, waiting to be
> @@ -85,11 +73,6 @@ struct v4l2_async_subdev {
> int adapter_id;
> unsigned short address;
> } i2c;
> - struct {
> - bool (*match)(struct device *dev,
> - struct v4l2_async_subdev *sd);
> - void *priv;
> - } custom;
> } match;
>
> /* v4l2-async core private: not to be used by drivers */
>
^ permalink raw reply [flat|nested] 12+ messages in thread