From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-15.8 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER, INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 5BB71C433E0 for ; Tue, 5 Jan 2021 03:47:23 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 2DF212256F for ; Tue, 5 Jan 2021 03:47:23 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1728221AbhAEDrH (ORCPT ); Mon, 4 Jan 2021 22:47:07 -0500 Received: from perceval.ideasonboard.com ([213.167.242.64]:60638 "EHLO perceval.ideasonboard.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726168AbhAEDrH (ORCPT ); Mon, 4 Jan 2021 22:47:07 -0500 Received: from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi [62.78.145.57]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 311BDDD; Tue, 5 Jan 2021 04:46:24 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1609818384; bh=eHDWnH6m1+eLhcCx84dnlp64GeJVIgEqWQP7+76DXE4=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=KIffBp0m1D+A07RbdhPsYfgoEj0O8eF25fOQfYFmk/QlzAly/fE5WdIl+9xMvLFri 6XDk8DcIqpe9MqdRHe8khZP5xFs+q2gVRmjA0JhbmWnonWXmTnjyn0/y3CrSaBqpig Po87clDBRP3bKoFEdIN2L+PdVvIph9APqSr1SUME= Date: Tue, 5 Jan 2021 05:46:11 +0200 From: Laurent Pinchart To: Ezequiel Garcia Cc: linux-media@vger.kernel.org, Hans Verkuil , kernel@collabora.com, Sakari Ailus Subject: Re: [PATCH v2] media: v4l2-async: Add waiting subdevices debugfs Message-ID: References: <20210104174840.144958-1-ezequiel@collabora.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20210104174840.144958-1-ezequiel@collabora.com> Precedence: bulk List-ID: X-Mailing-List: linux-media@vger.kernel.org Hi Ezequiel, Thank you for the patch. On Mon, Jan 04, 2021 at 02:48:40PM -0300, Ezequiel Garcia wrote: > There is currently little to none 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: > > Note that match-type "custom" prints no information. > Since there are no in-tree users of this match-type, > the implementation doesn't bother. I wonder if we should drop it. > Signed-off-by: Ezequiel Garcia > --- > 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 | 9 ++++ > 3 files changed, 80 insertions(+) > > diff --git a/drivers/media/v4l2-core/v4l2-async.c b/drivers/media/v4l2-core/v4l2-async.c > index e3ab003a6c85..d779808abb3b 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 > */ > > +#include > #include > #include > #include > @@ -14,6 +15,7 @@ > #include > #include > #include > +#include > #include > #include > > @@ -837,3 +839,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_CUSTOM: > + seq_puts(s, " [custom]\n"); > + break; > + case V4L2_ASYNC_MATCH_DEVNAME: > + seq_printf(s, " [devname] %s\n", asd->match.device_name); > + break; > + case V4L2_ASYNC_MATCH_I2C: > + seq_printf(s, " [i2c] %d-%04x\n", asd->match.i2c.adapter_id, > + asd->match.i2c.address); > + break; > + case V4L2_ASYNC_MATCH_FWNODE: { > + struct fwnode_handle *fwnode = asd->match.fwnode; > + > + if (fwnode_graph_is_endpoint(fwnode)) > + fwnode = fwnode_graph_get_port_parent(fwnode); > + > + seq_printf(s, " [fwnode] dev=%s, node=%pfw\n", > + fwnode->dev ? dev_name(fwnode->dev) : "nil", > + fwnode); Apart from Sakari's comment related to printing the endpoint node (but keeping the port's parent for ->dev), Reviewed-by: Laurent Pinchart > + 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 > #include > #include > #include > @@ -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 0e04b5b2ebb0..abc85474bb3b 100644 > --- a/include/media/v4l2-async.h > +++ b/include/media/v4l2-async.h > @@ -8,9 +8,11 @@ > #ifndef V4L2_ASYNC_H > #define V4L2_ASYNC_H > > +#include > #include > #include > > +struct dentry; > struct device; > struct device_node; > struct v4l2_device; > @@ -137,6 +139,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 > -- Regards, Laurent Pinchart