From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
To: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
Cc: linux-media@vger.kernel.org, devicetree@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-i2c@vger.kernel.org,
"Rob Herring" <robh+dt@kernel.org>,
"Krzysztof Kozlowski" <krzysztof.kozlowski+dt@linaro.org>,
"Wolfram Sang" <wsa@kernel.org>,
"Luca Ceresoli" <luca.ceresoli@bootlin.com>,
"Andy Shevchenko" <andriy.shevchenko@intel.com>,
"Matti Vaittinen" <Matti.Vaittinen@fi.rohmeurope.com>,
"Mauro Carvalho Chehab" <mchehab@kernel.org>,
"Peter Rosin" <peda@axentia.se>,
"Liam Girdwood" <lgirdwood@gmail.com>,
"Mark Brown" <broonie@kernel.org>,
"Sakari Ailus" <sakari.ailus@linux.intel.com>,
"Michael Tretter" <m.tretter@pengutronix.de>,
"Shawn Tu" <shawnx.tu@intel.com>,
"Hans Verkuil" <hverkuil@xs4all.nl>,
"Mike Pagano" <mpagano@gentoo.org>,
"Krzysztof Hałasa" <khalasa@piap.pl>,
"Marek Vasut" <marex@denx.de>
Subject: Re: [PATCH v5 7/8] media: i2c: add DS90UB913 driver
Date: Mon, 26 Dec 2022 19:01:11 +0200 [thread overview]
Message-ID: <Y6nTV16me9aQL3iT@pendragon.ideasonboard.com> (raw)
In-Reply-To: <4d349785-ca37-d930-db3c-2581bba9fde0@ideasonboard.com>
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 <tomi.valkeinen@ideasonboard.com>
> >> ---
> >> 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
next prev parent reply other threads:[~2022-12-26 17:01 UTC|newest]
Thread overview: 57+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-12-08 10:39 [PATCH v5 0/8] i2c-atr and FPDLink Tomi Valkeinen
2022-12-08 10:39 ` [PATCH v5 1/8] i2c: core: let adapters be notified of client attach/detach Tomi Valkeinen
2022-12-08 12:30 ` Andy Shevchenko
2022-12-08 16:10 ` Tomi Valkeinen
2022-12-11 16:55 ` Laurent Pinchart
2022-12-19 8:51 ` Luca Ceresoli
2022-12-19 9:48 ` Andy Shevchenko
2022-12-26 16:54 ` Laurent Pinchart
2022-12-27 20:07 ` Andy Shevchenko
2022-12-08 10:40 ` [PATCH v5 2/8] i2c: add I2C Address Translator (ATR) support Tomi Valkeinen
2022-12-08 12:53 ` Andy Shevchenko
2022-12-08 16:01 ` Tomi Valkeinen
2022-12-08 18:09 ` Andy Shevchenko
2022-12-08 10:40 ` [PATCH v5 3/8] dt-bindings: media: add bindings for TI DS90UB913 Tomi Valkeinen
2022-12-11 17:13 ` Laurent Pinchart
2022-12-11 17:21 ` Laurent Pinchart
2022-12-13 13:36 ` Tomi Valkeinen
2022-12-26 16:46 ` Laurent Pinchart
2023-01-04 8:12 ` Tomi Valkeinen
2023-01-04 12:52 ` Laurent Pinchart
2022-12-13 13:21 ` Tomi Valkeinen
2022-12-08 10:40 ` [PATCH v5 4/8] dt-bindings: media: add bindings for TI DS90UB953 Tomi Valkeinen
2022-12-09 21:27 ` Rob Herring
2023-01-04 8:26 ` Tomi Valkeinen
2022-12-11 17:34 ` Laurent Pinchart
2022-12-13 14:06 ` Tomi Valkeinen
2022-12-08 10:40 ` [PATCH v5 5/8] dt-bindings: media: add bindings for TI DS90UB960 Tomi Valkeinen
2022-12-09 21:30 ` Rob Herring
2022-12-11 17:58 ` Laurent Pinchart
2022-12-13 14:25 ` Tomi Valkeinen
2022-12-26 16:52 ` Laurent Pinchart
2023-01-04 8:59 ` Tomi Valkeinen
2023-01-04 12:57 ` Laurent Pinchart
2023-01-04 14:05 ` Tomi Valkeinen
2023-01-05 6:54 ` Laurent Pinchart
2022-12-08 10:40 ` [PATCH v5 6/8] media: i2c: add DS90UB960 driver Tomi Valkeinen
2022-12-08 10:40 ` [PATCH v5 7/8] media: i2c: add DS90UB913 driver Tomi Valkeinen
2022-12-11 18:33 ` Laurent Pinchart
2022-12-14 6:29 ` Tomi Valkeinen
2022-12-14 6:36 ` Tomi Valkeinen
2022-12-26 16:56 ` Laurent Pinchart
2022-12-26 19:25 ` Tomi Valkeinen
2023-01-04 13:55 ` Laurent Pinchart
2023-01-04 14:13 ` Tomi Valkeinen
2023-01-04 15:32 ` Laurent Pinchart
2023-01-04 15:43 ` Tomi Valkeinen
2022-12-26 17:01 ` Laurent Pinchart [this message]
2022-12-27 20:09 ` Andy Shevchenko
2023-01-04 13:29 ` Laurent Pinchart
2022-12-14 6:48 ` Tomi Valkeinen
2022-12-08 10:40 ` [PATCH v5 8/8] media: i2c: add DS90UB953 driver Tomi Valkeinen
2022-12-08 10:42 ` [PATCH v5 0/8] i2c-atr and FPDLink Tomi Valkeinen
2022-12-08 12:26 ` Andy Shevchenko
2022-12-08 14:40 ` Tomi Valkeinen
2022-12-08 15:57 ` Andy Shevchenko
2022-12-08 15:58 ` Andy Shevchenko
2022-12-08 16:05 ` Tomi Valkeinen
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=Y6nTV16me9aQL3iT@pendragon.ideasonboard.com \
--to=laurent.pinchart@ideasonboard.com \
--cc=Matti.Vaittinen@fi.rohmeurope.com \
--cc=andriy.shevchenko@intel.com \
--cc=broonie@kernel.org \
--cc=devicetree@vger.kernel.org \
--cc=hverkuil@xs4all.nl \
--cc=khalasa@piap.pl \
--cc=krzysztof.kozlowski+dt@linaro.org \
--cc=lgirdwood@gmail.com \
--cc=linux-i2c@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-media@vger.kernel.org \
--cc=luca.ceresoli@bootlin.com \
--cc=m.tretter@pengutronix.de \
--cc=marex@denx.de \
--cc=mchehab@kernel.org \
--cc=mpagano@gentoo.org \
--cc=peda@axentia.se \
--cc=robh+dt@kernel.org \
--cc=sakari.ailus@linux.intel.com \
--cc=shawnx.tu@intel.com \
--cc=tomi.valkeinen@ideasonboard.com \
--cc=wsa@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).