Hi guys,
On Fri, Feb 23, 2018 at 12:05:38PM +0200, Laurent Pinchart wrote:
> Hi Philipp,
>
> On Friday, 23 February 2018 11:56:52 EET Philipp Zabel wrote:
> > On Fri, 2018-02-23 at 11:29 +0200, Laurent Pinchart wrote:
> > > On Thursday, 22 February 2018 03:39:37 EET Steve Longerbeam wrote:
> > >> For some subdevices, a fwnode endpoint that has no connection to a
> > >> remote endpoint may not be an error. Let the parse_endpoint callback
> > > make that decision in v4l2_async_notifier_fwnode_parse_endpoint(). If
> > >> the callback indicates that is not an error, skip adding the asd to the
> > >> notifier and return 0.
> > >>
> > >> For the current users of v4l2_async_notifier_parse_fwnode_endpoints()
> > >> (omap3isp, rcar-vin, intel-ipu3), return -EINVAL in the callback for
> > >> unavailable remote fwnodes to maintain the previous behavior.
> > >
> > > I'm not sure this should be a per-driver decision.
> > >
> > > Generally speaking, if an endpoint node has no remote-endpoint property,
> > > the endpoint node is not needed. I've always considered such an endpoint
> > > node as invalid. The OF graphs DT bindings are however not clear on this
> > > subject.
> >
> > Documentation/devicetree/bindings/graph.txt says:
> >
> > Each endpoint should contain a 'remote-endpoint' phandle property
> > that points to the corresponding endpoint in the port of the remote
> > device.
> >
> > ("should", not "must").
>
> The DT bindings documentation has historically used "should" to mean "must" in
> many places :-( That was a big mistake.
"Shall", not "must".
Indeed, there's hardly use for an endpoint without the remote-endpoint
property. My understanding is that such nodes might be best ignored, that's
been at least the practice in a lot of DT parsing code. There are arguments
both ways I guess.
What comes to this patch is that I'd rather not burden individual drivers
with such checks.
>
> > Later, the remote-node property explicitly lists the remote-endpoint
> > property as optional.
>
> I've seen that too, and that's why I mentioned that the documentation isn't
> clear on the subject.
>
> > > I have either failed to notice when they got merged, or they slowly
> > > evolved over time to contain contradictory information. In any case, I
> > > think we should decide on whether such a situation is valid or not from
> > > an OF graph point of view, and then always reject or always accept and
> > > ignore those endpoints.
> >
> > We are currently using this on i.MX6 to provide empty labeled endpoints
> > in the dtsi files for board DT writers to link to, both for the display
> > output and video capture ports.
> > See for example the endpoints with the labels ipu1_di0_disp0 and
> > ipu1_csi0_mux_from_parallel_sensor in arch/arm/boot/dts/imx6q.dtsi.
>
> This could also be achieved by adding the endpoints in the board DT files. See
> for instance the hdmi@fead0000 node in arch/arm64/boot/dts/renesas/
> r8a7795.dtsi and how it gets extended in arch/arm64/boot/dts/renesas/r8a7795-
> salvator-x.dts. On the other hand, I also have empty endpoints in the
> display@feb00000 node of arch/arm64/boot/dts/renesas/r8a7795.dtsi.
>
> I think we should first decide what we want to do going forward (allowing for
> empty endpoints or not), clarify the documentation, and then update the code.
> In any case I don't think it should be a per-device decision.
I don't think they should be allowed in the documentation. The
implementation could still simply ignore them.
Cc the devicetree list.
--
Regards,
Sakari Ailus
e-mail: sakari.ailus@xxxxxx
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
