Re: [PATCH 18/22] partial revert of "[media] tvp5150: add HW input connectors support"

[Marco Felsch <m.felsch@pengutronix.de>]

Hi Javier,

On 18-07-31 15:30, Marco Felsch wrote:
> Hi Javier,
> 
> On 18-07-31 14:52, Javier Martinez Canillas wrote:
> > Hi Marco,
> > 
> > On 07/31/2018 02:36 PM, Marco Felsch wrote:
> > 
> > [snip]
> > 
> > >>
> > >> Yes, another thing that patch 19/22 should take into account is DTs that
> > >> don't have input connectors defined. So probably TVP5150_PORT_YOUT should
> > >> be 0 instead of TVP5150_PORT_NUM - 1 as is the case in the current patch.
> > >>
> > >> In other words, it should work both when input connectors are defined in
> > >> the DT and when these are not defined and only an output port is defined.
> > > 
> > > Yes, it would be a approach to map the output port dynamicaly to the
> > > highest port number. I tried to keep things easy by a static mapping.
> > > Maybe a follow up patch can change this behaviour.
> > > 
> > > Anyway, input connectors aren't required. There must be at least one
> > > port child node with a correct port-number in the DT.
> > >
> > 
> > Yes, that was my point. But your patch uses the port child reg property as
> > the index for the struct device_node *endpoints[TVP5150_PORT_NUM] array.
> > 
> > If there's only one port child (for the output) then the DT binding says
> > that the reg property isn't required, so this will be 0 and your patch will
> > wrongly map it to TVP5150_PORT_AIP1A. That's why I said that the output port
> > should be the first one in your enum tvp5150_ports and not the last one.
> 
> Yes, now I got you. I implemted this in such a way in my first apporach.
> But at the moment I don't know why I changed this. Maybe to keep the
> decoder->input number in sync with the em28xx devices, which will set the
> port by the s_routing() callback.
> 
> Let me check this.

I checked it again. Your're right, it should be doable but IMHO it isn't
the right solution. I checked some drivers which use of_graph and all of
them put the output at the end. So the tvp5150 will be the only one
which maps the out put to the first pad and it isn't intuitive.

I discused it with a colleague. We think a better solution would be to fix
the v4l2-core parser code to allow a independent dt-port<->pad mapping.
Since now the pad's correspond to the port number. This mapping should
be done by a driver callback, so each driver can do it's own custom
mapping.

Regards,
Marco

> 
> Best Regards,
> Marco
> 
> > > Regards,
> > > Marco
> > > 
> > 
> > Best regards,
>