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 Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 73DC3C05027 for ; Thu, 9 Feb 2023 22:35:11 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References: Message-ID:Subject:Cc:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=b2P3+msTX+WU+iuD7ZoTtqZalE30bjkAdvnk9/ZL/4Y=; b=iwGlwDByvqqFCH RmuxpYgmFmr94meXvXh9NjdZnaY0paMCEImUGB4zB69WDKqrdCD2s5J7qXJH9iZ5uLSLF2RuGWXof uLUfNivszqi2ilulU1JNsP4AIsrrLGQdjcw0Mx2sDWRbf68Hs1LL7KMjIkJywGF9SGhYZ0s+hpOY6 qWhfjJbuJZpymkQrYC0tuSAlquDQmQQjP3fp+qN1CruLHFAwZcvzO1s+5T3kKgF5JVizd1SL5S1dZ LTdVsQEWndpDd9EOk1DgfUCXeucF0zlHAZ+o0qwH6aIXoipgQPa3nDE9n24zEDJcM0+ENJ9J3Ogbk l1v7vN7Na15Z9Wz4xDtA==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1pQFUW-003Pao-3A; Thu, 09 Feb 2023 22:34:08 +0000 Received: from perceval.ideasonboard.com ([213.167.242.64]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1pQFUS-003PZy-Hd for linux-arm-kernel@lists.infradead.org; Thu, 09 Feb 2023 22:34:06 +0000 Received: from pendragon.ideasonboard.com (213-243-189-158.bb.dnainternet.fi [213.243.189.158]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 327BF9CA; Thu, 9 Feb 2023 23:34:02 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1675982042; bh=jXWrMiRYk03KcsgUdV0/ClmprNlQWdcYO5mIQE8UvlI=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=pFxdGij6epUEdSzCOfkHr816Vpiu9PFEPAIiPJ7Y0nP+c9V2DyLVwQlnKFT4N3Jjw wTvwrM9FZwVSnxqPASoyVKOBQD8Sza0APnV0V50icB6WQxbc3xJP157DyPoDD0gewH oQ9Wjf0GBalZ9hg37qV75SRzXHZD6EcUME0v1fi8= Date: Fri, 10 Feb 2023 00:34:01 +0200 From: Laurent Pinchart To: Sakari Ailus Cc: linux-media@vger.kernel.org, frieder.schrempf@kontron.de, Tim Harvey , linux-arm-kernel@lists.infradead.org, Dave Stevenson , Javier Martinez Canillas Subject: Re: [RFC 1/1] v4l: async: Add some debug prints Message-ID: References: <049f2fea-1725-74d9-d20d-fc4f7506d504@kontron.de> <20230209221634.35239-1-sakari.ailus@linux.intel.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20230209221634.35239-1-sakari.ailus@linux.intel.com> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20230209_143404_774858_9D44B8C2 X-CRM114-Status: GOOD ( 29.90 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org Hi Sakari, Thank you for the patch. On Fri, Feb 10, 2023 at 12:16:34AM +0200, Sakari Ailus wrote: > Just add some debug prints for V4L2 async sub-device matching process. These > might come useful in figuring out why things don't work as expected. > > Signed-off-by: Sakari Ailus > --- > Frieder, > > Can you try this? It prints what is being matched with what. Perhaps this > could be merged in a bit more refined form if it proves useful. > > Not tested in any way. > > drivers/media/v4l2-core/v4l2-async.c | 41 ++++++++++++++++++++++++---- > 1 file changed, 36 insertions(+), 5 deletions(-) > > diff --git a/drivers/media/v4l2-core/v4l2-async.c b/drivers/media/v4l2-core/v4l2-async.c > index 2f1b718a9189..6c13a9488415 100644 > --- a/drivers/media/v4l2-core/v4l2-async.c > +++ b/drivers/media/v4l2-core/v4l2-async.c > @@ -86,13 +86,18 @@ match_fwnode_one(struct v4l2_async_notifier *notifier, > bool sd_fwnode_is_ep; > struct device *dev; > > + dev_dbg(sd->dev, "async fwnode match %pfw vs. %pfw\n", sd_fwnode, > + asd->match.fwnode); Let's be more explicit: dev_dbg(sd->dev, "async fwnode match: need %pfw, trying %pfw\n", sd_fwnode, asd->match.fwnode); (feel free to adjust, as long as we differentiate what we're looking for from what we're testing) > + > /* > * Both the subdev and the async subdev can provide either an endpoint > * fwnode or a device fwnode. Start with the simple case of direct > * fwnode matching. > */ > - if (sd_fwnode == asd->match.fwnode) > + if (sd_fwnode == asd->match.fwnode) { > + dev_dbg(sd->dev, "true\n"); dev_dbg(sd->dev, "direct match found\n"); > return true; > + } > > /* > * Otherwise, check if the sd fwnode and the asd fwnode refer to an > @@ -105,8 +110,12 @@ match_fwnode_one(struct v4l2_async_notifier *notifier, > sd_fwnode_is_ep = fwnode_graph_is_endpoint(sd_fwnode); > asd_fwnode_is_ep = fwnode_graph_is_endpoint(asd->match.fwnode); > > - if (sd_fwnode_is_ep == asd_fwnode_is_ep) > + dev_dbg(sd->dev, "async fwnode match %pfw vs. %pfw\n", sd_fwnode, > + asd->match.fwnode); You've already printed this above, no need to repeat it. > + if (sd_fwnode_is_ep == asd_fwnode_is_ep) { > + dev_dbg(sd->dev, "unmatching node types (false)\n"); dev_dbg(sd->dev, "direct match not found\n"); > return false; > + } > > /* > * The sd and asd fwnodes are of different types. Get the device fwnode > @@ -120,10 +129,15 @@ match_fwnode_one(struct v4l2_async_notifier *notifier, > other_fwnode = sd_fwnode; > } > > + dev_dbg(sd->dev, "async fwnode (compat) match %pfw vs. %pfw\n", > + dev_fwnode, other_fwnode); Same comment as above regarding "vs." not telling which is which. > + > fwnode_handle_put(dev_fwnode); > > - if (dev_fwnode != other_fwnode) > + if (dev_fwnode != other_fwnode) { > + dev_dbg(sd->dev, "false\n"); dev_dbg(sd->dev, "compat match not found\n"); > return false; > + } > > /* > * We have a heterogeneous match. Retrieve the struct device of the side > @@ -143,6 +157,8 @@ match_fwnode_one(struct v4l2_async_notifier *notifier, > dev->driver->name); > } > > + dev_dbg(sd->dev, "true\n"); dev_dbg(sd->dev, "compat match found\n"); > + > return true; > } > > @@ -255,7 +271,10 @@ v4l2_async_nf_can_complete(struct v4l2_async_notifier *notifier) > v4l2_async_find_subdev_notifier(sd); > > if (subdev_notifier && > - !v4l2_async_nf_can_complete(subdev_notifier)) > + !v4l2_async_nf_can_complete(subdev_notifier)) { > + if (subdev_notifier->sd) > + deb_dbg(subdev_notifier->sd->dev, > + "cannot complete\n"); I'd add a reference to v4l2-async, either directly in the string, or with a "%s: ", __func__ prefix. Otherwise the message will be easy to miss. Same in other messages. Maybe a "v4l2-async: " prefix for all debug messages ? > return false; > } > > @@ -273,18 +292,27 @@ v4l2_async_nf_try_complete(struct v4l2_async_notifier *notifier) > if (!list_empty(¬ifier->waiting)) > return 0; > > + if (notifier->sd) > + deb_dbg(notifier->sd->dev, "trying to complete\n"); > + > /* Check the entire notifier tree; find the root notifier first. */ > while (notifier->parent) > notifier = notifier->parent; > > /* This is root if it has v4l2_dev. */ > - if (!notifier->v4l2_dev) > + if (!notifier->v4l2_dev) { > + if (notifier->sd) > + deb_dbg(notifier->sd->dev, > + "V4L2 device not available\n"); > return 0; > + } > > /* Is everything ready? */ > if (!v4l2_async_nf_can_complete(notifier)) > return 0; > > + deb_dbg(notifier->sd->dev, "complete\n"); You guard against notifier->sd being NULL above, but not here. At least one of the two is wrong. > + > return v4l2_async_nf_call_complete(notifier); > } > > @@ -386,6 +414,9 @@ v4l2_async_nf_try_all_subdevs(struct v4l2_async_notifier *notifier) > continue; > > ret = v4l2_async_match_notify(notifier, v4l2_dev, sd, asd); > + deb_dbg(sd->dev, "bound to %s's notifier (ret %d)\n", > + notifier->sd ? dev_name(notifier->sd->dev) : "no-dev", > + ret); > if (ret < 0) > return ret; > -- Regards, Laurent Pinchart _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel