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 vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 7DC32C3DA79 for ; Mon, 26 Dec 2022 17:01:20 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229612AbiLZRBT (ORCPT ); Mon, 26 Dec 2022 12:01:19 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:52736 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229472AbiLZRBS (ORCPT ); Mon, 26 Dec 2022 12:01:18 -0500 Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 45600C2B; Mon, 26 Dec 2022 09:01:17 -0800 (PST) Received: from pendragon.ideasonboard.com (213-243-189-158.bb.dnainternet.fi [213.243.189.158]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 2A2A0A1C; Mon, 26 Dec 2022 18:01:15 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1672074075; bh=irlftFZ6CQR1KhY24DKXhiPsC4bMOa8WIpYTJayoAko=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=UwbNYJNX9mUQw62WXtxT7GocsxtkWmIttmo9rMCNgN3YbY3abYEM8CZb2E9XpkLFi M6C7YpKJs/gWFrjk137hF75hdYg4j37D0bBoObHOdyuGj1sGMlaXO/nuv2bdnZzbED Mk3La9FZVU1h65y+mCwkLyzsX17UDfYvazEcJTus= Date: Mon, 26 Dec 2022 19:01:11 +0200 From: Laurent Pinchart To: Tomi Valkeinen Cc: linux-media@vger.kernel.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, linux-i2c@vger.kernel.org, Rob Herring , Krzysztof Kozlowski , Wolfram Sang , Luca Ceresoli , Andy Shevchenko , Matti Vaittinen , Mauro Carvalho Chehab , Peter Rosin , Liam Girdwood , Mark Brown , Sakari Ailus , Michael Tretter , Shawn Tu , Hans Verkuil , Mike Pagano , Krzysztof =?utf-8?Q?Ha=C5=82asa?= , Marek Vasut Subject: Re: [PATCH v5 7/8] media: i2c: add DS90UB913 driver Message-ID: References: <20221208104006.316606-1-tomi.valkeinen@ideasonboard.com> <20221208104006.316606-8-tomi.valkeinen@ideasonboard.com> <4d349785-ca37-d930-db3c-2581bba9fde0@ideasonboard.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <4d349785-ca37-d930-db3c-2581bba9fde0@ideasonboard.com> Precedence: bulk List-ID: X-Mailing-List: devicetree@vger.kernel.org Hi Tomi, On Wed, Dec 14, 2022 at 08:29:48AM +0200, Tomi Valkeinen wrote: > On 11/12/2022 20:33, Laurent Pinchart wrote: > > On Thu, Dec 08, 2022 at 12:40:05PM +0200, Tomi Valkeinen wrote: > >> Add driver for TI DS90UB913 FPDLink-3 Serializer. > >> > >> Signed-off-by: Tomi Valkeinen > >> --- > >> drivers/media/i2c/Kconfig | 13 + > >> drivers/media/i2c/Makefile | 2 +- > >> drivers/media/i2c/ds90ub913.c | 892 ++++++++++++++++++++++++++++++++++ > >> 3 files changed, 906 insertions(+), 1 deletion(-) > >> create mode 100644 drivers/media/i2c/ds90ub913.c [snip] > >> diff --git a/drivers/media/i2c/ds90ub913.c b/drivers/media/i2c/ds90ub913.c > >> new file mode 100644 > >> index 000000000000..6001a622e622 > >> --- /dev/null > >> +++ b/drivers/media/i2c/ds90ub913.c > >> @@ -0,0 +1,892 @@ [snip] > >> +static int ub913_notify_bound(struct v4l2_async_notifier *notifier, > >> + struct v4l2_subdev *source_subdev, > >> + struct v4l2_async_subdev *asd) > >> +{ > >> + struct ub913_data *priv = sd_to_ub913(notifier->sd); > >> + struct device *dev = &priv->client->dev; > >> + unsigned int src_pad; > >> + int ret; > >> + > >> + dev_dbg(dev, "Bind %s\n", source_subdev->name); > > > > I'd drop this message. > > Why is that? Do we get this easily from the v4l2 core? These debug > prints in the bind/unbind process have been valuable for me. Because debug messages are not meant to be a tracing infrastructure, and because, if we want to keep this message, it would be best handled in the v4l2-async core instead of being duplicated across drivers. Same for the messages at the end of the function. > >> + > >> + ret = media_entity_get_fwnode_pad(&source_subdev->entity, > >> + source_subdev->fwnode, > >> + MEDIA_PAD_FL_SOURCE); > >> + if (ret < 0) { > >> + dev_err(dev, "Failed to find pad for %s\n", > >> + source_subdev->name); > >> + return ret; > >> + } > >> + > >> + priv->source_sd = source_subdev; > >> + src_pad = ret; > >> + > >> + ret = media_create_pad_link(&source_subdev->entity, src_pad, > >> + &priv->sd.entity, 0, > > > > &priv->sd.entity, UB913_PAD_SINK, > > Yep. > > >> + MEDIA_LNK_FL_ENABLED | > >> + MEDIA_LNK_FL_IMMUTABLE); > >> + if (ret) { > >> + dev_err(dev, "Unable to link %s:%u -> %s:0\n", > >> + source_subdev->name, src_pad, priv->sd.name); > >> + return ret; > >> + } > >> + > >> + dev_dbg(dev, "Bound %s:%u\n", source_subdev->name, src_pad); > >> + > >> + dev_dbg(dev, "All subdevs bound\n"); > > > > I'd drop this message. > > > >> + > >> + return 0; > >> +} > >> + > >> +static void ub913_notify_unbind(struct v4l2_async_notifier *notifier, > >> + struct v4l2_subdev *source_subdev, > >> + struct v4l2_async_subdev *asd) > >> +{ > >> + struct ub913_data *priv = sd_to_ub913(notifier->sd); > >> + struct device *dev = &priv->client->dev; > >> + > >> + dev_dbg(dev, "Unbind %s\n", source_subdev->name); > >> +} > > > > This is a no-op so you can drop it. > > This has been useful for development, but, yes, perhaps it's time to > drop it. > > >> + > >> +static const struct v4l2_async_notifier_operations ub913_notify_ops = { > >> + .bound = ub913_notify_bound, > >> + .unbind = ub913_notify_unbind, > >> +}; [snip] -- Regards, Laurent Pinchart