Re: [PATCH 3/3] arm64: dts: renesas: draak: Describe HDMI input

Niklas Söderlund
2018-05-14T10:11:18Z

Hi again,

On 2018-05-14 11:49:00 +0200, Niklas Söderlund wrote:
> Hi Laurent,
>
> On 2018-05-14 05:49:41 +0300, Laurent Pinchart wrote:
>
> [snip]
>
> > > > +&vin4 {
> > > > + pinctrl-0 = <&vin4_pins>;
> > > > + pinctrl-names = "default";
> > > > +
> > > > + status = "okay";
> > > > +
> > > > + ports {
> > > > + #address-cells = <1>;
> > > > + #size-cells = <0>;
> > > > +
> > > > + port@0 {
> > > > + reg = <0>;
> > > > +
> > > > + vin4_in: endpoint {
> > > > + hsync-active = <0>;
> > > > + vsync-active = <0>;
> > >
> > > Comparing this to the Gen2 bindings some properties are missing,
> > >
> > > bus-width = <24>;
> > > pclk-sample = <1>;
> > > data-active = <1>;
> > >
> > > This is not a big deal as the VIN driver don't use these properties so
> > > no functional change should come of this but still a difference.
> >
> > I think the VIN DT bindings should be updated to explicitly list the endpoint
> > properties that are mandatory, optional, or not allowed.
>
> I think it's documented as it reference video-interfaces.txt which lists
> all these properties as optional. And in deed they are all optional. If
> the VIN driver makes use of all the optional ones is another matter. How
> do we know that the remote subdevice is not looking at its remote
> endpoint for bus parameters not considered by the rcar-vin driver?
>
> The thing is that the rcar-vin driver only looks at the remote endpoint
> for these properties and ignores the on its local endpoint. Maybe some
> v4l2 framework change is needed here to make sure the bus properties are
> the same on both endpoints of a link. But I fear such a change would
> break a lot of stuff.

Jacopo pointed out this statement is untrue. The rcar-vin only looks at
it's local endpoint not the remote endpoint for it's bus parameters. The
callback provided to v4l2_async_notifier_parse_fwnode_endpoints()
confused me as the subdevice passed to it is the one describe the remote
endpoint while the v4l2_fwnode_endpoint argument is that of the local
endpoint. Sorry for the confusion and thanks Jacopo for correcting me.

--
Regards,
Niklas Söderlund