diff mbox series

[v3] media: v4l2-async: Add waiting subdevices debugfs

Message ID 20210108171728.39434-2-ezequiel@collabora.com
State Superseded
Headers show
Series [v3] media: v4l2-async: Add waiting subdevices debugfs | expand

Commit Message

Ezequiel Garcia Jan. 8, 2021, 5:17 p.m. UTC
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(+)

Comments

Sakari Ailus Jan. 14, 2021, 11:36 p.m. UTC | #1
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
Hans Verkuil Jan. 15, 2021, 10:34 a.m. UTC | #2
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, &notifier_list, list) {

> +		seq_printf(s, "%s:\n", v4l2_async_notifier_name(notif));

> +		list_for_each_entry(asd, &notif->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.

>   *

>
Ezequiel Garcia Jan. 15, 2021, 2:08 p.m. UTC | #3
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
Sakari Ailus Jan. 15, 2021, 3:05 p.m. UTC | #4
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
Ezequiel Garcia Jan. 15, 2021, 3:17 p.m. UTC | #5
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
Kieran Bingham Jan. 15, 2021, 7:47 p.m. UTC | #6
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, &notifier_list, list) {

> +		seq_printf(s, "%s:\n", v4l2_async_notifier_name(notif));

> +		list_for_each_entry(asd, &notif->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.

>   *

>
Kieran Bingham Jan. 20, 2021, 12:12 p.m. UTC | #7
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, &notifier_list, list) {

>> +		seq_printf(s, "%s:\n", v4l2_async_notifier_name(notif));

>> +		list_for_each_entry(asd, &notif->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.

>>   *

>>

>
diff mbox series

Patch

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, &notifier_list, list) {
+		seq_printf(s, "%s:\n", v4l2_async_notifier_name(notif));
+		list_for_each_entry(asd, &notif->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.
  *