From: Maxime Ripard <maxime.ripard@free-electrons.com>
To: Hans Verkuil <hverkuil@xs4all.nl>
Cc: Yong <yong.deng@magewell.com>,
Laurent Pinchart <laurent.pinchart@ideasonboard.com>,
Mauro Carvalho Chehab <mchehab@kernel.org>,
Rob Herring <robh+dt@kernel.org>,
Mark Rutland <mark.rutland@arm.com>, Chen-Yu Tsai <wens@csie.org>,
Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
"David S. Miller" <davem@davemloft.net>,
Arnd Bergmann <arnd@arndb.de>,
Hugues Fruchet <hugues.fruchet@st.com>,
Yannick Fertre <yannick.fertre@st.com>,
Philipp Zabel <p.zabel@pengutronix.de>,
Benoit Parrot <bparrot@ti.com>,
Benjamin Gaignard <benjamin.gaignard@linaro.org>,
Jean-Christophe Trotin <jean-christophe.trotin@st.com>,
Ramesh Shanmugasundaram <ramesh.shanmugasundaram@bp.renesas.com>,
Minghsiu Tsai <minghsiu.tsai@mediatek.com>,
Krzysztof Kozlowski <krzk@kernel.org>,
Robert Jarzmik <robert.jarzmik@free.fr>,
linux-media@vger.kernel.org, devicetree@vger.kernel.org,
linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org, linux-sunxi@googlegroups.com
Subject: Re: [PATCH v2 1/3] media: V3s: Add support for Allwinner CSI.
Date: Tue, 22 Aug 2017 22:17:31 +0200 [thread overview]
Message-ID: <20170822201731.hyjqrbkhggaoomfl@flea.home> (raw)
In-Reply-To: <8dd8c350-cd45-5cd9-65cc-67102944811f@xs4all.nl>
[-- Attachment #1: Type: text/plain, Size: 2078 bytes --]
On Tue, Aug 22, 2017 at 08:43:35AM +0200, Hans Verkuil wrote:
> >>> +static int sun6i_video_link_setup(struct media_entity *entity,
> >>> + const struct media_pad *local,
> >>> + const struct media_pad *remote, u32 flags)
> >>> +{
> >>> + struct video_device *vdev = media_entity_to_video_device(entity);
> >>> + struct sun6i_video *video = video_get_drvdata(vdev);
> >>> +
> >>> + if (WARN_ON(video == NULL))
> >>> + return 0;
> >>> +
> >>> + return sun6i_video_formats_init(video);
> >>
> >> Why is this called here? Why not in video_init()?
> >
> > sun6i_video_init is in the driver probe context. sun6i_video_formats_init
> > use media_entity_remote_pad and media_entity_to_v4l2_subdev to find the
> > subdevs.
> > The media_entity_remote_pad can't work before all the media pad linked.
>
> A video_init is typically called from the notify_complete callback.
> Actually, that's where the video_register_device should be created as well.
> When you create it in probe() there is possibly no sensor yet, so it would
> be a dead video node (or worse, crash when used).
>
> There are still a lot of platform drivers that create the video node in the
> probe, but it's not the right place if you rely on the async loading of
> subdevs.
That's not really related, but I'm not really sure it's a good way to
operate. This essentially means that you might wait forever for a
component in your pipeline to be probed, without any chance of it
happening (not compiled, compiled as a module and not loaded, hardware
defect preventing the driver from probing properly, etc), even though
that component might not be essential.
This is how DRM operates, and you sometimes end up in some very dumb
situations where you wait for say, the DSI controller to probe, while
you only care about HDMI in your system.
But this seems to be on of the hot topic these days, so we might
discuss it further in some other thread :)
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]
WARNING: multiple messages have this Message-ID (diff)
From: Maxime Ripard <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
To: Hans Verkuil <hverkuil-qWit8jRvyhVmR6Xm/wNWPw@public.gmane.org>
Cc: Yong <yong.deng-+3dxTMOEIRNWk0Htik3J/w@public.gmane.org>,
Laurent Pinchart
<laurent.pinchart-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org>,
Mauro Carvalho Chehab
<mchehab-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>,
Rob Herring <robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>,
Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org>,
Chen-Yu Tsai <wens-jdAy2FN1RRM@public.gmane.org>,
Greg Kroah-Hartman
<gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org>,
"David S. Miller" <davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>,
Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>,
Hugues Fruchet <hugues.fruchet-qxv4g6HH51o@public.gmane.org>,
Yannick Fertre <yannick.fertre-qxv4g6HH51o@public.gmane.org>,
Philipp Zabel <p.zabel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>,
Benoit Parrot <bparrot-l0cyMroinI0@public.gmane.org>,
Benjamin Gaignard
<benjamin.gaignard-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>,
Jean-Christophe Trotin
<jean-christophe.trotin-qxv4g6HH51o@public.gmane.org>,
Ramesh Shanmugasundaram
<ramesh.shanmugasundaram-kTT6dE0pTRh9uiUsa/gSgQ@public.gmane.org>,
Minghsiu Tsai
<minghsiu.tsai-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>,
Krzysztof Kozlowski
<krzk-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>,
Robert Jarzmik <robert.jarzmik-GANU6spQydw@public.gmane.org>,
linux-media-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-arm-kernel@list
Subject: Re: [PATCH v2 1/3] media: V3s: Add support for Allwinner CSI.
Date: Tue, 22 Aug 2017 22:17:31 +0200 [thread overview]
Message-ID: <20170822201731.hyjqrbkhggaoomfl@flea.home> (raw)
In-Reply-To: <8dd8c350-cd45-5cd9-65cc-67102944811f-qWit8jRvyhVmR6Xm/wNWPw@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 2028 bytes --]
On Tue, Aug 22, 2017 at 08:43:35AM +0200, Hans Verkuil wrote:
> >>> +static int sun6i_video_link_setup(struct media_entity *entity,
> >>> + const struct media_pad *local,
> >>> + const struct media_pad *remote, u32 flags)
> >>> +{
> >>> + struct video_device *vdev = media_entity_to_video_device(entity);
> >>> + struct sun6i_video *video = video_get_drvdata(vdev);
> >>> +
> >>> + if (WARN_ON(video == NULL))
> >>> + return 0;
> >>> +
> >>> + return sun6i_video_formats_init(video);
> >>
> >> Why is this called here? Why not in video_init()?
> >
> > sun6i_video_init is in the driver probe context. sun6i_video_formats_init
> > use media_entity_remote_pad and media_entity_to_v4l2_subdev to find the
> > subdevs.
> > The media_entity_remote_pad can't work before all the media pad linked.
>
> A video_init is typically called from the notify_complete callback.
> Actually, that's where the video_register_device should be created as well.
> When you create it in probe() there is possibly no sensor yet, so it would
> be a dead video node (or worse, crash when used).
>
> There are still a lot of platform drivers that create the video node in the
> probe, but it's not the right place if you rely on the async loading of
> subdevs.
That's not really related, but I'm not really sure it's a good way to
operate. This essentially means that you might wait forever for a
component in your pipeline to be probed, without any chance of it
happening (not compiled, compiled as a module and not loaded, hardware
defect preventing the driver from probing properly, etc), even though
that component might not be essential.
This is how DRM operates, and you sometimes end up in some very dumb
situations where you wait for say, the DSI controller to probe, while
you only care about HDMI in your system.
But this seems to be on of the hot topic these days, so we might
discuss it further in some other thread :)
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
WARNING: multiple messages have this Message-ID (diff)
From: maxime.ripard@free-electrons.com (Maxime Ripard)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH v2 1/3] media: V3s: Add support for Allwinner CSI.
Date: Tue, 22 Aug 2017 22:17:31 +0200 [thread overview]
Message-ID: <20170822201731.hyjqrbkhggaoomfl@flea.home> (raw)
In-Reply-To: <8dd8c350-cd45-5cd9-65cc-67102944811f@xs4all.nl>
On Tue, Aug 22, 2017 at 08:43:35AM +0200, Hans Verkuil wrote:
> >>> +static int sun6i_video_link_setup(struct media_entity *entity,
> >>> + const struct media_pad *local,
> >>> + const struct media_pad *remote, u32 flags)
> >>> +{
> >>> + struct video_device *vdev = media_entity_to_video_device(entity);
> >>> + struct sun6i_video *video = video_get_drvdata(vdev);
> >>> +
> >>> + if (WARN_ON(video == NULL))
> >>> + return 0;
> >>> +
> >>> + return sun6i_video_formats_init(video);
> >>
> >> Why is this called here? Why not in video_init()?
> >
> > sun6i_video_init is in the driver probe context. sun6i_video_formats_init
> > use media_entity_remote_pad and media_entity_to_v4l2_subdev to find the
> > subdevs.
> > The media_entity_remote_pad can't work before all the media pad linked.
>
> A video_init is typically called from the notify_complete callback.
> Actually, that's where the video_register_device should be created as well.
> When you create it in probe() there is possibly no sensor yet, so it would
> be a dead video node (or worse, crash when used).
>
> There are still a lot of platform drivers that create the video node in the
> probe, but it's not the right place if you rely on the async loading of
> subdevs.
That's not really related, but I'm not really sure it's a good way to
operate. This essentially means that you might wait forever for a
component in your pipeline to be probed, without any chance of it
happening (not compiled, compiled as a module and not loaded, hardware
defect preventing the driver from probing properly, etc), even though
that component might not be essential.
This is how DRM operates, and you sometimes end up in some very dumb
situations where you wait for say, the DSI controller to probe, while
you only care about HDMI in your system.
But this seems to be on of the hot topic these days, so we might
discuss it further in some other thread :)
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20170822/5f9d397b/attachment.sig>
next prev parent reply other threads:[~2017-08-22 20:17 UTC|newest]
Thread overview: 159+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-07-27 5:01 [PATCH v2 0/3] Initial Allwinner V3s CSI Support Yong Deng
2017-07-27 5:01 ` Yong Deng
2017-07-27 5:01 ` Yong Deng
2017-07-27 5:01 ` [PATCH v2 1/3] media: V3s: Add support for Allwinner CSI Yong Deng
2017-07-27 5:01 ` Yong Deng
2017-07-27 5:01 ` Yong Deng
2017-07-27 12:16 ` Baruch Siach
2017-07-27 12:16 ` Baruch Siach
2017-07-27 12:16 ` Baruch Siach
2017-07-27 12:25 ` Maxime Ripard
2017-07-27 12:25 ` Maxime Ripard
2017-07-27 12:25 ` Maxime Ripard
2017-07-31 0:47 ` Yong
2017-07-31 0:47 ` Yong
2017-07-31 0:47 ` Yong
2017-07-28 16:02 ` Maxime Ripard
2017-07-28 16:02 ` Maxime Ripard
2017-07-28 16:02 ` Maxime Ripard
2017-07-30 6:08 ` Baruch Siach
2017-07-30 6:08 ` Baruch Siach
2017-07-30 6:08 ` Baruch Siach
2017-07-31 1:48 ` Yong
2017-07-31 1:48 ` Yong
2017-07-31 1:48 ` Yong
2017-07-31 5:13 ` Baruch Siach
2017-07-31 5:13 ` Baruch Siach
2017-07-31 5:13 ` Baruch Siach
2017-08-21 20:21 ` Maxime Ripard
2017-08-21 20:21 ` Maxime Ripard
2017-08-21 20:21 ` Maxime Ripard
2017-08-23 2:41 ` Yong
2017-08-23 2:41 ` Yong
2017-08-23 2:41 ` Yong
2017-08-23 19:24 ` Maxime Ripard
2017-08-23 19:24 ` Maxime Ripard
2017-08-23 19:24 ` Maxime Ripard
2017-08-24 1:43 ` Yong
2017-08-24 1:43 ` Yong
2017-08-24 1:43 ` Yong
2017-07-31 3:16 ` Yong
2017-07-31 3:16 ` Yong
2017-07-31 3:16 ` Yong
2017-08-22 17:43 ` Maxime Ripard
2017-08-22 17:43 ` Maxime Ripard
2017-08-22 17:43 ` Maxime Ripard
2017-08-23 2:32 ` Yong
2017-08-23 2:32 ` Yong
2017-08-23 2:32 ` Yong
2017-08-25 13:41 ` Maxime Ripard
2017-08-25 13:41 ` Maxime Ripard
2017-08-25 13:41 ` Maxime Ripard
2017-08-28 7:00 ` Yong
2017-08-28 7:00 ` Yong
2017-08-28 7:00 ` Yong
2017-08-21 14:37 ` Hans Verkuil
2017-08-21 14:37 ` Hans Verkuil
2017-08-21 14:37 ` Hans Verkuil
2017-08-22 3:01 ` Yong
2017-08-22 3:01 ` Yong
2017-08-22 3:01 ` Yong
2017-08-22 6:43 ` Hans Verkuil
2017-08-22 6:43 ` Hans Verkuil
2017-08-22 6:43 ` Hans Verkuil
2017-08-22 7:51 ` Yong
2017-08-22 7:51 ` Yong
2017-08-22 7:51 ` Yong
2017-08-22 20:17 ` Maxime Ripard [this message]
2017-08-22 20:17 ` Maxime Ripard
2017-08-22 20:17 ` Maxime Ripard
2017-08-22 20:52 ` Laurent Pinchart
2017-08-22 20:52 ` Laurent Pinchart
2017-08-22 20:52 ` Laurent Pinchart
2017-08-23 6:52 ` Hans Verkuil
2017-08-23 6:52 ` Hans Verkuil
2017-08-23 6:52 ` Hans Verkuil
2017-08-23 7:43 ` Laurent Pinchart
2017-08-23 7:43 ` Laurent Pinchart
2017-08-23 7:43 ` Laurent Pinchart
2017-08-23 11:13 ` icenowy
2017-08-23 11:13 ` icenowy at aosc.io
2017-08-23 11:13 ` icenowy
2017-09-21 13:45 ` [linux-sunxi] " Ondřej Jirman
2017-09-21 13:45 ` Ondřej Jirman
2017-09-21 13:45 ` 'Ondřej Jirman' via linux-sunxi
2017-09-22 0:57 ` 回复：[linux-sunxi] " 邓永
2017-09-22 8:44 ` Mylene JOSSERAND
2017-09-22 8:44 ` Mylene JOSSERAND
2017-09-22 8:44 ` Mylene JOSSERAND
2017-09-22 9:08 ` Yong
2017-09-22 9:08 ` Yong
2017-09-22 9:08 ` Yong
2017-11-21 15:48 ` Maxime Ripard
2017-11-21 15:48 ` Maxime Ripard
2017-11-21 15:48 ` Maxime Ripard
2017-11-22 1:33 ` Yong
2017-11-22 1:33 ` Yong
2017-11-22 1:33 ` Yong
2017-11-22 9:45 ` Maxime Ripard
2017-11-22 9:45 ` Maxime Ripard
2017-11-22 9:45 ` Maxime Ripard
2017-11-23 1:14 ` Yong
2017-11-23 1:14 ` Yong
2017-11-23 1:14 ` Yong
2017-11-25 16:02 ` Maxime Ripard
2017-11-25 16:02 ` Maxime Ripard
2017-11-25 16:02 ` Maxime Ripard
2017-12-04 9:45 ` Yong
2017-12-04 9:45 ` Yong
2017-12-04 9:45 ` Yong
2017-12-15 10:50 ` Maxime Ripard
2017-12-15 10:50 ` Maxime Ripard
2017-12-15 10:50 ` Maxime Ripard
2017-12-15 11:01 ` Yong
2017-12-15 11:01 ` Yong
2017-12-15 11:01 ` Yong
2017-12-15 22:14 ` Maxime Ripard
2017-12-15 22:14 ` Maxime Ripard
2017-12-15 22:14 ` Maxime Ripard
2017-07-27 5:01 ` [PATCH v2 2/3] dt-bindings: media: Add Allwinner V3s Camera Sensor Interface (CSI) Yong Deng
2017-07-27 5:01 ` Yong Deng
2017-07-27 5:01 ` Yong Deng
2017-07-28 16:03 ` Maxime Ripard
2017-07-28 16:03 ` Maxime Ripard
2017-07-28 16:03 ` Maxime Ripard
2017-07-31 0:50 ` Yong
2017-07-31 0:50 ` Yong
2017-07-31 0:50 ` Yong
2017-08-03 19:14 ` Rob Herring
2017-08-03 19:14 ` Rob Herring
2017-08-03 19:14 ` Rob Herring
2017-08-07 1:00 ` Yong
2017-08-07 1:00 ` Yong
2017-08-07 1:00 ` Yong
2017-12-19 11:53 ` Sakari Ailus
2017-12-19 11:53 ` Sakari Ailus
2017-12-19 11:53 ` Sakari Ailus
2017-12-21 2:49 ` Yong
2017-12-21 2:49 ` Yong
2017-12-21 2:49 ` Yong
2017-12-27 21:47 ` Sakari Ailus
2017-12-27 21:47 ` Sakari Ailus
2017-12-27 21:47 ` Sakari Ailus
2017-12-28 1:04 ` Yong
2017-12-28 1:04 ` Yong
2017-12-28 1:04 ` Yong
2017-07-27 5:01 ` [PATCH v2 3/3] media: MAINTAINERS: add entries for Allwinner V3s CSI Yong Deng
2017-07-27 5:01 ` Yong Deng
2017-07-27 5:01 ` Yong Deng
2017-12-19 11:48 ` Sakari Ailus
2017-12-19 11:48 ` Sakari Ailus
2017-12-19 11:48 ` Sakari Ailus
2017-12-21 2:40 ` Yong
2017-12-21 2:40 ` Yong
2017-12-21 2:40 ` Yong
-- strict thread matches above, loose matches on Subject: below --
2017-07-27 3:51 [PATCH v2 0/3] Initial Allwinner V3s CSI Support Yong Deng
2017-07-27 3:51 ` [PATCH v2 1/3] media: V3s: Add support for Allwinner CSI Yong Deng
2017-07-27 3:51 ` Yong Deng
2017-07-27 3:51 ` Yong Deng
[not found] ` <1501127474-40895-1-git-send-email-yong.deng-+3dxTMOEIRNWk0Htik3J/w@public.gmane.org>
2017-07-27 3:51 ` Yong Deng
2017-07-27 3:51 ` Yong Deng
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=20170822201731.hyjqrbkhggaoomfl@flea.home \
--to=maxime.ripard@free-electrons.com \
--cc=arnd@arndb.de \
--cc=benjamin.gaignard@linaro.org \
--cc=bparrot@ti.com \
--cc=davem@davemloft.net \
--cc=devicetree@vger.kernel.org \
--cc=gregkh@linuxfoundation.org \
--cc=hugues.fruchet@st.com \
--cc=hverkuil@xs4all.nl \
--cc=jean-christophe.trotin@st.com \
--cc=krzk@kernel.org \
--cc=laurent.pinchart@ideasonboard.com \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-media@vger.kernel.org \
--cc=linux-sunxi@googlegroups.com \
--cc=mark.rutland@arm.com \
--cc=mchehab@kernel.org \
--cc=minghsiu.tsai@mediatek.com \
--cc=p.zabel@pengutronix.de \
--cc=ramesh.shanmugasundaram@bp.renesas.com \
--cc=robert.jarzmik@free.fr \
--cc=robh+dt@kernel.org \
--cc=wens@csie.org \
--cc=yannick.fertre@st.com \
--cc=yong.deng@magewell.com \
/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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.