Re: [PATCH v2 2/2] media: i2c: tvp7002: add OF support

[Sylwester Nawrocki]

Hi Prabhakar,

On 07/12/2013 06:43 AM, Prabhakar Lad wrote:
> On Fri, Jul 12, 2013 at 3:34 AM, Sylwester Nawrocki
> <sylvester.nawrocki@xxxxxxxxx>  wrote:
> > On 07/11/2013 07:09 PM, Prabhakar Lad wrote:
[...]
> > > > And include/media/tvp70002.h:
> > > >
> > > >    * fid_polarity:
> > > >    *                      0 ->   the field ID output is set to logic 1 for
> > > > an
> > > > odd
> > > >    *                           field (field 1) and set to logic 0 for an
> > > > even
> > > >    *                           field (field 0).
> > > >    *                      1 ->   operation with polarity inverted.
> > > >
> > > >
> > > > Do you know if the chip automatically selects video sync source
> > > > (sync-on-green
> > > > vs. VSYNC/HSYNC) and there is no need to configure this on the analogue
> > > > input
> > > > side ? At least the driver seems to always select the default SOGIN_1
> > > > input
> > > > (TVP7002_IN_MUX_SEL_1 register is set only at initialization time).
> > > Yes the driver is selecting the default SOGIN_1 input.
> > >
> > > > Or perhaps it just outputs on SOGOUT, VSOUT, HSOUT lines whatever is fed
> > > > to
> > > > its analogue inputs, and any further processing unit need to determine
> > > > what
> > > > synchronization signal is present and should be used ?
> > >
> > > Yes that correct, there is a register (Sync Detect Status) which
> > > detects the sync for you.
> > >
> > > > I suspect that we don't need, e.g. another endpoint node to specify the
> > > > configuration of the TVP7002 analogue input interface, that would contain
> > > > a property like video-sync.
> > > If I understand correctly you mean if there are two tvp7002 devices
> > > connected
> > > we don’t need to specify video-sync property, but my question how do we
> > > specify this property in common then ?
> >
> >
> > No, I thought about two port sub-nodes of a single device node, one for the
> > TVP7002 video input and one for the output. But it seems there is no need
> > for that, i.e. to specify the input configuration statically in the
> > firmware.
> > The chip detects the signals automatically, i.e. it uses whatever is
> > available,
> > and it allows querying the selection status at run time. What would really
> > need to be configured statically in DT in that case then ? Some initial
> > video
> > sync configuration ? I guess it could be well hard coded in the driver,
> > since
> > the hardware does run time detection anyway.
> Yes the chip detects the signal automatically, What I want to configure in
> the DT case is say if SOG signal is detected, I want to invert the polarity
> of it this is what I am trying to set in DT case whether to invert or not.
> 0 = Normal operation (default)
> 1 = SOG output polarity inverted
>
> Something similar to fid_polarity.

Then as I suggested earlier, let's just add 'sync-on-green-active' DT
property for that. I wouldn't expect the DT properties to be directly
replacing your driver platform_data members. Saying in the binding that
this is a normal operation and that is an inverted one is not very useful
in general, as you would need to dig in the binding's description what
"normal" exactly means. sync-on-green-active = <1>; seems much more
explicit than, e.g. sync-on-green-inverted. By looking at the
sync-on-green-active property each device driver would determine whether
it means normal or inverted operation for its device.

--
Thanks,
Sylwester
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/